scheduler: move hot region should be splitted if it's size is too large . #6281

bufferflies · 2023-04-07T01:53:57Z

What problem does this PR solve?

Issue Number: Ref #6328,Close tikv/tikv#14333, ref #tikv/tikv#14214, ref tikv/tikv#14135

What is changed and how does it work?

If the moved region's size is bigger than the max-movable-hot-peer-size, it will be splited by hot buckets.
It will brings two benefits:

bucket split will not be always happened, so the total region count is reasonable
bucket split is on time, not depends on split-bucket-scheduler

The split-bucket-scheduler will be removed from the default list

Check List

Tests

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

This PR:

master:

Code changes

Side effects

Related changes

Release note

move hot region should be splitted if it's size is too large .

ti-chi-bot · 2023-04-07T01:53:59Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

lhy1024
rleungx

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.

ti-chi-bot · 2023-04-07T01:53:59Z

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: bufferflies <1045931706@qq.com>

codecov · 2023-04-13T08:03:55Z

Codecov Report

Patch coverage: 58.97% and project coverage change: -0.16 ⚠️

Comparison is base (dbec11e) 75.18% compared to head (aedad83) 75.02%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6281      +/-   ##
==========================================
- Coverage   75.18%   75.02%   -0.16%     
==========================================
  Files         408      408              
  Lines       40532    40598      +66     
==========================================
- Hits        30475    30460      -15     
- Misses       7417     7488      +71     
- Partials     2640     2650      +10

Flag	Coverage Δ
unittests	`75.02% <58.97%> (-0.16%)`	⬇️

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

Impacted Files	Coverage Δ
server/cluster/cluster.go	`81.44% <0.00%> (-0.72%)`	⬇️
server/config/config.go	`75.76% <ø> (ø)`
pkg/schedule/schedulers/hot_region.go	`82.45% <44.64%> (-2.04%)`	⬇️
client/client.go	`63.42% <100.00%> (+0.05%)`	⬆️
pkg/mock/mockcluster/mockcluster.go	`94.71% <100.00%> (+0.45%)`	⬆️
pkg/schedule/operator/operator.go	`92.85% <100.00%> (+0.04%)`	⬆️
pkg/statistics/buckets/bucket_stat_informer.go	`94.28% <100.00%> (ø)`
pkg/statistics/buckets/hot_bucket_cache.go	`90.74% <100.00%> (+0.54%)`	⬆️
pkg/statistics/buckets/hot_bucket_task.go	`86.36% <100.00%> (+0.64%)`	⬆️
pkg/utils/keyutil/util.go	`100.00% <100.00%> (ø)`

... and 26 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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

Signed-off-by: bufferflies <1045931706@qq.com>

…t_bucket_v2

Signed-off-by: bufferflies <1045931706@qq.com>

rleungx · 2023-04-18T07:23:15Z

pkg/schedule/schedulers/hot_region.go

+	splitRegions := make([]*core.RegionInfo, 0)
+	if bs.opTy == movePeer {
+		for _, region := range []*core.RegionInfo{bs.cur.region, bs.cur.revertRegion} {
+			if region == nil {
+				continue
+			}
+			if region.GetApproximateSize() > bs.GetOpts().GetMaxMovableHotPeerSize() {
+				hotSchedulerNeedSplitBeforeScheduleCounter.Inc()
+				splitRegions = append(splitRegions, region)
+			}
+		}
+	}
+	if len(splitRegions) > 0 {
+		return bs.createSplitOperator(splitRegions)
+	}


Should we move it to the beginning of this function?

Good suggestion.

pkg/schedule/schedulers/hot_region.go

Signed-off-by: bufferflies <1045931706@qq.com>

bufferflies · 2023-04-23T10:21:38Z

/merge

ti-chi-bot · 2023-04-23T10:21:40Z

@bufferflies: 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 · 2023-04-23T10:21:41Z

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

Commit hash: 942804b

bufferflies · 2023-04-23T11:30:06Z

/cherry-pick release-7.1

bufferflies · 2023-04-24T02:11:15Z

/cherry-pick release-7.1

bufferflies · 2023-04-24T03:03:22Z

/cherry-pick-release-7.1

wuhuizuo · 2023-04-24T03:24:15Z

/cherry-pick release-7.1

wuhuizuo · 2023-04-24T03:26:19Z

/cherry-pick release-7.1

wuhuizuo · 2023-04-24T03:31:30Z

/cherry-pick release-7.1

wuhuizuo · 2023-04-24T03:44:37Z

/cherry-pick release-7.1

ti-chi-bot · 2023-04-24T03:45:34Z

@wuhuizuo: new pull request created to branch release-7.1: #6371.

In response to this:

/cherry-pick release-7.1

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.

…ge . (tikv#6281) * add createSplitOperator Signed-off-by: bufferflies <1045931706@qq.com> * consider string nil Signed-off-by: bufferflies <1045931706@qq.com> * add unit test for split buckets Signed-off-by: bufferflies <1045931706@qq.com> * enable split in hot region Signed-off-by: bufferflies <1045931706@qq.com> * lint Signed-off-by: bufferflies <1045931706@qq.com> * add stringer for resourceTy Signed-off-by: bufferflies <1045931706@qq.com> * remove region keys and hot split keys in additional info Signed-off-by: bufferflies <1045931706@qq.com> * pass unit test Signed-off-by: bufferflies <1045931706@qq.com> * address comments Signed-off-by: bufferflies <1045931706@qq.com> * remove one case from TestLoadKeyspaceGroupsAssignment Signed-off-by: bufferflies <1045931706@qq.com> * revert TestLoadKeyspaceGroupsAssignment Signed-off-by: bufferflies <1045931706@qq.com> --------- Signed-off-by: bufferflies <1045931706@qq.com> Co-authored-by: Ryan Leung <rleungx@gmail.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

…ge . (tikv#6281) * add createSplitOperator Signed-off-by: bufferflies <1045931706@qq.com> * consider string nil Signed-off-by: bufferflies <1045931706@qq.com> * add unit test for split buckets Signed-off-by: bufferflies <1045931706@qq.com> * enable split in hot region Signed-off-by: bufferflies <1045931706@qq.com> * lint Signed-off-by: bufferflies <1045931706@qq.com> * add stringer for resourceTy Signed-off-by: bufferflies <1045931706@qq.com> * remove region keys and hot split keys in additional info Signed-off-by: bufferflies <1045931706@qq.com> * pass unit test Signed-off-by: bufferflies <1045931706@qq.com> * address comments Signed-off-by: bufferflies <1045931706@qq.com> * remove one case from TestLoadKeyspaceGroupsAssignment Signed-off-by: bufferflies <1045931706@qq.com> * revert TestLoadKeyspaceGroupsAssignment Signed-off-by: bufferflies <1045931706@qq.com> --------- Signed-off-by: bufferflies <1045931706@qq.com> Co-authored-by: Ryan Leung <rleungx@gmail.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com> fix flaky key for TestLoadKeyspaceGroupsAssignment Signed-off-by: bufferflies <1045931706@qq.com>

…ge . (#6281) (#6371) ref #6281, ref #6328, ref tikv/tikv#14135, ref tikv/tikv#14214, close tikv/tikv#14333 Signed-off-by: bufferflies <1045931706@qq.com> Co-authored-by: buffer <1045931706@qq.com> Co-authored-by: Ryan Leung <rleungx@gmail.com>

ti-chi-bot added do-not-merge/needs-linked-issue do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. labels Apr 7, 2023

ti-chi-bot requested review from nolouch and rleungx April 7, 2023 01:54

add createSplitOperator

f0f1b59

Signed-off-by: bufferflies <1045931706@qq.com>

bufferflies force-pushed the split_bucket_v2 branch from bdcc9f2 to f0f1b59 Compare April 7, 2023 02:11

consider string nil

9de2d19

Signed-off-by: bufferflies <1045931706@qq.com>

bufferflies force-pushed the split_bucket_v2 branch from 3c5c6c1 to 9de2d19 Compare April 13, 2023 07:53

add unit test for split buckets

a94251c

Signed-off-by: bufferflies <1045931706@qq.com>

bufferflies force-pushed the split_bucket_v2 branch from cd85c7c to a94251c Compare April 13, 2023 08:40

Merge branch 'master' into split_bucket_v2

ca30982

bufferflies marked this pull request as ready for review April 13, 2023 08:41

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

enable split in hot region

94ef66b

Signed-off-by: bufferflies <1045931706@qq.com>

bufferflies changed the title ~~draft: move hot region should be splited if it's size is too large .~~ scheduler: move hot region should be splited if it's size is too large . Apr 13, 2023

bufferflies force-pushed the split_bucket_v2 branch 4 times, most recently from 71eed8c to 7c9ecfd Compare April 13, 2023 11:22

ti-chi-bot removed the do-not-merge/needs-linked-issue label Apr 17, 2023

Merge branch 'split_bucket_v2' of github.com:bufferflies/pd into spli…

70fc54d

…t_bucket_v2

bufferflies force-pushed the split_bucket_v2 branch from 7c9ecfd to 70fc54d Compare April 17, 2023 07:33

lint

bd4787a

Signed-off-by: bufferflies <1045931706@qq.com>

bufferflies force-pushed the split_bucket_v2 branch from 509c252 to bd4787a Compare April 18, 2023 06:49

bufferflies requested a review from lhy1024 April 18, 2023 06:49

rleungx reviewed Apr 18, 2023

View reviewed changes

rleungx and others added 2 commits April 23, 2023 17:40

Merge branch 'master' into split_bucket_v2

01a5a9f

revert TestLoadKeyspaceGroupsAssignment

942804b

Signed-off-by: bufferflies <1045931706@qq.com>

ti-chi-bot bot added the status/can-merge Indicates a PR has been approved by a committer. label Apr 23, 2023

Merge branch 'master' into split_bucket_v2

aedad83

ti-chi-bot bot merged commit 1b75108 into tikv:master Apr 23, 2023

bufferflies deleted the split_bucket_v2 branch April 23, 2023 10:37

bufferflies added needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. and removed affects-7.1 This bug affects the 7.1.x(LTS) versions. needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. labels Apr 23, 2023

bufferflies removed the needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. label Apr 24, 2023

bufferflies restored the split_bucket_v2 branch April 24, 2023 02:14

bufferflies added the needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. label Apr 24, 2023

ti-chi-bot mentioned this pull request Apr 24, 2023

scheduler: move hot region should be splitted if it's size is too large . (#6281) #6371

Merged

lhy1024 mentioned this pull request Apr 25, 2023

Hot statistic and hot scheduler issue list #5691

Open

64 tasks

ti-chi-bot bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels May 25, 2023

scheduler: move hot region should be splitted if it's size is too large . #6281

scheduler: move hot region should be splitted if it's size is too large . #6281

Uh oh!

Conversation

bufferflies commented Apr 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

Uh oh!

ti-chi-bot commented Apr 7, 2023 • edited by ti-chi-bot bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ti-chi-bot commented Apr 7, 2023

Uh oh!

codecov bot commented Apr 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

rleungx Apr 18, 2023

Choose a reason for hiding this comment

Uh oh!

bufferflies Apr 18, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bufferflies commented Apr 23, 2023

Uh oh!

ti-chi-bot bot commented Apr 23, 2023

Uh oh!

ti-chi-bot bot commented Apr 23, 2023

Uh oh!

bufferflies commented Apr 23, 2023

Uh oh!

bufferflies commented Apr 24, 2023

Uh oh!

bufferflies commented Apr 24, 2023

Uh oh!

wuhuizuo commented Apr 24, 2023

Uh oh!

wuhuizuo commented Apr 24, 2023

Uh oh!

wuhuizuo commented Apr 24, 2023

Uh oh!

wuhuizuo commented Apr 24, 2023

Uh oh!

ti-chi-bot commented Apr 24, 2023

Uh oh!

Uh oh!

bufferflies commented Apr 7, 2023 •

edited

Loading

ti-chi-bot commented Apr 7, 2023 •

edited by ti-chi-bot bot

Loading

codecov bot commented Apr 13, 2023 •

edited

Loading