Hide AbstractLeader maps

Collections should never be leaked, as they introduce the ability to
make unsynchronized changes. This patch hides the map itself, allowing
lookups.

Once we have done that, we have control over the interactions and thus
can see that the map is instantiation-constant. Use an ImmutableMap to
maintain the entries as it provides superior compactness and lookup
speeds.

Change-Id: I552372dbabe454111909bb8251637e9f3c0e0db7
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/behaviors/AbstractLeader.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractLeader.java
index 5606453e31be9a9303be5a28652c6e3b0b12caf9..bb6002809c32b802afd0fec99ec9f1ce0fba4d94 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
@@ -14,6 +14,8 @@ import akka.actor.Cancellable;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;

+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableMap.Builder;

 import com.google.protobuf.ByteString;
 import java.io.IOException;
 import java.util.ArrayList;
 import com.google.protobuf.ByteString;
 import java.io.IOException;
 import java.util.ArrayList;


@@ -76,8 +78,8 @@ public abstract class AbstractLeader extends AbstractRaftActorBehavior {
     // This would be passed as the hash code of the last chunk when sending the first chunk
     public static final int INITIAL_LAST_CHUNK_HASH_CODE = -1;
 
     // This would be passed as the hash code of the last chunk when sending the first chunk
     public static final int INITIAL_LAST_CHUNK_HASH_CODE = -1;
 

-    protected final Map<String, FollowerLogInformation> followerToLog = new HashMap<>();
-    protected final Map<String, FollowerToSnapshot> mapFollowerToSnapshot = new HashMap<>();
+    private final Map<String, FollowerLogInformation> followerToLog;
+    private final Map<String, FollowerToSnapshot> mapFollowerToSnapshot = new HashMap<>();

 
     protected final Set<String> followers;
 
 
     protected final Set<String> followers;
 


@@ -96,14 +98,16 @@ public abstract class AbstractLeader extends AbstractRaftActorBehavior {
 
         followers = context.getPeerAddresses().keySet();
 
 
         followers = context.getPeerAddresses().keySet();
 

+        final Builder<String, FollowerLogInformation> ftlBuilder = ImmutableMap.builder();

         for (String followerId : followers) {
             FollowerLogInformation followerLogInformation =
                 new FollowerLogInformationImpl(followerId,
                     context.getCommitIndex(), -1,
                     context.getConfigParams().getElectionTimeOutInterval());
 
         for (String followerId : followers) {
             FollowerLogInformation followerLogInformation =
                 new FollowerLogInformationImpl(followerId,
                     context.getCommitIndex(), -1,
                     context.getConfigParams().getElectionTimeOutInterval());
 

-            followerToLog.put(followerId, followerLogInformation);
+            ftlBuilder.put(followerId, followerLogInformation);

         }
         }

+        followerToLog = ftlBuilder.build();

 
         leaderId = context.getId();
 
 
         leaderId = context.getId();
 


@@ -773,7 +777,22 @@ public abstract class AbstractLeader extends AbstractRaftActorBehavior {
     }
 
     @VisibleForTesting
     }
 
     @VisibleForTesting

-    void markFollowerActive(String followerId) {
-        followerToLog.get(followerId).markFollowerActive();
+    public FollowerLogInformation getFollower(String followerId) {
+        return followerToLog.get(followerId);
+    }
+
+    @VisibleForTesting
+    protected void setFollowerSnapshot(String followerId, FollowerToSnapshot snapshot) {
+        mapFollowerToSnapshot.put(followerId, snapshot);
+    }
+
+    @VisibleForTesting
+    public int followerSnapshotSize() {
+        return mapFollowerToSnapshot.size();
+    }
+
+    @VisibleForTesting
+    public int followerLogSize() {
+        return followerToLog.size();

     }
 }
     }
 }

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 97ecef370f08f00c7a2041b1897498a10d42fd60..49bcfac61e4ba2d46dc968e3ca5c7310fbb1a639 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
@@ -5,7 +5,6 @@
  * terms of the Eclipse Public License v1.0 which accompanies this distribution,
  * and is available at http://www.eclipse.org/legal/epl-v10.html
  */
  * 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.behaviors;
 
 import akka.actor.ActorRef;
 package org.opendaylight.controller.cluster.raft.behaviors;
 
 import akka.actor.ActorRef;


@@ -103,7 +102,8 @@ public class Leader extends AbstractLeader {
             context.getActorSystem().dispatcher(), context.getActor());
     }
 
             context.getActorSystem().dispatcher(), context.getActor());
     }
 

-    @Override public void close() throws Exception {
+    @Override
+    public void close() throws Exception {

         stopInstallSnapshotSchedule();
         stopIsolatedLeaderCheckSchedule();
         super.close();
         stopInstallSnapshotSchedule();
         stopIsolatedLeaderCheckSchedule();
         super.close();


@@ -111,11 +111,11 @@ public class Leader extends AbstractLeader {
 
     @VisibleForTesting
     void markFollowerActive(String followerId) {
 
     @VisibleForTesting
     void markFollowerActive(String followerId) {

-        followerToLog.get(followerId).markFollowerActive();
+        getFollower(followerId).markFollowerActive();

     }
 
     @VisibleForTesting
     void markFollowerInActive(String followerId) {
     }
 
     @VisibleForTesting
     void markFollowerInActive(String followerId) {

-        followerToLog.get(followerId).markFollowerInActive();
+        getFollower(followerId).markFollowerInActive();

     }
 }
     }
 }

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 895fe35bff7588526fac71e996e9120968234af2..166fddd7b3ad51c22e34be5793cf8a591ba19090 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
@@ -67,7 +67,6 @@ public class LeaderTest extends AbstractRaftActorBehaviorTest {
         }};
     }
 
         }};
     }
 

-

     @Test
     public void testThatLeaderSendsAHeartbeatMessageToAllFollowers() {
         new JavaTestKit(getSystem()) {{
     @Test
     public void testThatLeaderSendsAHeartbeatMessageToAllFollowers() {
         new JavaTestKit(getSystem()) {{


@@ -562,10 +561,10 @@ public class LeaderTest extends AbstractRaftActorBehaviorTest {
 
             assertTrue(raftBehavior instanceof Leader);
 
 
             assertTrue(raftBehavior instanceof Leader);
 

-            assertEquals(leader.mapFollowerToSnapshot.size(), 0);
-            assertEquals(leader.followerToLog.size(), 1);
-            assertNotNull(leader.followerToLog.get(followerActor.path().toString()));
-            FollowerLogInformation fli = leader.followerToLog.get(followerActor.path().toString());
+            assertEquals(0, leader.followerSnapshotSize());
+            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().get());
             assertEquals(snapshotIndex, fli.getMatchIndex().get());
             assertEquals(snapshotIndex + 1, fli.getNextIndex().get());


@@ -1180,8 +1179,7 @@ public class LeaderTest extends AbstractRaftActorBehaviorTest {
 
         public void createFollowerToSnapshot(String followerId, ByteString bs ) {
             fts = new FollowerToSnapshot(bs);
 
         public void createFollowerToSnapshot(String followerId, ByteString bs ) {
             fts = new FollowerToSnapshot(bs);

-            mapFollowerToSnapshot.put(followerId, fts);
-
+            setFollowerSnapshot(followerId, fts);

         }
     }
 
         }
     }