diff --git a/openraft/src/engine.rs b/openraft/src/engine/engine.rs similarity index 97% rename from openraft/src/engine.rs rename to openraft/src/engine/engine.rs index fb23a9b37..1be8e1360 100644 --- a/openraft/src/engine.rs +++ b/openraft/src/engine/engine.rs @@ -6,8 +6,9 @@ use maplit::btreeset; use crate::entry::RaftEntry; use crate::error::InitializeError; -use crate::error::MissingNodeInfo; +use crate::error::NotAMembershipEntry; use crate::error::NotAllowed; +use crate::error::NotInMembers; use crate::storage::InitialState; use crate::EffectiveMembership; use crate::LogId; @@ -140,7 +141,7 @@ impl Engine { if let Some(m) = entry.get_membership() { self.check_members_contain_me(m)?; } else { - panic!("Initialization log has to be a membership config entry"); + Err(NotAMembershipEntry {})?; } self.try_update_membership(entry); @@ -245,11 +246,11 @@ impl Engine { } /// When initialize, the node that accept initialize request has to be a member of the initial config. - fn check_members_contain_me(&self, m: &Membership) -> Result<(), MissingNodeInfo> { + fn check_members_contain_me(&self, m: &Membership) -> Result<(), NotInMembers> { if !m.is_member(&self.id) { - let e = MissingNodeInfo { + let e = NotInMembers { node_id: self.id, - reason: "target should be a member".to_string(), + membership: m.clone(), }; Err(e) } else { diff --git a/openraft/src/engine/initialize_test.rs b/openraft/src/engine/initialize_test.rs new file mode 100644 index 000000000..2089ee5a4 --- /dev/null +++ b/openraft/src/engine/initialize_test.rs @@ -0,0 +1,120 @@ +use maplit::btreeset; + +use crate::engine::Command; +use crate::engine::Engine; +use crate::entry::EntryRef; +use crate::error::InitializeError; +use crate::error::NotAMembershipEntry; +use crate::error::NotAllowed; +use crate::error::NotInMembers; +use crate::EntryPayload; +use crate::LeaderId; +use crate::LogId; +use crate::Membership; +use crate::MetricsChangeFlags; +use crate::Vote; + +#[derive(Clone, serde::Serialize, serde::Deserialize)] +pub(crate) struct Req {} +#[derive(Clone, serde::Serialize, serde::Deserialize)] +pub(crate) struct Resp {} + +crate::declare_raft_types!( + pub(crate) Config: D = Req, R = Resp, NodeId = u64 +); + +#[test] +fn test_initialize() -> anyhow::Result<()> { + let eng = Engine::::default; + + let log_id0 = LogId { + leader_id: LeaderId::new(0, 0), + index: 0, + }; + let vote0 = Vote::new(0, 0); + + let m12 = || Membership::::new(vec![btreeset! {1,2}], None); + let payload = EntryPayload::::Membership(m12()); + let mut entries = [EntryRef::new(&payload)]; + + tracing::info!("--- ok"); + { + let mut eng = eng(); + eng.id = 1; + + eng.initialize(&mut entries)?; + assert_eq!(Some(log_id0), eng.state.last_log_id); + assert_eq!( + MetricsChangeFlags { + leader: false, + other_metrics: true + }, + eng.metrics_flags + ); + assert_eq!(m12(), eng.state.effective_membership.membership); + assert_eq!( + vec![ + Command::AppendInputEntries { range: 0..1 }, + Command::UpdateMembership { membership: m12() }, + Command::MoveInputCursorBy { n: 1 } + ], + eng.commands + ); + } + + tracing::info!("--- not allowed because of last_log_id"); + { + let mut eng = eng(); + eng.state.last_log_id = Some(log_id0); + + assert_eq!( + Err(InitializeError::NotAllowed(NotAllowed { + last_log_id: Some(log_id0), + vote: vote0, + })), + eng.initialize(&mut entries) + ); + } + + tracing::info!("--- not allowed because of vote"); + { + let mut eng = eng(); + eng.state.vote = Vote::new(0, 1); + + assert_eq!( + Err(InitializeError::NotAllowed(NotAllowed { + last_log_id: None, + vote: Vote::new(0, 1), + })), + eng.initialize(&mut entries) + ); + } + + tracing::info!("--- node id 0 is not in membership"); + { + let mut eng = eng(); + + assert_eq!( + Err(InitializeError::NotInMembers(NotInMembers { + node_id: 0, + membership: m12() + })), + eng.initialize(&mut entries) + ); + } + + tracing::info!("--- log entry is not a membership entry"); + { + let mut eng = eng(); + + let payload = EntryPayload::::Blank; + let mut entries = [EntryRef::new(&payload)]; + + assert_eq!( + Err(InitializeError::NotAMembershipEntry(NotAMembershipEntry {})), + eng.initialize(&mut entries) + ); + } + + Ok(()) +} diff --git a/openraft/src/engine/mod.rs b/openraft/src/engine/mod.rs new file mode 100644 index 000000000..c390ed46a --- /dev/null +++ b/openraft/src/engine/mod.rs @@ -0,0 +1,8 @@ +#[allow(clippy::module_inception)] +mod engine; + +#[cfg(test)] +mod initialize_test; + +pub(crate) use engine::Command; +pub(crate) use engine::Engine; diff --git a/openraft/src/error.rs b/openraft/src/error.rs index 0b77a20bf..5155bff75 100644 --- a/openraft/src/error.rs +++ b/openraft/src/error.rs @@ -11,6 +11,7 @@ use serde::Serialize; use crate::raft_types::SnapshotSegmentId; use crate::LogId; +use crate::Membership; use crate::Node; use crate::NodeId; use crate::RPCTypes; @@ -161,6 +162,12 @@ pub enum InitializeError { #[error(transparent)] NotAllowed(#[from] NotAllowed), + #[error(transparent)] + NotInMembers(#[from] NotInMembers), + + #[error(transparent)] + NotAMembershipEntry(#[from] NotAMembershipEntry), + #[error(transparent)] MissingNodeInfo(#[from] MissingNodeInfo), @@ -386,6 +393,19 @@ pub struct MissingNodeInfo { pub reason: String, } +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, thiserror::Error)] +#[serde(bound = "")] +#[error("node {node_id} has to be a member. membership:{membership:?}")] +pub struct NotInMembers { + pub node_id: NID, + pub membership: Membership, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, thiserror::Error)] +#[serde(bound = "")] +#[error("initializing log entry has to be a membership config entry")] +pub struct NotAMembershipEntry {} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, thiserror::Error)] #[error("new membership can not be empty")] pub struct EmptyMembership {} diff --git a/openraft/src/raft_types.rs b/openraft/src/raft_types.rs index 7f8d6eb2f..d6fa5cc08 100644 --- a/openraft/src/raft_types.rs +++ b/openraft/src/raft_types.rs @@ -145,7 +145,7 @@ pub enum Update { } /// Describes the need to update some aspect of the metrics. -#[derive(Debug, Clone, Default)] +#[derive(Debug, Clone, Default, PartialEq, Eq)] pub(crate) struct MetricsChangeFlags { pub leader: bool, // TODO: split other_metrics into data metrics and cluster metrics diff --git a/openraft/tests/initialize/t20_initialization.rs b/openraft/tests/initialize/t20_initialization.rs index 4ae19b54d..9b58564df 100644 --- a/openraft/tests/initialize/t20_initialization.rs +++ b/openraft/tests/initialize/t20_initialization.rs @@ -3,8 +3,8 @@ use std::time::Duration; use maplit::btreeset; use openraft::error::InitializeError; -use openraft::error::MissingNodeInfo; use openraft::error::NotAllowed; +use openraft::error::NotInMembers; use openraft::Config; use openraft::EffectiveMembership; use openraft::EntryPayload; @@ -152,9 +152,9 @@ async fn initialize_err_target_not_include_target() -> anyhow::Result<()> { let err = res.unwrap_err(); assert_eq!( - InitializeError::MissingNodeInfo(MissingNodeInfo { + InitializeError::NotInMembers(NotInMembers { node_id, - reason: "target should be a member".to_string() + membership: Membership::new(vec![btreeset! {9 }], None) }), err );