raftstore: Make unsafe recovery wait apply cover snapshot apply cases ref #10483#12308

Merged

ti-chi-bot merged 16 commits intotikv:masterfrom

v01dstar:wait_snapshot_apply

Apr 15, 2022

Member

v01dstar commented Mar 30, 2022 •

edited

Loading

Signed-off-by: v01dstar yang.zhang@pingcap.com

What is changed and how it works?

Issue Number: ref #10483

What's Changed:

Raft snapshot apply has a different code path to update the apply index, compare to other types of Raft entry which update apply index in raftstore/store/fsm/peer.rs:on_apply_res(). This path was not covered in the initial implementation of "unsafe recovery wait apply", and this PR fixes it.

Related changes

PR that adds the initial implementation of unsafe recovery wait apply: #11716

Check List

Tests

Integration test

Side effects

N/A

Release note

None

Member

ti-chi-bot commented Mar 30, 2022 •

edited

Loading

[REVIEW NOTIFICATION]

This pull request has been approved by:

BusyJay
Connor1996

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.

ti-chi-bot added release-note-none contribution size/L labels

Member Author

v01dstar commented Mar 31, 2022

/assign @Connor1996

ti-chi-bot assigned Connor1996

Connor1996 removed their assignment

Connor1996 self-requested a review

March 31, 2022 07:20

Connor1996 reviewed

View reviewed changes

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

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

Connor1996 reviewed

View reviewed changes

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

ti-chi-bot added size/XL and removed size/L labels

Member Author

v01dstar commented Apr 7, 2022

ti-chi-bot requested a review from BusyJay

April 7, 2022 01:42

BusyJay reviewed

View reviewed changes

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

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

Connor1996 reviewed

View reviewed changes

Member

Connor1996 left a comment

rest LGTM

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

tests/failpoints/cases/test_unsafe_recovery.rs Outdated Show resolved Hide resolved

ti-chi-bot added size/L size/XL and removed size/XL size/L labels

BusyJay reviewed

View reviewed changes

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

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

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

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

BusyJay reviewed

View reviewed changes

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

ti-chi-bot added size/L and removed size/XL labels

v01dstar force-pushed the wait_snapshot_apply branch from ec99f00 to a60f19a Compare

April 12, 2022 01:56

Connor1996 reviewed

View reviewed changes

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

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

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

Connor1996 reviewed

View reviewed changes

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

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

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

Connor1996 mentioned this pull request

Support online recovery for Raft majority failure #10483

Closed

44 tasks

ti-chi-bot added the needs-rebase label


          Make unsafe recovery wait apply cover snapshot apply cases ref tikv#1…

08ad218

…0483

Signed-off-by: v01dstar <yang.zhang@pingcap.com>

Member

Connor1996 commented Apr 14, 2022

/test

Member

Connor1996 commented Apr 14, 2022

/merge

Member

ti-chi-bot commented Apr 14, 2022

@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.

Member

ti-chi-bot commented Apr 14, 2022

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

Details

Commit hash: 0f1c747

ti-chi-bot added the status/can-merge label

Member

Connor1996 commented Apr 14, 2022

/test

7 similar comments

Member

Connor1996 commented Apr 14, 2022

/test

Member

Connor1996 commented Apr 14, 2022

/test

Member

Connor1996 commented Apr 14, 2022

/test

Member

Connor1996 commented Apr 14, 2022

/test

Member

Connor1996 commented Apr 14, 2022

/test

Member

Connor1996 commented Apr 14, 2022

/test

Member

Connor1996 commented Apr 14, 2022

/test

Member

BusyJay commented Apr 14, 2022

test_unsafe_recover_wait_for_snapshot_apply keep retrying will just bring more unstable case to master.

Member

Connor1996 commented Apr 14, 2022

test_unsafe_recover_wait_for_snapshot_apply keep retrying will just bring more unstable case to master.

previous failure isn't due to test_unsafe_recover_wait_for_snapshot_apply


          Stablize test

fedc098

Signed-off-by: v01dstar <yang.zhang@pingcap.com>

ti-chi-bot removed the status/can-merge label

Member Author

v01dstar commented Apr 14, 2022

test_unsafe_recover_wait_for_snapshot_apply keep retrying will just bring more unstable case to master.

previous failure isn't due to test_unsafe_recover_wait_for_snapshot_apply

I suspect that the test failed due to insufficient wait for the raft log GC, thus the snapshot path was not triggered at all. Added some new code to coordinate the data insertion and raft log GC, hope this can make the test bit more stable.

Member

Connor1996 commented Apr 15, 2022

/merge

Member

ti-chi-bot commented Apr 15, 2022

@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.

Member

ti-chi-bot commented Apr 15, 2022

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

Details

Commit hash: fedc098

ti-chi-bot added the status/can-merge label

ti-chi-bot merged commit 45fcb08 into tikv:master

v01dstar deleted the wait_snapshot_apply branch

April 15, 2022 02:44

Connor1996 pushed a commit to Connor1996/tikv that referenced this pull request


          raftstore: Make unsafe recovery wait apply cover snapshot apply cases…

04e4724

… ref tikv#10483 (tikv#12308)

ref tikv#10483

Signed-off-by: v01dstar <yang.zhang@pingcap.com>

Connor1996 pushed a commit to Connor1996/tikv that referenced this pull request


          raftstore: Make unsafe recovery wait apply cover snapshot apply cases…

7acedff

… ref tikv#10483 (tikv#12308)

ref tikv#10483

Signed-off-by: v01dstar <yang.zhang@pingcap.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

CalvinNeo pushed a commit to pingcap/tidb-engine-ext that referenced this pull request


          Raftstore proxy 6.0 try (#63)

df26083

* 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>

overvenus mentioned this pull request

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

Closed

6 tasks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contribution release-note-none size/XL status/can-merge status/LGT2