controller: fix error retry and add more metrics #8219

nolouch · 2024-05-27T13:35:51Z

What problem does this PR solve?

Issue Number: Close #8217

What is changed and how does it work?

controller: fix error retry and add more metrics

Check List

Tests

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

Release note

None.

ti-chi-bot · 2024-05-27T13:35:55Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

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

codecov · 2024-05-27T13:43:49Z

Codecov Report

Attention: Patch coverage is 78.57143% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 77.41%. Comparing base (b1cbc71) to head (ee0a429).

❗ Current head ee0a429 differs from pull request most recent head 53085ef

Please upload reports for the commit 53085ef to get more accurate results.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8219      +/-   ##
==========================================
+ Coverage   77.31%   77.41%   +0.10%     
==========================================
  Files         471      471              
  Lines       61317    61339      +22     
==========================================
+ Hits        47406    47486      +80     
+ Misses      10349    10290      -59     
- Partials     3562     3563       +1

Flag	Coverage Δ
unittests	`77.41% <78.57%> (+0.10%)`	⬆️

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

nolouch · 2024-05-28T03:51:52Z

ptal @glorv

Signed-off-by: nolouch <nolouch@gmail.com>

HuSharp · 2024-05-28T07:32:43Z

client/resource_group/controller/controller.go

+ tokenRequestCounter prometheus.Counter
+}
+
+func initMetrics(oldName, name string) *groupMetricsCollection {


When will we remove oldName?

HuSharp · 2024-05-28T07:35:01Z

client/resource_group/controller/controller.go

@@ -789,7 +806,7 @@ func (gc *groupCostController) initRunState() {
 case rmpb.GroupMode_RUMode:
 gc.run.requestUnitTokens = make(map[rmpb.RequestUnitType]*tokenCounter)
 for typ := range requestUnitLimitTypeList {
- limiter := NewLimiterWithCfg(now, cfgFunc(getRUTokenBucketSetting(gc.meta, typ)), gc.lowRUNotifyChan)
+ limiter := NewLimiterWithCfg(now, cfgFunc(getRUTokenBucketSetting(gc.meta, typ)), gc.lowRUNotifyChan).SetName(gc.name).SetupMetrics()


Can we put setName logic in NewLimiterWithCfg?

lhy1024

Does it need to be picked to 7.5? 7.5.2 will frozen code tomorrow

Signed-off-by: nolouch <nolouch@gmail.com>

client/resource_group/controller/limiter.go

Signed-off-by: nolouch <nolouch@gmail.com>

nolouch · 2024-05-28T09:38:16Z

/merge

ti-chi-bot · 2024-05-28T09:38:19Z

@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 · 2024-05-28T09:38:20Z

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

Commit hash: ee0a429

ti-chi-bot · 2024-05-28T09:52:58Z

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

close tikv#8217 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

close #8217 controller: fix error retry and add more metrics Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io> Signed-off-by: nolouch <nolouch@gmail.com> Co-authored-by: ShuNing <nolouch@gmail.com> Co-authored-by: nolouch <nolouch@gmail.com>

nolouch · 2024-05-30T09:33:43Z

/run-cherry-picker

close tikv#8217 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2024-05-30T09:34:03Z

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

close tikv#8217 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2024-05-30T09:34:04Z

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

close #8217 controller: fix error retry and add more metrics Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io> Signed-off-by: nolouch <nolouch@gmail.com> Co-authored-by: ShuNing <nolouch@gmail.com> Co-authored-by: nolouch <nolouch@gmail.com>

ti-chi-bot bot added do-not-merge/needs-triage-completed release-note-none Denotes a PR that doesn't merit a release note. labels May 27, 2024

ti-chi-bot bot requested review from HuSharp and JmPotato May 27, 2024 13:36

ti-chi-bot bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 27, 2024

ti-chi-bot bot removed the do-not-merge/needs-triage-completed label May 27, 2024

nolouch added the needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. label May 28, 2024

rleungx approved these changes May 28, 2024

View reviewed changes

ti-chi-bot bot added the status/LGT1 Indicates that a PR has LGTM 1. label May 28, 2024

controller: fix error retry and add more metrics

69a871b

Signed-off-by: nolouch <nolouch@gmail.com>

nolouch force-pushed the add-more-error branch from 4fe26f4 to 69a871b Compare May 28, 2024 06:51

HuSharp reviewed May 28, 2024

View reviewed changes

lhy1024 reviewed May 28, 2024

View reviewed changes

address

ea0051f

Signed-off-by: nolouch <nolouch@gmail.com>

HuSharp approved these changes May 28, 2024

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 May 28, 2024

HuSharp reviewed May 28, 2024

View reviewed changes

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

address

ee0a429

Signed-off-by: nolouch <nolouch@gmail.com>

ti-chi-bot bot added the status/can-merge Indicates a PR has been approved by a committer. label May 28, 2024

Merge branch 'master' into add-more-error

53085ef

ti-chi-bot bot merged commit b7d8b94 into tikv:master May 28, 2024
15 checks passed

ti-chi-bot pushed a commit to ti-chi-bot/pd that referenced this pull request May 28, 2024

This is an automated cherry-pick of tikv#8219

bf2ceb2

close tikv#8217 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request May 28, 2024

controller: fix error retry and add more metrics (#8219) #8222

Merged

nolouch deleted the add-more-error branch May 28, 2024 11:18

ti-chi-bot removed the needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. label May 29, 2024

nolouch added needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. needs-cherry-pick-release-8.1 Should cherry pick this PR to release-8.1 branch. labels May 30, 2024

ti-chi-bot pushed a commit to ti-chi-bot/pd that referenced this pull request May 30, 2024

This is an automated cherry-pick of tikv#8219

dd2fdb9

close tikv#8217 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot pushed a commit to ti-chi-bot/pd that referenced this pull request May 30, 2024

This is an automated cherry-pick of tikv#8219

27a9d98

close tikv#8217 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

This was referenced May 30, 2024

controller: fix error retry and add more metrics (#8219) #8232

Merged

controller: fix error retry and add more metrics (#8219) #8233

Merged

JmPotato mentioned this pull request Jun 3, 2024

Update the PD client to adopt the latest fix/optimization tikv/client-go#1360

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

controller: fix error retry and add more metrics #8219

controller: fix error retry and add more metrics #8219

nolouch commented May 27, 2024

ti-chi-bot bot commented May 27, 2024 •

edited

Loading

codecov bot commented May 27, 2024 •

edited

Loading

nolouch commented May 28, 2024

HuSharp May 28, 2024

nolouch May 28, 2024

HuSharp May 28, 2024

lhy1024 left a comment

nolouch commented May 28, 2024

ti-chi-bot bot commented May 28, 2024

ti-chi-bot bot commented May 28, 2024

ti-chi-bot commented May 28, 2024

nolouch commented May 30, 2024

ti-chi-bot commented May 30, 2024

ti-chi-bot commented May 30, 2024

controller: fix error retry and add more metrics #8219

controller: fix error retry and add more metrics #8219

Conversation

nolouch commented May 27, 2024

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

ti-chi-bot bot commented May 27, 2024 • edited Loading

codecov bot commented May 27, 2024 • edited Loading

Codecov Report

nolouch commented May 28, 2024

HuSharp May 28, 2024

Choose a reason for hiding this comment

nolouch May 28, 2024

Choose a reason for hiding this comment

HuSharp May 28, 2024

Choose a reason for hiding this comment

lhy1024 left a comment

Choose a reason for hiding this comment

nolouch commented May 28, 2024

ti-chi-bot bot commented May 28, 2024

ti-chi-bot bot commented May 28, 2024

ti-chi-bot commented May 28, 2024

nolouch commented May 30, 2024

ti-chi-bot commented May 30, 2024

ti-chi-bot commented May 30, 2024

ti-chi-bot bot commented May 27, 2024 •

edited

Loading

codecov bot commented May 27, 2024 •

edited

Loading