Support to set store limit in auto or manual mode #1887

shafreeck · 2019-11-04T10:20:08Z

What problem does this PR solve?

Support to set store limit in auto or manual mode, it is useful when we introduce the feature to tune store limits automatically.

What is changed and how it works?

Add two mode for setting limit: auto or manual, an auto-set value can be overwritten by a manual-set value, otherwise it is forbidden.
Wrap the store limit in the StoreLimit struct
Extract the store limit related code into a separate file

Check List

Tests

Unit test

Code changes

Has HTTP API interfaces change

Related changes

Need to update the documentation
Need to be included in the release notes

codecov-io · 2019-11-06T06:00:37Z

Codecov Report

Merging #1887 into master will decrease coverage by 0.03%.
The diff coverage is 93.87%.

@@            Coverage Diff             @@
##           master    #1887      +/-   ##
==========================================
- Coverage   77.43%   77.39%   -0.04%     
==========================================
  Files         177      178       +1     
  Lines       18120    18142      +22     
==========================================
+ Hits        14031    14041      +10     
- Misses       3006     3016      +10     
- Partials     1083     1085       +2

Impacted Files	Coverage Δ
server/schedule/operator_controller.go	`83.57% <100%> (-0.12%)`	⬇️
server/handler.go	`52.35% <100%> (-0.48%)`	⬇️
server/api/store.go	`66.8% <100%> (+0.4%)`	⬆️
server/schedule/store_limit.go	`86.95% <86.95%> (ø)`
pkg/testutil/operator_check.go	`88.88% <0%> (-11.12%)`	⬇️
pkg/metricutil/metricutil.go	`90.62% <0%> (-9.38%)`	⬇️
server/schedulers/hot_region.go	`87.08% <0%> (-1.66%)`	⬇️
server/tso/tso.go	`78.83% <0%> (-1.46%)`	⬇️
server/server.go	`82.51% <0%> (-0.99%)`	⬇️
server/config/option.go	`90.32% <0%> (-0.93%)`	⬇️
... and 14 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 035f65e...71ea54d. Read the comment docs.

server/schedule/operator_controller.go

server/api/store.go

lhy1024

LGTM. But why do we need to set different mode?

Signed-off-by: Shafreeck Sea <shafreeck@gmail.com>

The store limit is wrapped by a struct called StoreLimit, it exports the useful api of the ratelimit.Bucket. It records the limit's setting mode which can be "auto" or "manual". An "manual" set value can overwrite an "auto set value, otherwise it is forbidden. Signed-off-by: Shafreeck Sea <shafreeck@gmail.com>

Signed-off-by: Shafreeck Sea <shafreeck@gmail.com>

shafreeck · 2019-11-20T07:20:54Z

LGTM. But why do we need to set different mode?

@lhy1024 The store limit value set by PD maybe not the desired value wanted by the user in which case the user prefers to overwrite the value manually. A value set by the user should not be overwritten by PD.

server/schedule/operator_controller.go

shafreeck · 2019-12-02T09:42:52Z

@lhy1024 @rleungx PTAL

lhy1024

the rest LGTM

server/handler.go

server/schedule/store_limit.go

server/schedule/operator_controller.go

server/schedule/store_limit.go

rleungx

The rest LGTM

lhy1024

LGTM

shafreeck · 2019-12-03T09:40:08Z

@rleungx Would you mind to approve this PR if there are no more questions?

sre-bot · 2019-12-04T03:42:12Z

/run-all-tests

sre-bot · 2019-12-04T03:44:16Z

@shafreeck merge failed.

nolouch · 2019-12-05T04:10:17Z

ci failed @shafreeck

shafreeck added type/enhancement The issue or PR belongs to an enhancement. DNM labels Nov 4, 2019

shafreeck self-assigned this Nov 4, 2019

shafreeck changed the title ~~WIP: Support to set store limit in auto or manual mode~~ Support to set store limit in auto or manual mode Nov 6, 2019

shafreeck removed the DNM label Nov 6, 2019

shafreeck requested review from rleungx and nolouch November 6, 2019 03:50

shafreeck force-pushed the store-limit-mode branch from 4835cf9 to b5a2898 Compare November 6, 2019 06:01

shafreeck mentioned this pull request Nov 6, 2019

Adjust the store limit according to the cluster state dynamically #1902

Merged

shafreeck requested review from disksing and lhy1024 November 6, 2019 09:16

rleungx reviewed Nov 6, 2019

View reviewed changes

server/schedule/operator_controller.go Outdated Show resolved Hide resolved

rleungx reviewed Nov 6, 2019

View reviewed changes

server/api/store.go Show resolved Hide resolved

lhy1024 reviewed Nov 12, 2019

View reviewed changes

shafreeck added 6 commits November 20, 2019 14:48

feature: add the store limit mode definition

c699471

Signed-off-by: Shafreeck Sea <shafreeck@gmail.com>

store-limit: fix the returned value

c6c9089

Signed-off-by: Shafreeck Sea <shafreeck@gmail.com>

store_limit: fix test cases

74a7e1b

Signed-off-by: Shafreeck Sea <shafreeck@gmail.com>

tests: format file

46d962d

Signed-off-by: Shafreeck Sea <shafreeck@gmail.com>

schedule: use the public api directly

aaa70d9

Signed-off-by: Shafreeck Sea <shafreeck@gmail.com>

shafreeck force-pushed the store-limit-mode branch from b5a2898 to aaa70d9 Compare November 20, 2019 06:52

schedule: fix unit test compiling error

b98db78

Signed-off-by: Shafreeck Sea <shafreeck@gmail.com>

rleungx reviewed Nov 28, 2019

View reviewed changes

server/schedule/operator_controller.go Outdated Show resolved Hide resolved

lhy1024 reviewed Dec 2, 2019

View reviewed changes

server/handler.go Outdated Show resolved Hide resolved

server/schedule/store_limit.go Show resolved Hide resolved

server/schedule/store_limit.go Show resolved Hide resolved

server/schedule/operator_controller.go Show resolved Hide resolved

rleungx reviewed Dec 2, 2019

View reviewed changes

server/schedule/store_limit.go Show resolved Hide resolved

rleungx reviewed Dec 2, 2019

View reviewed changes

lhy1024 approved these changes Dec 3, 2019

View reviewed changes

rleungx approved these changes Dec 3, 2019

View reviewed changes

shafreeck added the status/can-merge Indicates a PR has been approved by a committer. label Dec 4, 2019

Merge branch 'master' into store-limit-mode

f67c783

Merge branch 'master' into store-limit-mode

71ea54d

shafreeck merged commit d11bff1 into tikv:master Dec 5, 2019

nolouch added this to the v4.0.0-beta milestone Jan 16, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support to set store limit in auto or manual mode #1887

Support to set store limit in auto or manual mode #1887

shafreeck commented Nov 4, 2019 •

edited

Loading

codecov-io commented Nov 6, 2019 •

edited

Loading

lhy1024 left a comment

shafreeck commented Nov 20, 2019 •

edited

Loading

shafreeck commented Dec 2, 2019

lhy1024 left a comment

rleungx left a comment

lhy1024 left a comment

shafreeck commented Dec 3, 2019

sre-bot commented Dec 4, 2019

sre-bot commented Dec 4, 2019

nolouch commented Dec 5, 2019

Support to set store limit in auto or manual mode #1887

Support to set store limit in auto or manual mode #1887

Conversation

shafreeck commented Nov 4, 2019 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

codecov-io commented Nov 6, 2019 • edited Loading

Codecov Report

lhy1024 left a comment

Choose a reason for hiding this comment

shafreeck commented Nov 20, 2019 • edited Loading

shafreeck commented Dec 2, 2019

lhy1024 left a comment

Choose a reason for hiding this comment

rleungx left a comment

Choose a reason for hiding this comment

lhy1024 left a comment

Choose a reason for hiding this comment

shafreeck commented Dec 3, 2019

sre-bot commented Dec 4, 2019

sre-bot commented Dec 4, 2019

nolouch commented Dec 5, 2019

shafreeck commented Nov 4, 2019 •

edited

Loading

codecov-io commented Nov 6, 2019 •

edited

Loading

shafreeck commented Nov 20, 2019 •

edited

Loading