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

schedulers: improve store-load-based scheduler #2053

Merged
merged 17 commits into from Jan 9, 2020
Merged

Conversation

@lhy1024
Copy link
Member

lhy1024 commented Dec 26, 2019

Signed-off-by: lhy1024 admin@liudos.us

What problem does this PR solve?

cherry pick from #1754

What is changed and how it works?

  • merge pending influence struct
  • ensure why remove AggregateScores and NormalizeStoresStats
  • fix test

Check List

Tests

  • Unit test
@lhy1024 lhy1024 force-pushed the lhy1024:new-hot branch 2 times, most recently from d94818e to db42509 Dec 27, 2019
@lhy1024 lhy1024 removed the WIP label Dec 27, 2019
@lhy1024 lhy1024 marked this pull request as ready for review Dec 27, 2019
@lhy1024 lhy1024 force-pushed the lhy1024:new-hot branch from 518e7fe to a3e55ee Dec 27, 2019
@lhy1024 lhy1024 added the WIP label Dec 30, 2019
@lhy1024 lhy1024 force-pushed the lhy1024:new-hot branch 2 times, most recently from d59e8a9 to 33acaf0 Dec 30, 2019
@lhy1024 lhy1024 removed the WIP label Dec 30, 2019
@lhy1024 lhy1024 requested review from Luffbee, rleungx and nolouch and removed request for Luffbee and rleungx Dec 30, 2019
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Dec 30, 2019

Codecov Report

Merging #2053 into master will increase coverage by 0.14%.
The diff coverage is 60%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2053      +/-   ##
==========================================
+ Coverage   76.83%   76.98%   +0.14%     
==========================================
  Files         193      193              
  Lines       19536    19536              
==========================================
+ Hits        15011    15039      +28     
+ Misses       3390     3368      -22     
+ Partials     1135     1129       -6
Impacted Files Coverage Δ
server/cluster/cluster_worker.go 72.26% <0%> (ø) ⬆️
server/api/scheduler.go 40.93% <100%> (ø) ⬆️
server/cluster/coordinator.go 78.57% <50%> (+0.82%) ⬆️
server/api/util.go 67.3% <50%> (ø) ⬆️
pkg/btree/btree.go 86.84% <0%> (-0.81%) ⬇️
server/cluster/cluster.go 81.31% <0%> (-0.47%) ⬇️
server/core/storage.go 73.43% <0%> (ø) ⬆️
server/grpc_service.go 57.66% <0%> (+0.43%) ⬆️
... and 7 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 e02828a...d68a9db. Read the comment docs.

writeFlowScoreInfos := NormalizeStoresStats(storesStats.GetStoresBytesWriteStat())
readFlowMean := MeanStoresStats(storesStats.GetStoresBytesReadStat())
writeFlowMean := MeanStoresStats(storesStats.GetStoresBytesWriteStat())
func filterUnhealthyStore(cluster opt.Cluster, storeStatsMap map[uint64]float64) {

This comment has been minimized.

Copy link
@nolouch

nolouch Dec 31, 2019

Member

why not use healthFilter?

This comment has been minimized.

Copy link
@lhy1024

lhy1024 Dec 31, 2019

Author Member

because healthFilter need storeInfo, but we only have storeID and score in this function.

server/schedulers/hot_region.go Outdated Show resolved Hide resolved
@lhy1024 lhy1024 force-pushed the lhy1024:new-hot branch 2 times, most recently from cb172cd to b6f4139 Dec 31, 2019
@nolouch

This comment has been minimized.

Copy link
Member

nolouch commented Jan 2, 2020

Have you tested the effect of this PR?

@lhy1024 lhy1024 added this to the v4.0.0-beta milestone Jan 2, 2020
lhy1024 added 3 commits Dec 30, 2019
…ad-based scheduler #1754

Signed-off-by: lhy1024 <admin@liudos.us>
Signed-off-by: lhy1024 <admin@liudos.us>
fix merge
Signed-off-by: lhy1024 <admin@liudos.us>
@lhy1024 lhy1024 force-pushed the lhy1024:new-hot branch from 2ee613e to ede24b0 Jan 6, 2020
server/schedulers/hot_region.go Outdated Show resolved Hide resolved
Co-Authored-By: Luffbee <luffbee@outlook.com>
server/schedulers/hot_test.go Outdated Show resolved Hide resolved
Signed-off-by: lhy1024 <admin@liudos.us>
@Luffbee
Luffbee approved these changes Jan 8, 2020
@lhy1024

This comment has been minimized.

Copy link
Member Author

lhy1024 commented Jan 8, 2020

@lhy1024

This comment has been minimized.

Copy link
Member Author

lhy1024 commented Jan 8, 2020

would you like to show the test result @Luffbee

@Luffbee

This comment has been minimized.

Copy link
Contributor

Luffbee commented Jan 8, 2020

The most important change of this PR is that it balance hot leader by count. So, I will only give leader distribution graph
image
There are 3 part of this graph. The first one comes from the latest master branch, the other two comes from this PR. We can see that the leader distribution is balanced in this PR.

Signed-off-by: lhy1024 <admin@liudos.us>
server/schedulers/hot_region.go Outdated Show resolved Hide resolved
lhy1024 added 2 commits Jan 9, 2020
Signed-off-by: lhy1024 <admin@liudos.us>
@nolouch
nolouch approved these changes Jan 9, 2020
@nolouch

This comment has been minimized.

Copy link
Member

nolouch commented Jan 9, 2020

/merge

@sre-bot sre-bot added the CanMerge label Jan 9, 2020
@sre-bot

This comment has been minimized.

Copy link
Contributor

sre-bot commented Jan 9, 2020

/run-all-tests

2 similar comments
@lhy1024

This comment has been minimized.

Copy link
Member Author

lhy1024 commented Jan 9, 2020

/run-all-tests

@lhy1024

This comment has been minimized.

Copy link
Member Author

lhy1024 commented Jan 9, 2020

/run-all-tests

@qrr1995

This comment has been minimized.

Copy link

qrr1995 commented Jan 9, 2020

/build

opInfo := e.Value.(*pendingInfluence)
if opInfo.isDone() {
if time.Now().After(opInfo.op.GetCreateTime().Add(statistics.StoreHeartBeatReportInterval * time.Second)) {
schedulerStatus.WithLabelValues(h.GetName(), "pending_op_infos").Dec()

This comment has been minimized.

Copy link
@rleungx

rleungx Jan 9, 2020

Member

Do we need to tell apart the different operators?

This comment has been minimized.

Copy link
@lhy1024

lhy1024 Jan 9, 2020

Author Member

This comment has been minimized.

Copy link
@lhy1024

lhy1024 Jan 9, 2020

Author Member

Or is it necessary to keep the metrics ?

This comment has been minimized.

Copy link
@Luffbee

Luffbee Jan 9, 2020

Contributor

Keep it. Change it in the future if necessary.

@lhy1024

This comment has been minimized.

Copy link
Member Author

lhy1024 commented Jan 9, 2020

/merge

@sre-bot

This comment has been minimized.

Copy link
Contributor

sre-bot commented Jan 9, 2020

/run-all-tests

@sre-bot

This comment has been minimized.

Copy link
Contributor

sre-bot commented Jan 9, 2020

@lhy1024 merge failed.

@lhy1024

This comment has been minimized.

Copy link
Member Author

lhy1024 commented Jan 9, 2020

/merge

@sre-bot

This comment has been minimized.

Copy link
Contributor

sre-bot commented Jan 9, 2020

/run-all-tests

@sre-bot

This comment has been minimized.

Copy link
Contributor

sre-bot commented Jan 9, 2020

@lhy1024 merge failed.

@lhy1024

This comment has been minimized.

Copy link
Member Author

lhy1024 commented Jan 9, 2020

/merge

@sre-bot

This comment has been minimized.

Copy link
Contributor

sre-bot commented Jan 9, 2020

/run-all-tests

@sre-bot sre-bot merged commit 49f194a into pingcap:master Jan 9, 2020
8 checks passed
8 checks passed
Hound No violations found. Woof!
continuous-integration/travis-ci/pr The Travis CI build passed
Details
idc-jenkins-ci-pd/integration-common-test Jenkins job succeeded.
Details
idc-jenkins-ci-pd/integration-compatibility-test Jenkins job succeeded.
Details
idc-jenkins-ci-pd/integration-ddl-test Jenkins job succeeded.
Details
idc-jenkins-ci/build Jenkins job succeeded.
Details
idc-jenkins-ci/test Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details
@lhy1024 lhy1024 deleted the lhy1024:new-hot branch Jan 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.