Doris-Breakwater commented on issue #69961: URL: https://github.com/apache/doris/issues/69961#issuecomment-6570226326
Breakwater-GitHub-Analysis-Slot: slot_9687202488e5 The reported callback-pairing defect is supported by source inspection at `5af2df323190986392ef221f82a37053efc30d20`. This is an FE transaction/Routine Load correctness issue in the shared-nothing path. The issue currently has no labels. This analysis is read-only; no runtime reproduction or FE tests were executed. **Verified code path** - [`RoutineLoadJob.beforeAborted()` → `executeBeforeCheck()`](https://github.com/apache/doris/blob/5af2df323190986392ef221f82a37053efc30d20/fe/fe-core/src/main/java/org/apache/doris/load/routineload/RoutineLoadJob.java#L1101-L1159) acquires the job write lock and retains it on successful return, including when the task is no longer present. [`afterAborted()`](https://github.com/apache/doris/blob/5af2df323190986392ef221f82a37053efc30d20/fe/fe-core/src/main/java/org/apache/doris/load/routineload/RoutineLoadJob.java#L1284-L1378) releases that lock in its own `finally`, even when `txnOperated == false`. - Both [`abortTransaction()` and `abortTransaction2PC()`](https://github.com/apache/doris/blob/5af2df323190986392ef221f82a37053efc30d20/fe/fe-core/src/main/java/org/apache/doris/transaction/DatabaseTransactionMgr.java#L2026-L2149) call the before callback, enter `synchronized (transactionState)`, perform the abort and persistence/enqueue, then invoke the after callback **without an enclosing `finally`**. An exception/error escaping that block skips the callback and leaves the acquired job lock held. - A late abort of a `COMMITTED` transaction is a concrete rejection path: `unprotectAbortTransaction()` throws before changing its status. `COMMITTED` is [not a final status](https://github.com/apache/doris/blob/5af2df323190986392ef221f82a37053efc30d20/fe/fe-core/src/main/java/org/apache/doris/transaction/TransactionStatus.java#L57-L59), so it can still be found in the ordinary abort path's running-transaction map. The rejection must leave it `COMMITTED` and propagate the original failure. - On the persistence path, `unprotectAbortTransaction()` changes the in-memory status to `ABORTED` and returns `true` **before** persistence/enqueue. Thus `txnOperated` is already true if that later call throws. In outside-lock mode, `awaitTransactionState()` is already after the callback; an await failure is not the same skipped-callback window. The journal scenario needs a precise failure mode. [`EditLog.submitEdit()`](https://github.com/apache/doris/blob/5af2df323190986392ef221f82a37053efc30d20/fe/fe-core/src/main/java/org/apache/doris/persist/EditLog.java#L1642-L1684) and [`logEdit()`](https://github.com/apache/doris/blob/5af2df323190986392ef221f82a37053efc30d20/fe/fe-core/src/main/java/org/apache/doris/persist/EditLog.java#L1709-L1727) can throw an `Error` when no stream exists, whereas direct journal-write failures call `System.exit(-1)`. A `finally` addresses stack-unwinding failures; it cannot guarantee cleanup on process termination or prove recovery/durability after a failed write. **Recommended fix boundaries** 1. In both abort entry points, pair each successfully completed before callback with exactly one after callback in an outer `finally`, after leaving the transaction monitor. Do not invoke after when before itself failed. The [commit path](https://github.com/apache/doris/blob/5af2df323190986392ef221f82a37053efc30d20/fe/fe-core/src/main/java/org/apache/doris/transaction/DatabaseTransactionMgr.java#L933-L963) already provides a local example of this structure. 2. Preserve the original abort/persistence failure if cleanup also fails. Keep `txnOperated` tied to whether the in-memory abort occurred, including when persistence subsequently fails. Do not convert persistence failure into success or broaden backend-clear operations to rejected aborts. 3. Audit callback consumers alongside the manager change. In particular, [`DeleteJob.afterAborted()`](https://github.com/apache/doris/blob/5af2df323190986392ef221f82a37053efc30d20/fe/fe-core/src/main/java/org/apache/doris/load/DeleteJob.java#L232-L253) currently removes its callback unconditionally. Calling it with `txnOperated == false` after a rejected abort could remove the callback still needed by `afterVisible()` to finish and record the delete. Gate that removal on an actual abort while preserving successful-abort cleanup. **Validation and missing evidence** Add focused FE tests covering both abort entry points and both values of `enable_txn_log_outside_lock`: a late `COMMITTED` abort, injected persistence/enqueue failures after the state transition, successful abort, before-callback failure, and a secondary cleanup failure. Assert callback count, `txnOperated`, unchanged `COMMITTED` status on rejection, preservation of the original failure, and `!Thread.holdsLock(transactionState)` inside cleanup. Use a real Routine Load job lock and verify another thread can acquire it with a bounded timeout; same-thread reentry would miss a leaked reentrant lock. Add a delete regression asserting callback retention after rejected abort, completion on visibility, and callback removal after successful abort. To connect the source defect to an observed incident, please supply the minimal reproducer or fault-injection test, the abort caller/timing relative to commit, full FE abort/persistence exception stack, and `enable_txn_log_outside_lock` / `enable_batch_editlog` settings. For a live hang, include FE thread dumps with lock ownership and correlated job/transaction IDs. The exact revision is already provided; these additional details are needed to establish the observed trigger and impact, not to identify the missing cleanup structure. -- 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]
