From bf63fbc347b7cf055e22c3711f22af9d286c6281 Mon Sep 17 00:00:00 2001 From: Moiz Raja Date: Thu, 27 Aug 2015 10:34:36 -0700 Subject: [PATCH] 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 --- .../behaviors/AbstractRaftActorBehavior.java | 31 ++++++----- .../cluster/raft/behaviors/Follower.java | 7 +-- .../cluster/raft/behaviors/FollowerTest.java | 52 ++++++++++++++++++- 3 files changed, 73 insertions(+), 17 deletions(-) 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 8692e9948a..6ef4914477 100644 --- 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 c1d261c561..d4755e6358 100644 --- 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 2cb0d7ce65..62b2112d79 100644 --- 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 @@ -64,7 +64,7 @@ public class FollowerTest extends AbstractRaftActorBehaviorTest { @Override protected RaftActorBehavior createBehavior(RaftActorContext actorContext) { - return new Follower(actorContext); + return new TestFollower(actorContext); } @Override @@ -79,6 +79,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(); @@ -115,6 +122,7 @@ public class FollowerTest extends AbstractRaftActorBehaviorTest { assertEquals("isVoteGranted", true, reply.isVoteGranted()); assertEquals("getTerm", term, reply.getTerm()); + assertEquals("schedule election", 1, getElectionTimeoutCount(follower)); } @Test @@ -132,6 +140,7 @@ public class FollowerTest extends AbstractRaftActorBehaviorTest { RequestVoteReply reply = MessageCollectorActor.expectFirstMatching(leaderActor, RequestVoteReply.class); assertEquals("isVoteGranted", false, reply.isVoteGranted()); + assertEquals("schedule election", 0, getElectionTimeoutCount(follower)); } @@ -190,7 +199,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 entries = Arrays.asList( @@ -903,6 +912,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(); @@ -961,4 +990,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; + } + } } -- 2.36.6