server/schedulers: introduce evict-slow-store-scheduler

[5kbpers]

What problem does this PR solve?

For addressing tikv/tikv#10539, we will introduce a slow score concept into tikv and report it to pd in store heartbeats. We then implement evict-slow-store-scheduler to detect and evict all leaders from one store that was considered slow.

Blocked by pingcap/kvproto#789

What is changed and how it works?

• implement evict-slow-store-scheduler
• show slow score in store info

Check List

Tests

• Unit test
• Integration test

Code changes

• Has persistent data change

Related changes

• Need to cherry-pick to the release branch

Release note

introduce evict-slow-store-scheduler for detecting and evicting slow stores

[ti-chi-bot]

[REVIEW NOTIFICATION]

This pull request has been approved by:

• disksing
• rleungx

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.

Details

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

[codecov]

Codecov Report

Merging #3922 (6451ef4) into master (b7ab40c) will decrease coverage by 0.06%.
The diff coverage is 61.67%.

@@            Coverage Diff             @@
##           master    #3922      +/-   ##
==========================================
- Coverage   74.65%   74.58%   -0.07%     
==========================================
  Files         246      247       +1     
  Lines       25135    25276     +141     
==========================================
+ Hits        18764    18852      +88     
- Misses       4714     4748      +34     
- Partials     1657     1676      +19     

Flag	Coverage Δ
unittests	74.58% <61.67%> (-0.07%)	⬇️

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

Impacted Files	Coverage Δ
server/api/scheduler.go	48.58% <0.00%> (-1.13%)	⬇️
server/cluster/cluster.go	82.33% <0.00%> (-0.20%)	⬇️
server/handler.go	43.38% <0.00%> (-0.11%)	⬇️
server/statistics/store_collection.go	93.66% <33.33%> (-2.03%)	⬇️
server/schedulers/evict_slow_store.go	52.22% <52.22%> (ø)
server/core/store.go	80.00% <63.63%> (-1.52%)	⬇️
server/schedulers/evict_leader.go	74.50% <84.61%> (+0.12%)	⬆️
server/api/store.go	65.24% <100.00%> (+0.10%)	⬆️
server/core/basic_cluster.go	91.27% <100.00%> (+0.31%)	⬆️
server/core/store_option.go	100.00% <100.00%> (ø)
... and 20 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 b0c9916...6451ef4. Read the comment docs.

[disksing]

Hi, I have some concerns about it:

• Can it really solve the problems of poor tikv performance, considering that even if the leader is removed, follower writes may still bring other problems when the speed is too slow.
• What should be the behavior if all tikv scores are low?
• The condition whether it is slow or not should be added to the stateFilter, otherwise other schedulers will conflict with evict-slow-scheduler.

[5kbpers]

Hi, I have some concerns about it:

• Can it really solve the problems of poor tikv performance, considering that even if the leader is removed, follower writes may still bring other problems when the speed is too slow.
• What should be the behavior if all tikv scores are low?
• The condition whether it is slow or not should be added to the stateFilter, otherwise other schedulers will conflict with evict-slow-scheduler.

• Currently we can only handle the situation that only one tikv gets slow down. Then the majority of replicas could have the ability to continue to serve requests without serious regression in most cases
• If most of TiKVs get slow down, the only thing we can do is alerting the user and handle it manually

[disksing]

As we can evict leader from only 1 slow store, why use a vector to store the ID>

[5kbpers]

I thought there would be a chance to introduce a more complex strategy for this scheduler. So use a vector here for keeping flexibility.

[disksing]

I think it is not a very good idea to use another scheduler as a toolkit because each scheduler has its own context. For example, scheduler names will be recorded to the filter metrics, it may happen that we want to record evict-slow-store-scheduler and end up with evict-leader-scheduler.
There are two possible ways to improve this: a) reimplement the similar code again b) extract the common parts and let different schedulers call them

[5kbpers]

The logic is basically the same so I think it would be better to reuse it directly. About the metrics, I saw that it is based on the name of the scheduler, may setting a customized name is a good solution?

[disksing]

Consider extracting the common part?

[lhy1024]

This pr will release in sprint4, PTAL

[bufferflies]

the schedule should add store_id param???

[5kbpers]

No, it just needs the id of the previous evicted store which was stored in etcd.

[rleungx]

Can we reuse PauseLeaderTransfer/ResumeLeaderTransfer?

[5kbpers]

That will make this scheduler conflicts with evict-leader-scheduler

[5kbpers]

/merge

[ti-chi-bot]

@5kbpers: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

Details

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]

@5kbpers: /merge is only allowed for the committers, you can assign this pull request to the committer in list by filling /assign @committer in the comment to help merge this pull request.

Details

In response to this:

/merge

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.

[disksing]

/merge

[ti-chi-bot]

@disksing: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

Details

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]

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

Details

Commit hash: 6451ef4