BUG-8618: eliminate SimpleShardDataTreeCohort subclasses

Now that we handle pre-cancommit failures useing reportFailure(),
there is no need to have specialized subclasses for cohorts, as
the initial failure can cleanly be handled via nextFailure.

This also places a guard in reportFailure() so we do not override
a failure once it is set -- which should only happen in the case
of a dead-on-arrival transaction and it timing out in READY state.

Change-Id: I057c5b36006843f51d60034d30af83bac4e02cd7
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit 2783c9dffdd91dae87d3351f4ebffbd8679e3133)

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTree.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTree.java
index 4c226aaa9ca918f7ed981e52e26301d3f6a229a0..27577b27d8d4f9342a90ed6db72ec2a5ca70c06c 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTree.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTree.java
@@ -719,8 +719,6 @@ public class ShardDataTree extends ShardDataTreeTransactionParent {
             LOG.debug("{}: Validating transaction {}", logContext, cohort.getIdentifier());
             Exception cause;
             try {
-                cohort.throwCanCommitFailure();
-
                 tip.validate(modification);
                 LOG.debug("{}: Transaction {} validated", logContext, cohort.getIdentifier());
                 cohort.successfulCanCommit();
@@ -942,15 +940,14 @@ public class ShardDataTree extends ShardDataTreeTransactionParent {
     @Override
     ShardDataTreeCohort createFailedCohort(final TransactionIdentifier txId, final DataTreeModification mod,
             final Exception failure) {
-        SimpleShardDataTreeCohort cohort = new SimpleShardDataTreeCohort.DeadOnArrival(this, mod, txId, failure);
+        final SimpleShardDataTreeCohort cohort = new SimpleShardDataTreeCohort(this, mod, txId, failure);
         pendingTransactions.add(new CommitEntry(cohort, readTime()));
         return cohort;
     }
 
     @Override
-    ShardDataTreeCohort createReadyCohort(final TransactionIdentifier txId,
-            final DataTreeModification mod) {
-        SimpleShardDataTreeCohort cohort = new SimpleShardDataTreeCohort.Normal(this, mod, txId,
+    ShardDataTreeCohort createReadyCohort(final TransactionIdentifier txId, final DataTreeModification mod) {
+        SimpleShardDataTreeCohort cohort = new SimpleShardDataTreeCohort(this, mod, txId,
                 cohortRegistry.createCohort(schemaContext, txId, COMMIT_STEP_TIMEOUT));
         pendingTransactions.add(new CommitEntry(cohort, readTime()));
         return cohort;

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/SimpleShardDataTreeCohort.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/SimpleShardDataTreeCohort.java
index f73d343bcec2ea4a0785ec84fa967b274345c3c6..cf9948cf03e0407f47103e4e2085d33a58d0346d 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/SimpleShardDataTreeCohort.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/SimpleShardDataTreeCohort.java
@@ -27,39 +27,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import scala.concurrent.Future;
 
-abstract class SimpleShardDataTreeCohort extends ShardDataTreeCohort {
-    static final class DeadOnArrival extends SimpleShardDataTreeCohort {
-        private final Exception failure;
-
-        DeadOnArrival(final ShardDataTree dataTree, final DataTreeModification transaction,
-            final TransactionIdentifier transactionId, final Exception failure) {
-            super(dataTree, transaction, transactionId, null);
-            this.failure = Preconditions.checkNotNull(failure);
-        }
-
-        @Override
-        void throwCanCommitFailure() throws Exception {
-            throw failure;
-        }
-
-        @Override
-        ToStringHelper addToStringAttributes(final ToStringHelper toStringHelper) {
-            return super.addToStringAttributes(toStringHelper).add("failure", failure);
-        }
-    }
-
-    static final class Normal extends SimpleShardDataTreeCohort {
-        Normal(final ShardDataTree dataTree, final DataTreeModification transaction,
-            final TransactionIdentifier transactionId, final CompositeDataTreeCohort userCohorts) {
-            super(dataTree, transaction, transactionId, Preconditions.checkNotNull(userCohorts));
-        }
-
-        @Override
-        void throwCanCommitFailure() {
-            // No-op
-        }
-    }
-
+final class SimpleShardDataTreeCohort extends ShardDataTreeCohort {
     private static final Logger LOG = LoggerFactory.getLogger(SimpleShardDataTreeCohort.class);
 
     private final DataTreeModification transaction;
@@ -77,7 +45,16 @@ abstract class SimpleShardDataTreeCohort extends ShardDataTreeCohort {
         this.dataTree = Preconditions.checkNotNull(dataTree);
         this.transaction = Preconditions.checkNotNull(transaction);
         this.transactionId = Preconditions.checkNotNull(transactionId);
-        this.userCohorts = userCohorts;
+        this.userCohorts = Preconditions.checkNotNull(userCohorts);
+    }
+
+    SimpleShardDataTreeCohort(final ShardDataTree dataTree, final DataTreeModification transaction,
+        final TransactionIdentifier transactionId, final Exception nextFailure) {
+        this.dataTree = Preconditions.checkNotNull(dataTree);
+        this.transaction = Preconditions.checkNotNull(transaction);
+        this.transactionId = Preconditions.checkNotNull(transactionId);
+        this.userCohorts = null;
+        this.nextFailure = Preconditions.checkNotNull(nextFailure);
     }
 
     @Override
@@ -258,16 +235,13 @@ abstract class SimpleShardDataTreeCohort extends ShardDataTreeCohort {
     }
 
     void reportFailure(final Exception cause) {
-        this.nextFailure = Preconditions.checkNotNull(cause);
+        if (nextFailure == null) {
+            this.nextFailure = Preconditions.checkNotNull(cause);
+        } else {
+            LOG.debug("Transaction {} already has a set failure, not updating it", transactionId, cause);
+        }
     }
 
-    /**
-     * If there is an initial failure, throw it so the caller can process it.
-     *
-     * @throws Exception reported failure.
-     */
-    abstract void throwCanCommitFailure() throws Exception;
-
     @Override
     public boolean isFailed() {
         return state == State.FAILED || nextFailure != null;

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/SimpleShardDataTreeCohortTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/SimpleShardDataTreeCohortTest.java
index 1e0153743115259f817b211591b1441420bf32a8..59933f874495768b60534d111f750e9ba91ae71b 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/SimpleShardDataTreeCohortTest.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/SimpleShardDataTreeCohortTest.java
@@ -62,7 +62,7 @@ public class SimpleShardDataTreeCohortTest extends AbstractTest {
         doNothing().when(mockUserCohorts).commit();
         doReturn(Optional.empty()).when(mockUserCohorts).abort();
 
-        cohort = new SimpleShardDataTreeCohort.Normal(mockShardDataTree, mockModification, nextTransactionId(),
+        cohort = new SimpleShardDataTreeCohort(mockShardDataTree, mockModification, nextTransactionId(),
             mockUserCohorts);
     }