Doris-Breakwater commented on issue #67120:
URL: https://github.com/apache/doris/issues/67120#issuecomment-5408102659
Breakwater-GitHub-Analysis-Slot: slot_4df113a889fe
## Initial triage
**Verdict: confirmed by static code inspection.** This is a high-priority
correctness issue in the Shared-Nothing CCR `IngestBinlog` path for UNIQUE KEY
merge-on-write tablets. A runtime reproduction was not provided or executed,
but the control flow is sufficient to confirm that an asynchronous
delete-bitmap failure can be lost and an incomplete bitmap can be attached to a
committed rowset.
The issue currently has no labels. Please classify it under the repository's
bug/CCR/MOW conventions and treat it as a data-correctness fix. The same
discarded-status and cleanup-capture patterns are present in the locally
configured `branch-3.1` (available 3.1.4-rc02 snapshot), `branch-4.0`,
`branch-4.1`, and `master` refs, so maintained-branch backports should be
considered after the primary fix.
## Verified code path
1. In `_ingest_binlog()` (`be/src/service/backend_service.cpp`), both
`BaseTablet::commit_phase_update_delete_bitmap(...)` and
`calc_delete_bitmap_token->wait()` have their returned `Status` explicitly
discarded. Execution then reaches `TxnManager::commit_txn()` and, for MOW,
`set_txn_related_delete_bitmap()`.
2. `CalcDeleteBitmapToken` records a worker failure in `_status`; `wait()`
is the operation that returns that failure to the caller. The worker log
therefore does not stop `IngestBinlog` when the wait result is ignored.
3. `BaseTablet::commit_phase_update_delete_bitmap()` snapshots the current
historical rowset IDs into `pre_rowset_ids` after task submission, before
asynchronous completion is known. If a worker later fails, publish receives an
incomplete bitmap together with a snapshot saying those historical rowsets were
already processed.
4. Publish-time `BaseTablet::update_delete_bitmap()` calculates only the
difference between the then-current rowset IDs and `txn_info->rowset_ids`. It
does not revisit historical rowsets already present in the commit-phase
snapshot. The multi-segment fallback only repairs bitmap calculation between
segments of the new rowset. This confirms the reported window in which missing
historical-key deletions can survive publication.
5. A synchronous submission failure is also incorrectly masked. Because
`pre_rowset_ids` is assigned only after successful submission, publish may
recalculate more work in that particular case, but relying on that fallback
does not provide the required transaction atomicity or error reporting.
6. The ordinary load path does propagate these errors:
`BaseRowsetBuilder::submit_calc_delete_bitmap_task()` returns the commit-phase
status, and `wait_calc_delete_bitmap()` returns the token's wait status.
7. The deferred error handler is created while `download_success_files` is
empty and captures it through `[=]`. Its cleanup loop therefore sees the
initial empty copy rather than files appended later. This independently
confirms the cleanup gap.
## Recommended fix and tests
- Check and propagate the status from `commit_phase_update_delete_bitmap()`
immediately. Then check and propagate `calc_delete_bitmap_token->wait()`. On
either error, set `tstatus` and return before `commit_txn()`; the existing
deferred handler will then abort the PREPARED tablet transaction.
- Capture `download_success_files` by reference in the deferred handler so
the failure path deletes the files accumulated during this ingest. Also log a
`batch_delete()` failure instead of reporting cleanup as completed
unconditionally.
- Extend
`regression-test/suites/ccr_mow_syncer_p0/test_ingest_binlog.groovy` with a
target MOW table that already contains the same key. Use the existing
`BaseTablet::calc_segment_delete_bitmap.inject_err` debug point on the target
BE to force an asynchronous failure, and verify:
- ingest reports failure (or the asynchronous query state becomes
`FAILED`);
- the target transaction is not committed/published and the failed version
is not visible;
- downloaded segment/index files from the failed attempt are removed;
- after disabling the debug point, retrying the same binlog succeeds and
the target exposes exactly one value for the unique key.
- Add separate coverage for task-submission rejection if practical, because
the existing debug point exercises worker execution failure rather than
`submit()` failure.
- Backport the fix to affected maintained branches after validation.
## Missing incident evidence
The code defect can be fixed without additional incident data. To confirm
whether a specific deployment already hit it, please provide the exact BE build
commit, target table DDL, target tablet/transaction/binlog version identifiers,
the target BE log span containing `failed to calc segment delete bitmap` and
the final `ingest binlog elapsed ... result`, CCR/FE commit-and-publish logs
for the same transaction, and a minimal before/after query showing the
duplicate key. Memory-arbitration logs around the failure would confirm the
proposed low-water-mark trigger; that trigger is plausible but is not
established by the current issue data.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
