BUG 2339 : TransactionChain id created by the Clustered Data Store are not unique

The fix is to use a simple atomically incrementing integer to form the transaction chain
identifier instead of using the system time.

I also added a few log statements in there which were helpful in debugging the issue.

Change-Id: Ie1465f1640d6be0ff9da0f66477ebafc7ebff137
Signed-off-by: Moiz Raja <moraja@cisco.com>

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/TransactionChainProxy.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/TransactionChainProxy.java
index 92de88e1126c19c38718f0f7c5c8cd51ad8430c0..2e671e3ce2f267807837225c95376847c6116eb8 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/TransactionChainProxy.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/TransactionChainProxy.java
@@ -14,6 +14,7 @@ import com.google.common.base.Preconditions;
 import java.util.AbstractMap.SimpleEntry;
 import java.util.Collections;
 import java.util.List;
 import java.util.AbstractMap.SimpleEntry;
 import java.util.Collections;
 import java.util.List;

+import java.util.concurrent.atomic.AtomicInteger;

 import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
 import org.opendaylight.controller.cluster.datastore.messages.CloseTransactionChain;
 import org.opendaylight.controller.cluster.datastore.utils.ActorContext;
 import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
 import org.opendaylight.controller.cluster.datastore.messages.CloseTransactionChain;
 import org.opendaylight.controller.cluster.datastore.utils.ActorContext;


@@ -22,6 +23,8 @@ import org.opendaylight.controller.sal.core.spi.data.DOMStoreReadTransaction;
 import org.opendaylight.controller.sal.core.spi.data.DOMStoreReadWriteTransaction;
 import org.opendaylight.controller.sal.core.spi.data.DOMStoreTransactionChain;
 import org.opendaylight.controller.sal.core.spi.data.DOMStoreWriteTransaction;
 import org.opendaylight.controller.sal.core.spi.data.DOMStoreReadWriteTransaction;
 import org.opendaylight.controller.sal.core.spi.data.DOMStoreTransactionChain;
 import org.opendaylight.controller.sal.core.spi.data.DOMStoreWriteTransaction;

+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;

 import scala.concurrent.Future;
 import scala.concurrent.Promise;
 
 import scala.concurrent.Future;
 import scala.concurrent.Promise;
 


@@ -29,6 +32,9 @@ import scala.concurrent.Promise;
  * TransactionChainProxy acts as a proxy for a DOMStoreTransactionChain created on a remote shard
  */
 public class TransactionChainProxy implements DOMStoreTransactionChain {
  * TransactionChainProxy acts as a proxy for a DOMStoreTransactionChain created on a remote shard
  */
 public class TransactionChainProxy implements DOMStoreTransactionChain {

+
+    private static final Logger LOG = LoggerFactory.getLogger(TransactionChainProxy.class);
+

     private interface State {
         boolean isReady();
 
     private interface State {
         boolean isReady();
 


@@ -92,10 +98,15 @@ public class TransactionChainProxy implements DOMStoreTransactionChain {
     private final ActorContext actorContext;
     private final String transactionChainId;
     private volatile State state = IDLE_STATE;
     private final ActorContext actorContext;
     private final String transactionChainId;
     private volatile State state = IDLE_STATE;

+    private static final AtomicInteger counter = new AtomicInteger(0);

 
     public TransactionChainProxy(ActorContext actorContext) {
         this.actorContext = actorContext;
 
     public TransactionChainProxy(ActorContext actorContext) {
         this.actorContext = actorContext;

-        transactionChainId = actorContext.getCurrentMemberName() + "-" + System.currentTimeMillis();
+        transactionChainId = actorContext.getCurrentMemberName() + "-transaction-chain-" + counter.incrementAndGet();
+    }
+
+    public String getTransactionChainId() {
+        return transactionChainId;

     }
 
     @Override
     }
 
     @Override


@@ -144,6 +155,7 @@ public class TransactionChainProxy implements DOMStoreTransactionChain {
 
         @Override
         protected void onTransactionReady(List<Future<ActorSelection>> readyFutures) {
 
         @Override
         protected void onTransactionReady(List<Future<ActorSelection>> readyFutures) {

+            LOG.debug("onTransactionReady {} pending readyFutures size {} chain {}", getIdentifier(), readyFutures.size(), TransactionChainProxy.this.transactionChainId);

             state.setReadyFutures(getIdentifier(), readyFutures);
         }
 
             state.setReadyFutures(getIdentifier(), readyFutures);
         }
 

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/TransactionProxy.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/TransactionProxy.java
index d93bae22e08d9fddb3f1ac7d9c12aa4a278d0ff6..226ac75467ff17e7066650e72cb875609bf418cd 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/TransactionProxy.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/TransactionProxy.java
@@ -220,7 +220,7 @@ public class TransactionProxy implements DOMStoreReadWriteTransaction {
             phantomReferenceCache.put(cleanup, cleanup);
         }
 
             phantomReferenceCache.put(cleanup, cleanup);
         }
 

-        LOG.debug("Created txn {} of type {}", identifier, transactionType);
+        LOG.debug("Created txn {} of type {} on chain {}", identifier, transactionType, transactionChainId);

     }
 
     @VisibleForTesting
     }
 
     @VisibleForTesting


@@ -423,8 +423,8 @@ public class TransactionProxy implements DOMStoreReadWriteTransaction {
 
         for(TransactionFutureCallback txFutureCallback : txFutureCallbackMap.values()) {
 
 
         for(TransactionFutureCallback txFutureCallback : txFutureCallbackMap.values()) {
 

-            LOG.debug("Tx {} Readying transaction for shard {}", identifier,
-                        txFutureCallback.getShardName());
+            LOG.debug("Tx {} Readying transaction for shard {} chain {}", identifier,
+                        txFutureCallback.getShardName(), transactionChainId);

 
             TransactionContext transactionContext = txFutureCallback.getTransactionContext();
             if(transactionContext != null) {
 
             TransactionContext transactionContext = txFutureCallback.getTransactionContext();
             if(transactionContext != null) {

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/TransactionChainProxyTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/TransactionChainProxyTest.java
index 4cca1bf9ad6698e9daa32409ced7055836cee351..ce0547c3883d19a12c01b8d9a9ce355770aead3b 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/TransactionChainProxyTest.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/TransactionChainProxyTest.java
@@ -10,6 +10,11 @@
 
 package org.opendaylight.controller.cluster.datastore;
 
 
 package org.opendaylight.controller.cluster.datastore;
 

+import static org.mockito.Matchers.anyObject;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;

 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;


@@ -20,12 +25,6 @@ import org.opendaylight.controller.sal.core.spi.data.DOMStoreTransaction;
 import org.opendaylight.controller.sal.core.spi.data.DOMStoreWriteTransaction;
 import org.opendaylight.yangtools.yang.model.api.SchemaContext;
 
 import org.opendaylight.controller.sal.core.spi.data.DOMStoreWriteTransaction;
 import org.opendaylight.yangtools.yang.model.api.SchemaContext;
 

-import static org.mockito.Matchers.anyObject;
-import static org.mockito.Mockito.doReturn;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.times;
-import static org.mockito.Mockito.verify;
-

 public class TransactionChainProxyTest {
     ActorContext actorContext = mock(ActorContext.class);
     SchemaContext schemaContext = mock(SchemaContext.class);
 public class TransactionChainProxyTest {
     ActorContext actorContext = mock(ActorContext.class);
     SchemaContext schemaContext = mock(SchemaContext.class);


@@ -68,4 +67,12 @@ public class TransactionChainProxyTest {
 
         verify(context, times(1)).broadcast(anyObject());
     }
 
         verify(context, times(1)).broadcast(anyObject());
     }

+
+    @Test
+    public void testTransactionChainsHaveUniqueId(){
+        TransactionChainProxy one = new TransactionChainProxy(mock(ActorContext.class));
+        TransactionChainProxy two = new TransactionChainProxy(mock(ActorContext.class));
+
+        Assert.assertNotEquals(one.getTransactionChainId(), two.getTransactionChainId());
+    }

 }
 }