Make Events immutable (avoid CMEs and fix non-deterministic behavior)

Description

A Drools rule caught a ConcurrentModificationException while marshaling an event using JaxbUtils.marshal(Event event).

No other Drools engines are subscribed to this UEI, and its logmsg.dest is donotpersist, so nothing else should be modifying the event.

Note that the Event.toString() method in the catch statement did not throw an exception.

The rule in question:

rule "CloneNewEvents" salience 999999 lock-on-active when $event : Event($uei : uei) then String logTag = "nodeProvisioning:cloneEvent ::"; LOG.debug("{} Deleting and cloning event with uei {}", logTag, $uei); delete($event); try { String strEvent = JaxbUtils.marshal($event); Event clonedEvent = JaxbUtils.unmarshal(Event.class, Sanitation.stripNonValidXMLCharactersByRegex(strEvent)); insert(clonedEvent); } catch (Exception ex) { LOG.error("{} Exception caught while cloning event {}:\n", logTag, $event.toString(), ex); } end

The exception, with the event's details redacted:

2019-05-29 05:16:47,723 ERROR [DroolsCorrelationEngine-Notify-Thread] D.Engine: nodeProvisioning:cloneEvent :: Exception caught while cloning event [[ JSONified event ]]: org.opennms.core.xml.MarshallingResourceFailureException: Failed to marshal/unmarshal XML file while marshalling Event: javax.xml.bind.MarshalException - with linked exception: [java.util.ConcurrentModificationException]; nested exception is javax.xml.bind.MarshalException - with linked exception: [java.util.ConcurrentModificationException] at org.opennms.core.xml.MarshallingExceptionTranslator.translate(MarshallingExceptionTranslator.java:61) ~[org.opennms.core.xml-2018.1.8.jar:?] at org.opennms.core.xml.JaxbUtils.marshal(JaxbUtils.java:182) ~[org.opennms.core.xml-2018.1.8.jar:?] at org.opennms.core.xml.JaxbUtils.marshal(JaxbUtils.java:117) ~[org.opennms.core.xml-2018.1.8.jar:?] at org.opennms.netmgt.correlation.drools.Rule_CloneNewEvents777744953.defaultConsequence(Rule_CloneNewEvents777744953.java:11) [drools-core-7.7.0.Final.jar:?] at org.opennms.netmgt.correlation.drools.Rule_CloneNewEvents777744953DefaultConsequenceInvokerGenerated.evaluate(Unknown Source) [?:?] at org.opennms.netmgt.correlation.drools.Rule_CloneNewEvents777744953DefaultConsequenceInvoker.evaluate(Unknown Source) [drools-core-7.7.0.Final.jar:?] at org.drools.core.phreak.RuleExecutor.innerFireActivation(RuleExecutor.java:431) [drools-core-7.7.0.Final.jar:7.7.0.Final] at org.drools.core.phreak.RuleExecutor.fireActivation(RuleExecutor.java:379) [drools-core-7.7.0.Final.jar:7.7.0.Final] at org.drools.core.phreak.RuleExecutor.fire(RuleExecutor.java:135) [drools-core-7.7.0.Final.jar:7.7.0.Final] at org.drools.core.phreak.RuleExecutor.evaluateNetworkAndFire(RuleExecutor.java:88) [drools-core-7.7.0.Final.jar:7.7.0.Final] at org.drools.core.concurrent.AbstractRuleEvaluator.internalEvaluateAndFire(AbstractRuleEvaluator.java:34) [drools-core-7.7.0.Final.jar:7.7.0.Final] at org.drools.core.concurrent.SequentialRuleEvaluator.evaluateAndFire(SequentialRuleEvaluator.java:43) [drools-core-7.7.0.Final.jar:7.7.0.Final] at org.drools.core.common.DefaultAgenda.fireLoop(DefaultAgenda.java:1067) [drools-core-7.7.0.Final.jar:7.7.0.Final] at org.drools.core.common.DefaultAgenda.internalFireAllRules(DefaultAgenda.java:1014) [drools-core-7.7.0.Final.jar:7.7.0.Final] at org.drools.core.common.DefaultAgenda.fireAllRules(DefaultAgenda.java:1006) [drools-core-7.7.0.Final.jar:7.7.0.Final] at org.drools.core.impl.StatefulKnowledgeSessionImpl.internalFireAllRules(StatefulKnowledgeSessionImpl.java:1308) [drools-core-7.7.0.Final.jar:7.7.0.Final] at org.drools.core.impl.StatefulKnowledgeSessionImpl.fireAllRules(StatefulKnowledgeSessionImpl.java:1299) [drools-core-7.7.0.Final.jar:7.7.0.Final] at org.drools.core.impl.StatefulKnowledgeSessionImpl.fireAllRules(StatefulKnowledgeSessionImpl.java:1283) [drools-core-7.7.0.Final.jar:7.7.0.Final] at org.opennms.netmgt.correlation.drools.DroolsCorrelationEngine.correlate(DroolsCorrelationEngine.java:136) [drools-correlation-engine-2018.1.8.jar:?] at org.opennms.netmgt.correlation.Correlator$EngineAdapter.onEvent(Correlator.java:91) [opennms-correlator-2018.1.8.jar:?] at org.opennms.netmgt.eventd.EventIpcManagerDefaultImpl$EventListenerExecutor$2.run(EventIpcManagerDefaultImpl.java:189) [org.opennms.features.events.daemon-2018.1.8.jar:?] at java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1626) [?:1.8.0_191] at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) [?:1.8.0_191] at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) [?:1.8.0_191] at org.opennms.core.concurrent.LogPreservingThreadFactory$2.run(LogPreservingThreadFactory.java:106) [opennms-util-2018.1.8.jar:?] at java.lang.Thread.run(Thread.java:748) [?:1.8.0_191] Caused by: javax.xml.bind.MarshalException at org.eclipse.persistence.jaxb.JAXBMarshaller.marshal(JAXBMarshaller.java:686) ~[org.eclipse.persistence.moxy-2.5.1.jar:?] at org.opennms.core.xml.JaxbUtils.marshal(JaxbUtils.java:180) ~[org.opennms.core.xml-2018.1.8.jar:?] ... 24 more Caused by: java.util.ConcurrentModificationException at java.util.ArrayList$Itr.checkForComodification(ArrayList.java:909) ~[?:1.8.0_191] at java.util.ArrayList$Itr.next(ArrayList.java:859) ~[?:1.8.0_191] at org.eclipse.persistence.internal.queries.InterfaceContainerPolicy.next(InterfaceContainerPolicy.java:274) ~[org.eclipse.persistence.core-2.5.1.jar:?] at org.eclipse.persistence.internal.queries.ContainerPolicy.next(ContainerPolicy.java:1166) ~[org.eclipse.persistence.core-2.5.1.jar:?] at org.eclipse.persistence.internal.queries.ContainerPolicy.next(ContainerPolicy.java:1) ~[org.eclipse.persistence.core-2.5.1.jar:?] at org.eclipse.persistence.internal.oxm.XMLCompositeCollectionMappingNodeValue.marshal(XMLCompositeCollectionMappingNodeValue.java:107) ~[org.eclipse.persistence.core-2.5.1.jar:?] at org.eclipse.persistence.internal.oxm.NodeValue.marshal(NodeValue.java:149) ~[org.eclipse.persistence.core-2.5.1.jar:?] at org.eclipse.persistence.internal.oxm.NodeValue.marshal(NodeValue.java:102) ~[org.eclipse.persistence.core-2.5.1.jar:?] at org.eclipse.persistence.internal.oxm.record.ObjectMarshalContext.marshal(ObjectMarshalContext.java:59) ~[org.eclipse.persistence.core-2.5.1.jar:?] at org.eclipse.persistence.internal.oxm.XPathNode.marshal(XPathNode.java:401) ~[org.eclipse.persistence.core-2.5.1.jar:?] at org.eclipse.persistence.internal.oxm.XPathNode.marshal(XPathNode.java:382) ~[org.eclipse.persistence.core-2.5.1.jar:?] at org.eclipse.persistence.internal.oxm.XPathObjectBuilder.buildRow(XPathObjectBuilder.java:240) ~[org.eclipse.persistence.core-2.5.1.jar:?] at org.eclipse.persistence.internal.oxm.TreeObjectBuilder.buildRow(TreeObjectBuilder.java:118) ~[org.eclipse.persistence.core-2.5.1.jar:?] at org.eclipse.persistence.internal.oxm.TreeObjectBuilder.buildRow(TreeObjectBuilder.java:1) ~[org.eclipse.persistence.core-2.5.1.jar:?] at org.eclipse.persistence.internal.oxm.XMLMarshaller.marshal(XMLMarshaller.java:751) ~[org.eclipse.persistence.core-2.5.1.jar:?] at org.eclipse.persistence.internal.oxm.XMLMarshaller.marshalStreamOrWriter(XMLMarshaller.java:1128) ~[org.eclipse.persistence.core-2.5.1.jar:?] at org.eclipse.persistence.internal.oxm.XMLMarshaller.marshal(XMLMarshaller.java:1079) ~[org.eclipse.persistence.core-2.5.1.jar:?] at org.eclipse.persistence.internal.oxm.XMLMarshaller.marshal(XMLMarshaller.java:1034) ~[org.eclipse.persistence.core-2.5.1.jar:?] at org.eclipse.persistence.jaxb.JAXBMarshaller.marshal(JAXBMarshaller.java:684) ~[org.eclipse.persistence.moxy-2.5.1.jar:?] at org.opennms.core.xml.JaxbUtils.marshal(JaxbUtils.java:180) ~[org.opennms.core.xml-2018.1.8.jar:?] ... 24 more

Environment

RHEL 6.10 x86_64 Oracle JDK 1.8.0_191

Acceptance / Success Criteria

None

Lucidchart Diagrams

Activity

Jesse White November 7, 2019 at 9:36 PM

We can define a new mutable event data structure for your use case and make it easy to clone, or map to/from immutable events.

What's important here is that once a event is sent to eventd to be persisted/broadcasted, it must not change - there's been way too many bugs and hard to find problems because of this.

Will Keaney November 7, 2019 at 9:25 PM

I've been thinking about this over the past few days, actually.
From a Drools POV, it's very useful to be able to modify a fact without destroying the old object and creating a new one. As discussed in https://opennms.atlassian.net/browse/NMS-12367#icft=NMS-12367, deleting/inserting facts for every modification has a significant performance impact.
But at the end, there needs to be a way to convert the fully enriched and refined fact back into an Event.

I've thought about using EventBuilder's EventBuilder(final Event event) method to clone the Event into a mutable object, updating it with the EventBuilder getters and setters, and finally pulling the updated clone out with getEvent(). That might work for many of the other components that pass Events back and forth, but it wouldn't help with the Drools performance case I mentioned earlier.

Jesse White November 7, 2019 at 8:53 PM
Edited

We recently encountered another problem that would not of happened if the events were immutable. See https://opennms.atlassian.net/browse/NMS-12390#icft=NMS-12390.

Jesse White July 31, 2019 at 3:55 PM

I think that making Event immutable would be the right way to solve this and prevent errors like these going forward.

Success criteria: Events received by EventListener and sent to an EventForwarder must be immutable. A mapper must also be provided that allows these to be converted to/from JAXB annotated objects (which may be mutable).

Fixed

Details

Assignee

Reporter

Sprint

Fix versions

Affects versions

Priority

PagerDuty

Created May 29, 2019 at 2:05 PM
Updated January 22, 2020 at 2:47 PM
Resolved January 22, 2020 at 2:46 PM

Flag notifications