[release-nextgen-202603] client: remove successful token wait logs by lhy1024 · Pull Request #10997 · tikv/pd

lhy1024 · 2026-07-13T08:03:20Z

What problem does this PR solve?

Issue Number: ref #10488

#10866 logs every successful client-side Resource Control token wait longer
than 10ms at WARN level. Successful waits are expected while requests are
throttled, so high-QPS workloads can produce excessive TiDB warning logs even
when all waits complete successfully.

What is changed and how does it work?

client/resource_group: remove successful token wait logs

Remove per-request WARN logs after successful request-side and response-side token waits.
Keep WARN logs for failed token waits.
Preserve the existing successful-request histogram and its metric semantics.

Check List

Tests

Unit test
- cd client && make gotest GOTEST_ARGS="./resource_group/controller/... -count=1"
- cd client && make static

Related changes

Follow-up fix for [release-nextgen-202603] client/resource_group: add client-side demand observability #10866.
Apply the same fix to client/resource_group: add client-side demand observability #10604 after this PR merges.

Release note

Reduce excessive TiDB warning logs for successful Resource Control token waits
while preserving wait-duration metrics.

Successful token waits are expected while resource control throttles requests. Logging every wait over 10ms at WARN can flood TiDB logs under high QPS. Keep the existing histogram for observability and record the accumulated retry and reservation wait duration. Signed-off-by: lhy1024 <19542290+lhy1024@users.noreply.github.com>

coderabbitai · 2026-07-13T08:03:29Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3c2a8284-9a61-4573-865d-eb9de28c7f0b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

lhy1024 · 2026-07-13T08:03:39Z

Dependency note: this PR fixes release-nextgen-202603 first because the backport #10866 is already merged there. After this PR lands, we will apply the same change to the still-open master PR #10604 so both branches keep consistent successful-wait logging and metric semantics.

codecov · 2026-07-13T08:15:41Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.02%. Comparing base (b7828f1) to head (0f397e3).
⚠️ Report is 2 commits behind head on release-nextgen-202603.

Additional details and impacted files

@@                    Coverage Diff                     @@
##           release-nextgen-202603   #10997      +/-   ##
==========================================================
- Coverage                   79.07%   79.02%   -0.05%     
==========================================================
  Files                         531      531              
  Lines                       72298    72294       -4     
==========================================================
- Hits                        57168    57130      -38     
- Misses                      11093    11124      +31     
- Partials                     4037     4040       +3

Flag	Coverage Δ
unittests	`79.02% <ø> (-0.05%)`	⬇️

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: lhy1024 <19542290+lhy1024@users.noreply.github.com>

JmPotato · 2026-07-13T08:56:01Z

-		if waitDuration > slowNotifyFilterDuration {
-			log.Warn("[resource group controller] waited for tokens", gc.logFields(waitDuration, nil)...)
-		}
+		gc.metrics.successfulRequestDuration.Observe(waitDuration.Seconds())


Is this metric semantic change intentional? Previously, SuccessfulRequestDuration observed only d, which is the wait duration of the final successful reservation. After this change it observes waitDuration, which also includes the retry/backoff or reconfiguration wait accumulated before the final reservation. For requests that retry, this changes the existing histogram bucket/sum/quantile values, rather than only removing the successful-wait log. Could you confirm that recording the total accumulated wait in the existing metric is the expected behavior?

Although I believe this change will make the monitoring more intuitive and easier to use, I’d still like to double-check, just in case.

Good catch. Although recording the total accumulated wait was initially intentional, it is out of scope for this log-noise fix and would change existing histogram and dashboard values. I restored SuccessfulRequestDuration to observe only d, preserving its existing semantics. The final PR now only removes the two successful-wait WARN logs. Updated in 0f397e3.

Signed-off-by: lhy1024 <19542290+lhy1024@users.noreply.github.com>

ti-chi-bot · 2026-07-14T03:53:47Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bufferflies, JmPotato

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [JmPotato,bufferflies]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2026-07-14T03:53:49Z

[LGTM Timeline notifier]

Timeline:

2026-07-13 12:26:30.057274999 +0000 UTC m=+630176.093370065: ☑️ agreed by JmPotato.
2026-07-14 03:53:48.33268394 +0000 UTC m=+685814.368779006: ☑️ agreed by bufferflies.

lhy1024 · 2026-07-14T04:00:07Z

/retest

lhy1024 · 2026-07-14T04:12:23Z

/retest

lhy1024 · 2026-07-14T04:24:00Z

/retest

lhy1024 · 2026-07-14T04:32:58Z

/retest

ref tikv/pd#10997

ti-chi-bot Bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates the PR's author has signed the dco. labels Jul 13, 2026

lhy1024 mentioned this pull request Jul 13, 2026

client/resource_group: add client-side demand observability #10604

Open

ti-chi-bot Bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 13, 2026

client/resource_group: simplify successful wait metrics

9618af7

Signed-off-by: lhy1024 <19542290+lhy1024@users.noreply.github.com>

ti-chi-bot Bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 13, 2026

JmPotato reviewed Jul 13, 2026

View reviewed changes

client/resource_group: preserve successful wait metric semantics

0f397e3

Signed-off-by: lhy1024 <19542290+lhy1024@users.noreply.github.com>

ti-chi-bot Bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 13, 2026

JmPotato approved these changes Jul 13, 2026

View reviewed changes

ti-chi-bot Bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Jul 13, 2026

bufferflies approved these changes Jul 14, 2026

View reviewed changes

ti-chi-bot Bot added the lgtm label Jul 14, 2026

ti-chi-bot Bot removed the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Jul 14, 2026

ti-chi-bot Bot merged commit 19ee324 into tikv:release-nextgen-202603 Jul 14, 2026
26 checks passed

lhy1024 deleted the fix-rc-success-wait-logs-nextgen-202603 branch July 14, 2026 05:43

lhy1024 mentioned this pull request Jul 14, 2026

go.mod: update PD client for pd#10997 pingcap/tidb#69837

Merged

13 tasks

ti-chi-bot Bot pushed a commit to pingcap/tidb that referenced this pull request Jul 14, 2026

go.mod: update PD client for pd#10997 (#69837)

8c0a0f0

ref tikv/pd#10997

Uh oh!

Conversation

lhy1024 commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

Uh oh!

coderabbitai Bot commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

lhy1024 commented Jul 13, 2026

Uh oh!

codecov Bot commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

JmPotato Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

JmPotato Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

lhy1024 Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot Bot commented Jul 14, 2026

Uh oh!

ti-chi-bot Bot commented Jul 14, 2026

[LGTM Timeline notifier]

Uh oh!

lhy1024 commented Jul 14, 2026

Uh oh!

lhy1024 commented Jul 14, 2026

Uh oh!

lhy1024 commented Jul 14, 2026

Uh oh!

lhy1024 commented Jul 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lhy1024 commented Jul 13, 2026 •

edited

Loading

coderabbitai Bot commented Jul 13, 2026 •

edited

Loading

codecov Bot commented Jul 13, 2026 •

edited

Loading