From 7eeebc36f171b7a56f2ebfc3eefbb66b51efe675 Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Sun, 30 Oct 2016 22:37:42 -0400 Subject: [PATCH] Changes for akka 2.4.11 - Fixed compile error in MeteringBehavior as signature of onReceive now declares Throwable. - Fixed intermittent failure in ShardManagerGetSnapshotReplyActorTest#testGetSnapshotTimeout. On failure, the actor would not get the ReceiveTimeout message from akka. However, oddly the test didn't fail if just testGetSnapshotTimeout was run - it would only fail if run with at least one of the other 2 tests. I tried creating a different ActorSystem for each test but that didn't fix it. I finally narrowed it down to creating the actors via the TestActorFactory although the exact reason is unknown. The tests don't really need to use the TestActorFactory so I remove its use. - Fixed intermittent failure in ShardTransactionTest#testShardTransactionInactivity. This was the same issue as the ShardManagerGetSnapshotReplyActorTest, i.e. the actor would not get the ReceiveTimeout message when no other message was sent. I found the TestActorFactory was again the culprit. I narrowed it down to the verifyActorReady method which creates an ActorSelection and sends an Identify message. This was put in to work around messages intermittently going to dead letters shortly after actor creaton. It seems this code somehow may interfere with the ReceiveTimeout functionality. So to workaround this, I allowed the caller to elide the verifyActorReady call. Change-Id: Ic8474def917414fa9bbe0f19b30b213f3052a1aa Signed-off-by: Tom Pantelis --- .../cluster/raft/TestActorFactory.java | 27 +++++++++++++---- .../common/actor/MeteringBehavior.java | 17 +++++++---- .../datastore/ShardTransactionTest.java | 2 +- ...ShardManagerGetSnapshotReplyActorTest.java | 30 ++++++------------- 4 files changed, 43 insertions(+), 33 deletions(-) diff --git a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/TestActorFactory.java b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/TestActorFactory.java index 92b401581c..e387584351 100644 --- a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/TestActorFactory.java +++ b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/TestActorFactory.java @@ -71,7 +71,7 @@ public class TestActorFactory implements AutoCloseable { */ public ActorRef createActor(Props props) { ActorRef actorRef = system.actorOf(props); - return addActor(actorRef); + return addActor(actorRef, true); } /** @@ -83,7 +83,19 @@ public class TestActorFactory implements AutoCloseable { */ public ActorRef createActor(Props props, String actorId) { ActorRef actorRef = system.actorOf(props, actorId); - return addActor(actorRef); + return addActor(actorRef, true); + } + + /** + * Create a normal actor with the passed in name w/o verifying that the actor is ready. + * + * @param props the actor Props + * @param actorId name of actor + * @return the ActorRef + */ + public ActorRef createActorNoVerify(Props props, String actorId) { + ActorRef actorRef = system.actorOf(props, actorId); + return addActor(actorRef, false); } /** @@ -97,7 +109,7 @@ public class TestActorFactory implements AutoCloseable { @SuppressWarnings("unchecked") public TestActorRef createTestActor(Props props, String actorId) { TestActorRef actorRef = TestActorRef.create(system, props, actorId); - return (TestActorRef) addActor(actorRef); + return (TestActorRef) addActor(actorRef, true); } /** @@ -110,12 +122,15 @@ public class TestActorFactory implements AutoCloseable { @SuppressWarnings("unchecked") public TestActorRef createTestActor(Props props) { TestActorRef actorRef = TestActorRef.create(system, props); - return (TestActorRef) addActor(actorRef); + return (TestActorRef) addActor(actorRef, true); } - private ActorRef addActor(T actorRef) { + private ActorRef addActor(T actorRef, boolean verify) { createdActors.add(actorRef); - verifyActorReady(actorRef); + if (verify) { + verifyActorReady(actorRef); + } + return actorRef; } diff --git a/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/common/actor/MeteringBehavior.java b/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/common/actor/MeteringBehavior.java index 45fe193338..5e631b90d3 100644 --- a/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/common/actor/MeteringBehavior.java +++ b/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/common/actor/MeteringBehavior.java @@ -12,6 +12,7 @@ import akka.japi.Procedure; import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.Timer; import com.google.common.base.Preconditions; +import com.google.common.base.Throwables; import org.opendaylight.controller.cluster.reporting.MetricsReporter; /** @@ -82,6 +83,7 @@ public class MeteringBehavior implements Procedure { * @param message the message to process * @throws Exception on message failure */ + @SuppressWarnings("checkstyle:IllegalCatch") @Override public void apply(final Object message) throws Exception { final String messageType = message.getClass().getSimpleName(); @@ -95,10 +97,15 @@ public class MeteringBehavior implements Procedure { final Timer.Context context = msgProcessingTimer.time(); final Timer.Context contextByMsgType = msgProcessingTimerByMsgType.time(); - meteredActor.onReceive(message); - - //stop timers - contextByMsgType.stop(); - context.stop(); + try { + meteredActor.onReceive(message); + } catch (Throwable e) { + Throwables.propagateIfPossible(e, Exception.class); + throw Throwables.propagate(e); + } finally { + //stop timers + contextByMsgType.stop(); + context.stop(); + } } } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTransactionTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTransactionTest.java index 1c4ef31e93..f8997106f6 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTransactionTest.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTransactionTest.java @@ -80,7 +80,7 @@ public class ShardTransactionTest extends AbstractActorTest { final AbstractShardDataTreeTransaction transaction, final String name) { Props props = ShardTransaction.props(type, transaction, shard, datastoreContext, shard.underlyingActor().getShardMBean()); - return actorFactory.createActor(props, name); + return actorFactory.createActorNoVerify(props, name); } private ReadOnlyShardDataTreeTransaction readOnlyTransaction() { diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/shardmanager/ShardManagerGetSnapshotReplyActorTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/shardmanager/ShardManagerGetSnapshotReplyActorTest.java index 84abaec8a9..23002ca532 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/shardmanager/ShardManagerGetSnapshotReplyActorTest.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/shardmanager/ShardManagerGetSnapshotReplyActorTest.java @@ -18,14 +18,12 @@ import java.util.Arrays; import java.util.List; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; -import org.junit.After; import org.junit.Test; import org.opendaylight.controller.cluster.access.concepts.MemberName; import org.opendaylight.controller.cluster.datastore.AbstractActorTest; import org.opendaylight.controller.cluster.datastore.identifiers.ShardIdentifier; import org.opendaylight.controller.cluster.datastore.messages.DatastoreSnapshot; import org.opendaylight.controller.cluster.datastore.messages.DatastoreSnapshot.ShardSnapshot; -import org.opendaylight.controller.cluster.raft.TestActorFactory; import org.opendaylight.controller.cluster.raft.client.messages.GetSnapshotReply; import scala.concurrent.duration.Duration; import scala.concurrent.duration.FiniteDuration; @@ -38,22 +36,14 @@ import scala.concurrent.duration.FiniteDuration; public class ShardManagerGetSnapshotReplyActorTest extends AbstractActorTest { private static final MemberName MEMBER_1 = MemberName.forName("member-1"); - private final TestActorFactory actorFactory = new TestActorFactory(getSystem()); - - @After - public void tearDown() { - actorFactory.close(); - } - @Test public void testSuccess() { JavaTestKit kit = new JavaTestKit(getSystem()); byte[] shardManagerSnapshot = new byte[]{0,5,9}; - ActorRef replyActor = actorFactory.createActor(ShardManagerGetSnapshotReplyActor.props( - Arrays.asList("shard1", "shard2", "shard3"), "config", - shardManagerSnapshot, kit.getRef(), "shard-manager", Duration.create(100, TimeUnit.SECONDS)), - actorFactory.generateActorId("actor")); + ActorRef replyActor = getSystem().actorOf(ShardManagerGetSnapshotReplyActor.props( + Arrays.asList("shard1", "shard2", "shard3"), "config", shardManagerSnapshot, kit.getRef(), + "shard-manager", Duration.create(100, TimeUnit.SECONDS)), "testSuccess"); kit.watch(replyActor); @@ -92,10 +82,9 @@ public class ShardManagerGetSnapshotReplyActorTest extends AbstractActorTest { JavaTestKit kit = new JavaTestKit(getSystem()); byte[] shardManagerSnapshot = new byte[]{0,5,9}; - ActorRef replyActor = actorFactory.createActor(ShardManagerGetSnapshotReplyActor.props( - Arrays.asList("shard1", "shard2"), "config", - shardManagerSnapshot, kit.getRef(), "shard-manager", Duration.create(100, TimeUnit.SECONDS)), - actorFactory.generateActorId("actor")); + ActorRef replyActor = getSystem().actorOf(ShardManagerGetSnapshotReplyActor.props( + Arrays.asList("shard1", "shard2"), "config", shardManagerSnapshot, kit.getRef(), "shard-manager", + Duration.create(100, TimeUnit.SECONDS)), "testGetSnapshotFailureReply"); kit.watch(replyActor); @@ -113,10 +102,9 @@ public class ShardManagerGetSnapshotReplyActorTest extends AbstractActorTest { JavaTestKit kit = new JavaTestKit(getSystem()); byte[] shardManagerSnapshot = new byte[]{0,5,9}; - ActorRef replyActor = actorFactory.createActor(ShardManagerGetSnapshotReplyActor.props( - Arrays.asList("shard1"), "config", - shardManagerSnapshot, kit.getRef(), "shard-manager", Duration.create(100, TimeUnit.MILLISECONDS)), - actorFactory.generateActorId("actor")); + ActorRef replyActor = getSystem().actorOf(ShardManagerGetSnapshotReplyActor.props( + Arrays.asList("shard1"), "config", shardManagerSnapshot, kit.getRef(), "shard-manager", + Duration.create(100, TimeUnit.MILLISECONDS)), "testGetSnapshotTimeout"); kit.watch(replyActor); -- 2.36.6