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 fcf1f45..c867fce 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 8e83449..72651ab 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 1a005d8..04854d2 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 7a21c8c..03aaace 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());
-  }
+    }
 }