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.

Sign up for GitHub

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

Jump to bottom

unsafe recover: multiple stage recovery plan generation #4704

Merged
merged 56 commits into from
May 18, 2022
Merged

unsafe recover: multiple stage recovery plan generation #4704

merged 56 commits into from
May 18, 2022

Conversation

Connor1996
Copy link
Member

@Connor1996 Connor1996 commented Mar 3, 2022 •
edited
Loading

Signed-off-by: Connor1996 [email protected]

What problem does this PR solve?

Issue Number: ref #4887

What is changed and how it works?

Simplify the original recovery plan, and introduce two stages for recovery:

  1. force leader stage: force one peer of the unhealthy region to be leader to make all the peers catch up with the latest log
  2. recover stage: dispatch recover operations including remove-peer, create-region and tombstone-peer

Output would be like this:
image

Check List

Tests

  • Unit test

Related changes

Release note

Introduce multiple stage online unsafe recovery

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Mar 3, 2022 •
edited
Loading

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • JmPotato
  • rleungx

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.

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

@ti-chi-bot ti-chi-bot added the release-note-none Denotes a PR that doesn't merit a release note. label Mar 3, 2022
@ti-chi-bot ti-chi-bot requested review from nolouch and rleungx March 3, 2022 13:11
@Connor1996 Connor1996 changed the title generate plan unsafe recover: simpliy the recovery plan Mar 3, 2022
@Connor1996
Copy link
Member Author

tests are to be added

@Connor1996 Connor1996 mentioned this pull request Mar 3, 2022
Closed
44 tasks
@ti-chi-bot ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 3, 2022
rleungx
rleungx reviewed Mar 4, 2022
View reviewed changes
@Connor1996
generate plan
66d2bf0 
Signed-off-by: Connor1996 <[email protected]>
@Connor1996
change kvproto
6338999 
Signed-off-by: Connor1996 <[email protected]>
@Connor1996
refine
b952ffe 
Signed-off-by: Connor1996 <[email protected]>
@ti-chi-bot ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 22, 2022
@Connor1996
change test
e907704 
Signed-off-by: Connor1996 <[email protected]>
@ti-chi-bot ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 30, 2022
@Connor1996 Connor1996 changed the title unsafe recover: simpliy the recovery plan unsafe recover: simplify the recovery plan Apr 12, 2022
@Connor1996
fix select leader
9243e7e 
Signed-off-by: Connor1996 <[email protected]>
@Connor1996
update kvproto
a9b64d2 
Signed-off-by: Connor1996 <[email protected]>
@Connor1996
seperate create empty region stage
4c1d145 
Signed-off-by: Connor1996 <[email protected]>
@Connor1996
update kvproto and add log
90a1d82 
Signed-off-by: Connor1996 <[email protected]>
@Connor1996
refine show output and test
078f33c 
Signed-off-by: Connor1996 <[email protected]>
@Connor1996
Copy link
Member Author

Tests to be added, It's ready for review. PTAL @rleungx @v01dstar

rleungx
rleungx reviewed Apr 25, 2022
View reviewed changes
@Connor1996 Connor1996 changed the title unsafe recover: simplify the recovery plan unsafe recover: multiple stage recovery plan generation Apr 25, 2022
@Connor1996
address comment
ae78fe1 
Signed-off-by: Connor1996 <[email protected]>
@ti-chi-bot ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 11, 2022
@ti-chi-bot ti-chi-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels May 12, 2022
@ti-chi-bot ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 15, 2022
v01dstar
v01dstar approved these changes May 16, 2022
View reviewed changes
Copy link
Contributor

@v01dstar v01dstar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ti-chi-bot
Copy link
Member

@v01dstar: Thanks for your review. The bot only counts approvals from reviewers and higher roles in list, but you're still welcome to leave your comments.

In response to this:

LGTM

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 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 May 18, 2022
@JmPotato
Copy link
Member

/merge

@ti-chi-bot
Copy link
Member

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

/run-all-tests

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
Copy link
Member

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

Commit hash: 4c286fe

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label May 18, 2022
@ti-chi-bot ti-chi-bot merged commit 9d31312 into tikv:master May 18, 2022
@v01dstar v01dstar mentioned this pull request May 20, 2022
Merged
ti-chi-bot added a commit that referenced this pull request May 20, 2022
@v01dstar @ti-chi-bot
Fix data race,ref #4857 (#4996)
aafc77d 
ref #4704, fix #4704, close #4857, ref #4881

Reorder the code logic, and acquire the lock before modifying the cluster to avoid data race.

Signed-off-by: v01dstar <[email protected]>

Co-authored-by: Ti Chi Robot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JmPotato JmPotato JmPotato approved these changes

@v01dstar v01dstar v01dstar approved these changes

@rleungx rleungx rleungx approved these changes

@nolouch nolouch Awaiting requested review from nolouch

Assignees
No one assigned
Labels
release-note Denotes a PR that will be considered when it comes time to generate release notes. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Connor1996 @ti-chi-bot @JmPotato @v01dstar @rleungx