fix(aorta): bound setup and run by timeout_seconds on daemon threads - #330
Merged
speriaswamy-amd merged 2 commits intoSep 11, 2026
Merged
Conversation
This was referenced Aug 14, 2026
speriaswamy-amd
force-pushed
the
surya/aorta-mn-05-timeouts
branch
from
September 8, 2026 15:18
5c811b9 to
525c285
Compare
speriaswamy-amd
force-pushed
the
surya/aorta-mn-05-timeouts
branch
from
September 8, 2026 15:29
525c285 to
2b1c1ff
Compare
amd-droy
approved these changes
Sep 9, 2026
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
force-pushed
the
surya/aorta-mn-05-timeouts
branch
from
September 10, 2026 23:13
2b1c1ff to
9faf30e
Compare
amd-droy
approved these changes
Sep 10, 2026
speriaswamy-amd
removed this pull request from stack #333
September 11, 2026 00:23
speriaswamy-amd
added this pull request to stack #414
September 11, 2026 00:23
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
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.
Stack 5/6 — splits #171. Base: #390.
Why
Neither
setup()norrun()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 reportingTIMEOUT.ThreadPoolExecutorcannot fix this on its own — its workers are non-daemon, and CPython's atexit hook joins every one of them at interpreter shutdown regardless ofshutdown(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. ReplacesThreadPoolExecutorin bothsetup()andrun()._setup_single_node()takes a cancelEvent: if it finishes launching its container aftersetup()has given up, it tears that container down itself. Otherwise it would register intoself._containersafterteardown()'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
ruffclean. Unit tests 623 → 631 (8 new, including a case asserting the worker thread is actually a daemon, and end-to-endsetup()/run()timeout cases that assert the call returns promptly).