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 <moraja@cisco.com>

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 fcf1f45bc26d5216ab8e06c4105e38d312bd1a73..c867fce75df2bf1980bfef71a3da658b7c758465 100644 (file)
--- 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;
+    /**
+     * <code>optional int64 timeStamp = 2;</code>
+     */
+    boolean hasTimeStamp();
+    /**
+     * <code>optional int64 timeStamp = 2;</code>
+     */
+    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<org.opendaylight.controller.protobuff.messages.persistent.PersistentMessages.Modification> 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_;
+    /**
+     * <code>optional int64 timeStamp = 2;</code>
+     */
+    public boolean hasTimeStamp() {
+      return ((bitField0_ & 0x00000001) == 0x00000001);
+    }
+    /**
+     * <code>optional int64 timeStamp = 2;</code>
+     */
+    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_ ;
+      /**
+       * <code>optional int64 timeStamp = 2;</code>
+       */
+      public boolean hasTimeStamp() {
+        return ((bitField0_ & 0x00000002) == 0x00000002);
+      }
+      /**
+       * <code>optional int64 timeStamp = 2;</code>
+       */
+      public long getTimeStamp() {
+        return timeStamp_;
+      }
+      /**
+       * <code>optional int64 timeStamp = 2;</code>
+       */
+      public Builder setTimeStamp(long value) {
+        bitField0_ |= 0x00000002;
+        timeStamp_ = value;
+        onChanged();
+        return this;
+      }
+      /**
+       * <code>optional int64 timeStamp = 2;</code>
+       */
+      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 8e834494cbe590aabdb2c68a49389f5d05c040aa..72651ab26054dbb69febec8fa7994f0d8f8832f4 100644 (file)
--- 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 1a005d856e30ceb3b9cd359b5fd14ef42bc14cf0..04854d26b2c060ae3b231481d1933a663e6d0b49 100644 (file)
--- 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 7a21c8cdc5eea9ea208c651286423c63b107a13b..03aaace0e354543f63d8e9f7ed5d74f998fc142c 100644 (file)
--- 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<NormalizedNode<?, ?>> 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<NormalizedNode<?,?>> data = readData(TestModel.TEST_PATH);
+        assertNotEquals(compositeModification.toSerializable(), compositeModification.toSerializable());
 
-    Assert.assertNotNull(data.get());
-    Assert.assertEquals(TestModel.TEST_QNAME, data.get().getNodeType());
-  }
+    }
 }