server: operator's timeout depends on steps #4548

bufferflies · 2022-01-05T10:41:05Z

What problem does this PR solve?

close #4547
depence #4456

What is changed and how it works?

The interface of OpStep adds one function: timeout(start time.Time)bool.
AddLearnerStep and AddPeerStep's timeout : max(10m, multi (the size of region, the store executor rate(6s/m)) )
ChangePeerV2Enter and ChangePeerV2Leave's timeout: multi(count(learner)+count(demote),FastOperatorWaitTime(10s))
the others is FastOperatorWaitTime(10s).
``
And the operator timeout depences on the steps.

Check List

Tests

Unit test

Code changes

Has configuration change

Side effects

Related changes

Release note

 None.

Signed-off-by: bufferflies <1045931706@qq.com>

…and operator-time-factor to scheduler config && close 4547 Signed-off-by: bufferflies <1045931706@qq.com>

ti-chi-bot · 2022-01-05T10:41:06Z

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

Signed-off-by: bufferflies <1045931706@qq.com>

codecov · 2022-01-05T12:37:58Z

Codecov Report

Merging #4548 (d57e796) into master (22cef40) will decrease coverage by 0.17%.
The diff coverage is 97.29%.

❗ Current head d57e796 differs from pull request most recent head 21cebb9. Consider uploading reports for the commit 21cebb9 to get more accurate results

@@            Coverage Diff             @@
##           master    #4548      +/-   ##
==========================================
- Coverage   75.01%   74.84%   -0.18%     
==========================================
  Files         282      284       +2     
  Lines       27791    27839      +48     
==========================================
- Hits        20847    20835      -12     
- Misses       5075     5136      +61     
+ Partials     1869     1868       -1

Flag	Coverage Δ
unittests	`74.84% <97.29%> (-0.18%)`	⬇️

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

Impacted Files	Coverage Δ
server/schedule/operator/step.go	`75.75% <96.00%> (+1.57%)`	⬆️
server/schedule/operator/operator.go	`94.81% <100.00%> (ø)`
server/schedule/operator/status_tracker.go	`94.00% <100.00%> (ø)`
server/tso/local_allocator.go	`64.86% <0.00%> (-13.52%)`	⬇️
server/api/trend.go	`73.23% <0.00%> (-11.27%)`	⬇️
pkg/requestutil/request_info.go	`81.81% <0.00%> (-9.10%)`	⬇️
server/schedule/hbstream/heartbeat_streams.go	`72.72% <0.00%> (-9.10%)`	⬇️
pkg/audit/audit.go	`83.33% <0.00%> (-8.34%)`	⬇️
server/api/middleware.go	`82.35% <0.00%> (-4.19%)`	⬇️
server/tso/tso.go	`64.20% <0.00%> (-2.28%)`	⬇️
... and 27 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 22cef40...21cebb9. Read the comment docs.

server/schedule/checker/merge_checker.go

server/schedule/operator/operator.go

server/config/config.go

Signed-off-by: bufferflies <1045931706@qq.com>

server/config/persist_options.go

Signed-off-by: bufferflies <1045931706@qq.com>

rleungx · 2022-01-25T02:59:30Z

Please add some tests

Signed-off-by: bufferflies <1045931706@qq.com>

server/schedule/operator/operator.go

server/schedule/operator/builder.go

Signed-off-by: bufferflies <1045931706@qq.com>

server/schedule/operator/operator.go

server/schedule/operator/step.go

server/schedule/operator/operator.go

server/schedule/operator/step.go

Signed-off-by: bufferflies <1045931706@qq.com>

rleungx

The rest LGTM.

server/schedule/operator/operator_test.go

JmPotato

The rest LGTM.

server/schedule/operator/operator.go

Signed-off-by: bufferflies <1045931706@qq.com>

nolouch · 2022-03-01T08:12:57Z

/merge

ti-chi-bot · 2022-03-01T08:12:58Z

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

/run-all-tests

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 · 2022-03-01T08:12:59Z

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

Commit hash: d57e796

bufferflies added 3 commits December 14, 2021 10:17

operator add region size properties and close tikv#4457

8e8b375

Signed-off-by: bufferflies <1045931706@qq.com>

lint check

98e6c9a

Signed-off-by: bufferflies <1045931706@qq.com>

adds timeoutfactor property to operator struct, adds max-region-size …

ff4e389

…and operator-time-factor to scheduler config && close 4547 Signed-off-by: bufferflies <1045931706@qq.com>

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jan 5, 2022

ti-chi-bot requested review from JmPotato and rleungx January 5, 2022 10:41

bufferflies added 2 commits January 5, 2022 19:40

merge master

de8f984

Signed-off-by: bufferflies <1045931706@qq.com>

pass ut

59a0dff

Signed-off-by: bufferflies <1045931706@qq.com>

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 5, 2022

JmPotato reviewed Jan 6, 2022

View reviewed changes

server/schedule/checker/merge_checker.go Outdated Show resolved Hide resolved

server/schedule/checker/merge_checker.go Outdated Show resolved Hide resolved

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

rleungx reviewed Jan 11, 2022

View reviewed changes

server/config/config.go Outdated Show resolved Hide resolved

server/config/config.go Outdated Show resolved Hide resolved

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 12, 2022

bufferflies added 2 commits January 13, 2022 17:36

replace max-region-size

b792225

Signed-off-by: bufferflies <1045931706@qq.com>

resolved confict

33933f9

Signed-off-by: bufferflies <1045931706@qq.com>

rleungx reviewed Jan 24, 2022

View reviewed changes

server/config/persist_options.go Outdated Show resolved Hide resolved

bufferflies added 4 commits January 24, 2022 22:34

check all in step

8a5a203

Signed-off-by: bufferflies <1045931706@qq.com>

remove unnecessary change

155a53d

Signed-off-by: bufferflies <1045931706@qq.com>

remove unnecessary change

0a9d9cc

Signed-off-by: bufferflies <1045931706@qq.com>

resolved conflict

b5413d6

Signed-off-by: bufferflies <1045931706@qq.com>

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 24, 2022

bufferflies added 2 commits January 24, 2022 22:55

resolved conflict

62340cf

Signed-off-by: bufferflies <1045931706@qq.com>

resolved conflict

1c7bdc8

Signed-off-by: bufferflies <1045931706@qq.com>

bufferflies changed the title ~~cluster: adds timeout-factor property to operator struct~~ server: operator's timeout depends on steps and adds max region size configuration Jan 25, 2022

rename TimeOut to timeout

1a796f9

Signed-off-by: bufferflies <1045931706@qq.com>

bufferflies force-pushed the feature/operator_factor branch 2 times, most recently from 9f3f1e1 to c9e5c3e Compare January 25, 2022 07:37

ti-chi-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Feb 17, 2022

resolv conflict

55ac82a

Signed-off-by: bufferflies <1045931706@qq.com>

bufferflies force-pushed the feature/operator_factor branch from c701fed to 55ac82a Compare February 17, 2022 03:51

bufferflies requested review from JmPotato and rleungx February 17, 2022 10:19

rleungx reviewed Feb 17, 2022

View reviewed changes

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

server/schedule/operator/builder.go Outdated Show resolved Hide resolved

change timeout signature

38cdf2e

Signed-off-by: bufferflies <1045931706@qq.com>

rleungx reviewed Feb 18, 2022

View reviewed changes

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

server/schedule/operator/step.go Outdated Show resolved Hide resolved

server/schedule/operator/step.go Outdated Show resolved Hide resolved

bufferflies force-pushed the feature/operator_factor branch 3 times, most recently from 130f9ec to 8bf3243 Compare February 21, 2022 03:11

rleungx reviewed Feb 21, 2022

View reviewed changes

server/schedule/operator/operator.go Show resolved Hide resolved

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

server/schedule/operator/step.go Outdated Show resolved Hide resolved

address comment

ceee2b7

Signed-off-by: bufferflies <1045931706@qq.com>

bufferflies force-pushed the feature/operator_factor branch from 8bf3243 to ceee2b7 Compare February 21, 2022 07:33

rleungx approved these changes Feb 21, 2022

View reviewed changes

server/schedule/operator/operator_test.go Outdated Show resolved Hide resolved

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Feb 21, 2022

JmPotato reviewed Feb 23, 2022

View reviewed changes

server/schedule/operator/operator.go Show resolved Hide resolved

server/schedule/operator/operator.go Show resolved Hide resolved

change comment

e74e2f7

Signed-off-by: bufferflies <1045931706@qq.com>

bufferflies force-pushed the feature/operator_factor branch from 27b8fdb to e74e2f7 Compare February 24, 2022 06:36

JmPotato approved these changes Feb 24, 2022

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Feb 24, 2022

Merge branch 'master' into feature/operator_factor

d57e796

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 1, 2022

Merge branch 'master' into feature/operator_factor

21cebb9

ti-chi-bot merged commit cc256b5 into tikv:master Mar 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

server: operator's timeout depends on steps #4548

server: operator's timeout depends on steps #4548

bufferflies commented Jan 5, 2022 •

edited

Loading

ti-chi-bot commented Jan 5, 2022 •

edited

Loading

codecov bot commented Jan 5, 2022 •

edited

Loading

rleungx commented Jan 25, 2022

rleungx left a comment

JmPotato left a comment

nolouch commented Mar 1, 2022

ti-chi-bot commented Mar 1, 2022

ti-chi-bot commented Mar 1, 2022

server: operator's timeout depends on steps #4548

server: operator's timeout depends on steps #4548

Conversation

bufferflies commented Jan 5, 2022 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Jan 5, 2022 • edited Loading

codecov bot commented Jan 5, 2022 • edited Loading

Codecov Report

rleungx commented Jan 25, 2022

rleungx left a comment

Choose a reason for hiding this comment

JmPotato left a comment

Choose a reason for hiding this comment

nolouch commented Mar 1, 2022

ti-chi-bot commented Mar 1, 2022

ti-chi-bot commented Mar 1, 2022

bufferflies commented Jan 5, 2022 •

edited

Loading

ti-chi-bot commented Jan 5, 2022 •

edited

Loading

codecov bot commented Jan 5, 2022 •

edited

Loading