Skip to content

ruv2: track TiFlash RU in RUDetails#1918

Merged
disksing merged 1 commit intotikv:masterfrom
disksing:disksing/tiflash-ru
Mar 19, 2026
Merged

ruv2: track TiFlash RU in RUDetails#1918
disksing merged 1 commit intotikv:masterfrom
disksing:disksing/tiflash-ru

Conversation

@disksing
Copy link
Copy Markdown
Collaborator

@disksing disksing commented Mar 19, 2026

Summary

  • track TiFlash RU separately in RUDetails while still accumulating RRU/WRU
  • keep TiKV RU v2 in floating-point form instead of truncating to int
  • remove unused TiDB RU v2 helpers and update the affected tests

Tests

  • go test ./config -run TestUpdateTiKVRUV2FromExecDetailsV2AndWriteRPCCount
  • go test ./internal/client -run TestSendRequestAsyncUpdateTiKVRUV2
  • go test ./util -run 'TestRUDetailsUpdateTiFlash|TestTimeDetailMergeNil'

Summary by CodeRabbit

Release Notes

  • New Features

    • Added TiFlash resource unit (RU) tracking for v2 RU calculations.
  • Refactor

    • Changed TiKV RU v2 return type to floating-point for improved precision.
    • Removed TiDB RU v2 tracking APIs.
  • Tests

    • Updated test assertions to accommodate floating-point RU comparisons.

Signed-off-by: disksing <i@disksing.com>
@ti-chi-bot ti-chi-bot bot added dco-signoff: yes Indicates the PR's author has signed the dco. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 19, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 19, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c071b6eb-6fcc-454f-82bc-8bf7f4433751

📥 Commits

Reviewing files that changed from the base of the PR and between ba15141 and ad03077.

📒 Files selected for processing (4)
  • config/ruv2_test.go
  • internal/client/client_async_test.go
  • util/execdetails.go
  • util/execdetails_test.go

📝 Walkthrough

Walkthrough

The pull request refactors RU (Resource Unit) tracking in the client library: TiKV RU v2 calculation changes from int64 to float64, all TiDB RU v2 tracking APIs are removed, TiFlash RU tracking is introduced with a new atomic float field, and test assertions are updated to use floating-point comparison with tolerance.

Changes

Cohort / File(s) Summary
Test Assertion Updates
config/ruv2_test.go, internal/client/client_async_test.go
Change test expectations for TiKV RU v2 from exact integer equality to approximate floating-point comparison using require.InDelta with 1e-9 tolerance.
RU Details Core Implementation
util/execdetails.go
Remove TiDB RU v2 tracking (APIs TiDBRUV2(), AddTiDBRUV2(), TotalRUV2()); convert TiKVRUV2() return type from int64 to float64; add new tiflashRU atomic float field and TiflashRU() getter; add UpdateTiFlash() method to accumulate TiFlash RU from consumption protos.
RU Details Test Coverage
util/execdetails_test.go
Add import for rmpb (resource_manager proto); introduce TestRUDetailsUpdateTiFlash to verify TiFlash RU accumulation, cloning, and preservation across operations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

lgtm, size/XL

Suggested reviewers

  • nolouch
  • ekexium

Poem

🐰 A rabbit hops through resource streams,
RU v2 flows like digital dreams,
TiKV now floats, no integers bound,
TiFlash joins in with nary a sound,
TiDB steps back—the refactoring's complete! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.57% 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 title 'ruv2: track TiFlash RU in RUDetails' directly describes the main change: adding TiFlash RU tracking to the RUDetails structure, which is the primary objective of this PR.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

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 needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Mar 19, 2026
@ti-chi-bot ti-chi-bot bot added the lgtm label Mar 19, 2026
@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot bot commented Mar 19, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cfzjywxk, 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:

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 Mar 19, 2026
@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot bot commented Mar 19, 2026

[LGTM Timeline notifier]

Timeline:

  • 2026-03-19 08:26:56.046206857 +0000 UTC m=+431943.133864395: ☑️ agreed by JmPotato.
  • 2026-03-19 08:58:09.359658582 +0000 UTC m=+433816.447316129: ☑️ agreed by cfzjywxk.

@disksing disksing merged commit 0569333 into tikv:master Mar 19, 2026
18 of 19 checks passed
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/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants