schedule: combine store limit and store balance rate #2437

rleungx · 2020-05-21T08:23:07Z

What problem does this PR solve?

Close #2245.

What is changed and how it works?

This PR mainly does these things:

remove store-balance-rate
persist store limit in schedule config
change the auto store limit policy
rename *region to *peer
move RemoveScheduler from Options to Cluster interface
some updates for testing and doc.

Check List

Tests

Unit test

Release note

No release note

codecov-commenter · 2020-06-10T06:21:54Z

Codecov Report

Merging #2437 into master will increase coverage by 0.37%.
The diff coverage is 84.54%.

@@            Coverage Diff             @@
##           master    #2437      +/-   ##
==========================================
+ Coverage   77.06%   77.43%   +0.37%     
==========================================
  Files         204      204              
  Lines       22079    22121      +42     
==========================================
+ Hits        17015    17130     +115     
+ Misses       3773     3714      -59     
+ Partials     1291     1277      -14

Impacted Files	Coverage Δ
server/statistics/store_collection.go	`59.70% <ø> (+0.44%)`	⬆️
server/cluster/store_limiter.go	`53.70% <46.15%> (ø)`
server/api/store.go	`65.50% <60.00%> (+0.15%)`	⬆️
pkg/mock/mockoption/mockoption.go	`90.76% <79.48%> (-4.98%)`	⬇️
server/schedule/operator_controller.go	`79.27% <83.33%> (+0.61%)`	⬆️
server/config/persist_options.go	`89.77% <86.95%> (-0.69%)`	⬇️
server/schedule/storelimit/store_limit.go	`84.00% <88.88%> (+8.24%)`	⬆️
server/handler.go	`53.20% <90.62%> (+1.59%)`	⬆️
server/cluster/cluster.go	`80.65% <92.00%> (+0.71%)`	⬆️
pkg/mock/mockcluster/mockcluster.go	`91.75% <100.00%> (+0.18%)`	⬆️
... and 28 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 0df431e...6dd271c. Read the comment docs.

tools/pd-ctl/pdctl/command/config_command.go

lhy1024 · 2020-06-11T09:27:49Z

server/statistics/store_collection.go

@@ -148,7 +148,6 @@ func (s *storeStatistics) Collect() {
 	configs["high-space-ratio"] = s.opt.GetHighSpaceRatio()
 	configs["low-space-ratio"] = s.opt.GetLowSpaceRatio()
 	configs["tolerant-size-ratio"] = s.opt.GetTolerantSizeRatio()
-	configs["store-balance-rate"] = s.opt.GetStoreBalanceRate()


when users try to set this config, will we show that it has been expired?

It will show the message now.

lhy1024 · 2020-06-11T09:32:17Z

pkg/mock/mockcluster/mockcluster.go

@@ -539,6 +546,11 @@ func (mc *Cluster) GetMaxReplicas() int {
 	return mc.ScheduleOptions.GetMaxReplicas()
 }

+// GetStoreLimitByType mocks method.


Do we need to emphasize ByType?

what do you mean "emphasize ByType"?

how about replacing with GetStoreLimit？

I think both GetStoreLimit and GetStoreLimitByType are ok here.

server/api/store.go

server/schedule/operator_controller_test.go

server/handler.go

server/cluster/cluster.go

server/config/config.go

tools/pd-ctl/README.md

rleungx · 2020-06-17T08:29:01Z

/run-all-tests

rleungx · 2020-06-17T08:41:40Z

/run-all-tests

nolouch

rest LGTM..

server/config/config.go

nolouch · 2020-06-17T17:23:25Z

tools/pd-ctl/README.md

->> store limit region-remove           // Show limits of removing region operation for all stores
+>> store limit                         // Show limits of adding peer operation for all stores
+>> store limit add-peer                // Show limits of adding peer operation for all stores
+>> store limit remove-peer             // Show limits of removing peer operation for all stores
 >> store limit all 5                   // Limit 5 adding region operations per minute for all stores


store limit all 5

the command only set for add-peer, can we set remove-peer at the same time?

I'm not sure it's better. What is your opinion? @lhy1024

Before, the user only needed one command. Now users think that the speed has been increased, but in fact there is still a speed limit for remove-peer.

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

rleungx · 2020-06-18T08:22:06Z

@lhy1024 @nolouch PTAL

pkg/mock/mockoption/mockoption.go

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

lhy1024

the rest LGTM

lhy1024 · 2020-06-18T09:01:29Z

server/handler.go

-	c, err := h.GetOperatorController()
-	if err != nil {
-		return err
+func (h *Handler) SetAllStoresLimit(ratePerMin float64, limitType storelimit.Type) error {


I found there is a lot of code about set storelimit, I don’t know if they can be simplified

nolouch · 2020-06-18T16:57:49Z

/merge

ti-srebot · 2020-06-18T17:01:10Z

/run-all-tests

ti-srebot · 2020-06-18T17:09:33Z

cherry pick to release-4.0 failed

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

rleungx force-pushed the refactor-store-limit branch from 284e68f to e916505 Compare May 21, 2020 08:44

wangrzneu mentioned this pull request Jun 9, 2020

Solve the issue: Consider set suitable default store limit of the tiflash node #2521

Closed

rleungx force-pushed the refactor-store-limit branch from e916505 to b42a43a Compare June 10, 2020 06:05

rleungx marked this pull request as ready for review June 10, 2020 06:11

rleungx requested review from lhy1024, HunDunDM and nolouch June 10, 2020 06:11

lhy1024 reviewed Jun 11, 2020

View reviewed changes

nolouch added the component/schedule Scheduling logic. label Jun 11, 2020

nolouch reviewed Jun 15, 2020

View reviewed changes

server/config/config.go Show resolved Hide resolved

tools/pd-ctl/README.md Show resolved Hide resolved

rleungx force-pushed the refactor-store-limit branch from 6dd271c to ea46523 Compare June 16, 2020 06:42

nolouch added the needs-cherry-pick-release-4.0 The PR needs to cherry pick to release-4.0 branch. label Jun 17, 2020

nolouch reviewed Jun 17, 2020

View reviewed changes

rleungx added 6 commits June 18, 2020 15:21

combine store limit and store balance rate

8d01f32

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

resolve the conflict

faef13d

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

address the comment

a242570

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

handle the compatible problems

9fd7bba

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

add a notice in docs

8797c82

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

change the default behavior without specified type

ce5764e

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

rleungx force-pushed the refactor-store-limit branch from e4c8e79 to ce5764e Compare June 18, 2020 08:09

nolouch reviewed Jun 18, 2020

View reviewed changes

pkg/mock/mockoption/mockoption.go Show resolved Hide resolved

add default store limit mode back

13919bc

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

lhy1024 reviewed Jun 18, 2020

View reviewed changes

nolouch approved these changes Jun 18, 2020

View reviewed changes

lhy1024 approved these changes Jun 18, 2020

View reviewed changes

ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Jun 18, 2020

Merge branch 'master' into refactor-store-limit

152b71e

ti-srebot merged commit 444498e into tikv:master Jun 18, 2020

rleungx added a commit to rleungx/pd that referenced this pull request Jun 19, 2020

schedule: combine store limit and store balance rate (tikv#2437)

15c704e

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

rleungx mentioned this pull request Jun 19, 2020

schedule: combine store limit and store balance rate (#2437) #2557

Merged

ti-srebot pushed a commit that referenced this pull request Jun 19, 2020

schedule: combine store limit and store balance rate (#2437) (#2557)

0534436

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

schedule: combine store limit and store balance rate #2437

schedule: combine store limit and store balance rate #2437

rleungx commented May 21, 2020

codecov-commenter commented Jun 10, 2020 •

edited

Loading

lhy1024 Jun 11, 2020

rleungx Jun 16, 2020

lhy1024 Jun 11, 2020

rleungx Jun 15, 2020

lhy1024 Jun 15, 2020

rleungx Jun 15, 2020

rleungx commented Jun 17, 2020

rleungx commented Jun 17, 2020

nolouch left a comment

nolouch Jun 17, 2020

rleungx Jun 18, 2020

nolouch Jun 18, 2020 •

edited

Loading

rleungx commented Jun 18, 2020

lhy1024 left a comment

lhy1024 Jun 18, 2020

nolouch commented Jun 18, 2020

ti-srebot commented Jun 18, 2020

ti-srebot commented Jun 18, 2020

schedule: combine store limit and store balance rate #2437

schedule: combine store limit and store balance rate #2437

Conversation

rleungx commented May 21, 2020

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

codecov-commenter commented Jun 10, 2020 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rleungx commented Jun 17, 2020

rleungx commented Jun 17, 2020

nolouch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nolouch Jun 18, 2020 • edited Loading

Choose a reason for hiding this comment

rleungx commented Jun 18, 2020

lhy1024 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nolouch commented Jun 18, 2020

ti-srebot commented Jun 18, 2020

ti-srebot commented Jun 18, 2020

codecov-commenter commented Jun 10, 2020 •

edited

Loading

nolouch Jun 18, 2020 •

edited

Loading