From 427ac06572d31e20e67b33edfcf1f5a2ac23661e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E7=82=8E=E6=B3=BC?= Date: Mon, 27 Jan 2025 16:07:21 +0800 Subject: [PATCH] Refactor: remove `OrdBy` Just use `as_ref_vote()` to build a `RefVote` for comparing `Vote`. --- openraft/src/base/mod.rs | 2 -- openraft/src/base/ord_by.rs | 36 ------------------- openraft/src/engine/engine_impl.rs | 3 +- .../engine/handler/establish_handler/mod.rs | 3 +- .../src/engine/handler/vote_handler/mod.rs | 5 ++- openraft/src/metrics/metric.rs | 4 +-- openraft/src/network/snapshot_transport.rs | 4 +-- openraft/src/raft/mod.rs | 6 ++-- openraft/src/raft_state/mod.rs | 3 +- openraft/src/replication/mod.rs | 5 ++- openraft/src/vote/raft_vote.rs | 15 -------- 11 files changed, 14 insertions(+), 72 deletions(-) delete mode 100644 openraft/src/base/ord_by.rs diff --git a/openraft/src/base/mod.rs b/openraft/src/base/mod.rs index 6ced31a13..43d621ab8 100644 --- a/openraft/src/base/mod.rs +++ b/openraft/src/base/mod.rs @@ -1,7 +1,5 @@ //! Basic types used in the Raft implementation. -pub(crate) mod ord_by; - pub use serde_able::OptionalSerde; pub use threaded::BoxAny; pub use threaded::BoxAsyncOnceMut; diff --git a/openraft/src/base/ord_by.rs b/openraft/src/base/ord_by.rs deleted file mode 100644 index a7b35e91f..000000000 --- a/openraft/src/base/ord_by.rs +++ /dev/null @@ -1,36 +0,0 @@ -/// A trait for types whose order can be determined by a key. -/// -/// Types implementing this trait define how they should be compared by providing a key -/// that implements [`PartialOrd`]. -/// -/// OpenRaft uses this trait to compare types that may not be [`PartialOrd`] themselves. -/// -/// # Type Parameters -/// - `Key<'k>`: The type of the comparison key, which must be partially ordered and must not out -/// live the value. -/// -/// # Examples -/// ```rust,ignore -/// # use openraft::base::ord_by::OrdBy; -/// -/// struct Person { -/// name: String, -/// age: u32, -/// } -/// -/// impl OrdBy<()> for Person { -/// type By<'k> = &'k str; -/// -/// fn ord_by(&self) -> Self::By<'_> { -/// &self.name -/// } -/// } -/// ``` -pub(crate) trait OrdBy { - /// The key type used for comparison. - type By<'k>: PartialOrd + 'k - where Self: 'k; - - /// Returns the key used for comparing this value. - fn ord_by(&self) -> Self::By<'_>; -} diff --git a/openraft/src/engine/engine_impl.rs b/openraft/src/engine/engine_impl.rs index dd3469b43..8f7a79db1 100644 --- a/openraft/src/engine/engine_impl.rs +++ b/openraft/src/engine/engine_impl.rs @@ -2,7 +2,6 @@ use std::time::Duration; use validit::Valid; -use crate::base::ord_by::OrdBy; use crate::core::raft_msg::AppendEntriesTx; use crate::core::raft_msg::ResultSender; use crate::core::sm; @@ -755,7 +754,7 @@ where C: RaftTypeConfig }; debug_assert!( - leader.committed_vote_ref().ord_by() >= self.state.vote_ref().ord_by(), + leader.committed_vote_ref().as_ref_vote() >= self.state.vote_ref().as_ref_vote(), "leader.vote({}) >= state.vote({})", leader.committed_vote_ref(), self.state.vote_ref() diff --git a/openraft/src/engine/handler/establish_handler/mod.rs b/openraft/src/engine/handler/establish_handler/mod.rs index d66b8b369..75f4a120f 100644 --- a/openraft/src/engine/handler/establish_handler/mod.rs +++ b/openraft/src/engine/handler/establish_handler/mod.rs @@ -1,4 +1,3 @@ -use crate::base::ord_by::OrdBy; use crate::engine::EngineConfig; use crate::proposer::Candidate; use crate::proposer::Leader; @@ -33,7 +32,7 @@ where C: RaftTypeConfig if let Some(l) = self.leader.as_ref() { #[allow(clippy::neg_cmp_op_on_partial_ord)] - if !(vote.ord_by() > l.committed_vote_ref().ord_by()) { + if !(vote.as_ref_vote() > l.committed_vote_ref().as_ref_vote()) { tracing::warn!( "vote is not greater than current existing leader vote. Do not establish new leader and quit" ); diff --git a/openraft/src/engine/handler/vote_handler/mod.rs b/openraft/src/engine/handler/vote_handler/mod.rs index e5830c78d..8fcc09fea 100644 --- a/openraft/src/engine/handler/vote_handler/mod.rs +++ b/openraft/src/engine/handler/vote_handler/mod.rs @@ -1,7 +1,6 @@ use std::fmt::Debug; use std::time::Duration; -use crate::base::ord_by::OrdBy; use crate::core::raft_msg::ResultSender; use crate::engine::handler::leader_handler::LeaderHandler; use crate::engine::handler::replication_handler::ReplicationHandler; @@ -106,7 +105,7 @@ where C: RaftTypeConfig // Partial ord compare: // Vote does not have to be total ord. // `!(a >= b)` does not imply `a < b`. - if vote.ord_by() >= self.state.vote_ref().ord_by() { + if vote.as_ref_vote() >= self.state.vote_ref().as_ref_vote() { // Ok } else { tracing::info!("vote {} is rejected by local vote: {}", vote, self.state.vote_ref()); @@ -126,7 +125,7 @@ where C: RaftTypeConfig Duration::default() }; - if vote.ord_by() > self.state.vote_ref().ord_by() { + if vote.as_ref_vote() > self.state.vote_ref().as_ref_vote() { tracing::info!("vote is changing from {} to {}", self.state.vote_ref(), vote); self.state.vote.update(C::now(), leader_lease, vote.clone()); diff --git a/openraft/src/metrics/metric.rs b/openraft/src/metrics/metric.rs index 428149ba9..ff275c0a2 100644 --- a/openraft/src/metrics/metric.rs +++ b/openraft/src/metrics/metric.rs @@ -1,9 +1,9 @@ use std::cmp::Ordering; -use crate::base::ord_by::OrdBy; use crate::metrics::metric_display::MetricDisplay; use crate::type_config::alias::LogIdOf; use crate::type_config::alias::VoteOf; +use crate::vote::raft_vote::RaftVoteExt; use crate::LogIdOptionExt; use crate::RaftMetrics; use crate::RaftTypeConfig; @@ -68,7 +68,7 @@ where C: RaftTypeConfig fn partial_cmp(&self, other: &Metric) -> Option { match other { Metric::Term(v) => Some(self.current_term.cmp(v)), - Metric::Vote(v) => self.vote.ord_by().partial_cmp(&v.ord_by()), + Metric::Vote(v) => self.vote.as_ref_vote().partial_cmp(&v.as_ref_vote()), Metric::LastLogIndex(v) => Some(self.last_log_index.cmp(v)), Metric::Applied(v) => Some(self.last_applied.cmp(v)), Metric::AppliedIndex(v) => Some(self.last_applied.index().cmp(v)), diff --git a/openraft/src/network/snapshot_transport.rs b/openraft/src/network/snapshot_transport.rs index e460c74cf..05c3ed484 100644 --- a/openraft/src/network/snapshot_transport.rs +++ b/openraft/src/network/snapshot_transport.rs @@ -18,7 +18,6 @@ mod tokio_rt { use super::Chunked; use super::SnapshotTransport; use super::Streaming; - use crate::base::ord_by::OrdBy; use crate::error::Fatal; use crate::error::InstallSnapshotError; use crate::error::RPCError; @@ -31,6 +30,7 @@ mod tokio_rt { use crate::storage::Snapshot; use crate::type_config::alias::VoteOf; use crate::type_config::TypeConfigExt; + use crate::vote::raft_vote::RaftVoteExt; use crate::ErrorSubject; use crate::ErrorVerb; use crate::OptionalSend; @@ -149,7 +149,7 @@ mod tokio_rt { } }; - if resp.vote.ord_by() > vote.ord_by() { + if resp.vote.as_ref_vote() > vote.as_ref_vote() { // Unfinished, return a response with a higher vote. // The caller checks the vote and return a HigherVote error. return Ok(SnapshotResponse::new(resp.vote)); diff --git a/openraft/src/raft/mod.rs b/openraft/src/raft/mod.rs index 5fd7e3e36..5d6f8c312 100644 --- a/openraft/src/raft/mod.rs +++ b/openraft/src/raft/mod.rs @@ -46,7 +46,6 @@ use tracing::Level; use crate::async_runtime::watch::WatchReceiver; use crate::async_runtime::MpscUnboundedSender; use crate::async_runtime::OneshotSender; -use crate::base::ord_by::OrdBy; use crate::base::BoxAsyncOnceMut; use crate::base::BoxFuture; use crate::base::BoxOnce; @@ -91,6 +90,7 @@ use crate::type_config::alias::SnapshotDataOf; use crate::type_config::alias::VoteOf; use crate::type_config::alias::WatchReceiverOf; use crate::type_config::TypeConfigExt; +use crate::vote::raft_vote::RaftVoteExt; use crate::LogIdOptionExt; use crate::LogIndexOptionExt; use crate::OptionalSend; @@ -472,7 +472,7 @@ where C: RaftTypeConfig // It is not mandatory because it is just a read operation // but prevent unnecessary snapshot transfer early. { - if req_vote.ord_by() >= my_vote.ord_by() { + if req_vote.as_ref_vote() >= my_vote.as_ref_vote() { // Ok } else { tracing::info!("vote {} is rejected by local vote: {}", req_vote, my_vote); @@ -691,7 +691,7 @@ where C: RaftTypeConfig // Condition failed to become Leader #[allow(clippy::neg_cmp_op_on_partial_ord)] - let fail = |m: &RaftMetrics| !(req.from_leader.ord_by() >= m.vote.ord_by()); + let fail = |m: &RaftMetrics| !(req.from_leader.as_ref_vote() >= m.vote.as_ref_vote()); let timeout = Some(Duration::from_millis(self.inner.config.election_timeout_min)); let metrics_res = diff --git a/openraft/src/raft_state/mod.rs b/openraft/src/raft_state/mod.rs index 7cf88fb7f..6236ed8f3 100644 --- a/openraft/src/raft_state/mod.rs +++ b/openraft/src/raft_state/mod.rs @@ -34,7 +34,6 @@ pub(crate) use log_state_reader::LogStateReader; pub use membership_state::MembershipState; pub(crate) use vote_state_reader::VoteStateReader; -use crate::base::ord_by::OrdBy; use crate::display_ext::DisplayOptionExt; use crate::proposer::Leader; use crate::proposer::LeaderQuorumSet; @@ -238,7 +237,7 @@ where C: RaftTypeConfig let new_vote = accepted.to_vote(); let current_vote = curr_accepted.clone().map(|io_id| io_id.to_vote()); assert!( - Some(new_vote.ord_by()) >= current_vote.as_ref().map(|x| x.ord_by()), + Some(new_vote.as_ref_vote()) >= current_vote.as_ref().map(|x| x.as_ref_vote()), "new accepted.committed_vote {} must be >= current accepted.committed_vote: {}", new_vote, current_vote.display(), diff --git a/openraft/src/replication/mod.rs b/openraft/src/replication/mod.rs index 5ea47efae..c4e61ff92 100644 --- a/openraft/src/replication/mod.rs +++ b/openraft/src/replication/mod.rs @@ -21,7 +21,6 @@ use tracing_futures::Instrument; use crate::async_runtime::MpscUnboundedReceiver; use crate::async_runtime::MpscUnboundedSender; use crate::async_runtime::MpscUnboundedWeakSender; -use crate::base::ord_by::OrdBy; use crate::config::Config; use crate::core::notification::Notification; use crate::core::sm::handle::SnapshotReader; @@ -484,7 +483,7 @@ where } AppendEntriesResponse::HigherVote(vote) => { debug_assert!( - vote.ord_by() > self.session_id.vote().ord_by(), + vote.as_ref_vote() > self.session_id.vote().as_ref_vote(), "higher vote({}) should be greater than leader's vote({})", vote, self.session_id.vote(), @@ -803,7 +802,7 @@ where // Handle response conditions. let sender_vote = self.session_id.vote(); - if resp.vote.ord_by() > sender_vote.ord_by() { + if resp.vote.as_ref_vote() > sender_vote.as_ref_vote() { return Err(ReplicationError::HigherVote(HigherVote { higher: resp.vote, sender_vote, diff --git a/openraft/src/vote/raft_vote.rs b/openraft/src/vote/raft_vote.rs index a5205bf12..6091449ea 100644 --- a/openraft/src/vote/raft_vote.rs +++ b/openraft/src/vote/raft_vote.rs @@ -1,7 +1,6 @@ use std::fmt::Debug; use std::fmt::Display; -use crate::base::ord_by::OrdBy; use crate::base::OptionalFeatures; use crate::type_config::alias::CommittedLeaderIdOf; use crate::vote::committed::CommittedVote; @@ -107,17 +106,3 @@ where T: RaftVote, { } - -impl OrdBy for T -where - C: RaftTypeConfig, - T: RaftVote, -{ - type By<'k> - = RefVote<'k, C> - where Self: 'k; - - fn ord_by(&self) -> Self::By<'_> { - self.as_ref_vote() - } -}