From: Moiz Raja Date: Sun, 3 Aug 2014 19:23:16 +0000 (-0700) Subject: Return a NoOpTransactionContext when the Primary for a shard is not found X-Git-Tag: release/helium~352 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=c5511f1c95ce9ce754e78df840b56e8681bbae2d Return a NoOpTransactionContext when the Primary for a shard is not found A NoOpTransactionContext is returned when a request to create a transaction results in the primary not being found or when the request times out. A CanCommit on the NoOpTransaction will always false. A read will always return an empty result. The consumer will need to deal with this situation. Change-Id: Id6f7d0688921acac718b9bb06fbf03d5e345d7ac Signed-off-by: Moiz Raja --- diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/TransactionProxy.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/TransactionProxy.java index b434c9e463..333a8f8617 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/TransactionProxy.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/TransactionProxy.java @@ -12,12 +12,11 @@ import akka.actor.ActorPath; import akka.actor.ActorRef; import akka.actor.ActorSelection; import akka.actor.Props; - import com.google.common.base.Optional; import com.google.common.util.concurrent.CheckedFuture; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListeningExecutorService; - +import org.opendaylight.controller.cluster.datastore.exceptions.PrimaryNotFoundException; import org.opendaylight.controller.cluster.datastore.exceptions.TimeoutException; import org.opendaylight.controller.cluster.datastore.messages.CloseTransaction; import org.opendaylight.controller.cluster.datastore.messages.CreateTransaction; @@ -200,9 +199,10 @@ public class TransactionProxy implements DOMStoreReadWriteTransaction { remoteTransactionPaths.put(shardName, transactionContext); } - } catch(TimeoutException e){ + } catch(TimeoutException | PrimaryNotFoundException e){ LOG.error("Creating NoOpTransaction because of : {}", e.getMessage()); - remoteTransactionPaths.put(shardName, new NoOpTransactionContext(shardName)); + remoteTransactionPaths.put(shardName, + new NoOpTransactionContext(shardName)); } } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/DistributedDataStoreIntegrationTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/DistributedDataStoreIntegrationTest.java index 5f4ac57da0..0a0c04b915 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/DistributedDataStoreIntegrationTest.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/DistributedDataStoreIntegrationTest.java @@ -5,7 +5,7 @@ import akka.testkit.JavaTestKit; import com.google.common.base.Optional; import com.google.common.util.concurrent.ListenableFuture; - +import junit.framework.Assert; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -56,9 +56,7 @@ public class DistributedDataStoreIntegrationTest{ distributedDataStore.onGlobalContextUpdated(TestModel.createTestContext()); - // This sleep is fragile - test can fail intermittently if all Shards aren't updated with - // the SchemaContext in time. Is there any way we can make this deterministic? - Thread.sleep(2000); + Thread.sleep(1500); DOMStoreReadWriteTransaction transaction = distributedDataStore.newReadWriteTransaction(); @@ -70,6 +68,8 @@ public class DistributedDataStoreIntegrationTest{ Optional> optional = future.get(); + Assert.assertTrue(optional.isPresent()); + NormalizedNode normalizedNode = optional.get(); assertEquals(TestModel.TEST_QNAME, normalizedNode.getNodeType()); diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/TransactionProxyTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/TransactionProxyTest.java index 7d9d2dad81..0cd029c2ff 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/TransactionProxyTest.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/TransactionProxyTest.java @@ -13,6 +13,8 @@ import junit.framework.Assert; import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.opendaylight.controller.cluster.datastore.exceptions.PrimaryNotFoundException; +import org.opendaylight.controller.cluster.datastore.exceptions.TimeoutException; import org.opendaylight.controller.cluster.datastore.messages.CloseTransaction; import org.opendaylight.controller.cluster.datastore.messages.DeleteData; import org.opendaylight.controller.cluster.datastore.messages.MergeData; @@ -32,10 +34,17 @@ import org.opendaylight.controller.protobuff.messages.transaction.ShardTransacti import org.opendaylight.controller.sal.core.spi.data.DOMStoreThreePhaseCommitCohort; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes; +import scala.concurrent.duration.FiniteDuration; import java.util.List; import java.util.concurrent.Executors; +import static junit.framework.Assert.fail; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + public class TransactionProxyTest extends AbstractActorTest { private final Configuration configuration = new MockConfiguration(); @@ -121,6 +130,68 @@ public class TransactionProxyTest extends AbstractActorTest { Assert.assertFalse(normalizedNodeOptional.isPresent()); } + @Test + public void testReadWhenAPrimaryNotFoundExceptionIsThrown() throws Exception { + final ActorContext actorContext = mock(ActorContext.class); + + when(actorContext.executeShardOperation(anyString(), any(), any( + FiniteDuration.class))).thenThrow(new PrimaryNotFoundException("test")); + + TransactionProxy transactionProxy = + new TransactionProxy(actorContext, + TransactionProxy.TransactionType.READ_ONLY, transactionExecutor, TestModel.createTestContext()); + + + ListenableFuture>> read = + transactionProxy.read(TestModel.TEST_PATH); + + Assert.assertFalse(read.get().isPresent()); + + } + + + @Test + public void testReadWhenATimeoutExceptionIsThrown() throws Exception { + final ActorContext actorContext = mock(ActorContext.class); + + when(actorContext.executeShardOperation(anyString(), any(), any( + FiniteDuration.class))).thenThrow(new TimeoutException("test", new Exception("reason"))); + + TransactionProxy transactionProxy = + new TransactionProxy(actorContext, + TransactionProxy.TransactionType.READ_ONLY, transactionExecutor, TestModel.createTestContext()); + + + ListenableFuture>> read = + transactionProxy.read(TestModel.TEST_PATH); + + Assert.assertFalse(read.get().isPresent()); + + } + + @Test + public void testReadWhenAAnyOtherExceptionIsThrown() throws Exception { + final ActorContext actorContext = mock(ActorContext.class); + + when(actorContext.executeShardOperation(anyString(), any(), any( + FiniteDuration.class))).thenThrow(new NullPointerException()); + + TransactionProxy transactionProxy = + new TransactionProxy(actorContext, + TransactionProxy.TransactionType.READ_ONLY, transactionExecutor, TestModel.createTestContext()); + + + try { + ListenableFuture>> read = + transactionProxy.read(TestModel.TEST_PATH); + fail("A null pointer exception was expected"); + } catch(NullPointerException e){ + + } + } + + + @Test public void testWrite() throws Exception { final Props props = Props.create(MessageCollectorActor.class);