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

[Feature]: Membership change #306

Closed
4 tasks done
themanforfree opened this issue Jun 5, 2023 · 3 comments
Closed
4 tasks done

[Feature]: Membership change #306

themanforfree opened this issue Jun 5, 2023 · 3 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@themanforfree
Copy link
Collaborator

themanforfree commented Jun 5, 2023

Description about the feature

Membership change has two different situations

  1. Change only one node at a time, in this case there must be duplicate nodes in majority of the old and new configurations, it is impossible to elect two leaders at the same time, we can handle this situation simply, after the new configuration is committed, the new configuration can be used to make decisions.
    image
  2. Change multiple nodes at a time, the following situation may occur. nodes using different configurations may have two disjoint majorities, which means that two leaders may be elected, the solution is introduce an intermediate state, after the leader receives the configuration change request, the new configuration node set is C_new, and the old configuration node set is C_old. After C_new is committed, it enters the joint consensus state. When an election occurs, C_old and C_new jointly decide, and then the leader creates an empty Configuration change log, after this log committed, cluster can use C_new alone to make decisions, and the cluster completes this configuration change node, and this configuration change is complete.
    image

Learner
When adding a new node to the cluster, the new node does not have the current data of the cluster. At this time, the leader needs to sync the data to the new node. This process may take a while, which will increase the risk of cluster unavailability. Therefore, the Learner node is introduced. Nodes only sync logs, do not participate in voting, wait until the Leaner node catches up with the progress of the leader, and then promote it to a normal node.

I plan to implement the membership change in three stages

  1. implement single node change, and prepare some struct for multiple nodes change.
  2. implement Learner node.
  3. implement multiple nodes change.

Tests

  • The member id and cluster id calculated by all nodes should be consistent when Initialize the cluster.
  • The new node should pull member IDs directly from the existing cluster and use them, and should not calculate IDs by itself.
  • Client should use new configuration after cluster member change

Related papers

  1. https://raft.github.io/raft.pdf
  2. https://web.stanford.edu/~ouster/cgi-bin/papers/OngaroPhD.pdf

Code of Conduct

  • I agree to follow this project's Code of Conduct
@themanforfree themanforfree added the enhancement New feature or request label Jun 5, 2023
@Phoenix500526 Phoenix500526 added this to the Release v0.5.0 milestone Jun 5, 2023
@themanforfree
Copy link
Collaborator Author

themanforfree commented Jun 9, 2023

More details of stage 1

@themanforfree
Copy link
Collaborator Author

Problem

At first, I planned to reuse the processing logic for command directly as follows, but the ER and ASR is defined externally, And ConfChange is an internal behavior of curp, so we can't make ConfChange produce externally defined ERs and ASRs.

...
TaskType::SpecExe(entry, pre_err) => {
    let er = if let Some(err_msg) = pre_err {
        Err(err_msg)
    } else {
        match entry.entry_data {
            EntryData::Command(ref cmd) => ce
                .execute(cmd, entry.index)
                .await
                .map_err(|e| e.to_string()),
            EntryData::ConfChange(_) => todo!(),
        }
    };
    let er_ok = er.is_ok();
    cb.write().insert_er(entry.id(), er);
    if !er_ok {
        sp.lock().remove(entry.id());
        let _ig = ucp.lock().remove(entry.id());
    }
    debug!(
        "{id} cmd({}) is speculatively executed, exe status: {er_ok}",
        entry.id()
    );
    er_ok
}
...

Solution

...
TaskType::SpecExe(entry, pre_err) => match entry.entry_data {
    EntryData::Command(ref cmd) => {
        let er = if let Some(err_msg) = pre_err {
            Err(err_msg)
        } else {
            ce.execute(cmd, entry.index)
                .await
                .map_err(|e| e.to_string())
        };
        let er_ok = er.is_ok();
        cb.write().insert_er(entry.id(), er);
        if !er_ok {
            sp.lock().remove(entry.id());
            let _ig = ucp.lock().remove(entry.id());
        }
        debug!(
            "{id} cmd({}) is speculatively executed, exe status: {er_ok}",
            entry.id()
        );
        er_ok
    }
    EntryData::ConfChange(_) => true,
}
...

In fact, cmd_worker only needs to return a bool value to indicate whether the task is running successfully, and does not care about the internal specific execution process, so there is no need for ConfChange to produce ER, just return a bool

@Phoenix500526
Copy link
Collaborator

Problem

At first, I planned to reuse the processing logic for command directly as follows, but the ER and ASR is defined externally, And ConfChange is an internal behavior of curp, so we can't make ConfChange produce externally defined ERs and ASRs.

...
TaskType::SpecExe(entry, pre_err) => {
    let er = if let Some(err_msg) = pre_err {
        Err(err_msg)
    } else {
        match entry.entry_data {
            EntryData::Command(ref cmd) => ce
                .execute(cmd, entry.index)
                .await
                .map_err(|e| e.to_string()),
            EntryData::ConfChange(_) => todo!(),
        }
    };
    let er_ok = er.is_ok();
    cb.write().insert_er(entry.id(), er);
    if !er_ok {
        sp.lock().remove(entry.id());
        let _ig = ucp.lock().remove(entry.id());
    }
    debug!(
        "{id} cmd({}) is speculatively executed, exe status: {er_ok}",
        entry.id()
    );
    er_ok
}
...

Solution

...
TaskType::SpecExe(entry, pre_err) => match entry.entry_data {
    EntryData::Command(ref cmd) => {
        let er = if let Some(err_msg) = pre_err {
            Err(err_msg)
        } else {
            ce.execute(cmd, entry.index)
                .await
                .map_err(|e| e.to_string())
        };
        let er_ok = er.is_ok();
        cb.write().insert_er(entry.id(), er);
        if !er_ok {
            sp.lock().remove(entry.id());
            let _ig = ucp.lock().remove(entry.id());
        }
        debug!(
            "{id} cmd({}) is speculatively executed, exe status: {er_ok}",
            entry.id()
        );
        er_ok
    }
    EntryData::ConfChange(_) => true,
}
...

In fact, cmd_worker only needs to return a bool value to indicate whether the task is running successfully, and does not care about the internal specific execution process, so there is no need for ConfChange to produce ER, just return a bool

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants