[opt](cloud) Add recycler rowset recycling benchmarks test by wyxxxcat · Pull Request #68043 · apache/doris

wyxxxcat · 2026-09-16T02:21:45Z

What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary:

Add comprehensive recycler rowset benchmarks covering legacy and new rowset formats, transaction aborts, delete bitmap variants, packed files, missing schemas, and mixed workloads.

Treat legacy rowsets with empty resource IDs as successfully queued for KV deletion instead of reporting a recycling failure.

The current statistical logic has a bug; there's no need to worry about recycled_num and recycled_bytes.

The performance check watermark can be controlled via the environment variable recycler_benchmark_duration_tolerance_ratio.

recycler benchmark: operation=recycle_rowsets
  branch                                           total_ms   recycled_num    recycled_bytes      get_keys      put_keys      del_keys    total_keys
  compacted_empty                                   1708.36          30000          46080000         30000             0         30000         60000
  compacted_with_data/data_only                     3489.12          30000          46080000         30001             0         30000         60001
  compacted_with_data/delete_bitmap_v1              4084.44          30000          46080000         30000             0         30000         60000
  compacted_with_data/delete_bitmap_v2              3869.88          30000          46080000         60000             0         60000        120000
  compacted_with_packed_data/data_only              9444.14              0                 0         63000         30000         33000        126000
  compacted_with_packed_data/delete_bitmap_v1       9465.94              0                 0         63000         30000         33000        126000
  compacted_with_packed_data/delete_bitmap_v2      10366.38          30000          46080000         93000         30000         63000        186000
  compacted_without_schema                          6544.83          30000          46080000         30000             0         30000         60000
  legacy_empty_resource                             1657.66              0                 0         30000             0         30000         60000
  legacy_with_resource                              5227.31          30000                 0         60000             0         60000        120000
  mixed                                            46285.68          90000          92160000        600000        120000        360000       1080000
  prepare_abort                                    22566.18              0                 0        270000         90000         90000        450000
  prepare_direct                                    5841.44              0                 0         60000             0         60000        120000
  prepare_mark                                      8208.94              0                 0        120000         30000         60000        210000
  total                                           138760.29         300000         368640000       1539001        330000        969000       2838001

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-16T02:21:49Z

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?

wyxxxcat · 2026-09-16T02:21:53Z

run buildall

wyxxxcat · 2026-09-16T03:33:47Z

run buildall

wyxxxcat · 2026-09-16T06:07:04Z

run buildall

hello-stephen · 2026-09-16T07:05:42Z

Cloud UT Coverage Report

Increment line coverage 100.00% (1/1) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	77.51% (2058/2655)
Line Coverage	65.76% (37741/57390)
Region Coverage	53.30% (35484/66568)
Branch Coverage	56.62% (11362/20066)

wyxxxcat · 2026-09-18T02:47:41Z

run cloudut

hello-stephen · 2026-09-18T03:37:23Z

Cloud UT Coverage Report

Increment line coverage 100.00% (1/1) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	77.52% (2059/2656)
Line Coverage	65.94% (37964/57571)
Region Coverage	53.23% (35507/66704)
Branch Coverage	56.78% (11424/20118)

github-actions

Request changes. The non-versioned status change is reasonable, but the same legacy shape is still treated as an error on active multi-version paths, and the benchmark integration has three determinism/portability problems.

Critical checkpoint conclusions:

Goal and test proof: the non-versioned branch is covered; the multi-version dropped-tablet path remains incomplete and untested.
Scope, parallel paths, and compatibility: the production edit is small, but parallel versioned handlers were missed; there is no wire/storage-format change.
Concurrency and lifecycle: worker-pool draining, vector ownership, real-S3 accessor precedence, and GUID-rooted cleanup look sound; no new lock, deadlock, or static-lifetime issue was found.
Persistence and deletion: the missed tablet path exits before its local range removal and can delay dropped-metadata cleanup until the separate legacy-expiration sweep.
Configuration and protocol: there are no production config or FE/BE variable changes; benchmark-only environment settings do not require runtime propagation.
Tests and performance: coverage spans many branches, but default correctness UT inherits host-derived wall-clock gates, one packed-file check risks FDB transaction timeout, and shared FDB state invalidates later scenarios after a partial failure.
Observability and errors: existing logs identify the instance, tablet, and key; asynchronous best-effort deletion failures retain keys for retry and were not raised as a separate issue.
User focus: none was supplied.

Validation was static-only as required; no local builds or tests were run. Exact-head Cloud UT currently reports pass, but that single controlled run does not clear the path and portability issues above.

wyxxxcat · 2026-09-21T07:02:08Z

run buildall

wyxxxcat · 2026-09-21T08:22:51Z

@@ -5880,7 +5880,7 @@ int InstanceRecycler::recycle_rowsets() {
                LOG(INFO) << "delete the recycle rowset kv that has empty resource_id, key="
                          << hex(k) << " value=" << proto_to_json(rowset);
                rowset_keys.emplace_back(k);
-                return -1;
+                return 0;


We need to ensure that the rowset recycled through this path is successful.

wyxxxcat · 2026-09-23T07:02:17Z

run cloudut

wyxxxcat · 2026-09-23T08:11:55Z

run cloudut

hello-stephen · 2026-09-23T09:21:26Z

Cloud UT Coverage Report

Increment line coverage 100.00% (1/1) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	77.54% (2064/2662)
Line Coverage	65.86% (37992/57686)
Region Coverage	53.34% (35642/66816)
Branch Coverage	56.84% (11475/20190)

wyxxxcat · 2026-09-24T01:55:22Z

run buildall

wyxxxcat · 2026-09-28T02:38:53Z

/review

github-actions

Review summary

Static review of the authoritative base/head diff for the requested scope (no user focus supplied). No builds or tests were run, per the review instructions.

Goal and correctness: the non-versioned empty-resource branch now queues the legacy key and reports success; the related versioned-path compatibility concern is already covered by the existing inline thread, with no additional recycler defect found.
Concurrency and lifecycle: recycler worker-pool completion and teardown were traced; no new production locking/lifecycle issue was found. The packed benchmark's intended cross-worker conflict workload is not actually concurrent (inline finding below).
Persistence and state: the benchmark's fixed namespace/partial-failure contamination is already covered by the existing state-isolation thread.
Compatibility and parallel paths: the unchanged versioned handlers remain covered by the existing recycler comment; this review does not duplicate it.
Test coverage: the new abort-order scenario is vacuous because it registers no observation callback (inline finding below).
Performance/observability: the default-UT wall-clock and packed verification transaction risks are already covered by existing threads; no additional distinct issue was retained.

The two inline findings below should be addressed before merge.

github-actions · 2026-09-28T03:09:55Z

+        ASSERT_EQ(recycler_->recycle_rowsets(), 0);
+        ASSERT_EQ(count_recycle_rowsets(), 1);
+
+        auto* sp = SyncPoint::get_instance();


[P2] Make the abort-before-delete check observe transaction state

prepare_abort_before_delete claims to validate ordering, but this block never calls set_call_back; it only enables an empty SyncPoint and then checks that the rowset eventually disappears. A regression that deletes the PREPARE rowset before aborting its transaction would still pass. Please install the existing MockAccessor::delete_prefix callback (and assert the seeded txn is TXN_STATUS_ABORTED) before checking rowset removal, or remove the ordering claim.

github-actions · 2026-09-28T03:09:55Z

+    packed_info.set_state(PackedFileInfoPB::NORMAL);
+    int64_t offset = 0;
+    for (int64_t j = 0; j < kRowsetsPerPackedFile; ++j) {
+        // Spread references across scan batches so different recycler workers can


[P2] Make the packed workload actually exercise cross-worker retries

The comment says these references are spread across scan batches so workers contend, but this test seeds exactly 10,000 rowset keys and scan_and_recycle fetches 10,000 per page; loop_done then submits that whole page to one delete_rowset_data worker, which updates packed-file KVs serially. Thus no same-packed-file conflict/retry is exercised, and the ten-pass loop only checks eventual counts. Please force multiple batches/workers (or add a barrier) and assert a retry/conflict was observed if this is intended to benchmark that path.

a

Problem Summary: Add comprehensive recycler rowset benchmarks covering legacy and new rowset formats, transaction aborts, delete bitmap variants, packed files, missing schemas, and mixed workloads. Treat legacy rowsets with empty resource IDs as successfully queued for KV deletion instead of reporting a recycling failure. **The current statistical logic has a bug; there's no need to worry about recycled_num and recycled_bytes.** The performance check watermark can be controlled via the environment variable `recycler_benchmark_duration_tolerance_ratio`. ``` recycler benchmark: operation=recycle_rowsets branch total_ms recycled_num recycled_bytes get_keys put_keys del_keys total_keys compacted_empty 1708.36 30000 46080000 30000 0 30000 60000 compacted_with_data/data_only 3489.12 30000 46080000 30001 0 30000 60001 compacted_with_data/delete_bitmap_v1 4084.44 30000 46080000 30000 0 30000 60000 compacted_with_data/delete_bitmap_v2 3869.88 30000 46080000 60000 0 60000 120000 compacted_with_packed_data/data_only 9444.14 0 0 63000 30000 33000 126000 compacted_with_packed_data/delete_bitmap_v1 9465.94 0 0 63000 30000 33000 126000 compacted_with_packed_data/delete_bitmap_v2 10366.38 30000 46080000 93000 30000 63000 186000 compacted_without_schema 6544.83 30000 46080000 30000 0 30000 60000 legacy_empty_resource 1657.66 0 0 30000 0 30000 60000 legacy_with_resource 5227.31 30000 0 60000 0 60000 120000 mixed 46285.68 90000 92160000 600000 120000 360000 1080000 prepare_abort 22566.18 0 0 270000 90000 90000 450000 prepare_direct 5841.44 0 0 60000 0 60000 120000 prepare_mark 8208.94 0 0 120000 30000 60000 210000 total 138760.29 300000 368640000 1539001 330000 969000 2838001 ```

…#68043 (#68581) Cherry-picked from #68043 Co-authored-by: Yixuan Wang <[email protected]>

…68622) ### What problem does this PR solve? Related PR: #68043 Problem Summary: The recycler benchmark ran as part of recycler_test and used fixed elapsed-time thresholds, making default test runs slower and timing-dependent. Move it into a standalone recycler_benchmark_test binary and skip it during default Cloud test discovery while preserving explicit selection with --test. Reduce the workload to 5,000 rowsets per branch, update all 14 observed KV count baselines, and remove elapsed-time failure thresholds. Timings remain in the benchmark report.

…pache#68622) ### What problem does this PR solve? Related PR: apache#68043 Problem Summary: The recycler benchmark ran as part of recycler_test and used fixed elapsed-time thresholds, making default test runs slower and timing-dependent. Move it into a standalone recycler_benchmark_test binary and skip it during default Cloud test discovery while preserving explicit selection with --test. Reduce the workload to 5,000 rowsets per branch, update all 14 observed KV count baselines, and remove elapsed-time failure thresholds. Timings remain in the benchmark report.

…68622) ### What problem does this PR solve? Related PR: #68043 Problem Summary: The recycler benchmark ran as part of recycler_test and used fixed elapsed-time thresholds, making default test runs slower and timing-dependent. Move it into a standalone recycler_benchmark_test binary and skip it during default Cloud test discovery while preserving explicit selection with --test. Reduce the workload to 5,000 rowsets per branch, update all 14 observed KV count baselines, and remove elapsed-time failure thresholds. Timings remain in the benchmark report.

wyxxxcat requested review from gavinchou, liaoxin01 and luwei16 as code owners September 16, 2026 02:21

wyxxxcat force-pushed the benchmark_recycler branch 7 times, most recently from f10532e to 29cd37b Compare September 16, 2026 03:23

wyxxxcat added dev/3.1.x dev/4.0.x dev/4.1.x labels Sep 16, 2026

wyxxxcat force-pushed the benchmark_recycler branch 2 times, most recently from b5e19dc to 5df96b4 Compare September 16, 2026 06:06

wyxxxcat force-pushed the benchmark_recycler branch 4 times, most recently from a590ae9 to 25f0ef2 Compare September 18, 2026 02:47

wyxxxcat force-pushed the benchmark_recycler branch 2 times, most recently from 970b682 to 444e884 Compare September 20, 2026 02:57

1

34c769f

wyxxxcat force-pushed the benchmark_recycler branch from 444e884 to 34c769f Compare September 20, 2026 03:57

github-actions Bot requested changes Sep 20, 2026

View reviewed changes

Comment thread cloud/src/recycler/recycler.cpp Resolved

Comment thread cloud/test/CMakeLists.txt Resolved

Comment thread cloud/test/recycler_benchmark_test.cpp Resolved

Comment thread cloud/test/recycler_benchmark_test.cpp Resolved

wyxxxcat force-pushed the benchmark_recycler branch from 779fc23 to 454de17 Compare September 21, 2026 07:01

wyxxxcat commented Sep 21, 2026

View reviewed changes

gavinchou changed the title ~~[feat](cloud) Add recycler rowset recycling benchmarks test~~ [opt](cloud) Add recycler rowset recycling benchmarks test Sep 21, 2026

gavinchou previously approved these changes Sep 23, 2026

View reviewed changes

wyxxxcat dismissed gavinchou’s stale review via 4a057f4 September 23, 2026 07:02

test

6174463

wyxxxcat force-pushed the benchmark_recycler branch from 4a057f4 to 6174463 Compare September 23, 2026 08:11

gavinchou approved these changes Sep 28, 2026

View reviewed changes

github-actions Bot previously requested changes Sep 28, 2026

View reviewed changes

gavinchou merged commit 53f055d into apache:master Sep 28, 2026
36 checks passed

gavinchou removed dev/3.1.x dev/4.0.x labels Sep 28, 2026

github-actions Bot mentioned this pull request Sep 28, 2026

branch-4.1: [opt](cloud) Add recycler rowset recycling benchmarks test #68043 #68581

Merged

wyxxxcat mentioned this pull request Sep 29, 2026

[chore](cloud) Remove time-consuming checks and update KV baselines #68622

Merged

yiguolei pushed a commit that referenced this pull request Sep 30, 2026

branch-4.1: [opt](cloud) Add recycler rowset recycling benchmarks test …

fb5560e

…#68043 (#68581) Cherry-picked from #68043 Co-authored-by: Yixuan Wang <[email protected]>

yiguolei added dev/4.1.5-merged and removed dev/4.1.x labels Sep 30, 2026

yiguolei pushed a commit that referenced this pull request Oct 8, 2026

branch-4.1: [opt](cloud) Add recycler rowset recycling benchmarks test …

ac940a4

…#68043 (#68581) Cherry-picked from #68043 Co-authored-by: Yixuan Wang <[email protected]>

Conversation

wyxxxcat commented Sep 16, 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 16, 2026

Uh oh!

wyxxxcat commented Sep 16, 2026

Uh oh!

wyxxxcat commented Sep 16, 2026

Uh oh!

wyxxxcat commented Sep 16, 2026

Uh oh!

hello-stephen commented Sep 16, 2026

Cloud UT Coverage Report

Uh oh!

wyxxxcat commented Sep 18, 2026

Uh oh!

hello-stephen commented Sep 18, 2026

Cloud UT Coverage Report

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!

wyxxxcat commented Sep 21, 2026

Uh oh!

wyxxxcat Sep 21, 2026

Choose a reason for hiding this comment

Uh oh!

wyxxxcat commented Sep 23, 2026

Uh oh!

wyxxxcat commented Sep 23, 2026

Uh oh!

hello-stephen commented Sep 23, 2026

Cloud UT Coverage Report

Uh oh!

wyxxxcat commented Sep 24, 2026

Uh oh!

wyxxxcat commented Sep 28, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Review summary

Uh oh!

github-actions Bot Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

wyxxxcat commented Sep 16, 2026 •

edited

Loading