From: Moiz Raja <moraja@cisco.com>
Date: Sat, 8 Nov 2014 02:12:45 +0000 (-0800)
Subject: BUG 2339 : TransactionChain id created by the Clustered Data Store are not unique
X-Git-Tag: release/lithium~908^2~1
X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=4dd0108a91d84a7133ef6b781911e87903981bc1

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 92de88e112..2e671e3ce2 100644
--- 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.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;
@@ -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.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 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 {
+
+    private static final Logger LOG = LoggerFactory.getLogger(TransactionChainProxy.class);
+
     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 static final AtomicInteger counter = new AtomicInteger(0);
 
     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
@@ -144,6 +155,7 @@ public class TransactionChainProxy implements DOMStoreTransactionChain {
 
         @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);
         }
 
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 d93bae22e0..226ac75467 100644
--- 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);
         }
 
-        LOG.debug("Created txn {} of type {}", identifier, transactionType);
+        LOG.debug("Created txn {} of type {} on chain {}", identifier, transactionType, transactionChainId);
     }
 
     @VisibleForTesting
@@ -423,8 +423,8 @@ public class TransactionProxy implements DOMStoreReadWriteTransaction {
 
         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) {
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 4cca1bf9ad..ce0547c388 100644
--- 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;
 
+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;
@@ -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 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);
@@ -68,4 +67,12 @@ public class TransactionChainProxyTest {
 
         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());
+    }
 }