Doris-Breakwater commented on issue #67048: URL: https://github.com/apache/doris/issues/67048#issuecomment-5389971455
Breakwater-GitHub-Analysis-Slot: slot_59bef4d5be4d This content is generated by AI for reference only. Initial assessment: **confirmed BE storage/transaction bug**. The issue currently has no labels, assignee, milestone, or linked PR. I inspected the reported revision `4a11d0b74b74d6b1b2ec92b9706fbf16618b75ce`; the affected commit itself changes only FE broker-load code, so it is the tested master revision rather than the introducing change. The same problematic BE code is still present at the current public `master` head I checked (`61330491eac9083db5ea53db823bad33fad7be48`). Verified root cause: 1. `PendingRowsetGuard` move construction/assignment nulls the source's `_pending_rowset_set`. Its move-assignment `CHECK` permits an initialized destination only when source and destination protect the same rowset IDs in the same set. 2. The first `TxnManager::commit_txn()` stores `std::move(guard)` in the new `TabletTxnInfo`, leaving the caller's guard moved-from. 3. A retry with the same transaction/load/tablet/rowset reaches the same-rowset duplicate branch in `be/src/storage/txn/txn_manager.cpp`. 4. That branch unconditionally executes `load_info->pending_rs_guard = std::move(guard)`. The destination is initialized while the retried source is uninitialized, so the `CHECK` in `PendingRowsetGuard::operator=()` deterministically aborts the BE. This is not only a property of the direct unit-test API. `RowsetBuilder::commit_txn()` always moves its `_pending_rs_guard`, and `LoadStreamWriter::close()` can call that method again on the same builder without a committed fast path. The exact production retry route would still be useful for integration coverage, but it is not required to establish the transaction-manager defect. The existing `CommitTxnTwiceWithSameRowsetId` test does not cover this case: it creates a fresh second guard. That behavior must remain supported. `PendingRowsetSet` is a plain set rather than a reference-counted set, so simply deleting the duplicate-path assignment would let a fresh incoming guard's destructor remove the rowset ID even though `TabletTxnInfo` still holds its original guard. Recommended fix and validation: - Make the duplicate-commit branch distinguish an initialized incoming guard from a moved-from one. For example, expose a narrow `PendingRowsetGuard::is_initialized()` query and only replace the stored guard when the incoming guard is initialized. A moved-from retry should leave the stored initialized guard untouched and return `OK`. - Preserve the existing invariant check for an initialized but mismatched guard; do not broadly weaken the move-assignment invariant unless its semantics are deliberately redesigned. - Add the reported moved-from retry test and assert both `Status::OK()` and that `pending_local_rowsets().contains(rowset_id)` remains true after the retry. - Keep the existing fresh-second-guard duplicate test as a regression test, including its pending-rowset assertion. These two tests protect the two distinct ownership cases. - If the production trigger is the load-stream close/retry path, add a focused close replay test at that layer as well so the RPC-level idempotency contract is covered. No additional information is needed to accept and fix the unit-level bug. To scope the production trigger and choose the correct higher-level regression test, the useful missing evidence is the BE fatal stack/`CHECK` line, load type, RPC or stream operation being retried, and the retry sequence around the lost/timed-out response. A Doris profile is not relevant to this failure. Suggested triage: add the repository's bug and BE storage/transaction labels (if available) and prioritize this above ordinary correctness issues because a legitimate idempotent retry can terminate the BE. The author has offered to submit a PR; the focused guard-state fix plus the two unit cases above should be a suitable next step. -- 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]
