Bug 5153: Add timestamp to TransactionIdentifier

TransactionIdentifiers are created locally but sent to the remote leader
so it's possible, after a restart, for the remote leader to see the same id
for 2 different txns since the local counter starts at 1. To alleviate
this I added a timestamp to TransactionIdentifier. I could've just used
a UUID but the counter is useful for debugging and a full UUID would
make the string version pretty long for logging. I think an additional
millisec timestamp is sufficient.

Change-Id: Iaabd3d25eb64dd14053f96336c48de90d4364678
Signed-off-by: Tom Pantelis <tpanteli@brocade.com>

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/identifiers/ChainedTransactionIdentifier.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/identifiers/ChainedTransactionIdentifier.java
index 78f68f6204fdb9a946fa37ac47b37729b528a469..4753437d2501ef535cb7babb9a891bbd64c0d608 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/identifiers/ChainedTransactionIdentifier.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/identifiers/ChainedTransactionIdentifier.java
@@ -25,8 +25,9 @@ public class ChainedTransactionIdentifier extends TransactionIdentifier {
         stringRepresentation = Suppliers.memoize(new Supplier<String>() {
             @Override
             public String get() {
-                return new StringBuilder(chainId.toString().length() + TX_SEPARATOR.length() + 10).
-                        append(chainId).append(TX_SEPARATOR).append(getCounter()).toString();
+                return new StringBuilder(chainId.toString().length() + TX_SEPARATOR.length() + 21).
+                        append(chainId).append(TX_SEPARATOR).append(getCounter()).append('-').
+                        append(getTimestamp()).toString();
             }
         });
     }

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/identifiers/TransactionIdentifier.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/identifiers/TransactionIdentifier.java
index 5a365f28a3e45abcc794b6de1f8349125cd9a2ee..6023e555288b58e18f6eab4e215615ffdd7254f0 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/identifiers/TransactionIdentifier.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/identifiers/TransactionIdentifier.java
@@ -13,27 +13,33 @@ import com.google.common.base.Preconditions;
 public class TransactionIdentifier {
     protected static final String TX_SEPARATOR = "-txn-";
 
-    protected String getMemberName() {
-        return memberName;
-    }
-
-    protected long getCounter() {
-        return counter;
-    }
-
     private final String memberName;
     private final long counter;
+    private final long timestamp;
     private String stringRepresentation;
 
     public TransactionIdentifier(String memberName, long counter) {
         this.memberName = Preconditions.checkNotNull(memberName, "memberName should not be null");
         this.counter = counter;
+        this.timestamp = System.currentTimeMillis();
     }
 
     public String getChainId() {
         return "";
     }
 
+    protected String getMemberName() {
+        return memberName;
+    }
+
+    protected long getCounter() {
+        return counter;
+    }
+
+    protected long getTimestamp() {
+        return timestamp;
+    }
+
     public static TransactionIdentifier create(String memberName, long counter) {
         return new TransactionIdentifier(memberName, counter);
     }
@@ -52,6 +58,11 @@ public class TransactionIdentifier {
         if (counter != that.counter) {
             return false;
         }
+
+        if (timestamp != that.timestamp) {
+            return false;
+        }
+
         if (!memberName.equals(that.memberName)) {
             return false;
         }
@@ -63,14 +74,16 @@ public class TransactionIdentifier {
     public int hashCode() {
         int result = memberName.hashCode();
         result = 31 * result + (int) (counter ^ (counter >>> 32));
+        result = 31 * result + (int)(timestamp ^ (timestamp >>> 32));
         return result;
     }
 
+
     @Override
     public String toString() {
         if(stringRepresentation == null) {
-            stringRepresentation = new StringBuilder(memberName.length() + TX_SEPARATOR.length() + 10).
-                append(memberName).append(TX_SEPARATOR).append(counter).toString();
+            stringRepresentation = new StringBuilder(memberName.length() + TX_SEPARATOR.length() + 21).
+                append(memberName).append(TX_SEPARATOR).append(counter).append('-').append(timestamp).toString();
         }
 
         return stringRepresentation;

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/identifiers/ChainedTransactionIdentifierTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/identifiers/ChainedTransactionIdentifierTest.java
index 77e5ea75994429d8d3b582392a97efb4ecb43a87..b4bb5aa1cbb42e1bad0269a169a1dbed5c9ddbd6 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/identifiers/ChainedTransactionIdentifierTest.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/identifiers/ChainedTransactionIdentifierTest.java
@@ -8,7 +8,8 @@
 
 package org.opendaylight.controller.cluster.datastore.identifiers;
 
-import static org.junit.Assert.assertEquals;
+import static org.hamcrest.CoreMatchers.startsWith;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import org.junit.Test;
 
@@ -25,7 +26,7 @@ public class ChainedTransactionIdentifierTest {
         assertTrue(txnId.contains("100"));
         assertTrue(txnId.contains("99"));
 
-        assertEquals("member-1-chn-99-txn-100", txnId);
+        assertThat(txnId, startsWith("member-1-chn-99-txn-100-"));
     }
 
 }
\ No newline at end of file

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/identifiers/TransactionChainIdentifierTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/identifiers/TransactionChainIdentifierTest.java
index 265ecb56b3d4104c373b7aec1c120d54338c3f17..1e965bf46d3a18a4a3c861e4739645408ffeaeaa 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/identifiers/TransactionChainIdentifierTest.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/identifiers/TransactionChainIdentifierTest.java
@@ -8,7 +8,9 @@
 
 package org.opendaylight.controller.cluster.datastore.identifiers;
 
+import static org.hamcrest.CoreMatchers.startsWith;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
 import org.junit.Test;
 
 public class TransactionChainIdentifierTest {
@@ -27,11 +29,11 @@ public class TransactionChainIdentifierTest {
 
         TransactionIdentifier txId1 = transactionChainIdentifier.newTransactionIdentifier();
 
-        assertEquals("member-1-chn-99-txn-1", txId1.toString());
+        assertThat(txId1.toString(), startsWith("member-1-chn-99-txn-1-"));
 
         TransactionIdentifier txId2 = transactionChainIdentifier.newTransactionIdentifier();
 
-        assertEquals("member-1-chn-99-txn-2", txId2.toString());
+        assertThat(txId2.toString(), startsWith("member-1-chn-99-txn-2-"));
     }
 
 }
\ No newline at end of file