LAC: smoothing tokens request and keep in high level (#10997)

[ti-chi-bot]

This is an automated cherry-pick of #10997

What problem does this PR solve?

Issue Number: close #10996

Summary

This change improves TiFlash Local Admission Controller token refill behavior to keep the local token bucket near a high watermark without requesting a large amount of tokens in a single GAC request.

Problem

The previous acquire calculation was based only on predicted consumption:

acquire_tokens = max(smoothed_speed * 5s * 1.1 - remaining_tokens, 0)

When the smoothed consumption speed was underestimated, a small positive token balance could make acquire_tokens zero. The local balance would then remain low and could be exhausted by a traffic burst, causing unexpected throttling.

Always refilling directly to the full bucket capacity would avoid this problem, but could transfer and retain too many tokens in TiFlash at once, reducing the tokens available to other clients such as TiDB.

Changes

• Added a proactive refill watermark at 80% of the local high watermark.
• Added a one-second refill check interval in normal mode.
• Included proactive refill checks in addition to the existing low-token and consumption-report triggers.
• Added incremental token acquisition for normal refills:

deficit = high_watermark - remaining_tokens

fallback_batch = min(
    5000,
    high_watermark * 20%
)

incremental_batch = max(
    smoothed_consumption_speed * 1s * 1.1,
    fallback_batch
)

acquire_tokens = min(deficit, incremental_batch)

• Preserved emergency refill behavior when the bucket reaches the existing low-token threshold. In that case, the incremental limit is bypassed to avoid request throttling.
• Before the first GAC token response, the Resource Group fill_rate is used as the local high watermark.
• After the first GAC response, the capacity assigned by GAC to the current client is used as the high watermark.
• Added has_gac_capacity state to distinguish the global Resource Group burst limit from the capacity assigned to the local client.
• Added a read-only TokenBucket::getCapacity() accessor.
• Kept the low-token threshold based on the actual post-grant token balance. This prevents a capacity increase from immediately classifying the bucket as low-token and triggering a large emergency refill.
• Preserved the existing five-second consumption reporting period and GAC target request period.
• Did not change RU accounting, token deduction, GAC grant handling, or trickle-mode semantics.

Resulting Behavior

• TiFlash starts refilling before the local bucket reaches a critically low balance.
• Normal refill requests are spread across smaller requests instead of immediately filling the entire capacity.
• High-throughput workloads can still request approximately one second of predicted consumption per refill.
• Low-token conditions retain an emergency path that prioritizes avoiding unexpected query throttling.
• A newly started TiFlash instance does not use the global Resource Group burst limit as its initial local refill target.
• Unused tokens are less likely to be transferred from GAC to TiFlash in one large request, reducing the impact on other clients sharing the Resource Group.

##Test
During bench tpch workload, after acquire tokens from GAC, the remaining_tokens keeps close to the high watermark.

Check List

Tests

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

Side effects

• Performance regression: Consumes more CPU
• Performance regression: Consumes more Memory
• Breaking backward compatibility

Documentation

• Affects user behaviors
• Contains syntax changes
• Contains variable changes
• Contains experimental features
• Changes MySQL compatibility

Release note

None

Summary by CodeRabbit

• Improvements

  • Improved token replenishment timing and incremental refill behavior for more consistent resource availability.
  • Added safeguards based on predicted consumption, bucket capacity, and low-token conditions.
  • Improved tracking of available capacity during resource-control responses.
  • Enhanced startup and emergency refill handling for resource groups.

• Tests

  • Added coverage for startup, incremental, predicted-consumption, and low-token refill scenarios.

[ti-chi-bot]

This cherry pick PR is for a release branch and has not yet been approved by triage owners.
Adding the do-not-merge/cherry-pick-not-approved label.

To merge this cherry pick:

1. It must be LGTMed and approved by the reviewers firstly.
2. For pull requests to TiDB-x branches, it must have no failed tests.
3. AFTER it has lgtm and approved labels, please wait for the cherry-pick merging approval from triage owners.

Details

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 kubernetes-sigs/prow repository.

[ti-chi-bot]

@JaySon-Huang This PR has conflicts, I have hold it.
Please resolve them or ask others to resolve them, then comment /unhold to remove the hold label.

[ti-chi-bot]

@ti-chi-bot: ## If you want to know how to resolve it, please read the guide in TiDB Dev Guide.

Details

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.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: eab67ba1-3868-478c-bb8c-3e9b9200af4a

📥 Commits

Reviewing files that changed from the base of the PR and between 5c92b72 and cc6532c.

📒 Files selected for processing (1)

• dbms/src/Flash/ResourceControl/LocalAdmissionController.cpp

🚧 Files skipped from review as they are similar to previous changes (1)

• dbms/src/Flash/ResourceControl/LocalAdmissionController.cpp

---

📝 Walkthrough

Walkthrough

LocalAdmissionController now performs timed, capacity-aware token refills. ResourceGroup tracks GAC capacity and refill thresholds. Tests cover startup, incremental, predicted-consumption, and low-token refills.

Changes

Resource-group token refill

Layer / File(s)	Summary
Refill state and contracts dbms/src/Flash/ResourceControl/LocalAdmissionController.h, dbms/src/Flash/ResourceControl/TokenBucket.h	Adds refill constants, helper declarations, GAC capacity state, and a TokenBucket::getCapacity() accessor.
Capacity-aware refill execution dbms/src/Flash/ResourceControl/LocalAdmissionController.cpp	Uses refill eligibility checks, high-watermark limits, predicted-consumption batches, low-token handling, and GAC capacity tracking. Request selection includes eligible scheduled refills.
Resource-group refill validation dbms/src/Flash/ResourceControl/tests/gtest_local_admission_controller.cpp	Adds tests for startup, capacity-adjusted, predicted-consumption, and low-token refills.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant LocalAdmissionController
  participant ResourceGroup
  participant TokenBucket
  participant GAC
  LocalAdmissionController->>ResourceGroup: check refill eligibility
  ResourceGroup->>TokenBucket: read capacity and token state
  LocalAdmissionController->>GAC: request refill tokens
  GAC-->>LocalAdmissionController: return capacity
  LocalAdmissionController->>ResourceGroup: record GAC capacity

Loading

Possibly related PRs

• pingcap/tiflash#10997: Implements related Local Admission Controller token-refill logic and tests.

Poem

A rabbit checks the refill clock,
Then fills each bucket past the lock.
GAC sends capacity through,
Low tokens get a full refill too.
Tests hop along the path.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the token request smoothing and high-watermark behavior changed by the PR.
Description check	✅ Passed	The description covers the problem, implementation, testing, checklist, side effects, documentation, and release-note sections.
Linked Issues check	✅ Passed	The PR addresses [#10996] by preventing persistently low remaining tokens through proactive and incremental refills.
Out of Scope Changes check	✅ Passed	The code changes and tests support the linked issue and stated token refill objectives without unrelated scope.

✨ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

dbms/src/Flash/ResourceControl/LocalAdmissionController.h (1)

185-189: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use DB numeric aliases in the new public interfaces.

Use Float64 and UInt32 in these declarations and matching definitions.

• dbms/src/Flash/ResourceControl/LocalAdmissionController.h#L185-L189: replace new double and uint32_t API types with Float64 and UInt32.
• dbms/src/Flash/ResourceControl/TokenBucket.h#L97-L97: return Float64 from getCapacity().
• dbms/src/Flash/ResourceControl/LocalAdmissionController.cpp#L153-L171: match the updated Float64 and UInt32 declaration types.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@dbms/src/Flash/ResourceControl/LocalAdmissionController.h` around lines 185 -
189, Replace the new public numeric types in
LocalAdmissionController.h#L185-L189 with Float64 and UInt32, update the
matching definitions in LocalAdmissionController.cpp#L153-L171, and change
TokenBucket.h#L97 getCapacity() to return Float64. Ensure all declarations and
definitions remain type-consistent.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@dbms/src/Flash/ResourceControl/LocalAdmissionController.cpp`:
- Around line 460-468: Resolve the conflict in the affected loop by removing all
merge markers and retaining the local-branch `iter` and
`local_low_token_resource_groups` symbols. Update `need_fetch_token` to also
include `iter.second->shouldRefillToken(current_tick)`, while keeping
`need_report` based on `iter.second->shouldReportRUConsumption(current_tick)`.

In `@dbms/src/Flash/ResourceControl/tests/gtest_local_admission_controller.cpp`:
- Around line 98-100: Update the affected tests to match the available
resource-group API: remove the keyspace_id assertion from the GACRequestInfo
checks, and construct ResourceGroup using the existing
resource_manager::ResourceGroup plus SteadyClock::time_point constructor instead
of the unsupported NullspaceID overload.

---

Nitpick comments:
In `@dbms/src/Flash/ResourceControl/LocalAdmissionController.h`:
- Around line 185-189: Replace the new public numeric types in
LocalAdmissionController.h#L185-L189 with Float64 and UInt32, update the
matching definitions in LocalAdmissionController.cpp#L153-L171, and change
TokenBucket.h#L97 getCapacity() to return Float64. Ensure all declarations and
definitions remain type-consistent.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 93af541b-c6a0-4c15-a4aa-4925176796aa

📥 Commits

Reviewing files that changed from the base of the PR and between 76536b2 and 3d30251.

📒 Files selected for processing (4)

• dbms/src/Flash/ResourceControl/LocalAdmissionController.cpp
• dbms/src/Flash/ResourceControl/LocalAdmissionController.h
• dbms/src/Flash/ResourceControl/TokenBucket.h
• dbms/src/Flash/ResourceControl/tests/gtest_local_admission_controller.cpp

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from jayson-huang. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

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

Details

Needs approval from an approver in each of these files:

• OWNERS

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]

Cherry-pick conflicts appear resolved; removing the do-not-merge/hold label.

[JaySon-Huang]

/test pull-integration-test