PCP: Support to pause a scheduler #1831 by hrbeuyz24 · Pull Request #1942 · tikv/pd

hrbeuyz24 · 2019-11-15T08:21:15Z

Which problem does this PR solve?

What have you changed?

Add a fileld in scheduleController struct named DelayUntil which is the end timestamp that the scheduling should be blocked, add a func isPaused which return sheduler's state, when scheduler start a new work loop, it will judge scheduler whether is paused. Set DelayUntil 0 when resume a scheduler.
Add pd-ctl command like
schedule pause <name><delay>
schedule resume <name>

For example
schedule pause balance-region-scheduler 10
schedule pause all 10
schedule resume balance-region-scheduler
schedule resume all

It can pause all scheduler in once, or pause a specific scheduler for delay specific seconds.
Pause a paused scheduler will return error, resume a un-paused scheduler won't return error.

What are the type of the changes?

New feature : add pause and resume func in pd-ctl

How has this PR been tested?

manual test and unit test

sre-bot · 2019-11-15T08:21:17Z

Thanks for your contribution. If your PR get merged, you will be rewarded 0 points.

CLAassistant · 2019-11-15T08:21:27Z

All committers have signed the CLA.

lhy1024

Thanks for your contribution. If your PR get merged, you will be rewarded 0 points.

why 0 points? It should be 100 points. Our rebot still silly.

server/api/router.go

shafreeck

Thanks a lot for your contribution, nice work!

It seems a little complex to use two channels. Maybe there are some simple solutions. For example: Add a field to a scheduler's configuration like DelayUntil which is the end timestamp that the scheduling should be blocked. Return false when calling AllowSchedule which checks the value of DelayUntil. Reset the value of DelayUntil to 0 when resuming a scheduler.

Feel free to have a discussion if you have better ideas.

hrbeuyz24 · 2019-11-15T12:55:59Z

Thanks a lot for your contribution, nice work!

It seems a little complex to use two channels. Maybe there are some simple solutions. For example: Add a field to a scheduler's configuration like DelayUntil which is the end timestamp that the scheduling should be blocked. Return false when calling AllowSchedule which checks the value of DelayUntil. Reset the value of DelayUntil to 0 when resuming a scheduler.

Feel free to have a discussion if you have better ideas.

Thanks for your reply! it seems that my idea is really complex, i will try your suggestion.

hrbeuyz24 · 2019-11-16T06:27:04Z

@lhy1024 @shafreeck please review the new commit.

hrbeuyz24 · 2019-11-16T09:59:17Z

/run-all-test

rleungx · 2019-11-18T03:32:35Z

@hrbeuyz24 Thanks for your contribution! And would you mind adding some unit tests?

server/api/router.go

server/coordinator.go

server/handler.go

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

server/coordinator.go

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

hrbeuyz24 · 2019-11-20T05:46:02Z

@rleungx @shafreeck @lhy1024
add some unit tests, make some changes according the previous review.

hrbeuyz24 · 2019-11-20T06:08:51Z

/run-al-test

server/api/scheduler_test.go

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

server/handler.go

server/coordinator.go

server/api/scheduler.go

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

server/coordinator.go

codecov-io · 2019-11-20T11:43:44Z

Codecov Report

Merging #1942 into master will decrease coverage by 0.08%.
The diff coverage is 56.47%.

@@            Coverage Diff             @@
##           master    #1942      +/-   ##
==========================================
- Coverage   77.84%   77.75%   -0.09%     
==========================================
  Files         171      171              
  Lines       17126    17210      +84     
==========================================
+ Hits        13332    13382      +50     
- Misses       2753     2779      +26     
- Partials     1041     1049       +8

Impacted Files	Coverage Δ
server/api/router.go	`99.09% <100%> (ø)`	⬆️
server/api/scheduler.go	`34.84% <38.46%> (+0.39%)`	⬆️
server/handler.go	`52.13% <38.88%> (-1.09%)`	⬇️
tools/pd-ctl/pdctl/command/scheduler.go	`70.29% <50%> (-1.83%)`	⬇️
server/coordinator.go	`84.85% <84%> (-0.13%)`	⬇️
pkg/testutil/operator_check.go	`88.88% <0%> (-11.12%)`	⬇️
server/schedule/placement/rule.go	`87.87% <0%> (-6.07%)`	⬇️
pkg/etcdutil/etcdutil.go	`78.26% <0%> (-4.35%)`	⬇️
server/schedule/operator/step.go	`75.88% <0%> (-2.13%)`	⬇️
... and 8 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 c6d98e2...adaa367. Read the comment docs.

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

server/api/scheduler.go

server/api/scheduler_test.go

server/handler.go

shafreeck

LGTM. Thanks a lot for your contribution and This is pleasant cooperation.

nolouch

reset LGTM.

server/coordinator.go

server/api/router.go

sre-bot · 2019-11-22T14:42:39Z

/run-all-tests

sre-bot · 2019-11-22T14:50:43Z

Team Trash Brother complete task #1831 and get 100 score, currerent score 100

hrbeuyz24 closed this Nov 15, 2019

hrbeuyz24 reopened this Nov 15, 2019

shafreeck self-requested a review November 15, 2019 09:15

lhy1024 reviewed Nov 15, 2019

View reviewed changes

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

lhy1024 added contributor labels Nov 15, 2019

shafreeck reviewed Nov 15, 2019

View reviewed changes

you06 mentioned this pull request Nov 15, 2019

PCP/Sign Up tidb-perf-challenge/pcp#60

Open

shafreeck reviewed Nov 18, 2019

View reviewed changes

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

shafreeck reviewed Nov 18, 2019

View reviewed changes

shafreeck added the priority/P0 The issue has P0 priority. label Nov 19, 2019

rleungx reviewed Nov 20, 2019

View reviewed changes

hrbeuyz24 requested review from lhy1024, rleungx and shafreeck November 20, 2019 07:42

shafreeck reviewed Nov 20, 2019

View reviewed changes

server/coordinator.go Outdated Show resolved Hide resolved

server/coordinator.go Outdated Show resolved Hide resolved

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

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

hrbeuyz24 requested a review from shafreeck November 20, 2019 08:47

shafreeck reviewed Nov 20, 2019

View reviewed changes

server/coordinator.go Outdated Show resolved Hide resolved

hrbeuyz24 requested a review from shafreeck November 20, 2019 11:39

shafreeck reviewed Nov 20, 2019

View reviewed changes

hrbeuyz24 requested a review from shafreeck November 20, 2019 13:17

hrbeuyz24 added 11 commits November 22, 2019 16:46

add unit tests

5c5d64e

code specification

5a1074b

fix : scheduler pause bug

6a9fb2c

fix : scheduler pause bug

0ad3c5b

remove unused code

1cccf6f

eliminate race

4c6b2e6

put s.isPaused inside of s.AllowSchedule

4f45fae

modify test code

f6d8cc7

fix : change pause scheduler args' type

da64b0f

remove redundant test

8f56923

fix : change pause scheduler args' type

0d7c11f

hrbeuyz24 requested a review from shafreeck November 22, 2019 08:50

shafreeck approved these changes Nov 22, 2019

View reviewed changes

hrbeuyz24 added 2 commits November 22, 2019 16:54

remove redundant changes

e71c8e8

merge master

be2e5cc

nolouch reviewed Nov 22, 2019

View reviewed changes

server/coordinator.go Outdated Show resolved Hide resolved

lhy1024 reviewed Nov 22, 2019

View reviewed changes

server/api/router.go Show resolved Hide resolved

hrbeuyz24 added 2 commits November 22, 2019 22:23

update api document

f82795c

fix : dealyUntilf field to anonymous

adaa367

hrbeuyz24 requested review from lhy1024 and nolouch November 22, 2019 14:31

nolouch approved these changes Nov 22, 2019

View reviewed changes

nolouch added the status/can-merge Indicates a PR has been approved by a committer. label Nov 22, 2019

Merge branch 'master' into feat-pause_scheduler

e840dc8

sre-bot merged commit cdd258b into tikv:master Nov 22, 2019

hrbeuyz24 deleted the feat-pause_scheduler branch November 23, 2019 05:38

kennytm mentioned this pull request Oct 6, 2020

Pause schedulers instead of Remove schedulers pingcap/br#549

Closed

lhy1024 mentioned this pull request Aug 9, 2023

tests: miss test about scheduler test #6928

Closed

Conversation

hrbeuyz24 commented Nov 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which problem does this PR solve?

What have you changed?

What are the type of the changes?

How has this PR been tested?

Uh oh!

sre-bot commented Nov 15, 2019

Uh oh!

CLAassistant commented Nov 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lhy1024 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shafreeck left a comment

Choose a reason for hiding this comment

Uh oh!

hrbeuyz24 commented Nov 15, 2019

Uh oh!

hrbeuyz24 commented Nov 16, 2019

Uh oh!

hrbeuyz24 commented Nov 16, 2019

Uh oh!

rleungx commented Nov 18, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hrbeuyz24 commented Nov 20, 2019

Uh oh!

hrbeuyz24 commented Nov 20, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-io commented Nov 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shafreeck left a comment

Choose a reason for hiding this comment

Uh oh!

nolouch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sre-bot commented Nov 22, 2019

Uh oh!

sre-bot commented Nov 22, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

hrbeuyz24 commented Nov 15, 2019 •

edited

Loading

CLAassistant commented Nov 15, 2019 •

edited

Loading

codecov-io commented Nov 20, 2019 •

edited

Loading