[refactor](recycler) Track recycler KV metrics inline and remove standalone statistics scan by wyxxxcat · Pull Request #68005 · apache/doris

wyxxxcat · 2026-09-15T07:12:43Z

What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary:

Release note

None

Check List (For Author)

Test
- Regression test
- Unit Test
- Manual test (add detailed scripts or steps below)
- No need to test or manual test. Explain why:
  - This is a refactor/code format and no logic has been changed.
  - Previous test can cover this change.
  - No code files have been changed.
  - Other reason
Behavior changed:
- No.
- Yes.
Does this need documentation?
- No.
- Yes.

Check List (For Reviewer who merge this PR)

Confirm the release note
Confirm test cases
Confirm document
Add branch pick label

hello-stephen · 2026-09-15T07:12:50Z

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

What problem was fixed (it's best to include specific error reporting information). How it was fixed.
Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
What features were added. Why was this function added?
Which code was refactored and why was this part of the code refactored?
Which functions were optimized and what is the difference before and after the optimization?

### What problem does this PR solve? Issue Number: N/A Related PR: N/A Problem Summary: Recycler metrics were reported through report() and finish_report(), with shared tablet and segment contexts. Update current-round scanned, expired, recycled, object-byte and duration metrics from context fields, and bind contexts to each instance. This is the first part of the metrics migration; legacy scanning and tests are updated in the following commits. ### Release note Replace legacy recycler round metrics with current-round metrics. ### Check List (For Author) - Test: Manual check of commit scope, git diff --check and final tree equality with the original commit. No build or unit tests were run for this history-only split. The format check could not run because clang-format 16 is unavailable. - Behavior changed: Yes. Recycler metric names and reporting behavior change. - Does this need documentation: No

### What problem does this PR solve? Issue Number: N/A Related PR: N/A Problem Summary: The legacy statistics path scanned KV records before recycling to estimate pending work. Remove these duplicate scans, their configuration options and the statistics_recycle HTTP handler. Actual recycling becomes the source of recycler statistics. Test adaptation follows in the next commit. ### Release note Remove statistics_recycle, enable_recycler_stats_metrics and instance_recycler_statistics_recycle_worker_pool_size. ### Check List (For Author) - Test: Manual check of commit scope, git diff --check and final tree equality with the original commit. No build or unit tests were run for this history-only split. The format check could not run because clang-format 16 is unavailable. - Behavior changed: Yes. Remove the legacy statistics endpoint, configuration and pre-scans. - Does this need documentation: No

wyxxxcat · 2026-09-16T07:32:42Z

run buildall

wyxxxcat · 2026-09-16T08:21:59Z

run buildall

wyxxxcat · 2026-09-20T01:49:38Z

run buildall

hello-stephen · 2026-09-20T02:24:08Z

Cloud UT Coverage Report

Increment line coverage 93.92% (278/296) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	78.90% (2075/2630)
Line Coverage	67.32% (38003/56451)
Region Coverage	53.73% (35544/66151)
Branch Coverage	57.30% (11402/19898)

Issue Number: N/A Related PR: N/A Problem Summary: Update Cloud tests to pass instance and operation names to RecyclerMetricsContext. Initialize the shared test context on first use to avoid bvar and glog access during static initialization. Remove assertions, configuration setup and scan tests for the deleted statistics interface. None - Test: Manual check of commit scope, git diff --check and final tree equality with the original commit. No build or unit tests were run for this history-only split. The format check could not run because clang-format 16 is unavailable. - Behavior changed: No - Does this need documentation: No

wyxxxcat · 2026-09-20T07:13:19Z

run buildall

hello-stephen · 2026-09-20T07:42:50Z

Cloud UT Coverage Report

Increment line coverage 93.92% (278/296) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	78.90% (2075/2630)
Line Coverage	67.35% (38019/56451)
Region Coverage	53.64% (35484/66151)
Branch Coverage	57.29% (11400/19898)

wyxxxcat · 2026-09-21T03:01:12Z

/review

github-actions

Requesting changes because the inline-metrics replacement does not yet preserve accurate or live recycler observability. The inline comments identify 16 distinct production/test defects: current values are not published on several paths; unused or concurrent contexts overwrite shared series; multiple task paths mix units or place counters on the wrong side of object/KV commit boundaries; and versioned/cross-vault rowset aggregation double-counts, omits, or misattributes physical deletion results.

Review completeness: capped/incomplete. The configured final convergence round produced a new validated retry-overcount finding, so this submission does not claim exhaustive convergence.

Critical checkpoint conclusions:

Goal: not achieved. Removing the standalone scan is a useful direction, but the replacement metrics do not consistently describe actual current work or committed/physical outcomes.
Scope/focus: the code removal is focused, but the replacement is distributed across many parallel recycler paths and is not implemented consistently.
Concurrency/thread safety: worker-owned counters use relaxed atomics appropriately and reviewed pools join before context destruction; no deadlock or worker lifetime defect was found. However, the scheduled and manual entry points can publish the same labels concurrently.
Lifecycle: automatic constructor reset/destructor finalization causes missing live publication, preflight zero overwrites, and cross-publisher clobbering. No static-initialization-order issue was found.
Configuration/compatibility: no new configuration is added and removed config/HTTP symbols have no remaining in-repository caller. The endpoint/config/metric removals are externally visible, but repository evidence did not establish a separate compatibility requirement.
Parallel paths/conditions: batch, fallback, prefix-delete, ref-counted, cross-vault, stream, stage, copy-job, and tmp-rowset branches were traced; the inline findings cover the divergent failure and retry boundaries.
Tests/results: the added unit test manually publishes an isolated context, and the regression helper checks only legacy-series presence. Production publishers, exact counter values, partial failures, retries, and shared-rowset outcomes are not covered. No tests or builds were run in this static-only review; passing author/CI checks are not independent validation.
Observability: this is the primary blocker; several new gauges are zero, stale, mislabeled, double-counted, or assigned to the wrong round. Logs retain instance/tablet identifiers, but they do not repair exported metric semantics.
Transactions/persistence/data writes: no new storage format or EditLog path is introduced. Existing object/KV deletion ordering was checked; several metrics are updated at the wrong physical-delete or TxnKV-commit boundary, as noted inline.
FE/BE variables: none added.
Performance: removing the standalone full statistics scan should reduce redundant work; no separate CPU/memory regression was found.
Other: no additional user focus points were supplied.

wyxxxcat requested review from gavinchou, liaoxin01 and luwei16 as code owners September 15, 2026 07:12

wyxxxcat force-pushed the recycler_metrics_opt branch 6 times, most recently from a54f2f9 to 8953cfd Compare September 16, 2026 03:51

wyxxxcat added 2 commits September 16, 2026 14:45

wyxxxcat force-pushed the recycler_metrics_opt branch from 8953cfd to 385ad0b Compare September 16, 2026 06:53

wyxxxcat force-pushed the recycler_metrics_opt branch 2 times, most recently from 3260283 to 53aeff0 Compare September 16, 2026 08:18

wyxxxcat force-pushed the recycler_metrics_opt branch 5 times, most recently from c4908c6 to 7a8fe8c Compare September 20, 2026 01:49

wyxxxcat force-pushed the recycler_metrics_opt branch from 7a8fe8c to 558a723 Compare September 20, 2026 06:33

github-actions Bot requested changes Sep 21, 2026

View reviewed changes

Conversation

wyxxxcat commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

Release note

Check List (For Author)

Check List (For Reviewer who merge this PR)

Uh oh!

hello-stephen commented Sep 15, 2026

Uh oh!

wyxxxcat commented Sep 16, 2026

Uh oh!

wyxxxcat commented Sep 16, 2026

Uh oh!

wyxxxcat commented Sep 20, 2026

Uh oh!

hello-stephen commented Sep 20, 2026

Cloud UT Coverage Report

Uh oh!

wyxxxcat commented Sep 20, 2026

Uh oh!

hello-stephen commented Sep 20, 2026

Cloud UT Coverage Report

Uh oh!

wyxxxcat commented Sep 21, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

wyxxxcat commented Sep 15, 2026 •

edited

Loading