Delay snapshot backed transaction ready error 25/68125/1
authorJaime Caamaño Ruiz <jcaamano@suse.com>
Fri, 9 Feb 2018 19:41:25 +0000 (20:41 +0100)
committerJaime Caamaño Ruiz <jcaamano@suse.com>
Fri, 9 Feb 2018 19:41:25 +0000 (20:41 +0100)
Delay snapshot backed transaction ready error to 3PC canCommit.

Change-Id: Ie4004db194483dfa1528258b464df8a2f8fa573e
JIRA: CONTROLLER-1812
Signed-off-by: Jaime Caamaño Ruiz <jcaamano@suse.com>
12 files changed:
dom/mdsal-dom-inmemory-datastore/src/main/java/org/opendaylight/mdsal/dom/store/inmemory/ChainedTransactionCommitImpl.java
dom/mdsal-dom-inmemory-datastore/src/main/java/org/opendaylight/mdsal/dom/store/inmemory/DOMStoreTransactionChainImpl.java
dom/mdsal-dom-inmemory-datastore/src/main/java/org/opendaylight/mdsal/dom/store/inmemory/InMemoryDOMDataStore.java
dom/mdsal-dom-inmemory-datastore/src/main/java/org/opendaylight/mdsal/dom/store/inmemory/InMemoryDOMStoreThreePhaseCommitCohort.java
dom/mdsal-dom-inmemory-datastore/src/test/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDataStoreTest.java
dom/mdsal-dom-inmemory-datastore/src/test/java/org/opendaylight/controller/md/sal/dom/store/impl/TestModel.java
dom/mdsal-dom-inmemory-datastore/src/test/java/org/opendaylight/mdsal/dom/store/inmemory/InMemoryDOMStoreThreePhaseCommitCohortTest.java
dom/mdsal-dom-inmemory-datastore/src/test/resources/odl-datastore-test.yang
dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/store/AbstractSnapshotBackedTransactionChain.java
dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/store/SnapshotBackedWriteTransaction.java
dom/mdsal-dom-spi/src/test/java/org/opendaylight/mdsal/dom/spi/store/AbstractSnapshotBackedTransactionChainTest.java
dom/mdsal-dom-spi/src/test/java/org/opendaylight/mdsal/dom/spi/store/SnapshotBackedWriteTransactionTest.java

index c3c9589ff0ce21efe2d174b7ab82720709bd2f24..a81ee295cb4138109e2b9c96399e6a7d1cd0b5a4 100644 (file)
@@ -16,9 +16,11 @@ final class ChainedTransactionCommitImpl extends InMemoryDOMStoreThreePhaseCommi
     private final DOMStoreTransactionChainImpl txChain;
 
     ChainedTransactionCommitImpl(final InMemoryDOMDataStore store,
-        final SnapshotBackedWriteTransaction<String> transaction,final DataTreeModification modification,
-            final DOMStoreTransactionChainImpl txChain) {
-        super(store, transaction, modification);
+                                 final SnapshotBackedWriteTransaction<String> transaction,
+                                 final DataTreeModification modification,
+                                 final DOMStoreTransactionChainImpl txChain,
+                                 final Exception operationError) {
+        super(store, transaction, modification, operationError);
         this.txChain = Preconditions.checkNotNull(txChain);
     }
 
index 7650b29a6973af5840bf62d03f18eec6862c753b..0018d0eb90e70dca8c449194c7c6f23bb3697b29 100644 (file)
@@ -23,8 +23,9 @@ final class DOMStoreTransactionChainImpl extends AbstractSnapshotBackedTransacti
 
     @Override
     protected DOMStoreThreePhaseCommitCohort createCohort(final SnapshotBackedWriteTransaction<String> tx,
-            final DataTreeModification modification) {
-        return new ChainedTransactionCommitImpl(store, tx, modification, this);
+                                                          final DataTreeModification modification,
+                                                          final Exception operationError) {
+        return new ChainedTransactionCommitImpl(store, tx, modification, this, operationError);
     }
 
     @Override
index 20f2f7d4cfe5cca5b68e77920827fdc9dc9a4e28..427a57b4527ac5a5f9450e5e269dfc4a31cfd5a0 100644 (file)
@@ -152,9 +152,10 @@ public class InMemoryDOMDataStore extends TransactionReadyPrototype<String> impl
 
     @Override
     protected DOMStoreThreePhaseCommitCohort transactionReady(final SnapshotBackedWriteTransaction<String> tx,
-            final DataTreeModification modification) {
+                                                              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() {
index 535623352a77d3bfb526e49f37e32b5055f4f422..6f16abb33d3c010b352d129aeac7df7661d5f668 100644 (file)
@@ -33,12 +33,16 @@ class InMemoryDOMStoreThreePhaseCommitCohort implements DOMStoreThreePhaseCommit
     private final DataTreeModification modification;
     private final InMemoryDOMDataStore store;
     private DataTreeCandidate candidate;
+    private final Exception operationError;
 
     InMemoryDOMStoreThreePhaseCommitCohort(final InMemoryDOMDataStore store,
-            final SnapshotBackedWriteTransaction<String> writeTransaction, final DataTreeModification modification) {
+                                           final SnapshotBackedWriteTransaction<String> 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) {
@@ -51,6 +55,10 @@ class InMemoryDOMStoreThreePhaseCommitCohort implements DOMStoreThreePhaseCommit
     @SuppressWarnings("checkstyle:IllegalCatch")
     @Override
     public final ListenableFuture<Boolean> canCommit() {
+        if (operationError != null) {
+            return Futures.immediateFailedFuture(operationError);
+        }
+
         try {
             store.validate(modification);
             LOG.debug("Store Transaction: {} can be committed", getTransaction().getIdentifier());
index 2476d22f63488be33321320387c5089527ec07c3..4547f6f801d0c05280ed019627c108de04f3f6af 100644 (file)
@@ -16,6 +16,7 @@ import com.google.common.base.Optional;
 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;
@@ -332,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 {
 
index e3944882d499e15d0376a74e4a48f083614b850b..be3bc98a128a8e1ef9f6e98eae8fe029300edd13 100644 (file)
@@ -28,6 +28,15 @@ public final class TestModel {
     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 TestModel() {
         throw new UnsupportedOperationException();
     }
index 1b87b0f937c3e657701bb512a1b5fcb9de40ee48..71523d4174c7d015f6df0ef30db253ff0e03b0cc 100644 (file)
@@ -15,14 +15,14 @@ import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.doThrow;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.verify;
+import static org.mockito.MockitoAnnotations.initMocks;
 
 import java.lang.reflect.Field;
 import java.util.concurrent.ExecutionException;
 import org.junit.Before;
 import org.junit.Test;
+import org.mockito.Mock;
 import org.opendaylight.mdsal.common.api.OptimisticLockFailedException;
 import org.opendaylight.mdsal.common.api.TransactionCommitFailedException;
 import org.opendaylight.mdsal.dom.spi.store.SnapshotBackedTransactions;
@@ -36,24 +36,34 @@ import org.opendaylight.yangtools.yang.data.api.schema.tree.DataValidationFailed
 
 public class InMemoryDOMStoreThreePhaseCommitCohortTest {
 
-    private static  InMemoryDOMStoreThreePhaseCommitCohort inMemoryDOMStoreThreePhaseCommitCohort = null;
-    private static final InMemoryDOMDataStore IN_MEMORY_DOM_DATA_STORE = mock(InMemoryDOMDataStore.class);
-    private static final DataTreeCandidate DATA_TREE_CANDIDATE = mock(DataTreeCandidate.class);
+    private static InMemoryDOMStoreThreePhaseCommitCohort inMemoryDOMStoreThreePhaseCommitCohort = null;
+
+    @Mock
+    private static InMemoryDOMDataStore IN_MEMORY_DOM_DATA_STORE;
+
+    @Mock
+    private static DataTreeCandidate DATA_TREE_CANDIDATE;
+
+    @Mock
+    private static TransactionReadyPrototype<String> TRANSACTION_READY_PROTOTYPE;
+
+    @Mock
+    private static DataTreeSnapshot DATA_TREE_SNAPSHOT;
+
+    @Mock
+    private static DataTreeModification DATA_TREE_MODIFICATION;
 
     @Before
     public void setUp() throws Exception {
-        reset(IN_MEMORY_DOM_DATA_STORE);
-        DataTreeSnapshot dataTreeSnapshot = mock(DataTreeSnapshot.class);
-        TransactionReadyPrototype<String> transactionReadyPrototype = mock(TransactionReadyPrototype.class);
-        DataTreeModification dataTreeModification = mock(DataTreeModification.class);
-        doReturn(dataTreeModification).when(dataTreeSnapshot).newModification();
-        doReturn("testModification").when(dataTreeModification).toString();
-
+        initMocks(this);
+        doReturn(DATA_TREE_MODIFICATION).when(DATA_TREE_SNAPSHOT).newModification();
+        doReturn("testModification").when(DATA_TREE_MODIFICATION).toString();
         inMemoryDOMStoreThreePhaseCommitCohort =
                 new InMemoryDOMStoreThreePhaseCommitCohort(IN_MEMORY_DOM_DATA_STORE,
-                        SnapshotBackedTransactions
-                                .newWriteTransaction("test", false, dataTreeSnapshot, transactionReadyPrototype),
-                        dataTreeModification);
+                        SnapshotBackedTransactions.newWriteTransaction(
+                                "test", false, DATA_TREE_SNAPSHOT, TRANSACTION_READY_PROTOTYPE),
+                        DATA_TREE_MODIFICATION,
+                        null);
     }
 
     @Test
@@ -63,6 +73,24 @@ public class InMemoryDOMStoreThreePhaseCommitCohortTest {
         verify(IN_MEMORY_DOM_DATA_STORE).validate(any());
     }
 
+    @Test
+    public void canCommitWithOperationError() throws Exception {
+        RuntimeException operationError = new RuntimeException();
+        inMemoryDOMStoreThreePhaseCommitCohort =
+                new InMemoryDOMStoreThreePhaseCommitCohort(IN_MEMORY_DOM_DATA_STORE,
+                        SnapshotBackedTransactions.newWriteTransaction(
+                                "test", false, DATA_TREE_SNAPSHOT, TRANSACTION_READY_PROTOTYPE),
+                        DATA_TREE_MODIFICATION,
+                        operationError);
+        doNothing().when(IN_MEMORY_DOM_DATA_STORE).validate(any());
+        try {
+            inMemoryDOMStoreThreePhaseCommitCohort.canCommit().get();
+            fail("Expected exception");
+        } catch (ExecutionException e) {
+            assertTrue(e.getCause() == operationError);
+        }
+    }
+
     @SuppressWarnings({ "checkstyle:IllegalThrows", "checkstyle:avoidHidingCauseException" })
     @Test(expected = OptimisticLockFailedException.class)
     public void canCommitTestWithOptimisticLockFailedException() throws Throwable {
index 93c3d4e0a73fb89e6bf280627ae3efb0af9a8919..b70a70e9d72227b2b05edbdb77ee803b1b3037b3 100644 (file)
@@ -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;
+        }
+    }
 }
\ No newline at end of file
index 5d28aa1b22650bf26a576ea1878efdb91a1f0aba..4301021eafb21c6290157c2e559b597059dafbab 100644 (file)
@@ -188,7 +188,9 @@ public abstract class AbstractSnapshotBackedTransactionChain<T>
 
     @Override
     protected final DOMStoreThreePhaseCommitCohort transactionReady(
-            final SnapshotBackedWriteTransaction<T> tx, final DataTreeModification tree) {
+            final SnapshotBackedWriteTransaction<T> tx,
+            final DataTreeModification tree,
+            final Exception readyError) {
         final State localState = state;
 
         if (localState instanceof Allocated) {
@@ -201,7 +203,7 @@ public abstract class AbstractSnapshotBackedTransactionChain<T>
             LOG.debug("Ignoring transaction {} readiness due to state {}", tx, localState);
         }
 
-        return createCohort(tx, tree);
+        return createCohort(tx, tree, readyError);
     }
 
     @Override
@@ -283,8 +285,10 @@ public abstract class AbstractSnapshotBackedTransactionChain<T>
      * Create a cohort for driving the transaction through the commit process.
      * @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(
-            SnapshotBackedWriteTransaction<T> transaction, DataTreeModification modification);
+    protected abstract DOMStoreThreePhaseCommitCohort createCohort(SnapshotBackedWriteTransaction<T> transaction,
+                                                                   DataTreeModification modification,
+                                                                   Exception operationError);
 }
index 5deb8fd2c5688c3e5cd701ae91e18918a92b8a0f..3ac0ae78c9b323829a98c11624644e2a60e99e14 100644 (file)
@@ -15,6 +15,7 @@ import com.google.common.base.Preconditions;
 import com.google.common.base.Throwables;
 import java.util.Optional;
 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;
@@ -136,6 +137,7 @@ public class SnapshotBackedWriteTransaction<T> extends AbstractDOMStoreTransacti
                 "Transaction %s is no longer open. No further modifications allowed.", getIdentifier());
     }
 
+    @SuppressWarnings("checkstyle:IllegalCatch")
     @Override
     public DOMStoreThreePhaseCommitCohort ready() {
         @SuppressWarnings("unchecked")
@@ -146,8 +148,13 @@ public class SnapshotBackedWriteTransaction<T> 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
@@ -195,9 +202,12 @@ public class SnapshotBackedWriteTransaction<T> 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<T> tx, DataTreeModification tree);
+        protected abstract DOMStoreThreePhaseCommitCohort transactionReady(SnapshotBackedWriteTransaction<T> tx,
+                                                                           DataTreeModification tree,
+                                                                           @Nullable Exception readyError);
     }
 }
index 29a3f3fd7612e4102587c05b1f7b7910512670d0..ddec0b94f137ab1072cb354ec2151f1c0f98e2b4 100644 (file)
@@ -39,7 +39,7 @@ public class AbstractSnapshotBackedTransactionChainTest extends AbstractSnapshot
         this.newWriteOnlyTransaction().close();
         this.newReadWriteTransaction().close();
 
-        this.transactionReady(snapshotBackedWriteTransaction, dataTreeModification);
+        this.transactionReady(snapshotBackedWriteTransaction, dataTreeModification, null);
 
 
         this.transactionAborted(snapshotBackedWriteTransaction);
@@ -67,7 +67,8 @@ public class AbstractSnapshotBackedTransactionChainTest extends AbstractSnapshot
 
     @Override
     protected DOMStoreThreePhaseCommitCohort createCohort(final SnapshotBackedWriteTransaction<Object> transaction,
-                                                          final DataTreeModification modification) {
+                                                          final DataTreeModification modification,
+                                                          final Exception operationError) {
         return domStoreThreePhaseCommitCohort;
     }
 }
\ No newline at end of file
index 3962dc56b1ac4ac4e3d245c5df3c28947d453227..07e5b0c8e3c96661109011a162ece8a9ee064fa5 100644 (file)
@@ -10,6 +10,8 @@ package org.opendaylight.mdsal.dom.spi.store;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Matchers.same;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.doThrow;
@@ -49,7 +51,9 @@ public class SnapshotBackedWriteTransactionTest {
         doNothing().when(TRANSACTION_READY_PROTOTYPE).transactionAborted(any());
         doReturn("testDataTreeModification").when(DATA_TREE_MODIFICATION).toString();
         doReturn("testNormalizedNode").when(NORMALIZED_NODE).toString();
-        doReturn(DOM_STORE_THREE_PHASE_COMMIT_COHORT).when(TRANSACTION_READY_PROTOTYPE).transactionReady(any(),any());
+        doReturn(DOM_STORE_THREE_PHASE_COMMIT_COHORT)
+                .when(TRANSACTION_READY_PROTOTYPE)
+                .transactionReady(any(),any(), any());
         doReturn(NORMALIZED_NODE_OPTIONAL).when(DATA_TREE_MODIFICATION).readNode(YangInstanceIdentifier.EMPTY);
         snapshotBackedWriteTransaction = new SnapshotBackedWriteTransaction<>(new Object(), false, DATA_TREE_SNAPSHOT,
                 TRANSACTION_READY_PROTOTYPE);
@@ -80,10 +84,18 @@ public class SnapshotBackedWriteTransactionTest {
         SnapshotBackedWriteTransaction<Object> tx = new SnapshotBackedWriteTransaction<>(new Object(), false,
                 DATA_TREE_SNAPSHOT, TRANSACTION_READY_PROTOTYPE);
         Assert.assertNotNull(tx.ready());
-        verify(TRANSACTION_READY_PROTOTYPE).transactionReady(any(), any());
+        verify(TRANSACTION_READY_PROTOTYPE).transactionReady(any(), any(), eq(null));
         tx.close();
     }
 
+    @Test
+    public void readyWithException() {
+        Exception thrown = new RuntimeException();
+        doThrow(thrown).when(DATA_TREE_MODIFICATION).ready();
+        Assert.assertNotNull(snapshotBackedWriteTransaction.ready());
+        verify(TRANSACTION_READY_PROTOTYPE).transactionReady(any(), any(), same(thrown));
+    }
+
     @Test(expected = IllegalArgumentException.class)
     public void writeWithException() throws Exception {
         doThrow(TestException.class).when(DATA_TREE_MODIFICATION).write(any(), any());