From: Moiz Raja Date: Fri, 29 Aug 2014 22:58:44 +0000 (-0700) Subject: BUG 1595 - Clustering : NPE on startup X-Git-Tag: release/helium~139^2 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=56faf3cea4400b35fde6686c4ab4a3bdb0da8fdc BUG 1595 - Clustering : NPE on startup This seemed to be happening because of recovery where the topology manager was trying to add something into the datastore which was already present in the journal. The fix was to add a timestamp to each modification so that they do not appear to be the same even if their content was exactly the same. Change-Id: I2d5c98bd87ded7c8b64b8cebf757bfa073327061 Signed-off-by: Moiz Raja --- diff --git a/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/protobuff/messages/persistent/PersistentMessages.java b/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/protobuff/messages/persistent/PersistentMessages.java index fcf1f45bc2..c867fce75d 100644 --- a/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/protobuff/messages/persistent/PersistentMessages.java +++ b/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/protobuff/messages/persistent/PersistentMessages.java @@ -926,6 +926,16 @@ public final class PersistentMessages { */ org.opendaylight.controller.protobuff.messages.persistent.PersistentMessages.ModificationOrBuilder getModificationOrBuilder( int index); + + // optional int64 timeStamp = 2; + /** + * optional int64 timeStamp = 2; + */ + boolean hasTimeStamp(); + /** + * optional int64 timeStamp = 2; + */ + long getTimeStamp(); } /** * Protobuf type {@code org.opendaylight.controller.mdsal.CompositeModification} @@ -986,6 +996,11 @@ public final class PersistentMessages { modification_.add(input.readMessage(org.opendaylight.controller.protobuff.messages.persistent.PersistentMessages.Modification.PARSER, extensionRegistry)); break; } + case 16: { + bitField0_ |= 0x00000001; + timeStamp_ = input.readInt64(); + break; + } } } } catch (com.google.protobuf.InvalidProtocolBufferException e) { @@ -1028,6 +1043,7 @@ public final class PersistentMessages { return PARSER; } + private int bitField0_; // repeated .org.opendaylight.controller.mdsal.Modification modification = 1; public static final int MODIFICATION_FIELD_NUMBER = 1; private java.util.List modification_; @@ -1064,8 +1080,25 @@ public final class PersistentMessages { return modification_.get(index); } + // optional int64 timeStamp = 2; + public static final int TIMESTAMP_FIELD_NUMBER = 2; + private long timeStamp_; + /** + * optional int64 timeStamp = 2; + */ + public boolean hasTimeStamp() { + return ((bitField0_ & 0x00000001) == 0x00000001); + } + /** + * optional int64 timeStamp = 2; + */ + public long getTimeStamp() { + return timeStamp_; + } + private void initFields() { modification_ = java.util.Collections.emptyList(); + timeStamp_ = 0L; } private byte memoizedIsInitialized = -1; public final boolean isInitialized() { @@ -1088,6 +1121,9 @@ public final class PersistentMessages { for (int i = 0; i < modification_.size(); i++) { output.writeMessage(1, modification_.get(i)); } + if (((bitField0_ & 0x00000001) == 0x00000001)) { + output.writeInt64(2, timeStamp_); + } getUnknownFields().writeTo(output); } @@ -1101,6 +1137,10 @@ public final class PersistentMessages { size += com.google.protobuf.CodedOutputStream .computeMessageSize(1, modification_.get(i)); } + if (((bitField0_ & 0x00000001) == 0x00000001)) { + size += com.google.protobuf.CodedOutputStream + .computeInt64Size(2, timeStamp_); + } size += getUnknownFields().getSerializedSize(); memoizedSerializedSize = size; return size; @@ -1224,6 +1264,8 @@ public final class PersistentMessages { } else { modificationBuilder_.clear(); } + timeStamp_ = 0L; + bitField0_ = (bitField0_ & ~0x00000002); return this; } @@ -1251,6 +1293,7 @@ public final class PersistentMessages { public org.opendaylight.controller.protobuff.messages.persistent.PersistentMessages.CompositeModification buildPartial() { org.opendaylight.controller.protobuff.messages.persistent.PersistentMessages.CompositeModification result = new org.opendaylight.controller.protobuff.messages.persistent.PersistentMessages.CompositeModification(this); int from_bitField0_ = bitField0_; + int to_bitField0_ = 0; if (modificationBuilder_ == null) { if (((bitField0_ & 0x00000001) == 0x00000001)) { modification_ = java.util.Collections.unmodifiableList(modification_); @@ -1260,6 +1303,11 @@ public final class PersistentMessages { } else { result.modification_ = modificationBuilder_.build(); } + if (((from_bitField0_ & 0x00000002) == 0x00000002)) { + to_bitField0_ |= 0x00000001; + } + result.timeStamp_ = timeStamp_; + result.bitField0_ = to_bitField0_; onBuilt(); return result; } @@ -1301,6 +1349,9 @@ public final class PersistentMessages { } } } + if (other.hasTimeStamp()) { + setTimeStamp(other.getTimeStamp()); + } this.mergeUnknownFields(other.getUnknownFields()); return this; } @@ -1574,6 +1625,39 @@ public final class PersistentMessages { return modificationBuilder_; } + // optional int64 timeStamp = 2; + private long timeStamp_ ; + /** + * optional int64 timeStamp = 2; + */ + public boolean hasTimeStamp() { + return ((bitField0_ & 0x00000002) == 0x00000002); + } + /** + * optional int64 timeStamp = 2; + */ + public long getTimeStamp() { + return timeStamp_; + } + /** + * optional int64 timeStamp = 2; + */ + public Builder setTimeStamp(long value) { + bitField0_ |= 0x00000002; + timeStamp_ = value; + onChanged(); + return this; + } + /** + * optional int64 timeStamp = 2; + */ + public Builder clearTimeStamp() { + bitField0_ = (bitField0_ & ~0x00000002); + timeStamp_ = 0L; + onChanged(); + return this; + } + // @@protoc_insertion_point(builder_scope:org.opendaylight.controller.mdsal.CompositeModification) } @@ -1610,11 +1694,12 @@ public final class PersistentMessages { "e\030\001 \002(\t\022C\n\004path\030\002 \002(\01325.org.opendaylight" + ".controller.mdsal.InstanceIdentifier\0225\n\004" + "data\030\003 \001(\0132\'.org.opendaylight.controller" + - ".mdsal.Node\"^\n\025CompositeModification\022E\n\014" + + ".mdsal.Node\"q\n\025CompositeModification\022E\n\014" + "modification\030\001 \003(\0132/.org.opendaylight.co" + - "ntroller.mdsal.ModificationBO\n9org.opend" + - "aylight.controller.protobuff.messages.pe", - "rsistentB\022PersistentMessages" + "ntroller.mdsal.Modification\022\021\n\ttimeStamp" + + "\030\002 \001(\003BO\n9org.opendaylight.controller.pr", + "otobuff.messages.persistentB\022PersistentM" + + "essages" }; com.google.protobuf.Descriptors.FileDescriptor.InternalDescriptorAssigner assigner = new com.google.protobuf.Descriptors.FileDescriptor.InternalDescriptorAssigner() { @@ -1632,7 +1717,7 @@ public final class PersistentMessages { internal_static_org_opendaylight_controller_mdsal_CompositeModification_fieldAccessorTable = new com.google.protobuf.GeneratedMessage.FieldAccessorTable( internal_static_org_opendaylight_controller_mdsal_CompositeModification_descriptor, - new java.lang.String[] { "Modification", }); + new java.lang.String[] { "Modification", "TimeStamp", }); return null; } }; diff --git a/opendaylight/md-sal/sal-clustering-commons/src/main/resources/Persistent.proto b/opendaylight/md-sal/sal-clustering-commons/src/main/resources/Persistent.proto index 8e834494cb..72651ab260 100644 --- a/opendaylight/md-sal/sal-clustering-commons/src/main/resources/Persistent.proto +++ b/opendaylight/md-sal/sal-clustering-commons/src/main/resources/Persistent.proto @@ -11,10 +11,12 @@ message Modification { required string type=1; required InstanceIdentifier path=2; optional Node data=3; + } message CompositeModification { repeated Modification modification=1; + optional int64 timeStamp = 2; } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/modification/MutableCompositeModification.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/modification/MutableCompositeModification.java index 1a005d856e..04854d26b2 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/modification/MutableCompositeModification.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/modification/MutableCompositeModification.java @@ -52,6 +52,8 @@ public class MutableCompositeModification PersistentMessages.CompositeModification.Builder builder = PersistentMessages.CompositeModification.newBuilder(); + builder.setTimeStamp(System.nanoTime()); + for (Modification m : modifications) { builder.addModification( (PersistentMessages.Modification) m.toSerializable()); diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/modification/MutableCompositeModificationTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/modification/MutableCompositeModificationTest.java index 7a21c8cdc5..03aaace0e3 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/modification/MutableCompositeModificationTest.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/modification/MutableCompositeModificationTest.java @@ -1,28 +1,42 @@ package org.opendaylight.controller.cluster.datastore.modification; import com.google.common.base.Optional; -import junit.framework.Assert; import org.junit.Test; import org.opendaylight.controller.md.cluster.datastore.model.TestModel; import org.opendaylight.controller.sal.core.spi.data.DOMStoreReadWriteTransaction; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; + public class MutableCompositeModificationTest extends AbstractModificationTest { - @Test - public void testApply() throws Exception { + @Test + public void testApply() throws Exception { + + MutableCompositeModification compositeModification = new MutableCompositeModification(); + compositeModification.addModification(new WriteModification(TestModel.TEST_PATH, + ImmutableNodes.containerNode(TestModel.TEST_QNAME), TestModel.createTestContext())); + + DOMStoreReadWriteTransaction transaction = store.newReadWriteTransaction(); + compositeModification.apply(transaction); + commitTransaction(transaction); + + Optional> data = readData(TestModel.TEST_PATH); - MutableCompositeModification compositeModification = new MutableCompositeModification(); - compositeModification.addModification(new WriteModification(TestModel.TEST_PATH, ImmutableNodes.containerNode(TestModel.TEST_QNAME), TestModel.createTestContext())); + assertNotNull(data.get()); + assertEquals(TestModel.TEST_QNAME, data.get().getNodeType()); + } - DOMStoreReadWriteTransaction transaction = store.newReadWriteTransaction(); - compositeModification.apply(transaction); - commitTransaction(transaction); + @Test + public void testEverySerializedCompositeModificationObjectMustBeDifferent(){ + MutableCompositeModification compositeModification = new MutableCompositeModification(); + compositeModification.addModification(new WriteModification(TestModel.TEST_PATH, + ImmutableNodes.containerNode(TestModel.TEST_QNAME), TestModel.createTestContext())); - Optional> data = readData(TestModel.TEST_PATH); + assertNotEquals(compositeModification.toSerializable(), compositeModification.toSerializable()); - Assert.assertNotNull(data.get()); - Assert.assertEquals(TestModel.TEST_QNAME, data.get().getNodeType()); - } + } }