[bugfix]: workers killed by a signal now log the reason - #1725
Open
KyleNeverGivesUp wants to merge 9 commits into
Open
[bugfix]: workers killed by a signal now log the reason#1725KyleNeverGivesUp wants to merge 9 commits into
KyleNeverGivesUp wants to merge 9 commits into
Conversation
mergify
Bot
added
type: bugfix
Contributor
Merge Protections🔴 1 of 1 protections blocking · waiting on 👀 reviews and 🤖 CI
🔴 PR merge requirementsWaiting for
This rule is failing.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3a05aef34
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
SolitaryThinker
force-pushed
the
report-signal-terminated-workers
branch
2 times, most recently
from
August 23, 2026 22:55
c822207 to
f38bbd4
Compare
`worker_main` installs a SIGTERM/SIGINT handler that raises `SystemExit`. That is a `BaseException` and not an `Exception`, so it walks straight past the `except Exception` below it. The worker exits having logged nothing, the parent sees only a closed pipe, and the user gets: Exception: WorkerMultiprocProc initialization failed due to an exception in a background process. See stack trace for root cause. with no stack trace anywhere. That is indistinguishable from a hang, a crash, an out-of-memory kill or a deliberate shutdown. It is not a rare path. A DGX Spark ships earlyoom configured as `-m 6,3 --prefer python`, so it sends SIGTERM to python first once available memory drops below six percent, which on that box is 7.3 GiB. Every memory-pressure failure there reports nothing at all. The parent's own fallback in `shutdown` also terminates workers that do not exit within five seconds, and that is equally silent today. Add a `SystemExit` clause ahead of the broad one. It logs with the traceback and re-raises, so exit semantics are unchanged and the parent still sees the pipe close. The traceback is the useful part: a signal handler runs on top of whatever the process was executing, so the frames are the frames that were interrupted, which is the only clue to what the worker was doing when it was told to stop. Recovering that by hand is what this change is meant to save, and it took most of a day of instrumenting a loader that turned out not to be the problem. fastvideo/tests/worker/test_signal_terminated_worker_is_reported.py: 3 passed, and reverting this change fails two of them. Driving `worker_main` for real needs a process, a pipe, a distributed init and a model, so the tests pin the structure instead: the handler exists, it is ordered ahead of the broad one, and it re-raises rather than swallowing the exit.
SolitaryThinker
force-pushed
the
report-signal-terminated-workers
branch
from
August 26, 2026 06:10
f38bbd4 to
b1867a1
Compare
Collaborator
|
/merge |
Collaborator
|
/test full |
Collaborator
|
/test full |
SolitaryThinker
force-pushed
the
report-signal-terminated-workers
branch
from
August 26, 2026 11:14
d28aa1b to
6476748
Compare
Contributor
|
This PR has merge conflicts with the base branch. Please rebase: git fetch origin main
git rebase origin/main
# Resolve any conflicts, then:
git push --force-with-lease |
Both sides added a bullet to the GB10 tuning list and git could not tell they were about different things. hao-ai-lab#1715 documents that FastVideo now disables the offload modes once a worker binds its device; this branch documents that earlyoom prefers Python and that a worker's SIGTERM traceback shows where it was interrupted, not why it was chosen. Neither replaces the other, so both are kept, offload first.
Contributor
|
This PR has merge conflicts with the base branch. Please rebase: git fetch origin main
git rebase origin/main
# Resolve any conflicts, then:
git push --force-with-lease |
Resolves the one conflict, in spark_performance.md. hao-ai-lab#1793 appended an H3 sequential-load bullet to the same Running safely list that this branch appends its earlyoom diagnostics bullet to. Both bullets are kept, H3 first. This branch's bullet drops its opening sentence about earlyoom preferring Python because the H3 bullet now states that.
Contributor
|
This PR has merge conflicts with the base branch. Please rebase: git fetch origin main
git rebase origin/main
# Resolve any conflicts, then:
git push --force-with-lease |
Resolves the one conflict, in spark_performance.md. main rewrote the H3 entry in the Running safely list, since lazy_module_load now owns the load split, and added the TAEH3 and two-Spark bullets. Those are taken as is. This branch's earlyoom diagnostics bullet is kept and moved to the end of the list.
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
2 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.
Purpose
Workers that receive SIGTERM or SIGINT currently exit without reporting which
signal arrived or where execution was interrupted. The parent then sees only a
closed pipe and tells the user to consult a stack trace that was never emitted.
On DGX Spark/GB10 systems, a userspace OOM daemon can make this a routine and
otherwise opaque failure mode.
Changes
SystemExitsubtype whileretaining the historical argument-less exit status.
construction, and the busy loop.
shutdown semantics remain unchanged.
root cause: it may come from an external process or parent cleanup after a
different worker failed.
earlyoom/system logs, and note that SIGKILLand kernel OOM kills cannot be caught by Python.
Test coverage
The regression suite exercises both deterministic in-process injection and real
spawned Linux workers using an actual pipe, logging queue, SIGTERM/SIGINT
delivery, traceback forwarding, cleanup, and exit status. It also verifies that
unrelated
SystemExitvalues are not intercepted.Scope
Other exception paths are unchanged. This deliberately handles only signals
installed by
worker_main; it does not add a broadBaseExceptioncatch.Checklist
main