Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

raft: become_pre_candidate resets votes #83

Merged
merged 4 commits into from
Jul 4, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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};

Expand Down
4 changes: 3 additions & 1 deletion src/raft.rs
Original file line number Diff line number Diff line change
Expand Up @@ -828,6 +828,7 @@ impl<T: Storage> Raft<T> {
// 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);
}

Expand Down Expand Up @@ -937,7 +938,8 @@ impl<T: Storage> Raft<T> {
|| 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
Expand Down
6 changes: 4 additions & 2 deletions src/raw_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
89 changes: 87 additions & 2 deletions tests/cases/test_raft.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -4132,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| {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use new_test_raft_with_prevote?

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 {:?}",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default error message is quite informative.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. I will change it here, just changed it in #84.

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!(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why Candidate here? not PreCandidate?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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,
);
}