Set to non-voting if not in server confguration

On recovery, if a RaftActor is not in its own recovered
ServerConfigurationPayload list, then set itself to a non-voting member
so it stays at Follower and doesn't try to start an election.

This scenario is an edge case for Shards as, normally, when a server is
removed, it self-destructs and is removed from the ShardManager. However
there is a small window where disconnect or shutdown could prevent
ShardManager removal from occurring. This patch protects against a server
restart causing disruption after removal.

Change-Id: I64ecd89cddec7a4e1711e0d8d17c7ea6b36e29a0
Signed-off-by: Tom Pantelis <tpanteli@brocade.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 4677ea946e65cf352779d027744e1c7e232bc572..0bf71ab7991b3e87bb4c6afd02577f84d749a730 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
@@ -192,9 +192,11 @@ public class RaftActorContextImpl implements RaftActorContext {
     @Override
     public void updatePeerIds(ServerConfigurationPayload serverConfig){
         votingMember = true;
     @Override
     public void updatePeerIds(ServerConfigurationPayload serverConfig){
         votingMember = true;

+        boolean foundSelf = false;

         Set<String> currentPeers = new HashSet<>(this.getPeerIds());
         for(ServerInfo server: serverConfig.getServerConfig()) {
             if(getId().equals(server.getId())) {
         Set<String> currentPeers = new HashSet<>(this.getPeerIds());
         for(ServerInfo server: serverConfig.getServerConfig()) {
             if(getId().equals(server.getId())) {

+                foundSelf = true;

                 if(!server.isVoting()) {
                     votingMember = false;
                 }
                 if(!server.isVoting()) {
                     votingMember = false;
                 }


@@ -212,6 +214,11 @@ public class RaftActorContextImpl implements RaftActorContext {
         for(String peerIdToRemove: currentPeers) {
             this.removePeer(peerIdToRemove);
         }
         for(String peerIdToRemove: currentPeers) {
             this.removePeer(peerIdToRemove);
         }

+
+        if(!foundSelf) {
+            votingMember = false;
+        }
+

         setDynamicServerConfigurationInUse();
     }
 
         setDynamicServerConfigurationInUse();
     }
 

diff --git a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/RaftActorContextImplTest.java b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/RaftActorContextImplTest.java
index f2903983e930490a37478fda36e2f64b2fa70a42..a879fd7ea2c961c7ed0564c281b4dfa3efe36a80 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/RaftActorContextImplTest.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/RaftActorContextImplTest.java
@@ -8,6 +8,8 @@
 package org.opendaylight.controller.cluster.raft;
 
 import static org.junit.Assert.assertEquals;
 package org.opendaylight.controller.cluster.raft;
 
 import static org.junit.Assert.assertEquals;

+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;

 import static org.mockito.Matchers.anyString;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;


@@ -18,11 +20,13 @@ import akka.actor.Props;
 import akka.testkit.TestActorRef;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;
 import akka.testkit.TestActorRef;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;

+import java.util.Arrays;

 import java.util.HashMap;
 import java.util.Map;
 import org.junit.After;
 import org.junit.Test;
 import org.opendaylight.controller.cluster.NonPersistentDataProvider;
 import java.util.HashMap;
 import java.util.Map;
 import org.junit.After;
 import org.junit.Test;
 import org.opendaylight.controller.cluster.NonPersistentDataProvider;

+import org.opendaylight.controller.cluster.raft.ServerConfigurationPayload.ServerInfo;

 import org.opendaylight.controller.cluster.raft.utils.DoNothingActor;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.opendaylight.controller.cluster.raft.utils.DoNothingActor;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;


@@ -86,4 +90,42 @@ public class RaftActorContextImplTest extends AbstractActorTest {
         context.setPeerAddress("peer2", "peerAddress2");
         assertEquals("getPeerAddress", null, context.getPeerAddress("peer2"));
     }
         context.setPeerAddress("peer2", "peerAddress2");
         assertEquals("getPeerAddress", null, context.getPeerAddress("peer2"));
     }

+
+    @Test
+    public void testUpdatePeerIds() {
+        RaftActorContextImpl context = new RaftActorContextImpl(actor, actor.underlyingActor().getContext(),
+                "self", new ElectionTermImpl(new NonPersistentDataProvider(), "test", log), -1, -1,
+                Maps.newHashMap(ImmutableMap.<String, String>of("peer1", "peerAddress1")),
+                new DefaultConfigParamsImpl(), new NonPersistentDataProvider(), log);
+
+        context.updatePeerIds(new ServerConfigurationPayload(Arrays.asList(new ServerInfo("self", false),
+                new ServerInfo("peer2", true), new ServerInfo("peer3", false))));
+        verifyPeerInfo(context, "peer1", null);
+        verifyPeerInfo(context, "peer2", true);
+        verifyPeerInfo(context, "peer3", false);
+        assertEquals("isVotingMember", false, context.isVotingMember());
+
+        context.updatePeerIds(new ServerConfigurationPayload(Arrays.asList(new ServerInfo("self", true),
+                new ServerInfo("peer2", true), new ServerInfo("peer3", true))));
+        verifyPeerInfo(context, "peer2", true);
+        verifyPeerInfo(context, "peer3", true);
+        assertEquals("isVotingMember", true, context.isVotingMember());
+
+        context.updatePeerIds(new ServerConfigurationPayload(Arrays.asList(new ServerInfo("peer2", true),
+                new ServerInfo("peer3", true))));
+        verifyPeerInfo(context, "peer2", true);
+        verifyPeerInfo(context, "peer3", true);
+        assertEquals("isVotingMember", false, context.isVotingMember());
+    }
+
+    private void verifyPeerInfo(RaftActorContextImpl context, String peerId, Boolean voting) {
+        PeerInfo peerInfo = context.getPeerInfo(peerId);
+        if(voting != null) {
+            assertNotNull("Expected peer " + peerId, peerInfo);
+            assertEquals("getVotingState for " + peerId, voting.booleanValue() ? VotingState.VOTING : VotingState.NON_VOTING,
+                    peerInfo.getVotingState());
+        } else {
+            assertNull("Unexpected peer " + peerId, peerInfo);
+        }
+    }

 }
 }