scheduler: enhance output when scheduler config update #6927

lhy1024 · 2023-08-09T11:07:14Z

What problem does this PR solve?

Issue Number: Close #4607

What is changed and how does it work?

reduce unnecessary config check in pd-ctl.
return result when run successfully
add log when config changed

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)

Release note

None.

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

ti-chi-bot · 2023-08-09T11:07:16Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

HuSharp
nolouch

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.

codecov · 2023-08-09T11:21:38Z

Codecov Report

Merging #6927 (8cf2243) into master (a7353bb) will increase coverage by 0.09%.
Report is 2 commits behind head on master.
The diff coverage is 72.22%.

❗ Current head 8cf2243 differs from pull request most recent head c36f8dd. Consider uploading reports for the commit c36f8dd to get more accurate results

@@            Coverage Diff             @@
##           master    #6927      +/-   ##
==========================================
+ Coverage   74.22%   74.32%   +0.09%     
==========================================
  Files         433      434       +1     
  Lines       46139    46073      -66     
==========================================
- Hits        34247    34242       -5     
+ Misses       8876     8821      -55     
+ Partials     3016     3010       -6

Flag	Coverage Δ
unittests	`74.32% <72.22%> (+0.09%)`	⬆️

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

server/api/scheduler_test.go

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

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

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

nolouch · 2023-08-28T05:31:19Z

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

-			cmd.Println("priorities shouldn't be repeated")
-			return
-		}
+		input[key] = strings.Split(value, ",")


why remove it? I think the check priority should be one of [%s, %s, %s] is necessary.

http interface will check it also

Can you point out the exact address which checked?

pd/pkg/schedule/schedulers/hot_region_config.go

Line 352 in 39cff3b

func isPriorityValid(priorities []string) (map[string]bool, error) {

HuSharp

btw, I noticed there are too many api/v1/scheduler-config var, do we need to unify them?

pkg/schedule/schedulers/balance_witness.go

pkg/schedule/schedulers/hot_region_config.go

HuSharp · 2023-08-30T09:42:33Z

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

-			cmd.Println("priorities shouldn't be repeated")
-			return
-		}
+		input[key] = strings.Split(value, ",")


Can you point out the exact address which checked?

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

nolouch

lgtm

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

HuSharp

LGTM! Thx for your work!

lhy1024 · 2023-08-31T09:29:28Z

/merge

ti-chi-bot · 2023-08-31T09:29:29Z

@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-08-31T09:29:31Z

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

Commit hash: c36f8dd

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

scheduler: enhance output when scheduler config update

14810c5

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

ti-chi-bot bot added the release-note-none Denotes a PR that doesn't merit a release note. label Aug 9, 2023

ti-chi-bot bot requested review from HunDunDM and rleungx August 9, 2023 11:07

ti-chi-bot bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 9, 2023

lhy1024 changed the title ~~scheduler: enhance scheduler config update~~ scheduler: enhance output when scheduler config update Aug 9, 2023

lhy1024 commented Aug 9, 2023

View reviewed changes

server/api/scheduler_test.go Outdated Show resolved Hide resolved

lhy1024 added 2 commits August 10, 2023 12:20

Merge branch 'master' of github.com:tikv/pd into fix-config

43892c6

add test

5a9edae

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

ti-chi-bot bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 10, 2023

nolouch reviewed Aug 10, 2023

View reviewed changes

tools/pd-ctl/pdctl/command/global.go Outdated Show resolved Hide resolved

address comments

0fac405

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

lhy1024 requested a review from nolouch August 14, 2023 07:12

nolouch reviewed Aug 28, 2023

View reviewed changes

HuSharp reviewed Aug 30, 2023

View reviewed changes

This comment was marked as duplicate.

Sign in to view

address comments

5e6a22e

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

nolouch approved these changes Aug 31, 2023

View reviewed changes

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

address comments

8cf2243

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

HuSharp approved these changes Aug 31, 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 Aug 31, 2023

Merge branch 'master' into fix-config

c36f8dd

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

ti-chi-bot bot merged commit a0b2f3d into tikv:master Aug 31, 2023
19 checks passed

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

scheduler: enhance output when scheduler config update (tikv#6927)

3da728b

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

scheduler: enhance output when scheduler config update #6927

scheduler: enhance output when scheduler config update #6927

lhy1024 commented Aug 9, 2023 •

edited

Loading

ti-chi-bot bot commented Aug 9, 2023 •

edited

Loading

codecov bot commented Aug 9, 2023 •

edited

Loading

nolouch Aug 28, 2023

lhy1024 Aug 29, 2023

HuSharp Aug 30, 2023

lhy1024 Aug 30, 2023

HuSharp left a comment

HuSharp Aug 30, 2023

This comment was marked as duplicate.

nolouch left a comment

HuSharp left a comment •

edited

Loading

lhy1024 commented Aug 31, 2023

ti-chi-bot bot commented Aug 31, 2023

ti-chi-bot bot commented Aug 31, 2023

scheduler: enhance output when scheduler config update #6927

scheduler: enhance output when scheduler config update #6927

Conversation

lhy1024 commented Aug 9, 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 Aug 9, 2023 • edited Loading

codecov bot commented Aug 9, 2023 • edited Loading

Codecov Report

nolouch Aug 28, 2023

Choose a reason for hiding this comment

lhy1024 Aug 29, 2023

Choose a reason for hiding this comment

HuSharp Aug 30, 2023

Choose a reason for hiding this comment

lhy1024 Aug 30, 2023

Choose a reason for hiding this comment

HuSharp left a comment

Choose a reason for hiding this comment

HuSharp Aug 30, 2023

Choose a reason for hiding this comment

This comment was marked as duplicate.

nolouch left a comment

Choose a reason for hiding this comment

HuSharp left a comment • edited Loading

Choose a reason for hiding this comment

lhy1024 commented Aug 31, 2023

ti-chi-bot bot commented Aug 31, 2023

ti-chi-bot bot commented Aug 31, 2023

lhy1024 commented Aug 9, 2023 •

edited

Loading

ti-chi-bot bot commented Aug 9, 2023 •

edited

Loading

codecov bot commented Aug 9, 2023 •

edited

Loading

HuSharp left a comment •

edited

Loading