morningman opened a new pull request, #70178: URL: https://github.com/apache/doris/pull/70178
### Background In shared-nothing mode, `DatabaseTransactionMgr` changes a transaction's status under the transaction's own monitor and notifies the job that owns the transaction through its `TxnStateChangeCallback`: `beforeAborted` runs before the change and `afterAborted` after it, and likewise for commit. A routine load job takes its write lock in `beforeAborted` and releases it in `afterAborted`, so every `beforeAborted` that returns needs exactly one `afterAborted`, even when the abort fails. #59990 replaced the database-level write lock with per-transaction monitors, and #68514 later restored this pairing on the commit path. ### What problem does this PR solve? Issue Number: close #69961 Related PR: #59990 (introduced the bug), #68514 (restored the same pairing on the commit path) PR Type: Bugfix Problem Summary: Since #59990, both abort entry points call `afterAborted` only when the abort and its edit-log write succeed. If the abort is rejected because the transaction is already COMMITTED, or the edit-log write throws, the routine load job's write lock stays held by the aborting thread until FE restarts: SHOW ROUTINE LOAD and PAUSE, RESUME or STOP on the job hang, the routine load scheduler stops for every job, and the thread that publishes the committed transaction blocks while holding the table's write lock. This PR calls `afterAborted` in a `finally` outside the transaction monitor, as the commit path does, and keeps the original failure; a delete job now keeps its callback when no abort happened. Most of the diff is unit tests. ### Details - **Frequency:** Not observed; found by code review (#69961). Triggers when an abort reaches a routine load transaction that FE already committed (BE's rollback after a timed-out `loadTxnCommit` RPC, or a stream load 2PC abort naming that `txn_id`, which any user with the LOAD privilege on the table can send), or when the abort's edit-log write throws. - **Root cause:** #59990 moved the abort in `DatabaseTransactionMgr.abortTransaction` and `abortTransaction2PC` under `synchronized (transactionState)` and dropped the `finally` that called `afterStateTransform`. `unprotectAbortTransaction` throws for a COMMITTED or VISIBLE transaction, so `RoutineLoadJob.afterAborted` never releases the lock that its `beforeAborted` took. - **Why existing tests missed it:** Unit: the abort cases in `DatabaseTransactionMgrTest` use transactions without a callback, and its callback-pairing cases cover only commit; `GlobalTransactionMgrTest` drives `KafkaRoutineLoadJob` only through commit, and `RoutineLoadJobTest` calls `afterAborted` directly. Regression: no suite aborts a COMMITTED transaction or fails the edit-log write; the routine load suites abort only PREPARE transactions, which succeed. - **Fix:** Call `afterStateTransform` from a `finally` outside the monitor, with the `catch` that #68514 uses on the commit path to log a failure of the callback itself, so the abort's own exception is the one the caller sees. Since a rejected abort now reaches `afterAborted` with `txnOperated` false, `DeleteJob` returns early then, as `LoadJob` does, keeping the callback that the still-COMMITTED transaction needs in `afterVisible`. - **Other paths and branches:** The commit path was fixed by #68514; cloud mode already calls `handleAfterAbort` from a `finally`; the visible path takes no lock in a before callback. Only master has #59990: `branch-4.2`, `branch-4.1`, `branch-4.0` and `branch-3.1` still pair the callbacks in a `finally`, so no pick is needed. - **Behavior change:** If a job's `afterAborted` throws after a successful abort, the abort now completes and the error is logged, as on the commit and visible paths; before, the request failed with that error and skipped the backend cleanup. `RoutineLoadJob.afterAborted` throws only when pausing the job fails. ### Testing - `DatabaseTransactionMgrTest`, 17 new cases over both entry points and both values of `enable_txn_log_outside_lock`, with a real `KafkaRoutineLoadJob` as the callback for a rejected abort, an edit-log failure and a successful abort, a mock callback whose `beforeAborted` or `afterAborted` throws, and one `DeleteJob` case. They check that the job lock is free, that `afterAborted` ran once outside the monitor, that the original exception propagates, and that another thread can still publish the committed transaction. - On the code before the fix, 10 of them fail: the publish thread times out waiting for the job lock, the lock stays held after an edit-log failure, and `afterAborted` is never called. Reverting only the `DeleteJob` change, dropping the `catch`, or running the callback under the monitor each fails at least one case. - `run-fe-ut.sh --run` over `DatabaseTransactionMgrTest`, `GlobalTransactionMgrTest`, `TransactionStateTest`, `DeleteJobTest` and `RoutineLoadJobTest`: 93 tests, 0 failures. All runs were local, before the rebase onto current master, which added two commits that do not touch this code. ### Notes for Reviewers - Unlike the commit path, `txnOperated` stays true when the edit-log write fails: the transaction is already ABORTED in memory, so the job must treat its task as aborted. - On a rejected abort, `RoutineLoadJob.afterAborted` still updates the `runningTxns` statistic and `OtherMsg`, as on the release branches. ### Release note None ### Check List (For Author) - Behavior changed: Yes (see Details) - Does this need documentation: No - Pre-submission status: Ready ### AI Disclosure - Tool: Claude Code (Claude Opus 5.5) - Generated: code, tests, and this description - Human review: not stated - Prompts: Fix #69961 with the develop-with-evidence skill. -- 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]
