From b1fab32cf5737af026f91efda6f58c5c088267cd Mon Sep 17 00:00:00 2001 From: Jay Lee Date: Fri, 18 May 2018 11:16:24 +0800 Subject: [PATCH 1/4] support least election timeout --- src/raft.rs | 21 +++++++++++++++++++-- tests/cases/test_raft.rs | 1 + 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/raft.rs b/src/raft.rs index 8d1f0fb57..3d4dc2930 100644 --- a/src/raft.rs +++ b/src/raft.rs @@ -124,6 +124,11 @@ pub struct Config { /// rejoins the cluster. pub pre_vote: bool, + /// The least election timeout. In some cases, we hope some nodes has less possibility + /// to become leader. This configuration ensures that the randomized election_timeout + /// always not less than this value. + pub least_election_timeout_tick: usize, + /// read_only_option specifies how the read only request is processed. pub read_only_option: ReadOnlyOption, @@ -154,6 +159,15 @@ impl Config { )); } + if self.least_election_timeout_tick < self.election_tick + || self.least_election_timeout_tick >= self.election_tick * 2 + { + return Err(Error::ConfigInvalid( + "lease election timeout tick must be in [election_tick, 2 * election_tick)" + .to_owned(), + )); + } + if self.max_inflight_msgs == 0 { return Err(Error::ConfigInvalid( "max inflight messages must be greater than 0".to_owned(), @@ -235,6 +249,7 @@ pub struct Raft { // [election_timeout, 2 * election_timeout - 1]. It gets reset // when raft changes its state to follower or candidate. randomized_election_timeout: usize, + least_election_timeout: usize, /// Will be called when step** is about to be called. /// return false will skip step**. @@ -322,6 +337,7 @@ impl Raft { vote: Default::default(), heartbeat_elapsed: Default::default(), randomized_election_timeout: 0, + least_election_timeout: c.least_election_timeout_tick, skip_bcast_commit: c.skip_bcast_commit, tag: c.tag.to_owned(), }; @@ -410,6 +426,7 @@ impl Raft { // for testing leader lease pub fn set_randomized_election_timeout(&mut self, t: usize) { + assert!(t >= self.least_election_timeout); self.randomized_election_timeout = t; } @@ -1972,8 +1989,8 @@ impl Raft { pub fn reset_randomized_election_timeout(&mut self) { let prev_timeout = self.randomized_election_timeout; - let timeout = - self.election_timeout + rand::thread_rng().gen_range(0, self.election_timeout); + let max_timeout = self.election_timeout * 2; + let timeout = rand::thread_rng().gen_range(self.least_election_timeout, max_timeout); debug!( "{} reset election timeout {} -> {} at {}", self.tag, prev_timeout, timeout, self.election_elapsed diff --git a/tests/cases/test_raft.rs b/tests/cases/test_raft.rs index a3251058e..4eea7b17f 100644 --- a/tests/cases/test_raft.rs +++ b/tests/cases/test_raft.rs @@ -74,6 +74,7 @@ pub fn new_test_config(id: u64, peers: Vec, election: usize, heartbeat: usi id: id, peers: peers, election_tick: election, + least_election_timeout_tick: election, heartbeat_tick: heartbeat, max_size_per_msg: NO_LIMIT, max_inflight_msgs: 256, From 2a133553060ed0410aebb5cd23545586a17056ba Mon Sep 17 00:00:00 2001 From: Jay Lee Date: Sat, 19 May 2018 16:54:36 +0800 Subject: [PATCH 2/4] support larger timeout --- src/raft.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/raft.rs b/src/raft.rs index 3d4dc2930..7df77ccc2 100644 --- a/src/raft.rs +++ b/src/raft.rs @@ -159,12 +159,9 @@ impl Config { )); } - if self.least_election_timeout_tick < self.election_tick - || self.least_election_timeout_tick >= self.election_tick * 2 - { + if self.least_election_timeout_tick < self.election_tick { return Err(Error::ConfigInvalid( - "lease election timeout tick must be in [election_tick, 2 * election_tick)" - .to_owned(), + "lease election timeout tick must not less than election_tick".to_owned(), )); } @@ -1989,7 +1986,8 @@ impl Raft { pub fn reset_randomized_election_timeout(&mut self) { let prev_timeout = self.randomized_election_timeout; - let max_timeout = self.election_timeout * 2; + let max_timeout = + (self.least_election_timeout / self.election_timeout + 1) * self.election_timeout; let timeout = rand::thread_rng().gen_range(self.least_election_timeout, max_timeout); debug!( "{} reset election timeout {} -> {} at {}", From 58290c40911936b25a7f924be3d353f119443775 Mon Sep 17 00:00:00 2001 From: Jay Lee Date: Sun, 20 May 2018 01:17:20 +0800 Subject: [PATCH 3/4] generalize the API --- src/raft.rs | 53 +++++++++++++++++++++++++++++----------- tests/cases/test_raft.rs | 34 +++++++++++++++++++++++++- 2 files changed, 72 insertions(+), 15 deletions(-) diff --git a/src/raft.rs b/src/raft.rs index 7df77ccc2..b6dec5309 100644 --- a/src/raft.rs +++ b/src/raft.rs @@ -124,10 +124,13 @@ pub struct Config { /// rejoins the cluster. pub pre_vote: bool, - /// The least election timeout. In some cases, we hope some nodes has less possibility + /// The range of election timeout. In some cases, we hope some nodes has less possibility /// to become leader. This configuration ensures that the randomized election_timeout - /// always not less than this value. - pub least_election_timeout_tick: usize, + /// will always be suit in [min_election_tick, max_election_tick). + /// If it is None, then election_tick will be chosen. + pub min_election_tick: Option, + /// If it is None, then 2 * election_tick will be chosen. + pub max_election_tick: Option, /// read_only_option specifies how the read only request is processed. pub read_only_option: ReadOnlyOption, @@ -142,6 +145,17 @@ pub struct Config { } impl Config { + #[inline] + pub fn min_election_tick(&self) -> usize { + self.min_election_tick.unwrap_or(self.election_tick) + } + + #[inline] + pub fn max_election_tick(&self) -> usize { + self.max_election_tick + .unwrap_or_else(|| 2 * self.election_tick) + } + pub fn validate(&self) -> Result<()> { if self.id == INVALID_ID { return Err(Error::ConfigInvalid("invalid node id".to_owned())); @@ -159,10 +173,20 @@ impl Config { )); } - if self.least_election_timeout_tick < self.election_tick { - return Err(Error::ConfigInvalid( - "lease election timeout tick must not less than election_tick".to_owned(), - )); + let min_timeout = self.min_election_tick(); + let max_timeout = self.max_election_tick(); + if min_timeout < self.election_tick { + return Err(Error::ConfigInvalid(format!( + "min election tick {} must not be less than election_tick {}", + min_timeout, self.election_tick + ))); + } + + if min_timeout >= max_timeout { + return Err(Error::ConfigInvalid(format!( + "min election tick {} should be less than max election tick {}", + min_timeout, max_timeout + ))); } if self.max_inflight_msgs == 0 { @@ -243,10 +267,11 @@ pub struct Raft { election_timeout: usize, // randomized_election_timeout is a random number between - // [election_timeout, 2 * election_timeout - 1]. It gets reset + // [min_election_timeout, max_election_timeout - 1]. It gets reset // when raft changes its state to follower or candidate. randomized_election_timeout: usize, - least_election_timeout: usize, + min_election_timeout: usize, + max_election_timeout: usize, /// Will be called when step** is about to be called. /// return false will skip step**. @@ -334,7 +359,8 @@ impl Raft { vote: Default::default(), heartbeat_elapsed: Default::default(), randomized_election_timeout: 0, - least_election_timeout: c.least_election_timeout_tick, + min_election_timeout: c.min_election_tick(), + max_election_timeout: c.max_election_tick(), skip_bcast_commit: c.skip_bcast_commit, tag: c.tag.to_owned(), }; @@ -423,7 +449,7 @@ impl Raft { // for testing leader lease pub fn set_randomized_election_timeout(&mut self, t: usize) { - assert!(t >= self.least_election_timeout); + assert!(self.min_election_timeout <= t && t < self.max_election_timeout); self.randomized_election_timeout = t; } @@ -1986,9 +2012,8 @@ impl Raft { pub fn reset_randomized_election_timeout(&mut self) { let prev_timeout = self.randomized_election_timeout; - let max_timeout = - (self.least_election_timeout / self.election_timeout + 1) * self.election_timeout; - let timeout = rand::thread_rng().gen_range(self.least_election_timeout, max_timeout); + let timeout = + rand::thread_rng().gen_range(self.min_election_timeout, self.max_election_timeout); debug!( "{} reset election timeout {} -> {} at {}", self.tag, prev_timeout, timeout, self.election_elapsed diff --git a/tests/cases/test_raft.rs b/tests/cases/test_raft.rs index 4eea7b17f..8e05553a9 100644 --- a/tests/cases/test_raft.rs +++ b/tests/cases/test_raft.rs @@ -74,7 +74,6 @@ pub fn new_test_config(id: u64, peers: Vec, election: usize, heartbeat: usi id: id, peers: peers, election_tick: election, - least_election_timeout_tick: election, heartbeat_tick: heartbeat, max_size_per_msg: NO_LIMIT, max_inflight_msgs: 256, @@ -4086,3 +4085,36 @@ fn test_learner_respond_vote() { do_campaign(&mut network); assert_eq!(network.peers[&1].state, StateRole::Leader); } + +#[test] +fn test_election_tick_range() { + let mut cfg = new_test_config(1, vec![1, 2, 3], 10, 1); + let mut raft = Raft::new(&cfg, new_storage()); + for _ in 0..1000 { + raft.reset_randomized_election_timeout(); + let randomized_timeout = raft.get_randomized_election_timeout(); + assert!( + cfg.election_tick <= randomized_timeout && randomized_timeout < 2 * cfg.election_tick + ); + } + + cfg.min_election_tick = Some(cfg.election_tick); + cfg.validate().unwrap(); + + // Too small election tick. + cfg.min_election_tick = Some(cfg.election_tick - 1); + cfg.validate().unwrap_err(); + + // max_election_tick should be larger than min_election_tick + cfg.min_election_tick = Some(cfg.election_tick); + cfg.max_election_tick = Some(cfg.election_tick); + cfg.validate().unwrap_err(); + + cfg.max_election_tick = Some(cfg.election_tick + 1); + raft = Raft::new(&cfg, new_storage()); + for _ in 0..100 { + raft.reset_randomized_election_timeout(); + let randomized_timeout = raft.get_randomized_election_timeout(); + assert_eq!(randomized_timeout, cfg.election_tick); + } +} From 7b781aa96a85fb7cafff10c69cd962537fccf004 Mon Sep 17 00:00:00 2001 From: Jay Lee Date: Sun, 20 May 2018 20:13:51 +0800 Subject: [PATCH 4/4] address comment --- src/raft.rs | 21 ++++++++++++++------- tests/cases/test_raft.rs | 10 +++++----- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/raft.rs b/src/raft.rs index b6dec5309..640907f78 100644 --- a/src/raft.rs +++ b/src/raft.rs @@ -127,10 +127,10 @@ pub struct Config { /// The range of election timeout. In some cases, we hope some nodes has less possibility /// to become leader. This configuration ensures that the randomized election_timeout /// will always be suit in [min_election_tick, max_election_tick). - /// If it is None, then election_tick will be chosen. - pub min_election_tick: Option, - /// If it is None, then 2 * election_tick will be chosen. - pub max_election_tick: Option, + /// If it is 0, then election_tick will be chosen. + pub min_election_tick: usize, + /// If it is 0, then 2 * election_tick will be chosen. + pub max_election_tick: usize, /// read_only_option specifies how the read only request is processed. pub read_only_option: ReadOnlyOption, @@ -147,13 +147,20 @@ pub struct Config { impl Config { #[inline] pub fn min_election_tick(&self) -> usize { - self.min_election_tick.unwrap_or(self.election_tick) + if self.min_election_tick == 0 { + self.election_tick + } else { + self.min_election_tick + } } #[inline] pub fn max_election_tick(&self) -> usize { - self.max_election_tick - .unwrap_or_else(|| 2 * self.election_tick) + if self.max_election_tick == 0 { + 2 * self.election_tick + } else { + self.max_election_tick + } } pub fn validate(&self) -> Result<()> { diff --git a/tests/cases/test_raft.rs b/tests/cases/test_raft.rs index 8e05553a9..e675bb99e 100644 --- a/tests/cases/test_raft.rs +++ b/tests/cases/test_raft.rs @@ -4098,19 +4098,19 @@ fn test_election_tick_range() { ); } - cfg.min_election_tick = Some(cfg.election_tick); + cfg.min_election_tick = cfg.election_tick; cfg.validate().unwrap(); // Too small election tick. - cfg.min_election_tick = Some(cfg.election_tick - 1); + cfg.min_election_tick = cfg.election_tick - 1; cfg.validate().unwrap_err(); // max_election_tick should be larger than min_election_tick - cfg.min_election_tick = Some(cfg.election_tick); - cfg.max_election_tick = Some(cfg.election_tick); + cfg.min_election_tick = cfg.election_tick; + cfg.max_election_tick = cfg.election_tick; cfg.validate().unwrap_err(); - cfg.max_election_tick = Some(cfg.election_tick + 1); + cfg.max_election_tick = cfg.election_tick + 1; raft = Raft::new(&cfg, new_storage()); for _ in 0..100 { raft.reset_randomized_election_timeout();