From f9c49de804eb7741df63d0a15889d485d65660e7 Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Mon, 7 Dec 2015 20:14:11 -0500 Subject: [PATCH] Bug 2187: Prevent non-voting member from initiating elections Change-Id: Id6359e6af5ff7389e1707e683310d103b167b402 Signed-off-by: Tom Pantelis --- .../cluster/raft/RaftActorContext.java | 5 ++++ .../cluster/raft/RaftActorContextImpl.java | 15 +++++++++-- .../behaviors/AbstractRaftActorBehavior.java | 15 ++++++----- .../cluster/raft/behaviors/Follower.java | 10 ++++--- .../cluster/raft/RaftActorTest.java | 27 +++++++++++++++++++ 5 files changed, 61 insertions(+), 11 deletions(-) diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActorContext.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActorContext.java index b20e9daa94..10db6d06c2 100644 --- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActorContext.java +++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActorContext.java @@ -236,4 +236,9 @@ public interface RaftActorContext { * dynamic server configurations are available, otherwise returns null */ @Nullable ServerConfigurationPayload getPeerServerInfo(); + + /** + * @return true if this RaftActor is a voting member of the cluster, false otherwise. + */ + boolean isVotingMember(); } 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 a96d502663..d9cfbcdd11 100644 --- 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 @@ -62,6 +62,8 @@ public class RaftActorContextImpl implements RaftActorContext { private short payloadVersion; + private boolean votingMember = true; + public RaftActorContextImpl(ActorRef actor, ActorContext context, String id, ElectionTerm termInformation, long commitIndex, long lastApplied, Map peerAddresses, ConfigParams configParams, DataPersistenceProvider persistenceProvider, Logger logger) { @@ -189,10 +191,14 @@ public class RaftActorContextImpl implements RaftActorContext { @Override public void updatePeerIds(ServerConfigurationPayload serverConfig){ - + votingMember = true; Set currentPeers = new HashSet<>(this.getPeerIds()); for(ServerInfo server: serverConfig.getServerConfig()) { - if(!getId().equals(server.getId())) { + if(getId().equals(server.getId())) { + if(!server.isVoting()) { + votingMember = false; + } + } else { VotingState votingState = server.isVoting() ? VotingState.VOTING: VotingState.NON_VOTING; if(!currentPeers.contains(server.getId())) { this.addToPeers(server.getId(), null, votingState); @@ -297,4 +303,9 @@ public class RaftActorContextImpl implements RaftActorContext { newConfig.add(new ServerInfo(getId(), true)); return (new ServerConfigurationPayload(newConfig)); } + + @Override + public boolean isVotingMember() { + return votingMember; + } } 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 48b68bdfd0..d17fba03d4 100644 --- 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 @@ -252,6 +252,10 @@ public abstract class AbstractRaftActorBehavior implements RaftActorBehavior { } } + protected boolean canStartElection() { + return context.getRaftPolicy().automaticElectionsEnabled() && context.isVotingMember(); + } + /** * schedule a new election * @@ -260,12 +264,11 @@ public abstract class AbstractRaftActorBehavior implements RaftActorBehavior { protected void scheduleElection(FiniteDuration interval) { stopElection(); - // Schedule an election. When the scheduler triggers an ElectionTimeout - // message is sent to itself - electionCancel = - context.getActorSystem().scheduler().scheduleOnce(interval, - context.getActor(), ELECTION_TIMEOUT, - context.getActorSystem().dispatcher(), context.getActor()); + if(canStartElection()) { + // Schedule an election. When the scheduler triggers an ElectionTimeout message is sent to itself + electionCancel = context.getActorSystem().scheduler().scheduleOnce(interval, context.getActor(), + ELECTION_TIMEOUT,context.getActorSystem().dispatcher(), context.getActor()); + } } /** 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 0149b57e6e..5952dc087f 100644 --- 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 @@ -54,7 +54,7 @@ public class Follower extends AbstractRaftActorBehavior { initialSyncStatusTracker = new SyncStatusTracker(context.getActor(), getId(), SYNC_THRESHOLD); - if(context.getRaftPolicy().automaticElectionsEnabled()) { + if(canStartElection()) { if (context.getPeerIds().isEmpty() && getLeaderId() == null) { actor().tell(ELECTION_TIMEOUT, actor()); } else { @@ -330,8 +330,12 @@ public class Follower extends AbstractRaftActorBehavior { } if (message instanceof ElectionTimeout) { - LOG.debug("{}: Received ElectionTimeout - switching to Candidate", logName()); - return internalSwitchBehavior(RaftState.Candidate); + if(canStartElection()) { + LOG.debug("{}: Received ElectionTimeout - switching to Candidate", logName()); + return internalSwitchBehavior(RaftState.Candidate); + } else { + return this; + } } else if (message instanceof InstallSnapshot) { InstallSnapshot installSnapshot = (InstallSnapshot) message; 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 c2ee4a26d1..e19135cbcb 100644 --- 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 @@ -63,6 +63,7 @@ import org.opendaylight.controller.cluster.PersistentDataProvider; import org.opendaylight.controller.cluster.notifications.LeaderStateChanged; import org.opendaylight.controller.cluster.notifications.RoleChanged; import org.opendaylight.controller.cluster.raft.MockRaftActorContext.MockPayload; +import org.opendaylight.controller.cluster.raft.ServerConfigurationPayload.ServerInfo; import org.opendaylight.controller.cluster.raft.base.messages.ApplyJournalEntries; import org.opendaylight.controller.cluster.raft.base.messages.ApplyLogEntries; import org.opendaylight.controller.cluster.raft.base.messages.ApplySnapshot; @@ -1305,4 +1306,30 @@ public class RaftActorTest extends AbstractActorTest { TEST_LOG.info("testRestoreFromSnapshotWithRecoveredData ending"); } + + @Test + public void testNonVotingOnRecovery() throws Exception { + TEST_LOG.info("testNonVotingOnRecovery starting"); + + DefaultConfigParamsImpl config = new DefaultConfigParamsImpl(); + config.setElectionTimeoutFactor(1); + config.setHeartBeatInterval(FiniteDuration.create(1, TimeUnit.MILLISECONDS)); + + String persistenceId = factory.generateActorId("test-actor-"); + InMemoryJournal.addEntry(persistenceId, 1, new MockRaftActorContext.MockReplicatedLogEntry(1, 0, + new ServerConfigurationPayload(Arrays.asList(new ServerInfo(persistenceId, false))))); + + TestActorRef raftActorRef = factory.createTestActor(MockRaftActor.builder().id(persistenceId). + config(config).props().withDispatcher(Dispatchers.DefaultDispatcherId()), persistenceId); + MockRaftActor mockRaftActor = raftActorRef.underlyingActor(); + + mockRaftActor.waitForInitializeBehaviorComplete(); + + // Sleep a bit and verify it didn't get an election timeout and schedule an election. + + Uninterruptibles.sleepUninterruptibly(400, TimeUnit.MILLISECONDS); + assertEquals("getRaftState", RaftState.Follower, mockRaftActor.getRaftState()); + + TEST_LOG.info("testNonVotingOnRecovery ending"); + } } -- 2.36.6