BUG 4212 : Follower should not reschedule election timeout in certain cases.

Before:
Follower rescheduled election whenever it received any message

Now:
Followe reschedules election only if
    - The message received is a RaftRPC message
    - If the RaftRPC message is a RequestVote then only reschedule
      if vote is granted

Change-Id: Ia59c65e4896d72dfc49e86e59b6a9e9331a945ca
Signed-off-by: Moiz Raja <moraja@cisco.com>
(cherry picked from commit bf63fbc347b7cf055e22c3711f22af9d286c6281)

diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractRaftActorBehavior.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractRaftActorBehavior.java
index a1bc3eea65a3003ff8d4ef38aa3a3cbbc44519a6..fd80d66ebfbc274aa676d098afba452547d7688a 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractRaftActorBehavior.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractRaftActorBehavior.java
@@ -168,6 +168,22 @@ public abstract class AbstractRaftActorBehavior implements RaftActorBehavior {
 
         LOG.debug("{}: In requestVote:  {}", logName(), requestVote);
 
+        boolean grantVote = canGrantVote(requestVote);
+
+        if(grantVote) {
+            context.getTermInformation().updateAndPersist(requestVote.getTerm(), requestVote.getCandidateId());
+        }
+
+        RequestVoteReply reply = new RequestVoteReply(currentTerm(), grantVote);
+
+        LOG.debug("{}: requestVote returning: {}", logName(), reply);
+
+        sender.tell(reply, actor());
+
+        return this;
+    }
+
+    protected boolean canGrantVote(RequestVote requestVote){
         boolean grantVote = false;
 
         //  Reply false if term < currentTerm (§5.1)
@@ -177,7 +193,7 @@ public abstract class AbstractRaftActorBehavior implements RaftActorBehavior {
             // If votedFor is null or candidateId, and candidate’s log is at
             // least as up-to-date as receiver’s log, grant vote (§5.2, §5.4)
         } else if (votedFor() == null || votedFor()
-            .equals(requestVote.getCandidateId())) {
+                .equals(requestVote.getCandidateId())) {
 
             boolean candidateLatest = false;
 
@@ -191,24 +207,15 @@ public abstract class AbstractRaftActorBehavior implements RaftActorBehavior {
             if (requestVote.getLastLogTerm() > lastTerm()) {
                 candidateLatest = true;
             } else if ((requestVote.getLastLogTerm() == lastTerm())
-                && requestVote.getLastLogIndex() >= lastIndex()) {
+                    && requestVote.getLastLogIndex() >= lastIndex()) {
                 candidateLatest = true;
             }
 
             if (candidateLatest) {
                 grantVote = true;
-                context.getTermInformation().updateAndPersist(requestVote.getTerm(),
-                    requestVote.getCandidateId());
             }
         }
-
-        RequestVoteReply reply = new RequestVoteReply(currentTerm(), grantVote);
-
-        LOG.debug("{}: requestVote returning: {}", logName(), reply);
-
-        sender.tell(reply, actor());
-
-        return this;
+        return grantVote;
     }
 
     /**

diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Follower.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Follower.java
index c1d261c56192cb0b1bbe6905b77faa7d1d8fad0c..d4755e63586f564cd71d106529cef910074b8cba 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Follower.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Follower.java
@@ -23,6 +23,7 @@ import org.opendaylight.controller.cluster.raft.messages.AppendEntriesReply;
 import org.opendaylight.controller.cluster.raft.messages.InstallSnapshot;
 import org.opendaylight.controller.cluster.raft.messages.InstallSnapshotReply;
 import org.opendaylight.controller.cluster.raft.messages.RaftRPC;
+import org.opendaylight.controller.cluster.raft.messages.RequestVote;
 import org.opendaylight.controller.cluster.raft.messages.RequestVoteReply;
 
 /**
@@ -37,8 +38,6 @@ import org.opendaylight.controller.cluster.raft.messages.RequestVoteReply;
  */
 public class Follower extends AbstractRaftActorBehavior {
 
-
-
     private SnapshotTracker snapshotTracker = null;
 
     private final InitialSyncStatusTracker initialSyncStatusTracker;
@@ -321,7 +320,9 @@ public class Follower extends AbstractRaftActorBehavior {
             handleInstallSnapshot(sender, installSnapshot);
         }
 
-        scheduleElection(electionDuration());
+        if(message instanceof RaftRPC && (!(message instanceof RequestVote) || (canGrantVote((RequestVote) message)))){
+            scheduleElection(electionDuration());
+        }
 
         return super.handleMessage(sender, message);
     }

diff --git a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/FollowerTest.java b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/FollowerTest.java
index 5b0841f1be93443125f8fa892d85e512699335d6..42e35d576731baf6484624743cec160f197d7c59 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/FollowerTest.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/FollowerTest.java
@@ -72,7 +72,7 @@ public class FollowerTest extends AbstractRaftActorBehaviorTest {
 
     @Override
     protected RaftActorBehavior createBehavior(RaftActorContext actorContext) {
-        return new Follower(actorContext);
+        return new TestFollower(actorContext);
     }
 
     @Override
@@ -87,6 +87,13 @@ public class FollowerTest extends AbstractRaftActorBehaviorTest {
         return context;
     }
 
+    private int getElectionTimeoutCount(RaftActorBehavior follower){
+        if(follower instanceof TestFollower){
+            return ((TestFollower) follower).getElectionTimeoutCount();
+        }
+        return -1;
+    }
+
     @Test
     public void testThatAnElectionTimeoutIsTriggered(){
         MockRaftActorContext actorContext = createActorContext();
@@ -123,6 +130,7 @@ public class FollowerTest extends AbstractRaftActorBehaviorTest {
 
         assertEquals("isVoteGranted", true, reply.isVoteGranted());
         assertEquals("getTerm", term, reply.getTerm());
+        assertEquals("schedule election", 1, getElectionTimeoutCount(follower));
     }
 
     @Test
@@ -140,6 +148,7 @@ public class FollowerTest extends AbstractRaftActorBehaviorTest {
         RequestVoteReply reply = MessageCollectorActor.expectFirstMatching(leaderActor, RequestVoteReply.class);
 
         assertEquals("isVoteGranted", false, reply.isVoteGranted());
+        assertEquals("schedule election", 0, getElectionTimeoutCount(follower));
     }
 
 
@@ -198,7 +207,7 @@ public class FollowerTest extends AbstractRaftActorBehaviorTest {
 
         MockRaftActorContext context = createActorContext();
         context.getReplicatedLog().clear(0,2);
-        context.getReplicatedLog().append(newReplicatedLogEntry(1,100, "bar"));
+        context.getReplicatedLog().append(newReplicatedLogEntry(1, 100, "bar"));
         context.getReplicatedLog().setSnapshotIndex(99);
 
         List<ReplicatedLogEntry> entries = Arrays.asList(
@@ -911,6 +920,26 @@ public class FollowerTest extends AbstractRaftActorBehaviorTest {
         MessageCollectorActor.assertNoneMatching(followerActor, ElectionTimeout.class, 500);
     }
 
+    @Test
+    public void testElectionScheduledWhenAnyRaftRPCReceived(){
+        MockRaftActorContext context = createActorContext();
+        follower = createBehavior(context);
+        follower.handleMessage(leaderActor, new RaftRPC() {
+            @Override
+            public long getTerm() {
+                return 100;
+            }
+        });
+        assertEquals("schedule election", 1, getElectionTimeoutCount(follower));
+    }
+
+    @Test
+    public void testElectionNotScheduledWhenNonRaftRPCMessageReceived(){
+        MockRaftActorContext context = createActorContext();
+        follower = createBehavior(context);
+        follower.handleMessage(leaderActor, "non-raft-rpc");
+        assertEquals("schedule election", 0, getElectionTimeoutCount(follower));
+    }
 
     public ByteString getNextChunk (ByteString bs, int offset, int chunkSize){
         int snapshotLength = bs.size();
@@ -969,4 +998,23 @@ public class FollowerTest extends AbstractRaftActorBehaviorTest {
         AppendEntriesReply reply = MessageCollectorActor.expectFirstMatching(replyActor, AppendEntriesReply.class);
         assertEquals("isSuccess", true, reply.isSuccess());
     }
+
+    private static class TestFollower extends Follower {
+
+        int electionTimeoutCount = 0;
+
+        public TestFollower(RaftActorContext context) {
+            super(context);
+        }
+
+        @Override
+        protected void scheduleElection(FiniteDuration interval) {
+            electionTimeoutCount++;
+            super.scheduleElection(interval);
+        }
+
+        public int getElectionTimeoutCount() {
+            return electionTimeoutCount;
+        }
+    }
 }