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.

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

Fix balance-leader won't work when placement rule enabled (#2725) #2726

Merged
merged 1 commit into from
Aug 6, 2020

Conversation

ti-srebot
Copy link
Contributor

cherry-pick #2725 to release-4.0


What problem does this PR solve?

When placement rule enabled, balance-leader won't work.

What is changed and how it works?

The region copy logic in ruleLeaderFilter's Target function is wrong. This request fix this bug.

Check List

Tests

  • Unit test

Related changes

  • Need to cherry-pick to the release branch

Release note

  • Fix the bug that balance-leader won't work when placement rule enabled (imported from v4.0.3)

@Yisaer
Copy link
Contributor

Yisaer commented Aug 6, 2020

/run-all-tests

@codecov-commenter
Copy link

Codecov Report

Merging #2726 into release-4.0 will decrease coverage by 0.02%.
The diff coverage is 75.00%.

Impacted file tree graph

@@               Coverage Diff               @@
##           release-4.0    #2726      +/-   ##
===============================================
- Coverage        77.50%   77.47%   -0.03%     
===============================================
  Files              206      206              
  Lines            22490    22499       +9     
===============================================
+ Hits             17431    17432       +1     
- Misses            3758     3765       +7     
- Partials          1301     1302       +1     
Impacted Files Coverage Δ
server/schedule/filter/filters.go 83.04% <40.00%> (-0.93%) ⬇️
pkg/mock/mockcluster/mockcluster.go 92.10% <100.00%> (+0.14%) ⬆️
server/schedulers/shuffle_hot_region.go 63.47% <0.00%> (-11.31%) ⬇️
server/tso/tso.go 78.94% <0.00%> (-4.61%) ⬇️
server/region_syncer/client.go 81.67% <0.00%> (-4.59%) ⬇️
pkg/etcdutil/etcdutil.go 88.40% <0.00%> (-2.90%) ⬇️
server/cluster/coordinator.go 70.02% <0.00%> (-1.92%) ⬇️
pkg/mock/mockhbstream/mockhbstream.go 89.23% <0.00%> (-1.54%) ⬇️
server/handler.go 50.76% <0.00%> (-0.44%) ⬇️
server/schedule/operator_controller.go 81.25% <0.00%> (-0.17%) ⬇️
... and 8 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 85014e2...6ec9bb0. Read the comment docs.

Copy link
Contributor

@nolouch nolouch 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-srebot ti-srebot added status/LGT1 Indicates that a PR has LGTM 1. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Aug 6, 2020
@ti-srebot ti-srebot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Aug 6, 2020
@disksing disksing merged commit c89a727 into tikv:release-4.0 Aug 6, 2020
@disksing disksing deleted the release-4.0-33666d8aaa97 branch August 6, 2020 07:47
@HunDunDM HunDunDM added this to the v4.0.5 milestone Sep 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/placement Placement rule. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants