BUG-5626: Move leaderId/leaderPayloadVersion fields 24/37024/5
authorRobert Varga <rovarga@cisco.com>
Fri, 1 Apr 2016 21:36:33 +0000 (23:36 +0200)
committerRobert Varga <rovarga@cisco.com>
Mon, 4 Apr 2016 17:38:16 +0000 (19:38 +0200)
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 <rovarga@cisco.com>
opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActorContextImpl.java
opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractLeader.java
opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractRaftActorBehavior.java
opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Candidate.java
opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Follower.java
opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/IsolatedLeader.java
opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Leader.java
opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/RaftActorTest.java

index d0deb71..fa85151 100644 (file)
@@ -86,6 +86,7 @@ public class RaftActorContextImpl implements RaftActorContext {
         }
     }
 
+    @VisibleForTesting
     public void setPayloadVersion(short payloadVersion) {
         this.payloadVersion = payloadVersion;
     }
index cfafc2c..6065103 100644 (file)
@@ -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()) {
index 43704d8..0210dc4 100644 (file)
@@ -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);
index 72e8e8c..a5def02 100644 (file)
@@ -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++;
index f483f01..2541dfa 100644 (file)
@@ -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);
 
index a623660..c969586 100644 (file)
@@ -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;
index 6d55316..c724e3b 100644 (file)
@@ -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);
         }
index 99b17fe..671d875 100644 (file)
@@ -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;
                 }