Change: Add RaftNetworkV2, remove feature flag generic-snapshot-data

- Remove `generic-snapshot-data`. The snapshot data trait is now consistently bound to `OptionalSend + 'static`. - Introduction of `RaftNetworkV2::full_snapshot()` for transmitting snapshots in one piece. - Retention of the original chunk-based snapshot transmission API in `RaftNetwork::install_snapshot()`. From this commit onwards: - To use the one-piece snapshot transmission, implement `RaftNetworkV2` when creating a `Raft` instance. - To continue using the chunk-based transmission, implement `RaftNetwork`. Compatibility: - `RaftNetworkV2` is automatically implemented for any `RaftNetwork` if `RaftTypeConfig::SnapshotData` implements `tokio::io::AsyncRead`, `tokio::io::AsyncWrite`, `tokio::io::AsyncSeek`, and `Unpin`. Upgrade tip: - Remove the `generic-snapshot-data` feature flag. - If you prefer using the original chunk-based method, no further action is required. Otherwise, to adopt the new one-piece snapshot transmission: - Replace `RaftNetwork` and `RaftNetworkFactory` with `RaftNetworkV2` and `RaftNetworkFactoryV2`.
databendlabs · May 6, 2024 · 4d0d103 · 4d0d103
1 parent 5c83e16
commit 4d0d103
Show file tree

Hide file tree

Showing 34 changed files with 204 additions and 220 deletions.
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
@@ -63,44 +63,6 @@ jobs:
           args: --features bench nothing-to-run --manifest-path openraft/Cargo.toml
 
 
-  # Run feature specific test for crate openraft.
-  openraft-features:
-    runs-on: ubuntu-latest
-
-    strategy:
-      fail-fast: false
-      matrix:
-        include:
-          - toolchain: "nightly"
-            features: "generic-snapshot-data"
-
-
-    steps:
-      - name: Setup | Checkout
-        uses: actions/checkout@v2
-
-
-      - name: Setup | Toolchain
-        uses: actions-rs/[email protected]
-        with:
-          toolchain: "${{ matrix.toolchain }}"
-          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
-
-
   # Run openraft unit test `openraft/` and integration test `tests/`.
   openraft-test:
     runs-on: ubuntu-latest
@@ -409,7 +371,7 @@ jobs:
         example:
           - "memstore"
           - "raft-kv-memstore"
-          - "raft-kv-memstore-generic-snapshot-data"
+          - "raft-kv-memstore-network-v2"
           - "raft-kv-memstore-opendal-snapshot-data"
           - "raft-kv-memstore-singlethreaded"
           - "raft-kv-rocksdb"

diff --git a/Cargo.toml b/Cargo.toml
@@ -60,7 +60,7 @@ exclude = [
     "examples/memstore",
     "examples/raft-kv-memstore",
     "examples/raft-kv-memstore-singlethreaded",
-    "examples/raft-kv-memstore-generic-snapshot-data",
+    "examples/raft-kv-memstore-network-v2",
     "examples/raft-kv-memstore-opendal-snapshot-data",
     "examples/raft-kv-rocksdb",
 ]
diff --git a/...memstore-generic-snapshot-data/.gitignore → ...es/raft-kv-memstore-network-v2/.gitignore b/...memstore-generic-snapshot-data/.gitignore → ...es/raft-kv-memstore-network-v2/.gitignore
diff --git a/...memstore-generic-snapshot-data/Cargo.toml → ...es/raft-kv-memstore-network-v2/Cargo.toml b/...memstore-generic-snapshot-data/Cargo.toml → ...es/raft-kv-memstore-network-v2/Cargo.toml
@@ -1,5 +1,5 @@
 [package]
-name = "raft-kv-memstore-generic-snapshot-data"
+name = "raft-kv-memstore-network-v2"
 version = "0.1.0"
 readme = "README.md"
 
@@ -17,7 +17,7 @@ repository = "https://github.com/datafuselabs/openraft"
 
 [dependencies]
 memstore = { path = "../memstore", features = [] }
-openraft = { path = "../../openraft", features = ["serde", "generic-snapshot-data", "type-alias"] }
+openraft = { path = "../../openraft", features = ["serde", "type-alias"] }
 
 clap = { version = "4.1.11", features = ["derive", "env"] }
 reqwest = { version = "0.11.9", features = ["json"] }

diff --git a/...-memstore-generic-snapshot-data/README.md → ...les/raft-kv-memstore-network-v2/README.md b/...-memstore-generic-snapshot-data/README.md → ...les/raft-kv-memstore-network-v2/README.md
@@ -1,13 +1,13 @@
-# Example Openraft kv-store with `generic-snapshot-data` enabled
+# Example Openraft kv-store using `RaftNetworkV2`
 
-With `generic-snapshot-data` feature flag enabled, Openraft allows application to use any data type for snapshot data,
+With `RaftNetworkV2`, Openraft allows application to use any data type for snapshot data,
 instead of a single-file like data format with `AsyncSeek + AsyncRead + AsyncWrite + Unpin` bounds.
 
 This example is similar to the basic raft-kv-memstore example
-but focuses on how to handle snapshot with `generic-snapshot-data` enabled.
+but focuses on how to handle snapshot with `RaftNetworkV2::full_snapshot()`.
 Other aspects are minimized.
 
-To send a complete snapshot, Refer to implementation of `RaftNetwork::full_snapshot()` in this example.
+To send a complete snapshot, Refer to implementation of `RaftNetworkV2::full_snapshot()` in this example.
 
 To receive a complete snapshot, Refer to implementation of `api::snapshot()` in this example.
 

diff --git a/...memstore-generic-snapshot-data/src/api.rs → ...es/raft-kv-memstore-network-v2/src/api.rs b/...memstore-generic-snapshot-data/src/api.rs → ...es/raft-kv-memstore-network-v2/src/api.rs
diff --git a/...memstore-generic-snapshot-data/src/app.rs → ...es/raft-kv-memstore-network-v2/src/app.rs b/...memstore-generic-snapshot-data/src/app.rs → ...es/raft-kv-memstore-network-v2/src/app.rs
diff --git a/...memstore-generic-snapshot-data/src/lib.rs → ...es/raft-kv-memstore-network-v2/src/lib.rs b/...memstore-generic-snapshot-data/src/lib.rs → ...es/raft-kv-memstore-network-v2/src/lib.rs
diff --git a/...tore-generic-snapshot-data/src/network.rs → ...aft-kv-memstore-network-v2/src/network.rs b/...tore-generic-snapshot-data/src/network.rs → ...aft-kv-memstore-network-v2/src/network.rs
@@ -2,6 +2,7 @@ use std::future::Future;
 
 use openraft::error::RemoteError;
 use openraft::error::ReplicationClosed;
+use openraft::network::v2::RaftNetworkV2;
 use openraft::network::RPCOption;
 use openraft::raft::AppendEntriesRequest;
 use openraft::raft::AppendEntriesResponse;
@@ -10,7 +11,6 @@ use openraft::raft::VoteRequest;
 use openraft::raft::VoteResponse;
 use openraft::BasicNode;
 use openraft::OptionalSend;
-use openraft::RaftNetwork;
 use openraft::RaftNetworkFactory;
 use openraft::Snapshot;
 use openraft::Vote;
@@ -36,7 +36,7 @@ impl RaftNetworkFactory<TypeConfig> for Router {
     }
 }
 
-impl RaftNetwork<TypeConfig> for Connection {
+impl RaftNetworkV2<TypeConfig> for Connection {
     async fn append_entries(
         &mut self,
         req: AppendEntriesRequest<TypeConfig>,

diff --git a/...store-generic-snapshot-data/src/router.rs → ...raft-kv-memstore-network-v2/src/router.rs b/...store-generic-snapshot-data/src/router.rs → ...raft-kv-memstore-network-v2/src/router.rs
diff --git a/...mstore-generic-snapshot-data/src/store.rs → .../raft-kv-memstore-network-v2/src/store.rs b/...mstore-generic-snapshot-data/src/store.rs → .../raft-kv-memstore-network-v2/src/store.rs
diff --git a/...ore-generic-snapshot-data/test-cluster.sh → ...ft-kv-memstore-network-v2/test-cluster.sh b/...ore-generic-snapshot-data/test-cluster.sh → ...ft-kv-memstore-network-v2/test-cluster.sh
diff --git a/...neric-snapshot-data/tests/cluster/main.rs → ...memstore-network-v2/tests/cluster/main.rs b/...neric-snapshot-data/tests/cluster/main.rs → ...memstore-network-v2/tests/cluster/main.rs
diff --git a/...apshot-data/tests/cluster/test_cluster.rs → ...-network-v2/tests/cluster/test_cluster.rs b/...apshot-data/tests/cluster/test_cluster.rs → ...-network-v2/tests/cluster/test_cluster.rs
@@ -4,10 +4,10 @@ use std::panic::PanicInfo;
 use std::time::Duration;
 
 use openraft::BasicNode;
-use raft_kv_memstore_generic_snapshot_data::new_raft;
-use raft_kv_memstore_generic_snapshot_data::router::Router;
-use raft_kv_memstore_generic_snapshot_data::store::Request;
-use raft_kv_memstore_generic_snapshot_data::typ;
+use raft_kv_memstore_network_v2::new_raft;
+use raft_kv_memstore_network_v2::router::Router;
+use raft_kv_memstore_network_v2::store::Request;
+use raft_kv_memstore_network_v2::typ;
 use tokio::task;
 use tokio::task::LocalSet;
 use tracing_subscriber::EnvFilter;

diff --git a/examples/raft-kv-memstore-opendal-snapshot-data/Cargo.toml b/examples/raft-kv-memstore-opendal-snapshot-data/Cargo.toml
@@ -18,7 +18,7 @@ repository = "https://github.com/datafuselabs/openraft"
 
 [dependencies]
 memstore = { path = "../memstore", features = [] }
-openraft = { path = "../../openraft", features = ["serde", "generic-snapshot-data", "type-alias"] }
+openraft = { path = "../../openraft", features = ["serde", "type-alias"] }
 
 serde = { version = "1.0.114", features = ["derive"] }
 serde_json = "1.0.57"

diff --git a/examples/raft-kv-memstore-opendal-snapshot-data/README.md b/examples/raft-kv-memstore-opendal-snapshot-data/README.md
@@ -1,15 +1,13 @@
 # Example Openraft kv-store with snapshot stored in remote storage
 
-With `generic-snapshot-data` feature flag enabled, Openraft allows application to use any data type for snapshot data,
-instead of a single-file like data format with `AsyncSeek + AsyncRead + AsyncWrite + Unpin` bounds.
-
-This example shows how to save and retrieve snapshot data from remote storage, allowing users to follow a similar pattern for implementing business logic such as snapshot backups.
+This example shows how to save and retrieve snapshot data from remote storage,
+allowing users to follow a similar pattern for implementing business logic such as snapshot backups.
 
 This example is similar to the basic raft-kv-memstore example
 but focuses on how to store and fetch snapshot data from remote storage.
 Other aspects are minimized.
 
-To send a complete snapshot, Refer to implementation of `RaftNetwork::full_snapshot()` in this example.
+To send a complete snapshot, Refer to implementation of `RaftNetworkV2::full_snapshot()` in this example.
 
 To receive a complete snapshot, Refer to implementation of `api::snapshot()` in this example.
 

diff --git a/examples/raft-kv-memstore-opendal-snapshot-data/src/network.rs b/examples/raft-kv-memstore-opendal-snapshot-data/src/network.rs
@@ -2,6 +2,7 @@ use std::future::Future;
 
 use openraft::error::RemoteError;
 use openraft::error::ReplicationClosed;
+use openraft::network::v2::RaftNetworkV2;
 use openraft::network::RPCOption;
 use openraft::raft::AppendEntriesRequest;
 use openraft::raft::AppendEntriesResponse;
@@ -10,7 +11,6 @@ use openraft::raft::VoteRequest;
 use openraft::raft::VoteResponse;
 use openraft::BasicNode;
 use openraft::OptionalSend;
-use openraft::RaftNetwork;
 use openraft::RaftNetworkFactory;
 use openraft::Snapshot;
 use openraft::Vote;
@@ -36,7 +36,7 @@ impl RaftNetworkFactory<TypeConfig> for Router {
     }
 }
 
-impl RaftNetwork<TypeConfig> for Connection {
+impl RaftNetworkV2<TypeConfig> for Connection {
     async fn append_entries(
         &mut self,
         req: AppendEntriesRequest<TypeConfig>,

diff --git a/openraft/Cargo.toml b/openraft/Cargo.toml
@@ -93,18 +93,6 @@ singlethreaded = ["openraft-macros/singlethreaded"]
 loosen-follower-log-revert = []
 
 
-# Enable this feature flag to eliminate the `AsyncRead + AsyncWrite + AsyncSeek
-# + Unpin` bound from `RaftTypeConfig::SnapshotData`.
-#
-# Enabling this feature allows applications to use a custom snapshot data format
-# and transport fragmentation, diverging from the default implementation which
-# typically relies on a single-file structure .
-#
-# By default it is off.
-# This feature is introduced in 0.9.0
-generic-snapshot-data = []
-
-
 # Enables "log" feature in `tracing` crate, to let tracing events emit log
 # record.
 # See: https://docs.rs/tracing/latest/tracing/#emitting-log-records
@@ -119,7 +107,6 @@ tracing-log = [ "tracing/log" ]
 features = [
     "bt",
     "compat",
-    "generic-snapshot-data",
     "loosen-follower-log-revert",
     "serde",
     "tracing-log",

diff --git a/openraft/src/core/raft_core.rs b/openraft/src/core/raft_core.rs
@@ -59,9 +59,9 @@ use crate::metrics::RaftDataMetrics;
 use crate::metrics::RaftMetrics;
 use crate::metrics::RaftServerMetrics;
 use crate::metrics::ReplicationMetrics;
+use crate::network::v2::RaftNetworkV2;
 use crate::network::RPCOption;
 use crate::network::RPCTypes;
-use crate::network::RaftNetwork;
 use crate::network::RaftNetworkFactory;
 use crate::progress::entry::ProgressEntry;
 use crate::progress::Inflight;

diff --git a/openraft/src/docs/feature_flags/feature-flags-toc.md b/openraft/src/docs/feature_flags/feature-flags-toc.md
@@ -1,7 +1,6 @@
 - [feature-flag `bench`](#feature-flag-bench)
 - [feature-flag `bt`](#feature-flag-bt)
 - [feature-flag `compat`](#feature-flag-compat)
-- [feature-flag `generic-snapshot-data`](#feature-flag-generic-snapshot-data)
 - [feature-flag `loosen-follower-log-revert`](#feature-flag-loosen-follower-log-revert)
 - [feature-flag `serde`](#feature-flag-serde)
 - [feature-flag `single-term-leader`](#feature-flag-single-term-leader)

diff --git a/openraft/src/docs/feature_flags/feature-flags.md b/openraft/src/docs/feature_flags/feature-flags.md
@@ -17,33 +17,6 @@ This feature works ONLY with nightly rust, because it requires unstable feature
 
 Enables compatibility supporting types.
 
-## feature-flag `generic-snapshot-data`
-
-Enable this feature flag
-to eliminate the `AsyncRead + AsyncWrite + AsyncSeek + Unpin` bound
-from [`RaftTypeConfig::SnapshotData`](crate::RaftTypeConfig::SnapshotData)
-Enabling this feature allows applications to use a custom snapshot data format and transport fragmentation,
-diverging from the default implementation which typically relies on a single-file structure.
-
-By default, it is off.
-This feature is introduced in 0.9.0
-
-On the sending end (leader that sends snapshot to follower):
-
-- Without `generic-snapshot-data`: [`RaftNetwork::full_snapshot()`]
-  provides a default implementation that invokes the chunk-based API
-  [`RaftNetwork::install_snapshot()`] for transmit.
-
-- With `generic-snapshot-data` enabled: [`RaftNetwork::full_snapshot()`]
-  must be implemented to provide application customized snapshot transmission.
-  Application does not need to implement [`RaftNetwork::install_snapshot()`].
-
-On the receiving end(follower):
-
-- `Raft::install_snapshot()` is available only when `generic-snapshot-data` is disabled.
-
-Refer to example `examples/raft-kv-memstore-generic-snapshot-data` with `generic-snapshot-data` enabled.
-
 ## feature-flag `loosen-follower-log-revert`
 
 Permit the follower's log to roll back to an earlier state without causing the leader to panic.

diff --git a/openraft/src/docs/getting_started/getting-started.md b/openraft/src/docs/getting_started/getting-started.md
@@ -166,16 +166,22 @@ The caller always assumes a completed writing is persistent.
 The raft correctness highly depends on a reliable store.
 
 
-## 4. Implement [`RaftNetwork`]
+## 4. Implement [`RaftNetwork`] or [`RaftNetworkV2`].
+
+Raft nodes communicate with each other to achieve consensus about the logs.
+The trait [`RaftNetwork`] and [`RaftNetworkV2`] defines the data transmission protocol.
+
+Your application can use either [`RaftNetwork`] or [`RaftNetworkV2`].
+The only difference between them is:
+- [`RaftNetwork`] sends snapshot in chunks with [`RaftNetwork::install_snapshot()`][`install_snapshot()`],
+- while [`RaftNetworkV2`] sends snapshot in one piece with [`RaftNetworkV2::full_snapshot()`][`full_snapshot()`].
 
-Raft nodes need to communicate with each other to achieve consensus about the logs.
-The trait [`RaftNetwork`] defines the data transmission requirements.
 
 ```ignore
 pub trait RaftNetwork<C: RaftTypeConfig>: Send + Sync + 'static {
     async fn vote(&mut self, rpc: VoteRequest<C::NodeId>) -> Result<...>;
     async fn append_entries(&mut self, rpc: AppendEntriesRequest<C>) -> Result<...>;
-    async fn snapshot(&mut self, vote: Vote<C::NodeId>, snapshot: Snapshot<C>) -> Result<...>;
+    async fn install_snapshot(&mut self, vote: Vote<C::NodeId>, snapshot: Snapshot<C>) -> Result<...>;
 }
 ```
 
@@ -186,11 +192,12 @@ and receiving messages between Raft nodes.
 Here is the list of methods that need to be implemented for the [`RaftNetwork`] trait:
 
 
-| [`RaftNetwork`] method | forward request          | to target                                      |
-|------------------------|--------------------------|------------------------------------------------|
-| [`append_entries()`]   | [`AppendEntriesRequest`] | remote node [`Raft::append_entries()`]         |
-| [`full_snapshot()`]    | [`Snapshot`]             | remote node [`Raft::install_full_snapshot()`] |
-| [`vote()`]             | [`VoteRequest`]          | remote node [`Raft::vote()`]                   |
+| [`RaftNetwork`] method | forward request            | to target                                     |
+|------------------------|----------------------------|-----------------------------------------------|
+| [`append_entries()`]   | [`AppendEntriesRequest`]   | remote node [`Raft::append_entries()`]        |
+| [`vote()`]             | [`VoteRequest`]            | remote node [`Raft::vote()`]                  |
+| [`install_snapshot()`] | [`InstallSnapshotRequest`] | remote node [`Raft::install_snapshot()`]      |
+| [`full_snapshot()`]    | [`Snapshot`]               | remote node [`Raft::install_full_snapshot()`] |
 
 [Mem KV Network](https://github.com/datafuselabs/openraft/blob/main/examples/raft-kv-memstore/src/network/raft_network_impl.rs)
 demonstrates how to forward messages to other Raft nodes using [`reqwest`](https://docs.rs/reqwest/latest/reqwest/) as network transport layer.
@@ -202,9 +209,9 @@ When the server receives a Raft RPC, it simply passes it to its `raft` instance
 For a real-world implementation, you may want to use [Tonic gRPC](https://github.com/hyperium/tonic) to handle gRPC-based communication between Raft nodes. The [databend-meta](https://github.com/datafuselabs/databend/blob/6603392a958ba8593b1f4b01410bebedd484c6a9/metasrv/src/network.rs#L89) project provides an excellent real-world example of a Tonic gRPC-based Raft network implementation.
 
 
-### Implement [`RaftNetworkFactory`]
+### Implement [`RaftNetworkFactory`].
 
-[`RaftNetworkFactory`] is a singleton responsible for creating [`RaftNetwork`] instances for each replication target node.
+[`RaftNetworkFactory`] is a singleton responsible for creating [`RaftNetworkV2`] instances for each replication target node.
 
 ```ignore
 pub trait RaftNetworkFactory<C: RaftTypeConfig>: Send + Sync + 'static {
@@ -353,7 +360,8 @@ Additionally, two test scripts for setting up a cluster are available:
 [`Raft`]:                               `crate::Raft`
 [`Raft::append_entries()`]:             `crate::Raft::append_entries`
 [`Raft::vote()`]:                       `crate::Raft::vote`
-[`Raft::install_full_snapshot()`]:  `crate::Raft::install_full_snapshot`
+[`Raft::install_full_snapshot()`]:      `crate::Raft::install_full_snapshot`
+[`Raft::install_snapshot()`]:           `crate::Raft::install_snapshot`
 
 [`AppendEntriesRequest`]:               `crate::raft::AppendEntriesRequest`
 [`VoteRequest`]:                        `crate::raft::VoteRequest`
@@ -396,11 +404,13 @@ Additionally, two test scripts for setting up a cluster are available:
 [`get_snapshot_builder()`]:             `crate::storage::RaftStateMachine::get_snapshot_builder`
 
 [`RaftNetworkFactory`]:                 `crate::network::RaftNetworkFactory`
-[`RaftNetworkFactory::new_client()`]:   `crate::network::RaftNetworkFactory::new_client`
 [`RaftNetwork`]:                        `crate::network::RaftNetwork`
+[`RaftNetworkFactory::new_client()`]:   `crate::network::RaftNetworkFactory::new_client`
 [`append_entries()`]:                   `crate::RaftNetwork::append_entries`
 [`vote()`]:                             `crate::RaftNetwork::vote`
-[`full_snapshot()`]:                         `crate::RaftNetwork::full_snapshot`
+[`install_snapshot()`]:                 `crate::RaftNetwork::install_snapshot`
+[`full_snapshot()`]:                    `crate::network::v2::RaftNetworkV2::full_snapshot`
+[`RaftNetworkV2`]:                      `crate::network::v2::RaftNetworkV2`
 
 
 [`RaftSnapshotBuilder`]:                `crate::storage::RaftSnapshotBuilder`

diff --git a/openraft/src/docs/upgrade_guide/upgrade-v08-v09.md b/openraft/src/docs/upgrade_guide/upgrade-v08-v09.md
@@ -193,7 +193,7 @@ To use arbitrary snapshot data, the application needs to:
 [`Raft::install_full_snapshot()`]:    `crate::Raft::install_full_snapshot`
 
 [`RaftNetwork`]:                      `crate::network::RaftNetwork`
-[`RaftNetwork::full_snapshot()`]:     `crate::network::RaftNetwork::full_snapshot`
+[`RaftNetwork::full_snapshot()`]:     `crate::network::v2::RaftNetworkV2::full_snapshot`
 
 [`RaftLogStorage::save_committed()`]: `crate::storage::RaftLogStorage::save_committed`
 

diff --git a/openraft/src/network/mod.rs b/openraft/src/network/mod.rs
@@ -1,15 +1,16 @@
 //! The Raft network interface.
 
 mod backoff;
-mod factory;
-#[allow(clippy::module_inception)] mod network;
 mod rpc_option;
 mod rpc_type;
 
+pub mod v1;
+pub mod v2;
+
 pub mod snapshot_transport;
 
 pub use backoff::Backoff;
-pub use factory::RaftNetworkFactory;
-pub use network::RaftNetwork;
 pub use rpc_option::RPCOption;
 pub use rpc_type::RPCTypes;
+pub use v1::RaftNetwork;
+pub use v1::RaftNetworkFactory;