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

Refactor: fix handling of non-voter leader #924

Merged
merged 2 commits into from
Nov 7, 2023

Conversation

drmingdrmer
Copy link
Member

@drmingdrmer drmingdrmer commented Nov 6, 2023

Changelog

Refactor: fix handling of non-voter leader

Openraft does not require Leader/Candidate to be a voter.
Before this commit, Openraft's handling of non-voter leader is
inconsistent:

For a node that has a vote proposed by this node and is committed, and is not a voter:

Since this commit,
calc_server_state() and is_leader() returns consistent result according to the following table:

For node-2:

  • Node-2 with vote (term=1, node_id=2, committed=true):

    • is a leader if it is present in config, either a voter or non-voter.
    • is a learner if it is absent in config.
  • Node-2 with vote (term=1, node_id=2, committed=false):

    • is a candidate if it is present in config, either a voter or non-voter.
    • is a learner if it is absent in config.
  • Node-3 with vote (term=1, node_id=99, committed=false|true):

    • is a follower if it is a voter in config,
    • is a learner if it is a non-voter or absent in config.
vote \ membership Voter Non-voter Absent
(term=1, node_id=2, committed=true) leader leader learner
(term=1, node_id=2, committed=false) candidate candidate learner
(term=1, node_id=99, committed=true) follower learner learner
(term=1, node_id=99, committed=false) follower learner learner

This change is Reviewable

Openraft does not require Leader/Candidate to be a voter.
Before this commit, Openraft's handling of non-voter leader is
inconsistent:

For a node that has a vote proposed by this node and is committed, and is not a voter:

- [`calc_server_state()`](https://github.com/datafuselabs/openraft/blob/97254a31c673e52429ee7187de96fd2ea2a5ce98/openraft/src/raft_state/mod.rs#L323) returns `Learner`.
- But [`is_leader()`](https://github.com/datafuselabs/openraft/blob/97254a31c673e52429ee7187de96fd2ea2a5ce98/openraft/src/raft_state/mod.rs#L353) returns true.

Since this commit,
`calc_server_state()` and `is_leader()` returns consistent result according to the following table:

For node-2:

- Node-2 with vote `(term=1, node_id=2, committed=true)`:
  - is a leader if it is **present** in config, either a voter or non-voter.
  - is a learner if it is **absent** in config.

- Node-2 with vote `(term=1, node_id=2, committed=false)`:
  - is a candidate if it is **present** in config, either a voter or non-voter.
  - is a learner if it is **absent** in config.

- Node-3 with vote `(term=1, node_id=99, committed=false|true)`:
  - is a follower if it is a **voter** in config,
  - is a learner if it is a **non-voter** or **absent** in config.

| vote \ membership                     | Voter     | Non-voter | Absent  |
|---------------------------------------|-----------|-----------|---------|
| (term=1, node_id=2, committed=true)   | leader    | leader    | learner |
| (term=1, node_id=2, committed=false)  | candidate | candidate | learner |
| (term=1, node_id=99, committed=true)  | follower  | learner   | learner |
| (term=1, node_id=99, committed=false) | follower  | learner   | learner |

- Fix: databendlabs#920
…ase 'add_learner_non_blocking'.

In this case, a replication failure status is supposed to be reported to
the matrix. But sometimes it times out before the failure report can be
delivered.

To address this issue, just extend the time out duration.
@drmingdrmer drmingdrmer merged commit 503fa49 into databendlabs:main Nov 7, 2023
24 checks passed
@drmingdrmer drmingdrmer deleted the 44-fix-920 branch November 7, 2023 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

error : it has to be a leader!!!
1 participant