Supervise every parallel batch: bound a collective hang and name the rank that caused it - #678
Conversation
…rank that caused it A rank blocked inside a collective produces no output and never returns, so an unsupervised batch spends the whole job budget in silence and is cancelled from outside with no diagnosis. Measured at 76 minutes on one run (#675), which is also how test_0855 and test_0873 managed to fail without leaving a traceback. Every mechanism we already had asks the stuck process to cooperate, which is the one thing it has stopped being able to do. A threading.Timer needs the GIL and fired zero times on blocked ranks. faulthandler.dump_traceback_later is a C thread walking other threads' live frames and can wedge or crash the process it is meant to diagnose (#661). MPI_Abort from a watchdog thread is not safe from inside a collective. mpirun --timeout is spelled differently by Open MPI and MPICH and tells you nothing when it fires. scripts/mpi_supervisor.py asks nothing of the job. It is a parent process holding a clock and a signal, so it behaves the same under either MPI and on either platform: it is POSIX process management and knows nothing about MPI. It triggers on SILENCE rather than elapsed time. A batch that is still printing is alive, and judging on total runtime would mean inventing a wall-clock budget for a matrix nobody has measured -- wrong the first time someone adds a slow test. The #675 failure was 76 minutes of nothing, not of slow progress. On silence it arms each rank through a usercustomize module that registers a SIGUSR1 handler (a signal handler runs on the blocked thread itself and never walks live frames concurrently, so it cannot reproduce #661), samples twice, and reports three independent pieces of evidence: whether anything moved between samples (stuck vs slow), which rank is somewhere the others are not, and CPU per rank. It then kills descendants individually before the group -- mpirun puts its children in a group of their own and killpg has been seen to refuse with EPERM on macOS -- and re-checks for survivors, because an unsupervised kill orphans mpirun and pytest children at 100% CPU (#639). Validated by a planted hang whose answer is known: rank 0 sits in a function no other rank can be inside while the rest wait in a barrier. The supervisor must end the job, call it stuck, and name rank 0. The control also asserts the other direction, that a healthy job passes through with its own exit status -- without it, every other assertion could be met by a supervisor that kills everything. At np=2 there is no majority, and the report says so rather than nominating a rank on a one-all split. Two findings worth recording from writing it: the spinning ranks are the INNOCENT ones (an MPI barrier busy-waits) while the guilty rank sleeps, and an earlier version that guessed the rank-to-CPU mapping reported that backwards -- the handler now records its own pid so the attribution is read, not inferred. tests/test_0063_mpi_hang_supervisor.py: 3 passed in 109s. Underworld development team with AI support from Claude Code Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E87Q7KrpapxeQiLD1RiNXv
There was a problem hiding this comment.
🟡 Changes recommended
The supervisor currently signals all descendant processes with SIGUSR1 (including non-Python MPI helper processes) and the test runner wrapper has unsafe shell word-splitting, both of which can break the intended diagnosis/robustness.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR introduces an MPI “silence supervisor” wrapper to prevent parallel test batches from hanging indefinitely when a rank blocks inside a collective, and adds documentation + a control test to ensure the mechanism both catches a planted hang and leaves healthy jobs untouched.
Changes:
- Add
scripts/mpi_supervisor.pyto runmpirun ...under a parent process that monitors output silence, requests per-rank stack dumps viaSIGUSR1, diagnoses divergence, and kills the job tree if needed. - Route existing parallel batches in
scripts/test.shthrough the supervisor (withPARALLEL_SILENCEoverride). - Add an end-to-end pytest control (
tests/test_0063_mpi_hang_supervisor.py) and a planted hang payload (tests/parallel/hang_controls/...) plus developer docs describing usage and rationale.
File summaries
| File | Description |
|---|---|
tests/test_0063_mpi_hang_supervisor.py |
Adds end-to-end tests that validate the supervisor kills a known hang, reports correctly, and does not interfere with healthy runs. |
tests/parallel/hang_controls/rank_zero_misses_the_collective.py |
Adds a planted “rank 0 misses collective” hang payload used by the supervisor tests. |
scripts/test.sh |
Wraps parallel pytest batches with the new supervisor to bound silent hangs in CI. |
scripts/mpi_supervisor.py |
Implements the supervisor: output-silence monitoring, stack-dump requests, diagnosis heuristics, and job-tree kill/verification. |
docs/developer/index.md |
Adds the new guide to the developer docs toctree. |
docs/developer/guides/mpi-hang-supervision.md |
Documents the motivation, usage, reporting semantics, and implementation details of the supervisor. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| handler_dir = Path(tempfile.mkdtemp(prefix="uw-supervisor-")) | ||
| (handler_dir / "usercustomize.py").write_text(_dump_handler_module(dump_dir)) | ||
|
|
||
| existing = env.get("PYTHONPATH") | ||
| env["PYTHONPATH"] = f"{handler_dir}{os.pathsep}{existing}" if existing else str(handler_dir) | ||
| return handler_dir |
| def _request_dumps(pids): | ||
| """Ask every rank for a stack. Ranks that are gone are simply not asked.""" | ||
| for pid in pids: | ||
| try: | ||
| os.kill(pid, signal.SIGUSR1) | ||
| except ProcessLookupError: | ||
| # The rank exited between listing and signalling. Nothing to dump. | ||
| pass | ||
|
|
||
|
|
||
| def _dump_blocks(text): | ||
| """The individual dumps in one rank's file. | ||
|
|
||
| ``faulthandler`` appends, so a file accumulates every sample we have asked | ||
| for. Splitting them apart is what lets one read answer both "where is this | ||
| rank" and "has it moved since last time". | ||
| """ | ||
| blocks = re.split(r"^Current thread ", text, flags=re.MULTILINE) | ||
| return [block for block in blocks if block.strip()] |
| SUPERVISE="python $(dirname "$0")/mpi_supervisor.py --silence ${PARALLEL_SILENCE:-300} --" | ||
|
|
||
| echo "Testing global statistics and parallel operations..." | ||
| mpirun -n $PARALLEL_RANKS python -m pytest --with-mpi tests/parallel/test_075*py || status=1 | ||
| $SUPERVISE mpirun -n $PARALLEL_RANKS python -m pytest --with-mpi tests/parallel/test_075*py || status=1 |
| It lives outside the ``test_*.py`` pattern deliberately: pytest must never | ||
| collect it, because running it under any harness that does not kill it is the | ||
| exact failure this whole exercise exists to prevent. It is launched by name, | ||
| only by ``tests/test_0056_mpi_hang_supervisor.py``. |
|
First real use, and it found the bug it was built for: #675 is root-caused in Rank 0 inside a collective HDF5 read, rank 1 waiting at a barrier it never One real limitation, worth a follow-up rather than a change now. Killing on Two ways to close it, neither in this PR:
Either belongs with #615, which rewrites these loops and decides how the batches |
…rvisor #678 wrapped the two globs this PR replaces, so the merge had to choose. It keeps both: the enumeration and batching from here, and every launch -- np=2 and the opt-in np=4 pass alike -- going through scripts/mpi_supervisor.py. The supervisor matters more on this side of the change than it did on the globs. Covering the directory by enumeration means files that had never run in CI now do, and #675 was exactly that: two of them failed on their first run and one took the job down with a rank-divergent collective, 76 minutes of silence with no diagnosis. Both of those are fixed, but a new file added to the directory is precisely where the next unbounded hang comes from. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E87Q7KrpapxeQiLD1RiNXv
Run singleton fresh-process phases directly when the target branch predates the MPI supervisor merged in development by underworldcode#678. Keep MPI restart validation conditional on that supervisor so parallel descendants remain bounded and diagnosable. This lets the SUPG feature branch validate PC2, CN, and BDF2 restart state without importing the unrelated 691-line supervisor change into this review. Underworld development team with AI support from Claude Code.
Summary
A rank blocked inside a collective produces no output and never returns. An
unsupervised batch therefore spends the whole job budget in silence and is
cancelled from outside with no diagnosis — measured at 76 minutes on one run
(#675), which is also why
test_0855andtest_0873managed to fail therewithout leaving a traceback behind.
scripts/mpi_supervisor.pybounds that and says what went wrong.Why not the mechanisms we already had
Every one of them asks the stuck process to cooperate, which is the one thing it
has stopped being able to do.
threading.Timer+ reportcomm.allreduce: fired zero times on the blocked ranks.faulthandler.dump_traceback_laterMPI_Abortfrom a watchdog threadTHREAD_MULTIPLE, not safe from inside a collective.mpirun --timeout--timeoutvs MPICH-timeout, different semantics, and it tells you nothing when it fires.The supervisor asks nothing of the job: a parent process holding a clock and a
signal. Identical behaviour under either MPI and on either platform, because it
is POSIX process management and knows nothing about MPI.
It triggers on silence, not elapsed time
A batch that is still printing is alive. Judging on total runtime means
inventing a wall-clock budget for a matrix nobody has measured, and that number
is wrong the first time someone adds a slow test. The #675 failure was 76
minutes of nothing, not of slow progress.
What it reports
Three independent pieces of evidence: whether anything moved between two samples
(stuck vs slow), which rank is somewhere the others are not, and CPU per rank.
Ranks are armed through a
usercustomizemodule that registers aSIGUSR1handler. That is a signal handler, not a concurrent walker thread — it runs on
the blocked thread itself, only when asked, so arming it costs a healthy run
nothing and it cannot reproduce #661.
The control
tests/test_0063_mpi_hang_supervisor.pyplants a hang whose answer is known:rank 0 sits in a function no other rank can be inside while the others wait in a
barrier. The supervisor must end the job, call it stuck, and name rank 0.
It also asserts the other direction — a healthy job passes through untouched
with its own exit status. Without that, every other assertion could be satisfied
by a supervisor that kills everything it is given.
3 passed in 109.62s. A harness that catches hangs is worth nothing until ithas caught one.
Two things worth flagging
The spinning ranks are the innocent ones. An MPI barrier busy-waits, so the
ranks burning 100% CPU are the ones waiting and the guilty rank is the one
asleep. An earlier version of this guessed the rank-to-CPU mapping by sorting
and reported it exactly backwards; the handler now records its own pid so the
attribution is read rather than inferred. A confident wrong label is worse than
no label.
At np=2 there is no majority. The report says so and declines to nominate a
rank on a one-all split, rather than manufacturing a culprit.
Scope
scripts/test.shroutes its two existing parallel batches through it(
PARALLEL_SILENCEoverrides the budget). This does not touch #615, whichrewrites those loops — that PR can rebase onto this and inherit the protection.
Does not fix #675 or #661; it makes #675 diagnosable and it stops CI arming the
mechanism behind #661.
Underworld development team with AI support from Claude Code
🤖 Generated with Claude Code
https://claude.ai/code/session_01E87Q7KrpapxeQiLD1RiNXv