feat(single-controller): pause generation during refit - #3929
feat(single-controller): pause generation during refit#3929youngeunkwon0405 wants to merge 8 commits into
Conversation
|
/ok to test e16a857 |
|
Real-GPU validation completed at exact PR head I ran the full-size native SingleController vLLM GRPO recipe (
The only traceback in each log was a post-success W&B service |
e16a857 to
daa045d
Compare
|
/ok to test daa045d |
|
Addressed the The failure was PR-caused but test-only: Validation:
|
Signed-off-by: Youngeun Kwon <youngeunk@nvidia.com>
daa045d to
15074e4
Compare
|
Addressed the remaining The failure was PR-caused but test-only: Validation:
|
|
/ok to test 15074e4 |
youngeunkwon0405
left a comment
There was a problem hiding this comment.
Team review: 4 agents (rl-expert, test-agent, bug-finder, devil's advocate) analyzed this PR; every finding below was independently re-verified against source, including the pinned vLLM 0.25.1 wheel.
Verified clean (no action): clear_cache-at-pause genuinely subsumes post-refit invalidation (vLLM keep-mode pause_scheduler resets caches at pause); _refit_leader_workers() targets exactly the set the old run_rank_0_only_axes dispatch selected (incl. pp>1) and matches the existing refit dispatches; interface default hooks keep unsupported backends safe; fail-stop resume semantics mirror trajectory_collector; removing the drain-gate TODO is consistent (docs + issue #2625 still track it). Ruff 0.9.9 check+format clean. The A/B W&B evidence (preserve vs recompute, 11 pause/refit/resume cycles each) is solid.
Findings: 1 hardening suggestion, 1 docs note, 2 optional test suggestions (both verified passing locally on CPU) — all inline.
Generated by Claude Code
…roller-refit-pause Signed-off-by: Youngeun Kwon <youngeunk@nvidia.com>
Signed-off-by: Youngeun Kwon <youngeunk@nvidia.com>
|
/ok to test 1f68285 |
|
Post-review validation for exact head 1f68285:
Claude review: all four threads were addressed in 1f68285, replied to, and resolved: bounded lifecycle calls, timeout-budget documentation, recovery ordering coverage, and pause-failure coverage. Exact-head CI was retriggered; branch-up-to-date, lint, docs, and script-coverage checks are passing, with container builds still running. |
Signed-off-by: Youngeun Kwon <youngeunk@nvidia.com>
|
/ok to test 463f247 |
|
Simplified the post-review implementation at exact head
Validation at this exact head:
The only post-success traceback was the same W&B service |
|
/ok to test 463f247 |
Signed-off-by: Youngeun Kwon <youngeunk@nvidia.com>
|
/ok to test df75b8c |
Signed-off-by: Youngeun Kwon <youngeunk@nvidia.com>
bb99591 to
c6a082b
Compare
|
/ok to test c6a082b |
|
/ok to test 40ca90a |
…roller-refit-pause Signed-off-by: Youngeun Kwon <youngeunk@nvidia.com> # Conflicts: # tests/unit/single_controller/test_checkpointing.py
|
/ok to test ae0ca27 |
|
/ok to test ae0ca27 |
Summary
Implementing the same feature as #3839 for the single controller path.
recompute_kv_cache_after_weight_updatesinto native pause and retain cache invalidation as the unsupported-backend fallbackImplementation
SingleControllerActor._sync_weights()continues to close the rollout-dispatch gate and cancel sampler-stale native requests first. After refit membership reconciliation and optional FP8 KV-scale calibration, it asks the generation backend to pause throughpause_generation_for_refit(clear_cache=...), runs the existingWeightSynchronizerrefit (including its one-shot recovery retry), and callsresume_generation_after_refit()before reopening dispatch.vLLM preserves active request state with native keep-mode pause. Backends that return unsupported keep their existing in-flight refit behavior, receive the shared warning, and retain the existing post-refit cache-invalidation path when recompute is enabled.
The lifecycle is in the shared SingleController
_sync_weights()path, so it applies to GRPO and PPO and to native and NeMo-Gym rollouts.Validation
git diff --checkpassed17015948innemo_rl.0825.sqsh: 92 focused tests passedtest_single_controller_actor.pytest_refit_recovery.py2134in the same image/account ran the fulltest_checkpointing.py: 62 passedgen_handle=None, CW-DFW Slurm job17510464in the same image/account ran the fulltest_train_pump_e2e.pyunder--hf-gated --mcore-onlywith a forced worker-environment rebuild: 1 passed, 66 warnings in 123.19s; SlurmCOMPLETED/0:0in 6:27e16a857503c0843df20d3b58f737be6bcde6bf76(current head differs only by two test-fixture compatibility fixes found by L0 CI; production source is unchanged)grpo-llama3.1-8b-instruct-2n8g-async-1off-single-controller-streaming2.yamlworkload: 2 nodes / 16 H100 GPUs, 64 prompts x 32 generations, global batch size 2048, max sequence length 409617016712,COMPLETED/0:0, 10 optimizer steps and 11 successful pause/refit/resume cycles; W&B17016711,COMPLETED/0:0, 10 optimizer steps and 11 successful pause/refit/resume cycles; W&B