From ce0c4f4fc0557aaf46a73913a18b83ec29051570 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Fri, 1 Apr 2016 23:36:33 +0200 Subject: [PATCH] BUG-5626: Move leaderId/leaderPayloadVersion fields Handling of getLeaderId() depends on behavior: - for (Isolated)Leader, it is always its local ID - for Candidate it is always null - for Follower it can vary The same is true of leader payload version. This patch moves these two fields to Follower, makes Candidate return constants and makes Leader return the values stored in the local context. Change-Id: I0fca58ab985f14460eef9647a89ede02242e6e7c Signed-off-by: Robert Varga --- .../cluster/raft/RaftActorContextImpl.java | 1 + .../raft/behaviors/AbstractLeader.java | 11 ++--- .../behaviors/AbstractRaftActorBehavior.java | 23 +--------- .../cluster/raft/behaviors/Candidate.java | 24 +++++++---- .../cluster/raft/behaviors/Follower.java | 42 +++++++++++++++---- .../raft/behaviors/IsolatedLeader.java | 2 +- .../cluster/raft/behaviors/Leader.java | 2 +- .../cluster/raft/RaftActorTest.java | 2 +- 8 files changed, 59 insertions(+), 48 deletions(-) diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActorContextImpl.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActorContextImpl.java index d0deb7101a..fa851515d7 100644 --- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActorContextImpl.java +++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActorContextImpl.java @@ -86,6 +86,7 @@ public class RaftActorContextImpl implements RaftActorContext { } } + @VisibleForTesting public void setPayloadVersion(short payloadVersion) { this.payloadVersion = payloadVersion; } diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractLeader.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractLeader.java index cfafc2c84d..6065103b13 100644 --- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractLeader.java +++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractLeader.java @@ -95,15 +95,11 @@ public abstract class AbstractLeader extends AbstractRaftActorBehavior { protected AbstractLeader(RaftActorContext context, RaftState state) { super(context, state); - setLeaderPayloadVersion(context.getPayloadVersion()); - for(PeerInfo peerInfo: context.getPeers()) { FollowerLogInformation followerLogInformation = new FollowerLogInformationImpl(peerInfo, -1, context); followerToLog.put(peerInfo.getId(), followerLogInformation); } - leaderId = context.getId(); - LOG.debug("{}: Election: Leader has following peers: {}", logName(), getFollowerIds()); updateMinReplicaCount(); @@ -775,10 +771,15 @@ public abstract class AbstractLeader extends AbstractRaftActorBehavior { } @Override - public String getLeaderId() { + public final String getLeaderId() { return context.getId(); } + @Override + public final short getLeaderPayloadVersion() { + return context.getPayloadVersion(); + } + protected boolean isLeaderIsolated() { int minPresent = getMinIsolatedLeaderPeerCount(); for (FollowerLogInformation followerLogInformation : followerToLog.values()) { 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 43704d8cfe..0210dc4b1d 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 @@ -54,13 +54,6 @@ public abstract class AbstractRaftActorBehavior implements RaftActorBehavior { */ private Cancellable electionCancel = null; - /** - * - */ - protected String leaderId = null; - - private short leaderPayloadVersion = -1; - private long replicatedToAllIndex = -1; private final String logName; @@ -95,7 +88,7 @@ public abstract class AbstractRaftActorBehavior implements RaftActorBehavior { return state; } - public String logName() { + protected final String logName() { return logName; } @@ -428,20 +421,6 @@ public abstract class AbstractRaftActorBehavior implements RaftActorBehavior { return this; } - @Override - public String getLeaderId() { - return leaderId; - } - - @Override - public short getLeaderPayloadVersion() { - return leaderPayloadVersion; - } - - public void setLeaderPayloadVersion(short leaderPayloadVersion) { - this.leaderPayloadVersion = leaderPayloadVersion; - } - @Override public RaftActorBehavior switchBehavior(RaftActorBehavior behavior) { return internalSwitchBehavior(behavior); diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Candidate.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Candidate.java index 72e8e8c2a7..a5def02b1f 100644 --- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Candidate.java +++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Candidate.java @@ -70,11 +70,20 @@ public class Candidate extends AbstractRaftActorBehavior { } else { scheduleElection(electionDuration()); } + } + @Override + public final String getLeaderId() { + return null; + } + @Override + public final short getLeaderPayloadVersion() { + return -1; } - @Override protected RaftActorBehavior handleAppendEntries(ActorRef sender, + @Override + protected RaftActorBehavior handleAppendEntries(ActorRef sender, AppendEntries appendEntries) { if(LOG.isDebugEnabled()) { @@ -92,17 +101,14 @@ public class Candidate extends AbstractRaftActorBehavior { return this; } - @Override protected RaftActorBehavior handleAppendEntriesReply(ActorRef sender, - AppendEntriesReply appendEntriesReply) { - + @Override + protected RaftActorBehavior handleAppendEntriesReply(ActorRef sender, AppendEntriesReply appendEntriesReply) { return this; } - @Override protected RaftActorBehavior handleRequestVoteReply(ActorRef sender, - RequestVoteReply requestVoteReply) { - - LOG.debug("{}: handleRequestVoteReply: {}, current voteCount: {}", logName(), requestVoteReply, - voteCount); + @Override + protected RaftActorBehavior handleRequestVoteReply(ActorRef sender, RequestVoteReply requestVoteReply) { + LOG.debug("{}: handleRequestVoteReply: {}, current voteCount: {}", logName(), requestVoteReply, voteCount); if (requestVoteReply.isVoteGranted()) { voteCount++; 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 f483f0199e..2541dfab6e 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 @@ -11,6 +11,7 @@ package org.opendaylight.controller.cluster.raft.behaviors; import akka.actor.ActorRef; import akka.japi.Procedure; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; import java.util.ArrayList; import org.opendaylight.controller.cluster.raft.RaftActorContext; import org.opendaylight.controller.cluster.raft.RaftState; @@ -50,6 +51,8 @@ public class Follower extends AbstractRaftActorBehavior { }; private SnapshotTracker snapshotTracker = null; + private String leaderId; + private short leaderPayloadVersion; public Follower(RaftActorContext context) { this(context, null, (short)-1); @@ -57,8 +60,8 @@ public class Follower extends AbstractRaftActorBehavior { public Follower(RaftActorContext context, String initialLeaderId, short initialLeaderPayloadVersion) { super(context, RaftState.Follower); - leaderId = initialLeaderId; - setLeaderPayloadVersion(initialLeaderPayloadVersion); + this.leaderId = initialLeaderId; + this.leaderPayloadVersion = initialLeaderPayloadVersion; initialSyncStatusTracker = new SyncStatusTracker(context.getActor(), getId(), SYNC_THRESHOLD); @@ -69,7 +72,26 @@ public class Follower extends AbstractRaftActorBehavior { scheduleElection(electionDuration()); } } + } + + @Override + public final String getLeaderId() { + return leaderId; + } + + @VisibleForTesting + protected final void setLeaderId(final String leaderId) { + this.leaderId = Preconditions.checkNotNull(leaderId); + } + + @Override + public short getLeaderPayloadVersion() { + return leaderPayloadVersion; + } + @VisibleForTesting + protected final void setLeaderPayloadVersion(short leaderPayloadVersion) { + this.leaderPayloadVersion = leaderPayloadVersion; } private boolean isLogEntryPresent(long index){ @@ -103,8 +125,8 @@ public class Follower extends AbstractRaftActorBehavior { initialSyncStatusTracker.update(leaderId, currentLeaderCommit, context.getCommitIndex()); } - @Override protected RaftActorBehavior handleAppendEntries(ActorRef sender, - AppendEntries appendEntries) { + @Override + protected RaftActorBehavior handleAppendEntries(ActorRef sender, AppendEntries appendEntries) { int numLogEntries = appendEntries.getEntries() != null ? appendEntries.getEntries().size() : 0; if(LOG.isTraceEnabled()) { @@ -132,8 +154,7 @@ public class Follower extends AbstractRaftActorBehavior { // If we got here then we do appear to be talking to the leader leaderId = appendEntries.getLeaderId(); - - setLeaderPayloadVersion(appendEntries.getPayloadVersion()); + leaderPayloadVersion = appendEntries.getPayloadVersion(); updateInitialSyncStatus(appendEntries.getLeaderCommit(), appendEntries.getLeaderId()); // First check if the logs are in sync or not @@ -312,17 +333,20 @@ public class Follower extends AbstractRaftActorBehavior { return outOfSync; } - @Override protected RaftActorBehavior handleAppendEntriesReply(ActorRef sender, + @Override + protected RaftActorBehavior handleAppendEntriesReply(ActorRef sender, AppendEntriesReply appendEntriesReply) { return this; } - @Override protected RaftActorBehavior handleRequestVoteReply(ActorRef sender, + @Override + protected RaftActorBehavior handleRequestVoteReply(ActorRef sender, RequestVoteReply requestVoteReply) { return this; } - @Override public RaftActorBehavior handleMessage(ActorRef sender, Object originalMessage) { + @Override + public RaftActorBehavior handleMessage(ActorRef sender, Object originalMessage) { Object message = fromSerializableMessage(originalMessage); diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/IsolatedLeader.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/IsolatedLeader.java index a623660dfd..c969586722 100644 --- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/IsolatedLeader.java +++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/IsolatedLeader.java @@ -39,7 +39,7 @@ public class IsolatedLeader extends AbstractLeader { // it can happen that this isolated leader interacts with a new leader in the cluster and // changes its state to Follower, hence we only need to switch to Leader if the state is still Isolated if (ret.state() == RaftState.IsolatedLeader && !isLeaderIsolated()) { - LOG.info("IsolatedLeader {} switching from IsolatedLeader to Leader", leaderId); + LOG.info("IsolatedLeader {} switching from IsolatedLeader to Leader", getLeaderId()); return internalSwitchBehavior(RaftState.Leader); } return ret; diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Leader.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Leader.java index 6d55316586..c724e3bc1c 100644 --- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Leader.java +++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Leader.java @@ -65,7 +65,7 @@ public class Leader extends AbstractLeader { if (ISOLATED_LEADER_CHECK.equals(originalMessage) && isLeaderIsolated()) { LOG.warn("{}: At least {} followers need to be active, Switching {} from Leader to IsolatedLeader", - context.getId(), getMinIsolatedLeaderPeerCount(), leaderId); + context.getId(), getMinIsolatedLeaderPeerCount(), getLeaderId()); return internalSwitchBehavior(RaftState.IsolatedLeader); } diff --git a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/RaftActorTest.java b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/RaftActorTest.java index 99b17fe8ab..671d875b39 100644 --- a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/RaftActorTest.java +++ b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/RaftActorTest.java @@ -505,7 +505,7 @@ public class RaftActorTest extends AbstractActorTest { Follower follower = new Follower(raftActor.getRaftActorContext()) { @Override public RaftActorBehavior handleMessage(ActorRef sender, Object message) { - leaderId = newLeaderId; + setLeaderId(newLeaderId); setLeaderPayloadVersion(newLeaderVersion); return this; } -- 2.36.6