fix(sc): keep the watchdog armed while the train pump drains - #3783
Open
tianyi-zhang-02 wants to merge 6 commits into
Open
tianyi-zhang-02 wants to merge 6 commits into
tianyi-zhang-02 wants to merge 6 commits into
Conversation
svcnvidia-nemo-ci
added
the
waiting-on-maintainers
tianyi-zhang-02
force-pushed
the
fix/sc-run-supervision
branch
2 times, most recently
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>
tianyi-zhang-02
force-pushed
the
fix/sc-run-supervision
branch
from
August 28, 2026 15:29
a7ea16c to
caa36ad
Compare
tianyi-zhang-02
force-pushed
the
fix/sc-run-supervision
branch
3 times, most recently
from
September 13, 2026 02:52
4ac740b to
ed72a3d
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>
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>
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>
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
September 15, 2026 02:24
ed72a3d to
bcb99a0
Compare
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 Single Controller train pump, rollout pump, stall watchdog, generation-fleet probe, and rollout-checkpoint telemetry task supervised until training exits.
Previously
run()waited only once withFIRST_COMPLETED. Normal rollout exhaustion could therefore leave the train pump draining after watchdog and fleet-probe failures stopped being observed. The supervision loop keeps pending tasks armed, preserves failure priority, and handles requested checkpoint shutdown separately from unexpected pump exit.Tests cover watchdog failure before and after rollout exhaustion, clean completion, rollout failure during a wedged train pump, fleet-probe failure during drain, and expected and unexpected checkpoint-pump completion.
Validation
Current head
bcb99a02eb97a3a8b5a52a124651dc854a871e1a, rebased on upstreammainatfeff70f5b6239d90dc703ae79b973ea809550732.