Skip to content

branch-4.2: [fix](cloud) Add the aggregation output rowset to the delete bitmap replacement test #69349 - #69357

Merged
yiguolei merged 2 commits into
branch-4.2from
auto-pick-69349-branch-4.2
Oct 24, 2026
Merged

yiguolei merged 2 commits into
branch-4.2from
auto-pick-69349-branch-4.2

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

Cherry-picked from #69349

…eplacement test (#69349)

### What problem does this PR solve?

Issue Number: None

Related PR: #68084, #68100

Problem Summary:

**In short**


`MetaServiceTest.GetDeleteBitmapReturnsCompleteReplacementAfterTxnTooOld`
has failed on master since #68084 and #68100 were merged two minutes
apart on 2026-10-20, so the Cloud UT pipeline is red for every PR. The
test, added by #68100, injects a compaction aggregation without the
output rowset that #68084 started to require, so the meta service skips
the aggregation as obsolete. This PR adds that rowset to the test, the
way #68084 did for the other aggregation tests. It changes one line of a
test and no meta service code.

**Background**

- A compaction that aggregates the delete bitmaps of versions [start,
end] sends `update_delete_bitmap` with `lock_id = -3`, `without_lock`,
and `pre_rowset_agg_start_version` / `pre_rowset_agg_end_version`.
- #68084 makes the meta service check, in every transaction, that the
output rowset of such an aggregation still exists: the rowset meta at
the end version must exist and cover [start, end]. Otherwise the request
is treated as obsolete and returns OK without writing anything. #68084
added that output rowset (`put_delete_bitmap_test_rowset(..., 2, "r1-2",
1)`) to the existing tests that send such requests.
- #68100 added
`GetDeleteBitmapReturnsCompleteReplacementAfterTxnTooOld`. While
`get_delete_bitmap` reads a bitmap across several transactions, a sync
point injects an aggregation of [1, 2] and makes the read restart with
`TXN_TOO_OLD`; the test checks that the restarted read returns the
complete replacement. It was written against a master without #68084.

**The problem, and what it cost**

- With #68084, the injected aggregation has no output rowset at version
2, so `check_pre_rowset_aggregation` skips it. The restarted read
returns the old three-chunk bitmap, the byte threshold stops it before
`following_rowset`, and the first iteration fails:

```
cloud/test/meta_service_test.cpp:8501: Failure
Expected equality of these values:
  get_res.rowset_ids_size()
    Which is: 3
  expected_bitmap_count
    Which is: 4
Google Test trace:
cloud/test/meta_service_test.cpp:8382: replacement_size=1
```

- The ASSERT ends the test, so the second iteration (`replacement_size =
3 * DEFAULT_BLOB_SPLIT_SIZE`) never runs. `meta_service_test` exits
non-zero, and since 2026-10-20 11:16 +08:00 the Cloud UT pipeline has
failed for every PR, whatever the PR changes.
- branch-4.2 already has the pick of #68100 (#69307). Once the pick of
#68084 (#69306) is merged, branch-4.2 fails the same way.

**How this PR fixes it**

Add the output rowset of the injected aggregation to the test, as #68084
did for the other aggregation tests:

```cpp
put_delete_bitmap_test_rowset(meta_service.get(), instance_id, tablet_id, 2, "r1-2", 1);
```

With it, the check passes, the replacement is written, and the test
exercises the restarted read it was written for. Only that check reads
the new rowset meta; `get_delete_bitmap` does not read rowset metas, so
nothing else in the test changes.

**Results**

`run-cloud-ut.sh --run` in a Linux arm64 container (Debian 12, ldb
toolchain v0.25, the linux-aarch64 prebuilt third-party tree, a local
FDB 7.1.57):

| Run, on master `33c460d7365` |
`GetDeleteBitmapReturnsCompleteReplacementAfterTxnTooOld` | Other tests
|
|---|---|---|
| without this PR (the added line removed) | fails as on the pipeline: 3
rowset ids instead of 4, under `replacement_size=1` | not run |
| with this PR | passes | all 33 test binaries pass except
`StopWatchTest.SimpleTest` |

The same two runs on master `1d03523339f`, before the rebase, gave the
same results.

**Classes, and how they call each other**

```
MetaServiceTest.GetDeleteBitmapReturnsCompleteReplacementAfterTxnTooOld
 ├─ put_delete_bitmap_test_rowset(version 1, rowset_id)
 ├─ put_delete_bitmap_test_rowset(version 2, "r1-2", start 1)       <- added
 └─ MetaServiceImpl::get_delete_bitmap
      └─ sync point "get_delete_bitmap_err" (in the middle of the read)
           ├─ MetaServiceImpl::update_delete_bitmap(lock_id -3, aggregation [1, 2])
           │    └─ check_pre_rowset_aggregation: the rowset meta at version 2 must cover [1, 2]
           └─ TXN_TOO_OLD -> get_delete_bitmap restarts the read
```

Nothing outside `cloud/test/meta_service_test.cpp` changes.

**Merge order**

No dependency on master. On branch-4.2 this should go in together with
#69306 (the pick of #68084), or right after it.
@github-actions
github-actions Bot requested a review from yiguolei as a code owner October 21, 2026 00:21
@hello-stephen

Copy link
Copy Markdown
Contributor

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:

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

@hello-stephen

Copy link
Copy Markdown
Contributor

run buildall

Add an optional start_version argument to put_delete_bitmap_test_rowset and populate both rowset version bounds. The backported replacement test passes six arguments to a helper that still accepted five, which prevents Cloud UT from compiling.

Keep the existing five-argument callers compatible by defaulting the start version to the rowset end version. Match the helper already present on branch-4.2 so the aggregation output covers versions [1, 2].

Validation: clang-format 16 and git diff --check pass; the helper matches current branch-4.2. A local translation-unit syntax check is blocked by incompatible pre-generated Protobuf files and installed headers.
@hello-stephen

Copy link
Copy Markdown
Contributor

Cloud UT Coverage Report

Increment line coverage 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 78.03% (1992/2553)
Line Coverage 65.49% (35832/54714)
Region Coverage 53.26% (34423/64627)
Branch Coverage 55.92% (10693/19121)

@liaoxin01

Copy link
Copy Markdown
Contributor

run buildall

@hello-stephen

Copy link
Copy Markdown
Contributor

Cloud UT Coverage Report

Increment line coverage 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 78.03% (1992/2553)
Line Coverage 65.48% (35828/54714)
Region Coverage 53.17% (34364/64627)
Branch Coverage 55.94% (10696/19121)

@github-actions

Copy link
Copy Markdown
Contributor Author

PR approved by anyone and no changes requested.

@github-actions github-actions Bot added the approved Indicates a PR has been approved by one committer. label Oct 23, 2026
@github-actions

Copy link
Copy Markdown
Contributor Author

PR approved by at least one committer and no changes requested.

@yiguolei

Copy link
Copy Markdown
Contributor

skip buildall

@yiguolei
yiguolei merged commit 1afd1d0 into branch-4.2 Oct 24, 2026
31 of 33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants