Skip to content

util: accumulate raw RUv2 in RUDetails#1942

Merged
ti-chi-bot[bot] merged 3 commits intotikv:masterfrom
oh-my-tidb:rudetails-ruv2
Apr 14, 2026
Merged

util: accumulate raw RUv2 in RUDetails#1942
ti-chi-bot[bot] merged 3 commits intotikv:masterfrom
oh-my-tidb:rudetails-ruv2

Conversation

@disksing
Copy link
Copy Markdown
Collaborator

@disksing disksing commented Apr 10, 2026

What is changed and how it works?

  • revert the previous write RUv2 aggregation in CommitDetails
  • add raw RUv2 accumulation and drain APIs on RUDetails so TiDB can incrementally sync raw counters into statement-level RUV2 metrics
  • accumulate execdetailsv2.ruv2 from coprocessor, get, batch get, prewrite and commit responses, while patching RPC counts on the client side when TiKV does not fill them
  • add tests for raw RUv2 drain behavior, RPC count patching, async unary requests and cop stream responses

Check List

  • Unit test

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Fixed RU v2 metric accumulation to ensure metrics are properly captured during execution.
  • Tests

    • Enhanced RU v2 draining validation across async client, streaming, and batch operations.
    • Added coverage for RPC count calculation for various command types.
  • Dependencies

    • Updated pinned dependency versions for core packages and indirect modules in integration tests.

This reverts commit d1ffcbb.

Signed-off-by: disksing <i@disksing.com>
Signed-off-by: disksing <i@disksing.com>
@ti-chi-bot ti-chi-bot Bot added the dco-signoff: yes Indicates the PR's author has signed the dco. label Apr 10, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 10, 2026

📝 Walkthrough

Walkthrough

The PR modifies RU v2 (Resource Unit v2) accounting by introducing raw RU v2 buffering and draining to RUDetails, removing the CommitDetails.WriteRUV2 field, ensuring UpdateTiKVRUV2FromExecDetailsV2 accumulates RU metrics via a new AddRUV2() call, and updating all related tests to validate drain behavior.

Changes

Cohort / File(s) Summary
RU v2 Configuration
config/ruv2.go, config/ruv2_test.go
Added AddRUV2() call in UpdateTiKVRUV2FromExecDetailsV2 to accumulate RU metrics; new test validates DrainRUV2() behavior and ReadRpcCount/WriteRpcCount patching.
Core RU v2 Accounting
util/execdetails.go, util/execdetails_test.go
Removed CommitDetails.Mu.WriteRUV2 field and related accessors/update logic; added RUDetails.rawRUV2 buffering with new AddRUV2() (accumulate), DrainRUV2() (extract and reset), and getRawRUV2() (clone) methods; updated merging/cloning to handle raw RU v2; replaced test helpers with direct RUDetails raw RUV2 handling tests.
Client & RPC Tests
internal/client/client_test.go, internal/client/client_async_test.go, tikvrpc/tikvrpc_test.go
Added test coverage for completedTiKVRUV2RPCCount() function; extended async RU v2 test and streaming RPC test to validate DrainRUV2() returns expected metrics once and then nil on subsequent calls.
Dependency Updates
integration_tests/go.mod
Bumped versions for github.com/pingcap/tidb, github.com/tikv/client-go/v2, google.golang.org/grpc, and transitive dependencies; removed replace directives for github.com/pingcap/tidb and related fork mappings.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • tikv/client-go#1933: Modifies the same UpdateTiKVRUV2FromExecDetailsV2 code path to populate/patch RU v2 read/write RPC counts and accumulate them into RUDetails.
  • tikv/client-go#1926: Touches the same UpdateTiKVRUV2FromExecDetailsV2 function path via a bypass flag alongside the raw RU v2 buffering and draining logic.
  • tikv/client-go#1921: Shares identical dependency version updates and removes the same github.com/pingcap/tidb fork replace directives in integration_tests/go.mod.

Suggested labels

lgtm, approved, size/L

Suggested reviewers

  • nolouch
  • ekexium

Poem

🐰 Hops with glee
Resource Units drain so clean,
Old burdens gone, new flow serene,
Raw RUV2 now buffered bright,
No WriteRUV2 in sight! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'util: accumulate raw RUv2 in RUDetails' directly and accurately summarizes the main changes: moving from CommitDetails write RUv2 aggregation to a new raw RUv2 accumulation mechanism in RUDetails.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 and usage tips.

@ti-chi-bot ti-chi-bot Bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Apr 10, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@integration_tests/go.mod`:
- Line 20: The dependency google.golang.org/grpc in integration_tests/go.mod is
pinned to v1.75.1 which has a critical authorization bypass
(GHSA-p77j-4mvh-x3m3); update the version string for google.golang.org/grpc to
v1.79.3 or later (e.g., v1.80.0) in integration_tests/go.mod and run go mod tidy
to refresh go.sum so the project uses the patched gRPC release.
🪄 Autofix (Beta)

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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3bde73ac-604a-4ee0-8f95-9d1477aa64e1

📥 Commits

Reviewing files that changed from the base of the PR and between d1ffcbb and 753c445.

⛔ Files ignored due to path filters (1)
  • integration_tests/go.sum is excluded by !**/*.sum
📒 Files selected for processing (8)
  • config/ruv2.go
  • config/ruv2_test.go
  • integration_tests/go.mod
  • internal/client/client_async_test.go
  • internal/client/client_test.go
  • tikvrpc/tikvrpc_test.go
  • util/execdetails.go
  • util/execdetails_test.go

Comment thread integration_tests/go.mod
Copy link
Copy Markdown
Contributor

@nolouch nolouch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@ti-chi-bot ti-chi-bot Bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Apr 14, 2026
@ti-chi-bot ti-chi-bot Bot added the lgtm label Apr 14, 2026
@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot Bot commented Apr 14, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ekexium, nolouch

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:

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 ti-chi-bot Bot added approved and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Apr 14, 2026
@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot Bot commented Apr 14, 2026

[LGTM Timeline notifier]

Timeline:

  • 2026-04-14 02:46:39.323913034 +0000 UTC m=+1442804.529273091: ☑️ agreed by nolouch.
  • 2026-04-14 03:34:53.171478701 +0000 UTC m=+1445698.376838758: ☑️ agreed by ekexium.

@ti-chi-bot ti-chi-bot Bot merged commit 1adc54c into tikv:master Apr 14, 2026
18 of 19 checks passed
@disksing disksing deleted the rudetails-ruv2 branch April 14, 2026 04:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved dco-signoff: yes Indicates the PR's author has signed the dco. lgtm size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants