From: Tom Pantelis Date: Sat, 6 Jun 2015 12:06:48 +0000 (-0400) Subject: Bug 3372: Convert backend DataTree exceptions appropriately X-Git-Tag: release/beryllium~514 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=14f8732b144cbe12c97a31555e794938cbe96e62 Bug 3372: Convert backend DataTree exceptions appropriately Specifically, convert ConflictingModificationAppliedException to OptimisticLockFailedException and DataValidationFailedException to TransactionCommitFailedException. Change-Id: I7962e80d1fd51e818d17ed29a3e81262a78f9c3d Signed-off-by: Tom Pantelis (cherry picked from commit aa6342c2982aa30ffbcca6d1212de164041d9477) --- diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractShardDataTreeTransaction.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractShardDataTreeTransaction.java index dd8ec0b12a..e2e44b8d3d 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractShardDataTreeTransaction.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractShardDataTreeTransaction.java @@ -28,6 +28,10 @@ abstract class AbstractShardDataTreeTransaction { this.id = Preconditions.checkNotNull(id); } + String getId() { + return id; + } + final T getSnapshot() { return snapshot; } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardCommitCoordinator.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardCommitCoordinator.java index 1b838ae0e6..6821fa721f 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardCommitCoordinator.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardCommitCoordinator.java @@ -216,7 +216,8 @@ class ShardCommitCoordinator { * @param shard the transaction's shard actor */ void handleReadyLocalTransaction(ReadyLocalTransaction message, ActorRef sender, Shard shard) { - final ShardDataTreeCohort cohort = new SimpleShardDataTreeCohort(dataTree, message.getModification()); + final ShardDataTreeCohort cohort = new SimpleShardDataTreeCohort(dataTree, message.getModification(), + message.getTransactionID()); final CohortEntry cohortEntry = new CohortEntry(message.getTransactionID(), cohort); cohortCache.put(message.getTransactionID(), cohortEntry); cohortEntry.setDoImmediateCommit(message.isDoCommitOnReady()); 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 d83dd22fcf..e852cfe420 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 @@ -46,7 +46,7 @@ import org.slf4j.LoggerFactory; * This class is not part of the API contract and is subject to change at any time. */ @NotThreadSafe -public final class ShardDataTree extends ShardDataTreeTransactionParent { +public class ShardDataTree extends ShardDataTreeTransactionParent { private static final Logger LOG = LoggerFactory.getLogger(ShardDataTree.class); private static final ShardDataTreeNotificationManager MANAGER = new ShardDataTreeNotificationManager(); private final Map transactionChains = new HashMap<>(); @@ -180,7 +180,7 @@ public final class ShardDataTree extends ShardDataTreeTransactionParent { ShardDataTreeCohort finishTransaction(final ReadWriteShardDataTreeTransaction transaction) { final DataTreeModification snapshot = transaction.getSnapshot(); snapshot.ready(); - return new SimpleShardDataTreeCohort(this, snapshot); + return new SimpleShardDataTreeCohort(this, snapshot, transaction.getId()); } void recoveryDone(){ 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 2b6b39684a..4860e38e8f 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 @@ -11,8 +11,12 @@ import com.google.common.base.Preconditions; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import org.opendaylight.controller.cluster.datastore.utils.PruningDataTreeModification; +import org.opendaylight.controller.md.sal.common.api.data.OptimisticLockFailedException; +import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException; +import org.opendaylight.yangtools.yang.data.api.schema.tree.ConflictingModificationAppliedException; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidateTip; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeModification; +import org.opendaylight.yangtools.yang.data.api.schema.tree.DataValidationFailedException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -22,11 +26,14 @@ final class SimpleShardDataTreeCohort extends ShardDataTreeCohort { private static final ListenableFuture VOID_FUTURE = Futures.immediateFuture(null); private final DataTreeModification transaction; private final ShardDataTree dataTree; + private final String transactionId; private DataTreeCandidateTip candidate; - SimpleShardDataTreeCohort(final ShardDataTree dataTree, final DataTreeModification transaction) { + SimpleShardDataTreeCohort(final ShardDataTree dataTree, final DataTreeModification transaction, + final String transactionId) { this.dataTree = Preconditions.checkNotNull(dataTree); this.transaction = Preconditions.checkNotNull(transaction); + this.transactionId = transactionId; } @Override @@ -36,11 +43,25 @@ final class SimpleShardDataTreeCohort extends ShardDataTreeCohort { @Override public ListenableFuture canCommit() { + DataTreeModification modification = dataTreeModification(); try { - dataTree.getDataTree().validate(dataTreeModification()); + dataTree.getDataTree().validate(modification); LOG.trace("Transaction {} validated", transaction); return TRUE_FUTURE; + } + catch (ConflictingModificationAppliedException e) { + LOG.warn("Store Tx {}: Conflicting modification for path {}.", transactionId, e.getPath()); + return Futures.immediateFailedFuture(new OptimisticLockFailedException("Optimistic lock failed.", e)); + } catch (DataValidationFailedException e) { + LOG.warn("Store Tx {}: Data validation failed for path {}.", transactionId, e.getPath(), e); + + // For debugging purposes, allow dumping of the modification. Coupled with the above + // precondition log, it should allow us to understand what went on. + LOG.debug("Store Tx {}: modifications: {} tree: {}", transactionId, modification, dataTree.getDataTree()); + + return Futures.immediateFailedFuture(new TransactionCommitFailedException("Data did not pass validation.", e)); } catch (Exception e) { + LOG.warn("Unexpected failure in validation phase", e); return Futures.immediateFailedFuture(e); } } 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 new file mode 100644 index 0000000000..700f0c5eee --- /dev/null +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/SimpleShardDataTreeCohortTest.java @@ -0,0 +1,142 @@ +/* + * Copyright (c) 2015 Brocade Communications Systems, Inc. and others. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v1.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v10.html + */ +package org.opendaylight.controller.cluster.datastore; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertSame; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import com.google.common.util.concurrent.ListenableFuture; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.opendaylight.controller.md.sal.common.api.data.OptimisticLockFailedException; +import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException; +import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; +import org.opendaylight.yangtools.yang.data.api.schema.tree.ConflictingModificationAppliedException; +import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidateTip; +import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeModification; +import org.opendaylight.yangtools.yang.data.api.schema.tree.DataValidationFailedException; +import org.opendaylight.yangtools.yang.data.api.schema.tree.TipProducingDataTree; + +/** + * Unit tests for SimpleShardDataTreeCohort. + * + * @author Thomas Pantelis + */ +public class SimpleShardDataTreeCohortTest { + @Mock + private TipProducingDataTree mockDataTree; + + @Mock + private ShardDataTree mockShardDataTree; + + @Mock + private DataTreeModification mockModification; + + private SimpleShardDataTreeCohort cohort; + + @Before + public void setup() { + MockitoAnnotations.initMocks(this); + + doReturn(mockDataTree).when(mockShardDataTree).getDataTree(); + + cohort = new SimpleShardDataTreeCohort(mockShardDataTree, mockModification, "tx"); + } + + @Test + public void testCanCommitSuccess() throws Exception { + ListenableFuture future = cohort.canCommit(); + assertNotNull("Future is null", future); + assertEquals("Future", true, future.get(3, TimeUnit.SECONDS)); + verify(mockDataTree).validate(mockModification); + } + + @Test(expected=OptimisticLockFailedException.class) + public void testCanCommitWithConflictingModEx() throws Throwable { + doThrow(new ConflictingModificationAppliedException(YangInstanceIdentifier.EMPTY, "mock")). + when(mockDataTree).validate(mockModification); + try { + cohort.canCommit().get(); + } catch (ExecutionException e) { + throw e.getCause(); + } + } + + @Test(expected=TransactionCommitFailedException.class) + public void testCanCommitWithDataValidationEx() throws Throwable { + doThrow(new DataValidationFailedException(YangInstanceIdentifier.EMPTY, "mock")). + when(mockDataTree).validate(mockModification); + try { + cohort.canCommit().get(); + } catch (ExecutionException e) { + throw e.getCause(); + } + } + + @Test(expected=IllegalArgumentException.class) + public void testCanCommitWithIllegalArgumentEx() throws Throwable { + doThrow(new IllegalArgumentException("mock")).when(mockDataTree).validate(mockModification); + try { + cohort.canCommit().get(); + } catch (ExecutionException e) { + throw e.getCause(); + } + } + + @Test + public void testPreCommitAndCommitSuccess() throws Exception { + DataTreeCandidateTip mockCandidate = mock(DataTreeCandidateTip.class); + doReturn(mockCandidate ).when(mockDataTree).prepare(mockModification); + + ListenableFuture future = cohort.preCommit(); + assertNotNull("Future is null", future); + future.get(); + verify(mockDataTree).prepare(mockModification); + + assertSame("getCandidate", mockCandidate, cohort.getCandidate()); + + future = cohort.commit(); + assertNotNull("Future is null", future); + future.get(); + verify(mockDataTree).commit(mockCandidate); + } + + @Test(expected=IllegalArgumentException.class) + public void testPreCommitWithIllegalArgumentEx() throws Throwable { + doThrow(new IllegalArgumentException("mock")).when(mockDataTree).prepare(mockModification); + try { + cohort.preCommit().get(); + } catch (ExecutionException e) { + throw e.getCause(); + } + } + + @Test(expected=IllegalArgumentException.class) + public void testCommitWithIllegalArgumentEx() throws Throwable { + doThrow(new IllegalArgumentException("mock")).when(mockDataTree).commit(any(DataTreeCandidateTip.class)); + try { + cohort.commit().get(); + } catch (ExecutionException e) { + throw e.getCause(); + } + } + + @Test + public void testAbort() throws Exception { + cohort.abort().get(); + } +}