From afeef85277501bd9304f6cd7eda90a821c771c5d Mon Sep 17 00:00:00 2001 From: Hoverbear Date: Fri, 29 Jun 2018 20:45:41 -0700 Subject: [PATCH 1/4] raft: become_pre_candidate resets votes --- src/raft.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/raft.rs b/src/raft.rs index f46cb7c29..8c5aa357b 100644 --- a/src/raft.rs +++ b/src/raft.rs @@ -828,6 +828,7 @@ impl Raft { // but doesn't change anything else. In particular it does not increase // self.term or change self.vote. self.state = StateRole::PreCandidate; + self.votes = FxHashMap::default(); info!("{} became pre-candidate at term {}", self.tag, self.term); } From adeec629209f6221da8ccad755eefd99c35bcb7c Mon Sep 17 00:00:00 2001 From: Hoverbear Date: Mon, 2 Jul 2018 18:13:01 -0700 Subject: [PATCH 2/4] fmt --- src/lib.rs | 11 +++++++---- src/raft.rs | 3 ++- src/raw_node.rs | 6 ++++-- tests/cases/test_raft.rs | 6 ++++-- 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 64fa30909..f97ecf850 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -227,8 +227,9 @@ pub mod util; pub use self::errors::{Error, Result, StorageError}; pub use self::log_unstable::Unstable; pub use self::progress::{Inflights, Progress, ProgressSet, ProgressState}; -pub use self::raft::{quorum, vote_resp_msg_type, Config, Raft, SoftState, StateRole, INVALID_ID, - INVALID_INDEX}; +pub use self::raft::{ + quorum, vote_resp_msg_type, Config, Raft, SoftState, StateRole, INVALID_ID, INVALID_INDEX, +}; pub use self::raft_log::{RaftLog, NO_LIMIT}; pub use self::raw_node::{is_empty_snap, Peer, RawNode, Ready, SnapshotStatus}; pub use self::read_only::{ReadOnlyOption, ReadState}; @@ -248,8 +249,10 @@ pub mod prelude { //! //! The prelude may grow over time as additional items see ubiquitous use. - pub use eraftpb::{ConfChange, ConfChangeType, ConfState, Entry, EntryType, HardState, Message, - MessageType, Snapshot, SnapshotMetadata}; + pub use eraftpb::{ + ConfChange, ConfChangeType, ConfState, Entry, EntryType, HardState, Message, MessageType, + Snapshot, SnapshotMetadata, + }; pub use raft::{Config, Raft}; diff --git a/src/raft.rs b/src/raft.rs index 8c5aa357b..309224d11 100644 --- a/src/raft.rs +++ b/src/raft.rs @@ -938,7 +938,8 @@ impl Raft { || m.get_msg_type() == MessageType::MsgRequestPreVote { let force = m.get_context() == CAMPAIGN_TRANSFER; - let in_lease = self.check_quorum && self.leader_id != INVALID_ID + let in_lease = self.check_quorum + && self.leader_id != INVALID_ID && self.election_elapsed < self.election_timeout; if !force && in_lease { // if a server receives RequestVote request within the minimum election diff --git a/src/raw_node.rs b/src/raw_node.rs index 920b696f0..400d8d2ea 100644 --- a/src/raw_node.rs +++ b/src/raw_node.rs @@ -27,8 +27,10 @@ use std::mem; -use eraftpb::{ConfChange, ConfChangeType, ConfState, Entry, EntryType, HardState, Message, - MessageType, Snapshot}; +use eraftpb::{ + ConfChange, ConfChangeType, ConfState, Entry, EntryType, HardState, Message, MessageType, + Snapshot, +}; use protobuf::{self, RepeatedField}; use super::errors::{Error, Result}; diff --git a/tests/cases/test_raft.rs b/tests/cases/test_raft.rs index 9996d6c55..0994eac98 100644 --- a/tests/cases/test_raft.rs +++ b/tests/cases/test_raft.rs @@ -32,8 +32,10 @@ use std::ops::DerefMut; use std::panic::{self, AssertUnwindSafe}; use protobuf::{self, RepeatedField}; -use raft::eraftpb::{ConfChange, ConfChangeType, ConfState, Entry, EntryType, HardState, Message, - MessageType, Snapshot}; +use raft::eraftpb::{ + ConfChange, ConfChangeType, ConfState, Entry, EntryType, HardState, Message, MessageType, + Snapshot, +}; use rand; use raft::storage::MemStorage; From 0a8d736c864a888ed3193e4ada41138935b4a06f Mon Sep 17 00:00:00 2001 From: Hoverbear Date: Mon, 2 Jul 2018 18:13:13 -0700 Subject: [PATCH 3/4] tests: Add prevote split vote test. From https://github.com/coreos/etcd/pull/8346/files --- tests/cases/test_raft.rs | 83 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/tests/cases/test_raft.rs b/tests/cases/test_raft.rs index 0994eac98..39095d98e 100644 --- a/tests/cases/test_raft.rs +++ b/tests/cases/test_raft.rs @@ -4134,3 +4134,86 @@ fn test_election_tick_range() { assert_eq!(randomized_timeout, cfg.election_tick); } } + +// TestPreVoteWithSplitVote verifies that after split vote, cluster can complete +// election in next round. +#[test] +fn test_prevote_with_split_vote() { + let bootstrap = |id| { + let mut cfg = new_test_config(id, vec![1, 2, 3], 10, 1); + cfg.pre_vote = true; + let mut raft = Raft::new(&cfg, new_storage()); + raft.become_follower(1, INVALID_ID); + Interface::new(raft) + }; + let (peer1, peer2, peer3) = (bootstrap(1), bootstrap(2), bootstrap(3)); + + let mut network = Network::new(vec![Some(peer1), Some(peer2), Some(peer3)]); + network.send(vec![new_message(1, 1, MessageType::MsgHup, 0)]); + + // simulate leader down. followers start split vote. + network.isolate(1); + network.send(vec![ + new_message(2, 2, MessageType::MsgHup, 0), + new_message(3, 3, MessageType::MsgHup, 0), + ]); + + // check whether the term values are expected + assert_eq!( + network.peers[&2].term, 3, + "peer 2 term: {:?}, want {:?}", + network.peers[&2].term, 3, + ); + assert_eq!( + network.peers[&3].term, 3, + "peer 3 term: {:?}, want {:?}", + network.peers[&3].term, 3, + ); + + // check state + assert_eq!( + network.peers[&2].state, + StateRole::Candidate, + "peer 2 state: {:?}, want {:?}", + network.peers[&2].state, + StateRole::Candidate, + ); + assert_eq!( + network.peers[&3].state, + StateRole::Candidate, + "peer 3 state: {:?}, want {:?}", + network.peers[&3].state, + StateRole::Candidate, + ); + + // node 2 election timeout first + network.send(vec![new_message(2, 2, MessageType::MsgHup, 0)]); + + // check whether the term values are expected + assert_eq!( + network.peers[&2].term, 4, + "peer 2 term: {:?}, want {:?}", + network.peers[&2].term, 4, + ); + assert_eq!( + network.peers[&3].term, 4, + "peer 3 term: {:?}, want {:?}", + network.peers[&3].term, 4, + ); + + // check state + assert_eq!( + network.peers[&2].state, + StateRole::Leader, + "peer 2 state: {:?}, want {:?}", + network.peers[&2].state, + StateRole::Leader, + ); + assert_eq!( + network.peers[&3].state, + StateRole::Follower, + "peer 3 state: {:?}, want {:?}", + network.peers[&3].state, + StateRole::Follower, + ); +} From 9e182857b7e89c946780e34b8be9bba49f4f86f9 Mon Sep 17 00:00:00 2001 From: Hoverbear Date: Tue, 3 Jul 2018 09:55:36 -0700 Subject: [PATCH 4/4] tests: Reflect comments. --- tests/cases/test_raft.rs | 62 +++++++++------------------------------- 1 file changed, 13 insertions(+), 49 deletions(-) diff --git a/tests/cases/test_raft.rs b/tests/cases/test_raft.rs index 39095d98e..7d2aa327e 100644 --- a/tests/cases/test_raft.rs +++ b/tests/cases/test_raft.rs @@ -4139,16 +4139,12 @@ fn test_election_tick_range() { // election in next round. #[test] fn test_prevote_with_split_vote() { - let bootstrap = |id| { - let mut cfg = new_test_config(id, vec![1, 2, 3], 10, 1); - cfg.pre_vote = true; - let mut raft = Raft::new(&cfg, new_storage()); + let peers = (1..=3).map(|id| { + let mut raft = new_test_raft_with_prevote(id, vec![1, 2, 3], 10, 1, new_storage(), true); raft.become_follower(1, INVALID_ID); - Interface::new(raft) - }; - let (peer1, peer2, peer3) = (bootstrap(1), bootstrap(2), bootstrap(3)); - - let mut network = Network::new(vec![Some(peer1), Some(peer2), Some(peer3)]); + Some(raft) + }); + let mut network = Network::new(peers.collect()); network.send(vec![new_message(1, 1, MessageType::MsgHup, 0)]); // simulate leader down. followers start split vote. @@ -4159,61 +4155,29 @@ fn test_prevote_with_split_vote() { ]); // check whether the term values are expected - assert_eq!( - network.peers[&2].term, 3, - "peer 2 term: {:?}, want {:?}", - network.peers[&2].term, 3, - ); - assert_eq!( - network.peers[&3].term, 3, - "peer 3 term: {:?}, want {:?}", - network.peers[&3].term, 3, - ); + assert_eq!(network.peers[&2].term, 3, "peer 2 term",); + assert_eq!(network.peers[&3].term, 3, "peer 3 term",); // check state assert_eq!( network.peers[&2].state, StateRole::Candidate, - "peer 2 state: {:?}, want {:?}", - network.peers[&2].state, - StateRole::Candidate, + "peer 2 state", ); assert_eq!( network.peers[&3].state, StateRole::Candidate, - "peer 3 state: {:?}, want {:?}", - network.peers[&3].state, - StateRole::Candidate, + "peer 3 state", ); // node 2 election timeout first network.send(vec![new_message(2, 2, MessageType::MsgHup, 0)]); // check whether the term values are expected - assert_eq!( - network.peers[&2].term, 4, - "peer 2 term: {:?}, want {:?}", - network.peers[&2].term, 4, - ); - assert_eq!( - network.peers[&3].term, 4, - "peer 3 term: {:?}, want {:?}", - network.peers[&3].term, 4, - ); + assert_eq!(network.peers[&2].term, 4, "peer 2 term",); + assert_eq!(network.peers[&3].term, 4, "peer 3 term",); // check state - assert_eq!( - network.peers[&2].state, - StateRole::Leader, - "peer 2 state: {:?}, want {:?}", - network.peers[&2].state, - StateRole::Leader, - ); - assert_eq!( - network.peers[&3].state, - StateRole::Follower, - "peer 3 state: {:?}, want {:?}", - network.peers[&3].state, - StateRole::Follower, - ); + assert_eq!(network.peers[&2].state, StateRole::Leader, "peer 2 state",); + assert_eq!(network.peers[&3].state, StateRole::Follower, "peer 3 state",); }