From e55a8bf42b17b3de042916e6b9eaaad8d271d241 Mon Sep 17 00:00:00 2001 From: Dhia Ayachi Date: Wed, 6 Oct 2021 20:31:14 -0400 Subject: [PATCH] Forbid removed node from being a `Candidate` (#476) * modify `TestRaft_RemoveFollower` to check removed node state * add test to verify that a removed node is not able to vote * do not transition to `Candidate` state if not part of stable configuration * add some comments to the tests * do not return if not transitioning state * add wait loop to test * remove test related to removed node voting * check `inConfig` instead of `hasVote` * updating warn log message to be more accurate --- configuration.go | 11 +++++++++++ raft.go | 13 ++++++++++--- raft_test.go | 3 +++ 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/configuration.go b/configuration.go index 5c6636058..1ac92fdc0 100644 --- a/configuration.go +++ b/configuration.go @@ -173,6 +173,17 @@ func hasVote(configuration Configuration, id ServerID) bool { return false } +// hasVote returns true if the server identified by 'id' is a Voter in the +// provided Configuration. +func inConfig(configuration Configuration, id ServerID) bool { + for _, server := range configuration.Servers { + if server.ID == id { + return true + } + } + return false +} + // checkConfiguration tests a cluster membership configuration for common // errors. func checkConfiguration(configuration Configuration) error { diff --git a/raft.go b/raft.go index 99f357585..a53492bd4 100644 --- a/raft.go +++ b/raft.go @@ -213,10 +213,17 @@ func (r *Raft) runFollower() { didWarn = true } } else { - r.logger.Warn("heartbeat timeout reached, starting election", "last-leader", lastLeader) metrics.IncrCounter([]string{"raft", "transition", "heartbeat_timeout"}, 1) - r.setState(Candidate) - return + if inConfig(r.configurations.latest, r.localID) { + r.logger.Warn("heartbeat timeout reached, starting election", "last-leader", lastLeader) + r.setState(Candidate) + return + } else { + if !didWarn { + r.logger.Warn("heartbeat timeout reached, not part of stable configuration, not triggering a leader election") + didWarn = true + } + } } case <-r.shutdownCh: diff --git a/raft_test.go b/raft_test.go index 51db44a82..bf4bfcbd4 100644 --- a/raft_test.go +++ b/raft_test.go @@ -697,6 +697,9 @@ func TestRaft_RemoveFollower(t *testing.T) { if configuration := c.getConfiguration(followers[1]); len(configuration.Servers) != 2 { t.Fatalf("too many peers") } + + // The removed node should remain in a follower state + require.Equal(t, Follower, follower.getState()) } func TestRaft_RemoveLeader(t *testing.T) {