From 6938cf852c2f073f83f51ce59ee00ab747ba5601 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 14:37:57 +0800 Subject: [PATCH] Change: change `LeaderId` to `LeaderId` This refactoring moves LeaderId from a per-NodeId type to a per-TypeConfig type, to make it consistent with `RaftTypeConfig` usage across the codebase. - Part of: #1278 Upgrade tip: LeaderId is now parameterized by `RaftTypeConfig` instead of `NodeId` - Change `LeaderId` to `LeaderId where C: RaftTypeConfig`, for example, change `LeaderId` to `LeaderId`. - Change `CommittedLeaderId` to `CommittedLeaderId`. --- openraft/src/log_id/mod.rs | 6 +-- openraft/src/log_id/raft_log_id.rs | 2 +- openraft/src/type_config.rs | 4 +- openraft/src/vote/committed.rs | 2 +- .../src/vote/leader_id/impl_into_leader_id.rs | 2 +- openraft/src/vote/leader_id/leader_id_adv.rs | 37 +++++++------- openraft/src/vote/leader_id/leader_id_std.rs | 48 ++++++++++++------- openraft/src/vote/ref_vote.rs | 4 +- openraft/src/vote/vote.rs | 15 ++++-- 9 files changed, 72 insertions(+), 48 deletions(-) diff --git a/openraft/src/log_id/mod.rs b/openraft/src/log_id/mod.rs index 0f799ac5d..5220a0a53 100644 --- a/openraft/src/log_id/mod.rs +++ b/openraft/src/log_id/mod.rs @@ -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: CommittedLeaderId, /// The index of a log in the storage. /// /// Log index is a consecutive integer. @@ -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: CommittedLeaderId, 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) -> &CommittedLeaderId { &self.leader_id } } diff --git a/openraft/src/log_id/raft_log_id.rs b/openraft/src/log_id/raft_log_id.rs index c5f2a79f3..c2d932a60 100644 --- a/openraft/src/log_id/raft_log_id.rs +++ b/openraft/src/log_id/raft_log_id.rs @@ -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) -> &CommittedLeaderId { self.get_log_id().committed_leader_id() } diff --git a/openraft/src/type_config.rs b/openraft/src/type_config.rs index 69dea33ed..46bf359f2 100644 --- a/openraft/src/type_config.rs +++ b/openraft/src/type_config.rs @@ -153,7 +153,7 @@ 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 LeaderIdOf = crate::LeaderId; + pub type CommittedLeaderIdOf = crate::CommittedLeaderId; pub type SerdeInstantOf = crate::metrics::SerdeInstant>; } diff --git a/openraft/src/vote/committed.rs b/openraft/src/vote/committed.rs index f458645c2..5fd5f7bc6 100644 --- a/openraft/src/vote/committed.rs +++ b/openraft/src/vote/committed.rs @@ -41,7 +41,7 @@ where C: RaftTypeConfig Self { vote } } - pub(crate) fn committed_leader_id(&self) -> CommittedLeaderId { + pub(crate) fn committed_leader_id(&self) -> CommittedLeaderId { 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 index 25b30a7bb..d4389c513 100644 --- a/openraft/src/vote/leader_id/impl_into_leader_id.rs +++ b/openraft/src/vote/leader_id/impl_into_leader_id.rs @@ -2,7 +2,7 @@ use crate::vote::leader_id::CommittedLeaderId; use crate::vote::Vote; use crate::RaftTypeConfig; -impl From> for CommittedLeaderId +impl From> for CommittedLeaderId where C: RaftTypeConfig { fn from(vote: Vote) -> Self { diff --git a/openraft/src/vote/leader_id/leader_id_adv.rs b/openraft/src/vote/leader_id/leader_id_adv.rs index 929f1c8ec..35510c999 100644 --- a/openraft/src/vote/leader_id/leader_id_adv.rs +++ b/openraft/src/vote/leader_id/leader_id_adv.rs @@ -1,6 +1,6 @@ use std::fmt; -use crate::NodeId; +use crate::RaftTypeConfig; /// LeaderId is identifier of a `leader`. /// @@ -14,15 +14,17 @@ use crate::NodeId; #[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] #[derive(PartialOrd, Ord)] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize), serde(bound = ""))] -pub struct LeaderId -where NID: NodeId +pub struct LeaderId +where C: RaftTypeConfig { pub term: u64, - pub node_id: NID, + pub node_id: C::NodeId, } -impl LeaderId { - pub fn new(term: u64, node_id: NID) -> Self { +impl LeaderId +where C: RaftTypeConfig +{ + pub fn new(term: u64, node_id: C::NodeId) -> Self { Self { term, node_id } } @@ -30,24 +32,26 @@ impl LeaderId { self.term } - pub fn voted_for(&self) -> Option { + pub fn voted_for(&self) -> Option { Some(self.node_id.clone()) } #[allow(clippy::wrong_self_convention)] - pub(crate) fn to_committed(&self) -> CommittedLeaderId { + 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 { + pub(crate) fn is_same_as_committed(&self, other: &CommittedLeaderId) -> bool { self == other } } -impl fmt::Display for LeaderId { +impl fmt::Display for LeaderId +where C: RaftTypeConfig +{ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "T{}-N{}", self.term, self.node_id) } @@ -55,20 +59,21 @@ impl fmt::Display for LeaderId { /// The unique identifier of a leader that is already granted by a quorum in phase-1(voting). /// -/// [`CommittedLeaderId`] may contains less information than [`LeaderId`], because it implies the +/// [`CommittedLeaderId`] may contain less information than [`LeaderId`], because it implies the /// constraint that **a quorum has granted it**. /// /// For a total order `LeaderId`, the [`CommittedLeaderId`] is the same. /// -/// For a partial order `LeaderId`, we know that all of the granted +/// For a partial order `LeaderId`, 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, 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. -pub type CommittedLeaderId = LeaderId; +pub type CommittedLeaderId = LeaderId; #[cfg(test)] mod tests { + use crate::engine::testing::UTConfig; use crate::LeaderId; #[cfg(feature = "serde")] @@ -76,11 +81,11 @@ mod tests { fn test_committed_leader_id_serde() -> anyhow::Result<()> { use crate::CommittedLeaderId; - let c = CommittedLeaderId::::new(5, 10); + let c = CommittedLeaderId::::new(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)?; + let c2: CommittedLeaderId = serde_json::from_str(&s)?; assert_eq!(CommittedLeaderId::new(5, 10), c2); Ok(()) @@ -89,7 +94,7 @@ mod tests { #[test] fn test_leader_id_partial_order() -> anyhow::Result<()> { #[allow(clippy::redundant_closure)] - let lid = |term, node_id| LeaderId::::new(term, node_id); + let lid = |term, node_id| LeaderId::::new(term, node_id); // Compare term first assert!(lid(2, 2) > lid(1, 2)); diff --git a/openraft/src/vote/leader_id/leader_id_std.rs b/openraft/src/vote/leader_id/leader_id_std.rs index 832924fe7..71a8b2aa1 100644 --- a/openraft/src/vote/leader_id/leader_id_std.rs +++ b/openraft/src/vote/leader_id/leader_id_std.rs @@ -4,18 +4,21 @@ use std::marker::PhantomData; use crate::display_ext::DisplayOptionExt; use crate::NodeId; +use crate::RaftTypeConfig; #[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize), serde(bound = ""))] -pub struct LeaderId -where NID: NodeId +pub struct LeaderId +where C: RaftTypeConfig { pub term: u64, - pub voted_for: Option, + pub voted_for: Option, } -impl PartialOrd for LeaderId { +impl PartialOrd for LeaderId +where C: RaftTypeConfig +{ #[inline] fn partial_cmp(&self, other: &Self) -> Option { match PartialOrd::partial_cmp(&self.term, &other.term) { @@ -39,8 +42,10 @@ impl PartialOrd for LeaderId { } } -impl LeaderId { - pub fn new(term: u64, node_id: NID) -> Self { +impl LeaderId +where C: RaftTypeConfig +{ + pub fn new(term: u64, node_id: C::NodeId) -> Self { Self { term, voted_for: Some(node_id), @@ -51,24 +56,26 @@ impl LeaderId { self.term } - pub fn voted_for(&self) -> Option { + pub fn voted_for(&self) -> Option { self.voted_for.clone() } #[allow(clippy::wrong_self_convention)] - pub(crate) fn to_committed(&self) -> CommittedLeaderId { - CommittedLeaderId::new(self.term, NID::default()) + pub(crate) fn to_committed(&self) -> CommittedLeaderId { + 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 { + pub(crate) fn is_same_as_committed(&self, other: &CommittedLeaderId) -> bool { self.term == other.term } } -impl fmt::Display for LeaderId { +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()) } @@ -78,19 +85,23 @@ impl fmt::Display for LeaderId { #[derive(PartialOrd, Ord)] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize), serde(bound = ""))] #[cfg_attr(feature = "serde", serde(transparent))] -pub struct CommittedLeaderId { +pub struct CommittedLeaderId { pub term: u64, - p: PhantomData, + p: PhantomData, } -impl fmt::Display for CommittedLeaderId { +impl fmt::Display for CommittedLeaderId +where C: RaftTypeConfig +{ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{}", self.term) } } -impl CommittedLeaderId { - pub fn new(term: u64, node_id: NID) -> Self { +impl CommittedLeaderId +where C: RaftTypeConfig +{ + pub fn new(term: u64, node_id: C::NodeId) -> Self { let _ = node_id; Self { term, p: PhantomData } } @@ -99,6 +110,7 @@ impl CommittedLeaderId { #[cfg(test)] #[allow(clippy::nonminimal_bool)] mod tests { + use crate::engine::testing::UTConfig; use crate::LeaderId; #[cfg(feature = "serde")] @@ -120,9 +132,9 @@ mod tests { #[allow(clippy::neg_cmp_op_on_partial_ord)] fn test_leader_id_partial_order() -> anyhow::Result<()> { #[allow(clippy::redundant_closure)] - let lid = |term, node_id| LeaderId::::new(term, node_id); + let lid = |term, node_id| LeaderId::::new(term, node_id); - let lid_none = |term| LeaderId:: { term, voted_for: None }; + let lid_none = |term| LeaderId:: { term, voted_for: None }; // Compare term first assert!(lid(2, 2) > lid(1, 2)); diff --git a/openraft/src/vote/ref_vote.rs b/openraft/src/vote/ref_vote.rs index 8919d0c86..f81743e4a 100644 --- a/openraft/src/vote/ref_vote.rs +++ b/openraft/src/vote/ref_vote.rs @@ -11,14 +11,14 @@ use crate::RaftTypeConfig; pub(crate) struct RefVote<'a, C> where C: RaftTypeConfig { - pub(crate) leader_id: &'a LeaderId, + pub(crate) leader_id: &'a 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 LeaderId, committed: bool) -> Self { Self { leader_id, committed } } diff --git a/openraft/src/vote/vote.rs b/openraft/src/vote/vote.rs index 18f519220..9af5d371f 100644 --- a/openraft/src/vote/vote.rs +++ b/openraft/src/vote/vote.rs @@ -10,15 +10,22 @@ use crate::LeaderId; use crate::RaftTypeConfig; /// `Vote` represent the privilege of a node. -#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +#[derive(Debug, Clone, Default, PartialEq, Eq)] #[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: LeaderId, pub committed: bool, } +impl Copy for Vote +where + C: RaftTypeConfig, + C::NodeId: Copy, +{ +} + impl PartialOrd for Vote where C: RaftTypeConfig { @@ -91,11 +98,11 @@ where C: RaftTypeConfig /// 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) -> &LeaderId { &self.leader_id } - pub(crate) fn is_same_leader(&self, leader_id: &CommittedLeaderId) -> bool { + pub(crate) fn is_same_leader(&self, leader_id: &CommittedLeaderId) -> bool { self.leader_id().is_same_as_committed(leader_id) } }