raftstore: `campaign` newly created regions in time after `Split` by LykxSassinator · Pull Request #17625 · tikv/tikv

LykxSassinator · 2024-10-10T07:09:08Z

What is changed and how it works?

Issue Number: Close #12410 and #17602.

What's Changed:

As issues #12410 and #17602 shows, the original design of CmdEpochChecker allows the BatchSplit or Split proposals even though the last TransferLeader is still on-going, making the newly created region cannot campaign leader immediately. The root cause of these issues is that TransferLeader does not be recorded into the proposal queue in CmdEpochChecker as it will not change the conf_ver, if the proposal queue has no conflicts admin commands.
And these issues will damage the stability of TiKV, returning 9005 errors to the Client who wanna access the relative data.

So, to tackle these issues, this pr make the campaign of the newly splitted regions triggered in time, when the leadership of the parent region is stable after on_role_changed. And the newly added progress can be reviewed from the following diagram on step 6.

This pr make the `campaign` of the newly splitted regions triggered in time, when the leadership of the parent region is stable after `on_role_changed`.

Related changes

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

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Release note

This pr introduces an supplementary rule, making `TransferLeader` mutually exclusive with other commands 
which will change the `conf_ver`, for checking the execution validity.

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

ti-chi-bot · 2024-10-10T07:09:11Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

LykxSassinator · 2024-10-10T08:53:18Z

/test pull-unit-test

components/raftstore/src/store/peer.rs

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

…ansfer leader. Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

SpadeA-Tang · 2024-10-22T05:00:04Z

If Im right, the error is caused by:

peer 1 transfer leader to peer 2
peer 1 becomes follower, peer 2 becomes candidate
on_ready_split_region
the child region will not compagin because peer 1 is not leader
the same for peer 2

So, the child region will wait election timeout.

For this, why not just change the compain condition from leader to leader/candidate?

LykxSassinator · 2024-10-22T12:27:08Z

Yep, this proposal is valid.

However, we want to make "TransferLeader" and other operations which changes the conf_ver mutually exclusive, and it's used to enforce the rule that "only one operation can exist in a region before the subsequent operation", avoiding unexpected behaviors that we do not observe right now.

So, this implementation enhance the checking of the propose_check_epoch.

glorv · 2024-10-23T02:38:57Z

For this, why not just change the compaign condition from leader to leader/candidate?

@SpadeA-Tang I thinks there are 2 ways that a follower can become candidate: 1) transfer leader, 2) election timeout. So if the following became candidate due to election timeout and also apply split at the same time, the new created region should not start a election(and may become leader) because thus there may be two leader for the same region if the old leader does not finish executing the split command.

overvenus · 2024-10-23T05:09:02Z

If Im right, the error is caused by:

peer 1 transfer leader to peer 2

peer 1 becomes follower, peer 2 becomes candidate

on_ready_split_region
the child region will not compagin because peer 1 is not leader
the same for peer 2

So, the child region will wait election timeout.

For this, why not just change the compain condition from leader to leader/candidate?

It's not enough, peer 2 may already split before it receives MsgTimeoutNow.

components/raftstore/src/store/peer.rs

Connor1996

LGTM

ti-chi-bot · 2024-11-19T09:03:29Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Connor1996, hbisheng, overvenus, SpadeA-Tang

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [Connor1996,SpadeA-Tang,overvenus]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

LykxSassinator · 2024-11-19T09:24:12Z

/test all

…kv#17625) close tikv#12410 This pr make the `campaign` of the newly splitted regions triggered in time, when the leadership of the parent region is stable after `on_role_changed`. Signed-off-by: lucasliang <nkcs_lykx@hotmail.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

ti-chi-bot · 2024-11-19T10:23:03Z

In response to a cherrypick label: new pull request created to branch release-8.5: #17846.

…7625) (#17846) close #12410 This pr make the `campaign` of the newly splitted regions triggered in time, when the leadership of the parent region is stable after `on_role_changed`. Signed-off-by: lucasliang <nkcs_lykx@hotmail.com> Co-authored-by: lucasliang <nkcs_lykx@hotmail.com>

close tikv#12410 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2024-12-11T09:52:54Z

In response to a cherrypick label: new pull request created to branch release-8.1: #17983.

…7625) (#17983) close #12410 This pr make the `campaign` of the newly splitted regions triggered in time, when the leadership of the parent region is stable after `on_role_changed`. Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io> Signed-off-by: lucasliang <nkcs_lykx@hotmail.com> Co-authored-by: lucasliang <nkcs_lykx@hotmail.com>

close tikv#12410 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2024-12-13T05:35:01Z

In response to a cherrypick label: new pull request created to branch release-7.5: #17996.

close tikv#12410 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2025-01-21T03:37:40Z

In response to a cherrypick label: new pull request created to branch release-6.5: #18148.

…7625) (#18148) close #12410 This pr make the `campaign` of the newly splitted regions triggered in time, when the leadership of the parent region is stable after `on_role_changed`. Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io> Signed-off-by: lucasliang <nkcs_lykx@hotmail.com> Co-authored-by: lucasliang <nkcs_lykx@hotmail.com>

…7625) (#17996) close #12410 This pr make the `campaign` of the newly splitted regions triggered in time, when the leadership of the parent region is stable after `on_role_changed`. Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io> Signed-off-by: lucasliang <nkcs_lykx@hotmail.com> Co-authored-by: lucasliang <nkcs_lykx@hotmail.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

LykxSassinator added 4 commits October 9, 2024 14:34

raftstore: mutually exclusive execution on admin commands.

56dce3c

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

Refactor the previous redundant designs.

8d03481

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

Polish annotations.

e685986

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

Remove redundant codes.

f6662fb

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

ti-chi-bot bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Oct 10, 2024

ti-chi-bot bot added dco-signoff: yes Indicates the PR's author has signed the dco. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 10, 2024

Polish annotations.

7ed137b

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

LykxSassinator requested review from Connor1996, SpadeA-Tang, glorv and overvenus October 10, 2024 07:30

LykxSassinator marked this pull request as ready for review October 10, 2024 07:30

ti-chi-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 10, 2024

overvenus reviewed Oct 11, 2024

View reviewed changes

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

LykxSassinator added 2 commits October 21, 2024 11:07

Merge branch 'master' into postpone_split_region

7c49d6b

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

Refactor the implementation to ensure the validation when ready to tr…

de0733e

…ansfer leader. Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

ti-chi-bot bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 21, 2024

LykxSassinator added 3 commits October 21, 2024 16:08

Fix lint errs.

ce7ee3e

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

Fix ut errs.

beeaba6

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

Fix ut errs.

cf03a9e

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

SpadeA-Tang reviewed Oct 23, 2024

View reviewed changes

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

Connor1996 approved these changes Nov 19, 2024

View reviewed changes

Merge branch 'master' into postpone_split_region

ba73a11

ti-chi-bot bot merged commit 361a8eb into tikv:master Nov 19, 2024

ti-chi-bot bot added this to the Pool milestone Nov 19, 2024

LykxSassinator added the needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. label Nov 19, 2024

LykxSassinator deleted the postpone_split_region branch November 19, 2024 10:22

ti-chi-bot mentioned this pull request Nov 19, 2024

raftstore: campaign newly created regions in time after Split (#17625) #17846

Merged

9 tasks

LykxSassinator added the needs-cherry-pick-release-8.1 Should cherry pick this PR to release-8.1 branch. label Dec 11, 2024

ti-chi-bot pushed a commit to ti-chi-bot/tikv that referenced this pull request Dec 11, 2024

This is an automated cherry-pick of tikv#17625

6b312ca

close tikv#12410 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Dec 11, 2024

raftstore: campaign newly created regions in time after Split (#17625) #17983

Merged

9 tasks

ti-chi-bot bot added the needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. label Dec 13, 2024

ti-chi-bot pushed a commit to ti-chi-bot/tikv that referenced this pull request Dec 13, 2024

This is an automated cherry-pick of tikv#17625

6b7f57f

close tikv#12410 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Dec 13, 2024

raftstore: campaign newly created regions in time after Split (#17625) #17996

Merged

9 tasks

ti-chi-bot bot removed the needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. label Dec 13, 2024

ti-chi-bot bot added the needs-cherry-pick-release-6.5 Should cherry pick this PR to release-6.5 branch. label Jan 21, 2025

ti-chi-bot pushed a commit to ti-chi-bot/tikv that referenced this pull request Jan 21, 2025

This is an automated cherry-pick of tikv#17625

3fa1a97

close tikv#12410 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Jan 21, 2025

raftstore: campaign newly created regions in time after Split (#17625) #18148

Merged

9 tasks

Conversation

LykxSassinator commented Oct 10, 2024 • 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 bot commented Oct 10, 2024

Uh oh!

LykxSassinator commented Oct 10, 2024

Uh oh!

Uh oh!

SpadeA-Tang commented Oct 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LykxSassinator commented Oct 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

glorv commented Oct 23, 2024

Uh oh!

overvenus commented Oct 23, 2024

Uh oh!

Uh oh!

Connor1996 left a comment

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot bot commented Nov 19, 2024

Uh oh!

LykxSassinator commented Nov 19, 2024

Uh oh!

ti-chi-bot commented Nov 19, 2024

Uh oh!

ti-chi-bot commented Dec 11, 2024

Uh oh!

ti-chi-bot commented Dec 13, 2024

Uh oh!

ti-chi-bot commented Jan 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

LykxSassinator commented Oct 10, 2024 •

edited

Loading

SpadeA-Tang commented Oct 22, 2024 •

edited

Loading

LykxSassinator commented Oct 22, 2024 •

edited

Loading