Do not leak AtomicLong from FollowerLogInformation

Leaking this implementation details allows callers to interact with
state outside of the official contract. Disallow that.

Change-Id: I880310f2a9692fd11ba2cd33830501d19d0a1b65
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/FollowerLogInformation.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/FollowerLogInformation.java
index 4d2bad5c2effe1f55a7abb3e74786e30390abaab..6d0c14e733a8c81bb2e29a663915eb5776422a6d 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/FollowerLogInformation.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/FollowerLogInformation.java
@@ -5,11 +5,8 @@
  * terms of the Eclipse Public License v1.0 which accompanies this distribution,
  * and is available at http://www.eclipse.org/legal/epl-v10.html
  */
-
 package org.opendaylight.controller.cluster.raft;
 
-import java.util.concurrent.atomic.AtomicLong;
-
 /**
  * The state of the followers log as known by the Leader
  */
@@ -19,13 +16,13 @@ public interface FollowerLogInformation {
      * Increment the value of the nextIndex
      * @return
      */
-    public long incrNextIndex();
+    long incrNextIndex();
 
     /**
      * Decrement the value of the nextIndex
      * @return
      */
-    public long decrNextIndex();
+    long decrNextIndex();
 
     /**
      *
@@ -37,45 +34,43 @@ public interface FollowerLogInformation {
      * Increment the value of the matchIndex
      * @return
      */
-    public long incrMatchIndex();
+    long incrMatchIndex();
 
-    public void setMatchIndex(long matchIndex);
+    void setMatchIndex(long matchIndex);
 
     /**
      * The identifier of the follower
      * This could simply be the url of the remote actor
      */
-    public String getId();
+    String getId();
 
     /**
      * for each server, index of the next log entry
      * to send to that server (initialized to leader
      *    last log index + 1)
      */
-    public AtomicLong getNextIndex();
+    long getNextIndex();
 
     /**
      * for each server, index of highest log entry
      * known to be replicated on server
      *    (initialized to 0, increases monotonically)
      */
-    public AtomicLong getMatchIndex();
+    long getMatchIndex();
 
     /**
      * Checks if the follower is active by comparing the last updated with the duration
      * @return boolean
      */
-    public boolean isFollowerActive();
+    boolean isFollowerActive();
 
     /**
      * restarts the timeout clock of the follower
      */
-    public void markFollowerActive();
+    void markFollowerActive();
 
     /**
      * This will stop the timeout clock
      */
-    public void markFollowerInActive();
-
-
+    void markFollowerInActive();
 }

diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/FollowerLogInformationImpl.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/FollowerLogInformationImpl.java
index 9820638ab706ba0753da5ff1d574bc657da9b962..91a5dddb981b3f7b09e9a79dd44fa0c09d4448a2 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/FollowerLogInformationImpl.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/FollowerLogInformationImpl.java
@@ -9,10 +9,9 @@
 package org.opendaylight.controller.cluster.raft;
 
 import com.google.common.base.Stopwatch;
-import scala.concurrent.duration.FiniteDuration;
-
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicLong;
+import scala.concurrent.duration.FiniteDuration;
 
 public class FollowerLogInformationImpl implements FollowerLogInformation {
 
@@ -35,6 +34,7 @@ public class FollowerLogInformationImpl implements FollowerLogInformation {
         this.followerTimeoutMillis = followerTimeoutDuration.toMillis();
     }
 
+    @Override
     public long incrNextIndex(){
         return nextIndex.incrementAndGet();
     }
@@ -49,6 +49,7 @@ public class FollowerLogInformationImpl implements FollowerLogInformation {
         this.nextIndex.set(nextIndex);
     }
 
+    @Override
     public long incrMatchIndex(){
         return matchIndex.incrementAndGet();
     }
@@ -58,16 +59,19 @@ public class FollowerLogInformationImpl implements FollowerLogInformation {
         this.matchIndex.set(matchIndex);
     }
 
+    @Override
     public String getId() {
         return id;
     }
 
-    public AtomicLong getNextIndex() {
-        return nextIndex;
+    @Override
+    public long getNextIndex() {
+        return nextIndex.get();
     }
 
-    public AtomicLong getMatchIndex() {
-        return matchIndex;
+    @Override
+    public long getMatchIndex() {
+        return matchIndex.get();
     }
 
     @Override

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 4993d25f202ecee0be55f7ec17b26528924a2e67..0a31cde90fb2c0139b72ec805aa5e4f83483bcb4 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
@@ -204,7 +204,7 @@ public abstract class AbstractLeader extends AbstractRaftActorBehavior {
             int replicatedCount = 1;
 
             for (FollowerLogInformation info : followerToLog.values()) {
-                if (info.getMatchIndex().get() >= N) {
+                if (info.getMatchIndex() >= N) {
                     replicatedCount++;
                 }
             }
@@ -228,6 +228,7 @@ public abstract class AbstractLeader extends AbstractRaftActorBehavior {
         return this;
     }
 
+    @Override
     protected ClientRequestTracker removeClientRequestTracker(long logIndex) {
 
         ClientRequestTracker toRemove = findClientRequestTracker(logIndex);
@@ -238,6 +239,7 @@ public abstract class AbstractLeader extends AbstractRaftActorBehavior {
         return toRemove;
     }
 
+    @Override
     protected ClientRequestTracker findClientRequestTracker(long logIndex) {
         for (ClientRequestTracker tracker : trackerList) {
             if (tracker.getIndex() == logIndex) {
@@ -330,8 +332,8 @@ public abstract class AbstractLeader extends AbstractRaftActorBehavior {
                     mapFollowerToSnapshot.remove(followerId);
 
                     if(LOG.isDebugEnabled()) {
-                        LOG.debug("followerToLog.get(followerId).getNextIndex().get()=" +
-                            followerToLog.get(followerId).getNextIndex().get());
+                        LOG.debug("followerToLog.get(followerId).getNextIndex()=" +
+                            followerToLog.get(followerId).getNextIndex());
                     }
 
                     if (mapFollowerToSnapshot.isEmpty()) {
@@ -398,7 +400,7 @@ public abstract class AbstractLeader extends AbstractRaftActorBehavior {
 
             if (followerActor != null) {
                 FollowerLogInformation followerLogInformation = followerToLog.get(followerId);
-                long followerNextIndex = followerLogInformation.getNextIndex().get();
+                long followerNextIndex = followerLogInformation.getNextIndex();
                 boolean isFollowerActive = followerLogInformation.isFollowerActive();
                 List<ReplicatedLogEntry> entries = null;
 
@@ -484,7 +486,7 @@ public abstract class AbstractLeader extends AbstractRaftActorBehavior {
             final ActorSelection followerActor = context.getPeerActorSelection(e.getKey());
 
             if (followerActor != null) {
-                long nextIndex = e.getValue().getNextIndex().get();
+                long nextIndex = e.getValue().getNextIndex();
 
                 if (!context.getReplicatedLog().isPresent(nextIndex) &&
                     context.getReplicatedLog().isInSnapshot(nextIndex)) {
@@ -535,7 +537,7 @@ public abstract class AbstractLeader extends AbstractRaftActorBehavior {
             ActorSelection followerActor = context.getPeerActorSelection(e.getKey());
 
             if (followerActor != null) {
-                long nextIndex = e.getValue().getNextIndex().get();
+                long nextIndex = e.getValue().getNextIndex();
 
                 if (!context.getReplicatedLog().isPresent(nextIndex) &&
                     context.getReplicatedLog().isInSnapshot(nextIndex)) {

diff --git a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/LeaderTest.java b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/LeaderTest.java
index 166fddd7b3ad51c22e34be5793cf8a591ba19090..151015e97ec785277beaf51b0bdb4fe0681f6582 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/LeaderTest.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/LeaderTest.java
@@ -72,6 +72,7 @@ public class LeaderTest extends AbstractRaftActorBehaviorTest {
         new JavaTestKit(getSystem()) {{
 
             new Within(duration("1 seconds")) {
+                @Override
                 protected void run() {
 
                     ActorRef followerActor = getTestActor();
@@ -91,6 +92,7 @@ public class LeaderTest extends AbstractRaftActorBehaviorTest {
                     final String out =
                         new ExpectMsg<String>(duration("1 seconds"), "match hint") {
                             // do not put code outside this method, will run afterwards
+                            @Override
                             protected String match(Object in) {
                                 Object msg = fromSerializableMessage(in);
                                 if (msg instanceof AppendEntries) {
@@ -116,6 +118,7 @@ public class LeaderTest extends AbstractRaftActorBehaviorTest {
         new JavaTestKit(getSystem()) {{
 
             new Within(duration("1 seconds")) {
+                @Override
                 protected void run() {
 
                     ActorRef followerActor = getTestActor();
@@ -144,6 +147,7 @@ public class LeaderTest extends AbstractRaftActorBehaviorTest {
                     final String out =
                         new ExpectMsg<String>(duration("1 seconds"), "match hint") {
                             // do not put code outside this method, will run afterwards
+                            @Override
                             protected String match(Object in) {
                                 Object msg = fromSerializableMessage(in);
                                 if (msg instanceof AppendEntries) {
@@ -168,6 +172,7 @@ public class LeaderTest extends AbstractRaftActorBehaviorTest {
         new JavaTestKit(getSystem()) {{
 
             new Within(duration("1 seconds")) {
+                @Override
                 protected void run() {
 
                     ActorRef raftActor = getTestActor();
@@ -194,6 +199,7 @@ public class LeaderTest extends AbstractRaftActorBehaviorTest {
                         new ExpectMsg<String>(duration("1 seconds"),
                             "match hint") {
                             // do not put code outside this method, will run afterwards
+                            @Override
                             protected String match(Object in) {
                                 if (in instanceof ApplyState) {
                                     if (((ApplyState) in).getIdentifier().equals("state-id")) {
@@ -481,6 +487,7 @@ public class LeaderTest extends AbstractRaftActorBehaviorTest {
             final String out =
                 new ExpectMsg<String>(duration("1 seconds"), "match hint") {
                     // do not put code outside this method, will run afterwards
+                    @Override
                     protected String match(Object in) {
                         if (in instanceof InstallSnapshotMessages.InstallSnapshot) {
                             InstallSnapshot is = (InstallSnapshot)
@@ -565,9 +572,9 @@ public class LeaderTest extends AbstractRaftActorBehaviorTest {
             assertEquals(1, leader.followerLogSize());
             assertNotNull(leader.getFollower(followerActor.path().toString()));
             FollowerLogInformation fli = leader.getFollower(followerActor.path().toString());
-            assertEquals(snapshotIndex, fli.getMatchIndex().get());
-            assertEquals(snapshotIndex, fli.getMatchIndex().get());
-            assertEquals(snapshotIndex + 1, fli.getNextIndex().get());
+            assertEquals(snapshotIndex, fli.getMatchIndex());
+            assertEquals(snapshotIndex, fli.getMatchIndex());
+            assertEquals(snapshotIndex + 1, fli.getNextIndex());
         }};
     }
 
@@ -778,6 +785,7 @@ public class LeaderTest extends AbstractRaftActorBehaviorTest {
         return createActorContext(leaderActor);
     }
 
+    @Override
     protected RaftActorContext createActorContext(ActorRef actorRef) {
         return new MockRaftActorContext("test", getSystem(), actorRef);
     }