schedulers: add reload config function #7406

lhy1024 · 2023-11-21T16:49:34Z

What problem does this PR solve?

Issue Number: Close #7257

What is changed and how does it work?

allow all scheduler test to run in two modes
refactor lock
add reload config function

Check List

Tests

Unit test

Release note

None.

Signed-off-by: lhy1024 <[email protected]>

ti-chi-bot · 2023-11-21T16:49:37Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

JmPotato
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.

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

lhy1024 · 2023-11-21T16:51:41Z

pkg/schedule/schedulers/balance_region.go

@@ -51,6 +51,8 @@ var (
 type balanceRegionSchedulerConfig struct {
 	Name   string          `json:"name"`
 	Ranges []core.KeyRange `json:"ranges"`
+	// TODO: whether to support reload it?


We need to talk about the schedulers whose config only contains Ranges. Do we need to support ReloadConfig for them?

After discussion, we will keep TODO for these schedulers.

Signed-off-by: lhy1024 <[email protected]>

codecov · 2023-11-21T17:44:44Z

Codecov Report

Merging #7406 (0efb536) into master (8e6dcfb) will decrease coverage by 0.02%.
The diff coverage is 81.04%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7406      +/-   ##
==========================================
- Coverage   74.24%   74.23%   -0.02%     
==========================================
  Files         451      451              
  Lines       49180    49386     +206     
==========================================
+ Hits        36515    36663     +148     
- Misses       9430     9454      +24     
- Partials     3235     3269      +34

Flag	Coverage Δ
unittests	`74.23% <81.04%> (-0.02%)`	⬇️

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

Signed-off-by: lhy1024 <[email protected]>

pkg/schedule/schedulers/evict_slow_store.go

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

rleungx · 2023-11-22T06:21:03Z

pkg/schedule/schedulers/shuffle_hot_region.go

+	if len(cfgData) == 0 {
+		return nil
+	}
+	newCfg := &shuffleHotRegionSchedulerConfig{}


When do we need to create a newCfg instead of using the old one?

When the scheduling server watches the change of config, it will call ReloadConfig to update it.

I mean such as splitBucketScheduler, the ReloadConfig doesn't create a newCfg.

pkg/schedule/schedulers/evict_slow_trend.go

pkg/schedule/schedulers/evict_slow_store.go

JmPotato · 2023-11-22T08:42:18Z

/merge

ti-chi-bot · 2023-11-22T08:42:20Z

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

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

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 · 2023-11-22T08:42:22Z

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

Commit hash: 1a9a793

ti-chi-bot · 2023-11-22T08:42:35Z

@lhy1024: Your PR was out of date, I have automatically updated it for you.

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

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.

lhy1024 · 2023-11-22T08:44:18Z

/hold

Signed-off-by: lhy1024 <[email protected]>

lhy1024 · 2023-11-22T09:02:42Z

After discussion, we will keep TODO for these schedulers.

lhy1024 · 2023-11-22T09:03:14Z

/merge

ti-chi-bot · 2023-11-22T09:03:16Z

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

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

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 · 2023-11-22T09:03:18Z

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

Commit hash: 0efb536

close tikv#7257 Signed-off-by: lhy1024 <[email protected]>

schedulers: add reload config function

240728f

Signed-off-by: lhy1024 <[email protected]>

lhy1024 requested review from JmPotato, CabinfeverB and rleungx November 21, 2023 16:49

ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/needs-triage-completed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed do-not-merge/needs-triage-completed labels Nov 21, 2023

lhy1024 commented Nov 21, 2023

View reviewed changes

lhy1024 added 2 commits November 22, 2023 01:09

fix lock

8a2100f

Signed-off-by: lhy1024 <[email protected]>

fix test

081f3af

Signed-off-by: lhy1024 <[email protected]>

lhy1024 added 3 commits November 22, 2023 04:23

address comments

e2f4e3f

Signed-off-by: lhy1024 <[email protected]>

replace sync.Mutex with syncutil

20cab6d

Signed-off-by: lhy1024 <[email protected]>

address comments

129e348

Signed-off-by: lhy1024 <[email protected]>

rleungx reviewed Nov 22, 2023

View reviewed changes

rleungx approved these changes Nov 22, 2023

View reviewed changes

ti-chi-bot bot added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 22, 2023

JmPotato approved these changes Nov 22, 2023

View reviewed changes

ti-chi-bot bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Nov 22, 2023

ti-chi-bot bot added the status/can-merge Indicates a PR has been approved by a committer. label Nov 22, 2023

ti-chi-bot bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 22, 2023

update TODO

792f26c

Signed-off-by: lhy1024 <[email protected]>

lhy1024 force-pushed the reload-config branch from 64ac60c to 792f26c Compare November 22, 2023 09:02

ti-chi-bot bot removed the status/can-merge Indicates a PR has been approved by a committer. label Nov 22, 2023

lhy1024 removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 22, 2023

Merge branch 'master' into reload-config

0efb536

ti-chi-bot bot added the status/can-merge Indicates a PR has been approved by a committer. label Nov 22, 2023

ti-chi-bot bot merged commit fe84829 into tikv:master Nov 22, 2023
23 checks passed

lhy1024 mentioned this pull request Nov 22, 2023

mcs: forward request to scheduling server #7178

Closed

25 tasks

rleungx mentioned this pull request Nov 30, 2023

Separate scheduling into an independent service #5839

Closed

rleungx pushed a commit to rleungx/pd that referenced this pull request Dec 1, 2023

schedulers: add reload config function (tikv#7406)

cad53c9

close tikv#7257 Signed-off-by: lhy1024 <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

schedulers: add reload config function #7406

schedulers: add reload config function #7406

lhy1024 commented Nov 21, 2023 •

edited

Loading

ti-chi-bot bot commented Nov 21, 2023 •

edited

Loading

lhy1024 Nov 21, 2023 •

edited

Loading

lhy1024 Nov 22, 2023

codecov bot commented Nov 21, 2023 •

edited

Loading

rleungx Nov 22, 2023

lhy1024 Nov 22, 2023

rleungx Nov 22, 2023

JmPotato commented Nov 22, 2023

ti-chi-bot bot commented Nov 22, 2023

ti-chi-bot bot commented Nov 22, 2023

ti-chi-bot bot commented Nov 22, 2023

lhy1024 commented Nov 22, 2023

lhy1024 commented Nov 22, 2023

lhy1024 commented Nov 22, 2023

ti-chi-bot bot commented Nov 22, 2023

ti-chi-bot bot commented Nov 22, 2023

schedulers: add reload config function #7406

schedulers: add reload config function #7406

Conversation

lhy1024 commented Nov 21, 2023 • edited Loading

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

ti-chi-bot bot commented Nov 21, 2023 • edited Loading

lhy1024 Nov 21, 2023 • edited Loading

Choose a reason for hiding this comment

lhy1024 Nov 22, 2023

Choose a reason for hiding this comment

codecov bot commented Nov 21, 2023 • edited Loading

Codecov Report

rleungx Nov 22, 2023

Choose a reason for hiding this comment

lhy1024 Nov 22, 2023

Choose a reason for hiding this comment

rleungx Nov 22, 2023

Choose a reason for hiding this comment

JmPotato commented Nov 22, 2023

ti-chi-bot bot commented Nov 22, 2023

ti-chi-bot bot commented Nov 22, 2023

ti-chi-bot bot commented Nov 22, 2023

lhy1024 commented Nov 22, 2023

lhy1024 commented Nov 22, 2023

lhy1024 commented Nov 22, 2023

ti-chi-bot bot commented Nov 22, 2023

ti-chi-bot bot commented Nov 22, 2023

lhy1024 commented Nov 21, 2023 •

edited

Loading

ti-chi-bot bot commented Nov 21, 2023 •

edited

Loading

lhy1024 Nov 21, 2023 •

edited

Loading

codecov bot commented Nov 21, 2023 •

edited

Loading