fix(sc): keep the watchdog armed while the train pump drains - #3783
Open
tianyi-zhang-02 wants to merge 5 commits into
Open
fix(sc): keep the watchdog armed while the train pump drains#3783tianyi-zhang-02 wants to merge 5 commits into
tianyi-zhang-02 wants to merge 5 commits into
Conversation
svcnvidia-nemo-ci
added
the
waiting-on-maintainers
tianyi-zhang-02
force-pushed
the
fix/sc-run-supervision
branch
from
August 26, 2026 20:29
868d493 to
f3d0b8e
Compare
run() waits once with FIRST_COMPLETED, handles whichever task finished, then falls through to a bare `await train_task`. From that point the watchdog and the fleet probe are still running but nothing awaits them again, and the `finally` retrieves their exceptions with gather(return_exceptions=True) and discards them -- so not even asyncio's "Task exception was never retrieved" warning fires. The window this opens is the normal path, not an edge case. The rollout pump finishing first is end-of-data, as run()'s own comment says: "A normally exhausted rollout pump leaves the train pump to drain committed groups." For the whole of that drain, a RolloutStall under stall_action: abort, a GenerationFleetExhausted, or an env-health abort lands on an unobserved task while run() parks on the wedged train pump. The job then holds its GPUs until the scheduler's wall clock kills it, with nothing in the driver log -- which is what WatchdogConfig, documented as "Last-resort detection for stalls that no other layer catches", exists to prevent. Loop the wait instead, so every task stays supervised until the train pump exits. The priority order (probe, watchdog, rollout, train) is unchanged, and so is the behaviour when the train pump finishes first. Three tests. The drain-phase one fails on main with a TimeoutError rather than the RolloutStall it asserts; the other two pin the paths that already worked. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Tianyi Zhang <123608656+tianyi-zhang-02@users.noreply.github.com>
tianyi-zhang-02
force-pushed
the
fix/sc-run-supervision
branch
from
August 27, 2026 02:14
f3d0b8e to
a7ea16c
Compare
tianyi-zhang-02
added a commit
to tianyi-zhang-02/RL
that referenced
this pull request
Aug 27, 2026
…ge reads Nine controller stubs in this file predate that attribute, so they pass here and fail once merged with current main -- NVIDIA-NeMo#3768 made _advantage_stage read it unconditionally. Same shape as the _rollout_manager stub gap in NVIDIA-NeMo#3783. Signed-off-by: Tianyi Zhang <zhangtianyi975@gmail.com> Signed-off-by: Tianyi Zhang <123608656+tianyi-zhang-02@users.noreply.github.com>
The three tests here all take the watchdog branch. Two others were unreached:
- rollout-pump failure. Every test has the rollout pump either exhaust
cleanly or never finish, so the branch whose comment promises immediate
propagation was never exercised. It is the branch that matters most under
the old single wait: a failed rollout task sits in pending, unawaited,
while run() parks on the train pump.
- the fleet probe. _bare_actor sets _gen_fleet = None, so run() creates no
probe task at all and the "probe_task in done" branch was dead in every
test.
Both use a distinct exception type so a test cannot pass by catching the
wrong task's failure.
Mutation-tested: collapsing the loop, and skipping either await, each turn
one of these red.
Signed-off-by: Tianyi Zhang <123608656+tianyi-zhang-02@users.noreply.github.com>
…pumps The branch predated that call, so the test passed here and failed once merged with current main -- mergeable, but red afterwards. Only showed up when I merged all twenty of my open PRs together and bisected back. Signed-off-by: Tianyi Zhang <zhangtianyi975@gmail.com> Signed-off-by: Tianyi Zhang <123608656+tianyi-zhang-02@users.noreply.github.com>
tianyi-zhang-02
force-pushed
the
fix/sc-run-supervision
branch
from
August 28, 2026 15:29
a7ea16c to
caa36ad
Compare
Signed-off-by: Tianyi Zhang <123608656+tianyi-zhang-02@users.noreply.github.com>
Signed-off-by: Tianyi Zhang <123608656+tianyi-zhang-02@users.noreply.github.com>
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.
What does this PR do?
Keeps the SingleController train pump, rollout pump, stall watchdog, and generation-fleet probe supervised until the train pump exits.
Previously
run()waited only once withFIRST_COMPLETED. A normally exhausted rollout pump then left the train pump draining while watchdog and fleet-probe failures were no longer observed; cleanup retrieved and discarded those exceptions. The new loop preserves the existing priority order—fleet probe, watchdog, rollout, then train—while continuing to watch every pending task.The tests cover watchdog failure both before and after rollout exhaustion, clean completion, rollout failure during a wedged train pump, and fleet-probe failure during the drain.
Validation
Final SHA:
7d740e95526d94e650c651168f521360310ea526, merged with currentmain(ccbcd4cc5).Environment: Runpod Secure Cloud,
nvcr.io/nvidia/nemo-rl:v0.7.0, Python3.13.14, PyTorch2.11.0+cu130; tests ran CPU-only withCUDA_VISIBLE_DEVICES=''.