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] 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"]