*: remove deprecated RU v2 code - #71056
Conversation
ti-chi-bot
Bot
added
release-note-none
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThis change removes legacy RU v2 configuration, metrics, executor accounting, session propagation, cursor consumption reporting, and TopSQL sampling. RU v3 paths remain. TiKV coprocessor response bytes remain as the retained RU v2 metric. ChangesRU v2 accounting removal
Priority: ⬇️ Low Estimated code review effort: 4 (Complex) | ~60 minutes Change: Other Suggested reviewers: Merge Risk: ⚪ Minimal · up to The change removes deprecated RU v2 accounting while preserving RU v3 charging and retained response-byte tracking; no concrete current-head failure was established. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. I’m a rabbit with tidy RU charts, Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #71056 +/- ##
================================================
- Coverage 76.3180% 72.4578% -3.8603%
================================================
Files 2041 2102 +61
Lines 556057 594884 +38827
================================================
+ Hits 424372 431040 +6668
- Misses 130785 162627 +31842
- Partials 900 1217 +317
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟡 Changes recommended
A critical configuration migration issue and a moderate TopRU behavior issue remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This pull request removes deprecated RU v2 accounting and instrumentation while retaining RU v3 response-byte collection.
Changes:
- Removes RU v2 metrics, weights, calculations, and obsolete tests.
- Simplifies RU tracking across executors, sessions, servers, and cursors.
- Updates configuration, mocks, metrics, and Bazel dependencies.
File summaries
| File | Summary |
|---|---|
pkg/util/topsql/stmtstats/stmtstats.go |
Removes RU v2 sampling. Moderate (1 vote): enabled TopRU can emit zero samples under RUVersionV2. |
pkg/util/topsql/stmtstats/stmtstats_test.go |
Removes obsolete RU v2 TopSQL tests. |
pkg/util/topsql/stmtstats/rustats.go |
Removes RU v2 statement statistics state. |
pkg/util/topsql/stmtstats/BUILD.bazel |
Updates TopSQL build dependencies. |
pkg/util/mock/context.go |
Removes deprecated RU v2 mock wiring. |
pkg/util/execdetails/ruv2_metrics.go |
Retains response-byte collection needed by RU v3. |
pkg/util/execdetails/runtime_stats.go |
Removes RU v2 runtime-stat accounting. |
pkg/util/execdetails/execdetails_test.go |
Updates execution-detail tests. |
pkg/util/execdetails/BUILD.bazel |
Updates execution-detail build inputs. |
pkg/sessionctx/variable/tests/session_test.go |
Updates session-variable tests. |
pkg/sessionctx/variable/tests/BUILD.bazel |
Updates session-variable test build configuration. |
pkg/sessionctx/variable/slow_log.go |
Removes RU v2 slow-log reporting. |
pkg/sessionctx/variable/session.go |
Removes RU v2 session configuration and accounting. |
pkg/session/tidb.go |
Updates TiDB session integration. |
pkg/session/tidb_test.go |
Updates TiDB session tests. |
pkg/session/session.go |
Removes RU v2 session handling. |
pkg/session/BUILD.bazel |
Updates session build dependencies. |
pkg/server/internal/resultset/resultset.go |
Simplifies cursor RU tracking. |
pkg/server/internal/resultset/cursor.go |
Removes obsolete cursor RU v2 accounting. |
pkg/server/internal/resultset/BUILD.bazel |
Updates resultset build inputs. |
pkg/server/driver_tidb.go |
Updates driver integration. |
pkg/server/conn.go |
Reports cursor response-byte deltas. |
pkg/server/conn_stmt.go |
Removes RU v2 statement handling. |
pkg/server/conn_stmt_test.go |
Updates connection statement tests. |
pkg/server/BUILD.bazel |
Updates server build dependencies. |
pkg/planner/core/stats.go |
Removes RU v2 planner statistics. |
pkg/planner/core/planbuilder.go |
Removes RU v2 plan accounting. |
pkg/planner/core/casetest/indexmerge/indexmerge_test.go |
Removes obsolete index-merge coverage. |
pkg/planner/core/casetest/indexmerge/BUILD.bazel |
Updates index-merge test build configuration. |
pkg/metrics/ru_v2.go |
Removes deprecated RU v2 metrics. |
pkg/metrics/metrics.go |
Updates metric registration. |
pkg/metrics/metrics_internal_test.go |
Updates internal metric tests. |
pkg/metrics/BUILD.bazel |
Updates metrics build dependencies. |
pkg/executor/update.go |
Removes RU v2 update accounting. |
pkg/executor/statement_ru_result_test.go |
Updates statement RU result tests. |
pkg/executor/replace.go |
Removes RU v2 replace accounting. |
pkg/executor/internal/exec/executor.go |
Removes executor RU v2 hooks. |
pkg/executor/internal/exec/executor_test.go |
Updates executor tests. |
pkg/executor/internal/exec/BUILD.bazel |
Updates executor build dependencies. |
pkg/executor/insert.go |
Removes RU v2 insert accounting. |
pkg/executor/insert_common.go |
Removes shared RU v2 insert handling. |
pkg/executor/explain.go |
Removes RU v2 explain accounting. |
pkg/executor/explain_unit_test.go |
Updates explain tests. |
pkg/executor/delete.go |
Removes RU v2 delete accounting. |
pkg/executor/BUILD.bazel |
Updates executor build configuration. |
pkg/executor/adapter.go |
Retains final RU evidence collection. |
pkg/executor/adapter_test.go |
Updates adapter tests. |
pkg/executor/adapter_slow_log.go |
Removes RU v2 slow-log handling. |
pkg/executor/adapter_internal_test.go |
Updates internal adapter tests. |
pkg/distsql/context/context.go |
Retains response bytes and removes RU v2 context state. |
pkg/distsql/context/context_test.go |
Updates distributed SQL context tests. |
pkg/config/config.toml.example |
Removes RU v2 weights. Nit (1 vote): update the stale heading for statement RU v3. |
pkg/config/config.go |
Removes RU v2 configuration. Critical (2 votes): add legacy keys to removedConfig and cover migration in tests. |
pkg/config/config_test.go |
Updates configuration tests. |
pkg/config/BUILD.bazel |
Updates configuration build dependencies. |
Review details
Suppressed comments (2)
pkg/config/config.toml.example:441
- The example section no longer contains RU v2 weights, but this heading still says it does. Users following the generated example will look for settings that no longer exist; update the heading to describe statement RU v3 reporting/weights.
# raw units by engine/opclass and calculation status for calibration.
pkg/util/topsql/stmtstats/stmtstats.go:248
- When the domain reports
RUVersionV2—the version used by the statement RU v3 path (pkg/executor/show_ddl_jobs.go:147-149)—this condition makes every TopRU sample zero. TopRU is still enabled byobserveStmtBeginForTopProfilingand its reporter/subscription path remains active, so V2 deployments silently emit no TopRU records instead of reporting or rejecting the feature. Please either feed TopRU from the supported RU v3 total or explicitly disable/remove TopRU for this version; returning zero leaves an enabled API as a no-op.
- Files reviewed: 55/55 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| // Statement weights convert RU v3 raw work units to RU. They must be finite | ||
| // and non-negative; zero disables the corresponding charge. Their defaults | ||
| // are uncalibrated internal placeholders, not billing values. |
There was a problem hiding this comment.
This is expected, "RU-V2" has never been GA, so don't need to consider its compatibility.
ti-chi-bot
Bot
added
lgtm
and removed
needs-1-more-lgtm
[LGTM Timeline notifier]Timeline:
|
|
/hold Copilot review comment. You can remove the hold if you can either resolve the comment or determine that it is not relevant. |
ti-chi-bot
Bot
added
the
do-not-merge/hold
terry1purcell
left a comment
terry1purcell
left a comment
There was a problem hiding this comment.
Pending removal of hold (which I am OK with if you confirm whether it is critical or not).
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: terry1purcell, windtalker, yibin87 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test check-dev2 |
|
/unhold |
ti-chi-bot
Bot
removed
the
do-not-merge/hold
|
/retest |
What problem does this PR solve?
Issue Number: ref #70747
Problem Summary: *: remove deprecated RU v2 code
What changed and how does it work?
Remove legacy RUv2 instrumentation, calculations, metrics, reporting paths, configuration, and obsolete tests. Preserve RUv3 accounting and the shared data collection it still requires.
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.
Summary by CodeRabbit
Configuration
Behavior Changes
Compatibility