Skip to content

checker: fix the too many orphan peers cannot be removed#6574

Merged
ti-chi-bot[bot] merged 2 commits intotikv:masterfrom
nolouch:fix-remove-orphan
Jun 8, 2023
Merged

checker: fix the too many orphan peers cannot be removed#6574
ti-chi-bot[bot] merged 2 commits intotikv:masterfrom
nolouch:fix-remove-orphan

Conversation

@nolouch
Copy link
Copy Markdown
Contributor

@nolouch nolouch commented Jun 8, 2023

What problem does this PR solve?

Issue Number: Close #6573

What is changed and how does it work?

rule-checker: fix the too many orphan peers that cannot be removed
- let the health peer can be removed once there exist redundant

Check List

Tests

  • Unit test

Release note

Fix the issue too many peers/learners cannot be removed

@nolouch nolouch force-pushed the fix-remove-orphan branch from c531cee to f56ca51 Compare June 8, 2023 08:00
@nolouch nolouch requested review from CabinfeverB and rleungx June 8, 2023 08:01
@nolouch nolouch changed the title checker: fix the too many orphan peers cannot be remove checker: fix the too many orphan peers cannot be removed Jun 8, 2023
@nolouch nolouch force-pushed the fix-remove-orphan branch from f56ca51 to 2db0579 Compare June 8, 2023 08:07
@codecov
Copy link
Copy Markdown

codecov bot commented Jun 8, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.04 🎉

Comparison is base (026ddf0) 74.61% compared to head (482cd9b) 74.66%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6574      +/-   ##
==========================================
+ Coverage   74.61%   74.66%   +0.04%     
==========================================
  Files         416      416              
  Lines       42576    42581       +5     
==========================================
+ Hits        31769    31794      +25     
+ Misses       7995     7991       -4     
+ Partials     2812     2796      -16     
Flag Coverage Δ
unittests 74.66% <100.00%> (+0.04%) ⬆️

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

Impacted Files Coverage Δ
pkg/schedule/checker/rule_checker.go 83.45% <100.00%> (+1.03%) ⬆️

... and 29 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@nolouch nolouch force-pushed the fix-remove-orphan branch from 2db0579 to 63e6a54 Compare June 8, 2023 08:25
Signed-off-by: nolouch <nolouch@gmail.com>
@nolouch nolouch force-pushed the fix-remove-orphan branch from 63e6a54 to 0229f17 Compare June 8, 2023 08:27
suite.cluster.AddLeaderStore(4, 1)
suite.cluster.AddLeaderStore(5, 1)
suite.cluster.AddLeaderStore(6, 1)
suite.cluster.AddRegionWithLearner(1, 1, []uint64{2, 3}, []uint64{4, 5, 6})
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This comment should have nothing to do with this PR. But I want to know what happen if here is suite.cluster.AddRegionWithLearner(1, 1, []uint64{2, 3}, []uint64{4}), and other behaviors are same.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It seems not return operator

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, because need len(OrphanPeers) >=2

Signed-off-by: nolouch <nolouch@gmail.com>
@CabinfeverB
Copy link
Copy Markdown
Member

Why bot does not add label for this PR..

@nolouch
Copy link
Copy Markdown
Contributor Author

nolouch commented Jun 8, 2023

/merge

@ti-chi-bot
Copy link
Copy Markdown
Contributor

ti-chi-bot bot commented Jun 8, 2023

@nolouch: 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
Copy link
Copy Markdown
Contributor

ti-chi-bot bot commented Jun 8, 2023

@nolouch: /merge in this pull request requires 2 approval(s).

Details

In response to this:

/merge

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.

@nolouch nolouch added the status/LGT2 Indicates that a PR has LGTM 2. label Jun 8, 2023
@ti-chi-bot ti-chi-bot bot removed the status/LGT2 Indicates that a PR has LGTM 2. label Jun 8, 2023
@wuhuizuo
Copy link
Copy Markdown
Contributor

wuhuizuo commented Jun 8, 2023

Why bot does not add label for this PR..

Bot offline for serval minutes, please re approve it again with github review approving.

@ti-chi-bot
Copy link
Copy Markdown
Contributor

ti-chi-bot bot commented Jun 8, 2023

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • CabinfeverB

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 ti-chi-bot bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jun 8, 2023
@CabinfeverB CabinfeverB added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Jun 8, 2023
@ti-chi-bot ti-chi-bot bot added needs-cherry-pick-release-6.5 Should cherry pick this PR to release-6.5 branch. needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. labels Jun 8, 2023
@CabinfeverB CabinfeverB added the require-LGT1 Indicates that the PR requires an LGTM. label Jun 8, 2023
@CabinfeverB
Copy link
Copy Markdown
Member

/merge

@ti-chi-bot
Copy link
Copy Markdown
Contributor

ti-chi-bot bot commented Jun 8, 2023

@CabinfeverB: 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
Copy link
Copy Markdown
Contributor

ti-chi-bot bot commented Jun 8, 2023

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

DetailsCommit hash: 482cd9b

@ti-chi-bot ti-chi-bot bot added the status/can-merge Indicates a PR has been approved by a committer. label Jun 8, 2023
@CabinfeverB
Copy link
Copy Markdown
Member

/test build

@ti-chi-bot ti-chi-bot bot merged commit e807811 into tikv:master Jun 8, 2023
@ti-chi-bot
Copy link
Copy Markdown
Member

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

@ti-chi-bot
Copy link
Copy Markdown
Member

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

@nolouch nolouch deleted the fix-remove-orphan branch June 8, 2023 11:56
ti-chi-bot bot pushed a commit that referenced this pull request Jun 9, 2023
close #6573, ref #6574

rule-checker: fix the too many orphan peers that cannot be removed
- let the health peer can be removed once there exist redundant

Signed-off-by: nolouch <nolouch@gmail.com>

Co-authored-by: nolouch <nolouch@gmail.com>
ti-chi-bot bot added a commit that referenced this pull request Jun 26, 2023
close #6573, ref #6574

rule-checker: fix the too many orphan peers that cannot be removed
- let the health peer can be removed once there exist redundant

Signed-off-by: nolouch <nolouch@gmail.com>

Co-authored-by: nolouch <nolouch@gmail.com>
Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-cherry-pick-release-6.5 Should cherry pick this PR to release-6.5 branch. needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. release-note Denotes a PR that will be considered when it comes time to generate release notes. require-LGT1 Indicates that the PR requires an LGTM. status/can-merge Indicates a PR has been approved by a committer. status/LGT1 Indicates that a PR has LGTM 1.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Too manly orphan peers cannot be remove

5 participants