client: return total wait duration in resource interceptor OnRequestWait call #7488

glorv · 2023-11-30T11:04:39Z

What problem does this PR solve?

Issue Number: ref #5851

What is changed and how does it work?

return total wait duration in resource interceptor OnRequestWait call so we can track the wait duration in tidb slow log.

Check List

Tests

Unit test

Code changes

Side effects

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.

Signed-off-by: glorv <glorvs@163.com>

ti-chi-bot · 2023-11-30T11:04:43Z

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

glorv · 2023-11-30T11:05:05Z

@nolouch @CabinfeverB @HuSharp PTAL

codecov · 2023-11-30T11:14:41Z

Codecov Report

Merging #7488 (b19ebbf) into master (3191594) will decrease coverage by 0.08%.
The diff coverage is 87.50%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7488      +/-   ##
==========================================
- Coverage   74.32%   74.24%   -0.08%     
==========================================
  Files         452      452              
  Lines       49915    49918       +3     
==========================================
- Hits        37098    37062      -36     
- Misses       9490     9512      +22     
- Partials     3327     3344      +17

Flag	Coverage Δ
unittests	`74.24% <87.50%> (-0.08%)`	⬇️

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

CabinfeverB

rest LGTM

CabinfeverB · 2023-11-30T13:52:41Z

client/resource_group/controller/controller.go

@@ -57,7 +57,7 @@ const (
 // ResourceGroupKVInterceptor is used as quota limit controller for resource group using kv store.
 type ResourceGroupKVInterceptor interface {
 	// OnRequestWait is used to check whether resource group has enough tokens. It maybe needs to wait some time.
-	OnRequestWait(ctx context.Context, resourceGroupName string, info RequestInfo) (*rmpb.Consumption, *rmpb.Consumption, uint32, error)
+	OnRequestWait(ctx context.Context, resourceGroupName string, info RequestInfo) (*rmpb.Consumption, *rmpb.Consumption, time.Duration, uint32, error)


Do we need to wrap into a struct?

Not sure if there is a best practice. In general, multiple return parameters are organized as a tuple, so should be no performance issue.

CabinfeverB · 2023-12-04T03:07:08Z

@glorv merge it right now?

CabinfeverB · 2023-12-04T03:45:19Z

/merge

ti-chi-bot · 2023-12-04T03:45:21Z

@CabinfeverB: 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-12-04T03:45:23Z

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

Commit hash: b19ebbf

glorv · 2024-02-02T07:21:11Z

/label cherry-pick-7.5

ti-chi-bot · 2024-02-02T07:21:14Z

@glorv: The label(s) cherry-pick-7.5 cannot be applied. These labels are supported: Hacktoberfest, challenge-program, ci-unstable, compatibility-breaker, high-performance, hptc, needs-cherry-pick-release-5.4, needs-cherry-pick-release-6.1, needs-cherry-pick-release-6.5, needs-cherry-pick-release-7.1, needs-cherry-pick-release-7.5, needs-cherry-pick-release-7.6, release-note, require-LGT1, wontfix, affects-5.4, affects-6.1, affects-6.5, affects-7.1, affects-7.5, affects-7.6, may-affects-5.4, may-affects-6.1, may-affects-6.5, may-affects-7.1, may-affects-7.5, may-affects-7.6.

In response to this:

/label cherry-pick-7.5

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.

glorv · 2024-02-02T07:21:42Z

/cherry-picker release-7.5

glorv · 2024-02-02T07:22:09Z

/label needs-cherry-pick-release-7.5

ti-chi-bot · 2024-02-02T07:22:47Z

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

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

ti-chi-bot · 2024-02-02T09:29:18Z

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

…ait call (#7488) (#7794) ref #5851 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io> Signed-off-by: glorv <glorvs@163.com> Co-authored-by: glorv <glorvs@163.com>

also return total wait duration in OnRequestWait

b19ebbf

Signed-off-by: glorv <glorvs@163.com>

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

ti-chi-bot bot requested review from lhy1024 and nolouch November 30, 2023 11:04

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

glorv mentioned this pull request Nov 30, 2023

resource_control: add ru details in ExecDetails tikv/client-go#1070

Merged

CabinfeverB reviewed Nov 30, 2023

View reviewed changes

CabinfeverB approved these changes Dec 1, 2023

View reviewed changes

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

HuSharp approved these changes Dec 1, 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 Dec 1, 2023

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

ti-chi-bot bot merged commit 259435d into tikv:master Dec 4, 2023
27 checks passed

glorv deleted the rg-wiat-dur branch February 2, 2024 07:14

ti-chi-bot bot added the needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. label Feb 2, 2024

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

This is an automated cherry-pick of tikv#7488

2d59247

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

ti-chi-bot mentioned this pull request Feb 2, 2024

client: return total wait duration in resource interceptor OnRequestWait call (#7488) #7794

Merged

nolouch added the needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. label Feb 2, 2024

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

This is an automated cherry-pick of tikv#7488

7dd9b8e

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

ti-chi-bot mentioned this pull request Feb 2, 2024

client: return total wait duration in resource interceptor OnRequestWait call (#7488) #7796

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

client: return total wait duration in resource interceptor OnRequestWait call #7488

client: return total wait duration in resource interceptor OnRequestWait call #7488

glorv commented Nov 30, 2023

ti-chi-bot bot commented Nov 30, 2023 •

edited

Loading

glorv commented Nov 30, 2023

codecov bot commented Nov 30, 2023

CabinfeverB left a comment

CabinfeverB Nov 30, 2023

glorv Dec 1, 2023

CabinfeverB commented Dec 4, 2023

CabinfeverB commented Dec 4, 2023

ti-chi-bot bot commented Dec 4, 2023

ti-chi-bot bot commented Dec 4, 2023

glorv commented Feb 2, 2024

ti-chi-bot bot commented Feb 2, 2024

glorv commented Feb 2, 2024

glorv commented Feb 2, 2024

ti-chi-bot commented Feb 2, 2024

ti-chi-bot commented Feb 2, 2024

client: return total wait duration in resource interceptor OnRequestWait call #7488

client: return total wait duration in resource interceptor OnRequestWait call #7488

Conversation

glorv commented Nov 30, 2023

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

ti-chi-bot bot commented Nov 30, 2023 • edited Loading

glorv commented Nov 30, 2023

codecov bot commented Nov 30, 2023

Codecov Report

CabinfeverB left a comment

Choose a reason for hiding this comment

CabinfeverB Nov 30, 2023

Choose a reason for hiding this comment

glorv Dec 1, 2023

Choose a reason for hiding this comment

CabinfeverB commented Dec 4, 2023

CabinfeverB commented Dec 4, 2023

ti-chi-bot bot commented Dec 4, 2023

ti-chi-bot bot commented Dec 4, 2023

glorv commented Feb 2, 2024

ti-chi-bot bot commented Feb 2, 2024

glorv commented Feb 2, 2024

glorv commented Feb 2, 2024

ti-chi-bot commented Feb 2, 2024

ti-chi-bot commented Feb 2, 2024

ti-chi-bot bot commented Nov 30, 2023 •

edited

Loading