From cd3a0e09db5a1def00c46a4be245dbdf648b539c Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Mon, 3 Jul 2017 19:57:38 +0200 Subject: [PATCH] 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 (cherry picked from commit 2783c9dffdd91dae87d3351f4ebffbd8679e3133) --- .../cluster/datastore/ShardDataTree.java | 9 +-- .../datastore/SimpleShardDataTreeCohort.java | 58 +++++-------------- .../SimpleShardDataTreeCohortTest.java | 2 +- 3 files changed, 20 insertions(+), 49 deletions(-) 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 4c226aaa9c..27577b27d8 100644 --- 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 f73d343bce..cf9948cf03 100644 --- 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 1e01537431..59933f8744 100644 --- 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); } -- 2.36.6