From 06ef448b236190c2c38d940e0cc4e73c5810ec5f Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Thu, 1 Oct 2026 21:20:02 -0600 Subject: [PATCH 01/36] Test: overlap the follower lease with the election timeout after leader loss After an unplanned leader loss a follower waits for its leader lease (election_timeout_max) and then for a full sampled election timeout before it campaigns. The two windows start from the same last leader contact, so the first campaign is held back for at least election_timeout_max + election_timeout_min. The new case isolates the leader of a three-voter cluster with a [1500, 1600) ms election timeout and a 50 ms heartbeat. It expects a survivor to be elected within election_timeout_max plus one tick and a fixed slack, and never before election_timeout_min. On this revision it fails: a survivor is elected after 3.18 s against a 2.275 s bound, in every repeated run, with and without single-term-leader. Signed-off-by: VerifiedOrganic --- tests/tests/elect/main.rs | 1 + .../elect/t20_lease_and_timeout_overlap.rs | 90 +++++++++++++++++++ 2 files changed, 91 insertions(+) create mode 100644 tests/tests/elect/t20_lease_and_timeout_overlap.rs diff --git a/tests/tests/elect/main.rs b/tests/tests/elect/main.rs index a2ac37d03..d6b8a6518 100644 --- a/tests/tests/elect/main.rs +++ b/tests/tests/elect/main.rs @@ -11,3 +11,4 @@ mod fixtures; mod t10_elect_compare_last_log; mod t11_elect_seize_leadership; mod t13_elect_while_leader; +mod t20_lease_and_timeout_overlap; diff --git a/tests/tests/elect/t20_lease_and_timeout_overlap.rs b/tests/tests/elect/t20_lease_and_timeout_overlap.rs new file mode 100644 index 000000000..c628bbc10 --- /dev/null +++ b/tests/tests/elect/t20_lease_and_timeout_overlap.rs @@ -0,0 +1,90 @@ +use std::sync::Arc; +use std::time::Duration; + +use anyhow::Result; +use maplit::btreeset; +use openraft::Config; +use openraft::ServerState; +use tokio::time::Instant; + +use crate::fixtures::init_default_ut_tracing; +use crate::fixtures::RaftRouter; + +/// After an unplanned leader loss, a follower campaigns once both its leader lease and its sampled +/// election timeout have expired. The two run in parallel from the last leader contact; they are +/// not waited for one after the other. +/// +/// Every sampled election timeout covers the follower lease, so the first campaign starts within +/// `election_timeout_max` plus one tick of the last leader contact, never before +/// `election_timeout_min`. Waiting for the sum would hold every survivor back for at least +/// `election_timeout_max + election_timeout_min`. +#[async_entry::test(worker_threads = 8, init = "init_default_ut_tracing()", tracing_span = "debug")] +async fn leader_loss_campaign_overlaps_lease_and_election_timeout() -> Result<()> { + let config = Arc::new( + Config { + heartbeat_interval: 50, + election_timeout_min: 1_500, + election_timeout_max: 1_600, + ..Default::default() + } + .validate()?, + ); + let tick = Duration::from_millis(config.heartbeat_interval * 3 / 2); + let election_timeout_min = Duration::from_millis(config.election_timeout_min); + let election_timeout_max = Duration::from_millis(config.election_timeout_max); + + let mut router = RaftRouter::new(config.clone()); + + tracing::info!("--- create cluster of 0,1,2; node 0 becomes leader"); + router.new_cluster(btreeset! {0,1,2}, btreeset! {}).await?; + + let n0 = router.get_raft_handle(&0)?; + n0.wait(timeout()).state(ServerState::Leader, "node 0 is the initial leader").await?; + for id in [1, 2] { + router + .wait(&id, timeout()) + .metrics(|m| m.current_leader == Some(0), "the follower follows node 0") + .await?; + } + + tracing::info!("--- lose the leader: node 0 can neither send nor receive"); + let lost_at = Instant::now(); + router.set_network_error(0, true); + + tracing::info!("--- a survivor is elected"); + for id in [1, 2] { + router + .wait(&id, Some(Duration::from_secs(10))) + .metrics( + |m| m.current_leader == Some(1) || m.current_leader == Some(2), + "a survivor leads after the leader loss", + ) + .await?; + } + let elapsed = lost_at.elapsed(); + tracing::info!("--- a survivor was elected {:?} after the leader loss", elapsed); + + // The last leader contact is at most one tick before the loss. The campaign then starts on + // the first tick after the longer of the lease and the sampled timeout; the remaining slack + // covers the in-memory vote round trip and scheduling. + let slack = Duration::from_millis(600); + assert!( + elapsed < election_timeout_max + tick + slack, + "the first campaign waits for the longer of the lease and the timeout, not their sum: \ + elected after {:?}, expected below {:?}", + elapsed, + election_timeout_max + tick + slack + ); + assert!( + elapsed + tick >= election_timeout_min, + "the lease still holds back every campaign: elected after {:?}, lease {:?}", + elapsed, + election_timeout_min + ); + + Ok(()) +} + +fn timeout() -> Option { + Some(Duration::from_millis(2_000)) +} From 641b6f50498ee2a9c8df18528afc642ec050a68d Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Thu, 1 Oct 2026 21:23:02 -0600 Subject: [PATCH 02/36] Fix: campaign after the longer of the leader lease and election timeout A follower that held a committed vote campaigned only after its leader lease and then a full sampled election timeout, both measured from its last leader contact. With the lease fixed at election_timeout_max, an unplanned leader loss went undetected for election_timeout_max plus the sampled timeout plus one tick. The lease and the election timeout now run in parallel: a follower campaigns once the longer of the two has expired. The follower lease is election_timeout_min, the minimum election timeout of the Raft dissertation (section 4.2.3), so every sampled timeout covers it and campaigns stay randomized. While the lease is valid a follower still rejects every other candidate. The heartbeat_reject_vote case now keeps its followers from campaigning while it waits for the lease to expire, because the first campaign is no longer held back a further election timeout after expiry. The leader-loss case from the previous commit now elects a survivor after about 1.6 s, within its 2.275 s bound. Signed-off-by: VerifiedOrganic --- openraft/src/core/raft_core.rs | 8 ++-- openraft/src/docs/protocol/leader_lease.md | 16 ++++++-- openraft/src/engine/engine_config.rs | 5 ++- openraft/src/engine/tests/elect_test.rs | 40 ++++++++++++++++++- openraft/src/engine/time_state.rs | 20 +++++++++- .../t61_heartbeat_reject_vote.rs | 5 +++ 6 files changed, 81 insertions(+), 13 deletions(-) diff --git a/openraft/src/core/raft_core.rs b/openraft/src/core/raft_core.rs index 6a9c398e0..8c515e5d9 100644 --- a/openraft/src/core/raft_core.rs +++ b/openraft/src/core/raft_core.rs @@ -1632,11 +1632,9 @@ where let utime = self.engine.state.vote_last_modified(); let timer_config = &self.engine.config.timer_config; - let mut election_timeout = if current_vote.is_committed() { - timer_config.leader_lease + timer_config.election_timeout - } else { - timer_config.election_timeout - }; + // The leader lease and the sampled election timeout run in parallel from the last + // update of a committed vote: campaign after the longer of the two. + let mut election_timeout = timer_config.election_wait(current_vote.is_committed()); if self.engine.is_there_greater_log() { election_timeout += timer_config.smaller_log_timeout; diff --git a/openraft/src/docs/protocol/leader_lease.md b/openraft/src/docs/protocol/leader_lease.md index f2c728a07..4c6d73271 100644 --- a/openraft/src/docs/protocol/leader_lease.md +++ b/openraft/src/docs/protocol/leader_lease.md @@ -56,15 +56,23 @@ For an uncommitted vote (the vote that is saved upon receiving a `VoteRequest`): For a committed vote (the vote that is saved upon receiving `AppendEntriesRequest` or `InstallSnapshotRequest`): - The node **WILL NOT** handle a `VoteRequest` before the `leader_lease` expires; -- The node **WILL NOT** elect itself until `leader_lease + election_timeout` has passed. +- The node **WILL NOT** elect itself until both the `leader_lease` and the + `election_timeout` have expired. Both start at `Vote.last_update_time` and run + in parallel, so the node waits for the longer of the two, not for their sum. + +The `leader_lease` is `election_timeout_min`: a follower rejects other +candidates for the minimum election timeout after it last heard from the +leader (Raft dissertation, section 4.2.3). Every sampled `election_timeout` is +at least that long, so the next election time is the sampled timeout and +campaigns stay randomized. ```text .--------- Accept VoteRequest | - | .- Next election time - | | ---------+----+-------+--------------> wall clock time + | .- Next election time + | | +--------+----+----+--------------> wall clock time | '- Vote.last_update_time diff --git a/openraft/src/engine/engine_config.rs b/openraft/src/engine/engine_config.rs index 9e0053a2c..ee4a3cf26 100644 --- a/openraft/src/engine/engine_config.rs +++ b/openraft/src/engine/engine_config.rs @@ -65,7 +65,10 @@ impl EngineConfig { timer_config: time_state::Config { election_timeout, smaller_log_timeout: Duration::from_millis(config.election_timeout_max * 2), - leader_lease: Duration::from_millis(config.election_timeout_max), + // A follower rejects other candidates for the minimum election timeout after its + // last leader contact (Raft dissertation 4.2.3). Every sampled election timeout + // covers this lease, so the lease never delays a campaign. + leader_lease: Duration::from_millis(config.election_timeout_min), }, } } diff --git a/openraft/src/engine/tests/elect_test.rs b/openraft/src/engine/tests/elect_test.rs index 472d22ecd..da083780b 100644 --- a/openraft/src/engine/tests/elect_test.rs +++ b/openraft/src/engine/tests/elect_test.rs @@ -181,7 +181,7 @@ fn test_default_election_timer_bounds_are_coherent() { assert!(minimum <= config.timer_config.election_timeout); assert!(config.timer_config.election_timeout < maximum); - assert_eq!(maximum, config.timer_config.leader_lease); + assert_eq!(minimum, config.timer_config.leader_lease); assert!(config.timer_config.smaller_log_timeout > maximum); } @@ -193,7 +193,7 @@ fn test_election_timeout_is_resampled_for_every_campaign() { let leader_lease = config.timer_config.leader_lease; let smaller_log_timeout = config.timer_config.smaller_log_timeout; assert_eq!(Duration::from_millis(100), config.timer_config.election_timeout); - assert_eq!(Duration::from_millis(104), leader_lease); + assert_eq!(Duration::from_millis(100), leader_lease); assert_eq!(Duration::from_millis(208), smaller_log_timeout); let mut engine = scripted_engine(1, config); @@ -209,6 +209,42 @@ fn test_election_timeout_is_resampled_for_every_campaign() { assert_rng_script_exhausted(); } +#[test] +fn test_leased_election_wait_is_the_longer_of_lease_and_timeout() { + let mut timer = crate::engine::time_state::Config { + election_timeout: Duration::from_millis(103), + smaller_log_timeout: Duration::from_millis(208), + leader_lease: Duration::from_millis(100), + }; + assert_eq!(Duration::from_millis(103), timer.election_wait(true)); + assert_eq!(Duration::from_millis(103), timer.election_wait(false)); + + // A lease longer than the sampled timeout bounds the wait; it is never added to it. + timer.leader_lease = Duration::from_millis(150); + assert_eq!(Duration::from_millis(150), timer.election_wait(true)); + assert_eq!(Duration::from_millis(103), timer.election_wait(false)); +} + +#[test] +fn test_every_sampled_election_timeout_covers_the_lease() { + set_rng_script([0, 1_u64 << 62, 1_u64 << 63, 3_u64 << 62]); + + let config = EngineConfig::new::(1, &election_config()); + assert_eq!(Duration::from_millis(100), config.timer_config.leader_lease); + + let mut engine = scripted_engine(1, config); + let timer = &engine.config.timer_config; + assert_eq!(timer.election_timeout, timer.election_wait(true)); + for _ in 0..3 { + engine.elect(); + // The wait follows each newly sampled timeout, so campaigns stay randomized. + let timer = &engine.config.timer_config; + assert!(timer.leader_lease <= timer.election_timeout); + assert_eq!(timer.election_timeout, timer.election_wait(true)); + } + assert_rng_script_exhausted(); +} + #[test] fn test_repeated_tied_campaigns_resample_to_different_timeouts() { set_rng_script([0, 0, 1_u64 << 62, 3_u64 << 62, 1_u64 << 63, 0]); diff --git a/openraft/src/engine/time_state.rs b/openraft/src/engine/time_state.rs index 4cf1c6686..66677322d 100644 --- a/openraft/src/engine/time_state.rs +++ b/openraft/src/engine/time_state.rs @@ -19,6 +19,8 @@ pub(crate) struct Config { /// /// When a follower or learner perceives an active leader, such as by receiving an AppendEntries /// message, it should not grant another candidate to become the leader during this period. + /// + /// It is the minimum election timeout, so every sampled `election_timeout` covers it. pub(crate) leader_lease: Duration, } @@ -27,7 +29,23 @@ impl Default for Config { Self { election_timeout: Duration::from_millis(150), smaller_log_timeout: Duration::from_millis(600), - leader_lease: Duration::from_millis(300), + leader_lease: Duration::from_millis(150), + } + } +} + +impl Config { + /// Return how long a voter waits after the last update of its vote before it campaigns. + /// + /// A committed vote carries the leader lease. The lease and the sampled election timeout both + /// start at the vote's last update and run in parallel, so the voter waits for the longer of + /// the two, never for their sum. While the lease is valid the voter also rejects other + /// candidates. An uncommitted vote, such as a vote granted to a candidate, has no lease. + pub(crate) fn election_wait(&self, leased: bool) -> Duration { + if leased { + std::cmp::max(self.leader_lease, self.election_timeout) + } else { + self.election_timeout } } } diff --git a/tests/tests/append_entries/t61_heartbeat_reject_vote.rs b/tests/tests/append_entries/t61_heartbeat_reject_vote.rs index 5161e8a9b..670736c05 100644 --- a/tests/tests/append_entries/t61_heartbeat_reject_vote.rs +++ b/tests/tests/append_entries/t61_heartbeat_reject_vote.rs @@ -74,6 +74,11 @@ async fn heartbeat_reject_vote() -> Result<()> { tracing::info!(log_index, "--- disable heartbeat, vote request will be granted"); { + // A follower campaigns as soon as both its lease and its election timeout expire. Keep the + // followers from campaigning so that only the expired lease is observed here. + for id in [1, 2] { + router.get_raft_handle(&id)?.runtime_config().elect(false); + } node0.runtime_config().heartbeat(false); sleep(Duration::from_millis(1500)).await; From 5b881be2e5b7d3de251701fe8ad949e6a62a5dfc Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Thu, 1 Oct 2026 21:24:54 -0600 Subject: [PATCH 03/36] Test: bound AppendEntries by its own deadline instead of the heartbeat Every AppendEntries RPC, including heartbeats and the heartbeats that confirm leadership for a linearizable read, is bounded by heartbeat_interval. A deployment that wants frequent heartbeats must therefore give every AppendEntries a deadline no longer than the heartbeat, and a follower that needs longer than one heartbeat to answer is abandoned and sent the request again. Add Config::append_entries_timeout, documented as the deadline of one AppendEntries RPC and defaulting to heartbeat_interval. It is parsed and reported but not used yet. The test router records the hard deadline of every AppendEntries RPC. The new case uses a 50 ms heartbeat and a 1,000 ms AppendEntries deadline. On this revision every AppendEntries carries 50 ms, and with that assertion disabled, a follower that holds one AppendEntries for 300 ms receives it four times instead of once. Signed-off-by: VerifiedOrganic --- openraft/src/config/config.rs | 19 +++++ openraft/src/config/config_test.rs | 18 +++++ tests/tests/append_entries/main.rs | 1 + .../t62_append_entries_deadline.rs | 77 +++++++++++++++++++ tests/tests/fixtures/mod.rs | 12 ++- 5 files changed, 126 insertions(+), 1 deletion(-) create mode 100644 tests/tests/append_entries/t62_append_entries_deadline.rs diff --git a/openraft/src/config/config.rs b/openraft/src/config/config.rs index 8a1370794..8fe53a9dc 100644 --- a/openraft/src/config/config.rs +++ b/openraft/src/config/config.rs @@ -128,6 +128,17 @@ pub struct Config { #[clap(long, default_value = "50")] pub heartbeat_interval: u64, + /// The deadline of one AppendEntries RPC in milliseconds. + /// + /// It bounds every AppendEntries RPC a leader sends: log replication, heartbeats and the + /// heartbeats that confirm leadership for a linearizable read. Heartbeats are still sent every + /// `heartbeat_interval`, so a follower may take longer than one heartbeat interval to answer + /// without its request being abandoned and resent. It must be positive. + /// + /// When unset, the deadline is `heartbeat_interval`. + #[clap(long)] + pub append_entries_timeout: Option, + /// The timeout for sending then installing the last snapshot segment, /// in millisecond. It is also used as the timeout for sending a non-last segment, if /// `send_snapshot_timeout` is 0. @@ -272,6 +283,14 @@ impl Config { RT::thread_rng().gen_range(self.election_timeout_min..self.election_timeout_max) } + /// Get the deadline of one AppendEntries RPC. + /// + /// It is [`append_entries_timeout`](Self::append_entries_timeout) when set, otherwise + /// [`heartbeat_interval`](Self::heartbeat_interval). + pub fn append_entries_timeout(&self) -> Duration { + Duration::from_millis(self.append_entries_timeout.unwrap_or(self.heartbeat_interval)) + } + /// Get the timeout for sending and installing the last snapshot segment. pub fn install_snapshot_timeout(&self) -> Duration { Duration::from_millis(self.install_snapshot_timeout) diff --git a/openraft/src/config/config_test.rs b/openraft/src/config/config_test.rs index 640ed6d80..d0530cabc 100644 --- a/openraft/src/config/config_test.rs +++ b/openraft/src/config/config_test.rs @@ -98,6 +98,24 @@ fn test_build() -> anyhow::Result<()> { Ok(()) } +#[test] +fn test_config_append_entries_timeout() -> anyhow::Result<()> { + let config = Config::build(&["foo", "--heartbeat-interval=40"])?; + assert_eq!(None, config.append_entries_timeout); + assert_eq!( + Duration::from_millis(40), + config.append_entries_timeout(), + "by default an AppendEntries RPC is bounded by the heartbeat interval" + ); + + let config = Config::build(&["foo", "--heartbeat-interval=40", "--append-entries-timeout=900"])?; + assert_eq!(Some(900), config.append_entries_timeout); + assert_eq!(Duration::from_millis(900), config.append_entries_timeout()); + assert_eq!(40, config.heartbeat_interval); + + Ok(()) +} + #[test] fn test_config_snapshot_policy() -> anyhow::Result<()> { let config = Config::build(&["foo", "--snapshot-policy=never"])?; diff --git a/tests/tests/append_entries/main.rs b/tests/tests/append_entries/main.rs index 37b54e51e..3d5675312 100644 --- a/tests/tests/append_entries/main.rs +++ b/tests/tests/append_entries/main.rs @@ -18,4 +18,5 @@ mod t30_replication_1_voter_to_isolated_learner; mod t60_enable_heartbeat; mod t61_heartbeat_reject_vote; mod t61_large_heartbeat; +mod t62_append_entries_deadline; mod t90_issue_216_stale_last_log_id; diff --git a/tests/tests/append_entries/t62_append_entries_deadline.rs b/tests/tests/append_entries/t62_append_entries_deadline.rs new file mode 100644 index 000000000..b697749d2 --- /dev/null +++ b/tests/tests/append_entries/t62_append_entries_deadline.rs @@ -0,0 +1,77 @@ +use std::sync::Arc; +use std::time::Duration; + +use anyhow::Result; +use maplit::btreeset; +use openraft::Config; +use tokio::time::sleep; + +use crate::fixtures::init_default_ut_tracing; +use crate::fixtures::RaftRouter; + +/// AppendEntries is bounded by its own deadline, not by the heartbeat interval. +/// +/// Log replication, heartbeats and the heartbeats that confirm leadership for a linearizable read +/// all carry `Config::append_entries_timeout` as their hard deadline. A follower that holds one +/// AppendEntries for several heartbeat intervals is waited for instead of being abandoned and sent +/// the request again. +#[async_entry::test(worker_threads = 8, init = "init_default_ut_tracing()", tracing_span = "debug")] +async fn append_entries_deadline_is_independent_of_the_heartbeat_interval() -> Result<()> { + let config = Arc::new( + Config { + heartbeat_interval: 50, + append_entries_timeout: Some(1_000), + election_timeout_min: 1_500, + election_timeout_max: 1_600, + ..Default::default() + } + .validate()?, + ); + let deadline = Duration::from_millis(1_000); + + let mut router = RaftRouter::new(config.clone()); + + tracing::info!("--- create cluster of 0,1,2; node 0 becomes leader"); + let mut log_index = router.new_cluster(btreeset! {0,1,2}, btreeset! {}).await?; + + tracing::info!("--- replicate logs and confirm leadership for a linearizable read"); + router.client_request_many(0, "foo", 3).await?; + log_index += 3; + for id in [0, 1, 2] { + router.wait(&id, timeout()).applied_index(Some(log_index), "logs are replicated").await?; + } + router.ensure_linearizable(0).await?; + + let sent = router.append_entries_deadlines(); + assert!(!sent.is_empty(), "AppendEntries RPCs were sent"); + for (target, ttl) in &sent { + assert_eq!( + deadline, *ttl, + "an AppendEntries RPC to node {} is bounded by the AppendEntries deadline", + target + ); + } + + tracing::info!("--- node 2 holds AppendEntries for six heartbeat intervals"); + let before_hold = router.append_entries_deadlines().len(); + router.set_rpc_blocked(2, true); + sleep(Duration::from_millis(300)).await; + router.set_rpc_blocked(2, false); + let during_hold = + router.append_entries_deadlines()[before_hold..].iter().filter(|(target, _)| *target == 2).count(); + assert_eq!( + 1, during_hold, + "the held AppendEntries is waited for within its deadline, not abandoned and resent" + ); + + tracing::info!("--- node 2 keeps up after the hold"); + router.client_request_many(0, "foo", 1).await?; + log_index += 1; + router.wait(&2, timeout()).applied_index(Some(log_index), "node 2 keeps replicating").await?; + + Ok(()) +} + +fn timeout() -> Option { + Some(Duration::from_millis(2_000)) +} diff --git a/tests/tests/fixtures/mod.rs b/tests/tests/fixtures/mod.rs index cad2044f0..0a7ba41bf 100644 --- a/tests/tests/fixtures/mod.rs +++ b/tests/tests/fixtures/mod.rs @@ -274,6 +274,9 @@ pub struct TypedRaftRouter { /// Targets whose AppendEntries RPC never returns, emulating a hung follower. blocked_rpc: Arc>>, + + /// The target and hard deadline of every AppendEntries RPC, in the order they are sent. + append_entries_deadlines: Arc>>, } /// Default `RaftRouter` for memstore. @@ -312,6 +315,7 @@ impl Builder { rpc_pre_hook: Default::default(), rpc_observers: Default::default(), blocked_rpc: Default::default(), + append_entries_deadlines: Default::default(), } } } @@ -379,6 +383,11 @@ impl TypedRaftRouter { self.rpc_count.lock().unwrap().clone() } + /// The target and hard deadline of every AppendEntries RPC sent so far, in sending order. + pub fn append_entries_deadlines(&self) -> Vec<(MemNodeId, Duration)> { + self.append_entries_deadlines.lock().unwrap().clone() + } + /// Create a cluster: 0 is the initial leader, others are voters and learners /// /// NOTE: it create a single node cluster first, then change it to a multi-voter cluster. @@ -1051,11 +1060,12 @@ impl RaftNetwork for RaftRouterNetwork { async fn append_entries( &mut self, mut rpc: AppendEntriesRequest, - _option: RPCOption, + option: RPCOption, ) -> Result, RPCError>> { let from_id = rpc.vote.leader_id().voted_for().unwrap(); tracing::debug!("append_entries to id={} {}", self.target, rpc.summary()); + self.owner.append_entries_deadlines.lock().unwrap().push((self.target, option.hard_ttl())); self.owner.count_rpc(RPCTypes::AppendEntries); self.owner.call_rpc_pre_hook(rpc.clone(), from_id, self.target)?; self.owner.emit_rpc_error(from_id, self.target)?; From 4c08c6f4f0e35e57337be075ca699c5772cab686 Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Thu, 1 Oct 2026 21:28:04 -0600 Subject: [PATCH 04/36] Fix: bound AppendEntries by its own deadline Log replication, heartbeats and the heartbeats that confirm leadership for a linearizable read now use Config::append_entries_timeout as their hard deadline instead of heartbeat_interval. Heartbeats are still scheduled every heartbeat interval. Unset, the deadline remains the heartbeat interval. The deadline case from the previous commit now passes: every AppendEntries carries the configured 1,000 ms deadline, and a follower that holds one AppendEntries for 300 ms receives it once. Signed-off-by: VerifiedOrganic --- openraft/src/core/raft_core.rs | 5 +++-- openraft/src/replication/mod.rs | 8 ++++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/openraft/src/core/raft_core.rs b/openraft/src/core/raft_core.rs index 8c515e5d9..9b52483c6 100644 --- a/openraft/src/core/raft_core.rs +++ b/openraft/src/core/raft_core.rs @@ -337,7 +337,8 @@ where let my_id = self.id.clone(); let my_vote = self.engine.state.vote_ref().clone(); - let ttl = Duration::from_millis(self.config.heartbeat_interval); + // Leadership is confirmed with heartbeats, each bounded by the AppendEntries deadline. + let ttl = self.config.append_entries_timeout(); let eff_mem = self.engine.state.membership_state.effective().clone(); let core_tx = self.tx_notify.clone(); @@ -348,7 +349,7 @@ where return; } - // Spawn parallel requests, all with the standard timeout for heartbeats. + // Spawn parallel requests, all with the AppendEntries deadline. let mut pending = FuturesUnordered::new(); let voter_progresses = { diff --git a/openraft/src/replication/mod.rs b/openraft/src/replication/mod.rs index ab0a50334..5d0f8a846 100644 --- a/openraft/src/replication/mod.rs +++ b/openraft/src/replication/mod.rs @@ -507,14 +507,18 @@ where }; // Send the payload. + // AppendEntries, heartbeats included, has its own deadline. Heartbeats are still scheduled + // every heartbeat interval; a request that is not answered within the heartbeat interval is + // not abandoned before this deadline. + let the_timeout = self.config.append_entries_timeout(); + tracing::debug!( payload=%payload.summary(), now = debug(leader_time), "start sending append_entries, timeout: {:?}", - self.config.heartbeat_interval + the_timeout ); - let the_timeout = Duration::from_millis(self.config.heartbeat_interval); let option = RPCOption::new(the_timeout); let res = C::timeout(the_timeout, self.network.append_entries(payload, option)).await; From 198333bfc6dabbfc3ec4dcaeac005c8ba4a82e56 Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Thu, 1 Oct 2026 21:29:16 -0600 Subject: [PATCH 05/36] Test: observe the first campaign and isolate the triggered election With single-term-leader, the first survivor to campaign after a leader loss can be rejected by a survivor whose last leader contact was one heartbeat later and whose lease is therefore still valid. The two then split the next term, and the leader is elected one election timeout later. The overlap case now measures the start of the first campaign, a survivor's term exceeding the lost leader's term, and separately waits for a new leader. Restored to the engine before the fix, it still fails: the first campaign starts after 3.18 s against a 2.275 s bound, with and without single-term-leader. elect_seize_leadership disables heartbeats, so every follower campaigns as soon as its lease and election timeout expire and competes with the triggered election. It now disables automatic elections and lets the leases expire before it triggers the election. Signed-off-by: VerifiedOrganic --- .../tests/elect/t11_elect_seize_leadership.rs | 6 ++ .../elect/t20_lease_and_timeout_overlap.rs | 57 ++++++++++++------- 2 files changed, 44 insertions(+), 19 deletions(-) diff --git a/tests/tests/elect/t11_elect_seize_leadership.rs b/tests/tests/elect/t11_elect_seize_leadership.rs index 089e6e228..ea1a02241 100644 --- a/tests/tests/elect/t11_elect_seize_leadership.rs +++ b/tests/tests/elect/t11_elect_seize_leadership.rs @@ -12,9 +12,12 @@ use crate::fixtures::RaftRouter; /// A node with higher term takes leadership from the current leader. #[async_entry::test(worker_threads = 8, init = "init_default_ut_tracing()", tracing_span = "debug")] async fn elect_seize_leadership() -> Result<()> { + // Only the triggered election runs. Without heartbeats every follower would otherwise campaign + // as soon as its lease and election timeout expire, and compete with the triggered one. let config = Arc::new( Config { enable_heartbeat: false, + enable_elect: false, ..Default::default() } .validate()?, @@ -28,6 +31,9 @@ async fn elect_seize_leadership() -> Result<()> { let n0 = router.get_raft_handle(&0)?; n0.wait(timeout()).state(ServerState::Leader, "node 0 becomes leader").await?; + tracing::info!(log_index, "--- let every leader lease expire without heartbeats"); + tokio::time::sleep(Duration::from_millis(config.election_timeout_max)).await; + tracing::info!(log_index, "--- trigger election on node 1"); { let n1 = router.get_raft_handle(&1)?; diff --git a/tests/tests/elect/t20_lease_and_timeout_overlap.rs b/tests/tests/elect/t20_lease_and_timeout_overlap.rs index c628bbc10..d4ca7c47c 100644 --- a/tests/tests/elect/t20_lease_and_timeout_overlap.rs +++ b/tests/tests/elect/t20_lease_and_timeout_overlap.rs @@ -5,6 +5,7 @@ use anyhow::Result; use maplit::btreeset; use openraft::Config; use openraft::ServerState; +use tokio::time::sleep; use tokio::time::Instant; use crate::fixtures::init_default_ut_tracing; @@ -18,6 +19,10 @@ use crate::fixtures::RaftRouter; /// `election_timeout_max` plus one tick of the last leader contact, never before /// `election_timeout_min`. Waiting for the sum would hold every survivor back for at least /// `election_timeout_max + election_timeout_min`. +/// +/// The first campaign is observed as a survivor's term exceeding the lost leader's term, whether +/// or not that campaign wins: a survivor whose last leader contact was a little later can still +/// reject it under its own lease, and then a later campaign elects the new leader. #[async_entry::test(worker_threads = 8, init = "init_default_ut_tracing()", tracing_span = "debug")] async fn leader_loss_campaign_overlaps_lease_and_election_timeout() -> Result<()> { let config = Arc::new( @@ -47,41 +52,55 @@ async fn leader_loss_campaign_overlaps_lease_and_election_timeout() -> Result<() .await?; } + let leader_term = n0.metrics().borrow().current_term; + tracing::info!("--- lose the leader: node 0 can neither send nor receive"); let lost_at = Instant::now(); router.set_network_error(0, true); - tracing::info!("--- a survivor is elected"); - for id in [1, 2] { - router - .wait(&id, Some(Duration::from_secs(10))) - .metrics( - |m| m.current_leader == Some(1) || m.current_leader == Some(2), - "a survivor leads after the leader loss", - ) - .await?; - } - let elapsed = lost_at.elapsed(); - tracing::info!("--- a survivor was elected {:?} after the leader loss", elapsed); + tracing::info!("--- a survivor campaigns: its term exceeds the lost leader's term"); + let survivors = [router.get_raft_handle(&1)?, router.get_raft_handle(&2)?]; + let campaigned = loop { + if survivors.iter().any(|n| n.metrics().borrow().current_term > leader_term) { + break lost_at.elapsed(); + } + assert!( + lost_at.elapsed() < Duration::from_secs(10), + "a survivor campaigns after the leader loss" + ); + sleep(Duration::from_millis(5)).await; + }; + tracing::info!("--- a survivor campaigned {:?} after the leader loss", campaigned); // The last leader contact is at most one tick before the loss. The campaign then starts on // the first tick after the longer of the lease and the sampled timeout; the remaining slack - // covers the in-memory vote round trip and scheduling. + // covers polling and scheduling. let slack = Duration::from_millis(600); assert!( - elapsed < election_timeout_max + tick + slack, + campaigned < election_timeout_max + tick + slack, "the first campaign waits for the longer of the lease and the timeout, not their sum: \ - elected after {:?}, expected below {:?}", - elapsed, + campaigned after {:?}, expected below {:?}", + campaigned, election_timeout_max + tick + slack ); assert!( - elapsed + tick >= election_timeout_min, - "the lease still holds back every campaign: elected after {:?}, lease {:?}", - elapsed, + campaigned + tick >= election_timeout_min, + "the lease still holds back every campaign: campaigned after {:?}, lease {:?}", + campaigned, election_timeout_min ); + tracing::info!("--- a survivor is elected"); + for id in [1, 2] { + router + .wait(&id, Some(Duration::from_secs(10))) + .metrics( + |m| m.current_leader == Some(1) || m.current_leader == Some(2), + "a survivor leads after the leader loss", + ) + .await?; + } + Ok(()) } From ef374a9a427268a2ab31818fd849f7dfb36f5d33 Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Thu, 1 Oct 2026 21:33:16 -0600 Subject: [PATCH 06/36] Test: keep a cut-off voter from deposing a healthy leader on rejoin A voter that is cut off from consensus traffic campaigns on every election timeout and persists each new term. When it can be reached again, or after it restarts from its durable state, the leader sees the higher term in an AppendEntries response and steps down, although a quorum served it throughout. Add Config::enable_pre_vote, documented as running a Pre-Vote round before a real election and treated as disabled when unset. It is parsed but not used yet. The new cases enable it and cut one voter of a three- and a five-voter cluster off for four election timeouts while the others keep committing. The voter then reconnects, or restarts from its stores while still cut off and then reconnects. They expect every voter to stay in the leader's term, the leader to keep leading, and the voter to catch up. A further case expects a voter whose peers are all unreachable to keep its term, and one checks that a survivor is still elected after the leader is lost. On this revision the cut-off voter reaches term 5 from term 1 in all four rejoin cases and in the unreachable case; with the rejoin checks ordered after catch-up, the leader was deposed and re-elected in term 6 or 7. The leader-loss case passes. Signed-off-by: VerifiedOrganic --- openraft/src/config/config.rs | 24 ++++ openraft/src/config/config_test.rs | 17 +++ tests/tests/elect/main.rs | 1 + tests/tests/elect/t21_pre_vote.rs | 215 +++++++++++++++++++++++++++++ 4 files changed, 257 insertions(+) create mode 100644 tests/tests/elect/t21_pre_vote.rs diff --git a/openraft/src/config/config.rs b/openraft/src/config/config.rs index 8fe53a9dc..9948a0846 100644 --- a/openraft/src/config/config.rs +++ b/openraft/src/config/config.rs @@ -254,6 +254,30 @@ pub struct Config { default_missing_value = "true" )] pub enable_elect: bool, + + /// Whether a follower runs a Pre-Vote round before it increments its term and starts a real + /// election. + /// + /// When enabled, a voter whose election timer fires asks the other voters whether they *would* + /// grant it a vote at `term + 1`, without persisting a vote or changing its term. Only when a + /// quorum would grant does it start the real election. A voter that cannot currently win, for + /// example one that is cut off from the others, restarted, or behind on the log, therefore + /// does not raise its term and depose a healthy leader once it can be reached again. + /// + /// Pre-Vote uses the separate + /// [`RaftNetwork::pre_vote`](`crate::network::RaftNetwork::pre_vote`) RPC. A network that does + /// not implement it reports every Pre-Vote as granted, so elections proceed as without + /// Pre-Vote; a transport error is never counted as a grant. + /// + /// `None`, the default, is treated as disabled. + // clap 4 requires `num_args = 0..=1`, or it complains about missing arg error + // https://github.com/clap-rs/clap/discussions/4374 + #[clap(long, + action = clap::ArgAction::Set, + num_args = 0..=1, + default_missing_value = "true" + )] + pub enable_pre_vote: Option, } /// Updatable config for a raft runtime. diff --git a/openraft/src/config/config_test.rs b/openraft/src/config/config_test.rs index d0530cabc..f59729dc7 100644 --- a/openraft/src/config/config_test.rs +++ b/openraft/src/config/config_test.rs @@ -116,6 +116,23 @@ fn test_config_append_entries_timeout() -> anyhow::Result<()> { Ok(()) } +#[test] +fn test_config_enable_pre_vote() -> anyhow::Result<()> { + let config = Config::build(&["foo"])?; + assert_eq!(None, config.enable_pre_vote); + + let config = Config::build(&["foo", "--enable-pre-vote"])?; + assert_eq!(Some(true), config.enable_pre_vote); + + let config = Config::build(&["foo", "--enable-pre-vote=true"])?; + assert_eq!(Some(true), config.enable_pre_vote); + + let config = Config::build(&["foo", "--enable-pre-vote=false"])?; + assert_eq!(Some(false), config.enable_pre_vote); + + Ok(()) +} + #[test] fn test_config_snapshot_policy() -> anyhow::Result<()> { let config = Config::build(&["foo", "--snapshot-policy=never"])?; diff --git a/tests/tests/elect/main.rs b/tests/tests/elect/main.rs index d6b8a6518..0e2a236d0 100644 --- a/tests/tests/elect/main.rs +++ b/tests/tests/elect/main.rs @@ -12,3 +12,4 @@ mod t10_elect_compare_last_log; mod t11_elect_seize_leadership; mod t13_elect_while_leader; mod t20_lease_and_timeout_overlap; +mod t21_pre_vote; diff --git a/tests/tests/elect/t21_pre_vote.rs b/tests/tests/elect/t21_pre_vote.rs new file mode 100644 index 000000000..67704f8c1 --- /dev/null +++ b/tests/tests/elect/t21_pre_vote.rs @@ -0,0 +1,215 @@ +use std::sync::Arc; +use std::time::Duration; + +use anyhow::Result; +use maplit::btreeset; +use openraft::Config; +use openraft::ServerState; +use tokio::time::sleep; +use tokio::time::Instant; + +use crate::fixtures::init_default_ut_tracing; +use crate::fixtures::RaftRouter; + +/// A fault window several election timeouts long, as in a fault campaign that cuts one voter off +/// from consensus traffic while the others keep committing. +const ISOLATION_ELECTION_TIMEOUTS: u64 = 4; + +fn pre_vote_config() -> Result> { + Ok(Arc::new( + Config { + heartbeat_interval: 50, + election_timeout_min: 500, + election_timeout_max: 600, + enable_pre_vote: Some(true), + ..Default::default() + } + .validate()?, + )) +} + +/// How the isolated voter returns to the cluster. +#[derive(Clone, Copy, Debug)] +enum Rejoin { + /// Its consensus traffic is admitted again. + Reconnect, + /// It is restarted from its own durable state while still cut off, then admitted again. + Restart, +} + +/// With Pre-Vote, a voter cut off from consensus traffic for several election timeouts does not +/// raise its term, so it rejoins as a follower without deposing the healthy leader. +/// +/// Without Pre-Vote the isolated voter campaigns on every election timeout and persists each new +/// term. Once it is reachable again, the leader sees that higher term in an AppendEntries response +/// and steps down, although a quorum was serving it throughout. +async fn isolated_voter_rejoins_without_deposing_the_leader(voter_count: u64, rejoin: Rejoin) -> Result<()> { + let config = pre_vote_config()?; + let isolation = Duration::from_millis(config.election_timeout_max * ISOLATION_ELECTION_TIMEOUTS); + + let mut router = RaftRouter::new(config.clone()); + + let voters = (0..voter_count).collect::>(); + tracing::info!("--- create cluster of {:?}; node 0 becomes leader", voters); + let mut log_index = router.new_cluster(voters.clone(), btreeset! {}).await?; + + let n0 = router.get_raft_handle(&0)?; + n0.wait(timeout()).state(ServerState::Leader, "node 0 is the initial leader").await?; + for id in voters.iter() { + router + .wait(id, timeout()) + .metrics(|m| m.current_leader == Some(0), "every voter follows node 0") + .await?; + } + let leader_term = n0.metrics().borrow().current_term; + + tracing::info!("--- cut node 1 off from consensus traffic for {:?}", isolation); + router.set_network_error(1, true); + let isolated_at = Instant::now(); + let mut highest_isolated_term = router.get_raft_handle(&1)?.metrics().borrow().current_term; + while isolated_at.elapsed() < isolation { + // The other voters keep committing while node 1 is cut off. + router.client_request_many(0, "isolated", 1).await?; + log_index += 1; + let term = router.get_raft_handle(&1)?.metrics().borrow().current_term; + highest_isolated_term = highest_isolated_term.max(term); + sleep(Duration::from_millis(50)).await; + } + + if let Rejoin::Restart = rejoin { + tracing::info!("--- restart node 1 from its durable state while it is still cut off"); + let (n1, log_store, sm) = router.remove_node(1).unwrap(); + n1.shutdown().await?; + router.new_raft_node_with_sto(1, log_store, sm).await; + router + .wait(&1, timeout()) + .metrics( + |m| m.current_term >= leader_term, + "node 1 restarted with its durable vote", + ) + .await?; + let term = router.get_raft_handle(&1)?.metrics().borrow().current_term; + highest_isolated_term = highest_isolated_term.max(term); + } + + tracing::info!("--- admit node 1 again and watch the cluster for two more election timeouts"); + router.set_network_error(1, false); + let watched_at = Instant::now(); + while watched_at.elapsed() < Duration::from_millis(config.election_timeout_max * 2) { + for id in voters.iter() { + let m = router.get_raft_handle(id)?.metrics().borrow().clone(); + assert_eq!( + leader_term, m.current_term, + "node {} stays in the healthy leader's term after node 1 rejoins ({:?})", + id, rejoin + ); + } + sleep(Duration::from_millis(20)).await; + } + + assert_eq!( + leader_term, highest_isolated_term, + "the cut-off voter never raised its term ({:?})", + rejoin + ); + router.wait(&1, timeout()).applied_index(Some(log_index), "node 1 catches up").await?; + let m = n0.metrics().borrow().clone(); + assert_eq!(ServerState::Leader, m.state, "node 0 is still the leader"); + assert_eq!(Some(0), m.current_leader); + for id in voters.iter() { + router + .wait(id, timeout()) + .metrics(|m| m.current_leader == Some(0), "every voter still follows node 0") + .await?; + } + + tracing::info!("--- the leader keeps serving writes with node 1"); + router.client_request_many(0, "rejoined", 1).await?; + log_index += 1; + router.wait(&1, timeout()).applied_index(Some(log_index), "node 1 replicates again").await?; + + Ok(()) +} + +#[async_entry::test(worker_threads = 8, init = "init_default_ut_tracing()", tracing_span = "debug")] +async fn three_voters_isolated_voter_reconnects_without_deposing_the_leader() -> Result<()> { + isolated_voter_rejoins_without_deposing_the_leader(3, Rejoin::Reconnect).await +} + +#[async_entry::test(worker_threads = 8, init = "init_default_ut_tracing()", tracing_span = "debug")] +async fn five_voters_isolated_voter_reconnects_without_deposing_the_leader() -> Result<()> { + isolated_voter_rejoins_without_deposing_the_leader(5, Rejoin::Reconnect).await +} + +#[async_entry::test(worker_threads = 8, init = "init_default_ut_tracing()", tracing_span = "debug")] +async fn three_voters_isolated_voter_restarts_without_deposing_the_leader() -> Result<()> { + isolated_voter_rejoins_without_deposing_the_leader(3, Rejoin::Restart).await +} + +#[async_entry::test(worker_threads = 8, init = "init_default_ut_tracing()", tracing_span = "debug")] +async fn five_voters_isolated_voter_restarts_without_deposing_the_leader() -> Result<()> { + isolated_voter_rejoins_without_deposing_the_leader(5, Rejoin::Restart).await +} + +/// Pre-Vote keeps elections possible: after the leader is lost, the survivors grant each other's +/// Pre-Vote once their leases expire, and one of them is elected. +#[async_entry::test(worker_threads = 8, init = "init_default_ut_tracing()", tracing_span = "debug")] +async fn pre_vote_elects_a_survivor_after_leader_loss() -> Result<()> { + let config = pre_vote_config()?; + + let mut router = RaftRouter::new(config.clone()); + + tracing::info!("--- create cluster of 0,1,2; node 0 becomes leader"); + router.new_cluster(btreeset! {0,1,2}, btreeset! {}).await?; + let n0 = router.get_raft_handle(&0)?; + n0.wait(timeout()).state(ServerState::Leader, "node 0 is the initial leader").await?; + + tracing::info!("--- lose the leader"); + router.set_network_error(0, true); + + for id in [1, 2] { + router + .wait(&id, Some(Duration::from_secs(5))) + .metrics( + |m| m.current_leader == Some(1) || m.current_leader == Some(2), + "a survivor is elected through Pre-Vote", + ) + .await?; + } + + Ok(()) +} + +/// An unreachable peer is not a Pre-Vote grant: a voter whose every peer is unreachable keeps its +/// term. +#[async_entry::test(worker_threads = 8, init = "init_default_ut_tracing()", tracing_span = "debug")] +async fn unreachable_peers_do_not_grant_pre_vote() -> Result<()> { + let config = pre_vote_config()?; + + let mut router = RaftRouter::new(config.clone()); + + tracing::info!("--- create cluster of 0,1,2; node 0 becomes leader"); + router.new_cluster(btreeset! {0,1,2}, btreeset! {}).await?; + let n1 = router.get_raft_handle(&1)?; + n1.wait(timeout()).state(ServerState::Follower, "node 1 is a follower").await?; + let term = n1.metrics().borrow().current_term; + + tracing::info!("--- make node 1 unreachable for several election timeouts"); + router.set_unreachable(1, true); + sleep(Duration::from_millis( + config.election_timeout_max * ISOLATION_ELECTION_TIMEOUTS, + )) + .await; + + assert_eq!( + term, + n1.metrics().borrow().current_term, + "an unreachable peer never counts toward a Pre-Vote quorum" + ); + + Ok(()) +} + +fn timeout() -> Option { + Some(Duration::from_millis(3_000)) +} From 2f9b863c54702b27e9225be2069471cb7f4659b4 Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Thu, 1 Oct 2026 21:43:22 -0600 Subject: [PATCH 07/36] Feature: add Pre-Vote to keep a cut-off voter from raising its term With Config::enable_pre_vote, a voter whose election timer fires first asks the other voters whether they would grant it a vote at term + 1. Answering persists nothing and changes no term, lease or server state; a voter answers by the leader-lease, last-log-id and vote rules of a real vote request. Only when a quorum would grant does the voter start the real election. A voter that is cut off, restarted, or behind on the log therefore keeps its term, and does not depose a healthy leader when it can be reached again. - RaftNetwork::pre_vote carries the round and Raft::pre_vote answers it. The default implementation reports a grant without contacting the target, so a network without Pre-Vote keeps the election behavior without it. An error, including an unreachable peer, is never a grant. - A grant from a quorum starts the real election. A rejection with a greater log delays the next attempt, and one with a strictly higher vote catches the voter up to it in non-committed form. - A real election, a won election, prepared shutdown and any vote this node accepts, including a heartbeat from the current leader, end an in-flight round. The tick-driven re-evaluation of the server state does not, because it accepts no vote. - A round is retried after one newly sampled election timeout. A single voter grants its own round and elects at once. - The design follows the upstream Openraft Pre-Vote, including its later fixes for adopting a higher vote and cancelling a round once its election is won, adapted to this branch. The cut-off voter cases from the previous commit now pass: the voter stays in term 1 and the leader keeps leading after a reconnect or a restart, with three and five voters, with and without single-term-leader, and with a 30 ms send delay. Signed-off-by: VerifiedOrganic --- openraft/src/config/config.rs | 9 + openraft/src/core/notify.rs | 20 + openraft/src/core/raft_core.rs | 150 +++++++- openraft/src/core/raft_msg/mod.rs | 9 + openraft/src/docs/protocol/mod.rs | 4 + openraft/src/docs/protocol/pre_vote.md | 69 ++++ openraft/src/engine/command.rs | 6 + openraft/src/engine/engine_impl.rs | 172 +++++++++ openraft/src/engine/engine_output.rs | 1 + .../src/engine/handler/vote_handler/mod.rs | 6 + openraft/src/engine/leadership_transfer.rs | 1 + openraft/src/engine/mod.rs | 1 + openraft/src/engine/tests/pre_vote_test.rs | 349 ++++++++++++++++++ openraft/src/network/network.rs | 21 ++ openraft/src/proposer/candidate.rs | 5 + openraft/src/raft/mod.rs | 13 + openraft/src/raft/runtime_config_handle.rs | 8 + tests/tests/fixtures/mod.rs | 21 ++ 18 files changed, 851 insertions(+), 14 deletions(-) create mode 100644 openraft/src/docs/protocol/pre_vote.md create mode 100644 openraft/src/engine/tests/pre_vote_test.rs diff --git a/openraft/src/config/config.rs b/openraft/src/config/config.rs index 9948a0846..9c062a13c 100644 --- a/openraft/src/config/config.rs +++ b/openraft/src/config/config.rs @@ -284,6 +284,7 @@ pub struct Config { pub(crate) struct RuntimeConfig { pub(crate) enable_heartbeat: AtomicBool, pub(crate) enable_elect: AtomicBool, + pub(crate) enable_pre_vote: AtomicBool, } impl RuntimeConfig { @@ -291,6 +292,7 @@ impl RuntimeConfig { Self { enable_heartbeat: AtomicBool::from(config.enable_heartbeat), enable_elect: AtomicBool::from(config.enable_elect), + enable_pre_vote: AtomicBool::from(config.get_enable_pre_vote()), } } } @@ -307,6 +309,13 @@ impl Config { RT::thread_rng().gen_range(self.election_timeout_min..self.election_timeout_max) } + /// Whether a voter runs a Pre-Vote round before it starts a real election. + /// + /// `None` is treated as disabled. + pub(crate) fn get_enable_pre_vote(&self) -> bool { + self.enable_pre_vote.unwrap_or(false) + } + /// Get the deadline of one AppendEntries RPC. /// /// It is [`append_entries_timeout`](Self::append_entries_timeout) when set, otherwise diff --git a/openraft/src/core/notify.rs b/openraft/src/core/notify.rs index 92efc7cae..b680074e7 100644 --- a/openraft/src/core/notify.rs +++ b/openraft/src/core/notify.rs @@ -20,6 +20,14 @@ where C: RaftTypeConfig sender_vote: Vote, }, + PreVoteResponse { + target: C::NodeId, + resp: VoteResponse, + + /// The hypothetical next-term vote of the Pre-Vote round that sent the request. + sender_vote: Vote, + }, + /// Seen a higher `vote`. HigherVote { /// The ID of the target node from which the new term was observed. @@ -73,6 +81,18 @@ where C: RaftTypeConfig } => { format!("VoteResponse: from: {}: {}, res-vote: {}", target, resp.summary(), vote) } + Self::PreVoteResponse { + target, + resp, + sender_vote: vote, + } => { + format!( + "PreVoteResponse: from: {}: {}, pre-vote: {}", + target, + resp.summary(), + vote + ) + } Self::HigherVote { ref target, higher: ref new_vote, diff --git a/openraft/src/core/raft_core.rs b/openraft/src/core/raft_core.rs index 9b52483c6..53dacd074 100644 --- a/openraft/src/core/raft_core.rs +++ b/openraft/src/core/raft_core.rs @@ -219,6 +219,22 @@ where pub(crate) _p: PhantomData, } +/// Whether [`RaftCore::spawn_parallel_vote_requests`] sends Vote or Pre-Vote requests. +#[derive(Clone, Copy, Debug)] +enum VoteRequestKind { + Vote, + PreVote, +} + +impl VoteRequestKind { + fn as_str(self) -> &'static str { + match self { + VoteRequestKind::Vote => "vote", + VoteRequestKind::PreVote => "pre-vote", + } + } +} + impl RaftCore where C: RaftTypeConfig, @@ -1173,9 +1189,12 @@ where Ok(at_most) } - /// Spawn parallel vote requests to all cluster members. + /// Spawn parallel Vote or Pre-Vote requests, selected by `kind`, to all other voters. + /// + /// Only a response counts toward a quorum. A transport error, including an unreachable peer, + /// is never a grant: otherwise a voter that is cut off would assemble a Pre-Vote quorum. #[tracing::instrument(level = "trace", skip_all, fields(vote=vote_req.summary()))] - async fn spawn_parallel_vote_requests(&mut self, vote_req: &VoteRequest) { + async fn spawn_parallel_vote_requests(&mut self, vote_req: &VoteRequest, kind: VoteRequestKind) { let members = self.engine.state.membership_state.effective().voter_ids(); let vote = vote_req.vote.clone(); @@ -1205,7 +1224,12 @@ where let vote = vote.clone(); async move { - let tm_res = C::AsyncRuntime::timeout(ttl, client.vote(req, option)).await; + let tm_res = match kind { + VoteRequestKind::Vote => C::AsyncRuntime::timeout(ttl, client.vote(req, option)).await, + VoteRequestKind::PreVote => { + C::AsyncRuntime::timeout(ttl, client.pre_vote(req, option)).await + } + }; let res = match tm_res { Ok(res) => res, @@ -1216,26 +1240,43 @@ where target: target.clone(), timeout: ttl, }; - tracing::error!({error = %timeout_err, target = display(&target)}, "timeout"); + tracing::error!( + {error = %timeout_err, target = display(&target)}, + "timeout while requesting {}", + kind.as_str() + ); return; } }; match res { Ok(resp) => { - let _ = tx.send(Notify::VoteResponse { - target, - resp, - sender_vote: vote, - }); + let notify = match kind { + VoteRequestKind::Vote => Notify::VoteResponse { + target, + resp, + sender_vote: vote, + }, + VoteRequestKind::PreVote => Notify::PreVoteResponse { + target, + resp, + sender_vote: vote, + }, + }; + let _ = tx.send(notify); } - Err(err) => tracing::error!({error=%err, target=display(&target)}, "while requesting vote"), + Err(err) => tracing::error!( + {error=%err, target=display(&target)}, + "while requesting {}", + kind.as_str() + ), } } } .instrument(tracing::debug_span!( parent: &Span::current(), "send_vote_req", + kind = kind.as_str(), target = display(&target) )), ); @@ -1253,6 +1294,18 @@ where }); } + #[tracing::instrument(level = "debug", skip_all)] + pub(super) fn handle_pre_vote_request(&mut self, req: VoteRequest, tx: VoteTx) { + tracing::info!(req = display(req.summary()), func = func_name!()); + + // A Pre-Vote persists nothing, so there is no IO to wait for. + let resp = self.engine.handle_pre_vote_req(req); + self.engine.output.push_command(Command::Respond { + when: None, + resp: Respond::new(Ok(resp), tx), + }); + } + #[tracing::instrument(level = "debug", skip_all)] pub(super) fn handle_append_entries_request(&mut self, req: AppendEntriesRequest, tx: AppendEntriesTx) { tracing::debug!(req = display(req.summary()), func = func_name!()); @@ -1284,6 +1337,11 @@ where self.handle_vote_request(rpc, tx); } + RaftMsg::RequestPreVote { rpc, tx } => { + tracing::info!(pre_vote_request = display(&rpc), "received RaftMsg::RequestPreVote"); + + self.handle_pre_vote_request(rpc, tx); + } RaftMsg::BeginReceivingSnapshot { tx } => { self.engine.handle_begin_receiving_snapshot(tx); } @@ -1391,6 +1449,23 @@ where } } + Notify::PreVoteResponse { + target, + resp, + sender_vote, + } => { + tracing::info!( + resp = display(resp.summary()), + target = display(&target), + "received Notify::PreVoteResponse: {}", + func_name!() + ); + + if self.does_pre_vote_match(&sender_vote) { + self.engine.handle_pre_vote_resp(target, resp); + } + } + Notify::HigherVote { target, higher, @@ -1620,7 +1695,9 @@ where return; } - if self.engine.state.membership_state.effective().voter_ids().count() == 1 { + let voter_count = self.engine.state.membership_state.effective().voter_ids().count(); + + if voter_count == 1 { if self.engine.candidate_ref().is_some() { tracing::debug!("skip election, single voter already has an active election in progress"); return; @@ -1658,11 +1735,36 @@ where tracing::info!("election timeout passed, check if it is a voter for election"); } + // Pre-Vote applies to multiple voters only: a single voter always wins its own Pre-Vote. + let pre_vote = self.runtime_config.enable_pre_vote.load(Ordering::Relaxed) && voter_count > 1; + + if pre_vote { + // A Pre-Vote round does not update the vote, so the expired election timer would + // restart it on every tick. Keep a round in flight for one sampled election timeout. + let timer_config = &self.engine.config.timer_config; + let mut round_timeout = timer_config.election_timeout; + if self.engine.is_there_greater_log() { + round_timeout += timer_config.smaller_log_timeout; + } + + if let Some(started) = self.engine.pre_candidate_ref().map(|x| x.starting_time()) { + if now < started + round_timeout { + tracing::debug!("a Pre-Vote round is already in flight"); + return; + } + } + } + // Every time elect, reset this flag. self.engine.reset_greater_log(); - tracing::info!("do trigger election"); - self.engine.elect(); + if pre_vote { + tracing::info!("do trigger Pre-Vote"); + self.engine.pre_elect(); + } else { + tracing::info!("do trigger election"); + self.engine.elect(); + } } #[tracing::instrument(level = "debug", skip_all)] @@ -1719,6 +1821,23 @@ where true } } + /// A Pre-Vote response belongs to the in-flight Pre-Vote round only if it answers that round's + /// vote; otherwise it is stale and ignored. + fn does_pre_vote_match(&self, sender_vote: &Vote) -> bool { + let my_vote = self.engine.pre_candidate_ref().map(|x| x.vote_ref()); + + if Some(sender_vote) != my_vote { + tracing::warn!( + "A Pre-Vote response will be ignored because the round changed: sent by: {}; current round: {}", + sender_vote, + my_vote.display(), + ); + false + } else { + true + } + } + /// If a message is sent by a previous replication session but is received by current server /// state, it is a stale message and should be just ignored. fn does_replication_session_match( @@ -1866,7 +1985,10 @@ where } } Command::SendVote { vote_req } => { - self.spawn_parallel_vote_requests(&vote_req).await; + self.spawn_parallel_vote_requests(&vote_req, VoteRequestKind::Vote).await; + } + Command::SendPreVote { vote_req } => { + self.spawn_parallel_vote_requests(&vote_req, VoteRequestKind::PreVote).await; } Command::ReplicateCommitted { committed } => { for node in self.replications.values() { diff --git a/openraft/src/core/raft_msg/mod.rs b/openraft/src/core/raft_msg/mod.rs index 69f17d0eb..ff4a629bf 100644 --- a/openraft/src/core/raft_msg/mod.rs +++ b/openraft/src/core/raft_msg/mod.rs @@ -55,6 +55,12 @@ where C: RaftTypeConfig tx: VoteTx, }, + /// Ask whether this node would grant a vote, without changing any state. + RequestPreVote { + rpc: VoteRequest, + tx: VoteTx, + }, + InstallFullSnapshot { vote: Vote, snapshot: Snapshot, @@ -130,6 +136,9 @@ where C: RaftTypeConfig RaftMsg::RequestVote { rpc, .. } => { format!("RequestVote: {}", rpc.summary()) } + RaftMsg::RequestPreVote { rpc, .. } => { + format!("RequestPreVote: {}", rpc.summary()) + } RaftMsg::BeginReceivingSnapshot { .. } => "BeginReceivingSnapshot".to_string(), RaftMsg::InstallFullSnapshot { vote, snapshot, .. } => { format!("InstallFullSnapshot: vote: {}, snapshot: {}", vote, snapshot) diff --git a/openraft/src/docs/protocol/mod.rs b/openraft/src/docs/protocol/mod.rs index 9b88cb02e..8e5f2d4a7 100644 --- a/openraft/src/docs/protocol/mod.rs +++ b/openraft/src/docs/protocol/mod.rs @@ -1,5 +1,9 @@ //! The protocol used by Openraft to replicate data. +pub mod pre_vote { + #![doc = include_str!("pre_vote.md")] +} + pub mod read { #![doc = include_str!("read.md")] } diff --git a/openraft/src/docs/protocol/pre_vote.md b/openraft/src/docs/protocol/pre_vote.md new file mode 100644 index 000000000..e60a9a5ad --- /dev/null +++ b/openraft/src/docs/protocol/pre_vote.md @@ -0,0 +1,69 @@ +# Pre-Vote + +Pre-Vote is an optional round that a voter runs **before** it increments its +term and starts a real election. It asks whether a quorum *would* grant it a +vote, without changing any state. Only if a quorum would grant does the voter +start the real election. + +It is disabled by default and enabled with +[`Config::enable_pre_vote`](`crate::Config::enable_pre_vote`). + + +## The problem it solves + +A voter that cannot currently win an election, because it is cut off from the +other voters, was restarted, or is behind on the log, still increments its term +every time its election timer fires. When it can be reached again, that higher +term makes the healthy leader step down: the leader sees the higher term in an +AppendEntries response from the returning voter. The cluster then holds an +election it did not need. + +The [leader lease](`crate::docs::protocol::replication::leader_lease`) already +keeps such a voter from *winning*: the other voters reject its vote requests +while their lease from the current leader is valid. Pre-Vote keeps its term +from climbing at all, so there is no higher term to depose the leader with. + + +## How it works + +When the election timer fires and Pre-Vote is enabled, a voter of a cluster +with more than one voter sends a Pre-Vote request to the other voters: *would +you grant me a vote at `term + 1`?* The request carries that hypothetical vote +and the voter's last log id. + +A voter answers with the rules it uses for a real vote request: + +- If it holds a committed vote whose + [leader lease](`crate::docs::protocol::replication::leader_lease`) has not + expired, it would not grant: a leader is serving it. +- If the requester's last log id is behind its own, it would not grant. +- If the requested vote is not greater than or equal to its own vote, it would + not grant. +- Otherwise it would grant. + +Unlike a real vote, a Pre-Vote request **persists nothing and changes +nothing**: no new term, no saved vote and no renewed lease. The requester does +not change its own state either; it stays a follower in its term until a quorum +would grant. + +If a quorum would grant, the voter starts the real election, increments its +term and persists its vote as usual. A response with a strictly higher vote +catches the requester up to that vote in non-committed form, as a rejected real +vote does. A rejection that reports a greater log delays the next attempt. Any +vote the requester accepts afterwards, including a heartbeat from the current +leader, ends the round. Otherwise the round is retried after one newly sampled +election timeout. + +Only a response counts toward the quorum. An error, including an unreachable +peer, is never a grant, so a voter that is cut off cannot assemble one. + +A voter that is the only voter wins its own Pre-Vote and elects at once. + + +## Networks without Pre-Vote + +Pre-Vote uses the separate +[`RaftNetwork::pre_vote`](`crate::network::RaftNetwork::pre_vote`) RPC. Its +default implementation reports a granted Pre-Vote without contacting the +target, so a network that does not implement it keeps the election behavior +without Pre-Vote. diff --git a/openraft/src/engine/command.rs b/openraft/src/engine/command.rs index 887ebd60a..f887b46ed 100644 --- a/openraft/src/engine/command.rs +++ b/openraft/src/engine/command.rs @@ -91,6 +91,9 @@ where C: RaftTypeConfig /// Send vote to all other members SendVote { vote_req: VoteRequest }, + /// Send a Pre-Vote request to all other voters. + SendPreVote { vote_req: VoteRequest }, + /// Purge log from the beginning to `upto`, inclusive. PurgeLog { upto: LogId }, @@ -138,6 +141,7 @@ where (Command::RebuildReplicationStreams { targets }, Command::RebuildReplicationStreams { targets: b }, ) => targets == b, (Command::SaveVote { vote }, Command::SaveVote { vote: b }) => vote == b, (Command::SendVote { vote_req }, Command::SendVote { vote_req: b }, ) => vote_req == b, + (Command::SendPreVote { vote_req }, Command::SendPreVote { vote_req: b }, ) => vote_req == b, (Command::PurgeLog { upto }, Command::PurgeLog { upto: b }) => upto == b, (Command::DeleteConflictLog { since }, Command::DeleteConflictLog { since: b }, ) => since == b, (Command::Respond { when, resp: send }, Command::Respond { when: b_when, resp: b }) => send == b && when == b_when, @@ -167,6 +171,7 @@ where C: RaftTypeConfig Command::ReplicateCommitted { .. } => CommandKind::Network, Command::Replicate { .. } => CommandKind::Network, Command::SendVote { .. } => CommandKind::Network, + Command::SendPreVote { .. } => CommandKind::Network, Command::StateMachine { .. } => CommandKind::StateMachine, // Apply is firstly handled by RaftCore, then forwarded to state machine worker. @@ -190,6 +195,7 @@ where C: RaftTypeConfig Command::RebuildReplicationStreams { .. } => None, Command::SaveVote { .. } => None, Command::SendVote { .. } => None, + Command::SendPreVote { .. } => None, Command::PurgeLog { .. } => None, Command::DeleteConflictLog { .. } => None, Command::Respond { when, .. } => when.as_ref(), diff --git a/openraft/src/engine/engine_impl.rs b/openraft/src/engine/engine_impl.rs index 16f8cc006..a8d5c2924 100644 --- a/openraft/src/engine/engine_impl.rs +++ b/openraft/src/engine/engine_impl.rs @@ -91,6 +91,12 @@ where C: RaftTypeConfig /// without losing leadership status. pub(crate) candidate: CandidateState, + /// The quorum tracker of an in-flight Pre-Vote round. + /// + /// Its vote is the hypothetical next-term vote this node would campaign with. It is never + /// persisted, and it is dropped once a real election starts or this node follows another vote. + pub(crate) pre_candidate: CandidateState, + /// Retirement is irreversible for this engine instance, including after a new vote. pub(crate) prepared_shutdown: Option>, @@ -111,6 +117,7 @@ where C: RaftTypeConfig seen_greater_log: false, leader: None, candidate: None, + pre_candidate: None, prepared_shutdown: None, output: EngineOutput::new(4096), } @@ -137,6 +144,27 @@ where C: RaftTypeConfig self.candidate.as_mut().unwrap() } + /// Create a new Pre-Vote round and return the mutable reference to its quorum tracker. + /// + /// Like [`new_candidate`](Self::new_candidate), but `vote` is only the hypothetical next-term + /// vote of the round and is never persisted. + pub(crate) fn new_pre_candidate(&mut self, vote: Vote) -> &mut Candidate> { + let now = C::now(); + let last_log_id = self.state.last_log_id().cloned(); + + let membership = self.state.membership_state.effective().membership(); + + self.pre_candidate = Some(Candidate::new( + now, + vote, + last_log_id, + membership.to_quorum_set(), + membership.learner_ids(), + )); + + self.pre_candidate.as_mut().unwrap() + } + /// Create a default Engine for testing. #[allow(dead_code)] pub(crate) fn testing_default(id: C::NodeId) -> Self { @@ -221,6 +249,9 @@ where C: RaftTypeConfig /// Start to elect this node as leader #[tracing::instrument(level = "debug", skip(self))] pub(crate) fn elect(&mut self) { + // A real election supersedes any in-flight Pre-Vote round. + self.pre_candidate = None; + if self.prepared_shutdown.is_some() { return; } @@ -256,10 +287,51 @@ where C: RaftTypeConfig self.server_state_handler().update_server_state_if_changed(); } + /// Start a Pre-Vote round. + /// + /// Ask the voters whether a quorum *would* grant this node a vote at `term + 1`, without + /// changing local state: no term bump, no persisted vote and no server-state change. When a + /// quorum would grant, [`handle_pre_vote_resp`](Self::handle_pre_vote_resp) starts the real + /// [`elect`](Self::elect). + #[tracing::instrument(level = "debug", skip(self))] + pub(crate) fn pre_elect(&mut self) { + if self.prepared_shutdown.is_some() || self.leader.is_some() { + return; + } + + // A Pre-Vote does not advance the persisted vote timestamp, so a new round is gated by a + // newly sampled timeout, as a real campaign is. + self.config.resample_election_timeout::(); + + let new_term = self.state.vote.leader_id().term + 1; + let pre_vote = Vote::new(new_term, self.config.id.clone()); + + let pre_candidate = self.new_pre_candidate(pre_vote.clone()); + tracing::info!("{}, new pre-candidate: {}", func_name!(), pre_candidate); + + let last_log_id = pre_candidate.last_log_id().cloned(); + + // A voter grants its own Pre-Vote. A single voter reaches a quorum at once and starts the + // real election without a round trip. + let id = self.config.id.clone(); + if self.pre_candidate.as_mut().unwrap().grant_by(&id) { + self.elect(); + return; + } + + self.output.push_command(Command::SendPreVote { + vote_req: VoteRequest::new(pre_vote, last_log_id), + }); + } + pub(crate) fn candidate_ref(&self) -> Option<&Candidate>> { self.candidate.as_ref() } + pub(crate) fn pre_candidate_ref(&self) -> Option<&Candidate>> { + self.pre_candidate.as_ref() + } + pub(crate) fn candidate_mut(&mut self) -> Option<&mut Candidate>> { self.candidate.as_mut() } @@ -361,6 +433,102 @@ where C: RaftTypeConfig VoteResponse::new(self.state.vote_ref(), self.state.last_log_id().cloned(), res.is_ok()) } + /// Handle a Pre-Vote request. + /// + /// Report whether this node *would* grant a vote for `req.vote`, judged by the same + /// leader-lease and last-log-id rules as [`handle_vote_req`](Self::handle_vote_req), but + /// without persisting any vote, changing the term or emitting any command. + #[tracing::instrument(level = "debug", skip_all)] + pub(crate) fn handle_pre_vote_req(&mut self, req: VoteRequest) -> VoteResponse { + let now = C::now(); + let lease = self.config.timer_config.leader_lease; + let vote = self.state.vote_ref(); + + // Make default vote-last-modified a low enough value, that expires leader lease. + let vote_utime = self.state.vote_last_modified().unwrap_or_else(|| now - lease - Duration::from_millis(1)); + + tracing::info!( + req = display(req.summary()), + my_vote = display(vote.summary()), + my_last_log_id = display(self.state.last_log_id().summary()), + "Engine::handle_pre_vote_req" + ); + + // While a leader's lease is valid this node would not grant a vote. + if vote.is_committed() && !self.state.vote.lease_disabled() && now <= vote_utime + lease { + tracing::info!("reject pre-vote-request: leader lease has not yet expired"); + return VoteResponse::new(self.state.vote_ref(), self.state.last_log_id().cloned(), false); + } + + if req.last_log_id.as_ref() >= self.state.last_log_id() { + // Ok + } else { + tracing::info!( + "reject pre-vote-request: by last_log_id: !(req.last_log_id({}) >= my_last_log_id({})", + req.last_log_id.summary(), + self.state.last_log_id().summary(), + ); + return VoteResponse::new(self.state.vote_ref(), self.state.last_log_id().cloned(), false); + } + + // Compared exactly as a real vote is accepted, but nothing is persisted or changed. + let granted = &req.vote >= self.state.vote_ref(); + VoteResponse::new(self.state.vote_ref(), self.state.last_log_id().cloned(), granted) + } + + /// Handle a Pre-Vote response. + /// + /// Count a granted Pre-Vote; once a quorum would grant, start the real election. A rejection + /// that reports a greater log delays the next campaign, and one that reports a strictly higher + /// vote catches this node up to that vote in non-committed form, as a rejected real vote does. + #[tracing::instrument(level = "debug", skip(self, resp))] + pub(crate) fn handle_pre_vote_resp(&mut self, target: C::NodeId, resp: VoteResponse) { + tracing::info!( + resp = display(resp.summary()), + target = display(&target), + my_vote = display(self.state.vote_ref()), + my_last_log_id = display(self.state.last_log_id().summary()), + "{}", + func_name!() + ); + + let Some(pre_candidate) = self.pre_candidate.as_mut() else { + // The Pre-Vote round has finished or been canceled; ignore the delayed response. + return; + }; + + // The responder does not adopt the Pre-Vote, so a grant is read from `vote_granted`. + if resp.vote_granted { + if pre_candidate.grant_by(&target) { + self.pre_candidate = None; + if self.leader.is_some() || !self.state.membership_state.effective().is_voter(&self.config.id) { + tracing::info!("a quorum would grant the vote, but this node leads or no longer votes"); + return; + } + tracing::info!("a quorum would grant the vote; start a real election"); + self.elect(); + } + return; + } + + if resp.last_log_id.as_ref() > self.state.last_log_id() { + tracing::info!( + greater_log_id = display(resp.last_log_id.summary()), + "seen a greater log id when {}", + func_name!() + ); + self.set_greater_log(); + } + + // Catch up to a strictly higher vote, never as committed. An equal vote is left alone: it + // would end the in-flight round without new information. + if &resp.vote > self.state.vote_ref() { + let mut vote = resp.vote.clone(); + vote.committed = false; + let _ = self.vote_handler().update_vote(&vote); + } + } + #[tracing::instrument(level = "debug", skip(self, resp))] pub(crate) fn handle_vote_resp(&mut self, target: C::NodeId, resp: VoteResponse) { tracing::info!( @@ -667,6 +835,9 @@ where C: RaftTypeConfig fn establish_leader(&mut self) { tracing::info!("{}", func_name!()); + // An overlapping Pre-Vote round must not start another campaign once this node leads. + self.pre_candidate = None; + let candidate = self.candidate.take().unwrap(); let leader = self.establish_handler().establish(candidate); @@ -757,6 +928,7 @@ where C: RaftTypeConfig output: &mut self.output, leader: &mut self.leader, candidate: &mut self.candidate, + pre_candidate: &mut self.pre_candidate, } } diff --git a/openraft/src/engine/engine_output.rs b/openraft/src/engine/engine_output.rs index b26e5f530..cea82cea3 100644 --- a/openraft/src/engine/engine_output.rs +++ b/openraft/src/engine/engine_output.rs @@ -56,6 +56,7 @@ where C: RaftTypeConfig Command::RebuildReplicationStreams { .. } => {} Command::SaveVote { .. } => {} Command::SendVote { .. } => {} + Command::SendPreVote { .. } => {} Command::PurgeLog { .. } => {} Command::DeleteConflictLog { .. } => {} Command::Respond { .. } => {} diff --git a/openraft/src/engine/handler/vote_handler/mod.rs b/openraft/src/engine/handler/vote_handler/mod.rs index fbb9a1d81..8a93acadd 100644 --- a/openraft/src/engine/handler/vote_handler/mod.rs +++ b/openraft/src/engine/handler/vote_handler/mod.rs @@ -40,6 +40,7 @@ where C: RaftTypeConfig pub(crate) output: &'st mut EngineOutput, pub(crate) leader: &'st mut LeaderState, pub(crate) candidate: &'st mut CandidateState, + pub(crate) pre_candidate: &'st mut CandidateState, } impl VoteHandler<'_, C> @@ -118,6 +119,11 @@ where C: RaftTypeConfig tracing::debug!(now = debug(C::now()), "{}", func_name!()); + // An accepted vote, including a heartbeat from the current leader, ends an in-flight + // Pre-Vote round: its outcome no longer reflects this node's view of the cluster. The + // periodic re-evaluation of the server state does not, because it changes no vote. + *self.pre_candidate = None; + self.update_internal_server_state(); Ok(()) diff --git a/openraft/src/engine/leadership_transfer.rs b/openraft/src/engine/leadership_transfer.rs index a9fc5f0a9..76199d089 100644 --- a/openraft/src/engine/leadership_transfer.rs +++ b/openraft/src/engine/leadership_transfer.rs @@ -35,6 +35,7 @@ impl Engine { None }; self.candidate = None; + self.pre_candidate = None; self.prepared_shutdown = Some(PreparedShutdown { handoff: handoff.clone(), }); diff --git a/openraft/src/engine/mod.rs b/openraft/src/engine/mod.rs index 82c75b681..d3cf8f70c 100644 --- a/openraft/src/engine/mod.rs +++ b/openraft/src/engine/mod.rs @@ -48,6 +48,7 @@ mod tests { mod install_full_snapshot_test; mod leadership_transfer_test; mod log_id_list_test; + mod pre_vote_test; mod startup_test; mod trigger_purge_log_test; } diff --git a/openraft/src/engine/tests/pre_vote_test.rs b/openraft/src/engine/tests/pre_vote_test.rs new file mode 100644 index 000000000..3afab5cb8 --- /dev/null +++ b/openraft/src/engine/tests/pre_vote_test.rs @@ -0,0 +1,349 @@ +use std::collections::BTreeSet; +use std::sync::Arc; +use std::time::Duration; + +use maplit::btreeset; +use pretty_assertions::assert_eq; + +use crate::core::ServerState; +use crate::engine::testing::UTConfig; +use crate::engine::Command; +use crate::engine::Engine; +use crate::engine::LogIdList; +use crate::raft::VoteRequest; +use crate::raft::VoteResponse; +use crate::testing::log_id; +use crate::utime::UTime; +use crate::EffectiveMembership; +use crate::Membership; +use crate::TokioInstant; +use crate::Vote; + +fn m1() -> Membership { + Membership::::new(vec![btreeset! {1}], None) +} + +fn m12() -> Membership { + Membership::::new(vec![btreeset! {1,2}], None) +} + +fn m123() -> Membership { + Membership::::new(vec![btreeset! {1,2,3}], None) +} + +/// A follower of node 2 at term 1, whose leader lease has expired. +fn eng(membership: Membership) -> Engine { + let mut eng = Engine::default(); + eng.state.enable_validation(false); // Disable validation for incomplete state + + eng.config.id = 1; + eng.state.vote = UTime::new(TokioInstant::now() - Duration::from_secs(1), Vote::new_committed(1, 2)); + eng.state.server_state = ServerState::Follower; + eng.state.log_ids = LogIdList::new(vec![log_id(1, 2, 3)]); + eng.state + .membership_state + .set_effective(Arc::new(EffectiveMembership::new(Some(log_id(1, 2, 1)), membership))); + eng.output.take_commands(); + + eng +} + +#[test] +fn test_pre_elect_changes_no_state() -> anyhow::Result<()> { + let mut eng = eng(m123()); + let vote_before = *eng.state.vote_ref(); + + eng.pre_elect(); + + // No term bump, no persisted vote and no server-state change. + assert_eq!(vote_before, *eng.state.vote_ref()); + assert_eq!(ServerState::Follower, eng.state.server_state); + assert!( + eng.candidate_ref().is_none(), + "no real campaign during a Pre-Vote round" + ); + + // A Pre-Vote round at the hypothetical next term, granted by this node itself. + let pre_candidate = eng.pre_candidate_ref().unwrap(); + assert_eq!(Vote::new(2, 1), *pre_candidate.vote_ref()); + assert_eq!(btreeset! {1}, pre_candidate.granters().collect::>()); + + // Only SendPreVote is emitted: no SaveVote and no SendVote. + assert_eq!( + vec![Command::SendPreVote { + vote_req: VoteRequest::new(Vote::new(2, 1), Some(log_id(1, 2, 3))) + }], + eng.output.take_commands() + ); + + Ok(()) +} + +#[test] +fn test_pre_elect_single_voter_starts_real_election() -> anyhow::Result<()> { + let mut eng = eng(m1()); + + eng.pre_elect(); + + // A single voter grants its own Pre-Vote and campaigns at once. + assert_eq!(Vote::new(2, 1), *eng.state.vote_ref()); + assert!( + eng.pre_candidate_ref().is_none(), + "the real election consumes the round" + ); + assert!(eng.candidate_ref().is_some()); + assert_eq!(ServerState::Candidate, eng.state.server_state); + + let commands = eng.output.take_commands(); + assert!(commands.contains(&Command::SaveVote { vote: Vote::new(2, 1) })); + assert!(commands.contains(&Command::SendVote { + vote_req: VoteRequest::new(Vote::new(2, 1), Some(log_id(1, 2, 3))), + })); + assert!(!commands.iter().any(|c| matches!(c, Command::SendPreVote { .. }))); + + Ok(()) +} + +#[test] +fn test_handle_pre_vote_req_rejected_by_leader_lease() -> anyhow::Result<()> { + let mut eng = eng(m123()); + eng.state.vote.update(TokioInstant::now(), Vote::new_committed(1, 2)); + let vote_before = *eng.state.vote_ref(); + + let resp = eng.handle_pre_vote_req(VoteRequest::new(Vote::new(2, 3), Some(log_id(1, 2, 3)))); + + assert_eq!( + VoteResponse::new(Vote::new_committed(1, 2), Some(log_id(1, 2, 3)), false), + resp + ); + assert_eq!(vote_before, *eng.state.vote_ref()); + assert_eq!(ServerState::Follower, eng.state.server_state); + assert_eq!(0, eng.output.take_commands().len()); + + Ok(()) +} + +#[test] +fn test_handle_pre_vote_req_rejected_by_smaller_last_log_id() -> anyhow::Result<()> { + let mut eng = eng(m123()); + let vote_before = *eng.state.vote_ref(); + + let resp = eng.handle_pre_vote_req(VoteRequest::new(Vote::new(2, 3), Some(log_id(1, 2, 2)))); + + assert_eq!( + VoteResponse::new(Vote::new_committed(1, 2), Some(log_id(1, 2, 3)), false), + resp + ); + assert_eq!(vote_before, *eng.state.vote_ref()); + assert_eq!(0, eng.output.take_commands().len()); + + Ok(()) +} + +#[test] +fn test_handle_pre_vote_req_granted_without_mutation() -> anyhow::Result<()> { + let mut eng = eng(m123()); + let vote_before = *eng.state.vote_ref(); + let utime_before = eng.state.vote_last_modified(); + + let resp = eng.handle_pre_vote_req(VoteRequest::new(Vote::new(2, 3), Some(log_id(1, 2, 3)))); + + assert_eq!( + VoteResponse::new(Vote::new_committed(1, 2), Some(log_id(1, 2, 3)), true), + resp + ); + // Granting a Pre-Vote changes nothing locally, not even the vote timestamp. + assert_eq!(vote_before, *eng.state.vote_ref()); + assert_eq!(utime_before, eng.state.vote_last_modified()); + assert_eq!(ServerState::Follower, eng.state.server_state); + assert_eq!(0, eng.output.take_commands().len()); + + Ok(()) +} + +#[test] +fn test_handle_pre_vote_req_rejects_a_vote_not_greater_than_the_local_one() -> anyhow::Result<()> { + let mut eng = eng(m123()); + // This node already granted node 3 a vote in term 2. + eng.state.vote = UTime::new(TokioInstant::now(), Vote::new(2, 3)); + let vote_before = *eng.state.vote_ref(); + + let resp = eng.handle_pre_vote_req(VoteRequest::new(Vote::new(1, 2), Some(log_id(1, 2, 3)))); + + assert_eq!(VoteResponse::new(Vote::new(2, 3), Some(log_id(1, 2, 3)), false), resp); + assert_eq!(vote_before, *eng.state.vote_ref()); + assert_eq!(0, eng.output.take_commands().len()); + + Ok(()) +} + +#[test] +fn test_handle_pre_vote_resp_quorum_starts_real_election() -> anyhow::Result<()> { + let mut eng = eng(m123()); + eng.pre_elect(); + eng.output.take_commands(); + + eng.handle_pre_vote_resp( + 3, + VoteResponse::new(Vote::new_committed(1, 2), Some(log_id(1, 2, 3)), true), + ); + + assert!( + eng.pre_candidate_ref().is_none(), + "the real election consumes the round" + ); + assert_eq!( + Vote::new(2, 1), + *eng.state.vote_ref(), + "the real election bumps the term" + ); + assert!(eng.candidate_ref().is_some()); + assert_eq!(ServerState::Candidate, eng.state.server_state); + + let commands = eng.output.take_commands(); + assert!(commands.contains(&Command::SaveVote { vote: Vote::new(2, 1) })); + assert!(commands.contains(&Command::SendVote { + vote_req: VoteRequest::new(Vote::new(2, 1), Some(log_id(1, 2, 3))), + })); + + Ok(()) +} + +#[test] +fn test_handle_pre_vote_resp_rejection_keeps_waiting() -> anyhow::Result<()> { + let mut eng = eng(m123()); + eng.pre_elect(); + eng.output.take_commands(); + let vote_before = *eng.state.vote_ref(); + + eng.handle_pre_vote_resp( + 3, + VoteResponse::new(Vote::new_committed(1, 2), Some(log_id(1, 2, 3)), false), + ); + + assert!(eng.pre_candidate_ref().is_some(), "the round is still in flight"); + assert!(eng.candidate_ref().is_none(), "no real election starts"); + assert_eq!(vote_before, *eng.state.vote_ref()); + assert!(!eng.is_there_greater_log()); + assert_eq!(0, eng.output.take_commands().len()); + + tracing::info!("--- a rejection that reports a greater log delays the next campaign"); + eng.handle_pre_vote_resp( + 2, + VoteResponse::new(Vote::new_committed(1, 2), Some(log_id(1, 2, 5)), false), + ); + assert!(eng.is_there_greater_log()); + assert!(eng.pre_candidate_ref().is_some()); + assert_eq!(vote_before, *eng.state.vote_ref()); + + Ok(()) +} + +#[test] +fn test_handle_pre_vote_resp_rejection_adopts_a_higher_vote() -> anyhow::Result<()> { + let mut eng = eng(m123()); + eng.pre_elect(); + eng.output.take_commands(); + + eng.handle_pre_vote_resp( + 3, + VoteResponse::new(Vote::new_committed(5, 3), Some(log_id(1, 2, 3)), false), + ); + + // Catch up to the strictly higher term, never as committed, and end the round. + assert_eq!(Vote::new(5, 3), *eng.state.vote_ref()); + assert!(eng.pre_candidate_ref().is_none(), "adopting a vote ends the round"); + assert!(eng.candidate_ref().is_none()); + assert_eq!(ServerState::Follower, eng.state.server_state); + assert!(eng.output.take_commands().contains(&Command::SaveVote { vote: Vote::new(5, 3) })); + + Ok(()) +} + +#[test] +fn test_handle_pre_vote_resp_rejection_with_equal_vote_keeps_the_round() -> anyhow::Result<()> { + let mut eng = eng(m123()); + eng.pre_elect(); + eng.output.take_commands(); + let vote_before = *eng.state.vote_ref(); + + eng.handle_pre_vote_resp(3, VoteResponse::new(vote_before, Some(log_id(1, 2, 3)), false)); + + assert!( + eng.pre_candidate_ref().is_some(), + "an equal vote does not end the round" + ); + assert_eq!(vote_before, *eng.state.vote_ref()); + assert_eq!(0, eng.output.take_commands().len()); + + Ok(()) +} + +#[test] +fn test_accepted_vote_ends_the_pre_vote_round() -> anyhow::Result<()> { + let mut eng = eng(m123()); + eng.pre_elect(); + eng.output.take_commands(); + + // A heartbeat from the current leader touches the same vote and ends the round. + let leader_vote = *eng.state.vote_ref(); + eng.vote_handler().update_vote(&leader_vote)?; + + assert!(eng.pre_candidate_ref().is_none()); + assert_eq!(leader_vote, *eng.state.vote_ref()); + + // A delayed grant for the ended round starts nothing. + eng.handle_pre_vote_resp(3, VoteResponse::new(leader_vote, Some(log_id(1, 2, 3)), true)); + assert!(eng.candidate_ref().is_none()); + assert_eq!(leader_vote, *eng.state.vote_ref()); + assert_eq!(0, eng.output.take_commands().len()); + + Ok(()) +} + +#[test] +fn test_server_state_refresh_keeps_the_pre_vote_round() -> anyhow::Result<()> { + let mut eng = eng(m123()); + eng.pre_elect(); + eng.output.take_commands(); + + // The periodic re-evaluation of the server state changes no vote and keeps the round. + eng.vote_handler().update_internal_server_state(); + + assert!(eng.pre_candidate_ref().is_some()); + + Ok(()) +} + +#[test] +fn test_winning_election_ends_an_overlapping_pre_vote_round() -> anyhow::Result<()> { + let mut eng = eng(m12()); + + // Round A wins a Pre-Vote quorum and starts election A. + eng.pre_elect(); + eng.handle_pre_vote_resp( + 2, + VoteResponse::new(Vote::new_committed(1, 2), Some(log_id(1, 2, 3)), true), + ); + assert_eq!(Vote::new(2, 1), *eng.candidate_ref().unwrap().vote_ref()); + // The local vote is granted once it is persisted. + eng.candidate_mut().unwrap().grant_by(&1); + + // Election A is pending when the next election timer fires and starts round B. + eng.pre_elect(); + assert_eq!(Vote::new(3, 1), *eng.pre_candidate_ref().unwrap().vote_ref()); + + // Election A wins before round B is answered. + eng.handle_vote_resp(2, VoteResponse::new(Vote::new(2, 1), Some(log_id(1, 2, 3)), true)); + assert!(eng.leader.is_some(), "election A established a leader"); + assert!(eng.pre_candidate_ref().is_none(), "winning election A ends round B"); + eng.output.take_commands(); + + // A delayed grant for round B starts no campaign on the leader. + eng.handle_pre_vote_resp(2, VoteResponse::new(Vote::new(2, 1), Some(log_id(1, 2, 3)), true)); + assert_eq!(Vote::new_committed(2, 1), *eng.state.vote_ref()); + assert_eq!(ServerState::Leader, eng.state.server_state); + assert_eq!(0, eng.output.take_commands().len()); + + Ok(()) +} diff --git a/openraft/src/network/network.rs b/openraft/src/network/network.rs index 9adadc65d..d8cdd7bb9 100644 --- a/openraft/src/network/network.rs +++ b/openraft/src/network/network.rs @@ -79,6 +79,27 @@ where C: RaftTypeConfig option: RPCOption, ) -> Result, RPCError>>; + /// Send a Pre-Vote RPC to the target. + /// + /// The node that receives it passes it to [`Raft::pre_vote()`]. It asks whether the target + /// *would* grant a vote for `rpc.vote`, the sender's next term, without the target persisting + /// a vote or changing its term. It is only sent when + /// [`Config::enable_pre_vote`](`crate::Config::enable_pre_vote`) is enabled. + /// + /// The default implementation reports a granted Pre-Vote without contacting the target, so a + /// network that does not implement it makes Pre-Vote a no-op and elections proceed as without + /// it. An implementation that cannot reach the target must return an error instead: an error + /// is never counted as a grant, so a voter that is cut off cannot assemble a quorum. + /// + /// [`Raft::pre_vote()`]: crate::Raft::pre_vote + async fn pre_vote( + &mut self, + rpc: VoteRequest, + _option: RPCOption, + ) -> Result, RPCError>> { + Ok(VoteResponse::new(rpc.vote, None, true)) + } + /// Send a complete Snapshot to the target. /// /// This method is responsible to fragment the snapshot and send it to the target node. diff --git a/openraft/src/proposer/candidate.rs b/openraft/src/proposer/candidate.rs index e7ad11b8b..ed9c537f6 100644 --- a/openraft/src/proposer/candidate.rs +++ b/openraft/src/proposer/candidate.rs @@ -82,6 +82,11 @@ where self.last_log_id.as_ref() } + /// The time at which this voting round started. + pub(crate) fn starting_time(&self) -> InstantOf { + self.starting_time + } + pub(crate) fn progress(&self) -> &VecProgress { &self.progress } diff --git a/openraft/src/raft/mod.rs b/openraft/src/raft/mod.rs index a14d72997..7c2b918ff 100644 --- a/openraft/src/raft/mod.rs +++ b/openraft/src/raft/mod.rs @@ -423,6 +423,19 @@ where C: RaftTypeConfig self.inner.call_core(RaftMsg::RequestVote { rpc, tx }, rx).await } + /// Submit a Pre-Vote RPC to this Raft node. + /// + /// A voter sends it before it increments its term, to ask whether this node *would* grant a + /// vote for `rpc.vote`. This node judges it by the same leader-lease and last-log-id rules as + /// [`Raft::vote()`], but it never persists a vote or changes its term in response. + #[tracing::instrument(level = "debug", skip(self, rpc))] + pub async fn pre_vote(&self, rpc: VoteRequest) -> Result, RaftError> { + tracing::info!(rpc = display(rpc.summary()), "Raft::pre_vote()"); + + let (tx, rx) = C::AsyncRuntime::oneshot(); + self.inner.call_core(RaftMsg::RequestPreVote { rpc, tx }, rx).await + } + /// Stop admitting leader operations and issue an exact planned handoff. /// /// The application delivers the returned request through its authenticated diff --git a/openraft/src/raft/runtime_config_handle.rs b/openraft/src/raft/runtime_config_handle.rs index 4a29c78a9..e31984a30 100644 --- a/openraft/src/raft/runtime_config_handle.rs +++ b/openraft/src/raft/runtime_config_handle.rs @@ -42,4 +42,12 @@ where C: RaftTypeConfig pub fn elect(&self, enabled: bool) { self.raft_inner.runtime_config.enable_elect.store(enabled, Ordering::Relaxed); } + + /// Enable or disable the Pre-Vote round that precedes an election started by the election + /// timer. + /// + /// See [`Config::enable_pre_vote`](`crate::Config::enable_pre_vote`). + pub fn pre_vote(&self, enabled: bool) { + self.raft_inner.runtime_config.enable_pre_vote.store(enabled, Ordering::Relaxed); + } } diff --git a/tests/tests/fixtures/mod.rs b/tests/tests/fixtures/mod.rs index 0a7ba41bf..0957af93f 100644 --- a/tests/tests/fixtures/mod.rs +++ b/tests/tests/fixtures/mod.rs @@ -1164,6 +1164,27 @@ impl RaftNetwork for RaftRouterNetwork { Ok(resp) } + + /// Send a Pre-Vote RPC to the target Raft node. + async fn pre_vote( + &mut self, + rpc: VoteRequest, + _option: RPCOption, + ) -> Result, RPCError>> { + let from_id = rpc.vote.leader_id().voted_for().unwrap(); + + self.owner.count_rpc(RPCTypes::Vote); + self.owner.call_rpc_pre_hook(rpc.clone(), from_id, self.target)?; + self.owner.emit_rpc_error(from_id, self.target)?; + self.owner.rand_send_delay().await; + + let node = self.owner.get_raft_handle(&self.target)?; + + let resp = node.pre_vote(rpc).await; + let resp = resp.map_err(|e| RemoteError::new(self.target, e))?; + + Ok(resp) + } } pub enum ValueTest { From 3c607ac830011589e794142fbdf63590f6f3727b Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Thu, 1 Oct 2026 21:43:47 -0600 Subject: [PATCH 08/36] Test: keep a voter that cannot receive from deposing the leader A voter that can still send but no longer receives consensus traffic stops hearing heartbeats, so its lease expires and it runs Pre-Vote rounds that reach the other voters. The other follower rejects them under its own lease. The leader, however, never renews the lease on its own vote, which expired one minimum election timeout after it was elected, so it grants the Pre-Vote and then the real vote, and steps down although a quorum keeps acknowledging it. The new case blocks every RPC to one follower of a three-voter cluster with Pre-Vote enabled and expects the leader to keep leading in its term, the other follower to stay in that term, and the cut-off voter never to raise its term. On this revision the leader steps down after about 0.6 s, in every run, with and without single-term-leader. Signed-off-by: VerifiedOrganic --- tests/tests/elect/main.rs | 1 + tests/tests/elect/t22_one_way_isolation.rs | 90 ++++++++++++++++++++++ 2 files changed, 91 insertions(+) create mode 100644 tests/tests/elect/t22_one_way_isolation.rs diff --git a/tests/tests/elect/main.rs b/tests/tests/elect/main.rs index 0e2a236d0..a20c8ca1b 100644 --- a/tests/tests/elect/main.rs +++ b/tests/tests/elect/main.rs @@ -13,3 +13,4 @@ mod t11_elect_seize_leadership; mod t13_elect_while_leader; mod t20_lease_and_timeout_overlap; mod t21_pre_vote; +mod t22_one_way_isolation; diff --git a/tests/tests/elect/t22_one_way_isolation.rs b/tests/tests/elect/t22_one_way_isolation.rs new file mode 100644 index 000000000..7f6ed1677 --- /dev/null +++ b/tests/tests/elect/t22_one_way_isolation.rs @@ -0,0 +1,90 @@ +use std::sync::Arc; +use std::time::Duration; + +use anyhow::Result; +use maplit::btreeset; +use openraft::Config; +use openraft::ServerState; +use tokio::time::sleep; +use tokio::time::Instant; + +use crate::fixtures::init_default_ut_tracing; +use crate::fixtures::Direction; +use crate::fixtures::RPCErrorType; +use crate::fixtures::RaftRouter; + +/// A voter that can still send but no longer receives consensus traffic does not depose the +/// healthy leader through Pre-Vote. +/// +/// The cut-off voter's lease expires, so it runs Pre-Vote rounds that reach the other voters. The +/// other follower rejects them under its own lease. The leader never renews the lease on its own +/// vote, so it must reject them under its quorum-acknowledged lease instead: a quorum keeps +/// acknowledging its heartbeats. Otherwise the leader's grant completes a quorum, the cut-off +/// voter campaigns, and the leader grants that vote too and steps down. +#[async_entry::test(worker_threads = 8, init = "init_default_ut_tracing()", tracing_span = "debug")] +async fn voter_that_cannot_receive_does_not_depose_the_leader() -> Result<()> { + let config = Arc::new( + Config { + heartbeat_interval: 50, + election_timeout_min: 500, + election_timeout_max: 600, + enable_pre_vote: Some(true), + ..Default::default() + } + .validate()?, + ); + let watch = Duration::from_millis(config.election_timeout_max * 4); + + let mut router = RaftRouter::new(config.clone()); + + tracing::info!("--- create cluster of 0,1,2; node 0 becomes leader"); + router.new_cluster(btreeset! {0,1,2}, btreeset! {}).await?; + + let n0 = router.get_raft_handle(&0)?; + let n1 = router.get_raft_handle(&1)?; + let n2 = router.get_raft_handle(&2)?; + n0.wait(timeout()).state(ServerState::Leader, "node 0 is the initial leader").await?; + for id in [1, 2] { + router + .wait(&id, timeout()) + .metrics(|m| m.current_leader == Some(0), "the follower follows node 0") + .await?; + } + let leader_term = n0.metrics().borrow().current_term; + + tracing::info!("--- node 1 can still send, but receives nothing for {:?}", watch); + router.set_rpc_failure(1, Direction::NetRecv, Some(RPCErrorType::NetworkError)); + + let started = Instant::now(); + let mut highest_cut_off_term = leader_term; + while started.elapsed() < watch { + let m0 = n0.metrics().borrow().clone(); + assert_eq!(ServerState::Leader, m0.state, "node 0 keeps leading"); + assert_eq!(leader_term, m0.current_term, "node 0 stays in its term"); + assert_eq!( + leader_term, + n2.metrics().borrow().current_term, + "node 2 stays in the leader's term" + ); + highest_cut_off_term = highest_cut_off_term.max(n1.metrics().borrow().current_term); + sleep(Duration::from_millis(20)).await; + } + assert_eq!( + leader_term, highest_cut_off_term, + "the cut-off voter never raised its term" + ); + + tracing::info!("--- node 1 receives again and follows node 0"); + router.set_rpc_failure(1, Direction::NetRecv, None); + router + .wait(&1, timeout()) + .metrics(|m| m.current_leader == Some(0), "node 1 follows node 0 again") + .await?; + router.client_request_many(0, "after", 1).await?; + + Ok(()) +} + +fn timeout() -> Option { + Some(Duration::from_millis(3_000)) +} From 9516a424a52114b3156dd79a57c0cf93a4a46e9d Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Thu, 1 Oct 2026 21:49:02 -0600 Subject: [PATCH 09/36] Fix: reject candidates while a quorum acknowledges the leader A leader never renews the lease on its own vote: AppendEntries acknowledgements renew it only on followers. Its own lease therefore expired one minimum election timeout after it was elected, and it granted any up-to-date candidate's Vote and Pre-Vote although a quorum kept acknowledging it. A leader now rejects Vote and Pre-Vote requests while a quorum acknowledged it within the leader lease. A follower that acknowledged an AppendEntries sent at t received it no earlier than t and rejects other candidates until at least t plus the lease, so the leader only joins that quorum's decision. A planned leadership transfer releases the retiring vote's lease, and its successor is still granted. A voter also no longer starts a Pre-Vote round while its own leader lease is valid. This follows the upstream Openraft change that gates votes by the quorum-acknowledged lease and Pre-Vote by the local lease, adapted to this branch. The one-way case from the previous commit now passes: the leader keeps leading in its term while a follower that cannot receive runs Pre-Vote rounds, with and without single-term-leader and with a 30 ms send delay. The full integration suites pass in every tested feature mode. Signed-off-by: VerifiedOrganic --- openraft/src/docs/protocol/leader_lease.md | 4 +- openraft/src/docs/protocol/pre_vote.md | 9 +++ openraft/src/engine/engine_impl.rs | 34 +++++++++ .../src/engine/tests/handle_vote_req_test.rs | 69 +++++++++++++++++++ openraft/src/engine/tests/pre_vote_test.rs | 43 ++++++++++++ openraft/src/proposer/leader.rs | 11 +++ 6 files changed, 169 insertions(+), 1 deletion(-) diff --git a/openraft/src/docs/protocol/leader_lease.md b/openraft/src/docs/protocol/leader_lease.md index 4c6d73271..517eaad61 100644 --- a/openraft/src/docs/protocol/leader_lease.md +++ b/openraft/src/docs/protocol/leader_lease.md @@ -55,7 +55,9 @@ For an uncommitted vote (the vote that is saved upon receiving a `VoteRequest`): For a committed vote (the vote that is saved upon receiving `AppendEntriesRequest` or `InstallSnapshotRequest`): -- The node **WILL NOT** handle a `VoteRequest` before the `leader_lease` expires; +- The node **WILL NOT** handle a `VoteRequest` before the `leader_lease` expires. + A leader does not renew the lease on its own vote; it rejects a `VoteRequest` + while a quorum acknowledged it within the `leader_lease` instead; - The node **WILL NOT** elect itself until both the `leader_lease` and the `election_timeout` have expired. Both start at `Vote.last_update_time` and run in parallel, so the node waits for the longer of the two, not for their sum. diff --git a/openraft/src/docs/protocol/pre_vote.md b/openraft/src/docs/protocol/pre_vote.md index e60a9a5ad..c9e57cb3d 100644 --- a/openraft/src/docs/protocol/pre_vote.md +++ b/openraft/src/docs/protocol/pre_vote.md @@ -31,11 +31,20 @@ with more than one voter sends a Pre-Vote request to the other voters: *would you grant me a vote at `term + 1`?* The request carries that hypothetical vote and the voter's last log id. +The round starts only when the voter's own leader lease has expired: while it +is valid, a leader is serving the voter and every voter applying the same rule +would reject the round. + A voter answers with the rules it uses for a real vote request: - If it holds a committed vote whose [leader lease](`crate::docs::protocol::replication::leader_lease`) has not expired, it would not grant: a leader is serving it. +- If it is the leader and a quorum acknowledged it within the leader lease, it + would not grant. A leader never renews the lease on its own vote, because + acknowledgements renew it only on followers, so it uses this + quorum-acknowledged lease instead. A real vote request is judged the same + way, except that a planned leadership transfer releases the lease. - If the requester's last log id is behind its own, it would not grant. - If the requested vote is not greater than or equal to its own vote, it would not grant. diff --git a/openraft/src/engine/engine_impl.rs b/openraft/src/engine/engine_impl.rs index a8d5c2924..d279268c9 100644 --- a/openraft/src/engine/engine_impl.rs +++ b/openraft/src/engine/engine_impl.rs @@ -299,6 +299,18 @@ where C: RaftTypeConfig return; } + // A voter that a live leader serves does not start a round: every voter applying the same + // lease rule would reject it. Return before sampling a new timeout so nothing changes. + let lease = self.config.timer_config.leader_lease; + if self.state.vote_ref().is_committed() && !self.state.vote.lease_disabled() { + if let Some(utime) = self.state.vote_last_modified() { + if C::now() <= utime + lease { + tracing::info!("skip pre-elect: the leader lease has not yet expired"); + return; + } + } + } + // A Pre-Vote does not advance the persisted vote timestamp, so a new round is gated by a // newly sampled timeout, as a real campaign is. self.config.resample_election_timeout::(); @@ -400,6 +412,18 @@ where C: RaftTypeConfig } } + // A leader does not renew the lease on its own vote. While a quorum keeps acknowledging it, + // it rejects other candidates as its followers do. A planned leadership transfer releases + // this vote's lease, and its successor's vote is granted. + if !self.state.vote.lease_disabled() { + if let Some(leader) = self.leader.as_mut() { + if leader.is_lease_valid(now, lease) { + tracing::info!("reject vote-request: the quorum-acknowledged leader lease has not yet expired"); + return VoteResponse::new(self.state.vote_ref(), self.state.last_log_id().cloned(), false); + } + } + } + // The first step is to check log. If the candidate has less log, nothing needs to be done. if req.last_log_id.as_ref() >= self.state.last_log_id() { @@ -460,6 +484,16 @@ where C: RaftTypeConfig return VoteResponse::new(self.state.vote_ref(), self.state.last_log_id().cloned(), false); } + // Nor would a leader that a quorum keeps acknowledging. + if !self.state.vote.lease_disabled() { + if let Some(leader) = self.leader.as_mut() { + if leader.is_lease_valid(now, lease) { + tracing::info!("reject pre-vote-request: the quorum-acknowledged leader lease has not yet expired"); + return VoteResponse::new(self.state.vote_ref(), self.state.last_log_id().cloned(), false); + } + } + } + if req.last_log_id.as_ref() >= self.state.last_log_id() { // Ok } else { diff --git a/openraft/src/engine/tests/handle_vote_req_test.rs b/openraft/src/engine/tests/handle_vote_req_test.rs index 8f4b5132d..b9c2e48c7 100644 --- a/openraft/src/engine/tests/handle_vote_req_test.rs +++ b/openraft/src/engine/tests/handle_vote_req_test.rs @@ -9,6 +9,7 @@ use crate::engine::testing::UTConfig; use crate::engine::Command; use crate::engine::Engine; use crate::engine::LogIdList; +use crate::progress::Progress; use crate::raft::VoteRequest; use crate::raft::VoteResponse; use crate::testing::log_id; @@ -61,6 +62,74 @@ fn test_handle_vote_req_rejected_by_leader_lease() -> anyhow::Result<()> { Ok(()) } +/// A leader of `{0,1}` whose own vote is no longer leased, and which node 0 acknowledged just now. +fn acknowledged_leader() -> Engine { + let mut eng = eng(); + eng.candidate = None; + eng.state.vote = UTime::new(TokioInstant::now() - Duration::from_secs(1), Vote::new_committed(2, 1)); + eng.testing_new_leader().clock_progress.increase_to(&0, Some(TokioInstant::now())).unwrap(); + eng.state.server_state = ServerState::Leader; + eng.output.take_commands(); + eng +} + +#[test] +fn test_handle_vote_req_rejected_by_quorum_acknowledged_lease() -> anyhow::Result<()> { + let mut eng = acknowledged_leader(); + + let resp = eng.handle_vote_req(VoteRequest { + vote: Vote::new(3, 0), + last_log_id: Some(log_id(2, 1, 3)), + }); + + assert_eq!(VoteResponse::new(Vote::new_committed(2, 1), None, false), resp); + assert_eq!(Vote::new_committed(2, 1), *eng.state.vote_ref()); + assert!(eng.leader.is_some(), "the leader keeps leading"); + assert_eq!(0, eng.output.take_commands().len()); + + Ok(()) +} + +#[test] +fn test_handle_vote_req_planned_transfer_overrides_quorum_acknowledged_lease() -> anyhow::Result<()> { + let mut eng = acknowledged_leader(); + // A planned handoff releases the lease of the retiring vote. + eng.state.vote.disable_lease(); + + let resp = eng.handle_vote_req(VoteRequest { + vote: Vote::new(3, 0), + last_log_id: Some(log_id(2, 1, 3)), + }); + + assert_eq!(VoteResponse::new(Vote::new(3, 0), None, true), resp); + assert_eq!(Vote::new(3, 0), *eng.state.vote_ref()); + assert!(eng.leader.is_none(), "granting the successor ends this leadership"); + + Ok(()) +} + +#[test] +fn test_handle_vote_req_granted_once_the_quorum_acknowledged_lease_expires() -> anyhow::Result<()> { + let mut eng = acknowledged_leader(); + let expired = TokioInstant::now() - eng.config.timer_config.leader_lease - Duration::from_millis(1); + eng.leader.as_mut().unwrap().clock_progress = crate::progress::VecProgress::new( + eng.state.membership_state.effective().membership().to_quorum_set(), + [], + None, + ); + eng.leader.as_mut().unwrap().clock_progress.increase_to(&0, Some(expired)).unwrap(); + + let resp = eng.handle_vote_req(VoteRequest { + vote: Vote::new(3, 0), + last_log_id: Some(log_id(2, 1, 3)), + }); + + assert_eq!(VoteResponse::new(Vote::new(3, 0), None, true), resp); + assert!(eng.leader.is_none()); + + Ok(()) +} + #[test] fn test_handle_vote_req_reject_smaller_vote() -> anyhow::Result<()> { let mut eng = eng(); diff --git a/openraft/src/engine/tests/pre_vote_test.rs b/openraft/src/engine/tests/pre_vote_test.rs index 3afab5cb8..038ec7b1a 100644 --- a/openraft/src/engine/tests/pre_vote_test.rs +++ b/openraft/src/engine/tests/pre_vote_test.rs @@ -10,6 +10,7 @@ use crate::engine::testing::UTConfig; use crate::engine::Command; use crate::engine::Engine; use crate::engine::LogIdList; +use crate::progress::Progress; use crate::raft::VoteRequest; use crate::raft::VoteResponse; use crate::testing::log_id; @@ -347,3 +348,45 @@ fn test_winning_election_ends_an_overlapping_pre_vote_round() -> anyhow::Result< Ok(()) } + +#[test] +fn test_pre_elect_refused_while_the_leader_lease_is_valid() -> anyhow::Result<()> { + let mut eng = eng(m123()); + // A follower that its leader served just now. + eng.state.vote.update(TokioInstant::now(), Vote::new_committed(1, 2)); + let timeout_before = eng.config.timer_config.election_timeout; + let vote_before = *eng.state.vote_ref(); + + eng.pre_elect(); + + // Refused before it changes anything, including the sampled timeout. + assert!(eng.pre_candidate_ref().is_none()); + assert!(eng.candidate_ref().is_none()); + assert_eq!(vote_before, *eng.state.vote_ref()); + assert_eq!(timeout_before, eng.config.timer_config.election_timeout); + assert_eq!(0, eng.output.take_commands().len()); + + Ok(()) +} + +#[test] +fn test_handle_pre_vote_req_rejected_by_quorum_acknowledged_lease() -> anyhow::Result<()> { + let mut eng = eng(m123()); + // A leader whose own vote is no longer leased, acknowledged by node 2 just now. + eng.state.vote = UTime::new(TokioInstant::now() - Duration::from_secs(1), Vote::new_committed(2, 1)); + eng.testing_new_leader().clock_progress.increase_to(&2, Some(TokioInstant::now())).unwrap(); + eng.state.server_state = ServerState::Leader; + let vote_before = *eng.state.vote_ref(); + + let resp = eng.handle_pre_vote_req(VoteRequest::new(Vote::new(3, 3), Some(log_id(1, 2, 3)))); + + assert_eq!( + VoteResponse::new(Vote::new_committed(2, 1), Some(log_id(1, 2, 3)), false), + resp + ); + assert_eq!(vote_before, *eng.state.vote_ref()); + assert!(eng.leader.is_some(), "the leader keeps leading"); + assert_eq!(0, eng.output.take_commands().len()); + + Ok(()) +} diff --git a/openraft/src/proposer/leader.rs b/openraft/src/proposer/leader.rs index 25ee84ccd..24b913ef9 100644 --- a/openraft/src/proposer/leader.rs +++ b/openraft/src/proposer/leader.rs @@ -1,4 +1,5 @@ use std::fmt; +use std::time::Duration; use crate::display_ext::DisplaySliceExt; use crate::progress::entry::ProgressEntry; @@ -182,6 +183,16 @@ where /// Note that the leader may not be in the QuorumSet at all. /// In such a case, the update operation will be just ignored, /// and the quorum-acked-time is totally determined by remove voters. + /// Whether a quorum acknowledged this leader within the last `lease` before `now`. + /// + /// A leader never renews the lease on its own vote: AppendEntries acknowledgements renew it + /// only on followers. Each follower that acknowledged an AppendEntries sent at `t` received it + /// no earlier than `t`, so it rejects other candidates until at least `t + lease`. While a + /// quorum has acknowledged this leader within `lease`, this is the leader's live lease. + pub(crate) fn is_lease_valid(&mut self, now: InstantOf, lease: Duration) -> bool { + self.last_quorum_acked_time().is_some_and(|acked| now < acked + lease) + } + pub(crate) fn last_quorum_acked_time(&mut self) -> Option> { // For `Leading`, the vote is always the leader's vote. // Thus vote.voted_for() is this node. From 5ab83b63bb199651f0b0a58a21acf89e24d77c27 Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Thu, 1 Oct 2026 21:49:54 -0600 Subject: [PATCH 10/36] Doc: explain why a Pre-Vote response cannot reach a later round Two Pre-Vote rounds in the same term carry the same hypothetical vote, so the round check compares only the vote. Record why that is enough: every request is abandoned after election_timeout_min, the next round starts at least one sampled election timeout later, and responses and ticks share one ordered channel. Signed-off-by: VerifiedOrganic --- openraft/src/core/raft_core.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/openraft/src/core/raft_core.rs b/openraft/src/core/raft_core.rs index 53dacd074..c7fe71d95 100644 --- a/openraft/src/core/raft_core.rs +++ b/openraft/src/core/raft_core.rs @@ -1823,6 +1823,11 @@ where } /// A Pre-Vote response belongs to the in-flight Pre-Vote round only if it answers that round's /// vote; otherwise it is stale and ignored. + /// + /// Two rounds in the same term carry the same vote. A response still cannot reach a later + /// round: each request is abandoned after `election_timeout_min`, a new round starts no earlier + /// than one sampled election timeout after the previous one, and responses and ticks are + /// delivered in order on the same channel. fn does_pre_vote_match(&self, sender_vote: &Vote) -> bool { let my_vote = self.engine.pre_candidate_ref().map(|x| x.vote_ref()); From 3e082a95cb045269e6774d0de1e634f0d2f2b837 Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Thu, 1 Oct 2026 22:10:37 -0600 Subject: [PATCH 11/36] Fix: require the minimum election timeout to outlast the heartbeat tick A leader sends heartbeats on its engine tick, every three halves of the heartbeat interval. Now that a follower campaigns once the longer of its lease and its sampled election timeout expires, instead of after their sum, a minimum election timeout at or below that tick lets every follower campaign between two heartbeats of a healthy leader. Config::validate now rejects such a configuration with the existing ElectionTimeoutLTHeartBeat error, whose message names the tick. The raft-kv-rocksdb example used a 250 ms heartbeat with [299, 300) ms elections; its cluster test failed in hosted CI when a write reached a follower with no known leader. It now uses [800, 1200) ms. Two test configurations that never elect are adjusted to stay valid. Signed-off-by: VerifiedOrganic --- examples/raft-kv-rocksdb/src/lib.rs | 3 ++- openraft/src/config/config.rs | 9 ++++++- openraft/src/config/config_test.rs | 26 ++++++++++++++++++++ openraft/src/config/error.rs | 4 ++- openraft/src/replication/no_progress_test.rs | 3 +++ tests/tests/client_api/t11_client_reads.rs | 4 +-- 6 files changed, 44 insertions(+), 5 deletions(-) diff --git a/examples/raft-kv-rocksdb/src/lib.rs b/examples/raft-kv-rocksdb/src/lib.rs index 0ff063160..e2d4c42e1 100644 --- a/examples/raft-kv-rocksdb/src/lib.rs +++ b/examples/raft-kv-rocksdb/src/lib.rs @@ -84,7 +84,8 @@ where // Create a configuration for the raft instance. let config = Config { heartbeat_interval: 250, - election_timeout_min: 299, + election_timeout_min: 800, + election_timeout_max: 1_200, ..Default::default() }; diff --git a/openraft/src/config/config.rs b/openraft/src/config/config.rs index 9c062a13c..028690bd5 100644 --- a/openraft/src/config/config.rs +++ b/openraft/src/config/config.rs @@ -117,6 +117,10 @@ pub struct Config { pub cluster_name: String, /// The minimum election timeout in milliseconds + /// + /// It must exceed three halves of `heartbeat_interval`, the interval at which a leader sends + /// heartbeats, and should be at least twice that so one late heartbeat does not start an + /// election. It is also the follower's leader lease. #[clap(long, default_value = "150")] pub election_timeout_min: u64, @@ -363,7 +367,10 @@ impl Config { }); } - if self.election_timeout_min <= self.heartbeat_interval { + // A leader sends heartbeats on its engine tick, every three halves of the heartbeat + // interval. A follower campaigns once its sampled election timeout expires, so the + // smallest timeout has to outlast that tick. + if self.election_timeout_min <= self.heartbeat_interval.saturating_mul(3) / 2 { return Err(ConfigError::ElectionTimeoutLTHeartBeat { election_timeout_min: self.election_timeout_min, heartbeat_interval: self.heartbeat_interval, diff --git a/openraft/src/config/config_test.rs b/openraft/src/config/config_test.rs index f59729dc7..4ed7d6ef8 100644 --- a/openraft/src/config/config_test.rs +++ b/openraft/src/config/config_test.rs @@ -98,6 +98,32 @@ fn test_build() -> anyhow::Result<()> { Ok(()) } +#[test] +fn test_election_timeout_min_must_outlast_the_heartbeat_tick() { + // Heartbeats go out every three halves of the heartbeat interval: 150 ms here. + let config = Config { + election_timeout_min: 150, + election_timeout_max: 300, + heartbeat_interval: 100, + ..Default::default() + }; + assert_eq!( + config.validate().unwrap_err(), + ConfigError::ElectionTimeoutLTHeartBeat { + election_timeout_min: 150, + heartbeat_interval: 100 + } + ); + + let config = Config { + election_timeout_min: 151, + election_timeout_max: 300, + heartbeat_interval: 100, + ..Default::default() + }; + assert!(config.validate().is_ok()); +} + #[test] fn test_config_append_entries_timeout() -> anyhow::Result<()> { let config = Config::build(&["foo", "--heartbeat-interval=40"])?; diff --git a/openraft/src/config/error.rs b/openraft/src/config/error.rs index 5ff8a4df4..79e9bb2bb 100644 --- a/openraft/src/config/error.rs +++ b/openraft/src/config/error.rs @@ -14,7 +14,9 @@ pub enum ConfigError { #[error("max_payload_entries must be > 0")] MaxPayloadIs0, - #[error("election_timeout_min({election_timeout_min}) must be > heartbeat_interval({heartbeat_interval})")] + #[error( + "election_timeout_min({election_timeout_min}) must be > heartbeat_interval({heartbeat_interval}) * 3 / 2, the interval at which a leader sends heartbeats" + )] ElectionTimeoutLTHeartBeat { election_timeout_min: u64, heartbeat_interval: u64, diff --git a/openraft/src/replication/no_progress_test.rs b/openraft/src/replication/no_progress_test.rs index 89d742d35..7b2adc48f 100644 --- a/openraft/src/replication/no_progress_test.rs +++ b/openraft/src/replication/no_progress_test.rs @@ -360,6 +360,9 @@ impl Run { config: Arc::new( Config { heartbeat_interval, + // No election runs here; keep the timeouts valid for every heartbeat interval. + election_timeout_min: 10_000, + election_timeout_max: 10_001, ..Config::default() } .validate() diff --git a/tests/tests/client_api/t11_client_reads.rs b/tests/tests/client_api/t11_client_reads.rs index 2f5390aa2..01a131f0d 100644 --- a/tests/tests/client_api/t11_client_reads.rs +++ b/tests/tests/client_api/t11_client_reads.rs @@ -78,8 +78,8 @@ async fn get_read_log_id() -> Result<()> { enable_heartbeat: false, enable_elect: false, heartbeat_interval: 100, - election_timeout_min: 101, - election_timeout_max: 102, + election_timeout_min: 151, + election_timeout_max: 152, ..Default::default() } .validate()?, From 252ee9589db998b5d80e2a339398892d84c5d5f7 Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Thu, 1 Oct 2026 22:10:41 -0600 Subject: [PATCH 12/36] Chore: move the single-threaded example's test module to the end Current nightly clippy denies items after a test module (clippy::items_after_test_module), which fails the example's lint and test job. Move the unchanged module below the store implementation. Signed-off-by: VerifiedOrganic --- .../src/store.rs | 37 ++++++++++--------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/examples/raft-kv-memstore-singlethreaded/src/store.rs b/examples/raft-kv-memstore-singlethreaded/src/store.rs index 6a06bd394..6ab876adc 100644 --- a/examples/raft-kv-memstore-singlethreaded/src/store.rs +++ b/examples/raft-kv-memstore-singlethreaded/src/store.rs @@ -48,24 +48,6 @@ impl Request { } } -#[cfg(test)] -mod tests { - use std::marker::PhantomData; - - use crate::store::Request; - - #[test] - fn test_serde() { - let a = Request::Set { - key: "foo".to_string(), - value: "bar".to_string(), - _p: PhantomData, - }; - - let b = serde_json::to_string(&a).unwrap(); - println!("{}", b); - } -} #[derive(Serialize, Deserialize, Debug, Clone)] pub struct Response { @@ -374,3 +356,22 @@ impl RaftLogStorage for Rc { self.clone() } } + +#[cfg(test)] +mod tests { + use std::marker::PhantomData; + + use crate::store::Request; + + #[test] + fn test_serde() { + let a = Request::Set { + key: "foo".to_string(), + value: "bar".to_string(), + _p: PhantomData, + }; + + let b = serde_json::to_string(&a).unwrap(); + println!("{}", b); + } +} From f658dc5d35645953e3776f10721ee8cf8d974d49 Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Thu, 1 Oct 2026 23:04:37 -0600 Subject: [PATCH 13/36] Chore: format the single-threaded example store Moving the test module left a double blank line, which the example's own rustfmt check rejects. Signed-off-by: VerifiedOrganic --- examples/raft-kv-memstore-singlethreaded/src/store.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/examples/raft-kv-memstore-singlethreaded/src/store.rs b/examples/raft-kv-memstore-singlethreaded/src/store.rs index 6ab876adc..8d74252ea 100644 --- a/examples/raft-kv-memstore-singlethreaded/src/store.rs +++ b/examples/raft-kv-memstore-singlethreaded/src/store.rs @@ -48,7 +48,6 @@ impl Request { } } - #[derive(Serialize, Deserialize, Debug, Clone)] pub struct Response { pub value: Option, From feaf48200c678b8eeb9eae56aa53e3c12121b5fd Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Fri, 2 Oct 2026 06:50:17 -0600 Subject: [PATCH 14/36] Test: a Pre-Vote rejection must not vote for its requester A network that cannot ask a voter, such as a voter of a release without Pre-Vote, has to answer that voter's Pre-Vote itself. If its rejection echoes the requested next-term vote, the requester adopts it as a strictly higher vote and so votes for itself in a term it never campaigned in, without sending any vote request. No voter can hold such a vote, so the requester must leave it alone. This test fails on the current engine: it adopts the echoed vote. Signed-off-by: VerifiedOrganic --- openraft/src/engine/tests/pre_vote_test.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/openraft/src/engine/tests/pre_vote_test.rs b/openraft/src/engine/tests/pre_vote_test.rs index 038ec7b1a..3c36972d4 100644 --- a/openraft/src/engine/tests/pre_vote_test.rs +++ b/openraft/src/engine/tests/pre_vote_test.rs @@ -390,3 +390,25 @@ fn test_handle_pre_vote_req_rejected_by_quorum_acknowledged_lease() -> anyhow::R Ok(()) } + +#[test] +fn test_handle_pre_vote_resp_rejection_never_adopts_a_vote_for_this_node() -> anyhow::Result<()> { + let mut eng = eng(m123()); + eng.pre_elect(); + eng.output.take_commands(); + let vote_before = *eng.state.vote_ref(); + let proposed = *eng.pre_candidate_ref().unwrap().vote_ref(); + assert!(proposed > vote_before); + + // A network that cannot ask the voter answers with a rejection of its own. If it echoed the + // proposed vote, adopting it would vote for this node in a term it never campaigned in. + eng.handle_pre_vote_resp(3, VoteResponse::new(proposed, None, false)); + + assert_eq!(vote_before, *eng.state.vote_ref()); + assert!( + eng.pre_candidate_ref().is_some(), + "the round keeps waiting for real answers" + ); + assert_eq!(0, eng.output.take_commands().len()); + Ok(()) +} From b26565054d0fa682acf47c158281cea17a663f36 Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Fri, 2 Oct 2026 06:50:30 -0600 Subject: [PATCH 15/36] Fix: never adopt a vote for this node from a Pre-Vote response A Pre-Vote rejection with a strictly higher vote catches the requester up to that vote. A vote for the requester itself in a term it never campaigned in cannot come from a voter, only from a network that answers for a voter it cannot ask. Adopting it voted for the requester without sending a vote request, so its term climbed on every round while no election ran. Leave such a vote alone, and keep the round waiting for real answers. Signed-off-by: VerifiedOrganic --- openraft/src/engine/engine_impl.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/openraft/src/engine/engine_impl.rs b/openraft/src/engine/engine_impl.rs index d279268c9..557b72cfe 100644 --- a/openraft/src/engine/engine_impl.rs +++ b/openraft/src/engine/engine_impl.rs @@ -555,8 +555,10 @@ where C: RaftTypeConfig } // Catch up to a strictly higher vote, never as committed. An equal vote is left alone: it - // would end the in-flight round without new information. - if &resp.vote > self.state.vote_ref() { + // would end the in-flight round without new information. A vote for this node is not + // adopted either: no voter holds one for a term this node did not campaign in, so it can + // only be a network's own answer, and adopting it would vote for this node unsent. + if &resp.vote > self.state.vote_ref() && resp.vote.leader_id().voted_for() != Some(self.config.id.clone()) { let mut vote = resp.vote.clone(); vote.committed = false; let _ = self.vote_handler().update_vote(&vote); From 1ef8d38cc4bc626f25c895e3a8308c6c6ee7d3ec Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Fri, 2 Oct 2026 07:48:34 -0600 Subject: [PATCH 16/36] Test: voters that cannot answer Pre-Vote must not block an election During a rolling upgrade some voters run a release without Pre-Vote. They campaign with the real vote alone, and a network can only answer their Pre-Vote for them, as a rejection. The test router gains that answer. When such voters miss the last entries and the leader is lost, the survivors with the up-to-date log cannot reach a Pre-Vote quorum without them, and the classic voters cannot win with their stale logs. With 3 and 5 voters, no leader is elected within ten election timeouts on the current engine. Signed-off-by: VerifiedOrganic --- tests/tests/elect/main.rs | 1 + .../elect/t23_classic_voter_stale_log.rs | 126 ++++++++++++++++++ tests/tests/fixtures/mod.rs | 21 +++ 3 files changed, 148 insertions(+) create mode 100644 tests/tests/elect/t23_classic_voter_stale_log.rs diff --git a/tests/tests/elect/main.rs b/tests/tests/elect/main.rs index a20c8ca1b..26a92670a 100644 --- a/tests/tests/elect/main.rs +++ b/tests/tests/elect/main.rs @@ -14,3 +14,4 @@ mod t13_elect_while_leader; mod t20_lease_and_timeout_overlap; mod t21_pre_vote; mod t22_one_way_isolation; +mod t23_classic_voter_stale_log; diff --git a/tests/tests/elect/t23_classic_voter_stale_log.rs b/tests/tests/elect/t23_classic_voter_stale_log.rs new file mode 100644 index 000000000..cc5f25f4c --- /dev/null +++ b/tests/tests/elect/t23_classic_voter_stale_log.rs @@ -0,0 +1,126 @@ +use std::collections::BTreeSet; +use std::sync::Arc; +use std::time::Duration; + +use anyhow::Result; +use maplit::btreeset; +use openraft::Config; +use openraft::ServerState; + +use crate::fixtures::init_default_ut_tracing; +use crate::fixtures::Direction; +use crate::fixtures::RPCErrorType; +use crate::fixtures::RaftRouter; + +fn pre_vote_config() -> Result> { + Ok(Arc::new( + Config { + heartbeat_interval: 50, + election_timeout_min: 500, + election_timeout_max: 600, + enable_pre_vote: Some(true), + ..Default::default() + } + .validate()?, + )) +} + +/// Voters that cannot answer Pre-Vote, such as voters of a release without it, and that miss the +/// last entries do not block an election. +/// +/// The survivors with the up-to-date log run Pre-Vote, and the network answers their Pre-Vote to a +/// classic voter as a rejection, so they cannot reach a Pre-Vote quorum without those voters. The +/// classic voters campaign with the classic vote, but their logs are behind, so they cannot win +/// either. A survivor that rejects such a candidate for its log must campaign itself, without +/// Pre-Vote and in a term above that candidate's, or no leader is ever elected. +async fn classic_voters_with_stale_logs_do_not_block_an_election(voter_count: u64) -> Result<()> { + let config = pre_vote_config()?; + let mut router = RaftRouter::new(config.clone()); + + let voters = (0..voter_count).collect::>(); + tracing::info!("--- create cluster of {:?}; node 0 becomes leader", voters); + let mut log_index = router.new_cluster(voters.clone(), btreeset! {}).await?; + let n0 = router.get_raft_handle(&0)?; + n0.wait(timeout()).state(ServerState::Leader, "node 0 is the initial leader").await?; + for id in voters.iter() { + router + .wait(id, timeout()) + .metrics(|m| m.current_leader == Some(0), "every voter follows node 0") + .await?; + } + + // A minority that the up-to-date survivors still need for a quorum once node 0 is lost. + let classic = (voter_count / 2 + 1..voter_count).collect::>(); + let current = (1..voter_count).filter(|id| !classic.contains(id)).collect::>(); + tracing::info!( + "--- voters {:?} cannot answer Pre-Vote and campaign without it", + classic + ); + for id in classic.iter() { + router.get_raft_handle(id)?.runtime_config().pre_vote(false); + router.set_pre_vote_unsupported(*id, true); + } + + tracing::info!("--- voters {:?} miss the last entries", classic); + for id in classic.iter() { + router.set_rpc_failure(*id, Direction::NetRecv, Some(RPCErrorType::NetworkError)); + } + router.client_request_many(0, "missed", 5).await?; + log_index += 5; + for id in current.iter() { + router + .wait(id, timeout()) + .applied_index(Some(log_index), "the up-to-date survivors have the entries") + .await?; + } + + tracing::info!("--- node 0 is lost; voters {:?} are reachable again", classic); + let (n0, _, _) = router.remove_node(0).unwrap(); + n0.shutdown().await?; + for id in classic.iter() { + router.set_rpc_failure(*id, Direction::NetRecv, None); + } + + let election = Some(Duration::from_millis(config.election_timeout_max * 10)); + let survivors = (1..voter_count).collect::>(); + let mut leader = None; + for id in survivors.iter() { + let m = router + .wait(id, election) + .metrics( + |m| m.current_leader.is_some_and(|l| current.contains(&l)), + "an up-to-date survivor is elected", + ) + .await?; + let elected = m.current_leader; + assert!( + leader.is_none() || leader == elected, + "the survivors agree on one leader" + ); + leader = elected; + } + let leader = leader.unwrap(); + + tracing::info!("--- node {} commits, and every survivor applies the log", leader); + router.client_request_many(leader, "elected", 1).await?; + log_index += 2; + for id in survivors.iter() { + router.wait(id, timeout()).applied_index(Some(log_index), "every survivor applies the log").await?; + } + + Ok(()) +} + +#[async_entry::test(worker_threads = 8, init = "init_default_ut_tracing()", tracing_span = "debug")] +async fn three_voters_classic_voter_with_a_stale_log_does_not_block_an_election() -> Result<()> { + classic_voters_with_stale_logs_do_not_block_an_election(3).await +} + +#[async_entry::test(worker_threads = 8, init = "init_default_ut_tracing()", tracing_span = "debug")] +async fn five_voters_classic_voters_with_stale_logs_do_not_block_an_election() -> Result<()> { + classic_voters_with_stale_logs_do_not_block_an_election(5).await +} + +fn timeout() -> Option { + Some(Duration::from_millis(3_000)) +} diff --git a/tests/tests/fixtures/mod.rs b/tests/tests/fixtures/mod.rs index 0957af93f..3d700db3c 100644 --- a/tests/tests/fixtures/mod.rs +++ b/tests/tests/fixtures/mod.rs @@ -266,6 +266,10 @@ pub struct TypedRaftRouter { /// Count of RPCs sent. rpc_count: Arc>>, + /// Nodes that cannot answer Pre-Vote, such as voters of a release without it. A Pre-Vote to + /// one of them is answered as a rejection without reaching it. + pre_vote_unsupported: Arc>>, + /// A hook function to be called when before an RPC is sent to target node. rpc_pre_hook: Arc>>, @@ -312,6 +316,7 @@ impl Builder { send_delay: Arc::new(AtomicU64::new(send_delay)), append_entries_quota: Arc::new(Mutex::new(None)), rpc_count: Default::default(), + pre_vote_unsupported: Default::default(), rpc_pre_hook: Default::default(), rpc_observers: Default::default(), blocked_rpc: Default::default(), @@ -566,6 +571,17 @@ impl TypedRaftRouter { } /// Set whether to emit a specified rpc error when sending to/receiving from a node. + /// Answer every Pre-Vote to `id` as a rejection without reaching it, as a network does for a + /// voter of a release without Pre-Vote. + pub fn set_pre_vote_unsupported(&self, id: MemNodeId, unsupported: bool) { + let mut nodes = self.pre_vote_unsupported.lock().unwrap(); + if unsupported { + nodes.insert(id); + } else { + nodes.remove(&id); + } + } + pub fn set_rpc_failure(&self, id: MemNodeId, dir: Direction, rpc_error_type: Option) { let mut fails = self.fail_rpc.lock().unwrap(); if let Some(rpc_error_type) = rpc_error_type { @@ -1178,6 +1194,11 @@ impl RaftNetwork for RaftRouterNetwork { self.owner.emit_rpc_error(from_id, self.target)?; self.owner.rand_send_delay().await; + if self.owner.pre_vote_unsupported.lock().unwrap().contains(&self.target) { + // A rejection that carries no vote to catch up to. + return Ok(VoteResponse::new(Vote::new(0, from_id), None, false)); + } + let node = self.owner.get_raft_handle(&self.target)?; let resp = node.pre_vote(rpc).await; From 5cebb7523ebc19ec33c565ddc0a6a41d822be8c8 Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Fri, 2 Oct 2026 08:05:32 -0600 Subject: [PATCH 17/36] Fix: campaign above a candidate rejected for its stale log A voter that runs Pre-Vote could wait forever when the voters it needs cannot answer Pre-Vote, as voters of a release without it cannot. Such a voter campaigns with the real vote, but when its log is behind it cannot win either, and nobody else campaigns. A voter that rejects a vote request because the candidate's log is behind its own, while it has no leader, now records the candidate's term. If no leader appears within one election timeout of the first rejection, it campaigns without Pre-Vote, in a term above every recorded term. The rejected candidate voted for itself in its own term, so a campaign in that term could not win its vote. The wait lets that candidate still win with the other voters' grants without being disrupted. Hearing from a leader, granting a vote, leading or campaigning clears the record. No vote rule changes: a voter still grants only a candidate whose log is at least as up to date as its own. Signed-off-by: VerifiedOrganic --- openraft/src/core/raft_core.rs | 8 ++- openraft/src/engine/engine_impl.rs | 55 ++++++++++++++- openraft/src/engine/tests/elect_test.rs | 28 ++++++++ .../src/engine/tests/handle_vote_req_test.rs | 68 +++++++++++++++++++ 4 files changed, 156 insertions(+), 3 deletions(-) diff --git a/openraft/src/core/raft_core.rs b/openraft/src/core/raft_core.rs index c7fe71d95..dc0f36fbf 100644 --- a/openraft/src/core/raft_core.rs +++ b/openraft/src/core/raft_core.rs @@ -1735,8 +1735,14 @@ where tracing::info!("election timeout passed, check if it is a voter for election"); } + // A node that rejected a candidate for its log, and has heard from no leader for one + // election timeout since, campaigns without Pre-Vote: see `Engine::stale_candidate`. + let stale_candidate_due = + self.engine.stale_candidate_campaign_due(now, self.engine.config.timer_config.election_timeout); + // Pre-Vote applies to multiple voters only: a single voter always wins its own Pre-Vote. - let pre_vote = self.runtime_config.enable_pre_vote.load(Ordering::Relaxed) && voter_count > 1; + let pre_vote = + self.runtime_config.enable_pre_vote.load(Ordering::Relaxed) && voter_count > 1 && !stale_candidate_due; if pre_vote { // A Pre-Vote round does not update the vote, so the expired election timer would diff --git a/openraft/src/engine/engine_impl.rs b/openraft/src/engine/engine_impl.rs index 557b72cfe..f2cfe57fc 100644 --- a/openraft/src/engine/engine_impl.rs +++ b/openraft/src/engine/engine_impl.rs @@ -43,6 +43,7 @@ use crate::raft::VoteResponse; use crate::raft_state::LogStateReader; use crate::raft_state::RaftState; use crate::summary::MessageSummary; +use crate::type_config::alias::InstantOf; use crate::type_config::alias::ResponderOf; use crate::type_config::alias::SnapshotDataOf; use crate::type_config::TypeConfigExt; @@ -56,6 +57,18 @@ use crate::Snapshot; use crate::SnapshotMeta; use crate::Vote; +/// A candidate whose vote request this node rejected because its log was behind; see +/// `Engine::stale_candidate`. +#[derive(Debug, Clone)] +pub(crate) struct StaleCandidate +where C: RaftTypeConfig +{ + /// The highest term among such rejected requests. + pub(crate) term: u64, + /// When the first of them was rejected. + pub(crate) rejected_at: InstantOf, +} + /// Raft protocol algorithm. /// /// It implement the complete raft algorithm except does not actually update any states. @@ -97,6 +110,18 @@ where C: RaftTypeConfig /// persisted, and it is dropped once a real election starts or this node follows another vote. pub(crate) pre_candidate: CandidateState, + /// A candidate whose vote request this node rejected because that candidate's log was behind + /// this node's, recorded only while this node has no leader. + /// + /// Such a candidate cannot win against this node's log. When it is a voter that cannot answer + /// Pre-Vote, as a voter of a release without Pre-Vote cannot, this node's own Pre-Vote may + /// never reach a quorum either. If no leader appears for one election timeout after the + /// first rejection, this node campaigns without Pre-Vote, in a term above every rejected + /// candidate's. The wait lets such a candidate still win with the other voters' grants + /// without being disrupted. Hearing from a leader, granting a vote, leading or campaigning + /// clears it. + pub(crate) stale_candidate: Option>, + /// Retirement is irreversible for this engine instance, including after a new vote. pub(crate) prepared_shutdown: Option>, @@ -118,6 +143,7 @@ where C: RaftTypeConfig leader: None, candidate: None, pre_candidate: None, + stale_candidate: None, prepared_shutdown: None, output: EngineOutput::new(4096), } @@ -267,7 +293,10 @@ where C: RaftTypeConfig self.leader.as_ref().map(|l| l.vote.to_string()).unwrap_or_default() ); - let new_term = self.state.vote.leader_id().term + 1; + // Campaign above every candidate this node rejected for its log, so that such a candidate's + // vote for itself in a term cannot block this campaign. + let stale_term = self.stale_candidate.take().map_or(0, |stale| stale.term); + let new_term = std::cmp::max(self.state.vote.leader_id().term, stale_term) + 1; let new_vote = Vote::new(new_term, self.config.id.clone()); let candidate = self.new_candidate(new_vote.clone()); @@ -437,6 +466,16 @@ where C: RaftTypeConfig // The res is not used yet. // let _res = Err(RejectVoteRequest::ByLastLogId(self.state.last_log_id().copied())); + // This node has no leader: its lease has expired, and it does not lead. Remember the + // candidate, which cannot win against this node's log; see `stale_candidate`. + let term = req.vote.leader_id().term; + match self.stale_candidate.as_mut() { + Some(stale) => stale.term = std::cmp::max(stale.term, term), + None => { + self.stale_candidate = Some(StaleCandidate { term, rejected_at: now }); + } + } + // Return the updated vote, this way the candidate knows which vote is granted, in case // the candidate's vote is changed after sending the vote request. return VoteResponse::new(self.state.vote_ref(), self.state.last_log_id().cloned(), false); @@ -445,6 +484,10 @@ where C: RaftTypeConfig // Then check vote just as it does for every incoming event. let res = self.vote_handler().update_vote(&req.vote); + if res.is_ok() { + // A granted vote follows another candidate. + self.stale_candidate = None; + } tracing::info!( req = display(req.summary()), @@ -670,7 +713,8 @@ where C: RaftTypeConfig ) -> Result<(), RejectAppendEntries> { self.vote_handler().update_vote(vote)?; - // Vote is legal. + // Vote is legal: this node hears from a leader. + self.stale_candidate = None; let mut fh = self.following_handler(); fh.ensure_log_consecutive(prev_log_id.clone())?; @@ -873,6 +917,7 @@ where C: RaftTypeConfig // An overlapping Pre-Vote round must not start another campaign once this node leads. self.pre_candidate = None; + self.stale_candidate = None; let candidate = self.candidate.take().unwrap(); let leader = self.establish_handler().establish(candidate); @@ -933,6 +978,12 @@ where C: RaftTypeConfig } } + /// Whether this node campaigns without Pre-Vote now, after rejecting a stale candidate at + /// least `wait` ago and hearing from no leader since; see `stale_candidate`. + pub(crate) fn stale_candidate_campaign_due(&self, now: InstantOf, wait: Duration) -> bool { + self.stale_candidate.as_ref().is_some_and(|stale| now >= stale.rejected_at + wait) + } + pub(crate) fn is_there_greater_log(&self) -> bool { self.seen_greater_log } diff --git a/openraft/src/engine/tests/elect_test.rs b/openraft/src/engine/tests/elect_test.rs index da083780b..b505b43e1 100644 --- a/openraft/src/engine/tests/elect_test.rs +++ b/openraft/src/engine/tests/elect_test.rs @@ -387,3 +387,31 @@ fn test_elect_multi_node_enter_candidate() -> anyhow::Result<()> { } Ok(()) } + +#[test] +fn test_elect_campaigns_above_a_rejected_stale_candidate() -> anyhow::Result<()> { + let mut eng = eng(); + eng.config.id = 1; + eng.state + .membership_state + .set_effective(Arc::new(EffectiveMembership::new(Some(log_id(0, 1, 1)), m12()))); + eng.state.log_ids = LogIdList::new(vec![log_id(1, 1, 1)]); + eng.stale_candidate = Some(crate::engine::engine_impl::StaleCandidate { + term: 4, + rejected_at: crate::TokioInstant::now(), + }); + + eng.elect(); + + // The stale candidate voted for itself in term 4, so a campaign in term 1 or 4 could not win + // its vote. + assert_eq!(Vote::new(5, 1), *eng.state.vote_ref()); + assert!(eng.stale_candidate.is_none(), "the campaign consumes the record"); + assert_eq!( + vec![Command::SaveVote { vote: Vote::new(5, 1) }, Command::SendVote { + vote_req: VoteRequest::new(Vote::new(5, 1), Some(log_id(1, 1, 1))) + },], + eng.output.take_commands() + ); + Ok(()) +} diff --git a/openraft/src/engine/tests/handle_vote_req_test.rs b/openraft/src/engine/tests/handle_vote_req_test.rs index b9c2e48c7..3251cde42 100644 --- a/openraft/src/engine/tests/handle_vote_req_test.rs +++ b/openraft/src/engine/tests/handle_vote_req_test.rs @@ -282,3 +282,71 @@ fn test_handle_vote_req_granted_follower_learner_does_not_emit_update_server_sta } Ok(()) } + +#[test] +fn test_handle_vote_req_smaller_last_log_id_records_the_stale_candidate() -> anyhow::Result<()> { + let mut eng = eng(); + eng.state.log_ids = LogIdList::new(vec![log_id(2, 1, 3)]); + let before = TokioInstant::now(); + + for (term, candidate) in [(3, 2), (5, 0), (4, 2)] { + let resp = eng.handle_vote_req(VoteRequest { + vote: Vote::new(term, candidate), + last_log_id: Some(log_id(1, 1, 3)), + }); + assert!(!resp.vote_granted); + } + + let stale = eng.stale_candidate.clone().expect("the rejected candidates are recorded"); + assert_eq!(5, stale.term, "the highest rejected term is kept"); + assert!(stale.rejected_at >= before, "the first rejection starts the wait"); + assert!(!eng.stale_candidate_campaign_due(TokioInstant::now(), Duration::from_secs(1))); + assert!(eng.stale_candidate_campaign_due(stale.rejected_at + Duration::from_secs(1), Duration::from_secs(1))); + + // Nothing else changes: the record is in memory only. + assert_eq!(Vote::new(2, 1), *eng.state.vote_ref()); + assert_eq!(0, eng.output.take_commands().len()); + Ok(()) +} + +#[test] +fn test_handle_vote_req_lease_rejection_records_no_stale_candidate() -> anyhow::Result<()> { + let mut eng = eng(); + eng.state.log_ids = LogIdList::new(vec![log_id(2, 1, 3)]); + eng.state.vote.update(TokioInstant::now(), Vote::new_committed(2, 1)); + + let resp = eng.handle_vote_req(VoteRequest { + vote: Vote::new(3, 2), + last_log_id: Some(log_id(1, 1, 3)), + }); + + assert!(!resp.vote_granted); + assert!( + eng.stale_candidate.is_none(), + "a node that still hears from its leader records nothing" + ); + Ok(()) +} + +#[test] +fn test_handle_vote_req_granted_vote_clears_the_stale_candidate() -> anyhow::Result<()> { + let mut eng = eng(); + eng.state.log_ids = LogIdList::new(vec![log_id(2, 1, 3)]); + eng.handle_vote_req(VoteRequest { + vote: Vote::new(3, 2), + last_log_id: Some(log_id(1, 1, 3)), + }); + assert!(eng.stale_candidate.is_some()); + + let resp = eng.handle_vote_req(VoteRequest { + vote: Vote::new(4, 2), + last_log_id: Some(log_id(2, 1, 3)), + }); + + assert!(resp.vote_granted); + assert!( + eng.stale_candidate.is_none(), + "a granted vote follows another candidate" + ); + Ok(()) +} From 38c6524a405a55e2d6808b136329ff62ba4e492d Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Fri, 2 Oct 2026 08:05:47 -0600 Subject: [PATCH 18/36] Doc: describe voters that cannot answer Pre-Vote Explain how a network answers for a voter that cannot decode a Pre-Vote request, why that voter counts as rejecting, and how the stale candidate rule keeps such voters from blocking an election. Note that a Pre-Vote response never makes the requester adopt a vote for itself. Signed-off-by: VerifiedOrganic --- openraft/src/docs/protocol/pre_vote.md | 34 +++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/openraft/src/docs/protocol/pre_vote.md b/openraft/src/docs/protocol/pre_vote.md index c9e57cb3d..7965ac3af 100644 --- a/openraft/src/docs/protocol/pre_vote.md +++ b/openraft/src/docs/protocol/pre_vote.md @@ -58,7 +58,9 @@ would grant. If a quorum would grant, the voter starts the real election, increments its term and persists its vote as usual. A response with a strictly higher vote catches the requester up to that vote in non-committed form, as a rejected real -vote does. A rejection that reports a greater log delays the next attempt. Any +vote does, unless it is a vote for the requester itself: no voter holds one for +a term the requester never campaigned in. A rejection that reports a greater +log delays the next attempt. Any vote the requester accepts afterwards, including a heartbeat from the current leader, ends the round. Otherwise the round is retried after one newly sampled election timeout. @@ -69,6 +71,36 @@ peer, is never a grant, so a voter that is cut off cannot assemble one. A voter that is the only voter wins its own Pre-Vote and elects at once. +## Voters that cannot answer Pre-Vote + +A network may reach voters that cannot answer a Pre-Vote request, such as +voters of a release without Pre-Vote during a rolling upgrade. Such a voter +campaigns with the real vote alone and grants real votes by the rules above. A +network that knows a voter is one of them answers its Pre-Vote locally, as a +rejection that carries no vote to catch up to, and never sends it a request it +could not decode. + +Counting such a voter as rejecting keeps a voter that runs Pre-Vote from +campaigning, and raising its term, before a voter without Pre-Vote that may +hold a more up-to-date log campaigns. That voter then wins with the real vote. +When its log is behind, it cannot win, and a voter whose log is more up to date +must campaign instead, although its own Pre-Vote may never reach a quorum: + +- A voter that rejects a real vote request because the candidate's log is + behind its own, while it has no leader, records that candidate's term. +- If it hears from no leader for one election timeout after the first such + rejection, it campaigns without Pre-Vote, in a term above every recorded + term. The candidate voted for itself in its own term, so a campaign in that + term could not win its vote. +- The wait lets that candidate still win with the other voters' grants, without + being disrupted. +- Hearing from a leader, granting a vote, leading or campaigning clears the + record. + +The rule changes no vote rule: a voter still grants only a candidate whose log +is at least as up to date as its own. It only decides when a voter campaigns. + + ## Networks without Pre-Vote Pre-Vote uses the separate From 5e5f57b1de490994fb6fefd9654051481a6f55e8 Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Fri, 2 Oct 2026 11:07:05 -0600 Subject: [PATCH 19/36] Test: a delayed Pre-Vote grant must not reach a later round Signed-off-by: VerifiedOrganic --- openraft/src/engine/tests/pre_vote_test.rs | 48 ++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/openraft/src/engine/tests/pre_vote_test.rs b/openraft/src/engine/tests/pre_vote_test.rs index 3c36972d4..108e89c35 100644 --- a/openraft/src/engine/tests/pre_vote_test.rs +++ b/openraft/src/engine/tests/pre_vote_test.rs @@ -412,3 +412,51 @@ fn test_handle_pre_vote_resp_rejection_never_adopts_a_vote_for_this_node() -> an assert_eq!(0, eng.output.take_commands().len()); Ok(()) } + +/// A grant that waited in the notification queue must not reach a later round. +/// +/// Rounds in one term share their hypothetical vote. A same-term AppendEntries that removes a +/// voter ends the round; if its flush outlasts the election timeout, the next tick starts a new +/// round with the same vote. The removed voter's grant for the ended round, delivered only then, +/// cannot be told apart by that vote, and it must neither count nor stop this node. +#[test] +fn test_delayed_pre_vote_grant_from_a_removed_voter_does_not_reach_a_later_round() -> anyhow::Result<()> { + let mut eng = eng(m123()); + eng.pre_elect(); + eng.output.take_commands(); + let first_round_vote = *eng.pre_candidate_ref().unwrap().vote_ref(); + + // Leader 2 replicates a membership without voter 3 in its unchanged term, which ends the round. + eng.vote_handler().update_vote(&Vote::new_committed(1, 2))?; + assert!(eng.pre_candidate_ref().is_none()); + eng.state + .membership_state + .set_effective(Arc::new(EffectiveMembership::new(Some(log_id(1, 2, 3)), m12()))); + + // The flush outlasts the election timeout, and the queued tick starts a new round. + eng.state.vote = UTime::new(TokioInstant::now() - Duration::from_secs(1), Vote::new_committed(1, 2)); + eng.pre_elect(); + eng.output.take_commands(); + assert_eq!( + first_round_vote, + *eng.pre_candidate_ref().unwrap().vote_ref(), + "both rounds propose the same vote" + ); + + // Voter 3's grant for the first round arrives now. + eng.handle_pre_vote_resp( + 3, + VoteResponse::new(Vote::new_committed(1, 2), Some(log_id(1, 2, 3)), true), + ); + + assert!(eng.pre_candidate_ref().is_some(), "the new round keeps waiting"); + assert_eq!( + btreeset! {1}, + eng.pre_candidate_ref().unwrap().granters().collect::>() + ); + assert!(eng.candidate_ref().is_none(), "no real election starts"); + assert_eq!(Vote::new_committed(1, 2), *eng.state.vote_ref()); + assert_eq!(0, eng.output.take_commands().len()); + + Ok(()) +} From 0cb72abbd106234fae927320ce40c0f7306d961d Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Fri, 2 Oct 2026 11:08:47 -0600 Subject: [PATCH 20/36] Fix: tell Pre-Vote rounds apart by an identifier, and ignore a grant from a non-voter Rounds in one term propose the same hypothetical vote, so a response was matched to the in-flight round by that vote. A grant delayed in the notification queue could therefore reach a later round in the same term, and if its sender had been removed from the membership in between, counting it panicked the Raft core. Each round now carries an identifier through SendPreVote and the PreVoteResponse notification, and the engine ignores a response to any round but the one in flight. Counting a grant from a node outside a round's quorum set is ignored instead of panicking. Signed-off-by: VerifiedOrganic --- openraft/src/core/notify.rs | 14 +++---- openraft/src/core/raft_core.rs | 49 +++++++--------------- openraft/src/engine/command.rs | 8 +++- openraft/src/engine/engine_impl.rs | 23 +++++++++- openraft/src/engine/tests/pre_vote_test.rs | 47 +++++++++++++++++---- openraft/src/proposer/candidate.rs | 14 ++++++- 6 files changed, 98 insertions(+), 57 deletions(-) diff --git a/openraft/src/core/notify.rs b/openraft/src/core/notify.rs index b680074e7..7ba77c10a 100644 --- a/openraft/src/core/notify.rs +++ b/openraft/src/core/notify.rs @@ -24,8 +24,8 @@ where C: RaftTypeConfig target: C::NodeId, resp: VoteResponse, - /// The hypothetical next-term vote of the Pre-Vote round that sent the request. - sender_vote: Vote, + /// The Pre-Vote round that sent the request. + round: u64, }, /// Seen a higher `vote`. @@ -81,16 +81,12 @@ where C: RaftTypeConfig } => { format!("VoteResponse: from: {}: {}, res-vote: {}", target, resp.summary(), vote) } - Self::PreVoteResponse { - target, - resp, - sender_vote: vote, - } => { + Self::PreVoteResponse { target, resp, round } => { format!( - "PreVoteResponse: from: {}: {}, pre-vote: {}", + "PreVoteResponse: from: {}: {}, round: {}", target, resp.summary(), - vote + round ) } Self::HigherVote { diff --git a/openraft/src/core/raft_core.rs b/openraft/src/core/raft_core.rs index dc0f36fbf..15fd84dac 100644 --- a/openraft/src/core/raft_core.rs +++ b/openraft/src/core/raft_core.rs @@ -219,18 +219,19 @@ where pub(crate) _p: PhantomData, } -/// Whether [`RaftCore::spawn_parallel_vote_requests`] sends Vote or Pre-Vote requests. +/// Whether [`RaftCore::spawn_parallel_vote_requests`] sends Vote requests or the Pre-Vote +/// requests of one round. #[derive(Clone, Copy, Debug)] enum VoteRequestKind { Vote, - PreVote, + PreVote { round: u64 }, } impl VoteRequestKind { fn as_str(self) -> &'static str { match self { VoteRequestKind::Vote => "vote", - VoteRequestKind::PreVote => "pre-vote", + VoteRequestKind::PreVote { .. } => "pre-vote", } } } @@ -1226,7 +1227,7 @@ where async move { let tm_res = match kind { VoteRequestKind::Vote => C::AsyncRuntime::timeout(ttl, client.vote(req, option)).await, - VoteRequestKind::PreVote => { + VoteRequestKind::PreVote { .. } => { C::AsyncRuntime::timeout(ttl, client.pre_vote(req, option)).await } }; @@ -1257,11 +1258,9 @@ where resp, sender_vote: vote, }, - VoteRequestKind::PreVote => Notify::PreVoteResponse { - target, - resp, - sender_vote: vote, - }, + VoteRequestKind::PreVote { round } => { + Notify::PreVoteResponse { target, resp, round } + } }; let _ = tx.send(notify); } @@ -1449,21 +1448,18 @@ where } } - Notify::PreVoteResponse { - target, - resp, - sender_vote, - } => { + Notify::PreVoteResponse { target, resp, round } => { tracing::info!( resp = display(resp.summary()), target = display(&target), + round, "received Notify::PreVoteResponse: {}", func_name!() ); - if self.does_pre_vote_match(&sender_vote) { - self.engine.handle_pre_vote_resp(target, resp); - } + // Rounds in one term propose the same vote, so only the round identifier tells a + // delayed response from a current one. The engine ignores any other round. + self.engine.handle_pre_vote_resp(target, round, resp); } Notify::HigherVote { @@ -1834,21 +1830,6 @@ where /// round: each request is abandoned after `election_timeout_min`, a new round starts no earlier /// than one sampled election timeout after the previous one, and responses and ticks are /// delivered in order on the same channel. - fn does_pre_vote_match(&self, sender_vote: &Vote) -> bool { - let my_vote = self.engine.pre_candidate_ref().map(|x| x.vote_ref()); - - if Some(sender_vote) != my_vote { - tracing::warn!( - "A Pre-Vote response will be ignored because the round changed: sent by: {}; current round: {}", - sender_vote, - my_vote.display(), - ); - false - } else { - true - } - } - /// If a message is sent by a previous replication session but is received by current server /// state, it is a stale message and should be just ignored. fn does_replication_session_match( @@ -1998,8 +1979,8 @@ where Command::SendVote { vote_req } => { self.spawn_parallel_vote_requests(&vote_req, VoteRequestKind::Vote).await; } - Command::SendPreVote { vote_req } => { - self.spawn_parallel_vote_requests(&vote_req, VoteRequestKind::PreVote).await; + Command::SendPreVote { vote_req, round } => { + self.spawn_parallel_vote_requests(&vote_req, VoteRequestKind::PreVote { round }).await; } Command::ReplicateCommitted { committed } => { for node in self.replications.values() { diff --git a/openraft/src/engine/command.rs b/openraft/src/engine/command.rs index f887b46ed..35a35cc53 100644 --- a/openraft/src/engine/command.rs +++ b/openraft/src/engine/command.rs @@ -92,7 +92,11 @@ where C: RaftTypeConfig SendVote { vote_req: VoteRequest }, /// Send a Pre-Vote request to all other voters. - SendPreVote { vote_req: VoteRequest }, + SendPreVote { + vote_req: VoteRequest, + /// The Pre-Vote round that every response must name. + round: u64, + }, /// Purge log from the beginning to `upto`, inclusive. PurgeLog { upto: LogId }, @@ -141,7 +145,7 @@ where (Command::RebuildReplicationStreams { targets }, Command::RebuildReplicationStreams { targets: b }, ) => targets == b, (Command::SaveVote { vote }, Command::SaveVote { vote: b }) => vote == b, (Command::SendVote { vote_req }, Command::SendVote { vote_req: b }, ) => vote_req == b, - (Command::SendPreVote { vote_req }, Command::SendPreVote { vote_req: b }, ) => vote_req == b, + (Command::SendPreVote { vote_req, round }, Command::SendPreVote { vote_req: b, round: b_round }, ) => vote_req == b && round == b_round, (Command::PurgeLog { upto }, Command::PurgeLog { upto: b }) => upto == b, (Command::DeleteConflictLog { since }, Command::DeleteConflictLog { since: b }, ) => since == b, (Command::Respond { when, resp: send }, Command::Respond { when: b_when, resp: b }) => send == b && when == b_when, diff --git a/openraft/src/engine/engine_impl.rs b/openraft/src/engine/engine_impl.rs index f2cfe57fc..2347a8749 100644 --- a/openraft/src/engine/engine_impl.rs +++ b/openraft/src/engine/engine_impl.rs @@ -110,6 +110,12 @@ where C: RaftTypeConfig /// persisted, and it is dropped once a real election starts or this node follows another vote. pub(crate) pre_candidate: CandidateState, + /// The identifier of the latest Pre-Vote round. + /// + /// Rounds in one term propose the same hypothetical vote, so a response names its round by + /// this identifier instead. A response to any round but the one in flight is ignored. + pub(crate) pre_vote_round: u64, + /// A candidate whose vote request this node rejected because that candidate's log was behind /// this node's, recorded only while this node has no leader. /// @@ -143,6 +149,7 @@ where C: RaftTypeConfig leader: None, candidate: None, pre_candidate: None, + pre_vote_round: 0, stale_candidate: None, prepared_shutdown: None, output: EngineOutput::new(4096), @@ -347,6 +354,7 @@ where C: RaftTypeConfig let new_term = self.state.vote.leader_id().term + 1; let pre_vote = Vote::new(new_term, self.config.id.clone()); + self.pre_vote_round = self.pre_vote_round.wrapping_add(1); let pre_candidate = self.new_pre_candidate(pre_vote.clone()); tracing::info!("{}, new pre-candidate: {}", func_name!(), pre_candidate); @@ -362,6 +370,7 @@ where C: RaftTypeConfig self.output.push_command(Command::SendPreVote { vote_req: VoteRequest::new(pre_vote, last_log_id), + round: self.pre_vote_round, }); } @@ -553,22 +562,32 @@ where C: RaftTypeConfig VoteResponse::new(self.state.vote_ref(), self.state.last_log_id().cloned(), granted) } - /// Handle a Pre-Vote response. + /// Handle a response to the Pre-Vote round `round`. /// /// Count a granted Pre-Vote; once a quorum would grant, start the real election. A rejection /// that reports a greater log delays the next campaign, and one that reports a strictly higher /// vote catches this node up to that vote in non-committed form, as a rejected real vote does. + /// A response to any round but the one in flight is ignored. #[tracing::instrument(level = "debug", skip(self, resp))] - pub(crate) fn handle_pre_vote_resp(&mut self, target: C::NodeId, resp: VoteResponse) { + pub(crate) fn handle_pre_vote_resp(&mut self, target: C::NodeId, round: u64, resp: VoteResponse) { tracing::info!( resp = display(resp.summary()), target = display(&target), + round, my_vote = display(self.state.vote_ref()), my_last_log_id = display(self.state.last_log_id().summary()), "{}", func_name!() ); + if round != self.pre_vote_round { + tracing::info!( + current_round = self.pre_vote_round, + "ignore a response to an earlier Pre-Vote round" + ); + return; + } + let Some(pre_candidate) = self.pre_candidate.as_mut() else { // The Pre-Vote round has finished or been canceled; ignore the delayed response. return; diff --git a/openraft/src/engine/tests/pre_vote_test.rs b/openraft/src/engine/tests/pre_vote_test.rs index 108e89c35..027d6b348 100644 --- a/openraft/src/engine/tests/pre_vote_test.rs +++ b/openraft/src/engine/tests/pre_vote_test.rs @@ -72,7 +72,8 @@ fn test_pre_elect_changes_no_state() -> anyhow::Result<()> { // Only SendPreVote is emitted: no SaveVote and no SendVote. assert_eq!( vec![Command::SendPreVote { - vote_req: VoteRequest::new(Vote::new(2, 1), Some(log_id(1, 2, 3))) + vote_req: VoteRequest::new(Vote::new(2, 1), Some(log_id(1, 2, 3))), + round: 1, }], eng.output.take_commands() ); @@ -186,6 +187,7 @@ fn test_handle_pre_vote_resp_quorum_starts_real_election() -> anyhow::Result<()> eng.handle_pre_vote_resp( 3, + eng.pre_vote_round, VoteResponse::new(Vote::new_committed(1, 2), Some(log_id(1, 2, 3)), true), ); @@ -219,6 +221,7 @@ fn test_handle_pre_vote_resp_rejection_keeps_waiting() -> anyhow::Result<()> { eng.handle_pre_vote_resp( 3, + eng.pre_vote_round, VoteResponse::new(Vote::new_committed(1, 2), Some(log_id(1, 2, 3)), false), ); @@ -231,6 +234,7 @@ fn test_handle_pre_vote_resp_rejection_keeps_waiting() -> anyhow::Result<()> { tracing::info!("--- a rejection that reports a greater log delays the next campaign"); eng.handle_pre_vote_resp( 2, + eng.pre_vote_round, VoteResponse::new(Vote::new_committed(1, 2), Some(log_id(1, 2, 5)), false), ); assert!(eng.is_there_greater_log()); @@ -248,6 +252,7 @@ fn test_handle_pre_vote_resp_rejection_adopts_a_higher_vote() -> anyhow::Result< eng.handle_pre_vote_resp( 3, + eng.pre_vote_round, VoteResponse::new(Vote::new_committed(5, 3), Some(log_id(1, 2, 3)), false), ); @@ -268,7 +273,11 @@ fn test_handle_pre_vote_resp_rejection_with_equal_vote_keeps_the_round() -> anyh eng.output.take_commands(); let vote_before = *eng.state.vote_ref(); - eng.handle_pre_vote_resp(3, VoteResponse::new(vote_before, Some(log_id(1, 2, 3)), false)); + eng.handle_pre_vote_resp( + 3, + eng.pre_vote_round, + VoteResponse::new(vote_before, Some(log_id(1, 2, 3)), false), + ); assert!( eng.pre_candidate_ref().is_some(), @@ -294,7 +303,11 @@ fn test_accepted_vote_ends_the_pre_vote_round() -> anyhow::Result<()> { assert_eq!(leader_vote, *eng.state.vote_ref()); // A delayed grant for the ended round starts nothing. - eng.handle_pre_vote_resp(3, VoteResponse::new(leader_vote, Some(log_id(1, 2, 3)), true)); + eng.handle_pre_vote_resp( + 3, + eng.pre_vote_round, + VoteResponse::new(leader_vote, Some(log_id(1, 2, 3)), true), + ); assert!(eng.candidate_ref().is_none()); assert_eq!(leader_vote, *eng.state.vote_ref()); assert_eq!(0, eng.output.take_commands().len()); @@ -324,6 +337,7 @@ fn test_winning_election_ends_an_overlapping_pre_vote_round() -> anyhow::Result< eng.pre_elect(); eng.handle_pre_vote_resp( 2, + eng.pre_vote_round, VoteResponse::new(Vote::new_committed(1, 2), Some(log_id(1, 2, 3)), true), ); assert_eq!(Vote::new(2, 1), *eng.candidate_ref().unwrap().vote_ref()); @@ -341,7 +355,11 @@ fn test_winning_election_ends_an_overlapping_pre_vote_round() -> anyhow::Result< eng.output.take_commands(); // A delayed grant for round B starts no campaign on the leader. - eng.handle_pre_vote_resp(2, VoteResponse::new(Vote::new(2, 1), Some(log_id(1, 2, 3)), true)); + eng.handle_pre_vote_resp( + 2, + eng.pre_vote_round, + VoteResponse::new(Vote::new(2, 1), Some(log_id(1, 2, 3)), true), + ); assert_eq!(Vote::new_committed(2, 1), *eng.state.vote_ref()); assert_eq!(ServerState::Leader, eng.state.server_state); assert_eq!(0, eng.output.take_commands().len()); @@ -402,7 +420,7 @@ fn test_handle_pre_vote_resp_rejection_never_adopts_a_vote_for_this_node() -> an // A network that cannot ask the voter answers with a rejection of its own. If it echoed the // proposed vote, adopting it would vote for this node in a term it never campaigned in. - eng.handle_pre_vote_resp(3, VoteResponse::new(proposed, None, false)); + eng.handle_pre_vote_resp(3, eng.pre_vote_round, VoteResponse::new(proposed, None, false)); assert_eq!(vote_before, *eng.state.vote_ref()); assert!( @@ -424,6 +442,7 @@ fn test_delayed_pre_vote_grant_from_a_removed_voter_does_not_reach_a_later_round let mut eng = eng(m123()); eng.pre_elect(); eng.output.take_commands(); + let first_round = eng.pre_vote_round; let first_round_vote = *eng.pre_candidate_ref().unwrap().vote_ref(); // Leader 2 replicates a membership without voter 3 in its unchanged term, which ends the round. @@ -442,12 +461,11 @@ fn test_delayed_pre_vote_grant_from_a_removed_voter_does_not_reach_a_later_round *eng.pre_candidate_ref().unwrap().vote_ref(), "both rounds propose the same vote" ); + assert_ne!(first_round, eng.pre_vote_round, "each round has its own identifier"); // Voter 3's grant for the first round arrives now. - eng.handle_pre_vote_resp( - 3, - VoteResponse::new(Vote::new_committed(1, 2), Some(log_id(1, 2, 3)), true), - ); + let grant = VoteResponse::new(Vote::new_committed(1, 2), Some(log_id(1, 2, 3)), true); + eng.handle_pre_vote_resp(3, first_round, grant.clone()); assert!(eng.pre_candidate_ref().is_some(), "the new round keeps waiting"); assert_eq!( @@ -458,5 +476,16 @@ fn test_delayed_pre_vote_grant_from_a_removed_voter_does_not_reach_a_later_round assert_eq!(Vote::new_committed(1, 2), *eng.state.vote_ref()); assert_eq!(0, eng.output.take_commands().len()); + // Even a grant that names the new round cannot count for a node outside its quorum set. + eng.handle_pre_vote_resp(3, eng.pre_vote_round, grant); + + assert!(eng.pre_candidate_ref().is_some(), "the new round keeps waiting"); + assert_eq!( + btreeset! {1}, + eng.pre_candidate_ref().unwrap().granters().collect::>() + ); + assert!(eng.candidate_ref().is_none(), "no real election starts"); + assert_eq!(0, eng.output.take_commands().len()); + Ok(()) } diff --git a/openraft/src/proposer/candidate.rs b/openraft/src/proposer/candidate.rs index ed9c537f6..a842983d4 100644 --- a/openraft/src/proposer/candidate.rs +++ b/openraft/src/proposer/candidate.rs @@ -92,8 +92,20 @@ where } /// Grant the vote by a node. + /// + /// A grant from a node outside this round's quorum set is ignored: it can never count toward + /// a quorum, and a delayed or misdirected response must never stop this node. pub(crate) fn grant_by(&mut self, target: &C::NodeId) -> bool { - let granted = *self.progress.update(target, true).expect("target not in quorum set"); + let granted = match self.progress.update(target, true) { + Ok(granted) => *granted, + Err(_) => { + tracing::warn!( + target = display(target), + "ignore a grant from a node that does not vote in this round" + ); + return false; + } + }; tracing::info!(voting = display(&self), "{}", func_name!()); From e1cd6a3c98063d392a7d2ee5bd34f6a45a98ced8 Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Fri, 2 Oct 2026 11:09:46 -0600 Subject: [PATCH 21/36] Test: a classic leader that cannot send must not block an election Signed-off-by: VerifiedOrganic --- tests/tests/elect/main.rs | 1 + .../tests/elect/t24_classic_leader_one_way.rs | 93 +++++++++++++++++++ 2 files changed, 94 insertions(+) create mode 100644 tests/tests/elect/t24_classic_leader_one_way.rs diff --git a/tests/tests/elect/main.rs b/tests/tests/elect/main.rs index 26a92670a..2aa45614d 100644 --- a/tests/tests/elect/main.rs +++ b/tests/tests/elect/main.rs @@ -15,3 +15,4 @@ mod t20_lease_and_timeout_overlap; mod t21_pre_vote; mod t22_one_way_isolation; mod t23_classic_voter_stale_log; +mod t24_classic_leader_one_way; diff --git a/tests/tests/elect/t24_classic_leader_one_way.rs b/tests/tests/elect/t24_classic_leader_one_way.rs new file mode 100644 index 000000000..2d3fb392f --- /dev/null +++ b/tests/tests/elect/t24_classic_leader_one_way.rs @@ -0,0 +1,93 @@ +use std::sync::Arc; +use std::time::Duration; + +use anyhow::Result; +use maplit::btreeset; +use openraft::Config; +use openraft::ServerState; + +use crate::fixtures::init_default_ut_tracing; +use crate::fixtures::Direction; +use crate::fixtures::RPCErrorType; +use crate::fixtures::RaftRouter; + +/// A voter that cannot answer Pre-Vote and still believes it leads does not block an election. +/// +/// Node 0 cannot answer Pre-Vote, as a voter of a release without it cannot, and leads. It is +/// cut off, nodes 1 and 2 elect a successor in a later term, and the successor is lost too. +/// Node 0 is reachable again, but nothing it sends arrives, so it never learns of the later term +/// and keeps believing it leads: it never campaigns. The remaining voter can still win a classic +/// vote with node 0's grant. If it ran Pre-Vote, node 0 could never answer, and no leader would +/// ever be elected. +#[async_entry::test(worker_threads = 8, init = "init_default_ut_tracing()", tracing_span = "debug")] +async fn classic_leader_that_cannot_send_does_not_block_an_election() -> Result<()> { + let config = Arc::new( + Config { + heartbeat_interval: 50, + election_timeout_min: 500, + election_timeout_max: 600, + enable_pre_vote: Some(true), + ..Default::default() + } + .validate()?, + ); + let mut router = RaftRouter::new(config.clone()); + + tracing::info!("--- create cluster of 0,1,2; node 0 becomes leader"); + let mut log_index = router.new_cluster(btreeset! {0,1,2}, btreeset! {}).await?; + let n0 = router.get_raft_handle(&0)?; + n0.wait(timeout()).state(ServerState::Leader, "node 0 is the initial leader").await?; + + tracing::info!("--- node 0 cannot answer Pre-Vote and campaigns without it"); + n0.runtime_config().pre_vote(false); + router.set_pre_vote_unsupported(0, true); + + tracing::info!("--- node 0 is cut off; nodes 1 and 2 elect a successor"); + router.set_network_error(0, true); + let election = Some(Duration::from_millis(config.election_timeout_max * 10)); + let m1 = router + .wait(&1, election) + .metrics( + |m| m.current_leader.is_some_and(|l| l != 0), + "nodes 1 and 2 elect a successor", + ) + .await?; + let successor = m1.current_leader.unwrap(); + let remaining = if successor == 1 { 2 } else { 1 }; + router.client_request_many(successor, "successor", 1).await?; + log_index += 2; + router + .wait(&remaining, timeout()) + .applied_index(Some(log_index), "the remaining voter has the successor's log") + .await?; + assert_eq!( + ServerState::Leader, + n0.metrics().borrow().state, + "the cut-off node 0 still believes it leads" + ); + + tracing::info!( + "--- the successor {} is lost; node 0 receives again, but nothing it sends arrives", + successor + ); + router.set_network_error(successor, true); + router.set_rpc_failure(0, Direction::NetRecv, None); + router.set_rpc_failure(0, Direction::NetSend, Some(RPCErrorType::NetworkError)); + + router + .wait(&remaining, election) + .state(ServerState::Leader, "the remaining voter is elected with node 0's vote") + .await?; + n0.wait(timeout()) + .metrics( + |m| m.state == ServerState::Follower && m.current_leader == Some(remaining), + "node 0 follows the remaining voter", + ) + .await?; + + Ok(()) +} + +fn timeout() -> Option { + Some(Duration::from_millis(3_000)) +} From f47be8ac99e2ca9a78d7758ed5951d5e120fca3f Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Fri, 2 Oct 2026 11:13:08 -0600 Subject: [PATCH 22/36] Fix: run the classic election when a voter cannot answer Pre-Vote A voter that cannot answer Pre-Vote, such as a voter of a release without it, was counted as rejecting. Pre-Vote then could not reach a quorum that needed such a voter. When that voter also believed it still led, because nothing it sent arrived, it never campaigned either, and no leader was ever elected. RaftNetwork::pre_vote now returns a PreVoteReply. A network sends the request and returns Answered only for a target that is positively known to answer Pre-Vote: the capability was negotiated on the connection, or the peer is an in-process peer that passes it to Raft::pre_vote. For any other reachable target it sends nothing and returns Unsupported, and the node runs the classic election for that campaign at once. The default implementation returns Unsupported, so a network that does not implement Pre-Vote always runs the classic election. An unreachable target is still an error: it can grant neither a Pre-Vote nor a vote, so it neither counts as a grant nor forces the classic election. Signed-off-by: VerifiedOrganic --- openraft/src/core/notify.rs | 12 +- openraft/src/core/raft_core.rs | 91 ++++++++------- openraft/src/engine/engine_impl.rs | 40 +++++-- openraft/src/engine/tests/pre_vote_test.rs | 108 +++++++++++++++--- openraft/src/network/network.rs | 25 ++-- openraft/src/raft/message/mod.rs | 1 + openraft/src/raft/message/vote.rs | 27 +++++ openraft/src/raft/mod.rs | 1 + .../elect/t23_classic_voter_stale_log.rs | 10 +- tests/tests/fixtures/mod.rs | 13 ++- 10 files changed, 239 insertions(+), 89 deletions(-) diff --git a/openraft/src/core/notify.rs b/openraft/src/core/notify.rs index 7ba77c10a..ce5e89585 100644 --- a/openraft/src/core/notify.rs +++ b/openraft/src/core/notify.rs @@ -1,5 +1,6 @@ use crate::core::sm; use crate::error::Fatal; +use crate::raft::PreVoteReply; use crate::raft::VoteResponse; use crate::replication; use crate::MessageSummary; @@ -22,7 +23,7 @@ where C: RaftTypeConfig PreVoteResponse { target: C::NodeId, - resp: VoteResponse, + reply: PreVoteReply, /// The Pre-Vote round that sent the request. round: u64, @@ -81,13 +82,8 @@ where C: RaftTypeConfig } => { format!("VoteResponse: from: {}: {}, res-vote: {}", target, resp.summary(), vote) } - Self::PreVoteResponse { target, resp, round } => { - format!( - "PreVoteResponse: from: {}: {}, round: {}", - target, - resp.summary(), - round - ) + Self::PreVoteResponse { target, reply, round } => { + format!("PreVoteResponse: from: {}: {}, round: {}", target, reply, round) } Self::HigherVote { ref target, diff --git a/openraft/src/core/raft_core.rs b/openraft/src/core/raft_core.rs index 15fd84dac..a8d8f7263 100644 --- a/openraft/src/core/raft_core.rs +++ b/openraft/src/core/raft_core.rs @@ -1225,51 +1225,58 @@ where let vote = vote.clone(); async move { - let tm_res = match kind { - VoteRequestKind::Vote => C::AsyncRuntime::timeout(ttl, client.vote(req, option)).await, - VoteRequestKind::PreVote { .. } => { - C::AsyncRuntime::timeout(ttl, client.pre_vote(req, option)).await - } - }; - let res = match tm_res { - Ok(res) => res, - - Err(_timeout) => { - let timeout_err = Timeout { - action: RPCTypes::Vote, - id, - target: target.clone(), - timeout: ttl, - }; - tracing::error!( - {error = %timeout_err, target = display(&target)}, - "timeout while requesting {}", - kind.as_str() - ); - return; - } + let timeout_err = || Timeout { + action: RPCTypes::Vote, + id: id.clone(), + target: target.clone(), + timeout: ttl, }; - - match res { - Ok(resp) => { - let notify = match kind { - VoteRequestKind::Vote => Notify::VoteResponse { - target, + let notify = match kind { + VoteRequestKind::Vote => { + match C::AsyncRuntime::timeout(ttl, client.vote(req, option)).await { + Ok(Ok(resp)) => Notify::VoteResponse { + target: target.clone(), resp, sender_vote: vote, }, - VoteRequestKind::PreVote { round } => { - Notify::PreVoteResponse { target, resp, round } + Ok(Err(err)) => { + tracing::error!({error=%err, target=display(&target)}, "while requesting vote"); + return; + } + Err(_timeout) => { + tracing::error!( + {error = %timeout_err(), target = display(&target)}, + "timeout while requesting vote" + ); + return; } - }; - let _ = tx.send(notify); + } } - Err(err) => tracing::error!( - {error=%err, target=display(&target)}, - "while requesting {}", - kind.as_str() - ), - } + VoteRequestKind::PreVote { round } => { + match C::AsyncRuntime::timeout(ttl, client.pre_vote(req, option)).await { + Ok(Ok(reply)) => Notify::PreVoteResponse { + target: target.clone(), + reply, + round, + }, + Ok(Err(err)) => { + tracing::error!( + {error=%err, target=display(&target)}, + "while requesting pre-vote" + ); + return; + } + Err(_timeout) => { + tracing::error!( + {error = %timeout_err(), target = display(&target)}, + "timeout while requesting pre-vote" + ); + return; + } + } + } + }; + let _ = tx.send(notify); } } .instrument(tracing::debug_span!( @@ -1448,9 +1455,9 @@ where } } - Notify::PreVoteResponse { target, resp, round } => { + Notify::PreVoteResponse { target, reply, round } => { tracing::info!( - resp = display(resp.summary()), + reply = display(&reply), target = display(&target), round, "received Notify::PreVoteResponse: {}", @@ -1459,7 +1466,7 @@ where // Rounds in one term propose the same vote, so only the round identifier tells a // delayed response from a current one. The engine ignores any other round. - self.engine.handle_pre_vote_resp(target, round, resp); + self.engine.handle_pre_vote_resp(target, round, reply); } Notify::HigherVote { diff --git a/openraft/src/engine/engine_impl.rs b/openraft/src/engine/engine_impl.rs index 2347a8749..c7250e0eb 100644 --- a/openraft/src/engine/engine_impl.rs +++ b/openraft/src/engine/engine_impl.rs @@ -37,6 +37,7 @@ use crate::proposer::LeaderQuorumSet; use crate::proposer::LeaderState; use crate::raft::responder::Responder; use crate::raft::AppendEntriesResponse; +use crate::raft::PreVoteReply; use crate::raft::SnapshotResponse; use crate::raft::VoteRequest; use crate::raft::VoteResponse; @@ -562,16 +563,19 @@ where C: RaftTypeConfig VoteResponse::new(self.state.vote_ref(), self.state.last_log_id().cloned(), granted) } - /// Handle a response to the Pre-Vote round `round`. + /// Handle a reply to the Pre-Vote round `round`. /// /// Count a granted Pre-Vote; once a quorum would grant, start the real election. A rejection /// that reports a greater log delays the next campaign, and one that reports a strictly higher /// vote catches this node up to that vote in non-committed form, as a rejected real vote does. - /// A response to any round but the one in flight is ignored. - #[tracing::instrument(level = "debug", skip(self, resp))] - pub(crate) fn handle_pre_vote_resp(&mut self, target: C::NodeId, round: u64, resp: VoteResponse) { + /// + /// A voter that cannot answer Pre-Vote ends the round, and this node runs the classic election + /// for this campaign at once: Pre-Vote is used only while every voter that can be reached + /// answers it. A reply to any round but the one in flight is ignored. + #[tracing::instrument(level = "debug", skip(self, reply))] + pub(crate) fn handle_pre_vote_resp(&mut self, target: C::NodeId, round: u64, reply: PreVoteReply) { tracing::info!( - resp = display(resp.summary()), + reply = display(&reply), target = display(&target), round, my_vote = display(self.state.vote_ref()), @@ -583,16 +587,36 @@ where C: RaftTypeConfig if round != self.pre_vote_round { tracing::info!( current_round = self.pre_vote_round, - "ignore a response to an earlier Pre-Vote round" + "ignore a reply to an earlier Pre-Vote round" ); return; } let Some(pre_candidate) = self.pre_candidate.as_mut() else { - // The Pre-Vote round has finished or been canceled; ignore the delayed response. + // The Pre-Vote round has finished or been canceled; ignore the delayed reply. return; }; + let resp = match reply { + PreVoteReply::Answered(resp) => resp, + PreVoteReply::Unsupported => { + if !self.state.membership_state.effective().is_voter(&target) { + tracing::warn!("ignore a Pre-Vote reply from a node that does not vote"); + return; + } + // The voter can still grant a vote, but it can never grant a Pre-Vote: without it + // a Pre-Vote quorum may never form. Campaign as a node without Pre-Vote does. + self.pre_candidate = None; + if self.leader.is_some() || !self.state.membership_state.effective().is_voter(&self.config.id) { + tracing::info!("a voter cannot answer Pre-Vote, but this node leads or no longer votes"); + return; + } + tracing::info!("a voter cannot answer Pre-Vote; start the classic election"); + self.elect(); + return; + } + }; + // The responder does not adopt the Pre-Vote, so a grant is read from `vote_granted`. if resp.vote_granted { if pre_candidate.grant_by(&target) { @@ -619,7 +643,7 @@ where C: RaftTypeConfig // Catch up to a strictly higher vote, never as committed. An equal vote is left alone: it // would end the in-flight round without new information. A vote for this node is not // adopted either: no voter holds one for a term this node did not campaign in, so it can - // only be a network's own answer, and adopting it would vote for this node unsent. + // only be a faulty answer, and adopting it would vote for this node unsent. if &resp.vote > self.state.vote_ref() && resp.vote.leader_id().voted_for() != Some(self.config.id.clone()) { let mut vote = resp.vote.clone(); vote.committed = false; diff --git a/openraft/src/engine/tests/pre_vote_test.rs b/openraft/src/engine/tests/pre_vote_test.rs index 027d6b348..48d8be0c9 100644 --- a/openraft/src/engine/tests/pre_vote_test.rs +++ b/openraft/src/engine/tests/pre_vote_test.rs @@ -11,6 +11,7 @@ use crate::engine::Command; use crate::engine::Engine; use crate::engine::LogIdList; use crate::progress::Progress; +use crate::raft::PreVoteReply; use crate::raft::VoteRequest; use crate::raft::VoteResponse; use crate::testing::log_id; @@ -188,7 +189,11 @@ fn test_handle_pre_vote_resp_quorum_starts_real_election() -> anyhow::Result<()> eng.handle_pre_vote_resp( 3, eng.pre_vote_round, - VoteResponse::new(Vote::new_committed(1, 2), Some(log_id(1, 2, 3)), true), + PreVoteReply::Answered(VoteResponse::new( + Vote::new_committed(1, 2), + Some(log_id(1, 2, 3)), + true, + )), ); assert!( @@ -222,7 +227,11 @@ fn test_handle_pre_vote_resp_rejection_keeps_waiting() -> anyhow::Result<()> { eng.handle_pre_vote_resp( 3, eng.pre_vote_round, - VoteResponse::new(Vote::new_committed(1, 2), Some(log_id(1, 2, 3)), false), + PreVoteReply::Answered(VoteResponse::new( + Vote::new_committed(1, 2), + Some(log_id(1, 2, 3)), + false, + )), ); assert!(eng.pre_candidate_ref().is_some(), "the round is still in flight"); @@ -235,7 +244,11 @@ fn test_handle_pre_vote_resp_rejection_keeps_waiting() -> anyhow::Result<()> { eng.handle_pre_vote_resp( 2, eng.pre_vote_round, - VoteResponse::new(Vote::new_committed(1, 2), Some(log_id(1, 2, 5)), false), + PreVoteReply::Answered(VoteResponse::new( + Vote::new_committed(1, 2), + Some(log_id(1, 2, 5)), + false, + )), ); assert!(eng.is_there_greater_log()); assert!(eng.pre_candidate_ref().is_some()); @@ -253,7 +266,11 @@ fn test_handle_pre_vote_resp_rejection_adopts_a_higher_vote() -> anyhow::Result< eng.handle_pre_vote_resp( 3, eng.pre_vote_round, - VoteResponse::new(Vote::new_committed(5, 3), Some(log_id(1, 2, 3)), false), + PreVoteReply::Answered(VoteResponse::new( + Vote::new_committed(5, 3), + Some(log_id(1, 2, 3)), + false, + )), ); // Catch up to the strictly higher term, never as committed, and end the round. @@ -276,7 +293,7 @@ fn test_handle_pre_vote_resp_rejection_with_equal_vote_keeps_the_round() -> anyh eng.handle_pre_vote_resp( 3, eng.pre_vote_round, - VoteResponse::new(vote_before, Some(log_id(1, 2, 3)), false), + PreVoteReply::Answered(VoteResponse::new(vote_before, Some(log_id(1, 2, 3)), false)), ); assert!( @@ -306,7 +323,7 @@ fn test_accepted_vote_ends_the_pre_vote_round() -> anyhow::Result<()> { eng.handle_pre_vote_resp( 3, eng.pre_vote_round, - VoteResponse::new(leader_vote, Some(log_id(1, 2, 3)), true), + PreVoteReply::Answered(VoteResponse::new(leader_vote, Some(log_id(1, 2, 3)), true)), ); assert!(eng.candidate_ref().is_none()); assert_eq!(leader_vote, *eng.state.vote_ref()); @@ -338,7 +355,11 @@ fn test_winning_election_ends_an_overlapping_pre_vote_round() -> anyhow::Result< eng.handle_pre_vote_resp( 2, eng.pre_vote_round, - VoteResponse::new(Vote::new_committed(1, 2), Some(log_id(1, 2, 3)), true), + PreVoteReply::Answered(VoteResponse::new( + Vote::new_committed(1, 2), + Some(log_id(1, 2, 3)), + true, + )), ); assert_eq!(Vote::new(2, 1), *eng.candidate_ref().unwrap().vote_ref()); // The local vote is granted once it is persisted. @@ -358,7 +379,7 @@ fn test_winning_election_ends_an_overlapping_pre_vote_round() -> anyhow::Result< eng.handle_pre_vote_resp( 2, eng.pre_vote_round, - VoteResponse::new(Vote::new(2, 1), Some(log_id(1, 2, 3)), true), + PreVoteReply::Answered(VoteResponse::new(Vote::new(2, 1), Some(log_id(1, 2, 3)), true)), ); assert_eq!(Vote::new_committed(2, 1), *eng.state.vote_ref()); assert_eq!(ServerState::Leader, eng.state.server_state); @@ -418,9 +439,13 @@ fn test_handle_pre_vote_resp_rejection_never_adopts_a_vote_for_this_node() -> an let proposed = *eng.pre_candidate_ref().unwrap().vote_ref(); assert!(proposed > vote_before); - // A network that cannot ask the voter answers with a rejection of its own. If it echoed the - // proposed vote, adopting it would vote for this node in a term it never campaigned in. - eng.handle_pre_vote_resp(3, eng.pre_vote_round, VoteResponse::new(proposed, None, false)); + // A faulty answer that echoes the proposed vote must not be adopted: that would vote for this + // node in a term it never campaigned in. + eng.handle_pre_vote_resp( + 3, + eng.pre_vote_round, + PreVoteReply::Answered(VoteResponse::new(proposed, None, false)), + ); assert_eq!(vote_before, *eng.state.vote_ref()); assert!( @@ -465,7 +490,7 @@ fn test_delayed_pre_vote_grant_from_a_removed_voter_does_not_reach_a_later_round // Voter 3's grant for the first round arrives now. let grant = VoteResponse::new(Vote::new_committed(1, 2), Some(log_id(1, 2, 3)), true); - eng.handle_pre_vote_resp(3, first_round, grant.clone()); + eng.handle_pre_vote_resp(3, first_round, PreVoteReply::Answered(grant.clone())); assert!(eng.pre_candidate_ref().is_some(), "the new round keeps waiting"); assert_eq!( @@ -477,7 +502,7 @@ fn test_delayed_pre_vote_grant_from_a_removed_voter_does_not_reach_a_later_round assert_eq!(0, eng.output.take_commands().len()); // Even a grant that names the new round cannot count for a node outside its quorum set. - eng.handle_pre_vote_resp(3, eng.pre_vote_round, grant); + eng.handle_pre_vote_resp(3, eng.pre_vote_round, PreVoteReply::Answered(grant)); assert!(eng.pre_candidate_ref().is_some(), "the new round keeps waiting"); assert_eq!( @@ -489,3 +514,60 @@ fn test_delayed_pre_vote_grant_from_a_removed_voter_does_not_reach_a_later_round Ok(()) } + +/// A voter that cannot answer Pre-Vote ends the round, and this node runs the classic election. +/// +/// Such a voter can still grant a vote but never a Pre-Vote, so a Pre-Vote quorum that needs it +/// would never form. +#[test] +fn test_unsupported_pre_vote_runs_the_classic_election() -> anyhow::Result<()> { + let mut eng = eng(m123()); + eng.pre_elect(); + eng.output.take_commands(); + + eng.handle_pre_vote_resp(3, eng.pre_vote_round, PreVoteReply::Unsupported); + + assert!(eng.pre_candidate_ref().is_none(), "the classic election ends the round"); + assert_eq!( + Vote::new(2, 1), + *eng.state.vote_ref(), + "the classic election bumps the term" + ); + assert!(eng.candidate_ref().is_some()); + assert_eq!(ServerState::Candidate, eng.state.server_state); + + let commands = eng.output.take_commands(); + assert!(commands.contains(&Command::SaveVote { vote: Vote::new(2, 1) })); + assert!(commands.contains(&Command::SendVote { + vote_req: VoteRequest::new(Vote::new(2, 1), Some(log_id(1, 2, 3))), + })); + + Ok(()) +} + +/// Only the round in flight, and only a voter, can make this node run the classic election. +#[test] +fn test_unsupported_pre_vote_of_an_ended_round_or_a_non_voter_starts_nothing() -> anyhow::Result<()> { + let mut eng = eng(m12()); + eng.pre_elect(); + eng.output.take_commands(); + let vote_before = *eng.state.vote_ref(); + + // Node 3 is not a voter. + eng.handle_pre_vote_resp(3, eng.pre_vote_round, PreVoteReply::Unsupported); + assert!(eng.pre_candidate_ref().is_some(), "the round keeps waiting"); + + // A reply to an earlier round. + eng.handle_pre_vote_resp(2, eng.pre_vote_round - 1, PreVoteReply::Unsupported); + assert!(eng.pre_candidate_ref().is_some(), "the round keeps waiting"); + + // A reply after a heartbeat ended the round. + eng.vote_handler().update_vote(&vote_before)?; + eng.handle_pre_vote_resp(2, eng.pre_vote_round, PreVoteReply::Unsupported); + + assert!(eng.candidate_ref().is_none(), "no election starts"); + assert_eq!(vote_before, *eng.state.vote_ref()); + assert_eq!(0, eng.output.take_commands().len()); + + Ok(()) +} diff --git a/openraft/src/network/network.rs b/openraft/src/network/network.rs index d8cdd7bb9..ad134f2f2 100644 --- a/openraft/src/network/network.rs +++ b/openraft/src/network/network.rs @@ -12,6 +12,7 @@ use crate::network::rpc_option::RPCOption; use crate::network::Backoff; use crate::raft::AppendEntriesRequest; use crate::raft::AppendEntriesResponse; +use crate::raft::PreVoteReply; use crate::raft::SnapshotResponse; use crate::raft::VoteRequest; use crate::raft::VoteResponse; @@ -86,18 +87,28 @@ where C: RaftTypeConfig /// a vote or changing its term. It is only sent when /// [`Config::enable_pre_vote`](`crate::Config::enable_pre_vote`) is enabled. /// - /// The default implementation reports a granted Pre-Vote without contacting the target, so a - /// network that does not implement it makes Pre-Vote a no-op and elections proceed as without - /// it. An implementation that cannot reach the target must return an error instead: an error - /// is never counted as a grant, so a voter that is cut off cannot assemble a quorum. + /// Send the request and return [`PreVoteReply::Answered`] only for a target that is positively + /// known to answer Pre-Vote: the capability was negotiated on the connection to it, or it is + /// an in-process peer that passes the request to [`Raft::pre_vote()`]. For any other target + /// that can be reached, such as a voter of a release without Pre-Vote, send nothing and return + /// [`PreVoteReply::Unsupported`]: the node then runs the classic election for this campaign, + /// because such a voter can grant a vote but can never answer the Pre-Vote. The default + /// implementation returns `Unsupported`, so a network that does not implement Pre-Vote always + /// runs the classic election. + /// + /// Return an error for a target that cannot be reached. It is never counted as a grant, so a + /// voter that is cut off cannot assemble a quorum, and it does not force the classic election: + /// a voter that cannot be reached can grant neither a Pre-Vote nor a vote. Never return an + /// error for a target that answers votes but not Pre-Vote, or no Pre-Vote quorum that needs it + /// can ever be reached. /// /// [`Raft::pre_vote()`]: crate::Raft::pre_vote async fn pre_vote( &mut self, - rpc: VoteRequest, + _rpc: VoteRequest, _option: RPCOption, - ) -> Result, RPCError>> { - Ok(VoteResponse::new(rpc.vote, None, true)) + ) -> Result, RPCError>> { + Ok(PreVoteReply::Unsupported) } /// Send a complete Snapshot to the target. diff --git a/openraft/src/raft/message/mod.rs b/openraft/src/raft/message/mod.rs index 6625364d5..27c692c6f 100644 --- a/openraft/src/raft/message/mod.rs +++ b/openraft/src/raft/message/mod.rs @@ -19,5 +19,6 @@ pub use install_snapshot::InstallSnapshotResponse; pub use install_snapshot::SnapshotResponse; pub use transfer_leader::TransferLeaderError; pub use transfer_leader::TransferLeaderRequest; +pub use vote::PreVoteReply; pub use vote::VoteRequest; pub use vote::VoteResponse; diff --git a/openraft/src/raft/message/vote.rs b/openraft/src/raft/message/vote.rs index c2964641c..c14ccf0da 100644 --- a/openraft/src/raft/message/vote.rs +++ b/openraft/src/raft/message/vote.rs @@ -91,3 +91,30 @@ where NID: NodeId ) } } + +/// The outcome of a Pre-Vote RPC, as a network reports it. +/// +/// See [`RaftNetwork::pre_vote()`](`crate::network::RaftNetwork::pre_vote`). +#[derive(Debug, Clone, PartialEq, Eq)] +#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize), serde(bound = ""))] +pub enum PreVoteReply { + /// The target answered whether it would grant the vote. + Answered(VoteResponse), + + /// The target is not positively known to answer Pre-Vote, so nothing was sent to it. + /// + /// Such a target can still grant a vote, so the node runs the classic election for this + /// campaign instead of a Pre-Vote round that could never reach a quorum without it. + Unsupported, +} + +impl fmt::Display for PreVoteReply +where NID: NodeId +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Answered(resp) => write!(f, "Answered{}", resp), + Self::Unsupported => write!(f, "Unsupported"), + } + } +} diff --git a/openraft/src/raft/mod.rs b/openraft/src/raft/mod.rs index 7c2b918ff..f7473a041 100644 --- a/openraft/src/raft/mod.rs +++ b/openraft/src/raft/mod.rs @@ -38,6 +38,7 @@ pub use message::ClientWriteResponse; pub use message::ClientWriteResult; pub use message::InstallSnapshotRequest; pub use message::InstallSnapshotResponse; +pub use message::PreVoteReply; pub use message::SnapshotResponse; pub use message::TransferLeaderError; pub use message::TransferLeaderRequest; diff --git a/tests/tests/elect/t23_classic_voter_stale_log.rs b/tests/tests/elect/t23_classic_voter_stale_log.rs index cc5f25f4c..de72aa7af 100644 --- a/tests/tests/elect/t23_classic_voter_stale_log.rs +++ b/tests/tests/elect/t23_classic_voter_stale_log.rs @@ -28,11 +28,11 @@ fn pre_vote_config() -> Result> { /// Voters that cannot answer Pre-Vote, such as voters of a release without it, and that miss the /// last entries do not block an election. /// -/// The survivors with the up-to-date log run Pre-Vote, and the network answers their Pre-Vote to a -/// classic voter as a rejection, so they cannot reach a Pre-Vote quorum without those voters. The -/// classic voters campaign with the classic vote, but their logs are behind, so they cannot win -/// either. A survivor that rejects such a candidate for its log must campaign itself, without -/// Pre-Vote and in a term above that candidate's, or no leader is ever elected. +/// The survivors with the up-to-date log can win only with a classic voter's vote, and a classic +/// voter can never grant a Pre-Vote: the network reports it as unsupported, so the survivors run +/// the classic election instead. The classic voters campaign with the classic vote too, but their +/// logs are behind, so they cannot win. A survivor campaigns in a term above every candidate it +/// rejected for its log, so their terms do not hold its campaign back. async fn classic_voters_with_stale_logs_do_not_block_an_election(voter_count: u64) -> Result<()> { let config = pre_vote_config()?; let mut router = RaftRouter::new(config.clone()); diff --git a/tests/tests/fixtures/mod.rs b/tests/tests/fixtures/mod.rs index 3d700db3c..bfca2dc96 100644 --- a/tests/tests/fixtures/mod.rs +++ b/tests/tests/fixtures/mod.rs @@ -43,6 +43,7 @@ use openraft::raft::AppendEntriesResponse; use openraft::raft::ClientWriteResponse; use openraft::raft::InstallSnapshotRequest; use openraft::raft::InstallSnapshotResponse; +use openraft::raft::PreVoteReply; use openraft::raft::VoteRequest; use openraft::raft::VoteResponse; use openraft::storage::Adaptor; @@ -267,7 +268,7 @@ pub struct TypedRaftRouter { rpc_count: Arc>>, /// Nodes that cannot answer Pre-Vote, such as voters of a release without it. A Pre-Vote to - /// one of them is answered as a rejection without reaching it. + /// one of them is reported as unsupported without reaching it. pre_vote_unsupported: Arc>>, /// A hook function to be called when before an RPC is sent to target node. @@ -571,7 +572,7 @@ impl TypedRaftRouter { } /// Set whether to emit a specified rpc error when sending to/receiving from a node. - /// Answer every Pre-Vote to `id` as a rejection without reaching it, as a network does for a + /// Report every Pre-Vote to `id` as unsupported without reaching it, as a network does for a /// voter of a release without Pre-Vote. pub fn set_pre_vote_unsupported(&self, id: MemNodeId, unsupported: bool) { let mut nodes = self.pre_vote_unsupported.lock().unwrap(); @@ -1186,7 +1187,7 @@ impl RaftNetwork for RaftRouterNetwork { &mut self, rpc: VoteRequest, _option: RPCOption, - ) -> Result, RPCError>> { + ) -> Result, RPCError>> { let from_id = rpc.vote.leader_id().voted_for().unwrap(); self.owner.count_rpc(RPCTypes::Vote); @@ -1195,8 +1196,8 @@ impl RaftNetwork for RaftRouterNetwork { self.owner.rand_send_delay().await; if self.owner.pre_vote_unsupported.lock().unwrap().contains(&self.target) { - // A rejection that carries no vote to catch up to. - return Ok(VoteResponse::new(Vote::new(0, from_id), None, false)); + // Send nothing to a voter that cannot answer Pre-Vote. + return Ok(PreVoteReply::Unsupported); } let node = self.owner.get_raft_handle(&self.target)?; @@ -1204,7 +1205,7 @@ impl RaftNetwork for RaftRouterNetwork { let resp = node.pre_vote(rpc).await; let resp = resp.map_err(|e| RemoteError::new(self.target, e))?; - Ok(resp) + Ok(PreVoteReply::Answered(resp)) } } From 1ca001b59b1fdcf2ef126c064247f57f861aaa75 Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Fri, 2 Oct 2026 11:16:56 -0600 Subject: [PATCH 23/36] Refactor: remove the stale-candidate rule that Pre-Vote gating replaces The rule let a node that rejected a candidate for its stale log campaign without Pre-Vote, in a term above that candidate's. It existed because a voter that cannot answer Pre-Vote was counted as rejecting, so the up-to-date voters could never reach a Pre-Vote quorum that needed it. Such a voter now makes the node run the classic election, so nothing waits on its Pre-Vote, and a stale candidate's term only delays the classic campaign as it does in any classic election: its rejection reports that term and the next campaign is above it. The regression for voters that cannot answer Pre-Vote and miss the last entries passes without the rule, so it is removed and the vote rules are again those of the base release. Signed-off-by: VerifiedOrganic --- openraft/src/core/raft_core.rs | 9 +-- openraft/src/engine/engine_impl.rs | 55 +-------------- openraft/src/engine/tests/elect_test.rs | 28 -------- .../src/engine/tests/handle_vote_req_test.rs | 68 ------------------- 4 files changed, 4 insertions(+), 156 deletions(-) diff --git a/openraft/src/core/raft_core.rs b/openraft/src/core/raft_core.rs index a8d8f7263..5dd604dd1 100644 --- a/openraft/src/core/raft_core.rs +++ b/openraft/src/core/raft_core.rs @@ -1738,14 +1738,9 @@ where tracing::info!("election timeout passed, check if it is a voter for election"); } - // A node that rejected a candidate for its log, and has heard from no leader for one - // election timeout since, campaigns without Pre-Vote: see `Engine::stale_candidate`. - let stale_candidate_due = - self.engine.stale_candidate_campaign_due(now, self.engine.config.timer_config.election_timeout); - // Pre-Vote applies to multiple voters only: a single voter always wins its own Pre-Vote. - let pre_vote = - self.runtime_config.enable_pre_vote.load(Ordering::Relaxed) && voter_count > 1 && !stale_candidate_due; + // A voter that cannot answer it makes this node run the classic election instead. + let pre_vote = self.runtime_config.enable_pre_vote.load(Ordering::Relaxed) && voter_count > 1; if pre_vote { // A Pre-Vote round does not update the vote, so the expired election timer would diff --git a/openraft/src/engine/engine_impl.rs b/openraft/src/engine/engine_impl.rs index c7250e0eb..c44818713 100644 --- a/openraft/src/engine/engine_impl.rs +++ b/openraft/src/engine/engine_impl.rs @@ -44,7 +44,6 @@ use crate::raft::VoteResponse; use crate::raft_state::LogStateReader; use crate::raft_state::RaftState; use crate::summary::MessageSummary; -use crate::type_config::alias::InstantOf; use crate::type_config::alias::ResponderOf; use crate::type_config::alias::SnapshotDataOf; use crate::type_config::TypeConfigExt; @@ -58,18 +57,6 @@ use crate::Snapshot; use crate::SnapshotMeta; use crate::Vote; -/// A candidate whose vote request this node rejected because its log was behind; see -/// `Engine::stale_candidate`. -#[derive(Debug, Clone)] -pub(crate) struct StaleCandidate -where C: RaftTypeConfig -{ - /// The highest term among such rejected requests. - pub(crate) term: u64, - /// When the first of them was rejected. - pub(crate) rejected_at: InstantOf, -} - /// Raft protocol algorithm. /// /// It implement the complete raft algorithm except does not actually update any states. @@ -117,18 +104,6 @@ where C: RaftTypeConfig /// this identifier instead. A response to any round but the one in flight is ignored. pub(crate) pre_vote_round: u64, - /// A candidate whose vote request this node rejected because that candidate's log was behind - /// this node's, recorded only while this node has no leader. - /// - /// Such a candidate cannot win against this node's log. When it is a voter that cannot answer - /// Pre-Vote, as a voter of a release without Pre-Vote cannot, this node's own Pre-Vote may - /// never reach a quorum either. If no leader appears for one election timeout after the - /// first rejection, this node campaigns without Pre-Vote, in a term above every rejected - /// candidate's. The wait lets such a candidate still win with the other voters' grants - /// without being disrupted. Hearing from a leader, granting a vote, leading or campaigning - /// clears it. - pub(crate) stale_candidate: Option>, - /// Retirement is irreversible for this engine instance, including after a new vote. pub(crate) prepared_shutdown: Option>, @@ -151,7 +126,6 @@ where C: RaftTypeConfig candidate: None, pre_candidate: None, pre_vote_round: 0, - stale_candidate: None, prepared_shutdown: None, output: EngineOutput::new(4096), } @@ -301,10 +275,7 @@ where C: RaftTypeConfig self.leader.as_ref().map(|l| l.vote.to_string()).unwrap_or_default() ); - // Campaign above every candidate this node rejected for its log, so that such a candidate's - // vote for itself in a term cannot block this campaign. - let stale_term = self.stale_candidate.take().map_or(0, |stale| stale.term); - let new_term = std::cmp::max(self.state.vote.leader_id().term, stale_term) + 1; + let new_term = self.state.vote.leader_id().term + 1; let new_vote = Vote::new(new_term, self.config.id.clone()); let candidate = self.new_candidate(new_vote.clone()); @@ -476,16 +447,6 @@ where C: RaftTypeConfig // The res is not used yet. // let _res = Err(RejectVoteRequest::ByLastLogId(self.state.last_log_id().copied())); - // This node has no leader: its lease has expired, and it does not lead. Remember the - // candidate, which cannot win against this node's log; see `stale_candidate`. - let term = req.vote.leader_id().term; - match self.stale_candidate.as_mut() { - Some(stale) => stale.term = std::cmp::max(stale.term, term), - None => { - self.stale_candidate = Some(StaleCandidate { term, rejected_at: now }); - } - } - // Return the updated vote, this way the candidate knows which vote is granted, in case // the candidate's vote is changed after sending the vote request. return VoteResponse::new(self.state.vote_ref(), self.state.last_log_id().cloned(), false); @@ -494,10 +455,6 @@ where C: RaftTypeConfig // Then check vote just as it does for every incoming event. let res = self.vote_handler().update_vote(&req.vote); - if res.is_ok() { - // A granted vote follows another candidate. - self.stale_candidate = None; - } tracing::info!( req = display(req.summary()), @@ -756,8 +713,7 @@ where C: RaftTypeConfig ) -> Result<(), RejectAppendEntries> { self.vote_handler().update_vote(vote)?; - // Vote is legal: this node hears from a leader. - self.stale_candidate = None; + // Vote is legal. let mut fh = self.following_handler(); fh.ensure_log_consecutive(prev_log_id.clone())?; @@ -960,7 +916,6 @@ where C: RaftTypeConfig // An overlapping Pre-Vote round must not start another campaign once this node leads. self.pre_candidate = None; - self.stale_candidate = None; let candidate = self.candidate.take().unwrap(); let leader = self.establish_handler().establish(candidate); @@ -1021,12 +976,6 @@ where C: RaftTypeConfig } } - /// Whether this node campaigns without Pre-Vote now, after rejecting a stale candidate at - /// least `wait` ago and hearing from no leader since; see `stale_candidate`. - pub(crate) fn stale_candidate_campaign_due(&self, now: InstantOf, wait: Duration) -> bool { - self.stale_candidate.as_ref().is_some_and(|stale| now >= stale.rejected_at + wait) - } - pub(crate) fn is_there_greater_log(&self) -> bool { self.seen_greater_log } diff --git a/openraft/src/engine/tests/elect_test.rs b/openraft/src/engine/tests/elect_test.rs index b505b43e1..da083780b 100644 --- a/openraft/src/engine/tests/elect_test.rs +++ b/openraft/src/engine/tests/elect_test.rs @@ -387,31 +387,3 @@ fn test_elect_multi_node_enter_candidate() -> anyhow::Result<()> { } Ok(()) } - -#[test] -fn test_elect_campaigns_above_a_rejected_stale_candidate() -> anyhow::Result<()> { - let mut eng = eng(); - eng.config.id = 1; - eng.state - .membership_state - .set_effective(Arc::new(EffectiveMembership::new(Some(log_id(0, 1, 1)), m12()))); - eng.state.log_ids = LogIdList::new(vec![log_id(1, 1, 1)]); - eng.stale_candidate = Some(crate::engine::engine_impl::StaleCandidate { - term: 4, - rejected_at: crate::TokioInstant::now(), - }); - - eng.elect(); - - // The stale candidate voted for itself in term 4, so a campaign in term 1 or 4 could not win - // its vote. - assert_eq!(Vote::new(5, 1), *eng.state.vote_ref()); - assert!(eng.stale_candidate.is_none(), "the campaign consumes the record"); - assert_eq!( - vec![Command::SaveVote { vote: Vote::new(5, 1) }, Command::SendVote { - vote_req: VoteRequest::new(Vote::new(5, 1), Some(log_id(1, 1, 1))) - },], - eng.output.take_commands() - ); - Ok(()) -} diff --git a/openraft/src/engine/tests/handle_vote_req_test.rs b/openraft/src/engine/tests/handle_vote_req_test.rs index 3251cde42..b9c2e48c7 100644 --- a/openraft/src/engine/tests/handle_vote_req_test.rs +++ b/openraft/src/engine/tests/handle_vote_req_test.rs @@ -282,71 +282,3 @@ fn test_handle_vote_req_granted_follower_learner_does_not_emit_update_server_sta } Ok(()) } - -#[test] -fn test_handle_vote_req_smaller_last_log_id_records_the_stale_candidate() -> anyhow::Result<()> { - let mut eng = eng(); - eng.state.log_ids = LogIdList::new(vec![log_id(2, 1, 3)]); - let before = TokioInstant::now(); - - for (term, candidate) in [(3, 2), (5, 0), (4, 2)] { - let resp = eng.handle_vote_req(VoteRequest { - vote: Vote::new(term, candidate), - last_log_id: Some(log_id(1, 1, 3)), - }); - assert!(!resp.vote_granted); - } - - let stale = eng.stale_candidate.clone().expect("the rejected candidates are recorded"); - assert_eq!(5, stale.term, "the highest rejected term is kept"); - assert!(stale.rejected_at >= before, "the first rejection starts the wait"); - assert!(!eng.stale_candidate_campaign_due(TokioInstant::now(), Duration::from_secs(1))); - assert!(eng.stale_candidate_campaign_due(stale.rejected_at + Duration::from_secs(1), Duration::from_secs(1))); - - // Nothing else changes: the record is in memory only. - assert_eq!(Vote::new(2, 1), *eng.state.vote_ref()); - assert_eq!(0, eng.output.take_commands().len()); - Ok(()) -} - -#[test] -fn test_handle_vote_req_lease_rejection_records_no_stale_candidate() -> anyhow::Result<()> { - let mut eng = eng(); - eng.state.log_ids = LogIdList::new(vec![log_id(2, 1, 3)]); - eng.state.vote.update(TokioInstant::now(), Vote::new_committed(2, 1)); - - let resp = eng.handle_vote_req(VoteRequest { - vote: Vote::new(3, 2), - last_log_id: Some(log_id(1, 1, 3)), - }); - - assert!(!resp.vote_granted); - assert!( - eng.stale_candidate.is_none(), - "a node that still hears from its leader records nothing" - ); - Ok(()) -} - -#[test] -fn test_handle_vote_req_granted_vote_clears_the_stale_candidate() -> anyhow::Result<()> { - let mut eng = eng(); - eng.state.log_ids = LogIdList::new(vec![log_id(2, 1, 3)]); - eng.handle_vote_req(VoteRequest { - vote: Vote::new(3, 2), - last_log_id: Some(log_id(1, 1, 3)), - }); - assert!(eng.stale_candidate.is_some()); - - let resp = eng.handle_vote_req(VoteRequest { - vote: Vote::new(4, 2), - last_log_id: Some(log_id(2, 1, 3)), - }); - - assert!(resp.vote_granted); - assert!( - eng.stale_candidate.is_none(), - "a granted vote follows another candidate" - ); - Ok(()) -} From ab184c0b28c4626e90ae5e2f94b98b78980b354a Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Fri, 2 Oct 2026 11:18:10 -0600 Subject: [PATCH 24/36] Test: retry a Pre-Vote round rejected by a running lease within the bound Signed-off-by: VerifiedOrganic --- tests/tests/elect/main.rs | 1 + .../elect/t25_rejected_pre_vote_retry.rs | 110 ++++++++++++++++++ 2 files changed, 111 insertions(+) create mode 100644 tests/tests/elect/t25_rejected_pre_vote_retry.rs diff --git a/tests/tests/elect/main.rs b/tests/tests/elect/main.rs index 2aa45614d..ab08f3558 100644 --- a/tests/tests/elect/main.rs +++ b/tests/tests/elect/main.rs @@ -16,3 +16,4 @@ mod t21_pre_vote; mod t22_one_way_isolation; mod t23_classic_voter_stale_log; mod t24_classic_leader_one_way; +mod t25_rejected_pre_vote_retry; diff --git a/tests/tests/elect/t25_rejected_pre_vote_retry.rs b/tests/tests/elect/t25_rejected_pre_vote_retry.rs new file mode 100644 index 000000000..09bd78d6a --- /dev/null +++ b/tests/tests/elect/t25_rejected_pre_vote_retry.rs @@ -0,0 +1,110 @@ +use std::sync::Arc; +use std::time::Duration; + +use anyhow::Result; +use maplit::btreeset; +use openraft::Config; +use openraft::ServerState; +use tokio::time::sleep; +use tokio::time::Instant; + +use crate::fixtures::init_default_ut_tracing; +use crate::fixtures::Direction; +use crate::fixtures::RPCErrorType; +use crate::fixtures::RaftRouter; + +/// How much earlier node 1 stops hearing from the leader than node 2. More than the width of the +/// election-timeout window and a tick, so node 1's first Pre-Vote round always meets node 2's +/// running lease; less than twice the minimum election timeout less the maximum and a tick, so a +/// retry one sampled election timeout later always lands beyond the bound. +const CONTACT_SKEW: Duration = Duration::from_millis(400); + +/// Scheduling and round-trip allowance on top of the bound. +const SLACK: Duration = Duration::from_millis(120); + +/// A Pre-Vote round rejected by a lease that is still running is retried in time. +/// +/// After an unplanned leader loss every survivor's lease runs out within the minimum election +/// timeout of the loss, and the survivor with the most up-to-date log starts its first Pre-Vote +/// round within the maximum election timeout and a tick of its last leader contact. Node 1 holds +/// the most up-to-date log but heard from the leader last before node 2, whose log is behind, so +/// node 2's lease rejects node 1's first round. Node 1 must retry within the width of the +/// election-timeout window, so it is still elected within the maximum election timeout and a tick +/// of the loss. Holding the rejected round for another sampled election timeout instead would push +/// the election beyond that bound. +#[async_entry::test(worker_threads = 8, init = "init_default_ut_tracing()", tracing_span = "debug")] +async fn pre_vote_round_rejected_by_a_running_lease_is_retried_within_the_bound() -> Result<()> { + let config = Arc::new( + Config { + heartbeat_interval: 20, + election_timeout_min: 1_000, + election_timeout_max: 1_150, + enable_pre_vote: Some(true), + ..Default::default() + } + .validate()?, + ); + let tick = Duration::from_millis(config.heartbeat_interval * 3 / 2); + let bound = Duration::from_millis(config.election_timeout_max) + tick; + let mut router = RaftRouter::new(config.clone()); + + tracing::info!("--- create cluster of 0,1,2; node 0 becomes leader"); + let mut log_index = router.new_cluster(btreeset! {0,1,2}, btreeset! {}).await?; + let n0 = router.get_raft_handle(&0)?; + let n1 = router.get_raft_handle(&1)?; + let n2 = router.get_raft_handle(&2)?; + n0.wait(timeout()).state(ServerState::Leader, "node 0 is the initial leader").await?; + + tracing::info!("--- node 2 misses the last entries"); + router.set_network_error(2, true); + router.client_request_many(0, "missed", 10).await?; + log_index += 10; + n1.wait(timeout()).applied_index(Some(log_index), "node 1 has the entries").await?; + + tracing::info!("--- node 2 hears from the leader again, but receives no entries"); + router.set_append_entries_quota(Some(0)); + router.set_network_error(2, false); + sleep(Duration::from_millis(200)).await; + let node2_last_log = n2.metrics().borrow().last_log_index; + assert!( + node2_last_log < Some(log_index), + "node 2's log stays behind node 1's: {:?} < {}", + node2_last_log, + log_index + ); + + tracing::info!( + "--- node 1 stops hearing from the leader {:?} before node 2", + CONTACT_SKEW + ); + router.set_rpc_failure(1, Direction::NetRecv, Some(RPCErrorType::NetworkError)); + sleep(CONTACT_SKEW).await; + + tracing::info!("--- the leader is lost"); + let lost = Instant::now(); + router.set_network_error(0, true); + router.set_rpc_failure(1, Direction::NetRecv, None); + router.set_append_entries_quota(None); + + n1.wait(Some(bound * 4)) + .state(ServerState::Leader, "node 1, with the most up-to-date log, is elected") + .await?; + let elected = lost.elapsed(); + tracing::info!("--- node 1 was elected {:?} after the loss; bound {:?}", elected, bound); + assert!( + elected <= bound + SLACK, + "node 1 was elected {:?} after the loss, beyond the maximum election timeout and a tick, {:?}", + elected, + bound + ); + assert_eq!( + n0.metrics().borrow().current_term + 1, + n1.metrics().borrow().current_term + ); + + Ok(()) +} + +fn timeout() -> Option { + Some(Duration::from_millis(3_000)) +} From 3ebc971257bcac1b6f50be19d8c3fc2ea1b306ea Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Fri, 2 Oct 2026 11:19:15 -0600 Subject: [PATCH 25/36] Fix: retry a rejected Pre-Vote round after the election-timeout window A Pre-Vote round that had not reached a quorum was held for a whole sampled election timeout. After an unplanned leader loss, a survivor whose last leader contact came shortly before another survivor's meets that survivor's running lease in its first round. Holding the rejected round for another sampled timeout delayed the first successful election up to about twice the maximum election timeout, beyond any bound that the maximum election timeout implies. A round is now held for the width of the election-timeout window and retried on a later tick. Every lease of a lost leader runs out within the minimum election timeout of the loss, so the survivor with the most up-to-date log starts a round that no lease rejects within the maximum election timeout and one tick of the loss, the same bound as its first round. In the regression, the election after the loss drops from about 1.7-1.9 s to 1.0-1.1 s, within the 1.18 s bound. Signed-off-by: VerifiedOrganic --- openraft/src/core/raft_core.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/openraft/src/core/raft_core.rs b/openraft/src/core/raft_core.rs index 5dd604dd1..979be7669 100644 --- a/openraft/src/core/raft_core.rs +++ b/openraft/src/core/raft_core.rs @@ -1744,12 +1744,20 @@ where if pre_vote { // A Pre-Vote round does not update the vote, so the expired election timer would - // restart it on every tick. Keep a round in flight for one sampled election timeout. - let timer_config = &self.engine.config.timer_config; - let mut round_timeout = timer_config.election_timeout; - if self.engine.is_there_greater_log() { - round_timeout += timer_config.smaller_log_timeout; - } + // restart it on every tick. Keep a round in flight for the width of the + // election-timeout window, then retry it on a later tick. + // + // After an unplanned leader loss, a round fails while a voter's lease still runs, and + // every lease runs out within the minimum election timeout of the loss. A survivor's + // first round starts within the maximum election timeout and a tick of its last leader + // contact, and each retry within the window's width and a tick of the round before. + // So the survivor with the most up-to-date log starts a round that no lease rejects + // within the maximum election timeout and a tick of the loss. Holding a rejected round + // for a whole sampled election timeout could double that. + let engine_config = &self.engine.config; + let round_timeout = Duration::from_millis( + engine_config.election_timeout_max.saturating_sub(engine_config.election_timeout_min), + ); if let Some(started) = self.engine.pre_candidate_ref().map(|x| x.starting_time()) { if now < started + round_timeout { From e2c9baa0094b3a81e9275c3457e74dbabf634e28 Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Fri, 2 Oct 2026 11:20:28 -0600 Subject: [PATCH 26/36] Test: answer a deposing vote without waiting for a hung learner Signed-off-by: VerifiedOrganic --- tests/tests/append_entries/main.rs | 1 + .../t63_vote_with_hung_learner.rs | 78 +++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 tests/tests/append_entries/t63_vote_with_hung_learner.rs diff --git a/tests/tests/append_entries/main.rs b/tests/tests/append_entries/main.rs index 3d5675312..1ebc90ec4 100644 --- a/tests/tests/append_entries/main.rs +++ b/tests/tests/append_entries/main.rs @@ -19,4 +19,5 @@ mod t60_enable_heartbeat; mod t61_heartbeat_reject_vote; mod t61_large_heartbeat; mod t62_append_entries_deadline; +mod t63_vote_with_hung_learner; mod t90_issue_216_stale_last_log_id; diff --git a/tests/tests/append_entries/t63_vote_with_hung_learner.rs b/tests/tests/append_entries/t63_vote_with_hung_learner.rs new file mode 100644 index 000000000..f48b0653a --- /dev/null +++ b/tests/tests/append_entries/t63_vote_with_hung_learner.rs @@ -0,0 +1,78 @@ +use std::sync::Arc; +use std::time::Duration; + +use anyhow::Result; +use maplit::btreeset; +use openraft::raft::VoteRequest; +use openraft::Config; +use openraft::ServerState; +use openraft::Vote; +use tokio::time::sleep; +use tokio::time::Instant; + +use crate::fixtures::init_default_ut_tracing; +use crate::fixtures::RaftRouter; + +/// A leader answers the vote that deposes it without waiting for an AppendEntries to a hung +/// learner. +/// +/// Granting the vote makes the leader stop leading, and it closes every replication stream before +/// it answers. A stream whose AppendEntries is held by a learner that never answers must give up +/// that request when it is closed. Otherwise the answer waits for the AppendEntries deadline, +/// which may be far longer than the candidate waits for a vote. +#[async_entry::test(worker_threads = 8, init = "init_default_ut_tracing()", tracing_span = "debug")] +async fn deposing_vote_is_answered_without_waiting_for_a_hung_learner() -> Result<()> { + let config = Arc::new( + Config { + heartbeat_interval: 50, + append_entries_timeout: Some(10_000), + election_timeout_min: 500, + election_timeout_max: 600, + ..Default::default() + } + .validate()?, + ); + let vote_deadline = Duration::from_millis(config.election_timeout_min); + + let mut router = RaftRouter::new(config.clone()); + + tracing::info!("--- create cluster of voters 0,1,2 and learner 3; node 0 becomes leader"); + router.new_cluster(btreeset! {0,1,2}, btreeset! {3}).await?; + let n0 = router.get_raft_handle(&0)?; + n0.wait(timeout()).state(ServerState::Leader, "node 0 is the initial leader").await?; + for id in [1, 2] { + router.get_raft_handle(&id)?.runtime_config().elect(false); + } + + tracing::info!("--- learner 3 holds every AppendEntries without answering"); + router.set_rpc_blocked(3, true); + sleep(Duration::from_millis(200)).await; + + tracing::info!("--- node 0 stops sending heartbeats, so its lease runs out"); + n0.runtime_config().heartbeat(false); + sleep(Duration::from_millis(config.election_timeout_max + 200)).await; + + let m0 = n0.metrics().borrow().clone(); + assert_eq!(ServerState::Leader, m0.state); + let req = VoteRequest::new(Vote::new(m0.current_term + 1, 1), m0.last_applied); + + tracing::info!("--- node 1 asks node 0 for its vote"); + let started = Instant::now(); + let resp = n0.vote(req).await?; + let answered = started.elapsed(); + router.set_rpc_blocked(3, false); + + assert!(resp.vote_granted, "node 0 grants the vote: {:?}", resp); + assert!( + answered < vote_deadline, + "node 0 answered the vote after {:?}, beyond the candidate's {:?} vote deadline", + answered, + vote_deadline + ); + + Ok(()) +} + +fn timeout() -> Option { + Some(Duration::from_millis(2_000)) +} From 89aa1b6f253edacb10835ecc72b00e398d84e94c Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Fri, 2 Oct 2026 11:22:52 -0600 Subject: [PATCH 27/36] Fix: give up an in-flight AppendEntries when its replication stream closes A leader that grants a vote stops leading and joins every replication stream before it answers, so that no stream still reads a log range a later truncation removes. A stream waiting for an AppendEntries answer from an unresponsive follower or learner did not notice that it was closed, so the vote answer waited for the whole AppendEntries deadline. Once that deadline was decoupled from the heartbeat interval and could exceed the candidate's vote deadline, a hung learner could block a handoff and the Raft core for that long. Closing a stream now drops a watch sender that an in-flight AppendEntries races, and the request is given up at once. The log entries are read before the request is sent, and a snapshot child is still cancelled and joined, so every storage operation a stream owns is still awaited. In the regression, the deposing vote is answered in milliseconds instead of after 9 s of a 10 s AppendEntries deadline. Signed-off-by: VerifiedOrganic --- openraft/src/core/raft_core.rs | 11 ++++++-- openraft/src/replication/mod.rs | 22 +++++++++++++++- openraft/src/replication/no_progress_test.rs | 27 ++++++++++++++++++++ 3 files changed, 57 insertions(+), 3 deletions(-) diff --git a/openraft/src/core/raft_core.rs b/openraft/src/core/raft_core.rs index 979be7669..53661b9ce 100644 --- a/openraft/src/core/raft_core.rs +++ b/openraft/src/core/raft_core.rs @@ -928,8 +928,15 @@ where tracing::debug!(%target, "retire replication"); // Close every sender before awaiting any task. A removed stream may still own // a log read or a snapshot, even though its progress notifications are stale. - drop(stream.tx_repl); - self.retired_replications.push(stream.join_handle); + // Closing also makes an in-flight AppendEntries give up at once. + let ReplicationHandle { + join_handle, + tx_repl, + tx_close, + } = stream; + drop(tx_repl); + drop(tx_close); + self.retired_replications.push(join_handle); } } diff --git a/openraft/src/replication/mod.rs b/openraft/src/replication/mod.rs index 5d0f8a846..235d698e4 100644 --- a/openraft/src/replication/mod.rs +++ b/openraft/src/replication/mod.rs @@ -25,6 +25,7 @@ pub(crate) use response::Response; use tokio::select; use tokio::sync::mpsc; use tokio::sync::oneshot; +use tokio::sync::watch; use tokio::sync::Mutex; use tracing_futures::Instrument; @@ -78,6 +79,9 @@ where C: RaftTypeConfig /// The channel used for communicating with the replication task. pub(crate) tx_repl: mpsc::UnboundedSender>, + + /// Dropped when the stream is closed, which makes an in-flight AppendEntries give up at once. + pub(crate) tx_close: watch::Sender<()>, } /// A task responsible for sending replication events to a target follower in the Raft cluster. @@ -104,6 +108,9 @@ where /// A channel for receiving events from the RaftCore and snapshot transmitting task. rx_event: mpsc::UnboundedReceiver>, + /// Closed when the RaftCore closes this stream; see [`ReplicationHandle::tx_close`]. + rx_close: watch::Receiver<()>, + /// A weak reference to the Sender for the separate sending-snapshot task to send callback. /// /// Because 1) ReplicationCore replies on the `close` event to shutdown. @@ -187,6 +194,7 @@ where // other component to ReplicationStream let (tx_event, rx_event) = mpsc::unbounded_channel(); + let (tx_close, rx_close) = watch::channel(()); let this = Self { target, @@ -202,6 +210,7 @@ where matching, tx_raft_core, rx_event, + rx_close, weak_tx_event: tx_event.downgrade(), next_action: None, entries_hint: Default::default(), @@ -212,6 +221,7 @@ where ReplicationHandle { join_handle, tx_repl: tx_event, + tx_close, } } @@ -520,7 +530,17 @@ where ); let option = RPCOption::new(the_timeout); - let res = C::timeout(the_timeout, self.network.append_entries(payload, option)).await; + let res = select! { + res = C::timeout(the_timeout, self.network.append_entries(payload, option)) => res, + // Nobody waits for the answer of a closed stream, while the RaftCore may wait to join + // this stream before it answers a vote. Give up the request at once instead of at its + // deadline. The log entries were already read, so no storage operation is abandoned. + _ = self.rx_close.changed() => { + return Err(ReplicationError::Closed(ReplicationClosed::new( + "RaftCore closed replication during AppendEntries", + ))); + } + }; tracing::debug!("append_entries res: {:?}", res); diff --git a/openraft/src/replication/no_progress_test.rs b/openraft/src/replication/no_progress_test.rs index 7b2adc48f..384774574 100644 --- a/openraft/src/replication/no_progress_test.rs +++ b/openraft/src/replication/no_progress_test.rs @@ -16,6 +16,7 @@ use std::time::Duration; use tokio::sync::mpsc; use tokio::sync::oneshot; +use tokio::sync::watch; use tokio::time::Instant; use super::ReplicationCore; @@ -300,6 +301,7 @@ impl RaftNetworkFactory for NetworkFactory { struct Run { main: Main, tx: Option>>, + tx_close: Option>, rx: mpsc::UnboundedReceiver>, leader: ReadyStore, follower: ReadyStore, @@ -344,12 +346,14 @@ impl Run { second_gate: None, }; let (tx, rx_event) = mpsc::unbounded_channel(); + let (tx_close, rx_close) = watch::channel(()); let (tx_raft_core, rx) = mpsc::unbounded_channel(); let core = Core { target: 2, session_id: ReplicationSessionId::new(vote(), Some(log_id(0))), tx_raft_core, rx_event, + rx_close, weak_tx_event: tx.downgrade(), network, snapshot_network: Arc::new(tokio::sync::Mutex::new(snapshot_network)), @@ -380,6 +384,7 @@ impl Run { Self { main, tx: Some(tx), + tx_close: Some(tx_close), rx, leader, follower, @@ -476,6 +481,7 @@ impl Run { fn close(&mut self) { let before = Instant::now(); self.tx.take(); + self.tx_close.take(); assert!( matches!(self.poll(), Poll::Ready(Ok(()))), "zero_progress_close_does_not_wait_for_retry_timer" @@ -497,6 +503,7 @@ impl Drop for Run { let _ = release.send(()); } self.tx.take(); + self.tx_close.take(); } } @@ -864,3 +871,23 @@ async fn elapsed_rpc_time_counts_toward_the_no_progress_interval() { run.close(); } } + +/// Closing a stream gives up an AppendEntries that never answers at once, not at its deadline. +/// +/// The RaftCore joins a closed stream before it answers the vote that made it stop leading, so a +/// stream held by an unresponsive follower or learner would otherwise delay that answer by the +/// whole AppendEntries deadline. The entries were read before the request was sent, so no storage +/// operation is abandoned. +#[tokio::test(start_paused = true)] +async fn close_gives_up_an_append_entries_that_never_answers() { + let mut run = Run::new(Some(log_id(0)), vec![Reply::Timeout], false).await; + run.start_logs(); + run.pending(); + assert_eq!( + run.counts(), + (1, 1, 0), + "the entries were read and the request is in flight" + ); + run.close(); + assert_eq!(run.counts(), (1, 1, 0), "the request never completed"); +} From 71f7ff7ed2a0a1783362264d597926a243d62b37 Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Fri, 2 Oct 2026 11:23:35 -0600 Subject: [PATCH 28/36] Doc: describe Pre-Vote gating, round identifiers and retries Signed-off-by: VerifiedOrganic --- openraft/src/config/config.rs | 7 ++- openraft/src/docs/protocol/pre_vote.md | 82 +++++++++++++++----------- 2 files changed, 53 insertions(+), 36 deletions(-) diff --git a/openraft/src/config/config.rs b/openraft/src/config/config.rs index 028690bd5..37d972803 100644 --- a/openraft/src/config/config.rs +++ b/openraft/src/config/config.rs @@ -269,9 +269,10 @@ pub struct Config { /// does not raise its term and depose a healthy leader once it can be reached again. /// /// Pre-Vote uses the separate - /// [`RaftNetwork::pre_vote`](`crate::network::RaftNetwork::pre_vote`) RPC. A network that does - /// not implement it reports every Pre-Vote as granted, so elections proceed as without - /// Pre-Vote; a transport error is never counted as a grant. + /// [`RaftNetwork::pre_vote`](`crate::network::RaftNetwork::pre_vote`) RPC. A voter that the + /// network does not know to answer it, including every voter of a network that does not + /// implement it, makes the campaign run the classic election instead; a transport error is + /// never counted as a grant. /// /// `None`, the default, is treated as disabled. // clap 4 requires `num_args = 0..=1`, or it complains about missing arg error diff --git a/openraft/src/docs/protocol/pre_vote.md b/openraft/src/docs/protocol/pre_vote.md index 7965ac3af..7a69164dc 100644 --- a/openraft/src/docs/protocol/pre_vote.md +++ b/openraft/src/docs/protocol/pre_vote.md @@ -60,10 +60,13 @@ term and persists its vote as usual. A response with a strictly higher vote catches the requester up to that vote in non-committed form, as a rejected real vote does, unless it is a vote for the requester itself: no voter holds one for a term the requester never campaigned in. A rejection that reports a greater -log delays the next attempt. Any -vote the requester accepts afterwards, including a heartbeat from the current -leader, ends the round. Otherwise the round is retried after one newly sampled -election timeout. +log delays the next attempt. Any vote the requester accepts afterwards, +including a heartbeat from the current leader, ends the round. + +Each round has its own identifier, which every response names. Rounds in one +term propose the same hypothetical vote, so the vote cannot tell a delayed +response from a current one; a response to any round but the one in flight is +ignored. A grant from a node that does not vote in the round never counts. Only a response counts toward the quorum. An error, including an unreachable peer, is never a grant, so a voter that is cut off cannot assemble one. @@ -71,40 +74,53 @@ peer, is never a grant, so a voter that is cut off cannot assemble one. A voter that is the only voter wins its own Pre-Vote and elects at once. +## Retrying a round + +A round that has not reached a quorum is held for the width of the +election-timeout window, `election_timeout_max - election_timeout_min`, and +then retried on a later tick while the election timer stays expired. + +After an unplanned leader loss, a round fails while another voter's lease +still runs. Every survivor's lease runs out within the minimum election timeout +of the loss. A survivor starts its first round within the maximum election +timeout and a tick of its last leader contact, and each retry within the +window's width and a tick of the round before it. So the survivor with the most +up-to-date log starts a round that no lease rejects within the maximum election +timeout and a tick of the loss, the same bound as its first round. Holding a +rejected round for a whole sampled election timeout instead could nearly double +that. + + ## Voters that cannot answer Pre-Vote A network may reach voters that cannot answer a Pre-Vote request, such as voters of a release without Pre-Vote during a rolling upgrade. Such a voter -campaigns with the real vote alone and grants real votes by the rules above. A -network that knows a voter is one of them answers its Pre-Vote locally, as a -rejection that carries no vote to catch up to, and never sends it a request it -could not decode. - -Counting such a voter as rejecting keeps a voter that runs Pre-Vote from -campaigning, and raising its term, before a voter without Pre-Vote that may -hold a more up-to-date log campaigns. That voter then wins with the real vote. -When its log is behind, it cannot win, and a voter whose log is more up to date -must campaign instead, although its own Pre-Vote may never reach a quorum: - -- A voter that rejects a real vote request because the candidate's log is - behind its own, while it has no leader, records that candidate's term. -- If it hears from no leader for one election timeout after the first such - rejection, it campaigns without Pre-Vote, in a term above every recorded - term. The candidate voted for itself in its own term, so a campaign in that - term could not win its vote. -- The wait lets that candidate still win with the other voters' grants, without - being disrupted. -- Hearing from a leader, granting a vote, leading or campaigning clears the - record. - -The rule changes no vote rule: a voter still grants only a candidate whose log -is at least as up to date as its own. It only decides when a voter campaigns. +campaigns with the real vote alone and grants real votes by the rules above, +but it can never grant a Pre-Vote. Pre-Vote is therefore used only while every +voter that can be reached is positively known to answer it: + +- [`RaftNetwork::pre_vote`](`crate::network::RaftNetwork::pre_vote`) sends the + request and returns [`PreVoteReply::Answered`](`crate::raft::PreVoteReply`) + only for a target that is positively known to answer Pre-Vote: the + capability was negotiated on the connection to it, or it is an in-process + peer that passes the request to + [`Raft::pre_vote`](`crate::Raft::pre_vote`). +- For any other target that it can reach, it sends nothing and returns + [`PreVoteReply::Unsupported`](`crate::raft::PreVoteReply`). The voter then + ends the round and runs the classic election for this campaign at once: + waiting for a Pre-Vote quorum that needs that target could take forever, for + example when the target still believes it leads and never campaigns itself. +- A target that cannot be reached is an error. It can grant neither a Pre-Vote + nor a vote, so it neither counts as a grant nor forces the classic election. + +Each campaign decides again, so a voter set that no longer holds such a voter +uses Pre-Vote again. ## Networks without Pre-Vote -Pre-Vote uses the separate -[`RaftNetwork::pre_vote`](`crate::network::RaftNetwork::pre_vote`) RPC. Its -default implementation reports a granted Pre-Vote without contacting the -target, so a network that does not implement it keeps the election behavior -without Pre-Vote. +The default implementation of +[`RaftNetwork::pre_vote`](`crate::network::RaftNetwork::pre_vote`) returns +[`PreVoteReply::Unsupported`](`crate::raft::PreVoteReply`) without contacting +the target, so a network that does not implement it keeps the election +behavior without Pre-Vote: every campaign runs the classic election. From 47ba29a605c34562df3c785034a8374af074ec72 Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Fri, 2 Oct 2026 11:58:02 -0600 Subject: [PATCH 29/36] Chore: drop the doc comment of the removed Pre-Vote round matcher Signed-off-by: VerifiedOrganic --- openraft/src/core/raft_core.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/openraft/src/core/raft_core.rs b/openraft/src/core/raft_core.rs index 53661b9ce..d72f5a339 100644 --- a/openraft/src/core/raft_core.rs +++ b/openraft/src/core/raft_core.rs @@ -1840,13 +1840,6 @@ where true } } - /// A Pre-Vote response belongs to the in-flight Pre-Vote round only if it answers that round's - /// vote; otherwise it is stale and ignored. - /// - /// Two rounds in the same term carry the same vote. A response still cannot reach a later - /// round: each request is abandoned after `election_timeout_min`, a new round starts no earlier - /// than one sampled election timeout after the previous one, and responses and ticks are - /// delivered in order on the same channel. /// If a message is sent by a previous replication session but is received by current server /// state, it is a stale message and should be just ignored. fn does_replication_session_match( From 9aeadfcdf3cb2affa80b0a94fe87e54d4ac0a81a Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Fri, 2 Oct 2026 13:28:37 -0600 Subject: [PATCH 30/36] Test: count Pre-Vote replies that arrive after the election-timeout window Signed-off-by: VerifiedOrganic --- tests/tests/elect/main.rs | 1 + .../elect/t26_delayed_pre_vote_replies.rs | 142 ++++++++++++++++++ tests/tests/fixtures/mod.rs | 41 ++++- 3 files changed, 177 insertions(+), 7 deletions(-) create mode 100644 tests/tests/elect/t26_delayed_pre_vote_replies.rs diff --git a/tests/tests/elect/main.rs b/tests/tests/elect/main.rs index ab08f3558..e1870951a 100644 --- a/tests/tests/elect/main.rs +++ b/tests/tests/elect/main.rs @@ -17,3 +17,4 @@ mod t22_one_way_isolation; mod t23_classic_voter_stale_log; mod t24_classic_leader_one_way; mod t25_rejected_pre_vote_retry; +mod t26_delayed_pre_vote_replies; diff --git a/tests/tests/elect/t26_delayed_pre_vote_replies.rs b/tests/tests/elect/t26_delayed_pre_vote_replies.rs new file mode 100644 index 000000000..9f37036b7 --- /dev/null +++ b/tests/tests/elect/t26_delayed_pre_vote_replies.rs @@ -0,0 +1,142 @@ +use std::sync::Arc; +use std::time::Duration; + +use anyhow::Result; +use maplit::btreeset; +use openraft::Config; +use openraft::ServerState; + +use crate::fixtures::init_default_ut_tracing; +use crate::fixtures::Direction; +use crate::fixtures::RPCErrorType; +use crate::fixtures::RaftRouter; + +/// The tightest valid election-timeout window: its width is 1 ms, and the heartbeat tick is 30 ms. +fn tight_config() -> Result> { + Ok(Arc::new( + Config { + heartbeat_interval: 20, + election_timeout_min: 1_000, + election_timeout_max: 1_001, + enable_pre_vote: Some(true), + ..Default::default() + } + .validate()?, + )) +} + +/// The timing of a production deployment: a 1,500 ms window and a 300 ms tick. +fn production_config() -> Result> { + Ok(Arc::new( + Config { + heartbeat_interval: 200, + append_entries_timeout: Some(2_000), + election_timeout_min: 5_000, + election_timeout_max: 6_500, + enable_pre_vote: Some(true), + ..Default::default() + } + .validate()?, + )) +} + +/// The surviving voters of a lost leader answer every Pre-Vote after `delay`, longer than the +/// election-timeout window and the tick but within the Pre-Vote deadline. A round that awaits +/// their answers must stay open to count them, or no round ever reaches a quorum and no leader is +/// elected. +async fn delayed_grants_elect_a_successor(config: Arc, delay: Duration) -> Result<()> { + let mut router = RaftRouter::new(config.clone()); + + tracing::info!("--- create cluster of 0,1,2; node 0 becomes leader"); + router.new_cluster(btreeset! {0,1,2}, btreeset! {}).await?; + router + .get_raft_handle(&0)? + .wait(timeout()) + .state(ServerState::Leader, "node 0 is the initial leader") + .await?; + + tracing::info!("--- nodes 1 and 2 answer every Pre-Vote after {:?}", delay); + for id in [1, 2] { + router.set_pre_vote_reply_delay(id, Some(delay)); + } + + tracing::info!("--- node 0 is lost"); + router.set_network_error(0, true); + + let election = Some(Duration::from_millis(config.election_timeout_max * 6) + delay * 4); + router + .wait(&1, election) + .metrics( + |m| m.current_leader.is_some_and(|leader| leader != 0), + "nodes 1 and 2 elect a successor", + ) + .await?; + + Ok(()) +} + +/// A voter that cannot answer Pre-Vote is reported only after `delay`, longer than the +/// election-timeout window and the tick but within the Pre-Vote deadline. Its log is behind, so the +/// other survivor can only win with its classic vote. The round that awaits the report must stay +/// open to receive it, or that survivor never runs the classic election and no leader is elected. +async fn delayed_unsupported_runs_the_classic_election(config: Arc, delay: Duration) -> Result<()> { + let mut router = RaftRouter::new(config.clone()); + + tracing::info!("--- create cluster of 0,1,2; node 0 becomes leader"); + let mut log_index = router.new_cluster(btreeset! {0,1,2}, btreeset! {}).await?; + router + .get_raft_handle(&0)? + .wait(timeout()) + .state(ServerState::Leader, "node 0 is the initial leader") + .await?; + + tracing::info!( + "--- node 2 cannot answer Pre-Vote, and that is reported after {:?}", + delay + ); + router.get_raft_handle(&2)?.runtime_config().pre_vote(false); + router.set_pre_vote_unsupported(2, true); + router.set_pre_vote_reply_delay(2, Some(delay)); + + tracing::info!("--- node 2 misses the last entries"); + router.set_rpc_failure(2, Direction::NetRecv, Some(RPCErrorType::NetworkError)); + router.client_request_many(0, "missed", 5).await?; + log_index += 5; + router.wait(&1, timeout()).applied_index(Some(log_index), "node 1 has the entries").await?; + + tracing::info!("--- node 0 is lost; node 2 is reachable again"); + router.set_network_error(0, true); + router.set_rpc_failure(2, Direction::NetRecv, None); + + let election = Some(Duration::from_millis(config.election_timeout_max * 6) + delay * 4); + router + .wait(&1, election) + .state(ServerState::Leader, "node 1 is elected with node 2's classic vote") + .await?; + + Ok(()) +} + +#[async_entry::test(worker_threads = 8, init = "init_default_ut_tracing()", tracing_span = "debug")] +async fn tight_window_delayed_pre_vote_grants_elect_a_successor() -> Result<()> { + delayed_grants_elect_a_successor(tight_config()?, Duration::from_millis(65)).await +} + +#[async_entry::test(worker_threads = 8, init = "init_default_ut_tracing()", tracing_span = "debug")] +async fn production_timing_delayed_pre_vote_grants_elect_a_successor() -> Result<()> { + delayed_grants_elect_a_successor(production_config()?, Duration::from_millis(1_900)).await +} + +#[async_entry::test(worker_threads = 8, init = "init_default_ut_tracing()", tracing_span = "debug")] +async fn tight_window_delayed_unsupported_runs_the_classic_election() -> Result<()> { + delayed_unsupported_runs_the_classic_election(tight_config()?, Duration::from_millis(65)).await +} + +#[async_entry::test(worker_threads = 8, init = "init_default_ut_tracing()", tracing_span = "debug")] +async fn production_timing_delayed_unsupported_runs_the_classic_election() -> Result<()> { + delayed_unsupported_runs_the_classic_election(production_config()?, Duration::from_millis(1_900)).await +} + +fn timeout() -> Option { + Some(Duration::from_millis(10_000)) +} diff --git a/tests/tests/fixtures/mod.rs b/tests/tests/fixtures/mod.rs index bfca2dc96..2892423dc 100644 --- a/tests/tests/fixtures/mod.rs +++ b/tests/tests/fixtures/mod.rs @@ -271,6 +271,9 @@ pub struct TypedRaftRouter { /// one of them is reported as unsupported without reaching it. pre_vote_unsupported: Arc>>, + /// How long the reply to a Pre-Vote sent to a node takes to arrive, per target. + pre_vote_reply_delay: Arc>>, + /// A hook function to be called when before an RPC is sent to target node. rpc_pre_hook: Arc>>, @@ -318,6 +321,7 @@ impl Builder { append_entries_quota: Arc::new(Mutex::new(None)), rpc_count: Default::default(), pre_vote_unsupported: Default::default(), + pre_vote_reply_delay: Default::default(), rpc_pre_hook: Default::default(), rpc_observers: Default::default(), blocked_rpc: Default::default(), @@ -574,6 +578,22 @@ impl TypedRaftRouter { /// Set whether to emit a specified rpc error when sending to/receiving from a node. /// Report every Pre-Vote to `id` as unsupported without reaching it, as a network does for a /// voter of a release without Pre-Vote. + /// Delay the reply to every Pre-Vote sent to `id` by `delay`, or remove the delay with `None`. + /// + /// A target that answers is asked at once and its answer is held back; an answer reported as + /// unsupported is held back the same way. + pub fn set_pre_vote_reply_delay(&self, id: MemNodeId, delay: Option) { + let mut delays = self.pre_vote_reply_delay.lock().unwrap(); + match delay { + Some(delay) => { + delays.insert(id, delay); + } + None => { + delays.remove(&id); + } + } + } + pub fn set_pre_vote_unsupported(&self, id: MemNodeId, unsupported: bool) { let mut nodes = self.pre_vote_unsupported.lock().unwrap(); if unsupported { @@ -1195,17 +1215,24 @@ impl RaftNetwork for RaftRouterNetwork { self.owner.emit_rpc_error(from_id, self.target)?; self.owner.rand_send_delay().await; - if self.owner.pre_vote_unsupported.lock().unwrap().contains(&self.target) { + let reply_delay = self.owner.pre_vote_reply_delay.lock().unwrap().get(&self.target).copied(); + + let reply = if self.owner.pre_vote_unsupported.lock().unwrap().contains(&self.target) { // Send nothing to a voter that cannot answer Pre-Vote. - return Ok(PreVoteReply::Unsupported); - } + PreVoteReply::Unsupported + } else { + let node = self.owner.get_raft_handle(&self.target)?; - let node = self.owner.get_raft_handle(&self.target)?; + let resp = node.pre_vote(rpc).await; + let resp = resp.map_err(|e| RemoteError::new(self.target, e))?; + PreVoteReply::Answered(resp) + }; - let resp = node.pre_vote(rpc).await; - let resp = resp.map_err(|e| RemoteError::new(self.target, e))?; + if let Some(delay) = reply_delay { + tokio::time::sleep(delay).await; + } - Ok(PreVoteReply::Answered(resp)) + Ok(reply) } } From f0e7893e5a3deffc8e6b4ffd84880e2f60069fe3 Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Fri, 2 Oct 2026 13:32:45 -0600 Subject: [PATCH 31/36] Fix: keep a Pre-Vote round open until its replies are due A Pre-Vote round that had not reached a quorum was replaced after the width of the election-timeout window, and a reply to the replaced round was discarded. When voters answered more slowly than that width and a tick, though within the Pre-Vote deadline, every reply reached a round that was already replaced: with a 1 ms window and a 30 ms tick, 65 ms replies; with a 1,500 ms window and a 300 ms tick, 1,900 ms replies. No round ever reached a quorum and a reachable majority stayed leaderless, and a late report that a voter cannot answer Pre-Vote was dropped the same way, so the classic fallback never ran. Rounds are now tracked while they may still collect replies, until one Pre-Vote deadline (election_timeout_min) after they started. While the latest round awaits replies that no voter has rejected, no new round starts. Once a voter rejects it, a new round may start after the width of the election-timeout window, as before, and the rejected round stays open beside it, so a late grant still completes it and a late unsupported report still starts the classic election. Accepting a vote, campaigning or leading ends every open round. Signed-off-by: VerifiedOrganic --- openraft/src/core/raft_core.rs | 27 +-- openraft/src/engine/engine_impl.rs | 124 +++++++----- .../src/engine/handler/vote_handler/mod.rs | 11 +- openraft/src/engine/leadership_transfer.rs | 2 +- openraft/src/engine/mod.rs | 1 + openraft/src/engine/pre_vote_rounds.rs | 128 ++++++++++++ openraft/src/engine/tests/pre_vote_test.rs | 191 ++++++++++++++++-- 7 files changed, 392 insertions(+), 92 deletions(-) create mode 100644 openraft/src/engine/pre_vote_rounds.rs diff --git a/openraft/src/core/raft_core.rs b/openraft/src/core/raft_core.rs index d72f5a339..ad87879c3 100644 --- a/openraft/src/core/raft_core.rs +++ b/openraft/src/core/raft_core.rs @@ -1751,26 +1751,21 @@ where if pre_vote { // A Pre-Vote round does not update the vote, so the expired election timer would - // restart it on every tick. Keep a round in flight for the width of the - // election-timeout window, then retry it on a later tick. + // restart it on every tick. A round that awaits replies no voter has rejected stays the + // latest until they are due, one Pre-Vote deadline after it started: a slow reply must + // still reach it. A rejected round is retried after the width of the election-timeout + // window, while it stays open for late grants. // // After an unplanned leader loss, a round fails while a voter's lease still runs, and // every lease runs out within the minimum election timeout of the loss. A survivor's // first round starts within the maximum election timeout and a tick of its last leader - // contact, and each retry within the window's width and a tick of the round before. - // So the survivor with the most up-to-date log starts a round that no lease rejects - // within the maximum election timeout and a tick of the loss. Holding a rejected round - // for a whole sampled election timeout could double that. - let engine_config = &self.engine.config; - let round_timeout = Duration::from_millis( - engine_config.election_timeout_max.saturating_sub(engine_config.election_timeout_min), - ); - - if let Some(started) = self.engine.pre_candidate_ref().map(|x| x.starting_time()) { - if now < started + round_timeout { - tracing::debug!("a Pre-Vote round is already in flight"); - return; - } + // contact. When every surviving voter answers within the window's width, each retry + // starts within that width and a tick of the round before, so the survivor with the + // most up-to-date log starts a round that no lease rejects within the maximum election + // timeout and a tick of the loss. + if !self.engine.pre_vote_round_due(now) { + tracing::debug!("a Pre-Vote round is already in flight"); + return; } } diff --git a/openraft/src/engine/engine_impl.rs b/openraft/src/engine/engine_impl.rs index c44818713..837baaffd 100644 --- a/openraft/src/engine/engine_impl.rs +++ b/openraft/src/engine/engine_impl.rs @@ -20,6 +20,7 @@ use crate::engine::handler::server_state_handler::ServerStateHandler; use crate::engine::handler::snapshot_handler::SnapshotHandler; use crate::engine::handler::vote_handler::VoteHandler; use crate::engine::leadership_transfer::PreparedShutdown; +use crate::engine::pre_vote_rounds::PreVoteRounds; use crate::engine::Command; use crate::engine::EngineOutput; use crate::engine::Respond; @@ -44,6 +45,7 @@ use crate::raft::VoteResponse; use crate::raft_state::LogStateReader; use crate::raft_state::RaftState; use crate::summary::MessageSummary; +use crate::type_config::alias::InstantOf; use crate::type_config::alias::ResponderOf; use crate::type_config::alias::SnapshotDataOf; use crate::type_config::TypeConfigExt; @@ -92,17 +94,12 @@ where C: RaftTypeConfig /// without losing leadership status. pub(crate) candidate: CandidateState, - /// The quorum tracker of an in-flight Pre-Vote round. + /// The Pre-Vote rounds that may still collect replies; see [`PreVoteRounds`]. /// - /// Its vote is the hypothetical next-term vote this node would campaign with. It is never - /// persisted, and it is dropped once a real election starts or this node follows another vote. - pub(crate) pre_candidate: CandidateState, - - /// The identifier of the latest Pre-Vote round. - /// - /// Rounds in one term propose the same hypothetical vote, so a response names its round by - /// this identifier instead. A response to any round but the one in flight is ignored. - pub(crate) pre_vote_round: u64, + /// Their vote is the hypothetical next-term vote this node would campaign with. It is never + /// persisted, and every round ends once a real election starts or this node follows another + /// vote. + pub(crate) pre_vote_rounds: PreVoteRounds, /// Retirement is irreversible for this engine instance, including after a new vote. pub(crate) prepared_shutdown: Option>, @@ -124,8 +121,7 @@ where C: RaftTypeConfig seen_greater_log: false, leader: None, candidate: None, - pre_candidate: None, - pre_vote_round: 0, + pre_vote_rounds: PreVoteRounds::default(), prepared_shutdown: None, output: EngineOutput::new(4096), } @@ -152,25 +148,23 @@ where C: RaftTypeConfig self.candidate.as_mut().unwrap() } - /// Create a new Pre-Vote round and return the mutable reference to its quorum tracker. + /// Open a new Pre-Vote round and return its identifier. /// - /// Like [`new_candidate`](Self::new_candidate), but `vote` is only the hypothetical next-term - /// vote of the round and is never persisted. - pub(crate) fn new_pre_candidate(&mut self, vote: Vote) -> &mut Candidate> { + /// Its quorum tracker is like [`new_candidate`](Self::new_candidate)'s, but `vote` is only the + /// hypothetical next-term vote of the round and is never persisted. + pub(crate) fn new_pre_vote_round(&mut self, vote: Vote) -> u64 { let now = C::now(); let last_log_id = self.state.last_log_id().cloned(); let membership = self.state.membership_state.effective().membership(); - self.pre_candidate = Some(Candidate::new( + self.pre_vote_rounds.open(Candidate::new( now, vote, last_log_id, membership.to_quorum_set(), membership.learner_ids(), - )); - - self.pre_candidate.as_mut().unwrap() + )) } /// Create a default Engine for testing. @@ -257,8 +251,8 @@ where C: RaftTypeConfig /// Start to elect this node as leader #[tracing::instrument(level = "debug", skip(self))] pub(crate) fn elect(&mut self) { - // A real election supersedes any in-flight Pre-Vote round. - self.pre_candidate = None; + // A real election supersedes every open Pre-Vote round. + self.pre_vote_rounds.clear(); if self.prepared_shutdown.is_some() { return; @@ -326,23 +320,28 @@ where C: RaftTypeConfig let new_term = self.state.vote.leader_id().term + 1; let pre_vote = Vote::new(new_term, self.config.id.clone()); - self.pre_vote_round = self.pre_vote_round.wrapping_add(1); - let pre_candidate = self.new_pre_candidate(pre_vote.clone()); - tracing::info!("{}, new pre-candidate: {}", func_name!(), pre_candidate); + let round = self.new_pre_vote_round(pre_vote.clone()); + let id = self.config.id.clone(); + let pre_round = self.pre_vote_rounds.latest_mut().unwrap(); + tracing::info!( + "{}, new Pre-Vote round {}: {}", + func_name!(), + round, + pre_round.candidate + ); - let last_log_id = pre_candidate.last_log_id().cloned(); + let last_log_id = pre_round.candidate.last_log_id().cloned(); // A voter grants its own Pre-Vote. A single voter reaches a quorum at once and starts the // real election without a round trip. - let id = self.config.id.clone(); - if self.pre_candidate.as_mut().unwrap().grant_by(&id) { + if pre_round.candidate.grant_by(&id) { self.elect(); return; } self.output.push_command(Command::SendPreVote { vote_req: VoteRequest::new(pre_vote, last_log_id), - round: self.pre_vote_round, + round, }); } @@ -350,8 +349,22 @@ where C: RaftTypeConfig self.candidate.as_ref() } + /// The quorum tracker of the latest Pre-Vote round, if it is still open. + #[cfg(test)] pub(crate) fn pre_candidate_ref(&self) -> Option<&Candidate>> { - self.pre_candidate.as_ref() + self.pre_vote_rounds.latest().map(|round| &round.candidate) + } + + /// Whether a new Pre-Vote round may start at `now`; see [`PreVoteRounds`]. + /// + /// The replies to a round are due one Pre-Vote deadline after it started: requests are + /// abandoned after `election_timeout_min`. A rejected round is retried after the width of the + /// election-timeout window. + pub(crate) fn pre_vote_round_due(&mut self, now: InstantOf) -> bool { + let deadline = Duration::from_millis(self.config.election_timeout_min); + let window = + Duration::from_millis(self.config.election_timeout_max.saturating_sub(self.config.election_timeout_min)); + self.pre_vote_rounds.next_round_due(now, window, deadline) } pub(crate) fn candidate_mut(&mut self) -> Option<&mut Candidate>> { @@ -522,13 +535,15 @@ where C: RaftTypeConfig /// Handle a reply to the Pre-Vote round `round`. /// - /// Count a granted Pre-Vote; once a quorum would grant, start the real election. A rejection - /// that reports a greater log delays the next campaign, and one that reports a strictly higher - /// vote catches this node up to that vote in non-committed form, as a rejected real vote does. + /// Count a granted Pre-Vote for its round; once a quorum would grant one round, start the real + /// election. A rejection marks its round rejected, so that a new round may start before its + /// remaining replies are due; one that reports a greater log also delays the next campaign, and + /// one that reports a strictly higher vote catches this node up to that vote in non-committed + /// form, as a rejected real vote does. /// - /// A voter that cannot answer Pre-Vote ends the round, and this node runs the classic election - /// for this campaign at once: Pre-Vote is used only while every voter that can be reached - /// answers it. A reply to any round but the one in flight is ignored. + /// A voter that cannot answer Pre-Vote ends every round, and this node runs the classic + /// election for this campaign at once: Pre-Vote is used only while every voter that can be + /// reached answers it. A reply to a round that is no longer open is ignored. #[tracing::instrument(level = "debug", skip(self, reply))] pub(crate) fn handle_pre_vote_resp(&mut self, target: C::NodeId, round: u64, reply: PreVoteReply) { tracing::info!( @@ -541,19 +556,14 @@ where C: RaftTypeConfig func_name!() ); - if round != self.pre_vote_round { + if !self.pre_vote_rounds.is_open(round) { tracing::info!( - current_round = self.pre_vote_round, - "ignore a reply to an earlier Pre-Vote round" + latest_round = self.pre_vote_rounds.latest_id(), + "ignore a reply to a Pre-Vote round that is no longer open" ); return; } - let Some(pre_candidate) = self.pre_candidate.as_mut() else { - // The Pre-Vote round has finished or been canceled; ignore the delayed reply. - return; - }; - let resp = match reply { PreVoteReply::Answered(resp) => resp, PreVoteReply::Unsupported => { @@ -563,7 +573,7 @@ where C: RaftTypeConfig } // The voter can still grant a vote, but it can never grant a Pre-Vote: without it // a Pre-Vote quorum may never form. Campaign as a node without Pre-Vote does. - self.pre_candidate = None; + self.pre_vote_rounds.clear(); if self.leader.is_some() || !self.state.membership_state.effective().is_voter(&self.config.id) { tracing::info!("a voter cannot answer Pre-Vote, but this node leads or no longer votes"); return; @@ -576,18 +586,24 @@ where C: RaftTypeConfig // The responder does not adopt the Pre-Vote, so a grant is read from `vote_granted`. if resp.vote_granted { - if pre_candidate.grant_by(&target) { - self.pre_candidate = None; + let quorum_granted = + self.pre_vote_rounds.get_mut(round).is_some_and(|pre_round| pre_round.candidate.grant_by(&target)); + if quorum_granted { + self.pre_vote_rounds.clear(); if self.leader.is_some() || !self.state.membership_state.effective().is_voter(&self.config.id) { tracing::info!("a quorum would grant the vote, but this node leads or no longer votes"); return; } - tracing::info!("a quorum would grant the vote; start a real election"); + tracing::info!(round, "a quorum would grant the vote; start a real election"); self.elect(); } return; } + if let Some(pre_round) = self.pre_vote_rounds.get_mut(round) { + pre_round.rejected = true; + } + if resp.last_log_id.as_ref() > self.state.last_log_id() { tracing::info!( greater_log_id = display(resp.last_log_id.summary()), @@ -598,9 +614,9 @@ where C: RaftTypeConfig } // Catch up to a strictly higher vote, never as committed. An equal vote is left alone: it - // would end the in-flight round without new information. A vote for this node is not - // adopted either: no voter holds one for a term this node did not campaign in, so it can - // only be a faulty answer, and adopting it would vote for this node unsent. + // would end the open rounds without new information. A vote for this node is not adopted + // either: no voter holds one for a term this node did not campaign in, so it can only be a + // faulty answer, and adopting it would vote for this node unsent. if &resp.vote > self.state.vote_ref() && resp.vote.leader_id().voted_for() != Some(self.config.id.clone()) { let mut vote = resp.vote.clone(); vote.committed = false; @@ -914,8 +930,8 @@ where C: RaftTypeConfig fn establish_leader(&mut self) { tracing::info!("{}", func_name!()); - // An overlapping Pre-Vote round must not start another campaign once this node leads. - self.pre_candidate = None; + // An open Pre-Vote round must not start another campaign once this node leads. + self.pre_vote_rounds.clear(); let candidate = self.candidate.take().unwrap(); let leader = self.establish_handler().establish(candidate); @@ -1007,7 +1023,7 @@ where C: RaftTypeConfig output: &mut self.output, leader: &mut self.leader, candidate: &mut self.candidate, - pre_candidate: &mut self.pre_candidate, + pre_vote_rounds: &mut self.pre_vote_rounds, } } diff --git a/openraft/src/engine/handler/vote_handler/mod.rs b/openraft/src/engine/handler/vote_handler/mod.rs index 8a93acadd..cbdd44f5f 100644 --- a/openraft/src/engine/handler/vote_handler/mod.rs +++ b/openraft/src/engine/handler/vote_handler/mod.rs @@ -4,6 +4,7 @@ use crate::core::raft_msg::ResultSender; use crate::engine::handler::leader_handler::LeaderHandler; use crate::engine::handler::replication_handler::ReplicationHandler; use crate::engine::handler::server_state_handler::ServerStateHandler; +use crate::engine::pre_vote_rounds::PreVoteRounds; use crate::engine::Command; use crate::engine::EngineConfig; use crate::engine::EngineOutput; @@ -40,7 +41,7 @@ where C: RaftTypeConfig pub(crate) output: &'st mut EngineOutput, pub(crate) leader: &'st mut LeaderState, pub(crate) candidate: &'st mut CandidateState, - pub(crate) pre_candidate: &'st mut CandidateState, + pub(crate) pre_vote_rounds: &'st mut PreVoteRounds, } impl VoteHandler<'_, C> @@ -119,10 +120,10 @@ where C: RaftTypeConfig tracing::debug!(now = debug(C::now()), "{}", func_name!()); - // An accepted vote, including a heartbeat from the current leader, ends an in-flight - // Pre-Vote round: its outcome no longer reflects this node's view of the cluster. The - // periodic re-evaluation of the server state does not, because it changes no vote. - *self.pre_candidate = None; + // An accepted vote, including a heartbeat from the current leader, ends every open Pre-Vote + // round: their outcome no longer reflects this node's view of the cluster. The periodic + // re-evaluation of the server state does not, because it changes no vote. + self.pre_vote_rounds.clear(); self.update_internal_server_state(); diff --git a/openraft/src/engine/leadership_transfer.rs b/openraft/src/engine/leadership_transfer.rs index 76199d089..c154eade4 100644 --- a/openraft/src/engine/leadership_transfer.rs +++ b/openraft/src/engine/leadership_transfer.rs @@ -35,7 +35,7 @@ impl Engine { None }; self.candidate = None; - self.pre_candidate = None; + self.pre_vote_rounds.clear(); self.prepared_shutdown = Some(PreparedShutdown { handoff: handoff.clone(), }); diff --git a/openraft/src/engine/mod.rs b/openraft/src/engine/mod.rs index d3cf8f70c..cbd27c3b8 100644 --- a/openraft/src/engine/mod.rs +++ b/openraft/src/engine/mod.rs @@ -34,6 +34,7 @@ mod engine_impl; mod engine_output; mod leadership_transfer; mod log_id_list; +pub(crate) mod pre_vote_rounds; pub(crate) mod handler; pub(crate) mod time_state; diff --git a/openraft/src/engine/pre_vote_rounds.rs b/openraft/src/engine/pre_vote_rounds.rs new file mode 100644 index 000000000..153da4d66 --- /dev/null +++ b/openraft/src/engine/pre_vote_rounds.rs @@ -0,0 +1,128 @@ +//! The Pre-Vote rounds of a node that may still collect replies. + +use std::time::Duration; + +use crate::proposer::Candidate; +use crate::proposer::LeaderQuorumSet; +use crate::type_config::alias::InstantOf; +use crate::RaftTypeConfig; + +/// One Pre-Vote round. +#[derive(Debug)] +pub(crate) struct PreVoteRound +where C: RaftTypeConfig +{ + /// The identifier that every reply to this round names. + pub(crate) id: u64, + + /// The quorum tracker of this round. Its vote is the hypothetical next-term vote, never + /// persisted. + pub(crate) candidate: Candidate>, + + /// Whether a voter rejected this round. + pub(crate) rejected: bool, +} + +/// The Pre-Vote rounds of a node that may still collect replies, oldest first. +/// +/// A round stays open until its replies are due, one Pre-Vote deadline after it started: a request +/// that is not answered by then never is, and no reply to an open round is discarded. While the +/// latest round awaits replies and no voter rejected it, no new round starts. Once a voter rejects +/// it, a new round may start after the width of the election-timeout window, and the rejected +/// round stays open beside it: a late grant still counts for the round it answers. +/// +/// Rounds in one term propose the same hypothetical vote, so every reply names its round by an +/// identifier. Accepting a vote, campaigning or leading ends every open round. +#[derive(Debug)] +pub(crate) struct PreVoteRounds +where C: RaftTypeConfig +{ + rounds: Vec>, + + /// The identifier of the latest round started, open or not. + latest_id: u64, +} + +impl Default for PreVoteRounds +where C: RaftTypeConfig +{ + fn default() -> Self { + Self { + rounds: Vec::new(), + latest_id: 0, + } + } +} + +impl PreVoteRounds +where C: RaftTypeConfig +{ + /// Open a new round tracked by `candidate`, and return its identifier. + pub(crate) fn open(&mut self, candidate: Candidate>) -> u64 { + self.latest_id = self.latest_id.wrapping_add(1); + self.rounds.push(PreVoteRound { + id: self.latest_id, + candidate, + rejected: false, + }); + self.latest_id + } + + /// The identifier of the latest round started. + pub(crate) fn latest_id(&self) -> u64 { + self.latest_id + } + + /// The latest round, if it is still open. + pub(crate) fn latest(&self) -> Option<&PreVoteRound> { + self.rounds.last().filter(|round| round.id == self.latest_id) + } + + pub(crate) fn latest_mut(&mut self) -> Option<&mut PreVoteRound> { + let latest_id = self.latest_id; + self.rounds.last_mut().filter(|round| round.id == latest_id) + } + + /// The open round `id`. + pub(crate) fn get_mut(&mut self, id: u64) -> Option<&mut PreVoteRound> { + self.rounds.iter_mut().find(|round| round.id == id) + } + + pub(crate) fn is_open(&self, id: u64) -> bool { + self.rounds.iter().any(|round| round.id == id) + } + + /// The number of open rounds. + #[cfg(test)] + pub(crate) fn len(&self) -> usize { + self.rounds.len() + } + + #[cfg(test)] + pub(crate) fn is_empty(&self) -> bool { + self.rounds.is_empty() + } + + /// End every open round. + pub(crate) fn clear(&mut self) { + self.rounds.clear(); + } + + /// Close the rounds whose replies are due by `now`, `deadline` after they started. + pub(crate) fn close_due(&mut self, now: InstantOf, deadline: Duration) { + self.rounds.retain(|round| now < round.candidate.starting_time() + deadline); + } + + /// Whether a new round may start at `now`. + /// + /// No round may start while the latest one awaits replies that no voter has rejected yet, until + /// they are due `deadline` after it started. Once a voter rejected it, a new round may start + /// `window` after it started. + pub(crate) fn next_round_due(&mut self, now: InstantOf, window: Duration, deadline: Duration) -> bool { + self.close_due(now, deadline); + match self.latest() { + None => true, + Some(latest) => latest.rejected && now >= latest.candidate.starting_time() + window, + } + } +} diff --git a/openraft/src/engine/tests/pre_vote_test.rs b/openraft/src/engine/tests/pre_vote_test.rs index 48d8be0c9..f3259c738 100644 --- a/openraft/src/engine/tests/pre_vote_test.rs +++ b/openraft/src/engine/tests/pre_vote_test.rs @@ -188,7 +188,7 @@ fn test_handle_pre_vote_resp_quorum_starts_real_election() -> anyhow::Result<()> eng.handle_pre_vote_resp( 3, - eng.pre_vote_round, + eng.pre_vote_rounds.latest_id(), PreVoteReply::Answered(VoteResponse::new( Vote::new_committed(1, 2), Some(log_id(1, 2, 3)), @@ -226,7 +226,7 @@ fn test_handle_pre_vote_resp_rejection_keeps_waiting() -> anyhow::Result<()> { eng.handle_pre_vote_resp( 3, - eng.pre_vote_round, + eng.pre_vote_rounds.latest_id(), PreVoteReply::Answered(VoteResponse::new( Vote::new_committed(1, 2), Some(log_id(1, 2, 3)), @@ -243,7 +243,7 @@ fn test_handle_pre_vote_resp_rejection_keeps_waiting() -> anyhow::Result<()> { tracing::info!("--- a rejection that reports a greater log delays the next campaign"); eng.handle_pre_vote_resp( 2, - eng.pre_vote_round, + eng.pre_vote_rounds.latest_id(), PreVoteReply::Answered(VoteResponse::new( Vote::new_committed(1, 2), Some(log_id(1, 2, 5)), @@ -265,7 +265,7 @@ fn test_handle_pre_vote_resp_rejection_adopts_a_higher_vote() -> anyhow::Result< eng.handle_pre_vote_resp( 3, - eng.pre_vote_round, + eng.pre_vote_rounds.latest_id(), PreVoteReply::Answered(VoteResponse::new( Vote::new_committed(5, 3), Some(log_id(1, 2, 3)), @@ -292,7 +292,7 @@ fn test_handle_pre_vote_resp_rejection_with_equal_vote_keeps_the_round() -> anyh eng.handle_pre_vote_resp( 3, - eng.pre_vote_round, + eng.pre_vote_rounds.latest_id(), PreVoteReply::Answered(VoteResponse::new(vote_before, Some(log_id(1, 2, 3)), false)), ); @@ -322,7 +322,7 @@ fn test_accepted_vote_ends_the_pre_vote_round() -> anyhow::Result<()> { // A delayed grant for the ended round starts nothing. eng.handle_pre_vote_resp( 3, - eng.pre_vote_round, + eng.pre_vote_rounds.latest_id(), PreVoteReply::Answered(VoteResponse::new(leader_vote, Some(log_id(1, 2, 3)), true)), ); assert!(eng.candidate_ref().is_none()); @@ -354,7 +354,7 @@ fn test_winning_election_ends_an_overlapping_pre_vote_round() -> anyhow::Result< eng.pre_elect(); eng.handle_pre_vote_resp( 2, - eng.pre_vote_round, + eng.pre_vote_rounds.latest_id(), PreVoteReply::Answered(VoteResponse::new( Vote::new_committed(1, 2), Some(log_id(1, 2, 3)), @@ -378,7 +378,7 @@ fn test_winning_election_ends_an_overlapping_pre_vote_round() -> anyhow::Result< // A delayed grant for round B starts no campaign on the leader. eng.handle_pre_vote_resp( 2, - eng.pre_vote_round, + eng.pre_vote_rounds.latest_id(), PreVoteReply::Answered(VoteResponse::new(Vote::new(2, 1), Some(log_id(1, 2, 3)), true)), ); assert_eq!(Vote::new_committed(2, 1), *eng.state.vote_ref()); @@ -443,7 +443,7 @@ fn test_handle_pre_vote_resp_rejection_never_adopts_a_vote_for_this_node() -> an // node in a term it never campaigned in. eng.handle_pre_vote_resp( 3, - eng.pre_vote_round, + eng.pre_vote_rounds.latest_id(), PreVoteReply::Answered(VoteResponse::new(proposed, None, false)), ); @@ -467,7 +467,7 @@ fn test_delayed_pre_vote_grant_from_a_removed_voter_does_not_reach_a_later_round let mut eng = eng(m123()); eng.pre_elect(); eng.output.take_commands(); - let first_round = eng.pre_vote_round; + let first_round = eng.pre_vote_rounds.latest_id(); let first_round_vote = *eng.pre_candidate_ref().unwrap().vote_ref(); // Leader 2 replicates a membership without voter 3 in its unchanged term, which ends the round. @@ -486,7 +486,11 @@ fn test_delayed_pre_vote_grant_from_a_removed_voter_does_not_reach_a_later_round *eng.pre_candidate_ref().unwrap().vote_ref(), "both rounds propose the same vote" ); - assert_ne!(first_round, eng.pre_vote_round, "each round has its own identifier"); + assert_ne!( + first_round, + eng.pre_vote_rounds.latest_id(), + "each round has its own identifier" + ); // Voter 3's grant for the first round arrives now. let grant = VoteResponse::new(Vote::new_committed(1, 2), Some(log_id(1, 2, 3)), true); @@ -502,7 +506,7 @@ fn test_delayed_pre_vote_grant_from_a_removed_voter_does_not_reach_a_later_round assert_eq!(0, eng.output.take_commands().len()); // Even a grant that names the new round cannot count for a node outside its quorum set. - eng.handle_pre_vote_resp(3, eng.pre_vote_round, PreVoteReply::Answered(grant)); + eng.handle_pre_vote_resp(3, eng.pre_vote_rounds.latest_id(), PreVoteReply::Answered(grant)); assert!(eng.pre_candidate_ref().is_some(), "the new round keeps waiting"); assert_eq!( @@ -525,7 +529,7 @@ fn test_unsupported_pre_vote_runs_the_classic_election() -> anyhow::Result<()> { eng.pre_elect(); eng.output.take_commands(); - eng.handle_pre_vote_resp(3, eng.pre_vote_round, PreVoteReply::Unsupported); + eng.handle_pre_vote_resp(3, eng.pre_vote_rounds.latest_id(), PreVoteReply::Unsupported); assert!(eng.pre_candidate_ref().is_none(), "the classic election ends the round"); assert_eq!( @@ -554,16 +558,16 @@ fn test_unsupported_pre_vote_of_an_ended_round_or_a_non_voter_starts_nothing() - let vote_before = *eng.state.vote_ref(); // Node 3 is not a voter. - eng.handle_pre_vote_resp(3, eng.pre_vote_round, PreVoteReply::Unsupported); + eng.handle_pre_vote_resp(3, eng.pre_vote_rounds.latest_id(), PreVoteReply::Unsupported); assert!(eng.pre_candidate_ref().is_some(), "the round keeps waiting"); // A reply to an earlier round. - eng.handle_pre_vote_resp(2, eng.pre_vote_round - 1, PreVoteReply::Unsupported); + eng.handle_pre_vote_resp(2, eng.pre_vote_rounds.latest_id() - 1, PreVoteReply::Unsupported); assert!(eng.pre_candidate_ref().is_some(), "the round keeps waiting"); // A reply after a heartbeat ended the round. eng.vote_handler().update_vote(&vote_before)?; - eng.handle_pre_vote_resp(2, eng.pre_vote_round, PreVoteReply::Unsupported); + eng.handle_pre_vote_resp(2, eng.pre_vote_rounds.latest_id(), PreVoteReply::Unsupported); assert!(eng.candidate_ref().is_none(), "no election starts"); assert_eq!(vote_before, *eng.state.vote_ref()); @@ -571,3 +575,158 @@ fn test_unsupported_pre_vote_of_an_ended_round_or_a_non_voter_starts_nothing() - Ok(()) } + +/// A follower of node 2 whose election-timeout window, 100 ms wide, is shorter than the 1,000 ms +/// Pre-Vote deadline. +fn eng_with_window(membership: Membership) -> Engine { + let mut eng = eng(membership); + eng.config.election_timeout_min = 1_000; + eng.config.election_timeout_max = 1_100; + eng +} + +fn window_and_deadline(eng: &Engine) -> (Duration, Duration) { + ( + Duration::from_millis(eng.config.election_timeout_max - eng.config.election_timeout_min), + Duration::from_millis(eng.config.election_timeout_min), + ) +} + +/// A round that awaits replies no voter rejected stays the latest until they are due: a slow +/// reply that arrives after the election-timeout window must still reach it. +#[test] +fn test_unanswered_round_stays_open_until_its_replies_are_due() -> anyhow::Result<()> { + let mut eng = eng_with_window(m123()); + let (window, deadline) = window_and_deadline(&eng); + eng.pre_elect(); + eng.output.take_commands(); + let round = eng.pre_vote_rounds.latest_id(); + let started = eng.pre_candidate_ref().unwrap().starting_time(); + + assert!(!eng.pre_vote_round_due(started + window + Duration::from_millis(1))); + assert!(!eng.pre_vote_round_due(started + deadline - Duration::from_millis(1))); + + // A grant within the deadline still counts. + eng.handle_pre_vote_resp( + 3, + round, + PreVoteReply::Answered(VoteResponse::new( + Vote::new_committed(1, 2), + Some(log_id(1, 2, 3)), + true, + )), + ); + assert_eq!(Vote::new(2, 1), *eng.state.vote_ref(), "the real election starts"); + assert!(eng.pre_vote_rounds.is_empty()); + + Ok(()) +} + +/// The replies to a round are due one Pre-Vote deadline after it started; the round then closes. +#[test] +fn test_round_closes_when_its_replies_are_due() -> anyhow::Result<()> { + let mut eng = eng_with_window(m123()); + let (_window, deadline) = window_and_deadline(&eng); + eng.pre_elect(); + eng.output.take_commands(); + let round = eng.pre_vote_rounds.latest_id(); + let started = eng.pre_candidate_ref().unwrap().starting_time(); + + assert!(eng.pre_vote_round_due(started + deadline)); + assert!(eng.pre_vote_rounds.is_empty(), "the round closed"); + + // A grant that could no longer arrive in time starts nothing. + let vote_before = *eng.state.vote_ref(); + eng.handle_pre_vote_resp( + 3, + round, + PreVoteReply::Answered(VoteResponse::new( + Vote::new_committed(1, 2), + Some(log_id(1, 2, 3)), + true, + )), + ); + assert_eq!(vote_before, *eng.state.vote_ref()); + assert_eq!(0, eng.output.take_commands().len()); + + Ok(()) +} + +/// A rejected round may be retried after the window's width, and it stays open: a late grant +/// still completes it. +#[test] +fn test_rejected_round_is_retried_after_the_window_and_still_counts_late_grants() -> anyhow::Result<()> { + let mut eng = eng_with_window(m123()); + let (window, _deadline) = window_and_deadline(&eng); + eng.pre_elect(); + eng.output.take_commands(); + let first_round = eng.pre_vote_rounds.latest_id(); + let started = eng.pre_candidate_ref().unwrap().starting_time(); + + // Voter 2 still serves the lost leader and rejects. + eng.handle_pre_vote_resp( + 2, + first_round, + PreVoteReply::Answered(VoteResponse::new( + Vote::new_committed(1, 2), + Some(log_id(1, 2, 3)), + false, + )), + ); + assert!(!eng.pre_vote_round_due(started + window - Duration::from_millis(1))); + assert!(eng.pre_vote_round_due(started + window)); + + eng.pre_elect(); + eng.output.take_commands(); + assert_ne!(first_round, eng.pre_vote_rounds.latest_id(), "a new round started"); + assert_eq!(2, eng.pre_vote_rounds.len(), "the rejected round stays open"); + + // Voter 3's slow grant for the first round completes it. + eng.handle_pre_vote_resp( + 3, + first_round, + PreVoteReply::Answered(VoteResponse::new( + Vote::new_committed(1, 2), + Some(log_id(1, 2, 3)), + true, + )), + ); + assert_eq!(Vote::new(2, 1), *eng.state.vote_ref(), "the real election starts"); + assert!(eng.candidate_ref().is_some()); + assert!(eng.pre_vote_rounds.is_empty(), "the election ends every round"); + + Ok(()) +} + +/// A voter reported as unable to answer Pre-Vote after a newer round started still makes this +/// node run the classic election. +#[test] +fn test_late_unsupported_for_an_open_round_runs_the_classic_election() -> anyhow::Result<()> { + let mut eng = eng_with_window(m123()); + let (window, _deadline) = window_and_deadline(&eng); + eng.pre_elect(); + eng.output.take_commands(); + let first_round = eng.pre_vote_rounds.latest_id(); + let started = eng.pre_candidate_ref().unwrap().starting_time(); + + eng.handle_pre_vote_resp( + 2, + first_round, + PreVoteReply::Answered(VoteResponse::new( + Vote::new_committed(1, 2), + Some(log_id(1, 2, 3)), + false, + )), + ); + assert!(eng.pre_vote_round_due(started + window)); + eng.pre_elect(); + eng.output.take_commands(); + + eng.handle_pre_vote_resp(3, first_round, PreVoteReply::Unsupported); + + assert_eq!(Vote::new(2, 1), *eng.state.vote_ref(), "the classic election starts"); + assert!(eng.candidate_ref().is_some()); + assert!(eng.pre_vote_rounds.is_empty()); + + Ok(()) +} From c44cf3ff371dc427b7cd04138531b82320be85eb Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Fri, 2 Oct 2026 13:36:08 -0600 Subject: [PATCH 32/36] Test: a voter that is behind defers to a fresher classic candidate Signed-off-by: VerifiedOrganic --- tests/tests/elect/main.rs | 1 + .../elect/t27_fresher_classic_candidate.rs | 99 +++++++++++++++++++ tests/tests/fixtures/mod.rs | 13 ++- 3 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 tests/tests/elect/t27_fresher_classic_candidate.rs diff --git a/tests/tests/elect/main.rs b/tests/tests/elect/main.rs index e1870951a..6d516cda8 100644 --- a/tests/tests/elect/main.rs +++ b/tests/tests/elect/main.rs @@ -18,3 +18,4 @@ mod t23_classic_voter_stale_log; mod t24_classic_leader_one_way; mod t25_rejected_pre_vote_retry; mod t26_delayed_pre_vote_replies; +mod t27_fresher_classic_candidate; diff --git a/tests/tests/elect/t27_fresher_classic_candidate.rs b/tests/tests/elect/t27_fresher_classic_candidate.rs new file mode 100644 index 000000000..605cb5162 --- /dev/null +++ b/tests/tests/elect/t27_fresher_classic_candidate.rs @@ -0,0 +1,99 @@ +use std::sync::Arc; +use std::time::Duration; + +use anyhow::Result; +use maplit::btreeset; +use openraft::Config; +use openraft::ServerState; +use tokio::time::Instant; + +use crate::fixtures::init_default_ut_tracing; +use crate::fixtures::RaftRouter; + +/// A voter whose log is behind defers to a classic candidate with a more up-to-date log. +/// +/// Node 1 cannot answer Pre-Vote and campaigns with the classic vote on slower timers, as a voter +/// of a previous release does; only it holds the most up-to-date log after node 0 is lost. Node 2 +/// is behind, so it can never win, but it runs the classic election because node 1 cannot answer +/// Pre-Vote. Node 2 campaigns first and votes for itself; node 1's first campaign then falls in +/// the same term, and node 2 rejects it: node 1's vote is not greater than its own. If node 2 +/// campaigned again before node 1's next campaign, it would again hold that term, and each of +/// node 1's campaigns could meet another self-vote of node 2. Once node 2 rejects a candidate +/// whose log is more up to date only because of its own vote, it defers its own next campaign, so +/// node 1 wins its second campaign. +#[async_entry::test(worker_threads = 8, init = "init_default_ut_tracing()", tracing_span = "debug")] +async fn voter_behind_defers_to_a_fresher_classic_candidate() -> Result<()> { + let config = Arc::new( + Config { + heartbeat_interval: 50, + election_timeout_min: 1_000, + election_timeout_max: 1_100, + enable_pre_vote: Some(true), + ..Default::default() + } + .validate()?, + ); + let classic = Arc::new( + Config { + heartbeat_interval: 50, + election_timeout_min: 2_500, + election_timeout_max: 2_600, + ..Default::default() + } + .validate()?, + ); + let classic_tick = Duration::from_millis(classic.heartbeat_interval * 3 / 2); + // Node 1's second campaign, measured from the loss: its lease and longest timeout, then its + // longest timeout again, each on its next tick. + let second_classic_campaign = (Duration::from_millis(classic.election_timeout_max) + classic_tick) * 2; + let slack = Duration::from_millis(300); + + let mut router = RaftRouter::new(config.clone()); + router.set_node_config(1, classic.clone()); + + tracing::info!("--- create cluster of 0,1,2; node 0 becomes leader"); + let mut log_index = router.new_cluster(btreeset! {0,1,2}, btreeset! {}).await?; + router + .get_raft_handle(&0)? + .wait(timeout()) + .state(ServerState::Leader, "node 0 is the initial leader") + .await?; + + tracing::info!("--- node 1 cannot answer Pre-Vote and campaigns without it"); + let n1 = router.get_raft_handle(&1)?; + n1.runtime_config().pre_vote(false); + router.set_pre_vote_unsupported(1, true); + + tracing::info!("--- node 2 misses the last entries"); + router.set_network_error(2, true); + router.client_request_many(0, "missed", 5).await?; + log_index += 5; + n1.wait(timeout()).applied_index(Some(log_index), "node 1 has the entries").await?; + + tracing::info!("--- node 0 is lost; node 2 is reachable again"); + let lost = Instant::now(); + router.set_network_error(0, true); + router.set_network_error(2, false); + + n1.wait(Some(second_classic_campaign * 4)) + .state(ServerState::Leader, "node 1, with the most up-to-date log, is elected") + .await?; + let elected = lost.elapsed(); + tracing::info!( + "--- node 1 was elected {:?} after the loss; its second campaign is due within {:?}", + elected, + second_classic_campaign + ); + assert!( + elected <= second_classic_campaign + slack, + "node 1 was elected {:?} after the loss, after its second campaign was due within {:?}", + elected, + second_classic_campaign + ); + + Ok(()) +} + +fn timeout() -> Option { + Some(Duration::from_millis(5_000)) +} diff --git a/tests/tests/fixtures/mod.rs b/tests/tests/fixtures/mod.rs index 2892423dc..91b90d4d4 100644 --- a/tests/tests/fixtures/mod.rs +++ b/tests/tests/fixtures/mod.rs @@ -274,6 +274,9 @@ pub struct TypedRaftRouter { /// How long the reply to a Pre-Vote sent to a node takes to arrive, per target. pre_vote_reply_delay: Arc>>, + /// The Raft config of a node that does not use the router's config. + node_configs: Arc>>>, + /// A hook function to be called when before an RPC is sent to target node. rpc_pre_hook: Arc>>, @@ -322,6 +325,7 @@ impl Builder { rpc_count: Default::default(), pre_vote_unsupported: Default::default(), pre_vote_reply_delay: Default::default(), + node_configs: Default::default(), rpc_pre_hook: Default::default(), rpc_observers: Default::default(), blocked_rpc: Default::default(), @@ -513,8 +517,15 @@ impl TypedRaftRouter { } #[tracing::instrument(level = "debug", skip_all)] + /// Create node `id` with `config` instead of the router's config, such as a node of another + /// release with other timers. It applies to nodes created afterwards. + pub fn set_node_config(&self, id: MemNodeId, config: Arc) { + self.node_configs.lock().unwrap().insert(id, config); + } + pub async fn new_raft_node_with_sto(&mut self, id: MemNodeId, log_store: MemLogStore, sm: MemStateMachine) { - let node = Raft::new(id, self.config.clone(), self.clone(), log_store.clone(), sm.clone()).await.unwrap(); + let config = self.node_configs.lock().unwrap().get(&id).cloned().unwrap_or_else(|| self.config.clone()); + let node = Raft::new(id, config, self.clone(), log_store.clone(), sm.clone()).await.unwrap(); let mut rt = self.nodes.lock().unwrap(); rt.insert(id, (node, log_store, sm)); } From 6308d8bfd134cd95cd334e6f9c77764a1d18fc83 Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Fri, 2 Oct 2026 13:39:05 -0600 Subject: [PATCH 33/36] Fix: defer the next campaign after refusing a fresher candidate A candidate whose log is behind can never win against a voter with a more up-to-date log, but it may still campaign with the classic vote, for instance when that voter cannot answer Pre-Vote. Each of its campaigns persists a vote for itself in its next term. A more up-to-date candidate that campaigned in the same term was refused, and when the stale candidate campaigned again before the next attempt, that attempt met another self-vote in its term. Openraft does not adopt the term of a vote request it refuses for its log, so the up-to-date candidate never learned to skip ahead: with three voters, it won only its third or fourth campaign. A campaigning node that refuses a candidate with a greater log only because of its own vote now defers its next campaign by the greater-log timeout from that moment, as it would after seeing a greater log in a vote response. The candidate's next campaign then finds no new self-vote, and this node grants it. Only timers change; no vote or log rule does. In the regression, the up-to-date classic candidate is now elected on its second campaign, after 5.0 to 5.3 s instead of 7.6 to 10.5 s. Signed-off-by: VerifiedOrganic --- openraft/src/engine/engine_impl.rs | 21 +++++++++ .../src/engine/tests/handle_vote_req_test.rs | 45 +++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/openraft/src/engine/engine_impl.rs b/openraft/src/engine/engine_impl.rs index 837baaffd..e4e3bc7be 100644 --- a/openraft/src/engine/engine_impl.rs +++ b/openraft/src/engine/engine_impl.rs @@ -469,6 +469,21 @@ where C: RaftTypeConfig let res = self.vote_handler().update_vote(&req.vote); + if res.is_err() && req.last_log_id.as_ref() > self.state.last_log_id() && self.is_campaigning() { + // A candidate whose log is more up to date than this node's is refused only because this + // node voted for itself, which cannot win against that log. Its next campaign is in a + // later term; had this node campaigned again before it, that term would hold another + // self-vote, and so on. Defer the next campaign by the greater-log timeout from now, as + // after seeing a greater log in a vote response: the candidate's next campaign finds no + // new self-vote, and this node grants it. + tracing::info!( + req = display(req.summary()), + "defer the next campaign to a candidate with a greater log" + ); + self.set_greater_log(); + self.state.vote.touch(C::now()); + } + tracing::info!( req = display(req.summary()), result = debug(&res), @@ -992,6 +1007,12 @@ where C: RaftTypeConfig } } + /// Whether this node campaigns: it holds an uncommitted vote for itself. + fn is_campaigning(&self) -> bool { + let vote = self.state.vote_ref(); + !vote.is_committed() && vote.leader_id().voted_for().as_ref() == Some(&self.config.id) + } + pub(crate) fn is_there_greater_log(&self) -> bool { self.seen_greater_log } diff --git a/openraft/src/engine/tests/handle_vote_req_test.rs b/openraft/src/engine/tests/handle_vote_req_test.rs index b9c2e48c7..41bcd28d1 100644 --- a/openraft/src/engine/tests/handle_vote_req_test.rs +++ b/openraft/src/engine/tests/handle_vote_req_test.rs @@ -282,3 +282,48 @@ fn test_handle_vote_req_granted_follower_learner_does_not_emit_update_server_sta } Ok(()) } + +/// A candidate refused only because this campaigning node voted for itself, whose log is more up +/// to date, makes this node defer its next campaign: by the greater-log timeout, from now. +#[test] +fn test_handle_vote_req_refused_fresher_candidate_defers_the_next_campaign() -> anyhow::Result<()> { + let mut eng = eng(); + eng.state.log_ids = LogIdList::new(vec![log_id(2, 1, 3)]); + let utime_before = eng.state.vote_last_modified().unwrap(); + assert!(!eng.is_there_greater_log()); + + let resp = eng.handle_vote_req(VoteRequest { + vote: Vote::new(2, 0), + last_log_id: Some(log_id(2, 1, 4)), + }); + + assert!(!resp.vote_granted, "this node already voted for itself in term 2"); + assert_eq!(Vote::new(2, 1), *eng.state.vote_ref()); + assert!(eng.is_there_greater_log(), "the greater-log timeout applies"); + assert!( + eng.state.vote_last_modified().unwrap() > utime_before, + "the election timer restarts from now" + ); + assert_eq!(0, eng.output.take_commands().len(), "nothing is persisted"); + + Ok(()) +} + +/// A candidate refused for its vote whose log is not more up to date changes nothing. +#[test] +fn test_handle_vote_req_refused_candidate_without_a_greater_log_defers_nothing() -> anyhow::Result<()> { + let mut eng = eng(); + eng.state.log_ids = LogIdList::new(vec![log_id(2, 1, 3)]); + let utime_before = eng.state.vote_last_modified(); + + let resp = eng.handle_vote_req(VoteRequest { + vote: Vote::new(2, 0), + last_log_id: Some(log_id(2, 1, 3)), + }); + + assert!(!resp.vote_granted); + assert!(!eng.is_there_greater_log()); + assert_eq!(utime_before, eng.state.vote_last_modified()); + + Ok(()) +} From 20f4f3123168907d5a00ec3772c78872e9a47820 Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Fri, 2 Oct 2026 13:39:22 -0600 Subject: [PATCH 34/36] Doc: describe open Pre-Vote rounds and deferring to a fresher candidate Signed-off-by: VerifiedOrganic --- openraft/src/docs/protocol/pre_vote.md | 54 +++++++++++++++++++------- 1 file changed, 41 insertions(+), 13 deletions(-) diff --git a/openraft/src/docs/protocol/pre_vote.md b/openraft/src/docs/protocol/pre_vote.md index 7a69164dc..1917fbd2a 100644 --- a/openraft/src/docs/protocol/pre_vote.md +++ b/openraft/src/docs/protocol/pre_vote.md @@ -65,8 +65,9 @@ including a heartbeat from the current leader, ends the round. Each round has its own identifier, which every response names. Rounds in one term propose the same hypothetical vote, so the vote cannot tell a delayed -response from a current one; a response to any round but the one in flight is -ignored. A grant from a node that does not vote in the round never counts. +response from a current one; a response names the round it answers, and it +counts only for that round. A grant from a node that does not vote in the round +never counts. Only a response counts toward the quorum. An error, including an unreachable peer, is never a grant, so a voter that is cut off cannot assemble one. @@ -74,21 +75,31 @@ peer, is never a grant, so a voter that is cut off cannot assemble one. A voter that is the only voter wins its own Pre-Vote and elects at once. -## Retrying a round +## Open rounds and retries -A round that has not reached a quorum is held for the width of the -election-timeout window, `election_timeout_max - election_timeout_min`, and -then retried on a later tick while the election timer stays expired. +A round stays open until its replies are due, one Pre-Vote deadline +(`election_timeout_min`) after it started: a request that is not answered by +then never is, and no reply to an open round is discarded. + +- While the latest round awaits replies that no voter has rejected, no new + round starts, however slowly the voters answer within the deadline. +- Once a voter rejects the latest round, a new round may start after the width + of the election-timeout window, `election_timeout_max - election_timeout_min`, + on a later tick while the election timer stays expired. The rejected round + stays open beside it: a late grant still completes it, and a late report that + a voter cannot answer Pre-Vote still starts the classic election. After an unplanned leader loss, a round fails while another voter's lease still runs. Every survivor's lease runs out within the minimum election timeout -of the loss. A survivor starts its first round within the maximum election -timeout and a tick of its last leader contact, and each retry within the -window's width and a tick of the round before it. So the survivor with the most -up-to-date log starts a round that no lease rejects within the maximum election -timeout and a tick of the loss, the same bound as its first round. Holding a -rejected round for a whole sampled election timeout instead could nearly double -that. +of the loss, and a survivor starts its first round within the maximum election +timeout and a tick of its last leader contact. When every surviving voter +answers within the window's width, each round is rejected or reaches a quorum +within that width, and each retry starts within the width and a tick of the +round before. So the survivor with the most up-to-date log starts a round that +no lease rejects within the maximum election timeout and a tick of the loss, +the same bound as its first round. A voter that answers more slowly, though +within the Pre-Vote deadline, delays the retries by the excess but never +prevents an election. ## Voters that cannot answer Pre-Vote @@ -117,6 +128,23 @@ Each campaign decides again, so a voter set that no longer holds such a voter uses Pre-Vote again. +## Deferring to a more up-to-date candidate + +A candidate whose log is behind can never win against a voter with a more +up-to-date log, but it may still campaign with the classic vote, for instance +because that voter cannot answer Pre-Vote. Each such campaign persists a vote +for itself in its next term. When a more up-to-date candidate campaigns in a +term that holds such a self-vote, it is refused, and a node does not adopt the +term of a vote request it refuses for its log, so the up-to-date candidate +learns nothing from that candidate's requests either. + +So a campaigning node that refuses a candidate with a greater log only because +of its own vote defers its next campaign by the greater-log timeout, from that +moment, as it would after seeing a greater log in a vote response. The +candidate's next campaign, in a later term, finds no new self-vote and is +granted. Only timers change; no vote or log rule does. + + ## Networks without Pre-Vote The default implementation of From 452df5e55b18840bea1311acd26cb5faafea88ab Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Fri, 2 Oct 2026 14:20:59 -0600 Subject: [PATCH 35/36] Chore: wrap a comment as the current nightly rustfmt does The examples run nightly rustfmt, which wraps comments; stable rustfmt leaves them as written. Signed-off-by: VerifiedOrganic --- openraft/src/engine/engine_impl.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/openraft/src/engine/engine_impl.rs b/openraft/src/engine/engine_impl.rs index e4e3bc7be..c5ca591df 100644 --- a/openraft/src/engine/engine_impl.rs +++ b/openraft/src/engine/engine_impl.rs @@ -470,11 +470,12 @@ where C: RaftTypeConfig let res = self.vote_handler().update_vote(&req.vote); if res.is_err() && req.last_log_id.as_ref() > self.state.last_log_id() && self.is_campaigning() { - // A candidate whose log is more up to date than this node's is refused only because this - // node voted for itself, which cannot win against that log. Its next campaign is in a - // later term; had this node campaigned again before it, that term would hold another - // self-vote, and so on. Defer the next campaign by the greater-log timeout from now, as - // after seeing a greater log in a vote response: the candidate's next campaign finds no + // A candidate whose log is more up to date than this node's is refused only because + // this node voted for itself, which cannot win against that log. Its next + // campaign is in a later term; had this node campaigned again before it, + // that term would hold another self-vote, and so on. Defer the next + // campaign by the greater-log timeout from now, as after seeing a greater + // log in a vote response: the candidate's next campaign finds no // new self-vote, and this node grants it. tracing::info!( req = display(req.summary()), From 38c448d71f1fd32e762b57026a76109098691e30 Mon Sep 17 00:00:00 2001 From: VerifiedOrganic Date: Sat, 3 Oct 2026 18:33:36 -0600 Subject: [PATCH 36/36] Test: preserve the stale voter through the retry timing assertion Keep AppendEntries capped until node 1 is elected and its timing and term assertions complete. A send admitted before leader isolation can still be delayed in the test router; clearing the quota early lets that send catch node 2 up and changes which candidate should win. Signed-off-by: VerifiedOrganic --- tests/tests/elect/t25_rejected_pre_vote_retry.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/tests/elect/t25_rejected_pre_vote_retry.rs b/tests/tests/elect/t25_rejected_pre_vote_retry.rs index 09bd78d6a..cb81fc225 100644 --- a/tests/tests/elect/t25_rejected_pre_vote_retry.rs +++ b/tests/tests/elect/t25_rejected_pre_vote_retry.rs @@ -84,8 +84,9 @@ async fn pre_vote_round_rejected_by_a_running_lease_is_retried_within_the_bound( let lost = Instant::now(); router.set_network_error(0, true); router.set_rpc_failure(1, Direction::NetRecv, None); - router.set_append_entries_quota(None); + // Keep the quota at zero through the election and timing assertions. An AppendEntries RPC + // admitted before isolating node 0 can still be waiting on the router's send delay. n1.wait(Some(bound * 4)) .state(ServerState::Leader, "node 1, with the most up-to-date log, is elected") .await?; @@ -101,6 +102,7 @@ async fn pre_vote_round_rejected_by_a_running_lease_is_retried_within_the_bound( n0.metrics().borrow().current_term + 1, n1.metrics().borrow().current_term ); + router.set_append_entries_quota(None); Ok(()) }