Fix DagProcessorJob crash on orphan processor kill - #69523
Open
hkc-8010 wants to merge 13 commits into
Open
Conversation
…phan kill After SIGKILL, stale socket registrations on the shared selector cause the next select() callback to write to an already-closed log handle. Add _deregister_processor_sockets() to clean up sockets before closing the handle, and guard target.log() in the supervisor against ValueError. Closes: apache#64959
Re-add test_deregister_processor_sockets_suppresses_already_unregistered and test_deregister_processor_sockets_suppresses_close_error which directly exercise the KeyError and OSError suppression paths in _deregister_processor_sockets.
`_deregister_processor_sockets` uses `contextlib.suppress` in three places but the module never imported `contextlib`, which `ruff` (F821) flags as an undefined name. Caught while rebasing PR apache#65002 onto current main; the PR's own CI run predates this code path.
hkc-8010
requested review from
amoghrajesh,
ashb,
ephraimbuddy,
jedcunningham and
kaxil
as code owners
July 7, 2026 06:16
potiuk
added
the
ready for maintainer review
eladkal
added
type:bug-fix
Contributor
|
LGTM! can we merged post review from code owners |
vatsrahul1001
approved these changes
Jul 29, 2026
kaxil
reviewed
Jul 29, 2026
jason810496
reviewed
Aug 4, 2026
| from cadwyn import VersionChange | ||
|
|
||
|
|
||
| class TrackDagProcessorSocketCleanupOwnership(VersionChange): |
Member
There was a problem hiding this comment.
Why do we need the VersionChange here? I don't see any downstream consumer for this.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Prevent
DagProcessorJobfrom crashing withValueError: write to closed filewhen orphaned or timed-out dag processor subprocesses are killed.
This change drains and deregisters processor log sockets before closing the
logger handle in
DagFileProcessorManager, and narrows supervisor-sideValueErrorhandling so only the closed-log-file case is skipped.It also adds regression coverage for:
closes: #64959
Was generative AI tooling used to co-author this PR?
Generated-by: OpenAI Codex (GPT-5) following the guidelines