From 4636cf6ddeb51c3143843c7e4c7608be2e8e2fe4 Mon Sep 17 00:00:00 2001 From: qupeng Date: Wed, 9 May 2018 16:04:31 +0800 Subject: [PATCH] Learner needs to respond vote requests. --- src/raft.rs | 28 ++++----------- tests/cases/test_raft.rs | 76 +++++++++++++++++++--------------------- 2 files changed, 43 insertions(+), 61 deletions(-) diff --git a/src/raft.rs b/src/raft.rs index e5ea4a865..c5e275cd0 100644 --- a/src/raft.rs +++ b/src/raft.rs @@ -27,16 +27,16 @@ use std::cmp; -use rand::{self, Rng}; use eraftpb::{Entry, EntryType, HardState, Message, MessageType, Snapshot}; use fxhash::FxHashMap; use protobuf::repeated::RepeatedField; +use rand::{self, Rng}; -use super::storage::Storage; -use super::progress::{Inflights, Progress, ProgressSet, ProgressState}; use super::errors::{Error, Result, StorageError}; +use super::progress::{Inflights, Progress, ProgressSet, ProgressState}; use super::raft_log::{self, RaftLog}; use super::read_only::{ReadOnly, ReadOnlyOption, ReadState}; +use super::storage::Storage; // CAMPAIGN_PRE_ELECTION represents the first phase of a normal election when // Config.pre_vote is true. @@ -828,6 +828,7 @@ impl Raft { return; } + // Only send vote request to voters. let prs = self.take_prs(); prs.voters() .keys() @@ -1016,24 +1017,6 @@ impl Raft { debug!("{} ignoring MsgHup because already leader", self.tag); }, MessageType::MsgRequestVote | MessageType::MsgRequestPreVote => { - if self.is_learner { - // TODO: learner may need to vote, in case of node down when confchange. - info!( - "{} [logterm: {}, index: {}, vote: {}] ignored {:?} from {} \ - [logterm: {}, index: {}] at term {}: learner can not vote", - self.tag, - self.raft_log.last_term(), - self.raft_log.last_index(), - self.vote, - m.get_msg_type(), - m.get_from(), - m.get_log_term(), - m.get_index(), - self.term, - ); - return Ok(()); - } - // We can vote if this is a repeat of a vote we've already cast... let can_vote = (self.vote == m.get_from()) || // ...we haven't voted and we don't think there's a leader yet in this term... @@ -1083,9 +1066,10 @@ impl Raft { fn log_vote_approve(&self, m: &Message) { info!( - "{} [logterm: {}, index: {}, vote: {}] cast {:?} for {} [logterm: {}, index: {}] \ + "{}({}) [logterm: {}, index: {}, vote: {}] cast {:?} for {} [logterm: {}, index: {}] \ at term {}", self.tag, + if self.is_learner { "learner" } else { "voter" }, self.raft_log.last_term(), self.raft_log.last_index(), self.vote, diff --git a/tests/cases/test_raft.rs b/tests/cases/test_raft.rs index 0cf4b74a8..9f12edc70 100644 --- a/tests/cases/test_raft.rs +++ b/tests/cases/test_raft.rs @@ -25,10 +25,10 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::cmp; use std::collections::HashMap; use std::ops::Deref; use std::ops::DerefMut; -use std::cmp; use std::panic::{self, AssertUnwindSafe}; use protobuf::{self, RepeatedField}; @@ -36,8 +36,8 @@ use raft::eraftpb::{ConfChange, ConfChangeType, ConfState, Entry, EntryType, Har MessageType, Snapshot}; use rand; -use raft::*; use raft::storage::MemStorage; +use raft::*; pub fn ltoa(raft_log: &RaftLog) -> String { let mut s = format!("committed: {}\n", raft_log.committed); @@ -897,34 +897,22 @@ fn test_vote_from_any_state_for_type(vt: MessageType) { StateRole::Follower ); assert_eq!( - r.term, - new_term, + r.term, new_term, "{:?},{:?}, term {}, want {}", - vt, - state, - r.term, - new_term + vt, state, r.term, new_term ); assert_eq!(r.vote, 2, "{:?},{:?}, vote {}, want 2", vt, state, r.vote); } else { // In a pre-vote, nothing changes. assert_eq!( - r.state, - state, + r.state, state, "{:?},{:?}, state {:?}, want {:?}", - vt, - state, - r.state, - state + vt, state, r.state, state ); assert_eq!( - r.term, - orig_term, + r.term, orig_term, "{:?},{:?}, term {}, want {}", - vt, - state, - r.term, - orig_term + vt, state, r.term, orig_term ); // If state == Follower or PreCandidate, r hasn't voted yet. // In Candidate or Leader, it's voted for itself. @@ -2030,11 +2018,9 @@ fn test_candidate_reset_term(message_type: MessageType) { // follower c term is reset with leader's assert_eq!( - nt.peers[&3].term, - nt.peers[&1].term, + nt.peers[&3].term, nt.peers[&1].term, "follower term expected same term as leader's {}, got {}", - nt.peers[&1].term, - nt.peers[&3].term, + nt.peers[&1].term, nt.peers[&3].term, ) } @@ -3798,21 +3784,6 @@ fn test_learner_promotion() { assert_eq!(network.peers[&2].state, StateRole::Leader); } -// TestLearnerCannotVote checks that a learner can't vote even it receives a valid Vote request. -#[test] -fn test_learner_cannot_vote() { - let mut n2 = new_test_learner_raft(2, vec![1], vec![2], 10, 1, new_storage()); - n2.become_follower(1, INVALID_ID); - - let mut msg_vote = new_message(1, 2, MessageType::MsgRequestVote, 0); - msg_vote.set_term(2); - msg_vote.set_log_term(11); - msg_vote.set_index(11); - n2.step(msg_vote).unwrap(); - - assert_eq!(n2.msgs.len(), 0); -} - // TestLearnerLogReplication tests that a learner can receive entries from the leader. #[test] fn test_learner_log_replication() { @@ -3978,3 +3949,30 @@ fn test_remove_learner() { assert!(n1.prs().nodes().is_empty()); assert!(n1.prs().learner_nodes().is_empty()); } + +#[test] +fn test_learner_respond_vote() { + let mut n1 = new_test_learner_raft(1, vec![1, 2, 3], vec![], 10, 1, new_storage()); + n1.become_follower(1, INVALID_ID); + n1.reset_randomized_election_timeout(); + + let mut n3 = new_test_learner_raft(2, vec![1, 2], vec![3], 10, 1, new_storage()); + n3.become_follower(1, INVALID_ID); + n3.reset_randomized_election_timeout(); + + let timeout = n1.get_election_timeout(); + + let mut network = Network::new(vec![Some(n1), None, Some(n3)]); + for _ in 0..timeout << 1 { + network.peers.get_mut(&1).unwrap().tick(); + network.peers.get_mut(&3).unwrap().tick(); + } + + // MsgRequeestVote should only come from 1. + let msgs = read_messages(network.peers.get_mut(&1).unwrap()); + msgs.iter().for_each(|m| assert_eq!(m.get_from(), 1)); + + // Learner can respond vote messages so that 1 will be leader. + network.send(msgs); + assert_eq!(network.peers[&1].state, StateRole::Leader); +}