operator: remove empty JointConsensus steps and fix single voter demote bug #4534

matchge-ca · 2022-01-03T05:38:24Z

Signed-off-by: Hua Lu hua.lu@pingcap.com

What problem does this PR solve?

Closes: #4362
Closes: #4444

What is changed and how it works?

Modify condition to enable useJointConsensus when there is only single non-demote peer change
Add logic in execChangePeerV2 to prevent generate empty enter / leave JointConsensus steps
Remove allowDemote and DemoteFollower step

Check List

Tests

Unit test

Related changes

Need to cherry-pick to the release branch

Release note

- fix a bug that operate steps may contain unnecessary or empty JointConsensus steps in certain conditions 
- fix a bug when demoting single voter directly

Signed-off-by: Hua Lu <hua.lu@pingcap.com>

ti-chi-bot · 2022-01-03T05:38:25Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

HunDunDM
disksing

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.

server/schedule/operator/builder_test.go

server/schedule/operator/builder.go

HunDunDM

The previous lgtm was a misoperation, sorry.

… improve code quality Signed-off-by: Hua Lu <hua.lu@pingcap.com>

Signed-off-by: Hua Lu <hua.lu@pingcap.com>

HunDunDM

rest LGTM

server/schedule/operator/builder.go

codecov · 2022-01-05T17:16:01Z

Codecov Report

Merging #4534 (7737b61) into master (0f0326c) will decrease coverage by 0.05%.
The diff coverage is 88.88%.

@@            Coverage Diff             @@
##           master    #4534      +/-   ##
==========================================
- Coverage   74.86%   74.80%   -0.06%     
==========================================
  Files         265      265              
  Lines       27856    27838      -18     
==========================================
- Hits        20854    20825      -29     
- Misses       5133     5143      +10     
- Partials     1869     1870       +1

Flag	Coverage Δ
unittests	`74.80% <88.88%> (-0.06%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
server/schedule/operator/step.go	`72.93% <ø> (-1.09%)`	⬇️
server/schedule/operator/builder.go	`86.46% <88.23%> (-2.85%)`	⬇️
server/schedule/operator_controller.go	`83.02% <100.00%> (-0.89%)`	⬇️
server/schedulers/shuffle_hot_region.go	`55.55% <0.00%> (-10.11%)`	⬇️
server/id/id.go	`76.19% <0.00%> (-9.53%)`	⬇️
server/region_syncer/client.go	`78.12% <0.00%> (-5.47%)`	⬇️
server/core/region_storage.go	`75.78% <0.00%> (-2.11%)`	⬇️
server/core/storage.go	`68.56% <0.00%> (-0.76%)`	⬇️
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0f0326c...7737b61. Read the comment docs.

Signed-off-by: Hua Lu <hua.lu@pingcap.com>

HunDunDM · 2022-01-06T03:45:59Z

ref tikv/tikv#11463

disksing · 2022-01-06T04:00:32Z

/merge

ti-chi-bot · 2022-01-06T04:00:32Z

@disksing: 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 · 2022-01-06T04:00:34Z

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

Commit hash: 7737b61

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

ti-chi-bot · 2022-01-06T05:33:28Z

In response to a cherrypick label: new pull request created: #4551.

ti-chi-bot · 2022-01-06T05:33:28Z

In response to a cherrypick label: new pull request created: #4552.

…te bug (#4534) (#4551) * remove empty jc steps and fix single peer demote bug Signed-off-by: Hua Lu <hua.lu@pingcap.com> * remove DemoteFollower from builder, step and operator_controller, and improve code quality Signed-off-by: Hua Lu <hua.lu@pingcap.com> * change leave step condition to cover rare case Signed-off-by: Hua Lu <hua.lu@pingcap.com> * minor changes on comments and logic flow of execChangePeerV2 Signed-off-by: Hua Lu <hua.lu@pingcap.com> * sync code Signed-off-by: HunDunDM <hundundm@gmail.com> Co-authored-by: Hua Lu <hua.lu@pingcap.com> Co-authored-by: HunDunDM <hundundm@gmail.com>

…te bug (#4534) (#4552) ref #4362, ref #4444, ref #4534 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io> Signed-off-by: HunDunDM <hundundm@gmail.com> Co-authored-by: matchge <74505524+matchge-ca@users.noreply.github.com> Co-authored-by: HunDunDM <hundundm@gmail.com>

remove empty jc steps and fix single peer demote bug

a4b4a6a

Signed-off-by: Hua Lu <hua.lu@pingcap.com>

ti-chi-bot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Jan 3, 2022

ti-chi-bot requested review from disksing and HunDunDM January 3, 2022 05:38

HunDunDM approved these changes Jan 3, 2022

View reviewed changes

server/schedule/operator/builder_test.go Show resolved Hide resolved

server/schedule/operator/builder.go Outdated Show resolved Hide resolved

server/schedule/operator/builder.go Outdated Show resolved Hide resolved

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

matchge-ca changed the title ~~operator: remove empty JointConsensus steps and fix single peer demote bug~~ operator: remove empty JointConsensus steps and fix single voter demote bug Jan 4, 2022

ti-chi-bot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. and removed release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Jan 4, 2022

HunDunDM self-requested a review January 4, 2022 01:17

HunDunDM removed the status/LGT1 Indicates that a PR has LGTM 1. label Jan 4, 2022

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

HunDunDM requested changes Jan 4, 2022

View reviewed changes

ti-chi-bot removed the status/LGT1 Indicates that a PR has LGTM 1. label Jan 4, 2022

remove DemoteFollower from builder, step and operator_controller, and…

e4ec6f2

… improve code quality Signed-off-by: Hua Lu <hua.lu@pingcap.com>

matchge-ca requested a review from HunDunDM January 4, 2022 01:39

matchge-ca added 2 commits January 4, 2022 14:01

Merge branch 'master' into schedule-remove-empty-jointconcensus-steps

ead315b

change leave step condition to cover rare case

4aad1ae

Signed-off-by: Hua Lu <hua.lu@pingcap.com>

HunDunDM reviewed Jan 5, 2022

View reviewed changes

server/schedule/operator/builder.go Outdated Show resolved Hide resolved

server/schedule/operator/builder.go Outdated Show resolved Hide resolved

HunDunDM mentioned this pull request Jan 5, 2022

server/schedule: use ChangePeerV2 to demote single voter directly #4445

Closed

Merge branch 'master' into schedule-remove-empty-jointconcensus-steps

7ac8769

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

minor changes on comments and logic flow of execChangePeerV2

7737b61

Signed-off-by: Hua Lu <hua.lu@pingcap.com>

matchge-ca requested a review from HunDunDM January 5, 2022 19:31

HunDunDM approved these changes Jan 6, 2022

View reviewed changes

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

HunDunDM mentioned this pull request Jan 6, 2022

server/schedule: prevent removing voters directly in 2 replica raft group #4412

Closed

disksing approved these changes Jan 6, 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 Jan 6, 2022

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

ti-chi-bot merged commit 1c0286c into tikv:master Jan 6, 2022

HunDunDM mentioned this pull request Jan 6, 2022

tikv error when demote follower to learner tikv/tikv#11463

Closed

HunDunDM added needs-cherry-pick-release-5.3 Type: Need cherry pick to release-5.3 needs-cherry-pick-release-5.4 Should cherry pick this PR to release-5.4 branch. labels Jan 6, 2022

ti-chi-bot pushed a commit to ti-chi-bot/pd that referenced this pull request Jan 6, 2022

This is an automated cherry-pick of tikv#4534

14d6a04

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

This was referenced Jan 6, 2022

operator: remove empty JointConsensus steps and fix single voter demote bug (#4534) #4551

Merged

operator: remove empty JointConsensus steps and fix single voter demote bug (#4534) #4552

Merged

lcwangchao mentioned this pull request Jan 14, 2022

Learner peers cannot be scheduled using placement settings pingcap/tidb#31173

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

operator: remove empty JointConsensus steps and fix single voter demote bug #4534

operator: remove empty JointConsensus steps and fix single voter demote bug #4534

matchge-ca commented Jan 3, 2022 •

edited by HunDunDM

Loading

ti-chi-bot commented Jan 3, 2022 •

edited

Loading

HunDunDM left a comment

HunDunDM left a comment

codecov bot commented Jan 5, 2022 •

edited

Loading

HunDunDM commented Jan 6, 2022

disksing commented Jan 6, 2022

ti-chi-bot commented Jan 6, 2022

ti-chi-bot commented Jan 6, 2022

ti-chi-bot commented Jan 6, 2022

ti-chi-bot commented Jan 6, 2022

operator: remove empty JointConsensus steps and fix single voter demote bug #4534

operator: remove empty JointConsensus steps and fix single voter demote bug #4534

Conversation

matchge-ca commented Jan 3, 2022 • edited by HunDunDM Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

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

HunDunDM left a comment

Choose a reason for hiding this comment

HunDunDM left a comment

Choose a reason for hiding this comment

codecov bot commented Jan 5, 2022 • edited Loading

Codecov Report

HunDunDM commented Jan 6, 2022

disksing commented Jan 6, 2022

ti-chi-bot commented Jan 6, 2022

ti-chi-bot commented Jan 6, 2022

ti-chi-bot commented Jan 6, 2022

ti-chi-bot commented Jan 6, 2022

matchge-ca commented Jan 3, 2022 •

edited by HunDunDM

Loading

ti-chi-bot commented Jan 3, 2022 •

edited

Loading

codecov bot commented Jan 5, 2022 •

edited

Loading