Change: rename variants in ChangeMembers, add AddVoters

Rename `ChangeMembers::AddVoter` to `AddVoterIds`, because it just
updates voter ids.

Rename `ChangeMembers::RemoveVoter` to `RemoveVoters`.

Add `ChangeMembers::AddVoters(BTreeMap)` to add voters with
corresponding `Node`, i.e., it adds nodes as learners and update
the voter-ids in a `Membership`.

Author: drmingdrmer

openraft/src/change_members.rs

@@ -8,11 +8,34 @@ use crate::NodeId;
 #[derive(PartialEq, Eq)]
 #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize), serde(bound = ""))]
 pub enum ChangeMembers<NID: NodeId, N: Node> {
-    AddVoter(BTreeSet<NID>),
-    RemoveVoter(BTreeSet<NID>),
+    /// Upgrade learners to voters.
+    ///
+    /// The learners have to present or [`error::LearnerNotFound`](`crate::error::LearnerNotFound`)
+    /// error will be returned.
+    AddVoterIds(BTreeSet<NID>),
+
+    /// Add voters with corresponding nodes.
+    AddVoters(BTreeMap<NID, N>),
+
+    /// Remove voters, leave removed voters as learner or not.
+    RemoveVoters(BTreeSet<NID>),
+
+    /// Replace voter ids with a new set. The node of every new voter has to already be a learner.
     ReplaceAllVoters(BTreeSet<NID>),
+
+    /// Add nodes to membership, as learners.
     AddNodes(BTreeMap<NID, N>),
+
+    /// Remove nodes from membership.
+    ///
+    /// If a node is still a voter, it returns
+    /// [`error::LearnerNotFound`](`crate::error::LearnerNotFound`) error.
     RemoveNodes(BTreeSet<NID>),
+
+    /// Replace all nodes with a new set.
+    ///
+    /// Every voter has to have a corresponding node in the new
+    /// set, otherwise it returns [`error::LearnerNotFound`](`crate::error::LearnerNotFound`) error.
     ReplaceAllNodes(BTreeMap<NID, N>),
 }
 

openraft/src/membership/change_handler.rs

@@ -99,7 +99,7 @@ mod tests {
     #[test]
     fn test_apply_not_committed() -> anyhow::Result<()> {
         let new = || MembershipState::new(effmem(2, 2, m1()), effmem(3, 4, m123_345()));
-        let res = new().change_handler().apply(ChangeMembers::AddVoter(btreeset! {1}), false);
+        let res = new().change_handler().apply(ChangeMembers::AddVoterIds(btreeset! {1}), false);
 
         assert_eq!(
             Err(ChangeMembershipError::InProgress(InProgress {
@@ -115,7 +115,7 @@ mod tests {
     #[test]
     fn test_apply_empty_voters() -> anyhow::Result<()> {
         let new = || MembershipState::new(effmem(3, 4, m1()), effmem(3, 4, m1()));
-        let res = new().change_handler().apply(ChangeMembers::RemoveVoter(btreeset! {1}), false);
+        let res = new().change_handler().apply(ChangeMembers::RemoveVoters(btreeset! {1}), false);
 
         assert_eq!(Err(ChangeMembershipError::EmptyMembership(EmptyMembership {})), res);
 
@@ -125,7 +125,7 @@ mod tests {
     #[test]
     fn test_apply_learner_not_found() -> anyhow::Result<()> {
         let new = || MembershipState::new(effmem(3, 4, m1()), effmem(3, 4, m1()));
-        let res = new().change_handler().apply(ChangeMembers::AddVoter(btreeset! {2}), false);
+        let res = new().change_handler().apply(ChangeMembers::AddVoterIds(btreeset! {2}), false);
 
         assert_eq!(
             Err(ChangeMembershipError::LearnerNotFound(LearnerNotFound { node_id: 2 })),
@@ -140,14 +140,14 @@ mod tests {
         let new = || MembershipState::new(effmem(3, 4, m12()), effmem(3, 4, m123_345()));
 
         // Do not leave removed voters as learner
-        let res = new().change_handler().apply(ChangeMembers::RemoveVoter(btreeset! {1,2}), false);
+        let res = new().change_handler().apply(ChangeMembers::RemoveVoters(btreeset! {1,2}), false);
         assert_eq!(
             Ok(Membership::new(vec![btreeset! {3,4,5}], btreemap! {3=>(),4=>(),5=>()})),
             res
         );
 
         // Leave removed voters as learner
-        let res = new().change_handler().apply(ChangeMembers::RemoveVoter(btreeset! {1,2}), true);
+        let res = new().change_handler().apply(ChangeMembers::RemoveVoters(btreeset! {1,2}), true);
         assert_eq!(
             Ok(Membership::new(
                 vec![btreeset! {3,4,5}],

openraft/src/membership/membership.rs

@@ -402,14 +402,22 @@ where
     ) -> Result<Self, ChangeMembershipError<NID>> {
         tracing::debug!(change = debug(&change), "{}", func_name!());
 
-        let last = self.get_joint_config().last().unwrap();
+        let last = self.get_joint_config().last().unwrap().clone();
 
         let new_membership = match change {
-            ChangeMembers::AddVoter(add_voter_ids) => {
+            ChangeMembers::AddVoterIds(add_voter_ids) => {
                 let new_voter_ids = last.union(&add_voter_ids).copied().collect::<BTreeSet<_>>();
                 self.next_coherent(new_voter_ids, retain)
             }
-            ChangeMembers::RemoveVoter(remove_voter_ids) => {
+            ChangeMembers::AddVoters(add_voters) => {
+                // Add nodes without overriding existent
+                self.nodes = Self::extend_nodes(self.nodes, &add_voters);
+
+                let add_voter_ids = add_voters.keys().copied().collect::<BTreeSet<_>>();
+                let new_voter_ids = last.union(&add_voter_ids).copied().collect::<BTreeSet<_>>();
+                self.next_coherent(new_voter_ids, retain)
+            }
+            ChangeMembers::RemoveVoters(remove_voter_ids) => {
                 let new_voter_ids = last.difference(&remove_voter_ids).copied().collect::<BTreeSet<_>>();
                 self.next_coherent(new_voter_ids, retain)
             }
@@ -480,7 +488,7 @@ mod tests {
 
         // Add: no such learner
         {
-            let res = m().change(ChangeMembers::AddVoter(btreeset! {4}), true);
+            let res = m().change(ChangeMembers::AddVoterIds(btreeset! {4}), true);
             assert_eq!(
                 Err(ChangeMembershipError::LearnerNotFound(LearnerNotFound { node_id: 4 })),
                 res
@@ -489,7 +497,7 @@ mod tests {
 
         // Add: ok
         {
-            let res = m().change(ChangeMembers::AddVoter(btreeset! {3}), true);
+            let res = m().change(ChangeMembers::AddVoterIds(btreeset! {3}), true);
             assert_eq!(
                 Ok(Membership::<u64, ()> {
                     configs: vec![btreeset! {1,2}, btreeset! {1,2,3}],
@@ -499,9 +507,21 @@ mod tests {
             );
         }
 
+        // AddVoters
+        {
+            let res = m().change(ChangeMembers::AddVoters(btreemap! {5=>()}), true);
+            assert_eq!(
+                Ok(Membership::<u64, ()> {
+                    configs: vec![btreeset! {1,2}, btreeset! {1,2,5}],
+                    nodes: btreemap! {1=>(),2=>(),3=>(),5=>()}
+                }),
+                res
+            );
+        }
+
         // Remove: no such voter
         {
-            let res = m().change(ChangeMembers::RemoveVoter(btreeset! {5}), true);
+            let res = m().change(ChangeMembers::RemoveVoters(btreeset! {5}), true);
             assert_eq!(
                 Ok(Membership::<u64, ()> {
                     configs: vec![btreeset! {1,2}],
@@ -513,13 +533,13 @@ mod tests {
 
         // Remove: become empty
         {
-            let res = m().change(ChangeMembers::RemoveVoter(btreeset! {1,2}), true);
+            let res = m().change(ChangeMembers::RemoveVoters(btreeset! {1,2}), true);
             assert_eq!(Err(ChangeMembershipError::EmptyMembership(EmptyMembership {})), res);
         }
 
         // Remove: OK retain
         {
-            let res = m().change(ChangeMembers::RemoveVoter(btreeset! {1}), true);
+            let res = m().change(ChangeMembers::RemoveVoters(btreeset! {1}), true);
             assert_eq!(
                 Ok(Membership::<u64, ()> {
                     configs: vec![btreeset! {1,2}, btreeset! {2}],
@@ -531,7 +551,7 @@ mod tests {
 
         // Remove: OK, not retain; learner not removed
         {
-            let res = m().change(ChangeMembers::RemoveVoter(btreeset! {1}), false);
+            let res = m().change(ChangeMembers::RemoveVoters(btreeset! {1}), false);
             assert_eq!(
                 Ok(Membership::<u64, ()> {
                     configs: vec![btreeset! {1,2}, btreeset! {2}],
@@ -547,7 +567,7 @@ mod tests {
                 configs: vec![btreeset! {1,2}, btreeset! {2}],
                 nodes: btreemap! {1=>(),2=>(),3=>()},
             };
-            let res = mem.change(ChangeMembers::RemoveVoter(btreeset! {1}), false);
+            let res = mem.change(ChangeMembers::RemoveVoters(btreeset! {1}), false);
             assert_eq!(
                 Ok(Membership::<u64, ()> {
                     configs: vec![btreeset! {2}],

openraft/tests/membership/t16_change_membership_cases.rs

@@ -244,7 +244,7 @@ async fn change_from_to(old: BTreeSet<MemNodeId>, change_members: BTreeSet<MemNo
 /// Test change-membership by adding voters.
 #[tracing::instrument(level = "debug")]
 async fn change_by_add(old: BTreeSet<MemNodeId>, add: &[MemNodeId]) -> anyhow::Result<()> {
-    let change = ChangeMembers::AddVoter(add.iter().copied().collect());
+    let change = ChangeMembers::AddVoterIds(add.iter().copied().collect());
 
     let mes = format!("from {:?} {:?}", old, change);
 
@@ -312,7 +312,7 @@ async fn change_by_add(old: BTreeSet<MemNodeId>, add: &[MemNodeId]) -> anyhow::R
 
 #[tracing::instrument(level = "debug")]
 async fn change_by_remove(old: BTreeSet<MemNodeId>, remove: &[MemNodeId]) -> anyhow::Result<()> {
-    let change = ChangeMembers::RemoveVoter(remove.iter().copied().collect());
+    let change = ChangeMembers::RemoveVoters(remove.iter().copied().collect());
 
     let mes = format!("from {:?} {:?}", old, change);