raghav-reglobe opened a new issue, #67031: URL: https://github.com/apache/doris/issues/67031
### Search before asking - [x] I had searched in the [issues](https://github.com/apache/doris/issues?q=is%3Aissue) and found no similar issues. ### Version master (present at current master head, 1f53b285c) and the 4.x lineage. ### What's Wrong? `PipelineTask::set_running` is used as a mutual-exclusion gate in two places: - `TaskScheduler::_do_work` re-queues a task when `set_running(true)` returns true, so two workers never execute the same task concurrently; - `RevokableTask` delegates `set_running` to its underlying task, so an in-flight spill revocation excludes concurrent execution of that task. The implementation is a single `compare_exchange_weak` attempt: ```cpp virtual bool set_running(bool running) { bool old_value = !running; _running.compare_exchange_weak(old_value, running); return old_value; } ``` `compare_exchange_weak` may fail **spuriously** on LL/SC architectures (aarch64). On a spurious failure with `_running == false`, `expected` is reloaded with the current value (`false`), so the function returns "was not running" **without ever setting the flag**. A second worker then acquires the same task and both execute it concurrently. One thread's `reset_hash_table` (which nulls the mapped slots of the live aggregation hash table before swapping it) or `close()` (which empties `_places`) races the other thread's `sink_impl`, which dereferences a just-nulled aggregate state. Observed in production on a 2-BE cluster on AWS Graviton (aarch64), within hours of heavy spill/memory-pressure churn arming the wake storms that hammer this gate (paused-query resume, `unblock_all_dependencies`, and the scheduler push-back loop — which re-queues a woken task while a revocation holds the flag, i.e. a tight retry loop on this exact CAS). Signature: repeated `doris_be` SIGSEGV, si_addr 0x0, fatal frame `AggSinkOperatorX::sink_impl` (`aggregation_sink_operator.cpp:999`) directly under the signal trampoline, including **two workers crashing in the same second attached to the same query** — the double-execute signature. x86 compiles weak CAS as never-spurious, which is why this does not reproduce on x86 CI or deployments; every ARM deployment is exposed. Note: every other `compare_exchange_weak` in `be/src` sits in a retry loop; this is the only no-retry gate. A second, independent defect was found in the same investigation: `QueryTaskController::revoke_memory` sizes its `SpillContext` to the number of chosen tasks, but a `RevokableTask` submit error aborts the loop via `RETURN_IF_ERROR` — the failed and unsubmitted tasks never call `on_task_finished()`, the `SpillContext` never completes, its callback never fires `set_memory_sufficient(true)`, and the query blocks on `_memory_sufficient_dependency` forever (a silent per-query hang). ### What You Expected? `set_running(true)` either acquires the gate and sets the flag, or reports "already running" — never reports acquisition without setting the flag. A failed revocation submit should never permanently strand the paused query. ### How to Reproduce? Hard to reproduce deterministically (needs a spurious LL/SC CAS failure under contention on aarch64). The conditions that made it fire within hours for us: aarch64 BEs + `enable_spill=true` sessions + heavy aggregations under a tight workload-group memory ceiling + concurrent query kills (cancel/unblock wake storms). A unit test asserting the mutual-exclusion invariant of `set_running` (8 threads hammering acquire/release, at most one holder) is included with the fix PR and can catch the old implementation probabilistically on ARM hardware. ### Anything Else? Fix PRs (both apply to master head): one makes `set_running` an unconditional `std::atomic::exchange` (identical intended semantics — return old value, always store the new one — and it cannot fail spuriously); the other accounts the failed + unsubmitted tasks to the `SpillContext` before returning the submit error. Related earlier fix in the same area for a different (in-thread exception-safety) class: #66102. ### Are you willing to submit PR? - [x] Yes I am willing to submit a PR! ### Code of Conduct - [x] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct) -- 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]
