Skip to content

config: update client-go and change RUScale default value to 1.34#67320

Merged
ti-chi-bot[bot] merged 4 commits into
pingcap:masterfrom
oh-my-tidb:feature/ru-scale-default-1.34
Mar 27, 2026
Merged

config: update client-go and change RUScale default value to 1.34#67320
ti-chi-bot[bot] merged 4 commits into
pingcap:masterfrom
oh-my-tidb:feature/ru-scale-default-1.34

Conversation

@disksing
Copy link
Copy Markdown
Contributor

@disksing disksing commented Mar 26, 2026

What problem does this PR solve?

Issue Number: ref #67199

Problem Summary:
Update client-go dependency and change RUScale default value from 5697.054498 to 1.34.

based on suggestions from @likidu.

What changed and how does it work?

  1. Update client-go to latest master version (tikv/client-go@7286767)
  2. Change RUScale default from 5697.054498 to 1.34

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No need to test
    • I checked and no code files have been changed.

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

Summary by CodeRabbit

  • Chores

    • Updated TiKV client dependency to a newer patched revision.
    • Changed default RU v2 scaling factors (primary default → 1.34; client-specific example → 1.40), affecting resource-unit calculations and shipped example config values.
  • Tests

    • Updated test expectations to reflect the new RU v2 scaling and resulting RU calculation outputs.

- Update client-go to latest master version
- Change RUScale default from 5697.054498 to 1.34

Signed-off-by: disksing <i@disksing.com>
@ti-chi-bot ti-chi-bot Bot added do-not-merge/needs-linked-issue do-not-merge/needs-tests-checked do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Mar 26, 2026
@pantheon-ai
Copy link
Copy Markdown

pantheon-ai Bot commented Mar 26, 2026

Review Complete

Findings: 1 issues
Posted: 0
Duplicates/Skipped: 1

ℹ️ Learn more details on Pantheon AI.

@ti-chi-bot ti-chi-bot Bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Mar 26, 2026
@tiprow
Copy link
Copy Markdown

tiprow Bot commented Mar 26, 2026

Hi @disksing. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

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.

@hawkingrei
Copy link
Copy Markdown
Member

/ok-to-test

@ti-chi-bot ti-chi-bot Bot added the ok-to-test Indicates a PR is ready to be tested. label Mar 26, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 26, 2026

📝 Walkthrough

Walkthrough

Bumped a tikv client dependency and changed default RU v2 scaling values in code and example config; updated tests and DEPS pin to align with the new scaling and dependency revision.

Changes

Cohort / File(s) Summary
Dependency / Bazel
go.mod, DEPS.bzl
Updated github.com/tikv/client-go/v2 pin from v2.0.8-0.20260319090224-0569333efa49v2.0.8-0.20260326064539-728676760deb; updated corresponding archive URLs/sha in DEPS.bzl.
RU v2 Defaults & Examples
pkg/config/config.go, pkg/config/config.toml.example
Changed default RU v2 scaling: code RUV2.RUScale 5697.0544981.34; example TOML updated ([ru-v2].ru-scale1.34, [tikv-client.ru-v2].ru-scale1.40).
Tests
pkg/config/config_test.go, pkg/util/execdetails/execdetails_test.go
Added assertions for new RUScale values and updated expected RU calculation/formatting outputs to match the new scaling factors.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • XuHuaiyu
  • nolouch
  • wjhuang2016

Poem

🐇 I hop through modules, tiny and spry,
A pin bumped forward, RUScale tucked shy.
Tests realigned, examples set neat,
I twitch my whiskers — the patch is complete. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% 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
Title check ✅ Passed The title clearly summarizes the two main changes: updating client-go and changing the RUScale default value to 1.34, matching the primary objectives of the PR.
Description check ✅ Passed PR description follows the template structure with issue reference, problem summary, changes explanation, and checked test coverage with appropriate release note.

✏️ 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.

@disksing
Copy link
Copy Markdown
Contributor Author

Recreating with proper template

@disksing disksing closed this Mar 26, 2026
@disksing disksing reopened this Mar 26, 2026
@ti-chi-bot ti-chi-bot Bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/needs-linked-issue do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Mar 26, 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: 3

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

Inline comments:
In `@go.mod`:
- Line 127: Upstream client-go change bumped TiKV RU v2 default to 1.40 (commit
728676760deb) causing runtime divergence with TiDB's DefaultRUV2Config() which
remains 1.34; update the code that seeds TiKV client defaults
(pkg/config/config.go where TiKVClient is initialized from
tikvcfg.DefaultConfig()) to explicitly set the RU v2 scale to TiDB's value (use
the same constant or literal 1.34) after calling tikvcfg.DefaultConfig(), or
alternatively pin the dependency in go.mod to the prior client-go commit, so
runtime [tikv-client.ru-v2].ru-scale matches DefaultRUV2Config(); reference
DefaultRUV2TiKVConfig(), DefaultRUV2Config(), TiKVClient, and
tikvcfg.DefaultConfig() when making the change.

In `@pkg/config/config.go`:
- Around line 367-370: The test hardcodes RU v2 expectations that no longer
match DefaultRUV2Config().RUScale (1.34); update the assertions in
pkg/util/execdetails/execdetails_test.go (the test around the hardcoded 114198.0
/ 296136.0) to either compute expected values by applying
DefaultRUV2Config().RUScale (from DefaultRUV2Config / RUV2Config.RUScale) to the
fixture weights used by pkg/util/execdetails/ruv2_metrics.go, or replace the
literals with the recalculated numbers (≈26.86 and ≈181964.86) so the test
reflects the current scaling behavior.

In `@pkg/config/config.toml.example`:
- Around line 440-442: Update the explanatory comment for the ru-scale setting
to accurately describe the new default behavior (that ru-scale was changed from
5697.054498 to 1.34 and that this drastically reduces computed RU outputs) and
ensure the same updated wording is applied to the RUV2Config comment in
pkg/config/config.go; locate the ru-scale key in config.toml.example and the
RUV2Config struct/comment in config.go and replace the outdated "match legacy RU
values for compatibility" phrasing with a note that documents the prior value,
the new value, and the impact on RU calculations so operators are not misled.
🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 1b1a9e57-631e-4a40-9a84-a1da6e13d1b4

📥 Commits

Reviewing files that changed from the base of the PR and between e07318b and d37ac48.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (3)
  • go.mod
  • pkg/config/config.go
  • pkg/config/config.toml.example

Comment thread go.mod
Comment thread pkg/config/config.go
Comment thread pkg/config/config.toml.example
@ti-chi-bot ti-chi-bot Bot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. and removed do-not-merge/needs-tests-checked release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Mar 26, 2026
@disksing disksing added the release-note-none Denotes a PR that doesn't merit a release note. label Mar 26, 2026
@ti-chi-bot ti-chi-bot Bot removed the do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. label Mar 26, 2026
Signed-off-by: disksing <i@disksing.com>
@disksing
Copy link
Copy Markdown
Contributor Author

/retest

Signed-off-by: disksing <i@disksing.com>
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.

♻️ Duplicate comments (1)
pkg/config/config.toml.example (1)

440-442: ⚠️ Potential issue | 🟡 Minor

Update stale ru-scale compatibility note.

At Line 441, the comment still says this value is chosen to match legacy RU values, but Line 442 now sets 1.34. Please update this wording (and the matching RUV2Config.RUScale comment in pkg/config/config.go) so operators are not misled.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/config/config.toml.example` around lines 440 - 442, The comment for
ru-scale is outdated and still claims the value matches legacy RU values while
the file sets ru-scale = 1.34; update the human-facing comment in
pkg/config/config.toml.example (the line above ru-scale) to remove or rephrase
the "match legacy RU values" claim and instead state the actual purpose and
rationale for the 1.34 value (e.g., "scale factor used to convert RU v2 float
values into scaled integer values; set to 1.34 for current
compatibility/rounding behavior"). Also update the corresponding Go doc comment
for RUV2Config.RUScale in pkg/config/config.go to the same corrected wording so
both descriptions are consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@pkg/config/config.toml.example`:
- Around line 440-442: The comment for ru-scale is outdated and still claims the
value matches legacy RU values while the file sets ru-scale = 1.34; update the
human-facing comment in pkg/config/config.toml.example (the line above ru-scale)
to remove or rephrase the "match legacy RU values" claim and instead state the
actual purpose and rationale for the 1.34 value (e.g., "scale factor used to
convert RU v2 float values into scaled integer values; set to 1.34 for current
compatibility/rounding behavior"). Also update the corresponding Go doc comment
for RUV2Config.RUScale in pkg/config/config.go to the same corrected wording so
both descriptions are consistent.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 7e64db84-0ee5-41bf-b245-cd91dcb2e02a

📥 Commits

Reviewing files that changed from the base of the PR and between ffcc8e4 and c17715b.

📒 Files selected for processing (3)
  • pkg/config/config.toml.example
  • pkg/config/config_test.go
  • pkg/util/execdetails/execdetails_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • pkg/config/config_test.go

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.9439%. Comparing base (9d93f42) to head (5398c3c).
⚠️ Report is 10 commits behind head on master.

Additional details and impacted files
@@               Coverage Diff                @@
##             master     #67320        +/-   ##
================================================
+ Coverage   77.7978%   77.9439%   +0.1460%     
================================================
  Files          2022       1943        -79     
  Lines        555528     547818      -7710     
================================================
- Hits         432189     426991      -5198     
+ Misses       121595     120794       -801     
+ Partials       1744         33      -1711     
Flag Coverage Δ
integration 41.4630% <100.0000%> (-6.6684%) ⬇️
unit 77.0202% <100.0000%> (+0.6826%) ⬆️

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

Components Coverage Δ
dumpling 61.5065% <ø> (ø)
parser ∅ <ø> (∅)
br 48.8787% <ø> (-11.9935%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: disksing <i@disksing.com>
@ti-chi-bot ti-chi-bot Bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 26, 2026
@ti-chi-bot ti-chi-bot Bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Mar 26, 2026
@ti-chi-bot ti-chi-bot Bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Mar 26, 2026
@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot Bot commented Mar 26, 2026

[LGTM Timeline notifier]

Timeline:

  • 2026-03-26 14:16:21.027564791 +0000 UTC m=+450577.063635041: ☑️ agreed by nolouch.
  • 2026-03-26 14:38:46.873886626 +0000 UTC m=+451922.909956876: ☑️ agreed by cfzjywxk.

@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot Bot commented Mar 27, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cfzjywxk, nolouch, yudongusa

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 the approved label Mar 27, 2026
@disksing
Copy link
Copy Markdown
Contributor Author

/test mysql-test

1 similar comment
@disksing
Copy link
Copy Markdown
Contributor Author

/test mysql-test

@tiprow
Copy link
Copy Markdown

tiprow Bot commented Mar 27, 2026

@disksing: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test fast_test_tiprow
/test tidb_parser_test

Use /test all to run all jobs.

Details

In response to this:

/test mysql-test

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.

@tiprow
Copy link
Copy Markdown

tiprow Bot commented Mar 27, 2026

@disksing: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test fast_test_tiprow
/test tidb_parser_test

Use /test all to run all jobs.

Details

In response to this:

/test mysql-test

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.

@disksing
Copy link
Copy Markdown
Contributor Author

/test unit-test

@tiprow
Copy link
Copy Markdown

tiprow Bot commented Mar 27, 2026

@disksing: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test fast_test_tiprow
/test tidb_parser_test

Use /test all to run all jobs.

Details

In response to this:

/test unit-test

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 ti-chi-bot Bot merged commit 3742691 into pingcap:master Mar 27, 2026
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved lgtm ok-to-test Indicates a PR is ready to be tested. release-note-none Denotes a PR that doesn't merit a release note. 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.

5 participants