fix(aorta): bound setup and run by timeout_seconds on daemon threads - #330
Open
speriaswamy-amd wants to merge 1 commit into
Open
fix(aorta): bound setup and run by timeout_seconds on daemon threads#330speriaswamy-amd wants to merge 1 commit into
speriaswamy-amd wants to merge 1 commit into
Conversation
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>
This was referenced Aug 14, 2026
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: #329.
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).