schedulers: region should splitted if region is too hot. #6618

bufferflies · 2023-06-16T01:35:12Z

What problem does this PR solve?

Issue Number: Close #6619

What is changed and how does it work?

Check List

Tests

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

Code changes

Side effects

Related changes

Need to cherry-pick to the release branch

Release note

None.

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

ti-chi-bot · 2023-06-16T01:35:15Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

lhy1024
nolouch

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.

lhy1024 · 2023-06-16T02:20:29Z

pkg/schedule/schedulers/hot_region.go

+// regionIsTooHot returns true if any dim of the hot region is greater than the store threshold.
+func regionIsTooHot(store *statistics.StoreLoadDetail, region *statistics.HotPeerStat) bool {
+	return slice.AnyOf(store.LoadPred.Current.Loads, func(i int) bool {
+		return region.Loads[i] > store.LoadPred.Current.Loads[i]*0.3


add const regionTooHotThreshold = 0.3?

good suggestion, fixed

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

HuSharp · 2023-06-16T06:36:04Z

Can u paste a picture to show region_is_too_hot?

lhy1024

LGTM. Do we need some manual test?

lhy1024

ci failed

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

bufferflies · 2023-06-26T03:00:03Z

LGTM. Do we need some manual test?

yes, I have upload the manual test result.

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

codecov · 2023-06-26T03:24:59Z

Codecov Report

Merging #6618 (6d8cdc7) into master (e18b9be) will increase coverage by 0.01%.
The diff coverage is 83.92%.

❗ Current head 6d8cdc7 differs from pull request most recent head 3bd39cb. Consider uploading reports for the commit 3bd39cb to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6618      +/-   ##
==========================================
+ Coverage   74.17%   74.18%   +0.01%     
==========================================
  Files         413      411       -2     
  Lines       43349    43067     -282     
==========================================
- Hits        32155    31951     -204     
+ Misses       8312     8247      -65     
+ Partials     2882     2869      -13

Flag	Coverage Δ
unittests	`74.18% <83.92%> (+0.01%)`	⬆️

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

pkg/schedule/schedulers/hot_region.go

tonyxuqqi · 2023-06-30T21:59:06Z

/release

lhy1024

If workload is append write, split is not helpful

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

…_region_split

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

nolouch

lgtm

bufferflies · 2023-07-05T12:48:55Z

/hold

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

lhy1024

LGTM, but ci failed

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

bufferflies · 2023-07-11T02:29:55Z

/merge

ti-chi-bot · 2023-07-11T02:29:57Z

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

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-07-11T02:29:58Z

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

Commit hash: 1305bd3

close tikv#6619 Signed-off-by: bufferflies <1045931706@qq.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

bufferflies added 2 commits June 14, 2023 19:29

hot region

dea2914

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

add test for write

f1676d4

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

ti-chi-bot bot added do-not-merge/needs-linked-issue release-note-none labels Jun 16, 2023

ti-chi-bot bot requested review from disksing and Yisaer June 16, 2023 01:35

ti-chi-bot bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed do-not-merge/needs-linked-issue labels Jun 16, 2023

bufferflies requested review from lhy1024 and rleungx and removed request for disksing and Yisaer June 16, 2023 01:40

lhy1024 reviewed Jun 16, 2023

View reviewed changes

extra const variable

9fe27cf

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

lhy1024 reviewed Jun 16, 2023

View reviewed changes

lhy1024 reviewed Jun 19, 2023

View reviewed changes

lint

25b0145

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

bufferflies force-pushed the hot_region_split branch from 0afad4a to 25b0145 Compare June 19, 2023 03:19

bufferflies changed the title ~~schedulers: region should splited if region is too hot.~~ schedulers: region should splitted if region is too hot. Jun 26, 2023

bufferflies added 2 commits June 26, 2023 11:09

pass ut

311bb19

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

Merge branch 'master' into hot_region_split

b90e13a

lhy1024 reviewed Jun 30, 2023

View reviewed changes

pkg/schedule/schedulers/hot_region.go Show resolved Hide resolved

lhy1024 approved these changes Jul 3, 2023

View reviewed changes

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

lhy1024 reviewed Jul 3, 2023

View reviewed changes

bufferflies added 4 commits July 5, 2023 16:46

Merge branch 'master' into hot_region_split

7aa5c81

add config for split thresholds

a3faee3

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

Merge branch 'hot_region_split' of github.com:bufferflies/pd into hot…

273b196

…_region_split

pass unit

6d8cdc7

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

nolouch reviewed Jul 5, 2023

View reviewed changes

nolouch approved these changes Jul 5, 2023

View reviewed changes

ti-chi-bot bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jul 5, 2023

ti-chi-bot bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 5, 2023

bufferflies added 5 commits July 6, 2023 15:15

avoid panic

a6006b1

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

logic error

5f25ce5

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

make config more reasonable

db9247d

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

Merge branch 'master' into hot_region_split

019d0a0

pass ut

3628645

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

lhy1024 approved these changes Jul 10, 2023

View reviewed changes

lhy1024 reviewed Jul 10, 2023

View reviewed changes

bufferflies added 2 commits July 10, 2023 17:10

pass ut

6310efe

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

update master

787f6f1

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

bufferflies removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 10, 2023

pass ut

1305bd3

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

bufferflies force-pushed the hot_region_split branch from 57f720f to 1305bd3 Compare July 10, 2023 10:10

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

Merge branch 'master' into hot_region_split

3bd39cb

ti-chi-bot bot merged commit 1d0bbb8 into tikv:master Jul 11, 2023
22 checks passed

lhy1024 mentioned this pull request Jul 12, 2023

Hot statistic and hot scheduler issue list #5691

Open

61 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

schedulers: region should splitted if region is too hot. #6618

schedulers: region should splitted if region is too hot. #6618

bufferflies commented Jun 16, 2023 •

edited

ti-chi-bot bot commented Jun 16, 2023 •

edited

lhy1024 Jun 16, 2023

bufferflies Jun 16, 2023

HuSharp commented Jun 16, 2023

lhy1024 left a comment

lhy1024 left a comment

bufferflies commented Jun 26, 2023

codecov bot commented Jun 26, 2023 •

edited

tonyxuqqi commented Jun 30, 2023

lhy1024 left a comment

nolouch left a comment

bufferflies commented Jul 5, 2023

lhy1024 left a comment

bufferflies commented Jul 11, 2023

ti-chi-bot bot commented Jul 11, 2023

ti-chi-bot bot commented Jul 11, 2023

schedulers: region should splitted if region is too hot. #6618

schedulers: region should splitted if region is too hot. #6618

Conversation

bufferflies commented Jun 16, 2023 • edited

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

ti-chi-bot bot commented Jun 16, 2023 • edited

lhy1024 Jun 16, 2023

Choose a reason for hiding this comment

bufferflies Jun 16, 2023

Choose a reason for hiding this comment

HuSharp commented Jun 16, 2023

lhy1024 left a comment

Choose a reason for hiding this comment

lhy1024 left a comment

Choose a reason for hiding this comment

bufferflies commented Jun 26, 2023

codecov bot commented Jun 26, 2023 • edited

Codecov Report

tonyxuqqi commented Jun 30, 2023

lhy1024 left a comment

Choose a reason for hiding this comment

nolouch left a comment

Choose a reason for hiding this comment

bufferflies commented Jul 5, 2023

lhy1024 left a comment

Choose a reason for hiding this comment

bufferflies commented Jul 11, 2023

ti-chi-bot bot commented Jul 11, 2023

ti-chi-bot bot commented Jul 11, 2023

bufferflies commented Jun 16, 2023 •

edited

ti-chi-bot bot commented Jun 16, 2023 •

edited

codecov bot commented Jun 26, 2023 •

edited