From 9681454e5a6acb2ce5e87c72c06e6a03e901c765 Mon Sep 17 00:00:00 2001
From: Tom Pantelis <tpanteli@brocade.com>
Date: Mon, 14 Dec 2015 14:17:12 -0500
Subject: [PATCH] Bug 4774: Add Tx ID to logging on Tx chain failures

To help with debugging, it's useful to see the tx ID when a create fails
due to previous tx not ready.

Change-Id: I0547048ea62340a0297affed3512271908eba65a
Signed-off-by: Tom Pantelis <tpanteli@brocade.com>
---
 .../AbstractTransactionContextFactory.java    |  4 ++--
 .../datastore/TransactionChainProxy.java      | 14 ++++++-------
 .../datastore/TransactionContextFactory.java  |  2 +-
 ...bstractSnapshotBackedTransactionChain.java | 21 ++++++++++---------
 4 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractTransactionContextFactory.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractTransactionContextFactory.java
index df32f8fcaa..a0071c3f47 100644
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractTransactionContextFactory.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractTransactionContextFactory.java
@@ -92,7 +92,7 @@ abstract class AbstractTransactionContextFactory<F extends LocalTransactionFacto
         final TransactionContextWrapper transactionContextWrapper =
                 new TransactionContextWrapper(parent.getIdentifier(), actorContext);
 
-        Future<PrimaryShardInfo> findPrimaryFuture = findPrimaryShard(shardName);
+        Future<PrimaryShardInfo> findPrimaryFuture = findPrimaryShard(shardName, parent.getIdentifier().toString());
         if(findPrimaryFuture.isCompleted()) {
             Try<PrimaryShardInfo> maybe = findPrimaryFuture.value().get();
             if(maybe.isSuccess()) {
@@ -154,7 +154,7 @@ abstract class AbstractTransactionContextFactory<F extends LocalTransactionFacto
      * @param shardName Shard name
      * @return Future containing shard information.
      */
-    protected abstract Future<PrimaryShardInfo> findPrimaryShard(String shardName);
+    protected abstract Future<PrimaryShardInfo> findPrimaryShard(String shardName, String txId);
 
     /**
      * Create local transaction factory for specified shard, backed by specified shard leader
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 0946b402fd..4b75fbbd5c 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
@@ -179,21 +179,21 @@ final class TransactionChainProxy extends AbstractTransactionContextFactory<Loca
      * previous Tx's ready operations haven't completed yet.
      */
     @Override
-    protected Future<PrimaryShardInfo> findPrimaryShard(final String shardName) {
+    protected Future<PrimaryShardInfo> findPrimaryShard(final String shardName, final String txId) {
         // Read current state atomically
         final State localState = currentState;
 
         // There are no outstanding futures, shortcut
         final Future<?> previous = localState.previousFuture();
         if (previous == null) {
-            return parent.findPrimaryShard(shardName);
+            return parent.findPrimaryShard(shardName, txId);
         }
 
         final String previousTransactionId;
 
         if(localState instanceof Pending){
             previousTransactionId = ((Pending) localState).getIdentifier().toString();
-            LOG.debug("Waiting for ready futures with pending Tx {}", previousTransactionId);
+            LOG.debug("Tx: {} - waiting for ready futures with pending Tx {}", txId, previousTransactionId);
         } else {
             previousTransactionId = "";
             LOG.debug("Waiting for ready futures on chain {}", getTransactionChainId());
@@ -207,15 +207,15 @@ final class TransactionChainProxy extends AbstractTransactionContextFactory<Loca
             public void onComplete(final Throwable failure, final Object notUsed) {
                 if (failure != null) {
                     // A Ready Future failed so fail the returned Promise.
-                    LOG.error("Ready future failed for Tx {}", previousTransactionId);
+                    LOG.error("Tx: {} - ready future failed for previous Tx {}", txId, previousTransactionId);
                     returnPromise.failure(failure);
                 } else {
-                    LOG.debug("Previous Tx {} readied - proceeding to FindPrimaryShard",
-                            previousTransactionId);
+                    LOG.debug("Tx: {} - previous Tx {} readied - proceeding to FindPrimaryShard",
+                            txId, previousTransactionId);
 
                     // Send the FindPrimaryShard message and use the resulting Future to complete the
                     // returned Promise.
-                    returnPromise.completeWith(parent.findPrimaryShard(shardName));
+                    returnPromise.completeWith(parent.findPrimaryShard(shardName, txId));
                 }
             }
         };
diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/TransactionContextFactory.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/TransactionContextFactory.java
index 2b5144aa5e..1d141aec2e 100644
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/TransactionContextFactory.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/TransactionContextFactory.java
@@ -45,7 +45,7 @@ final class TransactionContextFactory extends AbstractTransactionContextFactory<
     }
 
     @Override
-    protected Future<PrimaryShardInfo> findPrimaryShard(final String shardName) {
+    protected Future<PrimaryShardInfo> findPrimaryShard(final String shardName, final String txId) {
         return getActorContext().findPrimaryShardAsync(shardName);
     }
 
diff --git a/opendaylight/md-sal/sal-dom-spi/src/main/java/org/opendaylight/controller/sal/core/spi/data/AbstractSnapshotBackedTransactionChain.java b/opendaylight/md-sal/sal-dom-spi/src/main/java/org/opendaylight/controller/sal/core/spi/data/AbstractSnapshotBackedTransactionChain.java
index 7683937ce2..7b6e68dc51 100644
--- a/opendaylight/md-sal/sal-dom-spi/src/main/java/org/opendaylight/controller/sal/core/spi/data/AbstractSnapshotBackedTransactionChain.java
+++ b/opendaylight/md-sal/sal-dom-spi/src/main/java/org/opendaylight/controller/sal/core/spi/data/AbstractSnapshotBackedTransactionChain.java
@@ -32,7 +32,7 @@ public abstract class AbstractSnapshotBackedTransactionChain<T> extends Transact
          *
          * @return A new snapshot
          */
-        protected abstract DataTreeSnapshot getSnapshot();
+        protected abstract DataTreeSnapshot getSnapshot(Object transactionId);
     }
 
     private static final class Idle extends State {
@@ -43,7 +43,7 @@ public abstract class AbstractSnapshotBackedTransactionChain<T> extends Transact
         }
 
         @Override
-        protected DataTreeSnapshot getSnapshot() {
+        protected DataTreeSnapshot getSnapshot(Object transactionId) {
             return chain.takeSnapshot();
         }
     }
@@ -66,9 +66,10 @@ public abstract class AbstractSnapshotBackedTransactionChain<T> extends Transact
         }
 
         @Override
-        protected DataTreeSnapshot getSnapshot() {
+        protected DataTreeSnapshot getSnapshot(Object transactionId) {
             final DataTreeSnapshot ret = snapshot;
-            Preconditions.checkState(ret != null, "Previous transaction %s is not ready yet", transaction.getIdentifier());
+            Preconditions.checkState(ret != null, "Could not get snapshot for transaction %s - previous transaction %s is not ready yet",
+                    transactionId, transaction.getIdentifier());
             return ret;
         }
 
@@ -89,7 +90,7 @@ public abstract class AbstractSnapshotBackedTransactionChain<T> extends Transact
         }
 
         @Override
-        protected DataTreeSnapshot getSnapshot() {
+        protected DataTreeSnapshot getSnapshot(Object transactionId) {
             throw new IllegalStateException(message);
         }
     }
@@ -108,9 +109,9 @@ public abstract class AbstractSnapshotBackedTransactionChain<T> extends Transact
         state = idleState;
     }
 
-    private Entry<State, DataTreeSnapshot> getSnapshot() {
+    private Entry<State, DataTreeSnapshot> getSnapshot(T transactionId) {
         final State localState = state;
-        return new SimpleEntry<>(localState, localState.getSnapshot());
+        return new SimpleEntry<>(localState, localState.getSnapshot(transactionId));
     }
 
     private boolean recordTransaction(final State expected, final DOMStoreWriteTransaction transaction) {
@@ -124,7 +125,7 @@ public abstract class AbstractSnapshotBackedTransactionChain<T> extends Transact
     }
 
     protected DOMStoreReadTransaction newReadOnlyTransaction(T transactionId) {
-        final Entry<State, DataTreeSnapshot> entry = getSnapshot();
+        final Entry<State, DataTreeSnapshot> entry = getSnapshot(transactionId);
         return SnapshotBackedTransactions.newReadTransaction(transactionId, getDebugTransactions(), entry.getValue());
     }
 
@@ -138,7 +139,7 @@ public abstract class AbstractSnapshotBackedTransactionChain<T> extends Transact
         DOMStoreReadWriteTransaction ret;
 
         do {
-            entry = getSnapshot();
+            entry = getSnapshot(transactionId);
             ret = new SnapshotBackedReadWriteTransaction<T>(transactionId, getDebugTransactions(), entry.getValue(), this);
         } while (!recordTransaction(entry.getKey(), ret));
 
@@ -155,7 +156,7 @@ public abstract class AbstractSnapshotBackedTransactionChain<T> extends Transact
         DOMStoreWriteTransaction ret;
 
         do {
-            entry = getSnapshot();
+            entry = getSnapshot(transactionId);
             ret = new SnapshotBackedWriteTransaction<T>(transactionId, getDebugTransactions(), entry.getValue(), this);
         } while (!recordTransaction(entry.getKey(), ret));
 
-- 
2.36.6