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 <rovarga@cisco.com>

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 d0deb7101a2f05529539d09107bc3c9ee52a9d49..fa851515d70d94d7a8b7411652519926d5d85a1e 100644 (file)
--- 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 cfafc2c84d8a12971b172d2fa4594af5a0f36435..6065103b13dbd4d54d1e1ffb9352e9c2b072952a 100644 (file)
--- 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 43704d8cfed1ccc2c95266164258c6ad7999f7a1..0210dc4b1d0ed703a7e76fb8651f97b908563629 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
@@ -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 72e8e8c2a70b50ed5022b392918bbadc58697eba..a5def02b1f5ad3301edfdf7bca2669fc8ab2256e 100644 (file)
--- 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 f483f0199e393e84b9d31bc598279f26f1d4a749..2541dfab6edc6215086580d105b28891257abd1b 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
@@ -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 a623660dfd4b97d6b868a9aa8751d9cb645c1ab0..c969586722d3a849a7fd51ebf1d02c51463d4db3 100644 (file)
--- 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 6d55316586cf85c725bb8e39953ee64b4f08217c..c724e3bc1c8657be7c25ee7bed93689b7925e25e 100644 (file)
--- 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 99b17fe8ab2ac9d8b303504f98bde766dae6cf60..671d875b393f328f3a1003d4f91ef1d686f92884 100644 (file)
--- 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;
                 }