raftstore: Introduce force leader state by Connor1996 · Pull Request #11932 · tikv/tikv

Connor1996 · 2022-02-07T09:09:44Z

What is changed and how it works?

Issue Number: Close #6107

What's Changed:

Part of #10483, Introduce force leader state to advance commit index even if the quorum is not formed. This is used for online unsafe recover.

Related changes

PR to update pingcap/docs/pingcap/docs-cn:
PR to update pingcap/tidb-ansible:
Need to cherry-pick to the release branch

Check List

Tests

Unit test
Integration test

Release note

None

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

ti-chi-bot · 2022-02-07T09:09:46Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

BusyJay
NingLin-P

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Details

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

Connor1996 · 2022-02-07T09:35:06Z

PTAL too @v01dstar

v01dstar

Losing leadership in election timeout while check_quorum = true should be taken into consideration?

Connor1996 · 2022-02-08T04:06:21Z

Losing leadership in election timeout while check_quorum = true should be taken into consideration?

Yes, do you want to add an API to change that? If not, I'll do it.

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

components/error_code/src/raftstore.rs

components/raftstore/src/store/fsm/peer.rs

NingLin-P · 2022-02-14T13:16:53Z

I think there is a remaining problem about the learner role with the force leader mechanism, see #6107 (comment)

Connor1996 · 2022-02-15T09:50:16Z

I think there is a remaining problem about the learner role with the force leader mechanism, see #6107 (comment)

Good point. Adding a test to see if the learner can call became_leader successfully.

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

Connor1996 · 2022-02-15T12:29:38Z

I think there is a remaining problem about the learner role with the force leader mechanism, see #6107 (comment)

Add a case to force the learner to be the leader. Seems no problem, as we can promote it to be a voter once in force leader state.

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

…eader

NingLin-P · 2022-02-15T13:24:27Z

Add a case to force the learner to be the leader. Seems no problem, as we can promote it to be a voter once in force leader state.

It may more than just whether a learner can become leader, I worry about what may happen after a learner became leader as we always assume that only voter can become leader. /cc @BusyJay

components/raftstore/src/store/peer.rs

components/raftstore/src/store/fsm/peer.rs

tests/integrations/raftstore/test_unsafe_recovery.rs

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

components/raftstore/src/store/fsm/peer.rs

BusyJay · 2022-04-12T07:55:21Z

components/raftstore/src/store/fsm/peer.rs

+            .raft_group
+            .raft
+            .prs()
+            .has_quorum(&expected_alive_voter)


Supposing (a, b, c, d, e) -> (a, b, c, d, f) -> (a, b, c, g, f), and a, b, g, f all fail. c is isolated during the whole time. It may think it still has quorum (c, d, e).

Emmm.... In this case, force leader can't be performed as well, as we add pre-force-leader check expecting all votes from expected alive voters

Indeed. Perhaps what we should do is check if there is rejection instead of expecting all approvals.

The logic of forwarding the commit index also has the same issue. I'd like to leverage the mechanism introduced by this PR #7636 to filter the peers.

It's an extreme corner case, I'd record an issue now and fix it after the whole procedure is workable.

add a TODO item in #10483

components/raftstore/src/store/fsm/peer.rs

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

Connor1996 · 2022-04-12T11:28:50Z

/merge

ti-chi-bot · 2022-04-12T11:28:50Z

@Connor1996: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

ti-chi-bot · 2022-04-12T11:28:53Z

This pull request has been accepted and is ready to merge.

Details

Commit hash: cd6f7f2

Connor1996 · 2022-04-12T11:59:46Z

/test

close tikv#6107, ref tikv#10483 Signed-off-by: Connor1996 <zbk602423539@gmail.com>

* raftstore: Introduce force leader state (tikv#11932) close tikv#6107, ref tikv#10483 Signed-off-by: Connor1996 <zbk602423539@gmail.com> * raftstore: Wait ticks for hibernated peer when doing force leader (tikv#12364) ref tikv#10483 Force leader is rejected on a peer who is already a leader. For the hibernated leader, it doesn't step down to follower when quorum is missing due to not ticking election. So wait ticks in force leader process for hibernated peers to make sure election ticking is performed. Signed-off-by: Connor1996 <zbk602423539@gmail.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io> * raftstore: Make unsafe recovery wait apply cover snapshot apply cases ref tikv#10483 (tikv#12308) ref tikv#10483 Signed-off-by: v01dstar <yang.zhang@pingcap.com> * raftstore: Execute recovery plan via raft (tikv#12022) Signed-off-by: Connor1996 <zbk602423539@gmail.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io> Co-authored-by: Yang Zhang <yang.zhang@pingcap.com>

Connor1996 added 4 commits October 12, 2021 15:37

force leader

85f6f95

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

add test

1b89911

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

fix test

1b05de2

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

make format

de5d366

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 7, 2022

Connor1996 added 2 commits February 7, 2022 17:26

fix clippy

2088ef1

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

Merge remote-tracking branch 'upstream' into force-leader

256bd69

Connor1996 requested review from BusyJay and NingLin-P and removed request for NingLin-P February 7, 2022 09:32

fix build

7caf2f3

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

v01dstar reviewed Feb 7, 2022

View reviewed changes

move place

4322016

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

Connor1996 mentioned this pull request Feb 9, 2022

Support online recovery for Raft majority failure #10483

Closed

44 tasks

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 11, 2022

5kbpers reviewed Feb 14, 2022

View reviewed changes

components/error_code/src/raftstore.rs Outdated Show resolved Hide resolved

5kbpers reviewed Feb 14, 2022

View reviewed changes

components/raftstore/src/store/fsm/peer.rs Outdated Show resolved Hide resolved

components/raftstore/src/store/fsm/peer.rs Outdated Show resolved Hide resolved

add learner case

adf8164

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

Connor1996 added 2 commits February 15, 2022 20:46

update kvproto

fe57b42

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

Merge branch 'master' of https://github.com/pingcap/tikv into force-l…

a062bc0

…eader

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 15, 2022

Connor1996 force-pushed the force-leader branch from 991d573 to 720ed5b Compare April 11, 2022 10:03

NingLin-P approved these changes Apr 11, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Apr 11, 2022

BusyJay reviewed Apr 11, 2022

View reviewed changes

Connor1996 added 5 commits April 11, 2022 21:14

rename

cbd06d4

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

clean test

41987a2

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

use on_force_leader_fail

9830859

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

stable test

603c834

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

use failed_stores instead of alive_voter and add a test case

a685d66

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

BusyJay reviewed Apr 12, 2022

View reviewed changes

Connor1996 added 3 commits April 12, 2022 17:27

address comment

45c96c7

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

Merge remote-tracking branch 'upstream/master' into force-leader

50c3121

fix clippy

8231155

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

Connor1996 requested a review from BusyJay April 12, 2022 09:32

stablize tests

cd6f7f2

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

BusyJay approved these changes Apr 12, 2022

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Apr 12, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Apr 12, 2022

ti-chi-bot merged commit b602719 into tikv:master Apr 12, 2022

Connor1996 added a commit to Connor1996/tikv that referenced this pull request May 18, 2022

raftstore: Introduce force leader state (tikv#11932)

3cbd3fd

close tikv#6107, ref tikv#10483 Signed-off-by: Connor1996 <zbk602423539@gmail.com>

Connor1996 added a commit to Connor1996/tikv that referenced this pull request May 18, 2022

raftstore: Introduce force leader state (tikv#11932)

e332ae4

close tikv#6107, ref tikv#10483 Signed-off-by: Connor1996 <zbk602423539@gmail.com>

This was referenced May 18, 2022

Cherry pick unsafe recovery related PRs pingcap/tidb-engine-ext#62

Open

Cherry pick unsafe recovery related PRs pingcap/tidb-engine-ext#63

Merged

overvenus mentioned this pull request Jul 12, 2023

[Dynamic Region] Support online recovery for raftstore-v2 #15108

Closed

6 tasks

Conversation

Connor1996 commented Feb 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is changed and how it works?

Related changes

Check List

Release note

Uh oh!

ti-chi-bot commented Feb 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Connor1996 commented Feb 7, 2022

Uh oh!

v01dstar left a comment

Choose a reason for hiding this comment

Uh oh!

Connor1996 commented Feb 8, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

NingLin-P commented Feb 14, 2022

Uh oh!

Connor1996 commented Feb 15, 2022

Uh oh!

Connor1996 commented Feb 15, 2022

Uh oh!

NingLin-P commented Feb 15, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

BusyJay Apr 12, 2022

Choose a reason for hiding this comment

Uh oh!

Connor1996 Apr 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BusyJay Apr 12, 2022

Choose a reason for hiding this comment

Uh oh!

Connor1996 Apr 12, 2022

Choose a reason for hiding this comment

Uh oh!

Connor1996 Apr 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Connor1996 commented Apr 12, 2022

Uh oh!

ti-chi-bot commented Apr 12, 2022

Uh oh!

ti-chi-bot commented Apr 12, 2022

Uh oh!

Connor1996 commented Apr 12, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Connor1996 commented Feb 7, 2022 •

edited

Loading

ti-chi-bot commented Feb 7, 2022 •

edited

Loading

Connor1996 Apr 12, 2022 •

edited

Loading

Connor1996 Apr 12, 2022 •

edited

Loading