From: Jaime Caamaño Ruiz Date: Mon, 5 Feb 2018 11:07:42 +0000 (+0100) Subject: Delay snapshot backed transaction ready error X-Git-Tag: release/oxygen~10 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=7292faba613ab556babd7bbcdd78984f5668bf9b;hp=7ab6f974861e01daa16ff56658eeb1be163cbfec Delay snapshot backed transaction ready error Delay snapshot backed transaction ready error to 3PC canCommit. Change-Id: Ief659423b401936a286f04c2f6c3732722c5aabf JIRA: CONTROLLER-1812 Signed-off-by: Jaime Caamaño Ruiz --- diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LocalThreePhaseCommitCohort.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LocalThreePhaseCommitCohort.java index 610212fdf3..80d5417e0b 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LocalThreePhaseCommitCohort.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LocalThreePhaseCommitCohort.java @@ -41,12 +41,13 @@ class LocalThreePhaseCommitCohort implements DOMStoreThreePhaseCommitCohort { protected LocalThreePhaseCommitCohort(final ActorContext actorContext, final ActorSelection leader, final SnapshotBackedWriteTransaction transaction, - final DataTreeModification modification) { + final DataTreeModification modification, + final Exception operationError) { this.actorContext = Preconditions.checkNotNull(actorContext); this.leader = Preconditions.checkNotNull(leader); this.transaction = Preconditions.checkNotNull(transaction); this.modification = Preconditions.checkNotNull(modification); - this.operationError = null; + this.operationError = operationError; } protected LocalThreePhaseCommitCohort(final ActorContext actorContext, final ActorSelection leader, diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LocalTransactionChain.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LocalTransactionChain.java index de98dce562..10ea3c63fc 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LocalTransactionChain.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LocalTransactionChain.java @@ -61,8 +61,9 @@ final class LocalTransactionChain extends AbstractSnapshotBackedTransactionChain @Override protected DOMStoreThreePhaseCommitCohort createCohort( final SnapshotBackedWriteTransaction transaction, - final DataTreeModification modification) { - return new LocalChainThreePhaseCommitCohort(transaction, modification); + final DataTreeModification modification, + final Exception operationError) { + return new LocalChainThreePhaseCommitCohort(transaction, modification, operationError); } @Override @@ -102,8 +103,8 @@ final class LocalTransactionChain extends AbstractSnapshotBackedTransactionChain private class LocalChainThreePhaseCommitCohort extends LocalThreePhaseCommitCohort { protected LocalChainThreePhaseCommitCohort(SnapshotBackedWriteTransaction transaction, - DataTreeModification modification) { - super(parent.getActorContext(), leader, transaction, modification); + DataTreeModification modification, Exception operationError) { + super(parent.getActorContext(), leader, transaction, modification, operationError); } protected LocalChainThreePhaseCommitCohort(SnapshotBackedWriteTransaction transaction, diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LocalTransactionFactoryImpl.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LocalTransactionFactoryImpl.java index d01cae8451..a4b2e4e62c 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LocalTransactionFactoryImpl.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LocalTransactionFactoryImpl.java @@ -67,8 +67,10 @@ final class LocalTransactionFactoryImpl extends TransactionReadyPrototype tx, final DataTreeModification tree) { - return new LocalThreePhaseCommitCohort(actorContext, leader, tx, tree); + final SnapshotBackedWriteTransaction tx, + final DataTreeModification tree, + final Exception readyError) { + return new LocalThreePhaseCommitCohort(actorContext, leader, tx, tree, readyError); } @SuppressWarnings({"unchecked", "checkstyle:IllegalCatch"}) @@ -81,13 +83,6 @@ final class LocalTransactionFactoryImpl extends TransactionReadyPrototype)tx, operationError); } - try { - return (LocalThreePhaseCommitCohort) tx.ready(); - } catch (Exception e) { - // Unfortunately we need to cast to SnapshotBackedWriteTransaction here as it's required by - // LocalThreePhaseCommitCohort. - return new LocalThreePhaseCommitCohort(actorContext, leader, - (SnapshotBackedWriteTransaction)tx, e); - } + return (LocalThreePhaseCommitCohort) tx.ready(); } } 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 f8a16b606a..383a0e5a46 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 @@ -180,7 +180,11 @@ public abstract class AbstractSnapshotBackedTransactionChain extends Transact } @Override - protected final DOMStoreThreePhaseCommitCohort transactionReady(final SnapshotBackedWriteTransaction tx, final DataTreeModification tree) { + protected final DOMStoreThreePhaseCommitCohort transactionReady( + final SnapshotBackedWriteTransaction tx, + final DataTreeModification tree, + final Exception readyError) { + final State localState = state; if (localState instanceof Allocated) { @@ -192,7 +196,7 @@ public abstract class AbstractSnapshotBackedTransactionChain extends Transact LOG.debug("Ignoring transaction {} readiness due to state {}", tx, localState); } - return createCohort(tx, tree); + return createCohort(tx, tree, readyError); } @Override @@ -276,7 +280,10 @@ public abstract class AbstractSnapshotBackedTransactionChain extends Transact * * @param transaction Transaction handle * @param modification {@link DataTreeModification} which needs to be applied to the backend + * @param operationError Any previous error that could be reported through three phase commit * @return A {@link DOMStoreThreePhaseCommitCohort} cohort. */ - protected abstract DOMStoreThreePhaseCommitCohort createCohort(final SnapshotBackedWriteTransaction transaction, final DataTreeModification modification); + protected abstract DOMStoreThreePhaseCommitCohort createCohort(final SnapshotBackedWriteTransaction transaction, + final DataTreeModification modification, + final Exception operationError); } diff --git a/opendaylight/md-sal/sal-dom-spi/src/main/java/org/opendaylight/controller/sal/core/spi/data/SnapshotBackedWriteTransaction.java b/opendaylight/md-sal/sal-dom-spi/src/main/java/org/opendaylight/controller/sal/core/spi/data/SnapshotBackedWriteTransaction.java index a5b7ea6253..eaabb3a21f 100644 --- a/opendaylight/md-sal/sal-dom-spi/src/main/java/org/opendaylight/controller/sal/core/spi/data/SnapshotBackedWriteTransaction.java +++ b/opendaylight/md-sal/sal-dom-spi/src/main/java/org/opendaylight/controller/sal/core/spi/data/SnapshotBackedWriteTransaction.java @@ -15,6 +15,7 @@ import com.google.common.base.Optional; import com.google.common.base.Preconditions; import com.google.common.base.Throwables; import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; +import javax.annotation.Nullable; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeModification; @@ -138,8 +139,13 @@ public class SnapshotBackedWriteTransaction extends AbstractDOMStoreTransacti final DataTreeModification tree = mutableTree; TREE_UPDATER.lazySet(this, null); - tree.ready(); - return wasReady.transactionReady(this, tree); + try { + tree.ready(); + return wasReady.transactionReady(this, tree, null); + } catch (RuntimeException e) { + LOG.debug("Store transaction: {}: unexpected failure when readying", getIdentifier(), e); + return wasReady.transactionReady(this, tree, e); + } } @Override @@ -185,8 +191,12 @@ public class SnapshotBackedWriteTransaction extends AbstractDOMStoreTransacti * Transaction on which ready was invoked. * @param tree * Modified data tree which has been constructed. + * @param readyError + * Any error that has already happened when readying. * @return DOMStoreThreePhaseCommitCohort associated with transaction */ - protected abstract DOMStoreThreePhaseCommitCohort transactionReady(SnapshotBackedWriteTransaction tx, DataTreeModification tree); + protected abstract DOMStoreThreePhaseCommitCohort transactionReady(SnapshotBackedWriteTransaction tx, + DataTreeModification tree, + @Nullable Exception readyError); } } diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ChainedTransactionCommitImpl.java b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ChainedTransactionCommitImpl.java index 35d891dac0..89ca972b68 100644 --- a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ChainedTransactionCommitImpl.java +++ b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ChainedTransactionCommitImpl.java @@ -15,9 +15,12 @@ import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeModification final class ChainedTransactionCommitImpl extends InMemoryDOMStoreThreePhaseCommitCohort { private final DOMStoreTransactionChainImpl txChain; - ChainedTransactionCommitImpl(final InMemoryDOMDataStore store, final SnapshotBackedWriteTransaction transaction, - final DataTreeModification modification, final DOMStoreTransactionChainImpl txChain) { - super(store, transaction, modification); + ChainedTransactionCommitImpl(final InMemoryDOMDataStore store, + final SnapshotBackedWriteTransaction transaction, + final DataTreeModification modification, + final DOMStoreTransactionChainImpl txChain, + final Exception operationError) { + super(store, transaction, modification, operationError); this.txChain = Preconditions.checkNotNull(txChain); } diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/DOMStoreTransactionChainImpl.java b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/DOMStoreTransactionChainImpl.java index 2cf79d899b..84c9c46cdd 100644 --- a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/DOMStoreTransactionChainImpl.java +++ b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/DOMStoreTransactionChainImpl.java @@ -22,8 +22,10 @@ final class DOMStoreTransactionChainImpl extends AbstractSnapshotBackedTransacti } @Override - protected DOMStoreThreePhaseCommitCohort createCohort(final SnapshotBackedWriteTransaction tx, final DataTreeModification modification) { - return new ChainedTransactionCommitImpl(store, tx, modification, this); + protected DOMStoreThreePhaseCommitCohort createCohort(final SnapshotBackedWriteTransaction tx, + final DataTreeModification modification, + final Exception operationError) { + return new ChainedTransactionCommitImpl(store, tx, modification, this, operationError); } @Override diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java index b5e5e8331e..3582ff6ef1 100644 --- a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java +++ b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java @@ -223,9 +223,11 @@ public class InMemoryDOMDataStore extends TransactionReadyPrototype impl } @Override - protected DOMStoreThreePhaseCommitCohort transactionReady(final SnapshotBackedWriteTransaction tx, final DataTreeModification modification) { + protected DOMStoreThreePhaseCommitCohort transactionReady(final SnapshotBackedWriteTransaction tx, + final DataTreeModification modification, + final Exception readyError) { LOG.debug("Tx: {} is submitted. Modifications: {}", tx.getIdentifier(), modification); - return new InMemoryDOMStoreThreePhaseCommitCohort(this, tx, modification); + return new InMemoryDOMStoreThreePhaseCommitCohort(this, tx, modification, readyError); } String nextIdentifier() { diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMStoreThreePhaseCommitCohort.java b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMStoreThreePhaseCommitCohort.java index 8c326262b0..a77732a11f 100644 --- a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMStoreThreePhaseCommitCohort.java +++ b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMStoreThreePhaseCommitCohort.java @@ -32,11 +32,16 @@ class InMemoryDOMStoreThreePhaseCommitCohort implements DOMStoreThreePhaseCommit private final DataTreeModification modification; private final InMemoryDOMDataStore store; private DataTreeCandidate candidate; + private final Exception operationError; - public InMemoryDOMStoreThreePhaseCommitCohort(final InMemoryDOMDataStore store, final SnapshotBackedWriteTransaction writeTransaction, final DataTreeModification modification) { + public InMemoryDOMStoreThreePhaseCommitCohort(final InMemoryDOMDataStore store, + final SnapshotBackedWriteTransaction writeTransaction, + final DataTreeModification modification, + final Exception operationError) { this.transaction = Preconditions.checkNotNull(writeTransaction); this.modification = Preconditions.checkNotNull(modification); this.store = Preconditions.checkNotNull(store); + this.operationError = operationError; } private static void warnDebugContext(final AbstractDOMStoreTransaction transaction) { @@ -48,6 +53,10 @@ class InMemoryDOMStoreThreePhaseCommitCohort implements DOMStoreThreePhaseCommit @Override public final ListenableFuture canCommit() { + if (operationError != null) { + return Futures.immediateFailedFuture(operationError); + } + try { store.validate(modification); LOG.debug("Store Transaction: {} can be committed", getTransaction().getIdentifier()); diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/test/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDataStoreTest.java b/opendaylight/md-sal/sal-inmemory-datastore/src/test/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDataStoreTest.java index 568f88376c..5734f9ac5b 100644 --- a/opendaylight/md-sal/sal-inmemory-datastore/src/test/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDataStoreTest.java +++ b/opendaylight/md-sal/sal-inmemory-datastore/src/test/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDataStoreTest.java @@ -16,10 +16,12 @@ import com.google.common.util.concurrent.CheckedFuture; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.MoreExecutors; import java.util.concurrent.ExecutionException; +import org.junit.Assert; import org.junit.Before; import org.junit.Ignore; import org.junit.Test; import org.mockito.Mockito; +import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException; import org.opendaylight.controller.sal.core.spi.data.DOMStoreReadTransaction; import org.opendaylight.controller.sal.core.spi.data.DOMStoreReadWriteTransaction; @@ -31,10 +33,12 @@ import org.opendaylight.controller.sal.core.spi.data.SnapshotBackedWriteTransact import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier; import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode; +import org.opendaylight.yangtools.yang.data.api.schema.DataContainerChild; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeModification; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeSnapshot; import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes; +import org.opendaylight.yangtools.yang.data.impl.schema.builder.api.NormalizedNodeContainerBuilder; import org.opendaylight.yangtools.yang.data.impl.schema.builder.impl.ImmutableContainerNodeBuilder; import org.opendaylight.yangtools.yang.model.api.SchemaContext; @@ -329,6 +333,23 @@ public class InMemoryDataStoreTest { writeTx.ready(); } + @Test + public void testReadyWithMissingMandatoryData() throws InterruptedException { + DOMStoreWriteTransaction writeTx = domStore.newWriteOnlyTransaction(); + NormalizedNode testNode = ImmutableContainerNodeBuilder.create() + .withNodeIdentifier(new NodeIdentifier(TestModel.MANDATORY_DATA_TEST_QNAME)) + .addChild(ImmutableNodes.leafNode(TestModel.OPTIONAL_QNAME, "data")) + .build(); + writeTx.write(TestModel.MANDATORY_DATA_TEST_PATH, testNode); + DOMStoreThreePhaseCommitCohort ready = writeTx.ready(); + try { + ready.canCommit().get(); + Assert.fail("Expected exception on canCommit"); + } catch (ExecutionException e) { + // nop + } + } + @Test public void testTransactionAbort() throws InterruptedException, ExecutionException { diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/test/java/org/opendaylight/controller/md/sal/dom/store/impl/TestModel.java b/opendaylight/md-sal/sal-inmemory-datastore/src/test/java/org/opendaylight/controller/md/sal/dom/store/impl/TestModel.java index 72ecf3e6a5..96749b037d 100644 --- a/opendaylight/md-sal/sal-inmemory-datastore/src/test/java/org/opendaylight/controller/md/sal/dom/store/impl/TestModel.java +++ b/opendaylight/md-sal/sal-inmemory-datastore/src/test/java/org/opendaylight/controller/md/sal/dom/store/impl/TestModel.java @@ -27,6 +27,16 @@ public class TestModel { public static final YangInstanceIdentifier TEST_PATH = YangInstanceIdentifier.of(TEST_QNAME); public static final YangInstanceIdentifier OUTER_LIST_PATH = YangInstanceIdentifier.builder(TEST_PATH).node(OUTER_LIST_QNAME).build(); + + public static final QName MANDATORY_DATA_TEST_QNAME = + QName.create("urn:opendaylight:params:xml:ns:yang:controller:md:sal:dom:store:test", + "2014-03-13", + "mandatory-data-test"); + public static final QName OPTIONAL_QNAME = QName.create(MANDATORY_DATA_TEST_QNAME, "optional-data"); + public static final QName MANDATORY_QNAME = QName.create(MANDATORY_DATA_TEST_QNAME, "mandatory-data"); + public static final YangInstanceIdentifier MANDATORY_DATA_TEST_PATH = + YangInstanceIdentifier.of(MANDATORY_DATA_TEST_QNAME); + private static final String DATASTORE_TEST_YANG = "/odl-datastore-test.yang"; public static SchemaContext createTestContext() { diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/test/resources/odl-datastore-test.yang b/opendaylight/md-sal/sal-inmemory-datastore/src/test/resources/odl-datastore-test.yang index 2d7601e0b8..ffc8fe07c8 100644 --- a/opendaylight/md-sal/sal-inmemory-datastore/src/test/resources/odl-datastore-test.yang +++ b/opendaylight/md-sal/sal-inmemory-datastore/src/test/resources/odl-datastore-test.yang @@ -41,4 +41,16 @@ module odl-datastore-test { } } } + + container mandatory-data-test { + presence "needs to be present when empty"; + + leaf optional-data { + type string; + } + leaf mandatory-data { + type string; + mandatory true; + } + } }