Repository navigation
Conversation
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: When a merge-on-write unique-key load generates the delete bitmap asynchronously, segment compaction can compact and remove the original segment files before delete bitmap generation builds the temporary rowset. In that case the BE fails to reopen the removed segment file with NOT_FOUND, marks the tablet write as failed, cancels the node channel, and the stream load fails. This change adds targeted fault-injection points and a regression case that deterministically reproduces the race by pausing delete bitmap generation while segment compaction removes the original segment.
### Release note
None
### Check List (For Author)
- Test:
- Build: BUILD_TYPE=Debug ./build.sh --be
- Regression test: ./run-regression-test.sh --run -d fault_injection_p0 -s test_mow_load_delete_bitmap_segcompaction_race
- Behavior changed: No
- Does this need documentation: No
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: Fix the C++ format issue reported by CI for beta_rowset_writer.cpp in the MOW delete bitmap segment compaction race regression PR.
### Release note
None
### Check List (For Author)
- Test:
- Manual test: git diff --check
- Manual test: python3 build-support/run_clang_format.py --clang-format-executable /data/data3/huangruixin/include/src-master/ldb_toolchain/bin/clang-format --style file --inplace false be/src/storage/rowset/beta_rowset_writer.cpp
- Behavior changed: No
- Does this need documentation: No
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: In the MOW load path, delete bitmap calculation was made async and could overlap with segcompaction. When segcompaction deleted original segment files before delete bitmap finished reading them, stream load failed with NOT_FOUND on the segment file. This change records per-segment delete bitmap completion and stops segcompaction from consuming unfinished segments. The regression case is updated to expect the load to succeed and to verify the loaded data still matches.
### Release note
None
### Check List (For Author)
- Test: Regression test
- ./run-regression-test.sh --run -d fault_injection_p0 -s test_mow_load_delete_bitmap_segcompaction_race -forceGenOut
- ./run-regression-test.sh --run -d fault_injection_p0 -s test_mow_load_delete_bitmap_segcompaction_race
- Test: Build
- BUILD_TYPE=Debug ./build.sh --be
- Test: Manual test
- git diff --check
- CLANG_TIDY_BINARY=/data/data3/huangruixin/include/src-master/ldb_toolchain/bin/clang-tidy build-support/run-clang-tidy.sh --build-dir be/build_Debug (failed locally because clang-tidy could not analyze existing headers in this environment)
- Behavior changed: No
- Does this need documentation: No
…ap-segcompaction-race
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: This is a follow-up to the MoW async delete bitmap and segment compaction race fix. The core gate already prevents segment compaction from selecting a segment before its async delete bitmap work is complete. This change documents the non-MoW bypass, the intentional unmarked state on async failure, and the conservative mark-after-publish ordering. It also adds debug logging when segcompaction is stopped by the gate, renames the fault-injection point to match the actual pause point, and reduces regression-test flakiness by shortening the injected delay while increasing the stream load timeout.
### Release note
None
### Check List (For Author)
- Test:
- Regression test: ./run-regression-test.sh --run -d fault_injection_p0 -s test_mow_load_delete_bitmap_segcompaction_race -c 'jdbc:mysql://127.0.0.1:9035/?useLocalSessionState=true&allowLoadLocalInfile=true&zeroDateTimeBehavior=round' -ha 127.0.0.1:8035 -runMode not_cloud
- Build: BUILD_TYPE=Debug ./build.sh --be
- Static check: git diff --check
- Not run: build-support/check-format.sh and build-support/run-clang-tidy.sh were attempted, but the local machine does not have clang-format or clang-tidy installed.
- Behavior changed: No
- Does this need documentation: No
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated review completed after 2 convergence rounds. I found one blocking storage correctness issue in the MoW segcompaction/delete-bitmap race fix.
Critical checkpoint conclusions:
- Goal/test coverage: the PR addresses the delete-bitmap-vs-segcompaction race, but the current code can still miss all segcompaction opportunities while async bitmap work is pending; the added regression does not cover that build-time all-attempts-skipped case.
- Data correctness / MoW delete bitmap: marking segments only after bitmap publication is the right direction, but the new readiness gate needs a final retry or callback before the rowset segment-count check.
- Concurrency/lifecycle: the new readiness bitmap mutex, token cancellation, and segment-id namespace were reviewed; no separate lifecycle or conversion issue was found.
- Compatibility/config/observability: no new config or persisted format compatibility issue was found.
Validation: static review only, as this review environment is instructed not to build or run tests.
| int32_t segid; | ||
| for (segid = _segcompacted_point; | ||
| segid < last_segment && segments->size() < config::segcompaction_batch_size; segid++) { | ||
| if (!_is_segment_delete_bitmap_calculated(segid)) { |
There was a problem hiding this comment.
The gate prevents converting bitmap entries before the async task publishes them, but it also means every segcompaction trigger can be skipped while the first segment's bitmap task is still running. add_segment() calls _segcompaction_if_necessary() immediately after submitting the async work, this loop now breaks at the first unmarked _segcompacted_point, and build() later only waits for _calc_delete_bitmap_token before checking the segment limit; it never retries segcompaction after all marks are ready. A load that produces enough small MoW segments while the first bitmap task lags can therefore still reach TOO_MANY_SEGMENTS with _segcompacted_point == 0 and _num_segcompacted == 0. Please add a final retry after the bitmap wait, or otherwise trigger segcompaction when delayed marks become available, before the build-time segment-count check.
Merged
4 tasks
zclllyybb
added a commit
to apache/doris
that referenced
this pull request
Jul 12, 2026
…ability (#65178) The Codex GitHub Actions review pipeline needed stronger guarantees that agents review the intended PR snapshot, use one authoritative diff, pursue high-risk cross-module mechanisms, and expose the work of spawned subagents in Litefuse. This PR: - verifies the checked-out PR head and rechecks the live base/head after context preparation; - prepares an authoritative aggregate PR patch and changed-file list, with atomic bounded retry (`2s`, `4s`, `8s`) for transient GitHub API failures; - directs the main agent to perform an initial risk scan, dispatch focused subagents for suspicious mechanisms, maintain a shared append-only ledger, validate/deduplicate every candidate, and converge for at most three rounds; - requires paths outside the authoritative diff/changed-file context to be confirmed with `rg --files` before reading; - records separate Litefuse traces for each Codex subagent session, including command/tool I/O and non-empty encrypted-reasoning status, instead of mixing subagent activity into the main trace; - makes Litefuse ingestion more resilient to transport timeouts while keeping telemetry upload non-blocking for the review outcome. The repository review skill is kept focused on review invariants; orchestration and completion policy live in the workflow prompt, where they are available to the GitHub Actions invocation. ### Validation and experiment material - Static checks: `python3 -m py_compile .github/scripts/emit_litefuse_otel_io.py` and `git diff --check`. - Litefuse uploader validation: real/synthetic subagent session conversion, payload-limit coverage, and readback of independent subagent traces. - End-to-end replayed review experiments against fresh fork PRs built from the original PR base/head commits: | Replay PR | GitHub Actions run | Result | | --- | --- | --- | | [#136](zclllyybb#136) | [29112823062](https://github.com/zclllyybb/doris/actions/runs/29112823062) | Success; 2 inline comments | | [#137](zclllyybb#137) | [29114260631](https://github.com/zclllyybb/doris/actions/runs/29114260631) | Success; 3 inline comments | | [#138](zclllyybb#138) | [29115946034](https://github.com/zclllyybb/doris/actions/runs/29115946034) | Success; 1 inline comment | | [#139](zclllyybb#139) | [29150372758](https://github.com/zclllyybb/doris/actions/runs/29150372758) | Success; 1 inline comment | | [#140](zclllyybb#140) | [29151057520](https://github.com/zclllyybb/doris/actions/runs/29151057520) | Success; 1 inline comment | The latest replay trace, `86be226391a40e5992bb8d3b4e125eca`, confirms one authoritative `pr.diff` read, two changed-file-list reads, no `gh pr diff` or `git diff` calls from the reviewing agent, and separate subagent sessions. It also exposed one remaining prompt-compliance gap: a guessed adjacent `.cpp` path was read without a preceding `rg --files` lookup; this is documented test evidence rather than hidden by the validation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fresh replay PR for validating the current aggregate-diff review workflow. The base and head reproduce the referenced Apache Doris PR snapshot.