BUG-8494: propagate submit failure immediately
[controller.git] / opendaylight / md-sal / sal-distributed-datastore / src / main / java / org / opendaylight / controller / cluster / databroker / ConcurrentDOMDataBroker.java
index e8b19b8346cecbb4c8b9d740b434cfbee7d0be7e..71cd2dc7fe2df10fa9b7f3a6e4ea037ea419f217 100644 (file)
@@ -15,6 +15,7 @@ import com.google.common.util.concurrent.FutureCallback;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.MoreExecutors;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
@@ -55,12 +56,13 @@ public class ConcurrentDOMDataBroker extends AbstractDOMBroker {
      */
     private final Executor clientFutureCallbackExecutor;
 
-    public ConcurrentDOMDataBroker(final Map<LogicalDatastoreType, DOMStore> datastores, Executor listenableFutureExecutor) {
+    public ConcurrentDOMDataBroker(final Map<LogicalDatastoreType, DOMStore> datastores,
+            Executor listenableFutureExecutor) {
         this(datastores, listenableFutureExecutor, DurationStatisticsTracker.createConcurrent());
     }
 
-    public ConcurrentDOMDataBroker(final Map<LogicalDatastoreType, DOMStore> datastores, Executor listenableFutureExecutor,
-            DurationStatisticsTracker commitStatsTracker) {
+    public ConcurrentDOMDataBroker(final Map<LogicalDatastoreType, DOMStore> datastores,
+            Executor listenableFutureExecutor, DurationStatisticsTracker commitStatsTracker) {
         super(datastores);
         this.clientFutureCallbackExecutor = Preconditions.checkNotNull(listenableFutureExecutor);
         this.commitStatsTracker = Preconditions.checkNotNull(commitStatsTracker);
@@ -78,7 +80,7 @@ public class ConcurrentDOMDataBroker extends AbstractDOMBroker {
         Preconditions.checkArgument(cohorts != null, "Cohorts must not be null.");
         LOG.debug("Tx: {} is submitted for execution.", transaction.getIdentifier());
 
-        if(cohorts.isEmpty()){
+        if (cohorts.isEmpty()) {
             return Futures.immediateCheckedFuture(null);
         }
 
@@ -109,7 +111,7 @@ public class ConcurrentDOMDataBroker extends AbstractDOMBroker {
                             new TransactionCommitFailedException(
                                             "Can Commit failed, no detailed cause available."));
                 } else {
-                    if(!cohortIterator.hasNext()) {
+                    if (!cohortIterator.hasNext()) {
                         // All cohorts completed successfully - we can move on to the preCommit phase
                         doPreCommit(startTime, clientSubmitFuture, transaction, cohorts);
                     } else {
@@ -120,9 +122,9 @@ public class ConcurrentDOMDataBroker extends AbstractDOMBroker {
             }
 
             @Override
-            public void onFailure(Throwable t) {
+            public void onFailure(Throwable failure) {
                 handleException(clientSubmitFuture, transaction, cohorts, CAN_COMMIT,
-                        TransactionCommitFailedExceptionMapper.CAN_COMMIT_ERROR_MAPPER, t);
+                        TransactionCommitFailedExceptionMapper.CAN_COMMIT_ERROR_MAPPER, failure);
             }
         };
 
@@ -140,7 +142,7 @@ public class ConcurrentDOMDataBroker extends AbstractDOMBroker {
         FutureCallback<Void> futureCallback = new FutureCallback<Void>() {
             @Override
             public void onSuccess(Void notUsed) {
-                if(!cohortIterator.hasNext()) {
+                if (!cohortIterator.hasNext()) {
                     // All cohorts completed successfully - we can move on to the commit phase
                     doCommit(startTime, clientSubmitFuture, transaction, cohorts);
                 } else {
@@ -150,9 +152,9 @@ public class ConcurrentDOMDataBroker extends AbstractDOMBroker {
             }
 
             @Override
-            public void onFailure(Throwable t) {
+            public void onFailure(Throwable failure) {
                 handleException(clientSubmitFuture, transaction, cohorts, PRE_COMMIT,
-                        TransactionCommitFailedExceptionMapper.PRE_COMMIT_MAPPER, t);
+                        TransactionCommitFailedExceptionMapper.PRE_COMMIT_MAPPER, failure);
             }
         };
 
@@ -170,7 +172,7 @@ public class ConcurrentDOMDataBroker extends AbstractDOMBroker {
         FutureCallback<Void> futureCallback = new FutureCallback<Void>() {
             @Override
             public void onSuccess(Void notUsed) {
-                if(!cohortIterator.hasNext()) {
+                if (!cohortIterator.hasNext()) {
                     // All cohorts completed successfully - we're done.
                     commitStatsTracker.addDuration(System.nanoTime() - startTime);
 
@@ -182,9 +184,9 @@ public class ConcurrentDOMDataBroker extends AbstractDOMBroker {
             }
 
             @Override
-            public void onFailure(Throwable t) {
+            public void onFailure(Throwable throwable) {
                 handleException(clientSubmitFuture, transaction, cohorts, COMMIT,
-                        TransactionCommitFailedExceptionMapper.COMMIT_ERROR_MAPPER, t);
+                        TransactionCommitFailedExceptionMapper.COMMIT_ERROR_MAPPER, throwable);
             }
         };
 
@@ -192,53 +194,53 @@ public class ConcurrentDOMDataBroker extends AbstractDOMBroker {
         Futures.addCallback(commitFuture, futureCallback, MoreExecutors.directExecutor());
     }
 
+    @SuppressFBWarnings(value = "BC_UNCONFIRMED_CAST_OF_RETURN_VALUE",
+            justification = "Pertains to the assignment of the 'clientException' var. FindBugs flags this as an "
+                + "uncomfirmed cast but the generic type in TransactionCommitFailedExceptionMapper is "
+                + "TransactionCommitFailedException and thus should be deemed as confirmed.")
     private static void handleException(final AsyncNotifyingSettableFuture clientSubmitFuture,
             final DOMDataWriteTransaction transaction,
             final Collection<DOMStoreThreePhaseCommitCohort> cohorts,
             final String phase, final TransactionCommitFailedExceptionMapper exMapper,
-            final Throwable t) {
+            final Throwable throwable) {
 
         if (clientSubmitFuture.isDone()) {
             // We must have had failures from multiple cohorts.
             return;
         }
 
-        LOG.warn("Tx: {} Error during phase {}, starting Abort", transaction.getIdentifier(), phase, t);
-        final Exception e;
-        if(t instanceof NoShardLeaderException || t instanceof ShardLeaderNotRespondingException) {
-            e = new DataStoreUnavailableException(t.getMessage(), t);
-        } else if (t instanceof Exception) {
-            e = (Exception)t;
-        } else {
-            e = new RuntimeException("Unexpected error occurred", t);
-        }
-
-        final TransactionCommitFailedException clientException = exMapper.apply(e);
+        LOG.warn("Tx: {} Error during phase {}, starting Abort", transaction.getIdentifier(), phase, throwable);
 
         // Transaction failed - tell all cohorts to abort.
-
         @SuppressWarnings("unchecked")
         ListenableFuture<Void>[] canCommitFutures = new ListenableFuture[cohorts.size()];
-        int i = 0;
+        int index = 0;
         for (DOMStoreThreePhaseCommitCohort cohort : cohorts) {
-            canCommitFutures[i++] = cohort.abort();
+            canCommitFutures[index++] = cohort.abort();
         }
 
+        // Propagate the original exception
+        final Exception e;
+        if (throwable instanceof NoShardLeaderException || throwable instanceof ShardLeaderNotRespondingException) {
+            e = new DataStoreUnavailableException(throwable.getMessage(), throwable);
+        } else if (throwable instanceof Exception) {
+            e = (Exception)throwable;
+        } else {
+            e = new RuntimeException("Unexpected error occurred", throwable);
+        }
+        clientSubmitFuture.setException(exMapper.apply(e));
+
         ListenableFuture<List<Void>> combinedFuture = Futures.allAsList(canCommitFutures);
         Futures.addCallback(combinedFuture, new FutureCallback<List<Void>>() {
             @Override
             public void onSuccess(List<Void> notUsed) {
                 // Propagate the original exception to the client.
-                clientSubmitFuture.setException(clientException);
+                LOG.debug("Tx: {} aborted successfully", transaction.getIdentifier());
             }
 
             @Override
-            public void onFailure(Throwable t) {
-                LOG.error("Tx: {} Error during Abort.", transaction.getIdentifier(), t);
-
-                // Propagate the original exception as that is what caused the Tx to fail and is
-                // what's interesting to the client.
-                clientSubmitFuture.setException(clientException);
+            public void onFailure(Throwable failure) {
+                LOG.error("Tx: {} Error during Abort.", transaction.getIdentifier(), failure);
             }
         }, MoreExecutors.directExecutor());
     }
@@ -250,7 +252,6 @@ public class ConcurrentDOMDataBroker extends AbstractDOMBroker {
      * the thread that completed this future, as a common use case is to pass an executor that runs
      * tasks in the same thread as the caller (ie MoreExecutors#sameThreadExecutor)
      * to {@link #addListener}.
-     *
      * FIXME: This class should probably be moved to yangtools common utils for re-usability and
      * unified with AsyncNotifyingListenableFutureTask.
      */
@@ -259,7 +260,7 @@ public class ConcurrentDOMDataBroker extends AbstractDOMBroker {
         /**
          * ThreadLocal used to detect if the task completion thread is running the future listener Runnables.
          */
-        private static final ThreadLocal<Boolean> ON_TASK_COMPLETION_THREAD_TL = new ThreadLocal<Boolean>();
+        private static final ThreadLocal<Boolean> ON_TASK_COMPLETION_THREAD_TL = new ThreadLocal<>();
 
         private final Executor listenerExecutor;
 
@@ -325,4 +326,9 @@ public class ConcurrentDOMDataBroker extends AbstractDOMBroker {
             }
         }
     }
+
+    @Override
+    public String toString() {
+        return "Clustered ConcurrentDOMDataBroker";
+    }
 }