Repository navigation
Drain result_queue in LocalExecutor to prevent process join deadlock - #67881
Conversation
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide
|
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Fixes a potential deadlock in LocalExecutor shutdown where workers blocked on writing to a full result_queue would never exit, hanging the scheduler. Also makes terminate() actually terminate worker processes.
Changes:
- Drain
result_queuewhile waiting for workers to join inend(), withKeyboardInterrupt/SystemExitfallback that force-terminates workers. - Harden
_read_resultsagainstOSError/EOFErrorfrom a closed/broken queue. - Implement
terminate()to forcefully terminate all worker processes (previously a no-op).
|
@potiuk Fixed the Ruff pre-commit hook failures on this PR by addressing the following two lint issues:
All local checks now pass successfully and should pass the CI tests. |
33cd755 to
5684360
Compare
o-nikolas
left a comment
There was a problem hiding this comment.
Thanks for the updates, as long as the CI goes green it looks good to me
5684360 to
28fa007
Compare
|
@o-nikolas could you please approve the pending GitHub Actions workflows for the latest push? I fixed the single CI failure by removing the Conventional Commits-style On the previous run, the only failed job was |
Done! |
|
@o-nikolas CI is green now, including the previously failing |
|
Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions. |
…alExecutor Call _read_results() before each activity_queue.put() in _process_workloads so workers blocked on a full result_queue pipe can resume reading from activity_queue, preventing the circular deadlock described in apache#70526. This is the dispatch-side equivalent of the fix applied to end() in apache#67881.
…alExecutor Call _read_results() before each activity_queue.put() in _process_workloads so workers blocked on a full result_queue pipe can resume reading from activity_queue, preventing the circular deadlock described in apache#70526. This is the dispatch-side equivalent of the fix applied to end() in apache#67881.
|
@o-nikolas should this be included and backported in 3.3.2? Also #70545 depends on that too. |
@pierrejeambrun I don't feel too strongly about it personally. But since the call has been made for #70545 then we should pull this one in as well I reckon. |
|
Actually after second consideration I've moved that to 3.4.0 so it has time to sink in before 3.4.0 (and spend some time under testing before being released). But i'm ok if we want this in 3.3.2. |
Yeah, I think that's a good call @pierrejeambrun! |
…e dispatch deadlock Fixes the runtime/dispatch-path deadlock reported in apache#70526: during _process_workloads the scheduler could block in activity_queue.put() while workers were blocked writing results into a full result_queue pipe, a circular wait. Draining the result queue before each put breaks the cycle. The end()/join path is already handled by apache#67881; this covers normal dispatch. Adds a call-order unit test and a bounded real-queue regression test.
…e dispatch deadlock Fixes the runtime/dispatch-path deadlock reported in apache#70526: during _process_workloads the scheduler could block in activity_queue.put() while workers were blocked writing results into a full result_queue pipe, a circular wait. Draining the result queue before each put breaks the cycle. The end()/join path is already handled by apache#67881; this covers normal dispatch. Adds a call-order unit test and a bounded real-queue regression test.
…e dispatch deadlock (#70545) Fixes the runtime/dispatch-path deadlock reported in #70526: during _process_workloads the scheduler could block in activity_queue.put() while workers were blocked writing results into a full result_queue pipe, a circular wait. Draining the result queue before each put breaks the cycle. The end()/join path is already handled by #67881; this covers normal dispatch. Adds a call-order unit test and a bounded real-queue regression test.
…event runtime dispatch deadlock (#70545) (#74949) Fixes the runtime/dispatch-path deadlock reported in #70526: during _process_workloads the scheduler could block in activity_queue.put() while workers were blocked writing results into a full result_queue pipe, a circular wait. Draining the result queue before each put breaks the cycle. The end()/join path is already handled by #67881; this covers normal dispatch. Adds a call-order unit test and a bounded real-queue regression test. (cherry picked from commit b00cfeb) Co-authored-by: Saksham Kapoor <sakshamkapoor2911@gmail.com>
…event runtime dispatch deadlock (#70545) (#74949) Fixes the runtime/dispatch-path deadlock reported in #70526: during _process_workloads the scheduler could block in activity_queue.put() while workers were blocked writing results into a full result_queue pipe, a circular wait. Draining the result queue before each put breaks the cycle. The end()/join path is already handled by #67881; this covers normal dispatch. Adds a call-order unit test and a bounded real-queue regression test. (cherry picked from commit b00cfeb) Co-authored-by: Saksham Kapoor <sakshamkapoor2911@gmail.com>
…e dispatch deadlock (apache#70545) Fixes the runtime/dispatch-path deadlock reported in apache#70526: during _process_workloads the scheduler could block in activity_queue.put() while workers were blocked writing results into a full result_queue pipe, a circular wait. Draining the result queue before each put breaks the cycle. The end()/join path is already handled by apache#67881; this covers normal dispatch. Adds a call-order unit test and a bounded real-queue regression test.
closes: #67870
Description
This PR fixes a deadlock in the
LocalExecutorduring executor shutdown (end()).Currently, the executor calls
proc.join()on active worker processes. However, if a worker has written enough results to theresult_queueto fill the OS-level pipe buffer (typically 64KB), the worker process blocks indefinitely on itsput()call.Because the parent scheduler process is blocked on the unbounded
proc.join()and not reading fromresult_queue, a classic multiprocessing deadlock occurs. The scheduler hangs indefinitely, stopping heartbeats and preventing systemd/Kubernetes from restarting the process (as it never exits).Changes
LocalExecutor.end()to continuously drain theresult_queueusing_read_results()while waiting for worker processes to join with a small timeout._read_results()with atry/except (OSError, EOFError)block to gracefully handle cases where pipes are already broken or closed during task exit.LocalExecutor.terminate()method to forcefully kill any remaining workers when a hard stop is requested.Context
This contribution was created as part of the required capstone of the CodePath AI301 course, where students learn how to make responsible and effective use of generative AI tools for open-source software contributions.
Was generative AI tooling used to co-author this PR?
Generated-by: Antigravity IDE (Gemini) following the guidelines
Note
✅ Ready for review · @SakshamKapoor2911 →
@potiuk· 2026-06-12 13:30 UTCThanks @SakshamKapoor2911 — all checks are green and this PR is marked ready for maintainer review. The ball is with the maintainers now; a maintainer will take the next look.
Automated triage — may be imperfect.