Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix: The last leader in a cluster should step down #1056

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 44 additions & 2 deletions openraft/src/core/raft_core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1394,8 +1394,50 @@ where
// TODO: leader lease should be extended. Or it has to examine if it is leader
// before electing.
if self.engine.state.server_state == ServerState::Leader {
tracing::debug!("already a leader, do not elect again");
return;
tracing::debug!("already a leader, check lease expiration");

// TODO: the last-update time is updated to self.leader.vote.touch()
let leading = self.engine.internal_server_state.leading().unwrap();
let utime = leading.vote.utime();
// let utime = self.engine.state.vote_last_modified();

let timer_config = &self.engine.config.timer_config;

let election_timeout = timer_config.leader_lease + timer_config.election_timeout;

if utime > Some(now - election_timeout) {
// Leader lease not expired.

// Safe unwrap(): It is greater than a Some() thus must be a Some()
let last_modified = utime.unwrap();

let passed = now - last_modified;
tracing::debug!(
"Leader vote lease is not expired: {:?}(< timeout {:?}) passed the last-modified time {:?}; do not elect",
passed,
election_timeout,
last_modified
);

return;
} else {
// leader lease expired.
if let Some(last_modified) = utime {
let passed = now - last_modified;
tracing::info!(
"Leader vote lease is expired: {:?}(>=timeout {:?}) passed the last-modified time {:?}; check next election condition",
passed,
election_timeout,
last_modified
);
} else {
tracing::info!(
"Leader vote lease is considered expired: because the last-modified time is None; check next election condition",
);
}

// going on to check next condition.
}
}

if !self.engine.state.membership_state.effective().is_voter(&self.id) {
Expand Down
1 change: 1 addition & 0 deletions tests/tests/append_entries/t61_heartbeat_reject_vote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ async fn heartbeat_reject_vote() -> Result<()> {
let config = Arc::new(
Config {
heartbeat_interval: 200,
enable_elect: false,
election_timeout_min: 1000,
election_timeout_max: 1001,
..Default::default()
Expand Down
1 change: 1 addition & 0 deletions tests/tests/elect/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ mod fixtures;

mod t10_elect_compare_last_log;
mod t11_elect_seize_leadership;
mod t50_last_leader_expire;
61 changes: 61 additions & 0 deletions tests/tests/elect/t50_last_leader_expire.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
use std::sync::Arc;
use std::time::Duration;

use anyhow::Result;
use maplit::btreeset;
use openraft::AsyncRuntime;
use openraft::Config;
use openraft::RaftTypeConfig;
use openraft::ServerState;
use openraft_memstore::TypeConfig;

use crate::fixtures::init_default_ut_tracing;
use crate::fixtures::RaftRouter;

/// The leader leader should expire and re-elect,
/// even when the leader is the last live node in a cluster.
#[async_entry::test(worker_threads = 8, init = "init_default_ut_tracing()", tracing_span = "debug")]
async fn last_leader_expire() -> Result<()> {
let config = Arc::new(
Config {
enable_heartbeat: false,
..Default::default()
}
.validate()?,
);

let mut router = RaftRouter::new(config.clone());

tracing::info!("--- bring up cluster of 1 node");
let log_index = router.new_cluster(btreeset! {0,1}, btreeset! {}).await?;

tracing::info!(log_index, "--- shutdown node-1");
let (n1, _sto, _sm) = router.remove_node(1).unwrap();
n1.shutdown().await?;

tracing::info!(log_index, "--- ");
{
let n0 = router.get_raft_handle(&0)?;
let last_modify = n0.with_raft_state(|st| st.vote_last_modified()).await?;
let last_modify = last_modify.unwrap();
let now = <<TypeConfig as RaftTypeConfig>::AsyncRuntime as AsyncRuntime>::Instant::now();
println!(
"last_modify: {:?}, now: {:?}, {:?}",
last_modify,
now,
now - last_modify
);
}

tracing::info!(log_index, "--- node-0 should expire leader lease and step down");
{
router.wait(&0, timeout()).metrics(|m| m.current_leader.is_none(), "node-0 step down").await?;
router.wait(&0, timeout()).metrics(|m| m.state != ServerState::Leader, "node-0 step down").await?;
}

Ok(())
}

fn timeout() -> Option<Duration> {
Some(Duration::from_millis(2_000))
}
2 changes: 1 addition & 1 deletion tests/tests/fixtures/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ use openraft_memstore::IntoMemClientRequest;
use openraft_memstore::MemLogStore as LogStoreInner;
use openraft_memstore::MemNodeId;
use openraft_memstore::MemStateMachine as SMInner;
use openraft_memstore::TypeConfig;
pub use openraft_memstore::TypeConfig;
use openraft_memstore::TypeConfig as MemConfig;
#[allow(unused_imports)] use pretty_assertions::assert_eq;
#[allow(unused_imports)] use pretty_assertions::assert_ne;
Expand Down
Loading