statistics: fix the hot region API cannot work without hot scheduler #4424

rleungx · 2021-12-05T08:37:57Z

What problem does this PR solve?

close #3879. But this PR should be reviewed after the following PRs are merged:

What is changed and how it works?

This PR mainly makes the following changes:

remove hasHotStatus interface
no need to run a hot scheduler to get hot statistics
move all calculation logic of hot region into statistics package

Check List

Tests

Unit test

Release note

None

ti-chi-bot · 2021-12-05T08:37:59Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

bufferflies
lhy1024

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.

codecov · 2021-12-05T08:48:47Z

Codecov Report

Merging #4424 (4f68846) into master (d1b59c1) will decrease coverage by 0.14%.
The diff coverage is 97.54%.

@@            Coverage Diff             @@
##           master    #4424      +/-   ##
==========================================
- Coverage   74.96%   74.82%   -0.15%     
==========================================
  Files         277      279       +2     
  Lines       27627    27627              
==========================================
- Hits        20711    20671      -40     
- Misses       5082     5106      +24     
- Partials     1834     1850      +16

Flag	Coverage Δ
unittests	`74.82% <97.54%> (-0.15%)`	⬇️

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

Impacted Files	Coverage Δ
server/schedulers/metrics.go	`100.00% <ø> (ø)`
server/schedulers/utils.go	`91.04% <ø> (-3.10%)`	⬇️
server/statistics/store_load.go	`95.76% <60.00%> (-1.61%)`	⬇️
server/statistics/collector.go	`93.75% <93.75%> (ø)`
server/cluster/cluster.go	`84.25% <100.00%> (-0.58%)`	⬇️
server/cluster/coordinator.go	`75.59% <100.00%> (-1.21%)`	⬇️
server/core/basic_cluster.go	`91.66% <100.00%> (+0.18%)`	⬆️
server/schedulers/grant_hot_region.go	`58.54% <100.00%> (ø)`
server/schedulers/hot_region.go	`82.58% <100.00%> (-0.24%)`	⬇️
server/schedulers/shuffle_hot_region.go	`65.65% <100.00%> (ø)`
... and 17 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 4242300...4f68846. Read the comment docs.

rleungx · 2021-12-14T09:23:22Z

/run-unit-tests

server/statistics/metrics.go

rleungx · 2021-12-16T10:26:26Z

The lock problem needs to pay attention to.

bufferflies · 2021-12-24T04:09:48Z

server/cluster/coordinator.go

@@ -535,35 +517,33 @@ func (c *coordinator) resetSchedulerMetrics() {

 func (c *coordinator) collectHotSpotMetrics() {


There is some difference between collection and GetHotXXXXRegions.

rleungx · 2022-01-04T10:21:18Z

/hold cancel

Signed-off-by: Ryan Leung <rleungx@gmail.com>

lhy1024 · 2022-01-19T08:23:10Z

server/cluster/cluster.go

@@ -1590,23 +1581,17 @@ func (checker *prepareChecker) collect(region *core.RegionInfo) {

 // GetHotWriteRegions gets hot write regions' info.
 func (c *RaftCluster) GetHotWriteRegions(storeIDs ...uint64) *statistics.StoreHotPeersInfos {
-	c.RLock()


Do we no longer need locks?

I think so.

server/statistics/store_hot_peers_infos.go

Signed-off-by: Ryan Leung <rleungx@gmail.com>

rleungx · 2022-01-20T09:01:55Z

/merge

ti-chi-bot · 2022-01-20T09:01:56Z

@rleungx: 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-20T09:01:57Z

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

Commit hash: 4f68846

…ikv#4424) * refacter store load in hot region scheduler ref tikv#3879 Signed-off-by: Ryan Leung <rleungx@gmail.com> * resolve conflicts Signed-off-by: Ryan Leung <rleungx@gmail.com> * fix the hot region API cannot work when hot scheduler is disabled Signed-off-by: Ryan Leung <rleungx@gmail.com> * resolve conflicts Signed-off-by: Ryan Leung <rleungx@gmail.com> * clean up Signed-off-by: Ryan Leung <rleungx@gmail.com> * make drop region atomically Signed-off-by: Ryan Leung <rleungx@gmail.com> * address the comment Signed-off-by: Ryan Leung <rleungx@gmail.com>

nolouch · 2023-02-20T10:29:37Z

/run-cherry-pick

ti-chi-bot · 2023-02-20T10:30:18Z

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

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

ti-chi-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/invalid-commit-message labels Dec 5, 2021

ti-chi-bot requested review from HunDunDM and JmPotato December 5, 2021 08:38

rleungx force-pushed the remove-interface branch from 06ba0ad to 7e86154 Compare December 6, 2021 03:50

rleungx mentioned this pull request Dec 6, 2021

*: remove unnecessary read/write definitions #4426

Merged

rleungx force-pushed the remove-interface branch from 7e86154 to 0297d6f Compare December 6, 2021 08:00

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/invalid-commit-message release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Dec 6, 2021

rleungx changed the title ~~statistics: fix the hot region API cannot work when hot scheduler is disabled~~ statistics: fix the hot region API cannot work without hot scheduler Dec 6, 2021

rleungx force-pushed the remove-interface branch from 0297d6f to bcd5c4a Compare December 6, 2021 09:40

rleungx force-pushed the remove-interface branch from bcd5c4a to e73e323 Compare December 13, 2021 10:05

rleungx marked this pull request as ready for review December 13, 2021 10:06

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 Dec 13, 2021

rleungx force-pushed the remove-interface branch from edd1769 to 1b5a50b Compare December 14, 2021 08:51

bufferflies reviewed Dec 16, 2021

View reviewed changes

server/statistics/metrics.go Show resolved Hide resolved

rleungx mentioned this pull request Dec 22, 2021

scheduler: refactor hot region scheduler #4488

Merged

rleungx requested review from nolouch and removed request for JmPotato December 22, 2021 06:48

bufferflies reviewed Dec 24, 2021

View reviewed changes

rleungx added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 28, 2021

rleungx force-pushed the remove-interface branch from 1b5a50b to 1f1b30d Compare January 4, 2022 10:03

ti-chi-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 4, 2022

rleungx added 2 commits January 19, 2022 12:13

clean up

3d6a0a2

Signed-off-by: Ryan Leung <rleungx@gmail.com>

make drop region atomically

4222c18

Signed-off-by: Ryan Leung <rleungx@gmail.com>

rleungx force-pushed the remove-interface branch from cedc96c to 4222c18 Compare January 19, 2022 04:18

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

rleungx requested a review from JmPotato January 19, 2022 04:20

lhy1024 reviewed Jan 19, 2022

View reviewed changes

server/statistics/store_hot_peers_infos.go Outdated Show resolved Hide resolved

address the comment

cdcc256

Signed-off-by: Ryan Leung <rleungx@gmail.com>

rleungx requested a review from lhy1024 January 19, 2022 09:53

lhy1024 approved these changes Jan 19, 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 19, 2022

rleungx added 3 commits January 20, 2022 14:32

Merge branch 'master' into remove-interface

889c6c9

Merge branch 'master' into remove-interface

7bdfd4b

Merge branch 'master' into remove-interface

4f68846

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

ti-chi-bot merged commit f605a2c into tikv:master Jan 20, 2022

lhy1024 mentioned this pull request Sep 18, 2022

metrics: pending influence of hot scheduler is empty #5527

Closed

nolouch mentioned this pull request Feb 20, 2023

Remove scheduler trigger deadlock #6024

Closed

nolouch added the needs-cherry-pick-release-5.4 Should cherry pick this PR to release-5.4 branch. label Feb 20, 2023

nolouch removed the needs-cherry-pick-release-5.4 Should cherry pick this PR to release-5.4 branch. label Feb 20, 2023

ti-chi-bot mentioned this pull request Feb 20, 2023

statistics: fix the hot region API cannot work without hot scheduler (#4424) #6027

Closed

2 tasks

ti-chi-bot pushed a commit to ti-chi-bot/pd that referenced this pull request Feb 20, 2023

This is an automated cherry-pick of tikv#4424

158103b

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

rleungx deleted the remove-interface branch February 21, 2023 02:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

statistics: fix the hot region API cannot work without hot scheduler #4424

statistics: fix the hot region API cannot work without hot scheduler #4424

rleungx commented Dec 5, 2021 •

edited by bufferflies

Loading

ti-chi-bot commented Dec 5, 2021 •

edited

Loading

codecov bot commented Dec 5, 2021 •

edited

Loading

rleungx commented Dec 14, 2021

rleungx commented Dec 16, 2021

bufferflies Dec 24, 2021

rleungx commented Jan 4, 2022

lhy1024 Jan 19, 2022

rleungx Jan 19, 2022

rleungx commented Jan 20, 2022

ti-chi-bot commented Jan 20, 2022

ti-chi-bot commented Jan 20, 2022

nolouch commented Feb 20, 2023

ti-chi-bot commented Feb 20, 2023

		@@ -535,35 +517,33 @@ func (c *coordinator) resetSchedulerMetrics() {

		func (c *coordinator) collectHotSpotMetrics() {

statistics: fix the hot region API cannot work without hot scheduler #4424

statistics: fix the hot region API cannot work without hot scheduler #4424

Conversation

rleungx commented Dec 5, 2021 • edited by bufferflies Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Dec 5, 2021 • edited Loading

codecov bot commented Dec 5, 2021 • edited Loading

Codecov Report

rleungx commented Dec 14, 2021

rleungx commented Dec 16, 2021

bufferflies Dec 24, 2021

Choose a reason for hiding this comment

rleungx commented Jan 4, 2022

lhy1024 Jan 19, 2022

Choose a reason for hiding this comment

rleungx Jan 19, 2022

Choose a reason for hiding this comment

rleungx commented Jan 20, 2022

ti-chi-bot commented Jan 20, 2022

ti-chi-bot commented Jan 20, 2022

nolouch commented Feb 20, 2023

ti-chi-bot commented Feb 20, 2023

rleungx commented Dec 5, 2021 •

edited by bufferflies

Loading

ti-chi-bot commented Dec 5, 2021 •

edited

Loading

codecov bot commented Dec 5, 2021 •

edited

Loading