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 <tpanteli@brocade.com>

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 92b401581c3721bd8d978d1b0faf33f712c7d70d..e3875843515c8fca320443196d31e89ff02b0008 100644 (file)
--- 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 <T extends Actor> TestActorRef<T> createTestActor(Props props, String actorId) {
         TestActorRef<T> actorRef = TestActorRef.create(system, props, actorId);
-        return (TestActorRef<T>) addActor(actorRef);
+        return (TestActorRef<T>) addActor(actorRef, true);
     }
 
     /**
@@ -110,12 +122,15 @@ public class TestActorFactory implements AutoCloseable {
     @SuppressWarnings("unchecked")
     public <T extends Actor> TestActorRef<T> createTestActor(Props props) {
         TestActorRef<T> actorRef = TestActorRef.create(system, props);
-        return (TestActorRef<T>) addActor(actorRef);
+        return (TestActorRef<T>) addActor(actorRef, true);
     }
 
-    private <T extends ActorRef> ActorRef addActor(T actorRef) {
+    private <T extends ActorRef> 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 45fe19333835e83feea0bee6a15a41f6e7959a61..5e631b90d36eb7c1e273a604dfdb283d58b3e4f3 100644 (file)
--- 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<Object> {
      * @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<Object> {
         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 1c4ef31e93807831e0bc10fb27b702a1073ea8bf..f8997106f62e6124f0aedfcf0bd6abd4b09966e6 100644 (file)
--- 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 84abaec8a9ae035de90c34123c273d1b0bc4ef56..23002ca532831faf69871390a7c6cc52fbfbd35e 100644 (file)
--- 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);