From b230bc2aab13f5a8b7346659dbc9dc9e5dbdc6d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E7=82=8E=E6=B3=BC?= Date: Tue, 24 Dec 2024 09:51:59 +0800 Subject: [PATCH 1/4] Feature: Abstract `LeaderId` and `CommittedLeaderId` Add `LeaderId: RaftLeaderId` to `RaftTypeConfig` to allow customizing leader ID implementations. - Part of #1278 --- openraft/src/core/raft_core.rs | 5 +- openraft/src/core/tick.rs | 1 + openraft/src/engine/engine_impl.rs | 3 +- .../engine/handler/establish_handler/mod.rs | 5 +- .../leader_handler/append_entries_test.rs | 12 +--- .../log_handler/calc_purge_upto_test.rs | 5 +- .../append_membership_test.rs | 8 +-- .../src/engine/handler/vote_handler/mod.rs | 3 +- openraft/src/engine/testing.rs | 1 + openraft/src/engine/tests/elect_test.rs | 4 +- .../src/engine/tests/handle_vote_resp_test.rs | 4 +- openraft/src/engine/tests/initialize_test.rs | 16 +---- .../engine/tests/trigger_purge_log_test.rs | 4 +- openraft/src/impls/mod.rs | 13 ++++ openraft/src/log_id/mod.rs | 10 +-- openraft/src/log_id/raft_log_id.rs | 4 +- openraft/src/log_id_range.rs | 10 ++- openraft/src/metrics/wait_test.rs | 14 ++-- openraft/src/progress/entry/tests.rs | 5 +- openraft/src/progress/inflight/mod.rs | 3 +- openraft/src/progress/inflight/tests.rs | 5 +- openraft/src/proposer/leader.rs | 3 +- openraft/src/raft/declare_raft_types_test.rs | 10 --- openraft/src/raft/mod.rs | 2 + openraft/src/raft_state/mod.rs | 5 +- .../raft_state/tests/log_state_reader_test.rs | 7 +- .../src/raft_state/tests/validate_test.rs | 7 +- openraft/src/replication/mod.rs | 3 +- openraft/src/testing/common.rs | 4 +- openraft/src/testing/log/suite.rs | 8 +-- openraft/src/type_config.rs | 16 +++-- openraft/src/vote/committed.rs | 5 +- .../src/vote/leader_id/impl_into_leader_id.rs | 11 --- openraft/src/vote/leader_id/leader_id_adv.rs | 65 +++++++++--------- openraft/src/vote/leader_id/leader_id_std.rs | 42 ++++++------ openraft/src/vote/leader_id/mod.rs | 7 +- .../leader_id/raft_committed_leader_id.rs | 45 ++++++++++++ openraft/src/vote/leader_id/raft_leader_id.rs | 68 +++++++++++++++++++ openraft/src/vote/mod.rs | 5 +- openraft/src/vote/ref_vote.rs | 7 +- openraft/src/vote/vote.rs | 21 +++--- .../t10_conflict_with_empty_entries.rs | 15 ++-- .../append_entries/t10_see_higher_vote.rs | 5 +- .../append_entries/t11_append_conflicts.rs | 43 ++++++------ .../t11_append_entries_with_bigger_term.rs | 14 +--- .../t11_append_updates_membership.rs | 13 ++-- tests/tests/client_api/t10_client_writes.rs | 5 +- .../tests/client_api/t13_trigger_snapshot.rs | 13 +--- tests/tests/fixtures/mod.rs | 16 +++-- tests/tests/life_cycle/t10_initialization.rs | 12 ++-- .../life_cycle/t50_single_follower_restart.rs | 4 +- tests/tests/membership/t10_single_node.rs | 13 +--- tests/tests/membership/t11_add_learner.rs | 11 +-- tests/tests/membership/t31_remove_leader.rs | 5 +- tests/tests/metrics/t30_leader_metrics.rs | 6 +- .../snapshot_building/t10_build_snapshot.rs | 24 +++---- .../t30_purge_in_snapshot_logs.rs | 27 ++------ .../t31_snapshot_overrides_membership.rs | 29 ++------ .../t32_snapshot_uses_prev_snap_membership.rs | 21 +----- .../t33_snapshot_delete_conflict_logs.rs | 17 ++--- .../t34_replication_does_not_block_purge.rs | 8 +-- .../t50_snapshot_line_rate_to_snapshot.rs | 17 +---- .../t50_snapshot_when_lacking_log.rs | 25 ++----- ..._snapshot_add_learner_and_request_a_log.rs | 9 +-- .../t20_state_machine_apply_membership.rs | 7 +- 65 files changed, 392 insertions(+), 443 deletions(-) delete mode 100644 openraft/src/vote/leader_id/impl_into_leader_id.rs create mode 100644 openraft/src/vote/leader_id/raft_committed_leader_id.rs create mode 100644 openraft/src/vote/leader_id/raft_leader_id.rs diff --git a/openraft/src/core/raft_core.rs b/openraft/src/core/raft_core.rs index 9345a1012..8a17aa944 100644 --- a/openraft/src/core/raft_core.rs +++ b/openraft/src/core/raft_core.rs @@ -96,6 +96,7 @@ use crate::type_config::TypeConfigExt; use crate::vote::vote_status::VoteStatus; use crate::vote::CommittedVote; use crate::vote::NonCommittedVote; +use crate::vote::RaftLeaderId; use crate::ChangeMembers; use crate::Instant; use crate::LogId; @@ -583,7 +584,7 @@ where id: self.id.clone(), // --- data --- - current_term: st.vote_ref().leader_id().get_term(), + current_term: st.vote_ref().leader_id().term(), vote: st.io_state().io_progress.flushed().map(|io_id| io_id.to_vote()).unwrap_or_default(), last_log_index: st.last_log_id().index(), last_applied: st.io_applied().cloned(), @@ -726,7 +727,7 @@ where } // Safe unwrap(): vote that is committed has to already have voted for some node. - let id = vote.leader_id().voted_for().unwrap(); + let id = vote.leader_id().node_id_ref().cloned().unwrap(); // TODO: `is_voter()` is slow, maybe cache `current_leader`, // e.g., only update it when membership or vote changes diff --git a/openraft/src/core/tick.rs b/openraft/src/core/tick.rs index 828163925..8a809d063 100644 --- a/openraft/src/core/tick.rs +++ b/openraft/src/core/tick.rs @@ -180,6 +180,7 @@ mod tests { type NodeId = u64; type Node = (); type Term = u64; + type LeaderId = crate::impls::LeaderId; type Entry = crate::Entry; type SnapshotData = Cursor>; type AsyncRuntime = TokioRuntime; diff --git a/openraft/src/engine/engine_impl.rs b/openraft/src/engine/engine_impl.rs index b881d2880..9366e2344 100644 --- a/openraft/src/engine/engine_impl.rs +++ b/openraft/src/engine/engine_impl.rs @@ -48,6 +48,7 @@ use crate::type_config::alias::ResponderOf; use crate::type_config::alias::SnapshotDataOf; use crate::type_config::TypeConfigExt; use crate::vote::raft_term::RaftTerm; +use crate::vote::RaftLeaderId; use crate::LogId; use crate::LogIdOptionExt; use crate::Membership; @@ -209,7 +210,7 @@ where C: RaftTypeConfig /// Start to elect this node as leader #[tracing::instrument(level = "debug", skip(self))] pub(crate) fn elect(&mut self) { - let new_term = self.state.vote.leader_id().term.next(); + let new_term = self.state.vote.leader_id().term().next(); let new_vote = Vote::new(new_term, self.config.id.clone()); let candidate = self.new_candidate(new_vote.clone()); diff --git a/openraft/src/engine/handler/establish_handler/mod.rs b/openraft/src/engine/handler/establish_handler/mod.rs index 7506209ff..3cf3879fb 100644 --- a/openraft/src/engine/handler/establish_handler/mod.rs +++ b/openraft/src/engine/handler/establish_handler/mod.rs @@ -3,6 +3,7 @@ use crate::proposer::Candidate; use crate::proposer::Leader; use crate::proposer::LeaderQuorumSet; use crate::proposer::LeaderState; +use crate::vote::RaftLeaderId; use crate::RaftTypeConfig; /// Establish a leader for the Engine, when Candidate finishes voting stage. @@ -24,8 +25,8 @@ where C: RaftTypeConfig let vote = candidate.vote_ref().clone(); debug_assert_eq!( - vote.leader_id().voted_for(), - Some(self.config.id.clone()), + vote.leader_id().node_id_ref(), + Some(&self.config.id), "it can only commit its own vote" ); diff --git a/openraft/src/engine/handler/leader_handler/append_entries_test.rs b/openraft/src/engine/handler/leader_handler/append_entries_test.rs index 36cf68aeb..18f6a8408 100644 --- a/openraft/src/engine/handler/leader_handler/append_entries_test.rs +++ b/openraft/src/engine/handler/leader_handler/append_entries_test.rs @@ -23,10 +23,8 @@ use crate::testing::blank_ent; use crate::testing::log_id; use crate::type_config::TypeConfigExt; use crate::utime::Leased; -use crate::vote::CommittedLeaderId; use crate::EffectiveMembership; use crate::Entry; -use crate::LogId; use crate::Membership; use crate::MembershipState; use crate::Vote; @@ -131,10 +129,7 @@ fn test_leader_append_entries_normal() -> anyhow::Result<()> { ], eng.state.log_ids.key_log_ids() ); - assert_eq!( - Some(&LogId::new(CommittedLeaderId::new(3, 1), 6)), - eng.state.last_log_id() - ); + assert_eq!(Some(&log_id(3, 1, 6)), eng.state.last_log_id()); assert_eq!( MembershipState::new( Arc::new(EffectiveMembership::new(Some(log_id(1, 1, 1)), m01())), @@ -258,10 +253,7 @@ fn test_leader_append_entries_with_membership_log() -> anyhow::Result<()> { ], eng.state.log_ids.key_log_ids() ); - assert_eq!( - Some(&LogId::new(CommittedLeaderId::new(3, 1), 6)), - eng.state.last_log_id() - ); + assert_eq!(Some(&log_id(3, 1, 6)), eng.state.last_log_id()); assert_eq!( MembershipState::new( Arc::new(EffectiveMembership::new(Some(log_id(2, 1, 3)), m1())), diff --git a/openraft/src/engine/handler/log_handler/calc_purge_upto_test.rs b/openraft/src/engine/handler/log_handler/calc_purge_upto_test.rs index fb03e2c44..ca98c730a 100644 --- a/openraft/src/engine/handler/log_handler/calc_purge_upto_test.rs +++ b/openraft/src/engine/handler/log_handler/calc_purge_upto_test.rs @@ -1,12 +1,13 @@ use crate::engine::testing::UTConfig; use crate::engine::Engine; use crate::engine::LogIdList; -use crate::CommittedLeaderId; +use crate::type_config::alias::LeaderIdOf; +use crate::vote::RaftLeaderIdExt; use crate::LogId; fn log_id(term: u64, index: u64) -> LogId { LogId { - leader_id: CommittedLeaderId::new(term, 0), + leader_id: LeaderIdOf::::new_committed(term, 0), index, } } diff --git a/openraft/src/engine/handler/replication_handler/append_membership_test.rs b/openraft/src/engine/handler/replication_handler/append_membership_test.rs index fa4254d4d..a2dd35256 100644 --- a/openraft/src/engine/handler/replication_handler/append_membership_test.rs +++ b/openraft/src/engine/handler/replication_handler/append_membership_test.rs @@ -16,9 +16,7 @@ use crate::progress::Progress; use crate::testing::log_id; use crate::type_config::TypeConfigExt; use crate::utime::Leased; -use crate::CommittedLeaderId; use crate::EffectiveMembership; -use crate::LogId; use crate::Membership; use crate::MembershipState; use crate::Vote; @@ -110,11 +108,7 @@ fn test_leader_append_membership_update_learner_process() -> anyhow::Result<()> // learner or vice versa. let mut eng = eng(); - eng.state.log_ids = LogIdList::new([ - LogId::new(CommittedLeaderId::new(0, 0), 0), - log_id(1, 1, 1), - log_id(5, 1, 10), - ]); + eng.state.log_ids = LogIdList::new([log_id(0, 0, 0), log_id(1, 1, 1), log_id(5, 1, 10)]); eng.state .membership_state diff --git a/openraft/src/engine/handler/vote_handler/mod.rs b/openraft/src/engine/handler/vote_handler/mod.rs index 7da6c0b51..b936075cb 100644 --- a/openraft/src/engine/handler/vote_handler/mod.rs +++ b/openraft/src/engine/handler/vote_handler/mod.rs @@ -18,6 +18,7 @@ use crate::proposer::LeaderState; use crate::raft_state::IOId; use crate::raft_state::LogStateReader; use crate::type_config::TypeConfigExt; +use crate::vote::RaftLeaderId; use crate::LogId; use crate::OptionalSend; use crate::RaftState; @@ -220,7 +221,7 @@ where C: RaftTypeConfig /// This node then becomes raft-follower or raft-learner. pub(crate) fn become_following(&mut self) { debug_assert!( - self.state.vote_ref().leader_id().voted_for().as_ref() != Some(&self.config.id) + self.state.vote_ref().leader_id().node_id_ref() != Some(&self.config.id) || !self.state.membership_state.effective().membership().is_voter(&self.config.id), "It must hold: vote is not mine, or I am not a voter(leader just left the cluster)" ); diff --git a/openraft/src/engine/testing.rs b/openraft/src/engine/testing.rs index 0f816ef54..8de27b978 100644 --- a/openraft/src/engine/testing.rs +++ b/openraft/src/engine/testing.rs @@ -37,6 +37,7 @@ where N: Node + Ord type Node = N; type Entry = crate::Entry; type Term = u64; + type LeaderId = crate::impls::LeaderId; type SnapshotData = Cursor>; type AsyncRuntime = TokioRuntime; type Responder = crate::impls::OneshotResponder; diff --git a/openraft/src/engine/tests/elect_test.rs b/openraft/src/engine/tests/elect_test.rs index 25f8bd2fe..a9fceb30a 100644 --- a/openraft/src/engine/tests/elect_test.rs +++ b/openraft/src/engine/tests/elect_test.rs @@ -14,9 +14,7 @@ use crate::raft::VoteRequest; use crate::testing::log_id; use crate::type_config::TypeConfigExt; use crate::utime::Leased; -use crate::CommittedLeaderId; use crate::EffectiveMembership; -use crate::LogId; use crate::Membership; use crate::Vote; @@ -30,7 +28,7 @@ fn m12() -> Membership { fn eng() -> Engine { let mut eng = Engine::testing_default(0); - eng.state.log_ids = LogIdList::new([LogId::new(CommittedLeaderId::new(0, 0), 0)]); + eng.state.log_ids = LogIdList::new([log_id(0, 0, 0)]); eng.state.enable_validation(false); // Disable validation for incomplete state eng } diff --git a/openraft/src/engine/tests/handle_vote_resp_test.rs b/openraft/src/engine/tests/handle_vote_resp_test.rs index e76efcad7..ce22fee8d 100644 --- a/openraft/src/engine/tests/handle_vote_resp_test.rs +++ b/openraft/src/engine/tests/handle_vote_resp_test.rs @@ -20,10 +20,8 @@ use crate::replication::request::Replicate; use crate::testing::log_id; use crate::type_config::TypeConfigExt; use crate::utime::Leased; -use crate::CommittedLeaderId; use crate::EffectiveMembership; use crate::Entry; -use crate::LogId; use crate::Membership; use crate::Vote; @@ -39,7 +37,7 @@ fn eng() -> Engine { let mut eng = Engine::testing_default(0); eng.state.enable_validation(false); // Disable validation for incomplete state - eng.state.log_ids = LogIdList::new([LogId::new(CommittedLeaderId::new(0, 0), 0)]); + eng.state.log_ids = LogIdList::new([log_id(0, 0, 0)]); eng } diff --git a/openraft/src/engine/tests/initialize_test.rs b/openraft/src/engine/tests/initialize_test.rs index a80d7bab1..3911d51ed 100644 --- a/openraft/src/engine/tests/initialize_test.rs +++ b/openraft/src/engine/tests/initialize_test.rs @@ -17,7 +17,6 @@ use crate::raft_state::LogStateReader; use crate::testing::log_id; use crate::type_config::TypeConfigExt; use crate::utime::Leased; -use crate::vote::CommittedLeaderId; use crate::Entry; use crate::LogId; use crate::Membership; @@ -33,10 +32,7 @@ fn test_initialize_single_node() -> anyhow::Result<()> { eng }; - let log_id0 = LogId { - leader_id: CommittedLeaderId::new(0, 0), - index: 0, - }; + let log_id0 = log_id(0, 0, 0); let m1 = || Membership::::new_with_defaults(vec![btreeset! {1}], []); let entry = Entry::::new_membership(LogId::default(), m1()); @@ -86,10 +82,7 @@ fn test_initialize() -> anyhow::Result<()> { eng }; - let log_id0 = LogId { - leader_id: CommittedLeaderId::new(0, 0), - index: 0, - }; + let log_id0 = log_id(0, 0, 0); let m12 = || Membership::::new_with_defaults(vec![btreeset! {1,2}], []); let entry = || Entry::::new_membership(LogId::default(), m12()); @@ -122,10 +115,7 @@ fn test_initialize() -> anyhow::Result<()> { Command::SendVote { vote_req: VoteRequest { vote: Vote::new(1, 1), - last_log_id: Some(LogId { - leader_id: CommittedLeaderId::new(0, 0), - index: 0, - },), + last_log_id: Some(log_id(0, 0, 0)) }, }, ], diff --git a/openraft/src/engine/tests/trigger_purge_log_test.rs b/openraft/src/engine/tests/trigger_purge_log_test.rs index 070f0a952..db99a98a7 100644 --- a/openraft/src/engine/tests/trigger_purge_log_test.rs +++ b/openraft/src/engine/tests/trigger_purge_log_test.rs @@ -13,9 +13,7 @@ use crate::storage::SnapshotMeta; use crate::testing::log_id; use crate::type_config::TypeConfigExt; use crate::utime::Leased; -use crate::CommittedLeaderId; use crate::EffectiveMembership; -use crate::LogId; use crate::Membership; use crate::MembershipState; use crate::StoredMembership; @@ -33,7 +31,7 @@ fn eng() -> Engine { EffectiveMembership::new_arc(Some(log_id(1, 0, 1)), m12()), ); - eng.state.log_ids = LogIdList::new([LogId::new(CommittedLeaderId::new(0, 0), 0)]); + eng.state.log_ids = LogIdList::new([log_id(0, 0, 0)]); eng } diff --git a/openraft/src/impls/mod.rs b/openraft/src/impls/mod.rs index 1ade60387..b446525e2 100644 --- a/openraft/src/impls/mod.rs +++ b/openraft/src/impls/mod.rs @@ -6,3 +6,16 @@ pub use crate::node::EmptyNode; pub use crate::raft::responder::impls::OneshotResponder; #[cfg(feature = "tokio-rt")] pub use crate::type_config::async_runtime::tokio_impls::TokioRuntime; + +#[cfg(not(feature = "single-term-leader"))] +pub mod leader_id { + pub use crate::vote::leader_id::leader_id_adv::CommittedLeaderId; + pub use crate::vote::leader_id::leader_id_adv::LeaderId; +} +#[cfg(feature = "single-term-leader")] +pub mod leader_id { + pub use crate::vote::leader_id::leader_id_std::CommittedLeaderId; + pub use crate::vote::leader_id::leader_id_std::LeaderId; +} + +pub use leader_id::LeaderId; diff --git a/openraft/src/log_id/mod.rs b/openraft/src/log_id/mod.rs index 5220a0a53..42070a028 100644 --- a/openraft/src/log_id/mod.rs +++ b/openraft/src/log_id/mod.rs @@ -12,7 +12,7 @@ pub use log_id_option_ext::LogIdOptionExt; pub use log_index_option_ext::LogIndexOptionExt; pub use raft_log_id::RaftLogId; -use crate::CommittedLeaderId; +use crate::type_config::alias::CommittedLeaderIdOf; use crate::RaftTypeConfig; /// The identity of a raft log. @@ -25,7 +25,7 @@ pub struct LogId where C: RaftTypeConfig { /// The id of the leader that proposed this log - pub leader_id: CommittedLeaderId, + pub leader_id: CommittedLeaderIdOf, /// The index of a log in the storage. /// /// Log index is a consecutive integer. @@ -35,7 +35,7 @@ where C: RaftTypeConfig impl Copy for LogId where C: RaftTypeConfig, - C::NodeId: Copy, + CommittedLeaderIdOf: Copy, { } @@ -63,12 +63,12 @@ impl LogId where C: RaftTypeConfig { /// Creates a log id proposed by a committed leader with `leader_id` at the given index. - pub fn new(leader_id: CommittedLeaderId, index: u64) -> Self { + pub fn new(leader_id: CommittedLeaderIdOf, index: u64) -> Self { LogId { leader_id, index } } /// Returns the leader id that proposed this log. - pub fn committed_leader_id(&self) -> &CommittedLeaderId { + pub fn committed_leader_id(&self) -> &CommittedLeaderIdOf { &self.leader_id } } diff --git a/openraft/src/log_id/raft_log_id.rs b/openraft/src/log_id/raft_log_id.rs index c2d932a60..06346b33c 100644 --- a/openraft/src/log_id/raft_log_id.rs +++ b/openraft/src/log_id/raft_log_id.rs @@ -1,4 +1,4 @@ -use crate::CommittedLeaderId; +use crate::type_config::alias::CommittedLeaderIdOf; use crate::LogId; use crate::RaftTypeConfig; @@ -12,7 +12,7 @@ where C: RaftTypeConfig /// For example, a leader id in standard raft is `(term, node_id)`, but a log id does not have /// to store the `node_id`, because in standard raft there is at most one leader that can be /// established. - fn leader_id(&self) -> &CommittedLeaderId { + fn leader_id(&self) -> &CommittedLeaderIdOf { self.get_log_id().committed_leader_id() } diff --git a/openraft/src/log_id_range.rs b/openraft/src/log_id_range.rs index 5a1f6ed0a..ef7b01ea4 100644 --- a/openraft/src/log_id_range.rs +++ b/openraft/src/log_id_range.rs @@ -5,6 +5,7 @@ use std::fmt::Formatter; use validit::Validate; use crate::display_ext::DisplayOptionExt; +use crate::type_config::alias::CommittedLeaderIdOf; use crate::LogId; use crate::LogIdOptionExt; use crate::RaftTypeConfig; @@ -29,7 +30,7 @@ where C: RaftTypeConfig impl Copy for LogIdRange where C: RaftTypeConfig, - C::NodeId: Copy, + CommittedLeaderIdOf: Copy, { } @@ -69,14 +70,11 @@ mod tests { use crate::engine::testing::UTConfig; use crate::log_id_range::LogIdRange; - use crate::CommittedLeaderId; + use crate::testing; use crate::LogId; fn log_id(index: u64) -> LogId { - LogId { - leader_id: CommittedLeaderId::new(1, 1), - index, - } + testing::log_id(1, 1, index) } #[test] diff --git a/openraft/src/metrics/wait_test.rs b/openraft/src/metrics/wait_test.rs index f00fb7d04..83d291954 100644 --- a/openraft/src/metrics/wait_test.rs +++ b/openraft/src/metrics/wait_test.rs @@ -13,8 +13,6 @@ use crate::testing::log_id; use crate::type_config::alias::NodeIdOf; use crate::type_config::alias::WatchSenderOf; use crate::type_config::TypeConfigExt; -use crate::vote::CommittedLeaderId; -use crate::LogId; use crate::Membership; use crate::RaftMetrics; use crate::RaftTypeConfig; @@ -48,7 +46,7 @@ async fn test_wait() -> anyhow::Result<()> { sleep(Duration::from_millis(10)).await; let mut update = init.clone(); update.last_log_index = Some(3); - update.last_applied = Some(LogId::new(CommittedLeaderId::new(1, 0), 3)); + update.last_applied = Some(log_id(1, 0, 3)); let rst = tx.send(update); assert!(rst.is_ok()); }); @@ -115,14 +113,14 @@ async fn test_wait() -> anyhow::Result<()> { let h = tokio::spawn(async move { sleep(Duration::from_millis(10)).await; let mut update = init.clone(); - update.snapshot = Some(LogId::new(CommittedLeaderId::new(1, 0), 2)); + update.snapshot = Some(log_id(1, 0, 2)); let rst = tx.send(update); assert!(rst.is_ok()); }); - let got = w.snapshot(LogId::new(CommittedLeaderId::new(1, 0), 2), "snapshot").await?; + let got = w.snapshot(log_id(1, 0, 2), "snapshot").await?; h.await?; - assert_eq!(Some(LogId::new(CommittedLeaderId::new(1, 0), 2)), got.snapshot); + assert_eq!(Some(log_id(1, 0, 2)), got.snapshot); } tracing::info!("--- wait for snapshot, only index matches"); @@ -132,13 +130,13 @@ async fn test_wait() -> anyhow::Result<()> { let h = tokio::spawn(async move { sleep(Duration::from_millis(10)).await; let mut update = init.clone(); - update.snapshot = Some(LogId::new(CommittedLeaderId::new(3, 0), 2)); + update.snapshot = Some(log_id(3, 0, 2)); let rst = tx.send(update); assert!(rst.is_ok()); // delay otherwise the channel will be closed thus the error is shutdown. sleep(Duration::from_millis(200)).await; }); - let got = w.snapshot(LogId::new(CommittedLeaderId::new(1, 0), 2), "snapshot").await; + let got = w.snapshot(log_id(1, 0, 2), "snapshot").await; h.await?; match got.unwrap_err() { WaitError::Timeout(t, _) => { diff --git a/openraft/src/progress/entry/tests.rs b/openraft/src/progress/entry/tests.rs index 9ab6ae960..d7e99384a 100644 --- a/openraft/src/progress/entry/tests.rs +++ b/openraft/src/progress/entry/tests.rs @@ -5,12 +5,13 @@ use crate::engine::EngineConfig; use crate::progress::entry::ProgressEntry; use crate::progress::inflight::Inflight; use crate::raft_state::LogStateReader; -use crate::CommittedLeaderId; +use crate::type_config::alias::LeaderIdOf; +use crate::vote::RaftLeaderIdExt; use crate::LogId; fn log_id(index: u64) -> LogId { LogId { - leader_id: CommittedLeaderId::new(1, 1), + leader_id: LeaderIdOf::::new_committed(1, 1), index, } } diff --git a/openraft/src/progress/inflight/mod.rs b/openraft/src/progress/inflight/mod.rs index 73112e0b0..aca65f7d4 100644 --- a/openraft/src/progress/inflight/mod.rs +++ b/openraft/src/progress/inflight/mod.rs @@ -9,6 +9,7 @@ use validit::Validate; use crate::display_ext::DisplayOptionExt; use crate::log_id_range::LogIdRange; +use crate::type_config::alias::CommittedLeaderIdOf; use crate::LogId; use crate::LogIdOptionExt; use crate::RaftTypeConfig; @@ -41,7 +42,7 @@ where C: RaftTypeConfig impl Copy for Inflight where C: RaftTypeConfig, - C::NodeId: Copy, + CommittedLeaderIdOf: Copy, { } diff --git a/openraft/src/progress/inflight/tests.rs b/openraft/src/progress/inflight/tests.rs index 6bf7bd1fc..04ffe982a 100644 --- a/openraft/src/progress/inflight/tests.rs +++ b/openraft/src/progress/inflight/tests.rs @@ -3,12 +3,13 @@ use validit::Validate; use crate::engine::testing::UTConfig; use crate::log_id_range::LogIdRange; use crate::progress::Inflight; -use crate::CommittedLeaderId; +use crate::type_config::alias::LeaderIdOf; +use crate::vote::RaftLeaderIdExt; use crate::LogId; fn log_id(index: u64) -> LogId { LogId { - leader_id: CommittedLeaderId::new(1, 1), + leader_id: LeaderIdOf::::new_committed(1, 1), index, } } diff --git a/openraft/src/proposer/leader.rs b/openraft/src/proposer/leader.rs index 3d206aa87..78bbc618a 100644 --- a/openraft/src/proposer/leader.rs +++ b/openraft/src/proposer/leader.rs @@ -10,6 +10,7 @@ use crate::type_config::alias::InstantOf; use crate::type_config::alias::LogIdOf; use crate::type_config::TypeConfigExt; use crate::vote::CommittedVote; +use crate::vote::RaftLeaderId; use crate::LogId; use crate::LogIdOptionExt; use crate::RaftLogId; @@ -202,7 +203,7 @@ where // Thus vote.voted_for() is this node. // Safe unwrap: voted_for() is always non-None in Openraft - let node_id = self.committed_vote.clone().into_vote().leader_id().voted_for().unwrap(); + let node_id = self.committed_vote.clone().into_vote().leader_id().node_id_ref().cloned().unwrap(); let now = C::now(); tracing::debug!( diff --git a/openraft/src/raft/declare_raft_types_test.rs b/openraft/src/raft/declare_raft_types_test.rs index fa13ff6b3..99e9e5bf3 100644 --- a/openraft/src/raft/declare_raft_types_test.rs +++ b/openraft/src/raft/declare_raft_types_test.rs @@ -43,16 +43,6 @@ declare_raft_types!( AsyncRuntime = TokioRuntime, ); -// This raise an compile error: -// > error: Type not in its expected position : NodeId = u64, D = (), types must present -// > in this order : D, R, NodeId, Node, Entry, SnapshotData, AsyncRuntime -// declare_raft_types!( -// Foo: -// Node = (), -// NodeId = u64, -// D = (), -// ); - declare_raft_types!(EmptyWithColon:); declare_raft_types!(Empty); diff --git a/openraft/src/raft/mod.rs b/openraft/src/raft/mod.rs index 7ded9d116..2cb748a3d 100644 --- a/openraft/src/raft/mod.rs +++ b/openraft/src/raft/mod.rs @@ -115,6 +115,7 @@ use crate::Vote; /// NodeId = u64, /// Node = openraft::BasicNode, /// Term = u64, +/// LeaderId = openraft::impls::LeaderId, /// Entry = openraft::Entry, /// SnapshotData = Cursor>, /// Responder = openraft::impls::OneshotResponder, @@ -174,6 +175,7 @@ macro_rules! declare_raft_types { (NodeId , , u64 ), (Node , , $crate::impls::BasicNode ), (Term , , u64 ), + (LeaderId , , $crate::impls::LeaderId ), (Entry , , $crate::impls::Entry ), (SnapshotData , , std::io::Cursor> ), (Responder , , $crate::impls::OneshotResponder ), diff --git a/openraft/src/raft_state/mod.rs b/openraft/src/raft_state/mod.rs index c7a3d47a0..b234f04ab 100644 --- a/openraft/src/raft_state/mod.rs +++ b/openraft/src/raft_state/mod.rs @@ -41,6 +41,7 @@ use crate::proposer::Leader; use crate::proposer::LeaderQuorumSet; use crate::type_config::alias::InstantOf; use crate::type_config::alias::LogIdOf; +use crate::vote::RaftLeaderId; /// A struct used to represent the raft state which a Raft node needs. #[derive(Clone, Debug)] @@ -367,7 +368,7 @@ where C: RaftTypeConfig /// /// [Determine Server State]: crate::docs::data::vote#vote-and-membership-define-the-server-state pub(crate) fn is_leading(&self, id: &C::NodeId) -> bool { - self.membership_state.contains(id) && self.vote.leader_id().voted_for().as_ref() == Some(id) + self.membership_state.contains(id) && self.vote.leader_id().node_id_ref() == Some(id) } /// The node is leader @@ -407,7 +408,7 @@ where C: RaftTypeConfig if vote.is_committed() { // Safe unwrap(): vote that is committed has to already have voted for some node. - let id = vote.leader_id().voted_for().unwrap(); + let id = vote.leader_id().node_id_ref().cloned().unwrap(); return self.new_forward_to_leader(id); } diff --git a/openraft/src/raft_state/tests/log_state_reader_test.rs b/openraft/src/raft_state/tests/log_state_reader_test.rs index 27ed5c9fd..6fc9675fc 100644 --- a/openraft/src/raft_state/tests/log_state_reader_test.rs +++ b/openraft/src/raft_state/tests/log_state_reader_test.rs @@ -1,15 +1,12 @@ use crate::engine::testing::UTConfig; use crate::engine::LogIdList; use crate::raft_state::LogStateReader; -use crate::CommittedLeaderId; +use crate::testing; use crate::LogId; use crate::RaftState; fn log_id(term: u64, index: u64) -> LogId { - LogId { - leader_id: CommittedLeaderId::new(term, 0), - index, - } + testing::log_id(term, 0, index) } #[test] diff --git a/openraft/src/raft_state/tests/validate_test.rs b/openraft/src/raft_state/tests/validate_test.rs index 7587da25a..c1b83a7c6 100644 --- a/openraft/src/raft_state/tests/validate_test.rs +++ b/openraft/src/raft_state/tests/validate_test.rs @@ -3,15 +3,12 @@ use validit::Validate; use crate::engine::testing::UTConfig; use crate::engine::LogIdList; use crate::storage::SnapshotMeta; -use crate::CommittedLeaderId; +use crate::testing; use crate::LogId; use crate::RaftState; fn log_id(term: u64, index: u64) -> LogId { - LogId { - leader_id: CommittedLeaderId::new(term, 0), - index, - } + testing::log_id(term, 0, index) } #[test] diff --git a/openraft/src/replication/mod.rs b/openraft/src/replication/mod.rs index bded915bb..773a2b545 100644 --- a/openraft/src/replication/mod.rs +++ b/openraft/src/replication/mod.rs @@ -56,6 +56,7 @@ use crate::type_config::alias::OneshotReceiverOf; use crate::type_config::alias::OneshotSenderOf; use crate::type_config::async_runtime::mutex::Mutex; use crate::type_config::TypeConfigExt; +use crate::vote::RaftLeaderId; use crate::LogId; use crate::RaftLogId; use crate::RaftNetworkFactory; @@ -442,7 +443,7 @@ where let append_res = res.map_err(|_e| { let to = Timeout { action: RPCTypes::AppendEntries, - id: self.session_id.vote().leader_id().voted_for().unwrap(), + id: self.session_id.vote().leader_id().node_id_ref().cloned().unwrap(), target: self.target.clone(), timeout: the_timeout, }; diff --git a/openraft/src/testing/common.rs b/openraft/src/testing/common.rs index 96a0295de..4ea87291e 100644 --- a/openraft/src/testing/common.rs +++ b/openraft/src/testing/common.rs @@ -3,7 +3,7 @@ use std::collections::BTreeSet; use crate::entry::RaftEntry; -use crate::CommittedLeaderId; +use crate::vote::RaftLeaderIdExt; use crate::LogId; use crate::RaftTypeConfig; @@ -14,7 +14,7 @@ where C::Term: From, { LogId:: { - leader_id: CommittedLeaderId::new(term.into(), node_id), + leader_id: C::LeaderId::new_committed(term.into(), node_id), index, } } diff --git a/openraft/src/testing/log/suite.rs b/openraft/src/testing/log/suite.rs index ab40d9579..cc067a4fb 100644 --- a/openraft/src/testing/log/suite.rs +++ b/openraft/src/testing/log/suite.rs @@ -24,7 +24,7 @@ use crate::storage::RaftStateMachine; use crate::storage::StorageHelper; use crate::testing::log::StoreBuilder; use crate::type_config::TypeConfigExt; -use crate::vote::CommittedLeaderId; +use crate::vote::RaftLeaderIdExt; use crate::LogId; use crate::Membership; use crate::OptionalSend; @@ -634,7 +634,7 @@ where pub async fn get_initial_state_log_ids(mut store: LS, mut sm: SM) -> Result<(), StorageError> { let log_id = |t: u64, n: u64, i| LogId:: { - leader_id: CommittedLeaderId::::new(t.into(), n.into()), + leader_id: C::LeaderId::new_committed(t.into(), n.into()), index: i, }; @@ -1379,7 +1379,7 @@ where C::NodeId: From, { LogId { - leader_id: CommittedLeaderId::new(term.into(), NODE_ID.into()), + leader_id: C::LeaderId::new_committed(term.into(), NODE_ID.into()), index, } } @@ -1463,7 +1463,7 @@ where C::NodeId: From, { LogId { - leader_id: CommittedLeaderId::new(term.into(), node_id.into()), + leader_id: C::LeaderId::new_committed(term.into(), node_id.into()), index, } } diff --git a/openraft/src/type_config.rs b/openraft/src/type_config.rs index f51b12f0f..c6152ec73 100644 --- a/openraft/src/type_config.rs +++ b/openraft/src/type_config.rs @@ -17,6 +17,7 @@ use crate::entry::FromAppData; use crate::entry::RaftEntry; use crate::raft::responder::Responder; use crate::vote::raft_term::RaftTerm; +use crate::vote::RaftLeaderId; use crate::AppData; use crate::AppDataResponse; use crate::Node; @@ -45,6 +46,7 @@ use crate::OptionalSync; /// NodeId = u64, /// Node = openraft::BasicNode, /// Term = u64, +/// LeaderId = openraft::impls::LeaderId, /// Entry = openraft::Entry, /// SnapshotData = Cursor>, /// AsyncRuntime = openraft::TokioRuntime, @@ -66,9 +68,6 @@ pub trait RaftTypeConfig: /// Raft application level node data type Node: Node; - /// Raft log entry, which can be built from an AppData. - type Entry: RaftEntry + FromAppData; - /// Type representing a Raft term number. /// /// A term is a logical clock in Raft that is used to detect obsolete information, @@ -79,6 +78,12 @@ pub trait RaftTypeConfig: /// See: [`RaftTerm`] for the required methods. type Term: RaftTerm; + /// A Leader identifier in a cluster. + type LeaderId: RaftLeaderId; + + /// Raft log entry, which can be built from an AppData. + type Entry: RaftEntry + FromAppData; + /// Snapshot data for exposing a snapshot for reading & writing. /// /// See the [storage chapter of the guide][sto] for details on log compaction / snapshotting. @@ -112,6 +117,7 @@ pub mod alias { use crate::async_runtime::Oneshot; use crate::raft::responder::Responder; use crate::type_config::AsyncRuntime; + use crate::vote::RaftLeaderId; use crate::RaftTypeConfig; pub type DOf = ::D; @@ -119,6 +125,7 @@ pub mod alias { pub type NodeIdOf = ::NodeId; pub type NodeOf = ::Node; pub type TermOf = ::Term; + pub type LeaderIdOf = ::LeaderId; pub type EntryOf = ::Entry; pub type SnapshotDataOf = ::SnapshotData; pub type AsyncRuntimeOf = ::AsyncRuntime; @@ -166,7 +173,6 @@ pub mod alias { // Usually used types pub type LogIdOf = crate::LogId; pub type VoteOf = crate::Vote; - pub type LeaderIdOf = crate::LeaderId; - pub type CommittedLeaderIdOf = crate::CommittedLeaderId; + pub type CommittedLeaderIdOf = as RaftLeaderId>::Committed; pub type SerdeInstantOf = crate::metrics::SerdeInstant>; } diff --git a/openraft/src/vote/committed.rs b/openraft/src/vote/committed.rs index 5fd5f7bc6..8d114161c 100644 --- a/openraft/src/vote/committed.rs +++ b/openraft/src/vote/committed.rs @@ -1,8 +1,9 @@ use std::cmp::Ordering; use std::fmt; +use crate::type_config::alias::CommittedLeaderIdOf; use crate::vote::ref_vote::RefVote; -use crate::CommittedLeaderId; +use crate::vote::RaftLeaderId; use crate::RaftTypeConfig; use crate::Vote; @@ -41,7 +42,7 @@ where C: RaftTypeConfig Self { vote } } - pub(crate) fn committed_leader_id(&self) -> CommittedLeaderId { + pub(crate) fn committed_leader_id(&self) -> CommittedLeaderIdOf { self.vote.leader_id().to_committed() } diff --git a/openraft/src/vote/leader_id/impl_into_leader_id.rs b/openraft/src/vote/leader_id/impl_into_leader_id.rs deleted file mode 100644 index d4389c513..000000000 --- a/openraft/src/vote/leader_id/impl_into_leader_id.rs +++ /dev/null @@ -1,11 +0,0 @@ -use crate::vote::leader_id::CommittedLeaderId; -use crate::vote::Vote; -use crate::RaftTypeConfig; - -impl From> for CommittedLeaderId -where C: RaftTypeConfig -{ - fn from(vote: Vote) -> Self { - vote.leader_id.to_committed() - } -} diff --git a/openraft/src/vote/leader_id/leader_id_adv.rs b/openraft/src/vote/leader_id/leader_id_adv.rs index 6b95365de..5b2426b42 100644 --- a/openraft/src/vote/leader_id/leader_id_adv.rs +++ b/openraft/src/vote/leader_id/leader_id_adv.rs @@ -1,5 +1,7 @@ use std::fmt; +use crate::vote::RaftCommittedLeaderId; +use crate::vote::RaftLeaderId; use crate::RaftTypeConfig; /// LeaderId is identifier of a `leader`. @@ -21,34 +23,6 @@ where C: RaftTypeConfig pub node_id: C::NodeId, } -impl LeaderId -where C: RaftTypeConfig -{ - pub fn new(term: C::Term, node_id: C::NodeId) -> Self { - Self { term, node_id } - } - - pub fn get_term(&self) -> C::Term { - self.term - } - - pub fn voted_for(&self) -> Option { - Some(self.node_id.clone()) - } - - #[allow(clippy::wrong_self_convention)] - pub(crate) fn to_committed(&self) -> CommittedLeaderId { - self.clone() - } - - /// Return if it is the same leader as the committed leader id. - /// - /// A committed leader may have less info than a non-committed. - pub(crate) fn is_same_as_committed(&self, other: &CommittedLeaderId) -> bool { - self == other - } -} - impl fmt::Display for LeaderId where C: RaftTypeConfig { @@ -71,22 +45,49 @@ where C: RaftTypeConfig /// standard raft stores just a `term` in log entry. pub type CommittedLeaderId = LeaderId; +impl RaftLeaderId for LeaderId +where C: RaftTypeConfig +{ + type Committed = Self; + + fn new(term: C::Term, node_id: C::NodeId) -> Self { + Self { term, node_id } + } + + fn term(&self) -> C::Term { + self.term + } + + fn node_id_ref(&self) -> Option<&C::NodeId> { + Some(&self.node_id) + } + + fn to_committed(&self) -> Self::Committed { + self.clone() + } +} + +impl RaftCommittedLeaderId for LeaderId where C: RaftTypeConfig {} + #[cfg(test)] mod tests { use crate::engine::testing::UTConfig; + use crate::vote::RaftLeaderId; use crate::LeaderId; #[cfg(feature = "serde")] #[test] fn test_committed_leader_id_serde() -> anyhow::Result<()> { - use crate::CommittedLeaderId; + use crate::type_config::alias::CommittedLeaderIdOf; + use crate::type_config::alias::LeaderIdOf; + use crate::vote::RaftLeaderIdExt; - let c = CommittedLeaderId::::new(5, 10); + let c = LeaderIdOf::::new_committed(5, 10); let s = serde_json::to_string(&c)?; assert_eq!(r#"{"term":5,"node_id":10}"#, s); - let c2: CommittedLeaderId = serde_json::from_str(&s)?; - assert_eq!(CommittedLeaderId::new(5, 10), c2); + let c2: CommittedLeaderIdOf = serde_json::from_str(&s)?; + assert_eq!(LeaderIdOf::::new_committed(5, 10), c2); Ok(()) } diff --git a/openraft/src/vote/leader_id/leader_id_std.rs b/openraft/src/vote/leader_id/leader_id_std.rs index 95d7e7ab2..e3cbfaf4f 100644 --- a/openraft/src/vote/leader_id/leader_id_std.rs +++ b/openraft/src/vote/leader_id/leader_id_std.rs @@ -3,6 +3,8 @@ use std::fmt; use std::marker::PhantomData; use crate::display_ext::DisplayOptionExt; +use crate::vote::RaftCommittedLeaderId; +use crate::vote::RaftLeaderId; use crate::RaftTypeConfig; #[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] @@ -41,43 +43,37 @@ where C: RaftTypeConfig } } -impl LeaderId +impl fmt::Display for LeaderId where C: RaftTypeConfig { - pub fn new(term: C::Term, node_id: C::NodeId) -> Self { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "T{}-N{}", self.term, self.voted_for.display()) + } +} + +impl RaftLeaderId for LeaderId +where C: RaftTypeConfig +{ + type Committed = CommittedLeaderId; + + fn new(term: C::Term, node_id: C::NodeId) -> Self { Self { term, voted_for: Some(node_id), } } - pub fn get_term(&self) -> C::Term { + fn term(&self) -> C::Term { self.term } - pub fn voted_for(&self) -> Option { - self.voted_for.clone() + fn node_id_ref(&self) -> Option<&C::NodeId> { + self.voted_for.as_ref() } - #[allow(clippy::wrong_self_convention)] - pub(crate) fn to_committed(&self) -> CommittedLeaderId { + fn to_committed(&self) -> Self::Committed { CommittedLeaderId::new(self.term, C::NodeId::default()) } - - /// Return if it is the same leader as the committed leader id. - /// - /// A committed leader may have less info than a non-committed. - pub(crate) fn is_same_as_committed(&self, other: &CommittedLeaderId) -> bool { - self.term == other.term - } -} - -impl fmt::Display for LeaderId -where C: RaftTypeConfig -{ - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "T{}-N{}", self.term, self.voted_for.display()) - } } #[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] @@ -108,6 +104,8 @@ where C: RaftTypeConfig } } +impl RaftCommittedLeaderId for CommittedLeaderId where C: RaftTypeConfig {} + #[cfg(test)] #[allow(clippy::nonminimal_bool)] mod tests { diff --git a/openraft/src/vote/leader_id/mod.rs b/openraft/src/vote/leader_id/mod.rs index 38bc0b056..10b07d698 100644 --- a/openraft/src/vote/leader_id/mod.rs +++ b/openraft/src/vote/leader_id/mod.rs @@ -1,7 +1,7 @@ #[cfg(not(feature = "single-term-leader"))] -mod leader_id_adv; +pub(crate) mod leader_id_adv; #[cfg(feature = "single-term-leader")] -mod leader_id_std; +pub(crate) mod leader_id_std; #[cfg(not(feature = "single-term-leader"))] pub use leader_id_adv::CommittedLeaderId; @@ -12,4 +12,5 @@ pub use leader_id_std::CommittedLeaderId; #[cfg(feature = "single-term-leader")] pub use leader_id_std::LeaderId; -mod impl_into_leader_id; +pub(crate) mod raft_committed_leader_id; +pub(crate) mod raft_leader_id; diff --git a/openraft/src/vote/leader_id/raft_committed_leader_id.rs b/openraft/src/vote/leader_id/raft_committed_leader_id.rs new file mode 100644 index 000000000..d597f861a --- /dev/null +++ b/openraft/src/vote/leader_id/raft_committed_leader_id.rs @@ -0,0 +1,45 @@ +use std::fmt::Debug; +use std::fmt::Display; + +use crate::base::OptionalFeatures; +use crate::RaftTypeConfig; + +/// A Leader identifier that has been granted and committed by a quorum of the cluster. +/// +/// This type is used as part of the Log ID to identify the leader that proposed a log entry. +/// Because log can only be proposed by a Leader committed by a quorum. +/// For example, in standard Raft, committed LeaderId is just the term number, because in each term +/// there is only one established leader. +/// +/// # Implementation +/// +/// A simple non-optimized implementation of this trait is to use the same type as [`RaftLeaderId`]. +/// +/// # Total Ordering +/// +/// Unlike [`RaftLeaderId`], this type implements `Ord` because committed leader IDs +/// have a total ordering as they must be agreed upon by a quorum, and two incomparable +/// [`RaftLeaderId`] can not both be committed by two quorums, because only a **greater** +/// [`RaftLeaderId`] can override an existing value. +/// +/// A [`RaftCommittedLeaderId`] may contain less information than the corresponding +/// [`RaftLeaderId`], because it implies the constraint that **a quorum has granted it**. +/// +/// For a total order [`RaftLeaderId`], the [`RaftCommittedLeaderId`] is the same. +/// +/// For a partial order [`RaftLeaderId`], we know that all the granted leader-id must be a total +/// order set. Therefor once it is granted by a quorum, it only keeps the information that makes +/// leader-ids a correct total order set +/// +/// For example, in standard Raft: +/// - [`RaftLeaderId`] is `(term, voted_for)` - partially ordered +/// - [`RaftCommittedLeaderId`] is just `term` - totally ordered (The `voted_for` field can be +/// dropped since it's no longer needed for ordering) +/// +/// [`RaftLeaderId`]: crate::vote::RaftLeaderId +pub trait RaftCommittedLeaderId +where + C: RaftTypeConfig, + Self: OptionalFeatures + Ord + Clone + Debug + Display + Default + 'static, +{ +} diff --git a/openraft/src/vote/leader_id/raft_leader_id.rs b/openraft/src/vote/leader_id/raft_leader_id.rs new file mode 100644 index 000000000..6bc8c6f93 --- /dev/null +++ b/openraft/src/vote/leader_id/raft_leader_id.rs @@ -0,0 +1,68 @@ +use std::fmt::Debug; +use std::fmt::Display; + +use crate::base::OptionalFeatures; +use crate::vote::leader_id::raft_committed_leader_id::RaftCommittedLeaderId; +use crate::RaftTypeConfig; + +/// A Leader identifier in a OpenRaft cluster. +/// +/// In OpenRaft, a `LeaderId` represents either: +/// - A granted leader that received votes from a quorum (a `Leader` in standard Raft) +/// - A non-granted leader, i.e., candidate (a `Candidate` in standard Raft) +/// +/// The identity of the leader remains the same in both cases. Whether it is granted by a quorum +/// is determined by the `committed` field in [`Vote`]. +/// +/// # Partial Ordering +/// +/// [`RaftLeaderId`] implements `PartialOrd` but not `Ord`. Because to be compatible with standard +/// Raft, in which a `LeaderId` or `CandidateId` is a tuple of `(term, node_id)`: Two such IDs +/// with the same term but different node IDs (e.g., `(1,2)` and `(1,3)`) have no defined ordering - +/// neither can overwrite the other. +/// +/// [`Vote`]: crate::vote::Vote +pub trait RaftLeaderId +where + C: RaftTypeConfig, + Self: OptionalFeatures + PartialOrd + Eq + Clone + Debug + Display + Default + 'static, +{ + /// The committed version of this leader ID. + /// + /// A simple implementation of this trait would return `Self` as the committed version. + type Committed: RaftCommittedLeaderId; + + fn new(term: C::Term, node_id: C::NodeId) -> Self; + + /// Get the term number of this leader + fn term(&self) -> C::Term; + + /// Get the node ID of this leader, if one is set + fn node_id_ref(&self) -> Option<&C::NodeId>; + + /// Convert this leader ID to a committed leader ID. + /// + /// This is used when it has been granted by a quorum. + fn to_committed(&self) -> Self::Committed; +} + +pub trait RaftLeaderIdExt +where + C: RaftTypeConfig, + Self: RaftLeaderId, +{ + fn new_committed(term: C::Term, node_id: C::NodeId) -> Self::Committed { + Self::new(term, node_id).to_committed() + } + + fn node_id(&self) -> Option { + self.node_id_ref().cloned() + } +} + +impl RaftLeaderIdExt for T +where + C: RaftTypeConfig, + T: RaftLeaderId, +{ +} diff --git a/openraft/src/vote/mod.rs b/openraft/src/vote/mod.rs index 93ce1501c..c9210792e 100644 --- a/openraft/src/vote/mod.rs +++ b/openraft/src/vote/mod.rs @@ -1,5 +1,5 @@ pub(crate) mod committed; -mod leader_id; +pub(crate) mod leader_id; pub(crate) mod non_committed; pub(crate) mod ref_vote; #[allow(clippy::module_inception)] @@ -7,6 +7,9 @@ mod vote; pub(crate) mod vote_status; pub(crate) use committed::CommittedVote; +pub use leader_id::raft_committed_leader_id::RaftCommittedLeaderId; +pub use leader_id::raft_leader_id::RaftLeaderId; +pub use leader_id::raft_leader_id::RaftLeaderIdExt; pub use leader_id::CommittedLeaderId; pub use leader_id::LeaderId; pub(crate) use non_committed::NonCommittedVote; diff --git a/openraft/src/vote/ref_vote.rs b/openraft/src/vote/ref_vote.rs index f81743e4a..7d7c2a9af 100644 --- a/openraft/src/vote/ref_vote.rs +++ b/openraft/src/vote/ref_vote.rs @@ -1,24 +1,23 @@ use std::cmp::Ordering; use std::fmt::Formatter; -use crate::LeaderId; use crate::RaftTypeConfig; -/// Same as [`Vote`] but with a reference to the [`LeaderId`]. +/// Same as [`Vote`] but with a reference to the `LeaderId`. /// /// [`Vote`]: crate::vote::Vote #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) struct RefVote<'a, C> where C: RaftTypeConfig { - pub(crate) leader_id: &'a LeaderId, + pub(crate) leader_id: &'a C::LeaderId, pub(crate) committed: bool, } impl<'a, C> RefVote<'a, C> where C: RaftTypeConfig { - pub(crate) fn new(leader_id: &'a LeaderId, committed: bool) -> Self { + pub(crate) fn new(leader_id: &'a C::LeaderId, committed: bool) -> Self { Self { leader_id, committed } } diff --git a/openraft/src/vote/vote.rs b/openraft/src/vote/vote.rs index 6f975953b..1f5e08e5b 100644 --- a/openraft/src/vote/vote.rs +++ b/openraft/src/vote/vote.rs @@ -1,12 +1,12 @@ use std::cmp::Ordering; use std::fmt::Formatter; +use crate::type_config::alias::CommittedLeaderIdOf; use crate::vote::committed::CommittedVote; -use crate::vote::leader_id::CommittedLeaderId; use crate::vote::ref_vote::RefVote; use crate::vote::vote_status::VoteStatus; use crate::vote::NonCommittedVote; -use crate::LeaderId; +use crate::vote::RaftLeaderId; use crate::RaftTypeConfig; /// `Vote` represent the privilege of a node. @@ -14,7 +14,7 @@ use crate::RaftTypeConfig; #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize), serde(bound = ""))] pub struct Vote { /// The id of the node that tries to become the leader. - pub leader_id: LeaderId, + pub leader_id: C::LeaderId, pub committed: bool, } @@ -22,7 +22,7 @@ pub struct Vote { impl Copy for Vote where C: RaftTypeConfig, - C::NodeId: Copy, + C::LeaderId: Copy, { } @@ -53,14 +53,14 @@ where C: RaftTypeConfig { pub fn new(term: C::Term, node_id: C::NodeId) -> Self { Self { - leader_id: LeaderId::new(term, node_id), + leader_id: C::LeaderId::new(term, node_id), committed: false, } } pub fn new_committed(term: C::Term, node_id: C::NodeId) -> Self { Self { - leader_id: LeaderId::new(term, node_id), + leader_id: C::LeaderId::new(term, node_id), committed: true, } } @@ -95,15 +95,16 @@ where C: RaftTypeConfig self.committed } - /// Return the [`LeaderId`] this vote represents for. + /// Return the `LeaderId` this vote represents for. /// /// The leader may or may not be granted by a quorum. - pub fn leader_id(&self) -> &LeaderId { + pub fn leader_id(&self) -> &C::LeaderId { &self.leader_id } - pub(crate) fn is_same_leader(&self, leader_id: &CommittedLeaderId) -> bool { - self.leader_id().is_same_as_committed(leader_id) + // TODO: remove this method + pub(crate) fn is_same_leader(&self, leader_id: &CommittedLeaderIdOf) -> bool { + self.leader_id().to_committed() == *leader_id } } diff --git a/tests/tests/append_entries/t10_conflict_with_empty_entries.rs b/tests/tests/append_entries/t10_conflict_with_empty_entries.rs index 4fae50ec1..ebe37add3 100644 --- a/tests/tests/append_entries/t10_conflict_with_empty_entries.rs +++ b/tests/tests/append_entries/t10_conflict_with_empty_entries.rs @@ -7,11 +7,10 @@ use openraft::network::RPCOption; use openraft::network::RaftNetworkFactory; use openraft::raft::AppendEntriesRequest; use openraft::testing::blank_ent; -use openraft::CommittedLeaderId; +use openraft::testing::log_id; use openraft::Config; use openraft::Entry; use openraft::EntryPayload; -use openraft::LogId; use openraft::Vote; use openraft_memstore::ClientRequest; @@ -55,9 +54,9 @@ async fn conflict_with_empty_entries() -> Result<()> { let rpc = AppendEntriesRequest:: { vote: Vote::new_committed(1, 1), - prev_log_id: Some(LogId::new(CommittedLeaderId::new(1, 0), 5)), + prev_log_id: Some(log_id(1, 0, 5)), entries: vec![], - leader_commit: Some(LogId::new(CommittedLeaderId::new(1, 0), 5)), + leader_commit: Some(log_id(1, 0, 5)), }; let option = RPCOption::new(Duration::from_millis(1_000)); @@ -71,14 +70,14 @@ async fn conflict_with_empty_entries() -> Result<()> { vote: Vote::new_committed(1, 1), prev_log_id: None, entries: vec![blank_ent(0, 0, 0), blank_ent(1, 0, 1), Entry { - log_id: LogId::new(CommittedLeaderId::new(1, 0), 2), + log_id: log_id(1, 0, 2), payload: EntryPayload::Normal(ClientRequest { client: "foo".to_string(), serial: 1, status: "bar".to_string(), }), }], - leader_commit: Some(LogId::new(CommittedLeaderId::new(1, 0), 5)), + leader_commit: Some(log_id(1, 0, 5)), }; let option = RPCOption::new(Duration::from_millis(1_000)); @@ -91,9 +90,9 @@ async fn conflict_with_empty_entries() -> Result<()> { let rpc = AppendEntriesRequest:: { vote: Vote::new_committed(1, 1), - prev_log_id: Some(LogId::new(CommittedLeaderId::new(1, 0), 3)), + prev_log_id: Some(log_id(1, 0, 3)), entries: vec![], - leader_commit: Some(LogId::new(CommittedLeaderId::new(1, 0), 5)), + leader_commit: Some(log_id(1, 0, 5)), }; let option = RPCOption::new(Duration::from_millis(1_000)); diff --git a/tests/tests/append_entries/t10_see_higher_vote.rs b/tests/tests/append_entries/t10_see_higher_vote.rs index e28670814..ca2235ad1 100644 --- a/tests/tests/append_entries/t10_see_higher_vote.rs +++ b/tests/tests/append_entries/t10_see_higher_vote.rs @@ -7,9 +7,8 @@ use openraft::network::v2::RaftNetworkV2; use openraft::network::RPCOption; use openraft::network::RaftNetworkFactory; use openraft::raft::VoteRequest; -use openraft::CommittedLeaderId; +use openraft::testing::log_id; use openraft::Config; -use openraft::LogId; use openraft::ServerState; use openraft::Vote; use openraft_memstore::ClientRequest; @@ -50,7 +49,7 @@ async fn append_sees_higher_vote() -> Result<()> { .vote( VoteRequest { vote: Vote::new(10, 1), - last_log_id: Some(LogId::new(CommittedLeaderId::new(10, 1), 5)), + last_log_id: Some(log_id(10, 1, 5)), }, option, ) diff --git a/tests/tests/append_entries/t11_append_conflicts.rs b/tests/tests/append_entries/t11_append_conflicts.rs index 5436f11a1..a03b49c4c 100644 --- a/tests/tests/append_entries/t11_append_conflicts.rs +++ b/tests/tests/append_entries/t11_append_conflicts.rs @@ -6,10 +6,9 @@ use maplit::btreeset; use openraft::raft::AppendEntriesRequest; use openraft::storage::RaftLogStorage; use openraft::testing::blank_ent; -use openraft::CommittedLeaderId; +use openraft::testing::log_id; use openraft::Config; use openraft::Entry; -use openraft::LogId; use openraft::RaftLogReader; use openraft::RaftTypeConfig; use openraft::ServerState; @@ -49,7 +48,7 @@ async fn append_conflicts() -> Result<()> { vote: Vote::new_committed(1, 2), prev_log_id: None, entries: vec![], - leader_commit: Some(LogId::new(CommittedLeaderId::new(1, 0), 2)), + leader_commit: Some(log_id(1, 0, 2)), }; let resp = r0.append_entries(req).await?; @@ -63,7 +62,7 @@ async fn append_conflicts() -> Result<()> { vote: Vote::new_committed(1, 2), prev_log_id: None, entries: vec![blank_ent(0, 0, 0)], - leader_commit: Some(LogId::new(CommittedLeaderId::new(1, 0), 2)), + leader_commit: Some(log_id(1, 0, 2)), }; let resp = r0.append_entries(req).await?; @@ -74,9 +73,9 @@ async fn append_conflicts() -> Result<()> { let req = AppendEntriesRequest { vote: Vote::new_committed(1, 2), - prev_log_id: Some(LogId::new(CommittedLeaderId::new(0, 0), 0)), + prev_log_id: Some(log_id(0, 0, 0)), entries: vec![], - leader_commit: Some(LogId::new(CommittedLeaderId::new(1, 0), 2)), + leader_commit: Some(log_id(1, 0, 2)), }; let resp = r0.append_entries(req).await?; @@ -89,7 +88,7 @@ async fn append_conflicts() -> Result<()> { let req = || AppendEntriesRequest { vote: Vote::new_committed(1, 2), - prev_log_id: Some(LogId::new(CommittedLeaderId::new(0, 0), 0)), + prev_log_id: Some(log_id(0, 0, 0)), entries: vec![ blank_ent(1, 0, 1), blank_ent(1, 0, 2), @@ -97,7 +96,7 @@ async fn append_conflicts() -> Result<()> { blank_ent(1, 0, 4), ], // this set the last_applied to 2 - leader_commit: Some(LogId::new(CommittedLeaderId::new(1, 0), 2)), + leader_commit: Some(log_id(1, 0, 2)), }; let resp = r0.append_entries(req()).await?; @@ -119,9 +118,9 @@ async fn append_conflicts() -> Result<()> { let req = AppendEntriesRequest { vote: Vote::new_committed(1, 2), - prev_log_id: Some(LogId::new(CommittedLeaderId::new(1, 0), 1)), + prev_log_id: Some(log_id(1, 0, 1)), entries: vec![blank_ent(1, 0, 2)], - leader_commit: Some(LogId::new(CommittedLeaderId::new(1, 0), 2)), + leader_commit: Some(log_id(1, 0, 2)), }; let resp = r0.append_entries(req).await?; @@ -134,10 +133,10 @@ async fn append_conflicts() -> Result<()> { let req = AppendEntriesRequest { vote: Vote::new_committed(1, 2), - prev_log_id: Some(LogId::new(CommittedLeaderId::new(1, 0), 2)), + prev_log_id: Some(log_id(1, 0, 2)), entries: vec![blank_ent(2, 0, 3)], // this set the last_applied to 2 - leader_commit: Some(LogId::new(CommittedLeaderId::new(1, 0), 2)), + leader_commit: Some(log_id(1, 0, 2)), }; let resp = r0.append_entries(req).await?; @@ -149,9 +148,9 @@ async fn append_conflicts() -> Result<()> { // check last_log_id is updated: let req = AppendEntriesRequest { vote: Vote::new_committed(1, 2), - prev_log_id: Some(LogId::new(CommittedLeaderId::new(1, 0), 2000)), + prev_log_id: Some(log_id(1, 0, 2000)), entries: vec![], - leader_commit: Some(LogId::new(CommittedLeaderId::new(1, 0), 2)), + leader_commit: Some(log_id(1, 0, 2)), }; let resp = r0.append_entries(req).await?; @@ -164,9 +163,9 @@ async fn append_conflicts() -> Result<()> { let req = AppendEntriesRequest { vote: Vote::new_committed(1, 2), - prev_log_id: Some(LogId::new(CommittedLeaderId::new(3, 0), 3)), + prev_log_id: Some(log_id(3, 0, 3)), entries: vec![], - leader_commit: Some(LogId::new(CommittedLeaderId::new(1, 0), 2)), + leader_commit: Some(log_id(1, 0, 2)), }; let resp = r0.append_entries(req).await?; @@ -179,9 +178,9 @@ async fn append_conflicts() -> Result<()> { // refill logs let req = AppendEntriesRequest { vote: Vote::new_committed(1, 2), - prev_log_id: Some(LogId::new(CommittedLeaderId::new(1, 0), 2)), + prev_log_id: Some(log_id(1, 0, 2)), entries: vec![blank_ent(2, 0, 3), blank_ent(2, 0, 4), blank_ent(2, 0, 5)], - leader_commit: Some(LogId::new(CommittedLeaderId::new(1, 0), 2)), + leader_commit: Some(log_id(1, 0, 2)), }; let resp = r0.append_entries(req).await?; @@ -194,9 +193,9 @@ async fn append_conflicts() -> Result<()> { // prev_log_id matches let req = AppendEntriesRequest { vote: Vote::new_committed(1, 2), - prev_log_id: Some(LogId::new(CommittedLeaderId::new(2, 0), 3)), + prev_log_id: Some(log_id(2, 0, 3)), entries: vec![blank_ent(3, 0, 4)], - leader_commit: Some(LogId::new(CommittedLeaderId::new(1, 0), 2)), + leader_commit: Some(log_id(1, 0, 2)), }; let resp = r0.append_entries(req).await?; @@ -210,9 +209,9 @@ async fn append_conflicts() -> Result<()> { // refill logs let req = AppendEntriesRequest { vote: Vote::new_committed(1, 2), - prev_log_id: Some(LogId::new(CommittedLeaderId::new(1, 0), 200)), + prev_log_id: Some(log_id(1, 0, 200)), entries: vec![], - leader_commit: Some(LogId::new(CommittedLeaderId::new(1, 0), 2)), + leader_commit: Some(log_id(1, 0, 2)), }; let resp = r0.append_entries(req).await?; diff --git a/tests/tests/append_entries/t11_append_entries_with_bigger_term.rs b/tests/tests/append_entries/t11_append_entries_with_bigger_term.rs index a7e8fbb56..92fc421b7 100644 --- a/tests/tests/append_entries/t11_append_entries_with_bigger_term.rs +++ b/tests/tests/append_entries/t11_append_entries_with_bigger_term.rs @@ -8,9 +8,7 @@ use openraft::network::RPCOption; use openraft::network::RaftNetworkFactory; use openraft::raft::AppendEntriesRequest; use openraft::testing::log_id; -use openraft::CommittedLeaderId; use openraft::Config; -use openraft::LogId; use openraft::Vote; use crate::fixtures::ut_harness; @@ -37,15 +35,7 @@ async fn append_entries_with_bigger_term() -> Result<()> { let log_index = router.new_cluster(btreeset! {0}, btreeset! {1}).await?; // before append entries, check hard state in term 1 and vote for node 0 - router - .assert_storage_state( - 1, - log_index, - Some(0), - LogId::new(CommittedLeaderId::new(1, 0), log_index), - None, - ) - .await?; + router.assert_storage_state(1, log_index, Some(0), log_id(1, 0, log_index), None).await?; // append entries with term 2 and leader_id, this MUST cause hard state changed in node 0 let req = AppendEntriesRequest:: { @@ -71,7 +61,7 @@ async fn append_entries_with_bigger_term() -> Result<()> { 2, log_index, Some(1), - LogId::new(CommittedLeaderId::new(1, 0), log_index), + log_id(1, 0, log_index), &None, ) .await?; diff --git a/tests/tests/append_entries/t11_append_updates_membership.rs b/tests/tests/append_entries/t11_append_updates_membership.rs index a265f96b6..36178be31 100644 --- a/tests/tests/append_entries/t11_append_updates_membership.rs +++ b/tests/tests/append_entries/t11_append_updates_membership.rs @@ -5,11 +5,10 @@ use anyhow::Result; use maplit::btreeset; use openraft::raft::AppendEntriesRequest; use openraft::testing::blank_ent; -use openraft::CommittedLeaderId; +use openraft::testing::log_id; use openraft::Config; use openraft::Entry; use openraft::EntryPayload; -use openraft::LogId; use openraft::Membership; use openraft::ServerState; use openraft::Vote; @@ -51,17 +50,17 @@ async fn append_updates_membership() -> Result<()> { blank_ent(0, 0, 0), blank_ent(1, 0, 1), Entry { - log_id: LogId::new(CommittedLeaderId::new(1, 0), 2), + log_id: log_id(1, 0, 2), payload: EntryPayload::Membership(Membership::new_with_defaults(vec![btreeset! {1,2}], [])), }, blank_ent(1, 0, 3), Entry { - log_id: LogId::new(CommittedLeaderId::new(1, 0), 4), + log_id: log_id(1, 0, 4), payload: EntryPayload::Membership(Membership::new_with_defaults(vec![btreeset! {1,2,3,4}], [])), }, blank_ent(1, 0, 5), ], - leader_commit: Some(LogId::new(CommittedLeaderId::new(0, 0), 0)), + leader_commit: Some(log_id(0, 0, 0)), }; let resp = r0.append_entries(req).await?; @@ -75,9 +74,9 @@ async fn append_updates_membership() -> Result<()> { { let req = AppendEntriesRequest { vote: Vote::new_committed(2, 2), - prev_log_id: Some(LogId::new(CommittedLeaderId::new(1, 0), 2)), + prev_log_id: Some(log_id(1, 0, 2)), entries: vec![blank_ent(2, 0, 3)], - leader_commit: Some(LogId::new(CommittedLeaderId::new(0, 0), 0)), + leader_commit: Some(log_id(0, 0, 0)), }; let resp = r0.append_entries(req).await?; diff --git a/tests/tests/client_api/t10_client_writes.rs b/tests/tests/client_api/t10_client_writes.rs index d363fcc9b..ea1026365 100644 --- a/tests/tests/client_api/t10_client_writes.rs +++ b/tests/tests/client_api/t10_client_writes.rs @@ -4,9 +4,8 @@ use anyhow::Result; use futures::prelude::*; use maplit::btreeset; use openraft::raft::ClientWriteResponse; -use openraft::CommittedLeaderId; +use openraft::testing::log_id; use openraft::Config; -use openraft::LogId; use openraft::SnapshotPolicy; use openraft_memstore::ClientRequest; use openraft_memstore::IntoMemClientRequest; @@ -57,7 +56,7 @@ async fn client_writes() -> Result<()> { 1, log_index, Some(0), - LogId::new(CommittedLeaderId::new(1, 0), log_index), + log_id(1, 0, log_index), Some(((499..600).into(), 1)), ) .await?; diff --git a/tests/tests/client_api/t13_trigger_snapshot.rs b/tests/tests/client_api/t13_trigger_snapshot.rs index 6a1f2d003..dd7f1427b 100644 --- a/tests/tests/client_api/t13_trigger_snapshot.rs +++ b/tests/tests/client_api/t13_trigger_snapshot.rs @@ -2,9 +2,8 @@ use std::sync::Arc; use std::time::Duration; use maplit::btreeset; -use openraft::CommittedLeaderId; +use openraft::testing::log_id; use openraft::Config; -use openraft::LogId; use crate::fixtures::ut_harness; use crate::fixtures::RaftRouter; @@ -31,10 +30,7 @@ async fn trigger_snapshot() -> anyhow::Result<()> { let n1 = router.get_raft_handle(&1)?; n1.trigger().snapshot().await?; - router - .wait(&1, timeout()) - .snapshot(LogId::new(CommittedLeaderId::new(1, 0), log_index), "node-1 snapshot") - .await?; + router.wait(&1, timeout()).snapshot(log_id(1, 0, log_index), "node-1 snapshot").await?; } tracing::info!(log_index, "--- send some logs"); @@ -51,10 +47,7 @@ async fn trigger_snapshot() -> anyhow::Result<()> { let n0 = router.get_raft_handle(&0)?; n0.trigger().snapshot().await?; - router - .wait(&0, timeout()) - .snapshot(LogId::new(CommittedLeaderId::new(1, 0), log_index), "node-0 snapshot") - .await?; + router.wait(&0, timeout()).snapshot(log_id(1, 0, log_index), "node-0 snapshot").await?; } Ok(()) diff --git a/tests/tests/fixtures/mod.rs b/tests/tests/fixtures/mod.rs index f7610b89b..a823206d2 100644 --- a/tests/tests/fixtures/mod.rs +++ b/tests/tests/fixtures/mod.rs @@ -186,6 +186,8 @@ impl fmt::Display for Direction { } use openraft::network::v2::RaftNetworkV2; +use openraft::vote::RaftLeaderId; +use openraft::vote::RaftLeaderIdExt; use Direction::NetRecv; use Direction::NetSend; @@ -880,7 +882,7 @@ impl TypedRaftRouter { let vote = storage.read_vote().await?.unwrap_or_else(|| panic!("no hard state found for node {}", id)); assert_eq!( - vote.leader_id().get_term(), + vote.leader_id().term(), expect_term, "expected node {} to have term {}, got {:?}", id, @@ -890,8 +892,8 @@ impl TypedRaftRouter { if let Some(voted_for) = &expect_voted_for { assert_eq!( - vote.leader_id().voted_for(), - Some(*voted_for), + vote.leader_id().node_id_ref(), + Some(voted_for), "expected node {} to have voted for {}, got {:?}", id, voted_for, @@ -1020,7 +1022,7 @@ impl RaftNetworkV2 for RaftRouterNetwork { mut rpc: AppendEntriesRequest, _option: RPCOption, ) -> Result, RPCError> { - let from_id = rpc.vote.leader_id().voted_for().unwrap(); + let from_id = rpc.vote.leader_id().node_id_ref().cloned().unwrap(); tracing::debug!("append_entries to id={} {}", self.target, rpc); self.owner.count_rpc(RPCTypes::AppendEntries); @@ -1090,7 +1092,7 @@ impl RaftNetworkV2 for RaftRouterNetwork { _cancel: impl Future + OptionalSend + 'static, _option: RPCOption, ) -> Result, StreamingError> { - let from_id = vote.leader_id().voted_for().unwrap(); + let from_id = vote.leader_id().node_id().unwrap(); self.owner.count_rpc(RPCTypes::InstallSnapshot); self.owner.call_rpc_pre_hook(snapshot.clone(), from_id, self.target)?; @@ -1116,7 +1118,7 @@ impl RaftNetworkV2 for RaftRouterNetwork { rpc: VoteRequest, _option: RPCOption, ) -> Result, RPCError> { - let from_id = rpc.vote.leader_id().voted_for().unwrap(); + let from_id = rpc.vote.leader_id().node_id().unwrap(); self.owner.count_rpc(RPCTypes::Vote); self.owner.call_rpc_pre_hook(rpc.clone(), from_id, self.target)?; @@ -1141,7 +1143,7 @@ impl RaftNetworkV2 for RaftRouterNetwork { rpc: TransferLeaderRequest, _option: RPCOption, ) -> Result<(), RPCError> { - let from_id = rpc.from_leader().leader_id().voted_for().unwrap(); + let from_id = rpc.from_leader().leader_id().node_id().unwrap(); self.owner.count_rpc(RPCTypes::TransferLeader); self.owner.call_rpc_pre_hook(rpc.clone(), from_id, self.target)?; diff --git a/tests/tests/life_cycle/t10_initialization.rs b/tests/tests/life_cycle/t10_initialization.rs index 8733dc63d..8edbb636f 100644 --- a/tests/tests/life_cycle/t10_initialization.rs +++ b/tests/tests/life_cycle/t10_initialization.rs @@ -8,11 +8,10 @@ use openraft::error::InitializeError; use openraft::error::NotAllowed; use openraft::error::NotInMembers; use openraft::storage::RaftStateMachine; -use openraft::CommittedLeaderId; +use openraft::testing::log_id; use openraft::Config; use openraft::EffectiveMembership; use openraft::EntryPayload; -use openraft::LogId; use openraft::Membership; use openraft::RaftLogReader; use openraft::ServerState; @@ -104,7 +103,7 @@ async fn initialization() -> anyhow::Result<()> { for node_id in [0, 1, 2] { router.external_request(node_id, move |s| { let want = EffectiveMembership::new( - Some(LogId::new(CommittedLeaderId::new(0, 0), 0)), + Some(log_id(0, 0, 0)), Membership::new_with_defaults(vec![btreeset! {0,1,2}], []), ); let want = Arc::new(want); @@ -144,7 +143,7 @@ async fn initialization() -> anyhow::Result<()> { let sm_mem = sm.applied_state().await?.1; assert_eq!( StoredMembership::new( - Some(LogId::new(CommittedLeaderId::new(0, 0), 0)), + Some(log_id(0, 0, 0)), Membership::new_with_defaults(vec![btreeset! {0,1,2}], []) ), sm_mem @@ -247,10 +246,7 @@ async fn initialize_err_not_allowed() -> anyhow::Result<()> { assert_eq!( InitializeError::NotAllowed(NotAllowed { - last_log_id: Some(LogId { - leader_id: CommittedLeaderId::new(1, 0), - index: 1 - }), + last_log_id: Some(log_id(1, 0, 1)), vote: Vote::new_committed(1, 0) }), err.into_api_error().unwrap() diff --git a/tests/tests/life_cycle/t50_single_follower_restart.rs b/tests/tests/life_cycle/t50_single_follower_restart.rs index 80c221245..2e5d40da5 100644 --- a/tests/tests/life_cycle/t50_single_follower_restart.rs +++ b/tests/tests/life_cycle/t50_single_follower_restart.rs @@ -3,6 +3,8 @@ use std::time::Duration; use maplit::btreeset; use openraft::storage::RaftLogStorage; +use openraft::vote::RaftLeaderId; +use openraft::vote::RaftLeaderIdExt; use openraft::Config; use openraft::RaftLogReader; use openraft::ServerState; @@ -46,7 +48,7 @@ async fn single_follower_restart() -> anyhow::Result<()> { let v = sto.read_vote().await?.unwrap_or_default(); // Set a non-committed vote so that the node restarts as a follower. - sto.save_vote(&Vote::new(v.leader_id.get_term() + 1, v.leader_id.voted_for().unwrap())).await?; + sto.save_vote(&Vote::new(v.leader_id.term() + 1, v.leader_id.node_id().unwrap())).await?; tracing::info!(log_index, "--- restart node-0"); diff --git a/tests/tests/membership/t10_single_node.rs b/tests/tests/membership/t10_single_node.rs index 1ff30c317..f9d923c50 100644 --- a/tests/tests/membership/t10_single_node.rs +++ b/tests/tests/membership/t10_single_node.rs @@ -3,9 +3,8 @@ use std::time::Duration; use anyhow::Result; use maplit::btreeset; -use openraft::CommittedLeaderId; +use openraft::testing::log_id; use openraft::Config; -use openraft::LogId; use crate::fixtures::ut_harness; use crate::fixtures::RaftRouter; @@ -38,15 +37,7 @@ async fn single_node() -> Result<()> { // Write some data to the single node cluster. log_index += router.client_request_many(0, "0", 1000).await?; router.wait_for_log(&btreeset![0], Some(log_index), timeout(), "client_request_many").await?; - router - .assert_storage_state( - 1, - log_index, - Some(0), - LogId::new(CommittedLeaderId::new(1, 0), log_index), - None, - ) - .await?; + router.assert_storage_state(1, log_index, Some(0), log_id(1, 0, log_index), None).await?; // Read some data from the single node cluster. router.ensure_linearizable(0).await?; diff --git a/tests/tests/membership/t11_add_learner.rs b/tests/tests/membership/t11_add_learner.rs index 2db937379..b7b12710b 100644 --- a/tests/tests/membership/t11_add_learner.rs +++ b/tests/tests/membership/t11_add_learner.rs @@ -7,14 +7,12 @@ use maplit::btreeset; use openraft::error::ChangeMembershipError; use openraft::error::ClientWriteError; use openraft::error::InProgress; +use openraft::testing::log_id; use openraft::ChangeMembers; -use openraft::CommittedLeaderId; use openraft::Config; -use openraft::LogId; use openraft::Membership; use openraft::RaftLogReader; use openraft::StorageHelper; -use openraft_memstore::TypeConfig; use tokio::time::sleep; use crate::fixtures::ut_harness; @@ -345,10 +343,3 @@ async fn check_learner_after_leader_transferred() -> Result<()> { fn timeout() -> Option { Some(Duration::from_millis(3_000)) } - -pub fn log_id(term: u64, node_id: u64, index: u64) -> LogId { - LogId { - leader_id: CommittedLeaderId::new(term, node_id), - index, - } -} diff --git a/tests/tests/membership/t31_remove_leader.rs b/tests/tests/membership/t31_remove_leader.rs index f14a53715..1982601c1 100644 --- a/tests/tests/membership/t31_remove_leader.rs +++ b/tests/tests/membership/t31_remove_leader.rs @@ -4,9 +4,8 @@ use std::time::Duration; use anyhow::Result; use maplit::btreeset; use openraft::error::ClientWriteError; -use openraft::CommittedLeaderId; +use openraft::testing::log_id; use openraft::Config; -use openraft::LogId; use openraft::ServerState; use openraft_memstore::ClientRequest; use openraft_memstore::IntoMemClientRequest; @@ -100,7 +99,7 @@ async fn remove_leader() -> Result<()> { assert_eq!(metrics.current_term, 1); assert_eq!(metrics.last_log_index, Some(8)); - assert_eq!(metrics.last_applied, Some(LogId::new(CommittedLeaderId::new(1, 0), 8))); + assert_eq!(metrics.last_applied, Some(log_id(1, 0, 8))); assert_eq!(metrics.membership_config.membership().get_joint_config().clone(), vec![ btreeset![1, 2, 3] ]); diff --git a/tests/tests/metrics/t30_leader_metrics.rs b/tests/tests/metrics/t30_leader_metrics.rs index 151499298..8567d5b40 100644 --- a/tests/tests/metrics/t30_leader_metrics.rs +++ b/tests/tests/metrics/t30_leader_metrics.rs @@ -5,9 +5,7 @@ use anyhow::Result; use maplit::btreemap; use maplit::btreeset; use openraft::testing::log_id; -use openraft::CommittedLeaderId; use openraft::Config; -use openraft::LogId; use openraft::ServerState; #[allow(unused_imports)] use pretty_assertions::assert_eq; @@ -87,7 +85,7 @@ async fn leader_metrics() -> Result<()> { router.wait_for_log(&c01234, Some(log_index), timeout(), "change members to 0,1,2,3,4").await?; - let ww = Some(LogId::new(CommittedLeaderId::new(1, 0), log_index)); + let ww = Some(log_id(1, 0, log_index)); let want_repl = btreemap! { 0u64=>ww, 1u64=>ww, 2=>ww, 3=>ww, 4=>ww, }; router .wait_for_metrics( @@ -129,7 +127,7 @@ async fn leader_metrics() -> Result<()> { "--- replication metrics should reflect the replication state" ); { - let ww = Some(LogId::new(CommittedLeaderId::new(1, 0), log_index)); + let ww = Some(log_id(1, 0, log_index)); let want_repl = btreemap! { 0=>ww, 1=>ww, 2=>ww, 3=>ww}; router .wait_for_metrics( diff --git a/tests/tests/snapshot_building/t10_build_snapshot.rs b/tests/tests/snapshot_building/t10_build_snapshot.rs index 12cce8994..937c630d5 100644 --- a/tests/tests/snapshot_building/t10_build_snapshot.rs +++ b/tests/tests/snapshot_building/t10_build_snapshot.rs @@ -9,11 +9,10 @@ use openraft::network::RaftNetworkFactory; use openraft::raft::AppendEntriesRequest; use openraft::storage::RaftLogStorageExt; use openraft::testing::blank_ent; -use openraft::CommittedLeaderId; +use openraft::testing::log_id; use openraft::Config; use openraft::Entry; use openraft::EntryPayload; -use openraft::LogId; use openraft::Membership; use openraft::RaftLogReader; use openraft::SnapshotPolicy; @@ -60,21 +59,14 @@ async fn build_snapshot() -> Result<()> { tracing::info!(log_index, "--- log_index: {}", log_index); router.wait_for_log(&btreeset![0], Some(log_index), timeout(), "write").await?; - router - .wait_for_snapshot( - &btreeset![0], - LogId::new(CommittedLeaderId::new(1, 0), log_index), - None, - "snapshot", - ) - .await?; + router.wait_for_snapshot(&btreeset![0], log_id(1, 0, log_index), None, "snapshot").await?; router .assert_storage_state( 1, log_index, Some(0), - LogId::new(CommittedLeaderId::new(1, 0), log_index), + log_id(1, 0, log_index), Some((log_index.into(), 1)), ) .await?; @@ -82,7 +74,7 @@ async fn build_snapshot() -> Result<()> { // Add a new node and assert that it received the same snapshot. let (mut sto1, sm1) = router.new_store(); sto1.blocking_append([blank_ent(0, 0, 0), Entry { - log_id: LogId::new(CommittedLeaderId::new(1, 0), 1), + log_id: log_id(1, 0, 1), payload: EntryPayload::Membership(Membership::new_with_defaults(vec![btreeset! {0}], [])), }]) .await?; @@ -109,7 +101,7 @@ async fn build_snapshot() -> Result<()> { let (mut sto, _sm) = router.get_storage_handle(&1)?; let logs = sto.try_get_log_entries(..).await?; assert_eq!(2, logs.len()); - assert_eq!(LogId::new(CommittedLeaderId::new(1, 0), log_index - 1), logs[0].log_id) + assert_eq!(log_id(1, 0, log_index - 1), logs[0].log_id) } // log 0 counts @@ -119,7 +111,7 @@ async fn build_snapshot() -> Result<()> { 1, log_index, None, /* learner does not vote */ - LogId::new(CommittedLeaderId::new(1, 0), log_index), + log_id(1, 0, log_index), expected_snap, ) .await?; @@ -136,9 +128,9 @@ async fn build_snapshot() -> Result<()> { .append_entries( AppendEntriesRequest { vote: Vote::new_committed(1, 0), - prev_log_id: Some(LogId::new(CommittedLeaderId::new(1, 0), 2)), + prev_log_id: Some(log_id(1, 0, 2)), entries: vec![], - leader_commit: Some(LogId::new(CommittedLeaderId::new(0, 0), 0)), + leader_commit: Some(log_id(0, 0, 0)), }, option, ) diff --git a/tests/tests/snapshot_streaming/t30_purge_in_snapshot_logs.rs b/tests/tests/snapshot_streaming/t30_purge_in_snapshot_logs.rs index 104161b03..5ba05b54e 100644 --- a/tests/tests/snapshot_streaming/t30_purge_in_snapshot_logs.rs +++ b/tests/tests/snapshot_streaming/t30_purge_in_snapshot_logs.rs @@ -3,9 +3,8 @@ use std::time::Duration; use anyhow::Result; use maplit::btreeset; -use openraft::CommittedLeaderId; +use openraft::testing::log_id; use openraft::Config; -use openraft::LogId; use openraft::RaftLogReader; use tokio::time::sleep; @@ -42,13 +41,7 @@ async fn purge_in_snapshot_logs() -> Result<()> { { log_index += router.client_request_many(0, "0", 10).await?; leader.trigger().snapshot().await?; - leader - .wait(timeout()) - .snapshot( - LogId::new(CommittedLeaderId::new(1, 0), log_index), - "building 1st snapshot", - ) - .await?; + leader.wait(timeout()).snapshot(log_id(1, 0, log_index), "building 1st snapshot").await?; let (mut sto0, mut _sm0) = router.get_storage_handle(&0)?; // Wait for purge to complete. @@ -68,13 +61,7 @@ async fn purge_in_snapshot_logs() -> Result<()> { router.wait(&0, timeout()).applied_index(Some(log_index), "write another 5 logs").await?; leader.trigger().snapshot().await?; - leader - .wait(timeout()) - .snapshot( - LogId::new(CommittedLeaderId::new(1, 0), log_index), - "building 2nd snapshot", - ) - .await?; + leader.wait(timeout()).snapshot(log_id(1, 0, log_index), "building 2nd snapshot").await?; } // There may be a cached append-entries request that already loads log 10..15 from the store, @@ -88,13 +75,7 @@ async fn purge_in_snapshot_logs() -> Result<()> { { router.set_network_error(1, false); - learner - .wait(timeout()) - .snapshot( - LogId::new(CommittedLeaderId::new(1, 0), log_index), - "learner install snapshot", - ) - .await?; + learner.wait(timeout()).snapshot(log_id(1, 0, log_index), "learner install snapshot").await?; let (mut sto1, mut _sm) = router.get_storage_handle(&1)?; let logs = sto1.try_get_log_entries(..).await?; diff --git a/tests/tests/snapshot_streaming/t31_snapshot_overrides_membership.rs b/tests/tests/snapshot_streaming/t31_snapshot_overrides_membership.rs index 271cae3d6..e3ed2a5b3 100644 --- a/tests/tests/snapshot_streaming/t31_snapshot_overrides_membership.rs +++ b/tests/tests/snapshot_streaming/t31_snapshot_overrides_membership.rs @@ -9,12 +9,11 @@ use openraft::network::RaftNetworkFactory; use openraft::raft::AppendEntriesRequest; use openraft::storage::StorageHelper; use openraft::testing::blank_ent; -use openraft::CommittedLeaderId; +use openraft::testing::log_id; use openraft::Config; use openraft::EffectiveMembership; use openraft::Entry; use openraft::EntryPayload; -use openraft::LogId; use openraft::Membership; use openraft::SnapshotPolicy; use openraft::Vote; @@ -62,20 +61,13 @@ async fn snapshot_overrides_membership() -> Result<()> { ) .await?; - router - .wait_for_snapshot( - &btreeset![0], - LogId::new(CommittedLeaderId::new(1, 0), log_index), - timeout(), - "snapshot", - ) - .await?; + router.wait_for_snapshot(&btreeset![0], log_id(1, 0, log_index), timeout(), "snapshot").await?; router .assert_storage_state( 1, log_index, Some(0), - LogId::new(CommittedLeaderId::new(1, 0), log_index), + log_id(1, 0, log_index), Some((log_index.into(), 1)), ) .await?; @@ -93,10 +85,10 @@ async fn snapshot_overrides_membership() -> Result<()> { vote: Vote::new_committed(1, 0), prev_log_id: None, entries: vec![blank_ent(0, 0, 0), Entry { - log_id: LogId::new(CommittedLeaderId::new(1, 0), 1), + log_id: log_id(1, 0, 1), payload: EntryPayload::Membership(Membership::new_with_defaults(vec![btreeset! {2,3}], [])), }], - leader_commit: Some(LogId::new(CommittedLeaderId::new(0, 0), 0)), + leader_commit: Some(log_id(0, 0, 0)), }; let option = RPCOption::new(Duration::from_millis(1_000)); @@ -127,14 +119,7 @@ async fn snapshot_overrides_membership() -> Result<()> { tracing::info!(log_index, "--- DONE add learner"); router.wait_for_log(&btreeset![0, 1], Some(log_index), timeout(), "add learner").await?; - router - .wait_for_snapshot( - &btreeset![1], - LogId::new(CommittedLeaderId::new(1, 0), snapshot_index), - timeout(), - "", - ) - .await?; + router.wait_for_snapshot(&btreeset![1], log_id(1, 0, snapshot_index), timeout(), "").await?; let expected_snap = Some((snapshot_index.into(), 1)); @@ -143,7 +128,7 @@ async fn snapshot_overrides_membership() -> Result<()> { 1, log_index, None, /* learner does not vote */ - LogId::new(CommittedLeaderId::new(1, 0), log_index), + log_id(1, 0, log_index), expected_snap, ) .await?; diff --git a/tests/tests/snapshot_streaming/t32_snapshot_uses_prev_snap_membership.rs b/tests/tests/snapshot_streaming/t32_snapshot_uses_prev_snap_membership.rs index 9031973ad..f1d17e156 100644 --- a/tests/tests/snapshot_streaming/t32_snapshot_uses_prev_snap_membership.rs +++ b/tests/tests/snapshot_streaming/t32_snapshot_uses_prev_snap_membership.rs @@ -3,9 +3,8 @@ use std::time::Duration; use anyhow::Result; use maplit::btreeset; -use openraft::CommittedLeaderId; +use openraft::testing::log_id; use openraft::Config; -use openraft::LogId; use openraft::Membership; use openraft::RaftLogReader; use openraft::SnapshotPolicy; @@ -63,14 +62,7 @@ async fn snapshot_uses_prev_snap_membership() -> Result<()> { "send log to trigger snapshot", ) .await?; - router - .wait_for_snapshot( - &btreeset![0], - LogId::new(CommittedLeaderId::new(1, 0), log_index), - timeout(), - "1st snapshot", - ) - .await?; + router.wait_for_snapshot(&btreeset![0], log_id(1, 0, log_index), timeout(), "1st snapshot").await?; { let logs = sto0.try_get_log_entries(..).await?; @@ -96,14 +88,7 @@ async fn snapshot_uses_prev_snap_membership() -> Result<()> { log_index = snapshot_threshold * 2 - 1; router.wait_for_log(&btreeset![0, 1], Some(log_index), None, "send log to trigger snapshot").await?; - router - .wait_for_snapshot( - &btreeset![0], - LogId::new(CommittedLeaderId::new(1, 0), log_index), - None, - "2nd snapshot", - ) - .await?; + router.wait_for_snapshot(&btreeset![0], log_id(1, 0, log_index), None, "2nd snapshot").await?; } tracing::info!(log_index, "--- check membership"); diff --git a/tests/tests/snapshot_streaming/t33_snapshot_delete_conflict_logs.rs b/tests/tests/snapshot_streaming/t33_snapshot_delete_conflict_logs.rs index 4b5b2ab40..650bc5ef1 100644 --- a/tests/tests/snapshot_streaming/t33_snapshot_delete_conflict_logs.rs +++ b/tests/tests/snapshot_streaming/t33_snapshot_delete_conflict_logs.rs @@ -13,11 +13,9 @@ use openraft::storage::RaftStateMachine; use openraft::testing::blank_ent; use openraft::testing::log_id; use openraft::testing::membership_ent; -use openraft::CommittedLeaderId; use openraft::Config; use openraft::Entry; use openraft::EntryPayload; -use openraft::LogId; use openraft::Membership; use openraft::RaftLogReader; use openraft::RaftSnapshotBuilder; @@ -80,10 +78,7 @@ async fn snapshot_delete_conflicting_logs() -> Result<()> { log_index = snapshot_threshold - 1; router.wait(&0, timeout()).applied_index(Some(log_index), "trigger snapshot").await?; - router - .wait(&0, timeout()) - .snapshot(LogId::new(CommittedLeaderId::new(5, 0), log_index), "build snapshot") - .await?; + router.wait(&0, timeout()).snapshot(log_id(5, 0, log_index), "build snapshot").await?; } tracing::info!(log_index, "--- create node-1 and add conflicting logs"); @@ -98,7 +93,7 @@ async fn snapshot_delete_conflicting_logs() -> Result<()> { blank_ent(1, 0, 1), // conflict membership will be replaced with membership in snapshot Entry { - log_id: LogId::new(CommittedLeaderId::new(1, 0), 2), + log_id: log_id(1, 0, 2), payload: EntryPayload::Membership(Membership::new_with_defaults(vec![btreeset! {2,3}], [])), }, blank_ent(1, 0, 3), @@ -111,11 +106,11 @@ async fn snapshot_delete_conflicting_logs() -> Result<()> { blank_ent(1, 0, 10), // another conflict membership, will be removed Entry { - log_id: LogId::new(CommittedLeaderId::new(1, 0), 11), + log_id: log_id(1, 0, 11), payload: EntryPayload::Membership(Membership::new_with_defaults(vec![btreeset! {4,5}], [])), }, ], - leader_commit: Some(LogId::new(CommittedLeaderId::new(1, 0), 2)), + leader_commit: Some(log_id(1, 0, 2)), }; let option = RPCOption::new(Duration::from_millis(1_000)); @@ -185,12 +180,12 @@ async fn snapshot_delete_conflicting_logs() -> Result<()> { let log_st = sto1.get_log_state().await?; assert_eq!( - Some(LogId::new(CommittedLeaderId::new(5, 0), snapshot_threshold - 1)), + Some(log_id(5, 0, snapshot_threshold - 1)), log_st.last_purged_log_id, "purge up to last log id in snapshot" ); assert_eq!( - Some(LogId::new(CommittedLeaderId::new(5, 0), snapshot_threshold - 1)), + Some(log_id(5, 0, snapshot_threshold - 1)), log_st.last_log_id, "reverted to last log id in snapshot" ); diff --git a/tests/tests/snapshot_streaming/t34_replication_does_not_block_purge.rs b/tests/tests/snapshot_streaming/t34_replication_does_not_block_purge.rs index df2b1b157..091ccf170 100644 --- a/tests/tests/snapshot_streaming/t34_replication_does_not_block_purge.rs +++ b/tests/tests/snapshot_streaming/t34_replication_does_not_block_purge.rs @@ -3,9 +3,8 @@ use std::time::Duration; use anyhow::Result; use maplit::btreeset; -use openraft::CommittedLeaderId; +use openraft::testing::log_id; use openraft::Config; -use openraft::LogId; use openraft::RaftLogReader; use tokio::time::sleep; @@ -48,10 +47,7 @@ async fn replication_does_not_block_purge() -> Result<()> { log_index += router.client_request_many(0, "0", 10).await?; leader.trigger().snapshot().await?; - leader - .wait(timeout()) - .snapshot(LogId::new(CommittedLeaderId::new(1, 0), log_index), "built snapshot") - .await?; + leader.wait(timeout()).snapshot(log_id(1, 0, log_index), "built snapshot").await?; sleep(Duration::from_millis(500)).await; diff --git a/tests/tests/snapshot_streaming/t50_snapshot_line_rate_to_snapshot.rs b/tests/tests/snapshot_streaming/t50_snapshot_line_rate_to_snapshot.rs index 8c5fd351b..a59f174ce 100644 --- a/tests/tests/snapshot_streaming/t50_snapshot_line_rate_to_snapshot.rs +++ b/tests/tests/snapshot_streaming/t50_snapshot_line_rate_to_snapshot.rs @@ -3,9 +3,8 @@ use std::time::Duration; use anyhow::Result; use maplit::btreeset; -use openraft::CommittedLeaderId; +use openraft::testing::log_id; use openraft::Config; -use openraft::LogId; use openraft::SnapshotPolicy; use crate::fixtures::ut_harness; @@ -71,12 +70,7 @@ async fn snapshot_line_rate_to_snapshot() -> Result<()> { ) .await?; router - .wait_for_snapshot( - &btreeset![0], - LogId::new(CommittedLeaderId::new(1, 0), log_index), - timeout(), - "snapshot on node 0", - ) + .wait_for_snapshot(&btreeset![0], log_id(1, 0, log_index), timeout(), "snapshot on node 0") .await?; } @@ -86,12 +80,7 @@ async fn snapshot_line_rate_to_snapshot() -> Result<()> { router.wait_for_log(&btreeset![1], Some(log_index), timeout(), "replicate by snapshot").await?; router - .wait_for_snapshot( - &btreeset![1], - LogId::new(CommittedLeaderId::new(1, 0), log_index), - timeout(), - "snapshot on node 1", - ) + .wait_for_snapshot(&btreeset![1], log_id(1, 0, log_index), timeout(), "snapshot on node 1") .await?; } diff --git a/tests/tests/snapshot_streaming/t50_snapshot_when_lacking_log.rs b/tests/tests/snapshot_streaming/t50_snapshot_when_lacking_log.rs index d0bd293c2..2052b0deb 100644 --- a/tests/tests/snapshot_streaming/t50_snapshot_when_lacking_log.rs +++ b/tests/tests/snapshot_streaming/t50_snapshot_when_lacking_log.rs @@ -2,9 +2,8 @@ use std::sync::Arc; use anyhow::Result; use maplit::btreeset; -use openraft::CommittedLeaderId; +use openraft::testing::log_id; use openraft::Config; -use openraft::LogId; use openraft::SnapshotPolicy; use crate::fixtures::ut_harness; @@ -42,20 +41,13 @@ async fn switch_to_snapshot_replication_when_lacking_log() -> Result<()> { router.wait_for_log(&btreeset![0], Some(log_index), None, "send log to trigger snapshot").await?; - router - .wait_for_snapshot( - &btreeset![0], - LogId::new(CommittedLeaderId::new(1, 0), log_index), - None, - "snapshot", - ) - .await?; + router.wait_for_snapshot(&btreeset![0], log_id(1, 0, log_index), None, "snapshot").await?; router .assert_storage_state( 1, log_index, Some(0), - LogId::new(CommittedLeaderId::new(1, 0), log_index), + log_id(1, 0, log_index), Some((log_index.into(), 1)), ) .await?; @@ -77,21 +69,14 @@ async fn switch_to_snapshot_replication_when_lacking_log() -> Result<()> { log_index += 1; router.wait_for_log(&btreeset![0, 1], Some(log_index), None, "add learner").await?; - router - .wait_for_snapshot( - &btreeset![1], - LogId::new(CommittedLeaderId::new(1, 0), snapshot_threshold - 1), - None, - "", - ) - .await?; + router.wait_for_snapshot(&btreeset![1], log_id(1, 0, snapshot_threshold - 1), None, "").await?; let expected_snap = Some(((snapshot_threshold - 1).into(), 1)); router .assert_storage_state( 1, log_index, None, /* learner does not vote */ - LogId::new(CommittedLeaderId::new(1, 0), log_index), + log_id(1, 0, log_index), expected_snap, ) .await?; diff --git a/tests/tests/snapshot_streaming/t51_after_snapshot_add_learner_and_request_a_log.rs b/tests/tests/snapshot_streaming/t51_after_snapshot_add_learner_and_request_a_log.rs index 9a7c02164..234743c50 100644 --- a/tests/tests/snapshot_streaming/t51_after_snapshot_add_learner_and_request_a_log.rs +++ b/tests/tests/snapshot_streaming/t51_after_snapshot_add_learner_and_request_a_log.rs @@ -4,9 +4,7 @@ use std::time::Duration; use anyhow::Result; use maplit::btreeset; use openraft::testing::log_id; -use openraft::CommittedLeaderId; use openraft::Config; -use openraft::LogId; use openraft::SnapshotPolicy; use crate::fixtures::ut_harness; @@ -78,10 +76,7 @@ async fn after_snapshot_add_learner_and_request_a_log() -> Result<()> { router .wait(&1, timeout()) - .snapshot( - LogId::new(CommittedLeaderId::new(1, 0), snapshot_index), - "learner-1 receives snapshot", - ) + .snapshot(log_id(1, 0, snapshot_index), "learner-1 receives snapshot") .await?; log_index += router.client_request_many(0, "0", 1).await?; @@ -103,7 +98,7 @@ async fn after_snapshot_add_learner_and_request_a_log() -> Result<()> { 1, log_index, None, /* learner does not vote */ - LogId::new(CommittedLeaderId::new(1, 0), log_index), + log_id(1, 0, log_index), expected_snap, ) .await?; diff --git a/tests/tests/state_machine/t20_state_machine_apply_membership.rs b/tests/tests/state_machine/t20_state_machine_apply_membership.rs index 2b6ddec4b..dba3e3707 100644 --- a/tests/tests/state_machine/t20_state_machine_apply_membership.rs +++ b/tests/tests/state_machine/t20_state_machine_apply_membership.rs @@ -3,9 +3,8 @@ use std::sync::Arc; use anyhow::Result; use maplit::btreeset; use openraft::storage::RaftStateMachine; -use openraft::CommittedLeaderId; +use openraft::testing::log_id; use openraft::Config; -use openraft::LogId; use openraft::LogIdOptionExt; use openraft::Membership; use openraft::StoredMembership; @@ -40,7 +39,7 @@ async fn state_machine_apply_membership() -> Result<()> { let (_sto, mut sm) = router.get_storage_handle(&i)?; assert_eq!( StoredMembership::new( - Some(LogId::new(CommittedLeaderId::new(0, 0), 0)), + Some(log_id(0, 0, 0)), Membership::new_with_defaults(vec![btreeset! {0}], []) ), sm.applied_state().await?.1 @@ -88,7 +87,7 @@ async fn state_machine_apply_membership() -> Result<()> { let (_, last_membership) = sm.applied_state().await?; assert_eq!( StoredMembership::new( - Some(LogId::new(CommittedLeaderId::new(1, 0), log_index)), + Some(log_id(1, 0, log_index)), Membership::new_with_defaults(vec![btreeset! {0, 1, 2}], btreeset! {3,4}) ), last_membership From 6908a3cb0f3f7d4481c382bfe68cf4831004cadf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E7=82=8E=E6=B3=BC?= Date: Sat, 28 Dec 2024 17:44:34 +0800 Subject: [PATCH 2/4] Change: Remove feature flag `single-term-leader` In OpenRaft, whether a term supports multiple or single leaders is determined by the ordering implementation of `LeaderId`. This behavior is independent of compilation and can be configured at the application level, removing the need for a compilation flag `single-term-leader`. ### Changes: - The `single-term-leader` feature flag is removed. - Leader mode (multi-leader or single-leader per term) is now controlled by the `LeaderId` associated type in the application configuration. ### Configuration Guide: To enable **standard Raft mode (single leader per term)**: - Set `LeaderId` to `openraft::impls::leader_id_std::LeaderId` in the `declare_raft_types!` statement, or within the `RaftTypeConfig` implementation. Example: ```rust openraft::declare_raft_types!( pub TypeConfig: D = String, LeaderId = openraft::impls::leader_id_std::LeaderId, ); ``` To use the **default multi-leader per term mode**, leave `LeaderId` unset or explicitly set it to `openraft::impls::leader_id_adv::LeaderId`. Example: ```rust openraft::declare_raft_types!( pub TypeConfig: D = String, ); // Or: openraft::declare_raft_types!( pub TypeConfig: D = String, LeaderId = openraft::impls::leader_id_adv::LeaderId, ); ``` Upgrade tip: If the `single-term-leader` flag was previously used, remove it and configure `LeaderId` as follows: ```rust openraft::declare_raft_types!( pub TypeConfig: LeaderId = openraft::impls::leader_id_std::LeaderId, ); ``` --- .github/workflows/ci.yaml | 76 ++++++++++++------- cluster_benchmark/Cargo.toml | 2 +- cluster_benchmark/tests/benchmark/store.rs | 9 +++ examples/utils/declare_types.rs | 2 +- openraft/Cargo.toml | 16 +--- openraft/src/core/notification.rs | 4 +- openraft/src/core/raft_core.rs | 4 +- openraft/src/core/tick.rs | 2 +- openraft/src/docs/data/leader_id.md | 25 +++--- openraft/src/docs/data/vote.md | 10 ++- openraft/src/docs/faq/faq.md | 2 - .../src/docs/feature_flags/feature-flags.md | 6 ++ openraft/src/engine/command.rs | 2 +- openraft/src/engine/engine_impl.rs | 2 +- .../engine/handler/following_handler/mod.rs | 2 +- openraft/src/engine/testing.rs | 2 +- openraft/src/impls/mod.rs | 13 ++-- openraft/src/lib.rs | 2 - openraft/src/proposer/leader.rs | 2 +- openraft/src/raft/declare_raft_types_test.rs | 2 + openraft/src/raft/mod.rs | 24 +++--- openraft/src/raft_state/io_state/io_id.rs | 4 +- openraft/src/raft_state/io_state/log_io_id.rs | 2 +- .../src/replication/replication_session_id.rs | 2 +- openraft/src/replication/response.rs | 2 - openraft/src/summary.rs | 2 - openraft/src/type_config.rs | 6 +- openraft/src/vote/leader_id/leader_id_adv.rs | 12 +-- openraft/src/vote/leader_id/leader_id_std.rs | 31 +++++--- openraft/src/vote/leader_id/mod.rs | 23 +++--- openraft/src/vote/leader_id/raft_leader_id.rs | 3 + openraft/src/vote/mod.rs | 14 ++-- .../src/vote/raft_term/raft_term_impls.rs | 2 +- openraft/src/vote/vote.rs | 18 ++--- openraft/src/vote/vote_status.rs | 4 +- stores/memstore/Cargo.toml | 1 + stores/memstore/src/lib.rs | 9 +++ tests/Cargo.toml | 2 +- 38 files changed, 196 insertions(+), 150 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 321ab6ae4..ba8f72c30 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -63,8 +63,8 @@ jobs: args: --features bench nothing-to-run --manifest-path openraft/Cargo.toml - # Run openraft unit test `openraft/` and integration test `tests/`. - openraft-test: + # Run openraft unit test `openraft/` + test-crate-openraft: runs-on: ubuntu-latest strategy: @@ -72,20 +72,11 @@ jobs: matrix: include: - toolchain: "stable" - send_delay: "0" features: "" - # With network delay - toolchain: "nightly" - send_delay: "30" features: "" - # Feature-flag: Standard raft term - - toolchain: "nightly" - send_delay: "0" - features: "single-term-leader" - - steps: - name: Setup | Checkout uses: actions/checkout@v4 @@ -98,19 +89,57 @@ jobs: override: true - # - A store with defensive checks returns error when unexpected accesses are sent to RaftStore. - # - Raft should not depend on defensive error to work correctly. - name: Test crate `openraft/` uses: actions-rs/cargo@v1 with: command: test args: --features "${{ matrix.features }}" --manifest-path openraft/Cargo.toml env: - # Parallel tests block each other and result in timeout. - RUST_TEST_THREADS: 2 RUST_LOG: debug RUST_BACKTRACE: full - OPENRAFT_NETWORK_SEND_DELAY: ${{ matrix.send_delay }} + + + - name: Upload artifact + uses: actions/upload-artifact@v3 + if: failure() + with: + name: "ut-openraft-${{ matrix.toolchain }}-${{ matrix.features }}" + path: | + openraft/_log/ + + # Run integration test `tests/`. + test-crate-tests: + runs-on: ubuntu-latest + + strategy: + fail-fast: false + matrix: + include: + - toolchain: "stable" + send_delay: "0" + features: "" + + # With network delay + - toolchain: "nightly" + send_delay: "30" + features: "" + + # Feature-flag: Standard raft term + - toolchain: "nightly" + send_delay: "0" + features: "single-term-leader" + + + steps: + - name: Setup | Checkout + uses: actions/checkout@v4 + + + - name: Setup | Toolchain + uses: actions-rs/toolchain@v1.0.6 + with: + toolchain: "${{ matrix.toolchain }}" + override: true - name: Test crate `tests/` @@ -130,9 +159,8 @@ jobs: uses: actions/upload-artifact@v3 if: failure() with: - name: "ut-${{ matrix.toolchain }}-${{ matrix.features }}-${{ matrix.send_delay }}" + name: "ut-tests-${{ matrix.toolchain }}-${{ matrix.features }}-${{ matrix.send_delay }}" path: | - openraft/_log/ tests/_log/ @@ -252,16 +280,8 @@ jobs: - toolchain: "nightly" features: "serde" - # Some test requires feature single-term-leader on and serde off. - # This can only be tested without building another crate that enables - # `serde`. - # Move this test to unit-test when the snapshot API is upgraded to - # non-serde-dependent. - - toolchain: "nightly" - features: "single-term-leader" - - toolchain: "nightly" - features: "single-term-leader,serde,singlethreaded" + features: "serde,singlethreaded" steps: @@ -364,7 +384,7 @@ jobs: shell: bash run: | cargo clippy --no-deps --workspace --all-targets -- -D warnings - cargo clippy --no-deps --workspace --all-targets --features "bt,serde,bench,single-term-leader,compat" -- -D warnings + cargo clippy --no-deps --workspace --all-targets --features "bt,serde,bench,compat" -- -D warnings - name: Build-doc diff --git a/cluster_benchmark/Cargo.toml b/cluster_benchmark/Cargo.toml index 1cfac6891..6e0d45698 100644 --- a/cluster_benchmark/Cargo.toml +++ b/cluster_benchmark/Cargo.toml @@ -31,7 +31,7 @@ tracing = "0.1.29" [features] bt = ["openraft/bt"] -single-term-leader = ["openraft/single-term-leader"] +single-term-leader = [] [profile.release] debug = 2 diff --git a/cluster_benchmark/tests/benchmark/store.rs b/cluster_benchmark/tests/benchmark/store.rs index 157454c6c..10ac2f528 100644 --- a/cluster_benchmark/tests/benchmark/store.rs +++ b/cluster_benchmark/tests/benchmark/store.rs @@ -37,11 +37,20 @@ pub struct ClientResponse {} pub type NodeId = u64; +/// Choose a LeaderId implementation by feature flag. +mod leader_id_mode { + #[cfg(not(feature = "single-term-leader"))] + pub use openraft::impls::leader_id_adv::LeaderId; + #[cfg(feature = "single-term-leader")] + pub use openraft::impls::leader_id_std::LeaderId; +} + openraft::declare_raft_types!( pub TypeConfig: D = ClientRequest, R = ClientResponse, Node = (), + LeaderId = leader_id_mode::LeaderId, ); #[derive(Debug)] diff --git a/examples/utils/declare_types.rs b/examples/utils/declare_types.rs index 69491f66a..d7c6ec488 100644 --- a/examples/utils/declare_types.rs +++ b/examples/utils/declare_types.rs @@ -4,7 +4,7 @@ use super::TypeConfig; pub type Raft = openraft::Raft; pub type Vote = openraft::Vote; -pub type LeaderId = openraft::LeaderId; +pub type LeaderId = ::LeaderId; pub type LogId = openraft::LogId; pub type Entry = openraft::Entry; pub type EntryPayload = openraft::EntryPayload; diff --git a/openraft/Cargo.toml b/openraft/Cargo.toml index 13bef2751..2a0149ef6 100644 --- a/openraft/Cargo.toml +++ b/openraft/Cargo.toml @@ -62,17 +62,9 @@ bt = ["anyerror/backtrace", "anyhow/backtrace"] # If you'd like to use `serde` to serialize messages. serde = ["dep:serde"] -# Turn on this feature it allows at most ONE quorum-granted leader for each term. -# This is the way standard raft does, by making the LeaderId a partial order value. -# -# - With this feature on: -# It is more likely to conflict during election. But every log only needs to store one `term` in it. -# -# - With this feature off: -# Election conflict rate will be reduced, but every log has to store a `LeaderId{ term, node_id}`, -# which may be costly if an application uses a big NodeId type. -# -# This feature is disabled by default. +# This feature is removed. +# Use `openraft::impls::leader_id_std::Leader` for `RaftTypeConfig` +# to enable standard Raft leader election. single-term-leader = [] # Enable this feature to use `openraft::alias::*` type shortcuts. @@ -103,8 +95,6 @@ loosen-follower-log-revert = [] # See: https://docs.rs/tracing/latest/tracing/#emitting-log-records tracing-log = [ "tracing/log" ] -# default = ["single-term-leader"] - [package.metadata.docs.rs] # Enable these feature flags to show all types/mods, diff --git a/openraft/src/core/notification.rs b/openraft/src/core/notification.rs index 553882c45..f45342424 100644 --- a/openraft/src/core/notification.rs +++ b/openraft/src/core/notification.rs @@ -7,8 +7,8 @@ use crate::raft_state::IOId; use crate::replication; use crate::replication::ReplicationSessionId; use crate::type_config::alias::InstantOf; -use crate::vote::CommittedVote; -use crate::vote::NonCommittedVote; +use crate::vote::committed::CommittedVote; +use crate::vote::non_committed::NonCommittedVote; use crate::RaftTypeConfig; use crate::StorageError; use crate::Vote; diff --git a/openraft/src/core/raft_core.rs b/openraft/src/core/raft_core.rs index 8a17aa944..05019efa1 100644 --- a/openraft/src/core/raft_core.rs +++ b/openraft/src/core/raft_core.rs @@ -93,9 +93,9 @@ use crate::type_config::alias::ResponderOf; use crate::type_config::alias::WatchSenderOf; use crate::type_config::async_runtime::MpscUnboundedReceiver; use crate::type_config::TypeConfigExt; +use crate::vote::committed::CommittedVote; +use crate::vote::non_committed::NonCommittedVote; use crate::vote::vote_status::VoteStatus; -use crate::vote::CommittedVote; -use crate::vote::NonCommittedVote; use crate::vote::RaftLeaderId; use crate::ChangeMembers; use crate::Instant; diff --git a/openraft/src/core/tick.rs b/openraft/src/core/tick.rs index 8a809d063..425d0b7d2 100644 --- a/openraft/src/core/tick.rs +++ b/openraft/src/core/tick.rs @@ -180,7 +180,7 @@ mod tests { type NodeId = u64; type Node = (); type Term = u64; - type LeaderId = crate::impls::LeaderId; + type LeaderId = crate::impls::leader_id_adv::LeaderId; type Entry = crate::Entry; type SnapshotData = Cursor>; type AsyncRuntime = TokioRuntime; diff --git a/openraft/src/docs/data/leader_id.md b/openraft/src/docs/data/leader_id.md index a437d62de..20d2d0d6b 100644 --- a/openraft/src/docs/data/leader_id.md +++ b/openraft/src/docs/data/leader_id.md @@ -1,8 +1,13 @@ ## Leader-id in Advanced mode and Standard mode -Openraft provides two `LeaderId` types to switch between these two modes with feature [`single-term-leader`] : +Openraft provides two `LeaderId` types to switch between these two modes: +- the default mode: every term may have more than one leader + (enabled by default, or explicitly by setting [`RaftTypeConfig::LeaderId`] to [`leader_id_adv::LeaderId`]). +- and the standard Raft mode: every term has only one leader + (enabled by setting [`RaftTypeConfig::LeaderId`] to [`leader_id_std::LeaderId`]). -The feature [`single-term-leader`] affects the `PartialOrd` implementation of `LeaderId`, where `LeaderId` is defined as a tuple `(term, node_id)`. +[`leader_id_adv::LeaderId`] is totally ordered. +[`leader_id_std::LeaderId`] is `PartialOrd`. ### Definition of `LeaderId` @@ -15,27 +20,26 @@ Within Openraft, and also implicitly in the standard Raft, `LeaderId` is utilize `A.term > B.term ↔ A > B`.

-- Conversely, in Openraft, with the [`single-term-leader`] feature disabled by default, `LeaderId` follows a `total order` based on lexicographical comparison: +- Conversely, in Openraft, with [`leader_id_adv::LeaderId`], `LeaderId` follows a `total order` based on lexicographical comparison: `A.term > B.term || (A.term == B.term && A.node_id > B.node_id) ↔ A > B`. -Activating the `single-term-leader` feature makes `LeaderId` conform to the `partial order` seen in standard Raft. +Using [`leader_id_std::LeaderId`] makes `LeaderId` conform to the `partial order` seen in standard Raft. ### Usage of `LeaderId` When handling `VoteRequest`, both Openraft and standard Raft (though not explicitly detailed) rely on the ordering of `LeaderId` to decide whether to grant a vote: **a node will grant a vote with a `LeaderId` that is greater than any it has previously granted**. -Consequently, by default in Openraft (with `single-term-leader` disabled), it is possible to elect multiple `Leader`s within the same term, with the last elected `Leader` being recognized as valid. In contrast, under standard Raft protocol, only a single `Leader` is elected per `term`. +Consequently, by default in Openraft (with [`leader_id_adv::LeaderId`]), it is possible to elect multiple `Leader`s within the same term, with the last elected `Leader` being recognized as valid. In contrast, under standard Raft protocol, only a single `Leader` is elected per `term`. ### Default: advanced mode -`cargo build` without [`single-term-leader`][], is the advanced mode, the default mode: +Use `openraft::impls::leader_id_adv::LeaderId` for [`RaftTypeConfig::LeaderId`] or leave it to default to switch to advanced mode. `LeaderId` is defined as the following, and it is a **totally ordered** value(two or more leaders can be granted in the same term): ```ignore // Advanced mode(default): -#[cfg(not(feature = "single-term-leader"))] #[derive(PartialOrd, Ord)] pub struct LeaderId { @@ -57,12 +61,11 @@ elected(although only the last is valid and can commit logs). #### Standard mode -`cargo build --features "single-term-leader"` builds openraft in standard raft mode. +Use `openraft::impls::leader_id_std::LeaderId` for [`RaftTypeConfig::LeaderId`] to switch to standard mode. In the standard mode, `LeaderId` is defined as the following, and it is a **partially ordered** value(no two leaders can be granted in the same term): ```ignore // Standard raft mode: -#[cfg(feature = "single-term-leader")] pub struct LeaderId { pub term: u64, @@ -125,4 +128,6 @@ So let a committed `Vote` override a incomparable non-committed is safe. - Cons: election conflicting rate may increase. -[`single-term-leader`]: crate::docs::feature_flags +[`RaftTypeConfig::LeaderId`]: crate::RaftTypeConfig::LeaderId +[`leader_id_adv::LeaderId`]: `crate::impls::leader_id_adv::LeaderId` +[`leader_id_std::LeaderId`]: `crate::impls::leader_id_std::LeaderId` diff --git a/openraft/src/docs/data/vote.md b/openraft/src/docs/data/vote.md index c6c36fc31..9f44c4f0b 100644 --- a/openraft/src/docs/data/vote.md +++ b/openraft/src/docs/data/vote.md @@ -34,8 +34,10 @@ i.e., it is legal that `!(vote_a => vote_b) && !(vote_a <= vote_b)`. Because `Vote.leader_id` may be a partial order value: Openraft provides two election modes. -- the default mode: every term may have more than one leader. -- and the standard Raft mode: every term has only one leader(enabled by [`single-term-leader`]), +- the default mode: every term may have more than one leader + (enabled by default, or explicitly by setting [`RaftTypeConfig::LeaderId`] to [`leader_id_adv::LeaderId`]). +- and the standard Raft mode: every term has only one leader + (enabled by setting [`RaftTypeConfig::LeaderId`] to [`leader_id_std::LeaderId`]), The only difference between these two modes is the definition of `LeaderId`, and the `PartialOrd` implementation of it. See: [`leader-id`]. @@ -79,5 +81,7 @@ For node-2: [`Vote`]: `crate::vote::Vote` -[`single-term-leader`]: `crate::docs::feature_flags` +[`RaftTypeConfig::LeaderId`]: `crate::RaftTypeConfig::LeaderId` +[`leader_id_adv::LeaderId`]: `crate::impls::leader_id_adv::LeaderId` +[`leader_id_std::LeaderId`]: `crate::impls::leader_id_std::LeaderId` [`leader-id`]: `crate::docs::data::leader_id` diff --git a/openraft/src/docs/faq/faq.md b/openraft/src/docs/faq/faq.md index 48f6a28ff..97f514131 100644 --- a/openraft/src/docs/faq/faq.md +++ b/openraft/src/docs/faq/faq.md @@ -237,8 +237,6 @@ pub(crate) fn following_handler(&mut self) -> FollowingHandler { ``` -[`single-term-leader`]: `crate::docs::feature_flags#single_term_leader` - [`Linearizable Read`]: `crate::docs::protocol::read` [`leader_id`]: `crate::docs::data::leader_id` diff --git a/openraft/src/docs/feature_flags/feature-flags.md b/openraft/src/docs/feature_flags/feature-flags.md index 2dba5e27a..70eee4e12 100644 --- a/openraft/src/docs/feature_flags/feature-flags.md +++ b/openraft/src/docs/feature_flags/feature-flags.md @@ -24,6 +24,9 @@ in storage and network, such as `Vote` or `AppendEntriesRequest`. ## feature-flag `single-term-leader` +**This feature flag is removed**. +User [`leader_id_std::LeaderId`] in [`RaftTypeConfig`] instead. + Allows only one leader to be elected in each `term`. This is the standard raft policy, which increases election conflict rate but reduce `LogId` size(`(term, node_id, index)` to `(term, index)`). @@ -72,3 +75,6 @@ let snapshot_data: ::SnapshotData; Note that the type shortcuts are not stable and may be changed in the future. It is also a good idea to copy the type shortcuts to your own codebase if you want to use them. + +[`RaftTypeConfig`]: crate::RaftTypeConfig +[`leader_id_std::LeaderId`]: crate::impls::leader_id_std::LeaderId diff --git a/openraft/src/engine/command.rs b/openraft/src/engine/command.rs index a9a57be27..f95107ccb 100644 --- a/openraft/src/engine/command.rs +++ b/openraft/src/engine/command.rs @@ -21,7 +21,7 @@ use crate::raft_state::IOId; use crate::replication::request::Replicate; use crate::replication::ReplicationSessionId; use crate::type_config::alias::OneshotSenderOf; -use crate::vote::CommittedVote; +use crate::vote::committed::CommittedVote; use crate::LogId; use crate::OptionalSend; use crate::RaftTypeConfig; diff --git a/openraft/src/engine/engine_impl.rs b/openraft/src/engine/engine_impl.rs index 9366e2344..38587d2a2 100644 --- a/openraft/src/engine/engine_impl.rs +++ b/openraft/src/engine/engine_impl.rs @@ -47,8 +47,8 @@ use crate::storage::SnapshotMeta; use crate::type_config::alias::ResponderOf; use crate::type_config::alias::SnapshotDataOf; use crate::type_config::TypeConfigExt; -use crate::vote::raft_term::RaftTerm; use crate::vote::RaftLeaderId; +use crate::vote::RaftTerm; use crate::LogId; use crate::LogIdOptionExt; use crate::Membership; diff --git a/openraft/src/engine/handler/following_handler/mod.rs b/openraft/src/engine/handler/following_handler/mod.rs index 1881d328e..9fba6b744 100644 --- a/openraft/src/engine/handler/following_handler/mod.rs +++ b/openraft/src/engine/handler/following_handler/mod.rs @@ -16,7 +16,7 @@ use crate::error::RejectAppendEntries; use crate::raft_state::IOId; use crate::raft_state::LogStateReader; use crate::storage::Snapshot; -use crate::vote::CommittedVote; +use crate::vote::committed::CommittedVote; use crate::EffectiveMembership; use crate::LogId; use crate::LogIdOptionExt; diff --git a/openraft/src/engine/testing.rs b/openraft/src/engine/testing.rs index 8de27b978..fab5b3d75 100644 --- a/openraft/src/engine/testing.rs +++ b/openraft/src/engine/testing.rs @@ -37,7 +37,7 @@ where N: Node + Ord type Node = N; type Entry = crate::Entry; type Term = u64; - type LeaderId = crate::impls::LeaderId; + type LeaderId = crate::impls::leader_id_adv::LeaderId; type SnapshotData = Cursor>; type AsyncRuntime = TokioRuntime; type Responder = crate::impls::OneshotResponder; diff --git a/openraft/src/impls/mod.rs b/openraft/src/impls/mod.rs index b446525e2..950a756de 100644 --- a/openraft/src/impls/mod.rs +++ b/openraft/src/impls/mod.rs @@ -7,15 +7,12 @@ pub use crate::raft::responder::impls::OneshotResponder; #[cfg(feature = "tokio-rt")] pub use crate::type_config::async_runtime::tokio_impls::TokioRuntime; -#[cfg(not(feature = "single-term-leader"))] -pub mod leader_id { - pub use crate::vote::leader_id::leader_id_adv::CommittedLeaderId; +/// LeaderId implementation for advanced mode, allowing multiple leaders per term. +pub mod leader_id_adv { pub use crate::vote::leader_id::leader_id_adv::LeaderId; } -#[cfg(feature = "single-term-leader")] -pub mod leader_id { - pub use crate::vote::leader_id::leader_id_std::CommittedLeaderId; + +/// LeaderId implementation for standard Raft mode, enforcing single leader per term. +pub mod leader_id_std { pub use crate::vote::leader_id::leader_id_std::LeaderId; } - -pub use leader_id::LeaderId; diff --git a/openraft/src/lib.rs b/openraft/src/lib.rs index 3729cafb6..b3134a076 100644 --- a/openraft/src/lib.rs +++ b/openraft/src/lib.rs @@ -132,8 +132,6 @@ pub use crate::try_as_ref::TryAsRef; #[cfg(feature = "type-alias")] pub use crate::type_config::alias; pub use crate::type_config::RaftTypeConfig; -pub use crate::vote::CommittedLeaderId; -pub use crate::vote::LeaderId; pub use crate::vote::Vote; /// A trait defining application specific data. diff --git a/openraft/src/proposer/leader.rs b/openraft/src/proposer/leader.rs index 78bbc618a..f9113b674 100644 --- a/openraft/src/proposer/leader.rs +++ b/openraft/src/proposer/leader.rs @@ -9,7 +9,7 @@ use crate::quorum::QuorumSet; use crate::type_config::alias::InstantOf; use crate::type_config::alias::LogIdOf; use crate::type_config::TypeConfigExt; -use crate::vote::CommittedVote; +use crate::vote::committed::CommittedVote; use crate::vote::RaftLeaderId; use crate::LogId; use crate::LogIdOptionExt; diff --git a/openraft/src/raft/declare_raft_types_test.rs b/openraft/src/raft/declare_raft_types_test.rs index 99e9e5bf3..aeece573e 100644 --- a/openraft/src/raft/declare_raft_types_test.rs +++ b/openraft/src/raft/declare_raft_types_test.rs @@ -17,6 +17,8 @@ declare_raft_types!( #[allow(dead_code)] #[allow(dead_code)] R = (), + Term = u64, + LeaderId = crate::impls::leader_id_std::LeaderId, Entry = crate::Entry, SnapshotData = Cursor>, AsyncRuntime = TokioRuntime, diff --git a/openraft/src/raft/mod.rs b/openraft/src/raft/mod.rs index 2cb748a3d..061d3f79a 100644 --- a/openraft/src/raft/mod.rs +++ b/openraft/src/raft/mod.rs @@ -115,7 +115,7 @@ use crate::Vote; /// NodeId = u64, /// Node = openraft::BasicNode, /// Term = u64, -/// LeaderId = openraft::impls::LeaderId, +/// LeaderId = openraft::impls::leader_id_adv::LeaderId, /// Entry = openraft::Entry, /// SnapshotData = Cursor>, /// Responder = openraft::impls::OneshotResponder, @@ -128,6 +128,8 @@ use crate::Vote; /// - `R`: `String` /// - `NodeId`: `u64` /// - `Node`: `::openraft::impls::BasicNode` +/// - `Term`: `u64` +/// - `LeaderId`: `::openraft::impls::leader_id_adv::LeaderId` /// - `Entry`: `::openraft::impls::Entry` /// - `SnapshotData`: `Cursor>` /// - `Responder`: `::openraft::impls::OneshotResponder` @@ -170,16 +172,16 @@ macro_rules! declare_raft_types { $(($type_id, $(#[$inner])*, $type),)* // Default types: - (D , , String ), - (R , , String ), - (NodeId , , u64 ), - (Node , , $crate::impls::BasicNode ), - (Term , , u64 ), - (LeaderId , , $crate::impls::LeaderId ), - (Entry , , $crate::impls::Entry ), - (SnapshotData , , std::io::Cursor> ), - (Responder , , $crate::impls::OneshotResponder ), - (AsyncRuntime , , $crate::impls::TokioRuntime ), + (D , , String ), + (R , , String ), + (NodeId , , u64 ), + (Node , , $crate::impls::BasicNode ), + (Term , , u64 ), + (LeaderId , , $crate::impls::leader_id_adv::LeaderId ), + (Entry , , $crate::impls::Entry ), + (SnapshotData , , std::io::Cursor> ), + (Responder , , $crate::impls::OneshotResponder ), + (AsyncRuntime , , $crate::impls::TokioRuntime ), ); } diff --git a/openraft/src/raft_state/io_state/io_id.rs b/openraft/src/raft_state/io_state/io_id.rs index 0e59d03d4..cfaf840b9 100644 --- a/openraft/src/raft_state/io_state/io_id.rs +++ b/openraft/src/raft_state/io_state/io_id.rs @@ -2,9 +2,9 @@ use std::cmp::Ordering; use std::fmt; use crate::raft_state::io_state::log_io_id::LogIOId; +use crate::vote::committed::CommittedVote; +use crate::vote::non_committed::NonCommittedVote; use crate::vote::ref_vote::RefVote; -use crate::vote::CommittedVote; -use crate::vote::NonCommittedVote; use crate::ErrorSubject; use crate::ErrorVerb; use crate::LogId; diff --git a/openraft/src/raft_state/io_state/log_io_id.rs b/openraft/src/raft_state/io_state/log_io_id.rs index 45e9f7679..66b1159ba 100644 --- a/openraft/src/raft_state/io_state/log_io_id.rs +++ b/openraft/src/raft_state/io_state/log_io_id.rs @@ -1,7 +1,7 @@ use std::fmt; use crate::display_ext::DisplayOptionExt; -use crate::vote::CommittedVote; +use crate::vote::committed::CommittedVote; use crate::LogId; use crate::RaftTypeConfig; diff --git a/openraft/src/replication/replication_session_id.rs b/openraft/src/replication/replication_session_id.rs index 355a5030c..21b3edf8f 100644 --- a/openraft/src/replication/replication_session_id.rs +++ b/openraft/src/replication/replication_session_id.rs @@ -2,7 +2,7 @@ use std::fmt::Display; use std::fmt::Formatter; use crate::display_ext::DisplayOptionExt; -use crate::vote::CommittedVote; +use crate::vote::committed::CommittedVote; use crate::LogId; use crate::RaftTypeConfig; use crate::Vote; diff --git a/openraft/src/replication/response.rs b/openraft/src/replication/response.rs index 106457486..132639e49 100644 --- a/openraft/src/replication/response.rs +++ b/openraft/src/replication/response.rs @@ -76,8 +76,6 @@ mod tests { #[test] fn test_replication_result_display() { - // NOTE that with single-term-leader, log id is `1-3` - let result = ReplicationResult::(Ok(Some(log_id(1, 2, 3)))); let want = format!("(Match:{})", log_id::(1, 2, 3)); assert!(result.to_string().ends_with(&want), "{}", result.to_string()); diff --git a/openraft/src/summary.rs b/openraft/src/summary.rs index 28dd9923d..3f624e363 100644 --- a/openraft/src/summary.rs +++ b/openraft/src/summary.rs @@ -79,8 +79,6 @@ where T: MessageSummary #[cfg(test)] mod tests { - // With `single-term-leader`, log id is a two-element tuple - #[cfg(not(feature = "single-term-leader"))] #[test] fn test_display() { use crate::engine::testing::UTConfig; diff --git a/openraft/src/type_config.rs b/openraft/src/type_config.rs index c6152ec73..00e5ce3f9 100644 --- a/openraft/src/type_config.rs +++ b/openraft/src/type_config.rs @@ -16,8 +16,8 @@ pub use util::TypeConfigExt; use crate::entry::FromAppData; use crate::entry::RaftEntry; use crate::raft::responder::Responder; -use crate::vote::raft_term::RaftTerm; use crate::vote::RaftLeaderId; +use crate::vote::RaftTerm; use crate::AppData; use crate::AppDataResponse; use crate::Node; @@ -46,8 +46,8 @@ use crate::OptionalSync; /// NodeId = u64, /// Node = openraft::BasicNode, /// Term = u64, -/// LeaderId = openraft::impls::LeaderId, -/// Entry = openraft::Entry, +/// LeaderId = openraft::impls::leader_id_adv::LeaderId, +/// Entry = openraft::impls::Entry, /// SnapshotData = Cursor>, /// AsyncRuntime = openraft::TokioRuntime, /// ); diff --git a/openraft/src/vote/leader_id/leader_id_adv.rs b/openraft/src/vote/leader_id/leader_id_adv.rs index 5b2426b42..3ad7c3b5d 100644 --- a/openraft/src/vote/leader_id/leader_id_adv.rs +++ b/openraft/src/vote/leader_id/leader_id_adv.rs @@ -4,15 +4,11 @@ use crate::vote::RaftCommittedLeaderId; use crate::vote::RaftLeaderId; use crate::RaftTypeConfig; -/// LeaderId is identifier of a `leader`. +/// ID of a `leader`, allowing multiple leaders per term. /// -/// In raft spec that in a term there is at most one leader, thus a `term` is enough to -/// differentiate leaders. That is why raft uses `(term, index)` to uniquely identify a log -/// entry. +/// It includes the `term` and the `node_id`. /// -/// Under this dirty simplification, a `Leader` is actually identified by `(term, -/// voted_for:Option)`. -/// By introducing `LeaderId {term, node_id}`, things become easier to understand. +/// This is totally ordered to enable multiple leaders per term. #[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] #[derive(PartialOrd, Ord)] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize), serde(bound = ""))] @@ -71,9 +67,9 @@ impl RaftCommittedLeaderId for LeaderId where C: RaftTypeConfig {} #[cfg(test)] mod tests { + use super::LeaderId; use crate::engine::testing::UTConfig; use crate::vote::RaftLeaderId; - use crate::LeaderId; #[cfg(feature = "serde")] #[test] diff --git a/openraft/src/vote/leader_id/leader_id_std.rs b/openraft/src/vote/leader_id/leader_id_std.rs index e3cbfaf4f..9ed779856 100644 --- a/openraft/src/vote/leader_id/leader_id_std.rs +++ b/openraft/src/vote/leader_id/leader_id_std.rs @@ -7,6 +7,12 @@ use crate::vote::RaftCommittedLeaderId; use crate::vote::RaftLeaderId; use crate::RaftTypeConfig; +/// ID of a `leader`, enforcing single leader per term. +/// +/// It includes the `term` and the `node_id`. +/// +/// Raft specifies that in a term there is at most one leader, thus Leader ID is partially order as +/// defined below. #[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize), serde(bound = ""))] pub struct LeaderId @@ -76,8 +82,20 @@ where C: RaftTypeConfig } } +/// The unique identifier of a leader that is already granted by a quorum in phase-1(voting). +/// +/// [`CommittedLeaderId`] contain less information than [`LeaderId`], because it implies the +/// constraint that **a quorum has granted it**. +/// +/// For a partial order `LeaderId`, we know that all the committed leader-id must be a total order +/// set. Therefor once it is granted by a quorum, it only keeps the information that makes +/// leader-ids a correct total order set, e.g., in standard raft, `voted_for: Option` can +/// be removed from `(term, voted_for)` once it is granted. This is why standard Raft stores just a +/// `term` in log entry to identify the Leader proposing the log entry. #[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] #[derive(PartialOrd, Ord)] +#[derive(derive_more::Display)] +#[display("{}", term)] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize), serde(bound = ""))] #[cfg_attr(feature = "serde", serde(transparent))] pub struct CommittedLeaderId @@ -87,14 +105,6 @@ where C: RaftTypeConfig p: PhantomData, } -impl fmt::Display for CommittedLeaderId -where C: RaftTypeConfig -{ - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.term) - } -} - impl CommittedLeaderId where C: RaftTypeConfig { @@ -109,13 +119,14 @@ impl RaftCommittedLeaderId for CommittedLeaderId where C: RaftTypeConfi #[cfg(test)] #[allow(clippy::nonminimal_bool)] mod tests { + use super::LeaderId; use crate::engine::testing::UTConfig; - use crate::LeaderId; + use crate::vote::RaftLeaderId; #[cfg(feature = "serde")] #[test] fn test_committed_leader_id_serde() -> anyhow::Result<()> { - use crate::CommittedLeaderId; + use super::CommittedLeaderId; let c = CommittedLeaderId::::new(5, 10); let s = serde_json::to_string(&c)?; diff --git a/openraft/src/vote/leader_id/mod.rs b/openraft/src/vote/leader_id/mod.rs index 10b07d698..c6e6e3999 100644 --- a/openraft/src/vote/leader_id/mod.rs +++ b/openraft/src/vote/leader_id/mod.rs @@ -1,16 +1,13 @@ -#[cfg(not(feature = "single-term-leader"))] -pub(crate) mod leader_id_adv; -#[cfg(feature = "single-term-leader")] -pub(crate) mod leader_id_std; - -#[cfg(not(feature = "single-term-leader"))] -pub use leader_id_adv::CommittedLeaderId; -#[cfg(not(feature = "single-term-leader"))] -pub use leader_id_adv::LeaderId; -#[cfg(feature = "single-term-leader")] -pub use leader_id_std::CommittedLeaderId; -#[cfg(feature = "single-term-leader")] -pub use leader_id_std::LeaderId; +pub mod leader_id_adv; +pub mod leader_id_std; pub(crate) mod raft_committed_leader_id; pub(crate) mod raft_leader_id; + +#[cfg(feature = "single-term-leader")] +compile_error!( + r#"`single-term-leader` is removed. +To enable standard Raft mode: +- either add `LeaderId = openraft::impls::leader_id_std::LeaderId` to `declare_raft_types!(YourTypeConfig)` statement, +- or add `type LeaderId: opernaft::impls::leader_id_std::LeaderId` to the `RaftTypeConfig` implementation."# +); diff --git a/openraft/src/vote/leader_id/raft_leader_id.rs b/openraft/src/vote/leader_id/raft_leader_id.rs index 6bc8c6f93..e0a3ad55a 100644 --- a/openraft/src/vote/leader_id/raft_leader_id.rs +++ b/openraft/src/vote/leader_id/raft_leader_id.rs @@ -46,6 +46,9 @@ where fn to_committed(&self) -> Self::Committed; } +/// Extension methods for [`RaftLeaderId`]. +/// +/// This trait is implemented for all types that implement [`RaftLeaderId`]. pub trait RaftLeaderIdExt where C: RaftTypeConfig, diff --git a/openraft/src/vote/mod.rs b/openraft/src/vote/mod.rs index c9210792e..da5e1619c 100644 --- a/openraft/src/vote/mod.rs +++ b/openraft/src/vote/mod.rs @@ -1,3 +1,5 @@ +//! Defines election related types. + pub(crate) mod committed; pub(crate) mod leader_id; pub(crate) mod non_committed; @@ -6,13 +8,13 @@ pub(crate) mod ref_vote; mod vote; pub(crate) mod vote_status; -pub(crate) use committed::CommittedVote; +pub(crate) mod raft_term; + pub use leader_id::raft_committed_leader_id::RaftCommittedLeaderId; pub use leader_id::raft_leader_id::RaftLeaderId; pub use leader_id::raft_leader_id::RaftLeaderIdExt; -pub use leader_id::CommittedLeaderId; -pub use leader_id::LeaderId; -pub(crate) use non_committed::NonCommittedVote; -pub use vote::Vote; +pub use raft_term::RaftTerm; -pub mod raft_term; +pub use self::leader_id::leader_id_adv; +pub use self::leader_id::leader_id_std; +pub use self::vote::Vote; diff --git a/openraft/src/vote/raft_term/raft_term_impls.rs b/openraft/src/vote/raft_term/raft_term_impls.rs index bb2746a92..c44712058 100644 --- a/openraft/src/vote/raft_term/raft_term_impls.rs +++ b/openraft/src/vote/raft_term/raft_term_impls.rs @@ -1,4 +1,4 @@ -use crate::vote::raft_term::RaftTerm; +use crate::vote::RaftTerm; macro_rules! impl_raft_term { ($($t:ty),*) => { diff --git a/openraft/src/vote/vote.rs b/openraft/src/vote/vote.rs index 1f5e08e5b..37a667e1f 100644 --- a/openraft/src/vote/vote.rs +++ b/openraft/src/vote/vote.rs @@ -3,9 +3,9 @@ use std::fmt::Formatter; use crate::type_config::alias::CommittedLeaderIdOf; use crate::vote::committed::CommittedVote; +use crate::vote::non_committed::NonCommittedVote; use crate::vote::ref_vote::RefVote; use crate::vote::vote_status::VoteStatus; -use crate::vote::NonCommittedVote; use crate::vote::RaftLeaderId; use crate::RaftTypeConfig; @@ -111,7 +111,6 @@ where C: RaftTypeConfig #[cfg(test)] #[allow(clippy::nonminimal_bool)] mod tests { - #[cfg(not(feature = "single-term-leader"))] mod feature_no_single_term_leader { use crate::engine::testing::UTConfig; use crate::Vote; @@ -156,14 +155,15 @@ mod tests { } } - #[cfg(feature = "single-term-leader")] mod feature_single_term_leader { use std::panic::UnwindSafe; - use crate::engine::testing::UTConfig; - use crate::LeaderId; + use crate::declare_raft_types; + use crate::vote::leader_id_std::LeaderId; use crate::Vote; + declare_raft_types!(TC: D=(),R=(),LeaderId=LeaderId); + #[cfg(feature = "serde")] #[test] fn test_vote_serde() -> anyhow::Result<()> { @@ -171,7 +171,7 @@ mod tests { let s = serde_json::to_string(&v)?; assert_eq!(r#"{"leader_id":{"term":1,"voted_for":2},"committed":false}"#, s); - let v2: Vote = serde_json::from_str(&s)?; + let v2: Vote = serde_json::from_str(&s)?; assert_eq!(v, v2); Ok(()) @@ -181,15 +181,15 @@ mod tests { #[allow(clippy::neg_cmp_op_on_partial_ord)] fn test_vote_partial_order() -> anyhow::Result<()> { #[allow(clippy::redundant_closure)] - let vote = |term, node_id| Vote::::new(term, node_id); + let vote = |term, node_id| Vote::::new(term, node_id); - let none = |term| Vote:: { + let none = |term| Vote:: { leader_id: LeaderId { term, voted_for: None }, committed: false, }; #[allow(clippy::redundant_closure)] - let committed = |term, node_id| Vote::::new_committed(term, node_id); + let committed = |term, node_id| Vote::::new_committed(term, node_id); // Compare term first assert!(vote(2, 2) > vote(1, 2)); diff --git a/openraft/src/vote/vote_status.rs b/openraft/src/vote/vote_status.rs index 3b620ff0e..e2bf7556b 100644 --- a/openraft/src/vote/vote_status.rs +++ b/openraft/src/vote/vote_status.rs @@ -1,5 +1,5 @@ -use crate::vote::CommittedVote; -use crate::vote::NonCommittedVote; +use crate::vote::committed::CommittedVote; +use crate::vote::non_committed::NonCommittedVote; use crate::RaftTypeConfig; #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/stores/memstore/Cargo.toml b/stores/memstore/Cargo.toml index c3041f5ac..5862d9488 100644 --- a/stores/memstore/Cargo.toml +++ b/stores/memstore/Cargo.toml @@ -25,6 +25,7 @@ tracing = { workspace = true } [features] bt = ["openraft/bt"] +single-term-leader = [] [package.metadata.docs.rs] all-features = true diff --git a/stores/memstore/src/lib.rs b/stores/memstore/src/lib.rs index 9ba350af3..870d5aa8f 100644 --- a/stores/memstore/src/lib.rs +++ b/stores/memstore/src/lib.rs @@ -75,12 +75,21 @@ pub struct ClientResponse(pub Option); pub type MemNodeId = u64; +/// Choose a LeaderId implementation by feature flag. +mod leader_id_mode { + #[cfg(not(feature = "single-term-leader"))] + pub use openraft::impls::leader_id_adv::LeaderId; + #[cfg(feature = "single-term-leader")] + pub use openraft::impls::leader_id_std::LeaderId; +} + openraft::declare_raft_types!( /// Declare the type configuration for `MemStore`. pub TypeConfig: D = ClientRequest, R = ClientResponse, Node = (), + LeaderId = leader_id_mode::LeaderId, ); /// The application snapshot type which the `MemStore` works with. diff --git a/tests/Cargo.toml b/tests/Cargo.toml index 7ecef62c2..815ab1bfd 100644 --- a/tests/Cargo.toml +++ b/tests/Cargo.toml @@ -40,4 +40,4 @@ tracing-subscriber = { workspace = true } [features] bt = ["openraft/bt"] -single-term-leader = ["openraft/single-term-leader"] +single-term-leader = ["openraft-memstore/single-term-leader"] From 959a6631feee56556a064ae0f675204fadb6302c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E7=82=8E=E6=B3=BC?= Date: Tue, 31 Dec 2024 09:40:44 +0800 Subject: [PATCH 3/4] Refactor: update gRPC example to adapt to `LeaderId` changes - Update the `raft-kv-memstore-grpc` example to use the protobuf-defined `LeaderId`. - Automatically implement `CommittedLeaderId` for all types. - Add `openraft::vote::LeaderIdCompare` to provide comparison functions for both single-leader-per-term and multi-leader-per-term implementations. --- examples/raft-kv-memstore-grpc/build.rs | 7 + .../proto/internal_service.proto | 4 +- .../src/grpc/management_service.rs | 6 +- examples/raft-kv-memstore-grpc/src/lib.rs | 79 +++++------ .../raft-kv-memstore-grpc/src/network/mod.rs | 4 +- .../src/pb_impl/impl_leader_id.rs | 51 ++++++++ .../raft-kv-memstore-grpc/src/pb_impl/mod.rs | 3 + examples/utils/declare_types.rs | 2 + .../src/docs/feature_flags/feature-flags.md | 2 +- openraft/src/vote/leader_id/leader_id_adv.rs | 9 +- openraft/src/vote/leader_id/leader_id_cmp.rs | 123 ++++++++++++++++++ openraft/src/vote/leader_id/leader_id_std.rs | 28 +--- openraft/src/vote/leader_id/mod.rs | 1 + .../leader_id/raft_committed_leader_id.rs | 7 + openraft/src/vote/mod.rs | 1 + 15 files changed, 245 insertions(+), 82 deletions(-) create mode 100644 examples/raft-kv-memstore-grpc/src/pb_impl/impl_leader_id.rs create mode 100644 examples/raft-kv-memstore-grpc/src/pb_impl/mod.rs create mode 100644 openraft/src/vote/leader_id/leader_id_cmp.rs diff --git a/examples/raft-kv-memstore-grpc/build.rs b/examples/raft-kv-memstore-grpc/build.rs index e93e467ed..dcaa873a4 100644 --- a/examples/raft-kv-memstore-grpc/build.rs +++ b/examples/raft-kv-memstore-grpc/build.rs @@ -7,6 +7,9 @@ fn main() -> Result<(), Box> { "proto/management_service.proto", "proto/api_service.proto", ]; + + // TODO: remove serde + tonic_build::configure() .type_attribute("openraftpb.Node", "#[derive(Eq, serde::Serialize, serde::Deserialize)]") .type_attribute( @@ -17,6 +20,10 @@ fn main() -> Result<(), Box> { "openraftpb.Response", "#[derive(Eq, serde::Serialize, serde::Deserialize)]", ) + .type_attribute( + "openraftpb.LeaderId", + "#[derive(Eq, serde::Serialize, serde::Deserialize)]", + ) .compile_protos_with_config(config, &proto_files, &["proto"])?; Ok(()) } diff --git a/examples/raft-kv-memstore-grpc/proto/internal_service.proto b/examples/raft-kv-memstore-grpc/proto/internal_service.proto index 181b447de..9414a7c16 100644 --- a/examples/raft-kv-memstore-grpc/proto/internal_service.proto +++ b/examples/raft-kv-memstore-grpc/proto/internal_service.proto @@ -15,8 +15,8 @@ message Vote { // LogId represents the log identifier in Raft message LogId { - uint64 index = 1; - LeaderId leader_id = 2; + uint64 term = 1; + uint64 index = 2; } // VoteRequest represents a request for votes during leader election diff --git a/examples/raft-kv-memstore-grpc/src/grpc/management_service.rs b/examples/raft-kv-memstore-grpc/src/grpc/management_service.rs index efb4bc757..fa054d692 100644 --- a/examples/raft-kv-memstore-grpc/src/grpc/management_service.rs +++ b/examples/raft-kv-memstore-grpc/src/grpc/management_service.rs @@ -5,6 +5,7 @@ use tonic::Response; use tonic::Status; use tracing::debug; +use crate::pb; use crate::protobuf::management_service_server::ManagementService; use crate::protobuf::AddLearnerRequest; use crate::protobuf::ChangeMembershipRequest; @@ -12,7 +13,6 @@ use crate::protobuf::InitRequest; use crate::protobuf::RaftReplyString; use crate::protobuf::RaftRequestString; use crate::typ::*; -use crate::Node; /// Management service implementation for Raft cluster administration. /// Handles cluster initialization, membership changes, and metrics collection. @@ -62,11 +62,11 @@ impl ManagementService for ManagementServiceImpl { let req = request.into_inner(); // Convert nodes into required format - let nodes_map: BTreeMap = req + let nodes_map: BTreeMap = req .nodes .into_iter() .map(|node| { - (node.node_id, Node { + (node.node_id, pb::Node { rpc_addr: node.rpc_addr, node_id: node.node_id, }) diff --git a/examples/raft-kv-memstore-grpc/src/lib.rs b/examples/raft-kv-memstore-grpc/src/lib.rs index cbd41599a..0008f1a4f 100644 --- a/examples/raft-kv-memstore-grpc/src/lib.rs +++ b/examples/raft-kv-memstore-grpc/src/lib.rs @@ -1,8 +1,6 @@ #![allow(clippy::uninlined_format_args)] -use crate::protobuf::Node; -use crate::protobuf::Response; -use crate::protobuf::SetRequest; +use crate::protobuf as pb; use crate::store::StateMachineData; use crate::typ::*; @@ -12,14 +10,17 @@ pub mod store; #[cfg(test)] mod test; +mod pb_impl; + pub type NodeId = u64; openraft::declare_raft_types!( /// Declare the type configuration for example K/V store. pub TypeConfig: - D = SetRequest, - R = Response, - Node = Node, + D = pb::SetRequest, + R = pb::Response, + LeaderId = pb::LeaderId, + Node = pb::Node, SnapshotData = StateMachineData, ); @@ -33,59 +34,43 @@ pub mod protobuf { #[path = "../../utils/declare_types.rs"] pub mod typ; -impl From for LeaderId { - fn from(proto_leader_id: protobuf::LeaderId) -> Self { - LeaderId::new(proto_leader_id.term, proto_leader_id.node_id) - } -} - -impl From for typ::Vote { - fn from(proto_vote: protobuf::Vote) -> Self { - let leader_id: LeaderId = proto_vote.leader_id.unwrap().into(); +impl From for Vote { + fn from(proto_vote: pb::Vote) -> Self { + let leader_id: LeaderId = proto_vote.leader_id.unwrap(); if proto_vote.committed { - typ::Vote::new_committed(leader_id.term, leader_id.node_id) + Vote::new_committed(leader_id.term, leader_id.node_id) } else { - typ::Vote::new(leader_id.term, leader_id.node_id) + Vote::new(leader_id.term, leader_id.node_id) } } } -impl From for LogId { - fn from(proto_log_id: protobuf::LogId) -> Self { - let leader_id: LeaderId = proto_log_id.leader_id.unwrap().into(); - LogId::new(leader_id, proto_log_id.index) +impl From for LogId { + fn from(proto_log_id: pb::LogId) -> Self { + LogId::new(proto_log_id.term, proto_log_id.index) } } -impl From for VoteRequest { - fn from(proto_vote_req: protobuf::VoteRequest) -> Self { - let vote: typ::Vote = proto_vote_req.vote.unwrap().into(); +impl From for VoteRequest { + fn from(proto_vote_req: pb::VoteRequest) -> Self { + let vote: Vote = proto_vote_req.vote.unwrap().into(); let last_log_id = proto_vote_req.last_log_id.map(|log_id| log_id.into()); VoteRequest::new(vote, last_log_id) } } -impl From for VoteResponse { - fn from(proto_vote_resp: protobuf::VoteResponse) -> Self { - let vote: typ::Vote = proto_vote_resp.vote.unwrap().into(); +impl From for VoteResponse { + fn from(proto_vote_resp: pb::VoteResponse) -> Self { + let vote: Vote = proto_vote_resp.vote.unwrap().into(); let last_log_id = proto_vote_resp.last_log_id.map(|log_id| log_id.into()); VoteResponse::new(vote, last_log_id, proto_vote_resp.vote_granted) } } -impl From for protobuf::LeaderId { - fn from(leader_id: LeaderId) -> Self { - protobuf::LeaderId { - term: leader_id.term, - node_id: leader_id.node_id, - } - } -} - -impl From for protobuf::Vote { - fn from(vote: typ::Vote) -> Self { - protobuf::Vote { - leader_id: Some(protobuf::LeaderId { +impl From for pb::Vote { + fn from(vote: Vote) -> Self { + pb::Vote { + leader_id: Some(pb::LeaderId { term: vote.leader_id().term, node_id: vote.leader_id().node_id, }), @@ -93,27 +78,27 @@ impl From for protobuf::Vote { } } } -impl From for protobuf::LogId { +impl From for pb::LogId { fn from(log_id: LogId) -> Self { - protobuf::LogId { + pb::LogId { + term: log_id.leader_id, index: log_id.index, - leader_id: Some(log_id.leader_id.into()), } } } -impl From for protobuf::VoteRequest { +impl From for pb::VoteRequest { fn from(vote_req: VoteRequest) -> Self { - protobuf::VoteRequest { + pb::VoteRequest { vote: Some(vote_req.vote.into()), last_log_id: vote_req.last_log_id.map(|log_id| log_id.into()), } } } -impl From for protobuf::VoteResponse { +impl From for pb::VoteResponse { fn from(vote_resp: VoteResponse) -> Self { - protobuf::VoteResponse { + pb::VoteResponse { vote: Some(vote_resp.vote.into()), vote_granted: vote_resp.vote_granted, last_log_id: vote_resp.last_log_id.map(|log_id| log_id.into()), diff --git a/examples/raft-kv-memstore-grpc/src/network/mod.rs b/examples/raft-kv-memstore-grpc/src/network/mod.rs index dfee11803..b8798951c 100644 --- a/examples/raft-kv-memstore-grpc/src/network/mod.rs +++ b/examples/raft-kv-memstore-grpc/src/network/mod.rs @@ -7,6 +7,7 @@ use openraft::network::RPCOption; use openraft::RaftNetworkFactory; use tonic::transport::Channel; +use crate::protobuf as pb; use crate::protobuf::internal_service_client::InternalServiceClient; use crate::protobuf::RaftRequestBytes; use crate::protobuf::SnapshotRequest; @@ -14,7 +15,6 @@ use crate::protobuf::VoteRequest as PbVoteRequest; use crate::protobuf::VoteResponse as PbVoteResponse; use crate::typ::RPCError; use crate::typ::*; -use crate::Node; use crate::NodeId; use crate::TypeConfig; @@ -38,7 +38,7 @@ impl RaftNetworkFactory for Network { /// Represents an active network connection to a remote Raft node. /// Handles serialization and deserialization of Raft messages over gRPC. pub struct NetworkConnection { - target_node: Node, + target_node: pb::Node, } impl NetworkConnection { diff --git a/examples/raft-kv-memstore-grpc/src/pb_impl/impl_leader_id.rs b/examples/raft-kv-memstore-grpc/src/pb_impl/impl_leader_id.rs new file mode 100644 index 000000000..635006ae5 --- /dev/null +++ b/examples/raft-kv-memstore-grpc/src/pb_impl/impl_leader_id.rs @@ -0,0 +1,51 @@ +//! Implement [`RaftLeaderId`] for protobuf defined LeaderId, so that it can be used in OpenRaft + +use std::cmp::Ordering; +use std::fmt; + +use openraft::vote::LeaderIdCompare; +use openraft::vote::RaftLeaderId; + +use crate::protobuf as pb; +use crate::TypeConfig; + +/// Implements PartialOrd for LeaderId to enforce the standard Raft behavior of at most one leader +/// per term. +/// +/// In standard Raft, each term can have at most one leader. This is enforced by making leader IDs +/// with the same term incomparable (returning None), unless they refer to the same node. +/// +/// This differs from the [`PartialOrd`] default implementation which would allow multiple leaders +/// in the same term by comparing node IDs. +impl PartialOrd for pb::LeaderId { + #[inline] + fn partial_cmp(&self, other: &Self) -> Option { + LeaderIdCompare::std(self, other) + } +} + +impl fmt::Display for pb::LeaderId { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "T{}-N{}", self.term, self.node_id) + } +} + +impl RaftLeaderId for pb::LeaderId { + type Committed = u64; + + fn new(term: u64, node_id: u64) -> Self { + Self { term, node_id } + } + + fn term(&self) -> u64 { + self.term + } + + fn node_id_ref(&self) -> Option<&u64> { + Some(&self.node_id) + } + + fn to_committed(&self) -> Self::Committed { + self.term + } +} diff --git a/examples/raft-kv-memstore-grpc/src/pb_impl/mod.rs b/examples/raft-kv-memstore-grpc/src/pb_impl/mod.rs new file mode 100644 index 000000000..df953a355 --- /dev/null +++ b/examples/raft-kv-memstore-grpc/src/pb_impl/mod.rs @@ -0,0 +1,3 @@ +//! Implements traits for protobuf types + +mod impl_leader_id; diff --git a/examples/utils/declare_types.rs b/examples/utils/declare_types.rs index d7c6ec488..12e91b1c5 100644 --- a/examples/utils/declare_types.rs +++ b/examples/utils/declare_types.rs @@ -10,6 +10,8 @@ pub type Entry = openraft::Entry; pub type EntryPayload = openraft::EntryPayload; pub type StoredMembership = openraft::StoredMembership; +pub type Node = ::Node; + pub type LogState = openraft::storage::LogState; pub type SnapshotMeta = openraft::SnapshotMeta; diff --git a/openraft/src/docs/feature_flags/feature-flags.md b/openraft/src/docs/feature_flags/feature-flags.md index 70eee4e12..f90b5ac56 100644 --- a/openraft/src/docs/feature_flags/feature-flags.md +++ b/openraft/src/docs/feature_flags/feature-flags.md @@ -25,7 +25,7 @@ in storage and network, such as `Vote` or `AppendEntriesRequest`. ## feature-flag `single-term-leader` **This feature flag is removed**. -User [`leader_id_std::LeaderId`] in [`RaftTypeConfig`] instead. +Use [`leader_id_std::LeaderId`] in [`RaftTypeConfig`] instead. Allows only one leader to be elected in each `term`. This is the standard raft policy, which increases election conflict rate diff --git a/openraft/src/vote/leader_id/leader_id_adv.rs b/openraft/src/vote/leader_id/leader_id_adv.rs index 3ad7c3b5d..4ef45b9a1 100644 --- a/openraft/src/vote/leader_id/leader_id_adv.rs +++ b/openraft/src/vote/leader_id/leader_id_adv.rs @@ -1,6 +1,7 @@ +//! [`RaftLeaderId`] implementation that allows multiple leaders per term. + use std::fmt; -use crate::vote::RaftCommittedLeaderId; use crate::vote::RaftLeaderId; use crate::RaftTypeConfig; @@ -42,7 +43,7 @@ where C: RaftTypeConfig pub type CommittedLeaderId = LeaderId; impl RaftLeaderId for LeaderId -where C: RaftTypeConfig +where C: RaftTypeConfig { type Committed = Self; @@ -63,8 +64,6 @@ where C: RaftTypeConfig } } -impl RaftCommittedLeaderId for LeaderId where C: RaftTypeConfig {} - #[cfg(test)] mod tests { use super::LeaderId; @@ -89,7 +88,7 @@ mod tests { } #[test] - fn test_leader_id_partial_order() -> anyhow::Result<()> { + fn test_adv_leader_id_partial_order() -> anyhow::Result<()> { #[allow(clippy::redundant_closure)] let lid = |term, node_id| LeaderId::::new(term, node_id); diff --git a/openraft/src/vote/leader_id/leader_id_cmp.rs b/openraft/src/vote/leader_id/leader_id_cmp.rs new file mode 100644 index 000000000..0b8579c78 --- /dev/null +++ b/openraft/src/vote/leader_id/leader_id_cmp.rs @@ -0,0 +1,123 @@ +use std::cmp::Ordering; +use std::marker::PhantomData; + +use crate::vote::RaftLeaderId; +use crate::RaftTypeConfig; + +/// Provide comparison functions for [`RaftLeaderId`] implementations. +pub struct LeaderIdCompare(PhantomData); + +impl LeaderIdCompare +where C: RaftTypeConfig +{ + /// Implements [`PartialOrd`] for LeaderId to enforce the standard Raft behavior of at most one + /// leader per term. + /// + /// In standard Raft, each term can have at most one leader. This is enforced by making leader + /// IDs with the same term incomparable (returning None), unless they refer to the same + /// node. + pub fn std(a: &LID, b: &LID) -> Option + where LID: RaftLeaderId { + match a.term().cmp(&b.term()) { + Ordering::Equal => match (a.node_id_ref(), b.node_id_ref()) { + (None, None) => Some(Ordering::Equal), + (Some(_), None) => Some(Ordering::Greater), + (None, Some(_)) => Some(Ordering::Less), + (Some(a), Some(b)) => { + if a == b { + Some(Ordering::Equal) + } else { + None + } + } + }, + cmp => Some(cmp), + } + } + + /// Implements [`PartialOrd`] for LeaderId to allow multiple leaders per term. + pub fn adv(a: &LID, b: &LID) -> Option + where LID: RaftLeaderId { + let res = (a.term(), a.node_id_ref()).cmp(&(b.term(), b.node_id_ref())); + Some(res) + } +} + +#[cfg(test)] +mod tests { + use std::cmp::Ordering; + + use crate::engine::testing::UTConfig; + use crate::vote::RaftLeaderId; + + #[derive(Debug, PartialEq, Eq, Default, Clone, PartialOrd, derive_more::Display)] + #[display("T{}-N{:?}", _0, _1)] + #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] + struct LeaderId(u64, Option); + + impl RaftLeaderId for LeaderId { + type Committed = u64; + + fn new(term: u64, node_id: u64) -> Self { + Self(term, Some(node_id)) + } + + fn term(&self) -> u64 { + self.0 + } + + fn node_id_ref(&self) -> Option<&u64> { + self.1.as_ref() + } + + fn to_committed(&self) -> Self::Committed { + self.0 + } + } + + #[test] + fn test_std_cmp() { + use Ordering::*; + + use super::LeaderIdCompare as Cmp; + + let lid = |term, node_id| LeaderId(term, Some(node_id)); + let lid_none = |term| LeaderId(term, None); + + // Compare term first + assert_eq!(Cmp::std(&lid(2, 2), &lid(1, 2)), Some(Greater)); + assert_eq!(Cmp::std(&lid(1, 2), &lid(2, 2)), Some(Less)); + + // Equal term, Some > None + assert_eq!(Cmp::std(&lid(2, 2), &lid_none(2)), Some(Greater)); + assert_eq!(Cmp::std(&lid_none(2), &lid(2, 2)), Some(Less)); + + // Equal + assert_eq!(Cmp::std(&lid(2, 2), &lid(2, 2)), Some(Equal)); + + // Incomparable + assert_eq!(Cmp::std(&lid(2, 2), &lid(2, 1)), None); + assert_eq!(Cmp::std(&lid(2, 1), &lid(2, 2)), None); + assert_eq!(Cmp::std(&lid(2, 2), &lid(2, 3)), None); + } + + #[test] + fn test_adv_cmp() { + use Ordering::*; + + use super::LeaderIdCompare as Cmp; + + let lid = |term, node_id| LeaderId(term, Some(node_id)); + + // Compare term first + assert_eq!(Cmp::adv(&lid(2, 2), &lid(1, 2)), Some(Greater)); + assert_eq!(Cmp::adv(&lid(1, 2), &lid(2, 2)), Some(Less)); + + // Equal term + assert_eq!(Cmp::adv(&lid(2, 2), &lid(2, 1)), Some(Greater)); + assert_eq!(Cmp::adv(&lid(2, 1), &lid(2, 2)), Some(Less)); + + // Equal term, node_id + assert_eq!(Cmp::adv(&lid(2, 2), &lid(2, 2)), Some(Equal)); + } +} diff --git a/openraft/src/vote/leader_id/leader_id_std.rs b/openraft/src/vote/leader_id/leader_id_std.rs index 9ed779856..d8adea127 100644 --- a/openraft/src/vote/leader_id/leader_id_std.rs +++ b/openraft/src/vote/leader_id/leader_id_std.rs @@ -1,9 +1,12 @@ +//! [`RaftLeaderId`] implementation that enforces standard Raft behavior of at most one leader per +//! term. + use std::cmp::Ordering; use std::fmt; use std::marker::PhantomData; use crate::display_ext::DisplayOptionExt; -use crate::vote::RaftCommittedLeaderId; +use crate::vote::LeaderIdCompare; use crate::vote::RaftLeaderId; use crate::RaftTypeConfig; @@ -28,24 +31,7 @@ where C: RaftTypeConfig { #[inline] fn partial_cmp(&self, other: &Self) -> Option { - match PartialOrd::partial_cmp(&self.term, &other.term) { - Some(Ordering::Equal) => { - // - match (&self.voted_for, &other.voted_for) { - (None, None) => Some(Ordering::Equal), - (Some(_), None) => Some(Ordering::Greater), - (None, Some(_)) => Some(Ordering::Less), - (Some(a), Some(b)) => { - if a == b { - Some(Ordering::Equal) - } else { - None - } - } - } - } - cmp => cmp, - } + LeaderIdCompare::::std(self, other) } } @@ -114,8 +100,6 @@ where C: RaftTypeConfig } } -impl RaftCommittedLeaderId for CommittedLeaderId where C: RaftTypeConfig {} - #[cfg(test)] #[allow(clippy::nonminimal_bool)] mod tests { @@ -140,7 +124,7 @@ mod tests { #[test] #[allow(clippy::neg_cmp_op_on_partial_ord)] - fn test_leader_id_partial_order() -> anyhow::Result<()> { + fn test_std_leader_id_partial_order() -> anyhow::Result<()> { #[allow(clippy::redundant_closure)] let lid = |term, node_id| LeaderId::::new(term, node_id); diff --git a/openraft/src/vote/leader_id/mod.rs b/openraft/src/vote/leader_id/mod.rs index c6e6e3999..3dc606183 100644 --- a/openraft/src/vote/leader_id/mod.rs +++ b/openraft/src/vote/leader_id/mod.rs @@ -1,6 +1,7 @@ pub mod leader_id_adv; pub mod leader_id_std; +pub(crate) mod leader_id_cmp; pub(crate) mod raft_committed_leader_id; pub(crate) mod raft_leader_id; diff --git a/openraft/src/vote/leader_id/raft_committed_leader_id.rs b/openraft/src/vote/leader_id/raft_committed_leader_id.rs index d597f861a..3f84f01c4 100644 --- a/openraft/src/vote/leader_id/raft_committed_leader_id.rs +++ b/openraft/src/vote/leader_id/raft_committed_leader_id.rs @@ -43,3 +43,10 @@ where Self: OptionalFeatures + Ord + Clone + Debug + Display + Default + 'static, { } + +impl RaftCommittedLeaderId for T +where + C: RaftTypeConfig, + T: OptionalFeatures + Ord + Clone + Debug + Display + Default + 'static, +{ +} diff --git a/openraft/src/vote/mod.rs b/openraft/src/vote/mod.rs index da5e1619c..0d094f90d 100644 --- a/openraft/src/vote/mod.rs +++ b/openraft/src/vote/mod.rs @@ -16,5 +16,6 @@ pub use leader_id::raft_leader_id::RaftLeaderIdExt; pub use raft_term::RaftTerm; pub use self::leader_id::leader_id_adv; +pub use self::leader_id::leader_id_cmp::LeaderIdCompare; pub use self::leader_id::leader_id_std; pub use self::vote::Vote; From 5335fd8c35943a1bec78ff2fe95e593f99b64580 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E7=82=8E=E6=B3=BC?= Date: Fri, 3 Jan 2025 10:03:12 +0800 Subject: [PATCH 4/4] Refactor: rename `RaftLeaderId::node_id_ref()` to `node_id()` Follow the Rust naming guide: https://rust-lang.github.io/api-guidelines/naming.html#getter-names-follow-rust-convention-c-getter --- .../src/pb_impl/impl_leader_id.rs | 2 +- openraft/src/core/raft_core.rs | 2 +- openraft/src/engine/handler/establish_handler/mod.rs | 2 +- openraft/src/engine/handler/vote_handler/mod.rs | 2 +- openraft/src/proposer/leader.rs | 2 +- openraft/src/raft_state/mod.rs | 4 ++-- openraft/src/replication/mod.rs | 2 +- openraft/src/vote/leader_id/leader_id_adv.rs | 2 +- openraft/src/vote/leader_id/leader_id_cmp.rs | 6 +++--- openraft/src/vote/leader_id/leader_id_std.rs | 2 +- openraft/src/vote/leader_id/raft_leader_id.rs | 6 +++--- tests/tests/fixtures/mod.rs | 10 +++++----- tests/tests/life_cycle/t50_single_follower_restart.rs | 2 +- 13 files changed, 22 insertions(+), 22 deletions(-) diff --git a/examples/raft-kv-memstore-grpc/src/pb_impl/impl_leader_id.rs b/examples/raft-kv-memstore-grpc/src/pb_impl/impl_leader_id.rs index 635006ae5..71a1bb584 100644 --- a/examples/raft-kv-memstore-grpc/src/pb_impl/impl_leader_id.rs +++ b/examples/raft-kv-memstore-grpc/src/pb_impl/impl_leader_id.rs @@ -41,7 +41,7 @@ impl RaftLeaderId for pb::LeaderId { self.term } - fn node_id_ref(&self) -> Option<&u64> { + fn node_id(&self) -> Option<&u64> { Some(&self.node_id) } diff --git a/openraft/src/core/raft_core.rs b/openraft/src/core/raft_core.rs index 05019efa1..72d93ad4d 100644 --- a/openraft/src/core/raft_core.rs +++ b/openraft/src/core/raft_core.rs @@ -727,7 +727,7 @@ where } // Safe unwrap(): vote that is committed has to already have voted for some node. - let id = vote.leader_id().node_id_ref().cloned().unwrap(); + let id = vote.leader_id().node_id().cloned().unwrap(); // TODO: `is_voter()` is slow, maybe cache `current_leader`, // e.g., only update it when membership or vote changes diff --git a/openraft/src/engine/handler/establish_handler/mod.rs b/openraft/src/engine/handler/establish_handler/mod.rs index 3cf3879fb..1123012aa 100644 --- a/openraft/src/engine/handler/establish_handler/mod.rs +++ b/openraft/src/engine/handler/establish_handler/mod.rs @@ -25,7 +25,7 @@ where C: RaftTypeConfig let vote = candidate.vote_ref().clone(); debug_assert_eq!( - vote.leader_id().node_id_ref(), + vote.leader_id().node_id(), Some(&self.config.id), "it can only commit its own vote" ); diff --git a/openraft/src/engine/handler/vote_handler/mod.rs b/openraft/src/engine/handler/vote_handler/mod.rs index b936075cb..f20da67c8 100644 --- a/openraft/src/engine/handler/vote_handler/mod.rs +++ b/openraft/src/engine/handler/vote_handler/mod.rs @@ -221,7 +221,7 @@ where C: RaftTypeConfig /// This node then becomes raft-follower or raft-learner. pub(crate) fn become_following(&mut self) { debug_assert!( - self.state.vote_ref().leader_id().node_id_ref() != Some(&self.config.id) + self.state.vote_ref().leader_id().node_id() != Some(&self.config.id) || !self.state.membership_state.effective().membership().is_voter(&self.config.id), "It must hold: vote is not mine, or I am not a voter(leader just left the cluster)" ); diff --git a/openraft/src/proposer/leader.rs b/openraft/src/proposer/leader.rs index f9113b674..107be2a47 100644 --- a/openraft/src/proposer/leader.rs +++ b/openraft/src/proposer/leader.rs @@ -203,7 +203,7 @@ where // Thus vote.voted_for() is this node. // Safe unwrap: voted_for() is always non-None in Openraft - let node_id = self.committed_vote.clone().into_vote().leader_id().node_id_ref().cloned().unwrap(); + let node_id = self.committed_vote.clone().into_vote().leader_id().node_id().cloned().unwrap(); let now = C::now(); tracing::debug!( diff --git a/openraft/src/raft_state/mod.rs b/openraft/src/raft_state/mod.rs index b234f04ab..0b9fee114 100644 --- a/openraft/src/raft_state/mod.rs +++ b/openraft/src/raft_state/mod.rs @@ -368,7 +368,7 @@ where C: RaftTypeConfig /// /// [Determine Server State]: crate::docs::data::vote#vote-and-membership-define-the-server-state pub(crate) fn is_leading(&self, id: &C::NodeId) -> bool { - self.membership_state.contains(id) && self.vote.leader_id().node_id_ref() == Some(id) + self.membership_state.contains(id) && self.vote.leader_id().node_id() == Some(id) } /// The node is leader @@ -408,7 +408,7 @@ where C: RaftTypeConfig if vote.is_committed() { // Safe unwrap(): vote that is committed has to already have voted for some node. - let id = vote.leader_id().node_id_ref().cloned().unwrap(); + let id = vote.leader_id().node_id().cloned().unwrap(); return self.new_forward_to_leader(id); } diff --git a/openraft/src/replication/mod.rs b/openraft/src/replication/mod.rs index 773a2b545..2a58b50ef 100644 --- a/openraft/src/replication/mod.rs +++ b/openraft/src/replication/mod.rs @@ -443,7 +443,7 @@ where let append_res = res.map_err(|_e| { let to = Timeout { action: RPCTypes::AppendEntries, - id: self.session_id.vote().leader_id().node_id_ref().cloned().unwrap(), + id: self.session_id.vote().leader_id().node_id().cloned().unwrap(), target: self.target.clone(), timeout: the_timeout, }; diff --git a/openraft/src/vote/leader_id/leader_id_adv.rs b/openraft/src/vote/leader_id/leader_id_adv.rs index 4ef45b9a1..e5b695d5f 100644 --- a/openraft/src/vote/leader_id/leader_id_adv.rs +++ b/openraft/src/vote/leader_id/leader_id_adv.rs @@ -55,7 +55,7 @@ where C: RaftTypeConfig self.term } - fn node_id_ref(&self) -> Option<&C::NodeId> { + fn node_id(&self) -> Option<&C::NodeId> { Some(&self.node_id) } diff --git a/openraft/src/vote/leader_id/leader_id_cmp.rs b/openraft/src/vote/leader_id/leader_id_cmp.rs index 0b8579c78..d0063b810 100644 --- a/openraft/src/vote/leader_id/leader_id_cmp.rs +++ b/openraft/src/vote/leader_id/leader_id_cmp.rs @@ -19,7 +19,7 @@ where C: RaftTypeConfig pub fn std(a: &LID, b: &LID) -> Option where LID: RaftLeaderId { match a.term().cmp(&b.term()) { - Ordering::Equal => match (a.node_id_ref(), b.node_id_ref()) { + Ordering::Equal => match (a.node_id(), b.node_id()) { (None, None) => Some(Ordering::Equal), (Some(_), None) => Some(Ordering::Greater), (None, Some(_)) => Some(Ordering::Less), @@ -38,7 +38,7 @@ where C: RaftTypeConfig /// Implements [`PartialOrd`] for LeaderId to allow multiple leaders per term. pub fn adv(a: &LID, b: &LID) -> Option where LID: RaftLeaderId { - let res = (a.term(), a.node_id_ref()).cmp(&(b.term(), b.node_id_ref())); + let res = (a.term(), a.node_id()).cmp(&(b.term(), b.node_id())); Some(res) } } @@ -66,7 +66,7 @@ mod tests { self.0 } - fn node_id_ref(&self) -> Option<&u64> { + fn node_id(&self) -> Option<&u64> { self.1.as_ref() } diff --git a/openraft/src/vote/leader_id/leader_id_std.rs b/openraft/src/vote/leader_id/leader_id_std.rs index d8adea127..67f1df39e 100644 --- a/openraft/src/vote/leader_id/leader_id_std.rs +++ b/openraft/src/vote/leader_id/leader_id_std.rs @@ -59,7 +59,7 @@ where C: RaftTypeConfig self.term } - fn node_id_ref(&self) -> Option<&C::NodeId> { + fn node_id(&self) -> Option<&C::NodeId> { self.voted_for.as_ref() } diff --git a/openraft/src/vote/leader_id/raft_leader_id.rs b/openraft/src/vote/leader_id/raft_leader_id.rs index e0a3ad55a..55974e3ab 100644 --- a/openraft/src/vote/leader_id/raft_leader_id.rs +++ b/openraft/src/vote/leader_id/raft_leader_id.rs @@ -38,7 +38,7 @@ where fn term(&self) -> C::Term; /// Get the node ID of this leader, if one is set - fn node_id_ref(&self) -> Option<&C::NodeId>; + fn node_id(&self) -> Option<&C::NodeId>; /// Convert this leader ID to a committed leader ID. /// @@ -58,8 +58,8 @@ where Self::new(term, node_id).to_committed() } - fn node_id(&self) -> Option { - self.node_id_ref().cloned() + fn to_node_id(&self) -> Option { + self.node_id().cloned() } } diff --git a/tests/tests/fixtures/mod.rs b/tests/tests/fixtures/mod.rs index a823206d2..9c7fce9f4 100644 --- a/tests/tests/fixtures/mod.rs +++ b/tests/tests/fixtures/mod.rs @@ -892,7 +892,7 @@ impl TypedRaftRouter { if let Some(voted_for) = &expect_voted_for { assert_eq!( - vote.leader_id().node_id_ref(), + vote.leader_id().node_id(), Some(voted_for), "expected node {} to have voted for {}, got {:?}", id, @@ -1022,7 +1022,7 @@ impl RaftNetworkV2 for RaftRouterNetwork { mut rpc: AppendEntriesRequest, _option: RPCOption, ) -> Result, RPCError> { - let from_id = rpc.vote.leader_id().node_id_ref().cloned().unwrap(); + let from_id = rpc.vote.leader_id().node_id().cloned().unwrap(); tracing::debug!("append_entries to id={} {}", self.target, rpc); self.owner.count_rpc(RPCTypes::AppendEntries); @@ -1092,7 +1092,7 @@ impl RaftNetworkV2 for RaftRouterNetwork { _cancel: impl Future + OptionalSend + 'static, _option: RPCOption, ) -> Result, StreamingError> { - let from_id = vote.leader_id().node_id().unwrap(); + let from_id = vote.leader_id().to_node_id().unwrap(); self.owner.count_rpc(RPCTypes::InstallSnapshot); self.owner.call_rpc_pre_hook(snapshot.clone(), from_id, self.target)?; @@ -1118,7 +1118,7 @@ impl RaftNetworkV2 for RaftRouterNetwork { rpc: VoteRequest, _option: RPCOption, ) -> Result, RPCError> { - let from_id = rpc.vote.leader_id().node_id().unwrap(); + let from_id = rpc.vote.leader_id().to_node_id().unwrap(); self.owner.count_rpc(RPCTypes::Vote); self.owner.call_rpc_pre_hook(rpc.clone(), from_id, self.target)?; @@ -1143,7 +1143,7 @@ impl RaftNetworkV2 for RaftRouterNetwork { rpc: TransferLeaderRequest, _option: RPCOption, ) -> Result<(), RPCError> { - let from_id = rpc.from_leader().leader_id().node_id().unwrap(); + let from_id = rpc.from_leader().leader_id().to_node_id().unwrap(); self.owner.count_rpc(RPCTypes::TransferLeader); self.owner.call_rpc_pre_hook(rpc.clone(), from_id, self.target)?; diff --git a/tests/tests/life_cycle/t50_single_follower_restart.rs b/tests/tests/life_cycle/t50_single_follower_restart.rs index 2e5d40da5..a96c40ad7 100644 --- a/tests/tests/life_cycle/t50_single_follower_restart.rs +++ b/tests/tests/life_cycle/t50_single_follower_restart.rs @@ -48,7 +48,7 @@ async fn single_follower_restart() -> anyhow::Result<()> { let v = sto.read_vote().await?.unwrap_or_default(); // Set a non-committed vote so that the node restarts as a follower. - sto.save_vote(&Vote::new(v.leader_id.term() + 1, v.leader_id.node_id().unwrap())).await?; + sto.save_vote(&Vote::new(v.leader_id.term() + 1, v.leader_id.to_node_id().unwrap())).await?; tracing::info!(log_index, "--- restart node-0");