Skip to content

fix(aorta): bound setup and run by timeout_seconds on daemon threads - #330

Merged
speriaswamy-amd merged 2 commits into
surya/aorta-mn-04-torchrunfrom
surya/aorta-mn-05-timeouts
Sep 11, 2026
Merged

speriaswamy-amd merged 2 commits into
surya/aorta-mn-04-torchrunfrom
surya/aorta-mn-05-timeouts

Conversation

@speriaswamy-amd

@speriaswamy-amd speriaswamy-amd commented Aug 14, 2026 •

Copy link
Copy Markdown
Collaborator

Stack 5/6 — splits #171. Base: #390.

Why

Neither setup() nor run() put a deadline on their parallel per-node work, so a single stalled node hung the entire CVS invocation with no result. Hit live: an NCCL collective stalled on one node of a 2-node run and the process sat there indefinitely instead of reporting TIMEOUT.

ThreadPoolExecutor cannot fix this on its own — its workers are non-daemon, and CPython's atexit hook joins every one of them at interpreter shutdown regardless of shutdown(wait=False). A node stuck in a blocking Docker or SSH call would still wedge the process on the way out.

What changed

  • _run_bounded_parallel() — runs each task on a daemon thread against a shared deadline, returning per-key results / errors / timeouts. Abandoned threads cannot block exit. Replaces ThreadPoolExecutor in both setup() and run().
  • _setup_single_node() takes a cancel Event: if it finishes launching its container after setup() has given up, it tears that container down itself. Otherwise it would register into self._containers after teardown()'s one-time snapshot had already run, orphaning the container on the node. (Builds on fix(runner): tear down resources after a partially-successful setup #326.)

Test

ruff clean. Unit tests 623 → 631 (8 new, including a case asserting the worker thread is actually a daemon, and end-to-end setup()/run() timeout cases that assert the call returns promptly).

@amd-droy amd-droy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good. thanks.

speriaswamy-amd and others added 2 commits September 10, 2026 19:09
Neither setup() nor run() put a deadline on their parallel per-node work, so a
single stalled node hung the entire CVS invocation with no result. This was hit
live: an NCCL collective stalled on one node of a 2-node run and the process sat
there indefinitely instead of reporting TIMEOUT.

ThreadPoolExecutor cannot fix this on its own. Its workers are non-daemon and
CPython's atexit hook joins every one of them at interpreter shutdown regardless
of shutdown(wait=False), so a node stuck in a blocking Docker or SSH call would
still wedge the process on the way out. _run_bounded_parallel() runs each task on
a daemon thread against a shared deadline and reports per-key results, errors, and
timeouts; abandoned threads cannot block exit.

Also closes a setup/teardown race that the new deadline makes reachable:
_setup_single_node() takes a cancel Event and, if it finishes launching its
container after setup() has given up, tears that container down itself. Otherwise
it would register into self._containers after teardown()'s one-time snapshot had
already run, orphaning the container on the node.

Co-Authored-By: Claude <noreply@anthropic.com>
_setup_single_node() checked cancel_event and registered the launched
container into self._containers as two separate, unsynchronized steps.
teardown() could take its unsynchronized snapshot of that dict in the
gap between them, orphaning the container, or crash outright with
"dictionary changed size during iteration" if a registration landed
mid-iteration.

Make the check-and-register atomic under self._lock, and have
teardown() snapshot-and-clear under the same lock while setting a
_teardown_started flag a straggling setup thread can observe.
@speriaswamy-amd
speriaswamy-amd force-pushed the surya/aorta-mn-05-timeouts branch from 2b1c1ff to 9faf30e Compare September 10, 2026 23:13
@speriaswamy-amd
speriaswamy-amd removed this pull request from stack #333 September 11, 2026 00:23
@speriaswamy-amd
speriaswamy-amd added this pull request to stack #414 September 11, 2026 00:23
@speriaswamy-amd
speriaswamy-amd merged commit 519fb88 into main Sep 11, 2026
2 checks passed
@cijohnson
cijohnson deleted the surya/aorta-mn-05-timeouts branch September 15, 2026 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants