Do not expose the shutdown initiator in OnComplete 74/36874/3
authorRobert Varga <rovarga@cisco.com>
Wed, 30 Mar 2016 10:09:02 +0000 (12:09 +0200)
committerTom Pantelis <tpanteli@brocade.com>
Wed, 30 Mar 2016 13:50:32 +0000 (13:50 +0000)
The sender (replyTo) is not used from callbacks or anywhere else, it should
really be just internal to the RaftActor's onShutdown path. Also clarify Shutdown
message.

Change-Id: Ibc64e345e54c101f22ff360afc1d71d9d6a281f7
Signed-off-by: Robert Varga <rovarga@cisco.com>
opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActor.java
opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActorLeadershipTransferCohort.java
opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/client/messages/Shutdown.java
opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/RaftActorLeadershipTransferCohortTest.java

index ff464b0..43a9547 100644 (file)
@@ -274,15 +274,15 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor {
         LOG.debug("{}: Initiating leader transfer", persistenceId());
 
         if(leadershipTransferInProgress == null) {
-            leadershipTransferInProgress = new RaftActorLeadershipTransferCohort(this, getSender());
+            leadershipTransferInProgress = new RaftActorLeadershipTransferCohort(this);
             leadershipTransferInProgress.addOnComplete(new RaftActorLeadershipTransferCohort.OnComplete() {
                 @Override
-                public void onSuccess(ActorRef raftActorRef, ActorRef replyTo) {
+                public void onSuccess(ActorRef raftActorRef) {
                     leadershipTransferInProgress = null;
                 }
 
                 @Override
-                public void onFailure(ActorRef raftActorRef, ActorRef replyTo) {
+                public void onFailure(ActorRef raftActorRef) {
                     leadershipTransferInProgress = null;
                 }
             });
@@ -314,13 +314,13 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor {
         if (context.hasFollowers()) {
             initiateLeadershipTransfer(new RaftActorLeadershipTransferCohort.OnComplete() {
                 @Override
-                public void onSuccess(ActorRef raftActorRef, ActorRef replyTo) {
+                public void onSuccess(ActorRef raftActorRef) {
                     LOG.debug("{}: leader transfer succeeded - sending PoisonPill", persistenceId());
                     raftActorRef.tell(PoisonPill.getInstance(), raftActorRef);
                 }
 
                 @Override
-                public void onFailure(ActorRef raftActorRef, ActorRef replyTo) {
+                public void onFailure(ActorRef raftActorRef) {
                     LOG.debug("{}: leader transfer failed - sending PoisonPill", persistenceId());
                     raftActorRef.tell(PoisonPill.getInstance(), raftActorRef);
                 }
@@ -786,13 +786,13 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor {
         if (isLeader()) {
             initiateLeadershipTransfer(new RaftActorLeadershipTransferCohort.OnComplete() {
                 @Override
-                public void onSuccess(ActorRef raftActorRef, ActorRef replyTo) {
+                public void onSuccess(ActorRef raftActorRef) {
                     LOG.debug("{}: leader transfer succeeded after change to non-voting", persistenceId());
                     ensureFollowerState();
                 }
 
                 @Override
-                public void onFailure(ActorRef raftActorRef, ActorRef replyTo) {
+                public void onFailure(ActorRef raftActorRef) {
                     LOG.debug("{}: leader transfer failed after change to non-voting", persistenceId());
                     ensureFollowerState();
                 }
index d5dfcf3..2916867 100644 (file)
@@ -52,16 +52,14 @@ public class RaftActorLeadershipTransferCohort {
     private static final Logger LOG = LoggerFactory.getLogger(RaftActorLeadershipTransferCohort.class);
 
     private final RaftActor raftActor;
-    private final ActorRef replyTo;
     private Cancellable newLeaderTimer;
     private final List<OnComplete> onCompleteCallbacks = new ArrayList<>();
     private long newLeaderTimeoutInMillis = 2000;
     private final Stopwatch transferTimer = Stopwatch.createUnstarted();
     private boolean isTransferring;
 
-    RaftActorLeadershipTransferCohort(RaftActor raftActor, ActorRef replyTo) {
+    RaftActorLeadershipTransferCohort(RaftActor raftActor) {
         this.raftActor = raftActor;
-        this.replyTo = replyTo;
     }
 
     void init() {
@@ -170,9 +168,9 @@ public class RaftActorLeadershipTransferCohort {
 
         for(OnComplete onComplete: onCompleteCallbacks) {
             if(success) {
-                onComplete.onSuccess(raftActor.self(), replyTo);
+                onComplete.onSuccess(raftActor.self());
             } else {
-                onComplete.onFailure(raftActor.self(), replyTo);
+                onComplete.onFailure(raftActor.self());
             }
         }
     }
@@ -191,7 +189,7 @@ public class RaftActorLeadershipTransferCohort {
     }
 
     interface OnComplete {
-        void onSuccess(ActorRef raftActorRef, ActorRef replyTo);
-        void onFailure(ActorRef raftActorRef, ActorRef replyTo);
+        void onSuccess(ActorRef raftActorRef);
+        void onFailure(ActorRef raftActorRef);
     }
 }
index 754bf30..5b76685 100644 (file)
@@ -11,7 +11,8 @@ import java.io.Serializable;
 
 /**
  * Message sent to a raft actor to shutdown gracefully. If it's the leader it will transfer leadership to a
- * follower. As its last act, the actor self-destructs via a PoisonPill.
+ * follower. As its last act, the actor self-destructs via a PoisonPill. This message should only be used with
+ * Patterns.gracefulStop().
  *
  * @author Thomas Pantelis
  */
index 45c797c..754e3e4 100644 (file)
@@ -41,7 +41,7 @@ public class RaftActorLeadershipTransferCohortTest extends AbstractActorTest {
         mockRaftActor = factory.<MockRaftActor>createTestActor(MockRaftActor.builder().id(persistenceId).config(
                 config).pauseLeaderFunction(pauseLeaderFunction).props().withDispatcher(Dispatchers.DefaultDispatcherId()),
                 persistenceId).underlyingActor();
-        cohort = new RaftActorLeadershipTransferCohort(mockRaftActor, null);
+        cohort = new RaftActorLeadershipTransferCohort(mockRaftActor);
         cohort.addOnComplete(onComplete);
         mockRaftActor.waitForInitializeBehaviorComplete();
     }
@@ -52,15 +52,15 @@ public class RaftActorLeadershipTransferCohortTest extends AbstractActorTest {
         cohort.setNewLeaderTimeoutInMillis(20000);
 
         cohort.onNewLeader("new-leader");
-        verify(onComplete, never()).onSuccess(mockRaftActor.self(), null);
+        verify(onComplete, never()).onSuccess(mockRaftActor.self());
 
         cohort.transferComplete();
 
         cohort.onNewLeader(null);
-        verify(onComplete, never()).onSuccess(mockRaftActor.self(), null);
+        verify(onComplete, never()).onSuccess(mockRaftActor.self());
 
         cohort.onNewLeader("new-leader");
-        verify(onComplete).onSuccess(mockRaftActor.self(), null);
+        verify(onComplete).onSuccess(mockRaftActor.self());
     }
 
     @Test
@@ -68,7 +68,7 @@ public class RaftActorLeadershipTransferCohortTest extends AbstractActorTest {
         setup("testNewLeaderTimeout");
         cohort.setNewLeaderTimeoutInMillis(200);
         cohort.transferComplete();
-        verify(onComplete, timeout(3000)).onSuccess(mockRaftActor.self(), null);
+        verify(onComplete, timeout(3000)).onSuccess(mockRaftActor.self());
     }
 
     @Test
@@ -76,14 +76,14 @@ public class RaftActorLeadershipTransferCohortTest extends AbstractActorTest {
         config.setCustomRaftPolicyImplementationClass(DisableElectionsRaftPolicy.class.getName());
         setup("testNotLeaderOnDoTransfer");
         cohort.doTransfer();
-        verify(onComplete).onSuccess(mockRaftActor.self(), null);
+        verify(onComplete).onSuccess(mockRaftActor.self());
     }
 
     @Test
     public void testAbortTransfer() {
         setup("testAbortTransfer");
         cohort.abortTransfer();
-        verify(onComplete).onFailure(mockRaftActor.self(), null);
+        verify(onComplete).onFailure(mockRaftActor.self());
     }
 
     @Test
@@ -97,6 +97,6 @@ public class RaftActorLeadershipTransferCohortTest extends AbstractActorTest {
 
         setup("testPauseLeaderTimeout");
         cohort.init();
-        verify(onComplete, timeout(2000)).onFailure(mockRaftActor.self(), null);
+        verify(onComplete, timeout(2000)).onFailure(mockRaftActor.self());
     }
 }