Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change: Remove feature flag single-term-leader #1289

Merged
merged 4 commits into from
Jan 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 48 additions & 28 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -63,29 +63,20 @@ 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:
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
Expand All @@ -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/[email protected]
with:
toolchain: "${{ matrix.toolchain }}"
override: true


- name: Test crate `tests/`
Expand All @@ -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/


Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion cluster_benchmark/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions cluster_benchmark/tests/benchmark/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<TypeConfig>,
);

#[derive(Debug)]
Expand Down
7 changes: 7 additions & 0 deletions examples/raft-kv-memstore-grpc/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
"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(
Expand All @@ -17,6 +20,10 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
"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(())
}
4 changes: 2 additions & 2 deletions examples/raft-kv-memstore-grpc/proto/internal_service.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions examples/raft-kv-memstore-grpc/src/grpc/management_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ 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;
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.
Expand Down Expand Up @@ -62,11 +62,11 @@ impl ManagementService for ManagementServiceImpl {
let req = request.into_inner();

// Convert nodes into required format
let nodes_map: BTreeMap<u64, Node> = req
let nodes_map: BTreeMap<u64, pb::Node> = 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,
})
Expand Down
79 changes: 32 additions & 47 deletions examples/raft-kv-memstore-grpc/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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::*;

Expand All @@ -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,
);

Expand All @@ -33,87 +34,71 @@ pub mod protobuf {
#[path = "../../utils/declare_types.rs"]
pub mod typ;

impl From<protobuf::LeaderId> for LeaderId {
fn from(proto_leader_id: protobuf::LeaderId) -> Self {
LeaderId::new(proto_leader_id.term, proto_leader_id.node_id)
}
}

impl From<protobuf::Vote> for typ::Vote {
fn from(proto_vote: protobuf::Vote) -> Self {
let leader_id: LeaderId = proto_vote.leader_id.unwrap().into();
impl From<pb::Vote> 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<protobuf::LogId> 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<pb::LogId> for LogId {
fn from(proto_log_id: pb::LogId) -> Self {
LogId::new(proto_log_id.term, proto_log_id.index)
}
}

impl From<protobuf::VoteRequest> for VoteRequest {
fn from(proto_vote_req: protobuf::VoteRequest) -> Self {
let vote: typ::Vote = proto_vote_req.vote.unwrap().into();
impl From<pb::VoteRequest> 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<protobuf::VoteResponse> for VoteResponse {
fn from(proto_vote_resp: protobuf::VoteResponse) -> Self {
let vote: typ::Vote = proto_vote_resp.vote.unwrap().into();
impl From<pb::VoteResponse> 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<LeaderId> for protobuf::LeaderId {
fn from(leader_id: LeaderId) -> Self {
protobuf::LeaderId {
term: leader_id.term,
node_id: leader_id.node_id,
}
}
}

impl From<typ::Vote> for protobuf::Vote {
fn from(vote: typ::Vote) -> Self {
protobuf::Vote {
leader_id: Some(protobuf::LeaderId {
impl From<Vote> 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,
}),
committed: vote.is_committed(),
}
}
}
impl From<LogId> for protobuf::LogId {
impl From<LogId> 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<VoteRequest> for protobuf::VoteRequest {
impl From<VoteRequest> 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<VoteResponse> for protobuf::VoteResponse {
impl From<VoteResponse> 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()),
Expand Down
4 changes: 2 additions & 2 deletions examples/raft-kv-memstore-grpc/src/network/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@ 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;
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;

Expand All @@ -38,7 +38,7 @@ impl RaftNetworkFactory<TypeConfig> 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 {
Expand Down
Loading
Loading