resource_control: supports dynamically change the controller config #7042

nolouch · 2023-09-04T15:22:20Z

What problem does this PR solve?

Issue Number: Close #7043

What is changed and how does it work?

resource_control: supports dynamically change the controller config 
- supports dynamically changing the controller config
- export the `maxWaitDuration` for the local bucket limiter.

Check List

Tests

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

Code changes

Has the configuration change
Has HTTP API interfaces changed (Don't forget to add the declarative for the new API)
Has persistent data change

Side effects

Possible performance regression
Increased code complexity
Breaking backward compatibility

Related changes

PR to update pingcap/docs/pingcap/docs-cn:
PR to update pingcap/tiup:
Need to cherry-pick to the release branch

Release note

None.

ti-chi-bot · 2023-09-04T15:22:22Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

CabinfeverB
HuSharp

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.

ti-chi-bot · 2023-09-04T15:22:22Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

codecov · 2023-09-05T02:12:34Z

Codecov Report

Merging #7042 (d154fbf) into master (0888ef6) will decrease coverage by 0.18%.
The diff coverage is 63.51%.

❗ Current head d154fbf differs from pull request most recent head 9c414f0. Consider uploading reports for the commit 9c414f0 to get more accurate results

@@            Coverage Diff             @@
##           master    #7042      +/-   ##
==========================================
- Coverage   74.33%   74.15%   -0.18%     
==========================================
  Files         437      438       +1     
  Lines       46855    46895      +40     
==========================================
- Hits        34831    34777      -54     
- Misses       8947     9029      +82     
- Partials     3077     3089      +12

Flag	Coverage Δ
unittests	`74.15% <63.51%> (-0.18%)`	⬇️

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

client/resource_group/controller/util_test.go

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

HuSharp

LGTM!

CabinfeverB

REST LGTM

CabinfeverB · 2023-09-13T06:36:53Z

server/server.go

-		cb(ctx)
+		if err := cb(ctx); err != nil {
+			log.Error("failed to execute leader callback function", errs.ZapError(err))
+			return


How about continuing?

I think here some leaderCallbacks should make sure to run successfully. like resourcemanager load resource meta, or tso service init. ptal @rleungx

and return will re-election, I think it is like always do retry.

I think retry is ok to me and we should limit the retry count or time.

client/resource_group/controller/controller.go

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

nolouch · 2023-09-18T21:09:33Z

/merge

ti-chi-bot · 2023-09-18T21:09:34Z

@nolouch: 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-09-18T21:09:36Z

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

Commit hash: d154fbf

nolouch · 2023-12-05T03:32:44Z

/cherry-picker

nolouch · 2023-12-05T03:46:19Z

/run-cherry-picker

nolouch · 2023-12-05T04:27:12Z

/run-cherry-picker

ti-chi-bot · 2023-12-05T04:27:36Z

In response to a cherrypick label: new pull request created to branch release-7.1: #7495.

close tikv#7043 Signed-off-by: ti-chi-bot <[email protected]>

…ikv#7042) close tikv#7043 resource_control: supports dynamically change the controller config - supports dynamically changing the controller config - export the `maxWaitDuration` for the local bucket limiter. Signed-off-by: nolouch <[email protected]> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com> fix Signed-off-by: nolouch <[email protected]>

…7495) close #7043 resource_control: supports dynamically change the controller config - supports dynamically changing the controller config - export the `maxWaitDuration` for the local bucket limiter. Signed-off-by: nolouch <[email protected]> Signed-off-by: husharp <[email protected]> Co-authored-by: nolouch <[email protected]> Co-authored-by: Hu# <[email protected]> Co-authored-by: ShuNing <[email protected]>

ti-chi-bot bot added do-not-merge/needs-linked-issue release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Sep 4, 2023

ti-chi-bot bot requested review from HunDunDM and rleungx September 4, 2023 15:22

ti-chi-bot bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 4, 2023

nolouch force-pushed the export-config branch from 2a8a32d to d449b8b Compare September 4, 2023 15:25

nolouch removed request for HunDunDM and rleungx September 4, 2023 17:00

nolouch force-pushed the export-config branch from d449b8b to 62468e4 Compare September 4, 2023 17:23

ti-chi-bot bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 4, 2023

nolouch changed the title ~~resource_control: supports dynamictlly change the controller config~~ resource_control: supports dynamically change the controller config Sep 4, 2023

nolouch force-pushed the export-config branch from 62468e4 to bea0900 Compare September 4, 2023 17:24

ti-chi-bot bot removed the do-not-merge/needs-linked-issue label Sep 4, 2023

CabinfeverB reviewed Sep 5, 2023

View reviewed changes

client/resource_group/controller/util_test.go Outdated Show resolved Hide resolved

nolouch marked this pull request as ready for review September 5, 2023 16:35

ti-chi-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 5, 2023

nolouch force-pushed the export-config branch 3 times, most recently from a42cde3 to c35d680 Compare September 5, 2023 16:45

nolouch requested a review from HuSharp September 6, 2023 05:27

nolouch force-pushed the export-config branch 4 times, most recently from 3e3d08d to 8eda4ec Compare September 6, 2023 06:21

resource_control: supports dynamically change the controller config

4a631f6

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

nolouch force-pushed the export-config branch from 4fafa55 to a40b3c9 Compare September 11, 2023 09:29

HuSharp approved these changes Sep 13, 2023

View reviewed changes

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

CabinfeverB reviewed Sep 13, 2023

View reviewed changes

address

d154fbf

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

CabinfeverB approved these changes Sep 18, 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 Sep 18, 2023

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

Merge branch 'master' into export-config

9c414f0

ti-chi-bot bot merged commit 54eb4e4 into tikv:master Sep 18, 2023
19 checks passed

This was referenced Nov 13, 2023

Panic during initializing ResourceGroupController #7360

Closed

The latest version of resource control client may cause TiDB test unstable #7365

Closed

nolouch deleted the export-config branch December 5, 2023 03:32

nolouch added the affects-7.1 label Dec 5, 2023

nolouch added needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. and removed affects-7.1 labels Dec 5, 2023

ti-chi-bot mentioned this pull request Dec 5, 2023

resource_control: dynamically change the controller config (#7042) #7495

Merged

ti-chi-bot pushed a commit to ti-chi-bot/pd that referenced this pull request Dec 5, 2023

This is an automated cherry-pick of tikv#7042

5861e97

close tikv#7043 Signed-off-by: ti-chi-bot <[email protected]>

nolouch mentioned this pull request Mar 25, 2024

pd-ctl: support ltb-max-wait-duration #7948

Closed

CabinfeverB mentioned this pull request Apr 7, 2024

resource_control: remove unnecessary comment #8026

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

resource_control: supports dynamically change the controller config #7042

resource_control: supports dynamically change the controller config #7042

nolouch commented Sep 4, 2023 •

edited

Loading

ti-chi-bot bot commented Sep 4, 2023 •

edited

Loading

ti-chi-bot bot commented Sep 4, 2023

codecov bot commented Sep 5, 2023 •

edited

Loading

HuSharp left a comment

CabinfeverB left a comment

CabinfeverB Sep 13, 2023

nolouch Sep 14, 2023 •

edited

Loading

rleungx Sep 14, 2023

nolouch commented Sep 18, 2023

ti-chi-bot bot commented Sep 18, 2023

ti-chi-bot bot commented Sep 18, 2023

nolouch commented Dec 5, 2023

nolouch commented Dec 5, 2023

nolouch commented Dec 5, 2023

ti-chi-bot commented Dec 5, 2023

resource_control: supports dynamically change the controller config #7042

resource_control: supports dynamically change the controller config #7042

Conversation

nolouch commented Sep 4, 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 Sep 4, 2023 • edited Loading

ti-chi-bot bot commented Sep 4, 2023

codecov bot commented Sep 5, 2023 • edited Loading

Codecov Report

HuSharp left a comment

Choose a reason for hiding this comment

CabinfeverB left a comment

Choose a reason for hiding this comment

CabinfeverB Sep 13, 2023

Choose a reason for hiding this comment

nolouch Sep 14, 2023 • edited Loading

Choose a reason for hiding this comment

rleungx Sep 14, 2023

Choose a reason for hiding this comment

nolouch commented Sep 18, 2023

ti-chi-bot bot commented Sep 18, 2023

ti-chi-bot bot commented Sep 18, 2023

nolouch commented Dec 5, 2023

nolouch commented Dec 5, 2023

nolouch commented Dec 5, 2023

ti-chi-bot commented Dec 5, 2023

nolouch commented Sep 4, 2023 •

edited

Loading

ti-chi-bot bot commented Sep 4, 2023 •

edited

Loading

codecov bot commented Sep 5, 2023 •

edited

Loading

nolouch Sep 14, 2023 •

edited

Loading