Do not abort in-flight requests when bumping the engine weight version - #2589
Merged
Merged
Conversation
yueming-yuan
requested review from
Shi-Dong,
Zhichenzzz,
fzyzcjy,
maocheng23 and
yushengsu-thu
as code owners
August 17, 2026 21:15
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
Zhichenzzz
approved these changes
Aug 17, 2026
update_weight_version was added to the finalize step so full-param and LoRA updates share one version bump, but the sglang endpoint's UpdateWeightVersionReqInput defaults abort_all_requests=True, so every weight update ended by aborting all running requests -- the very requests an in_place pause had just preserved. Each aborted turn returns a partial message with finish_reason "abort"; the agent extracts no command from it and ends the episode, the sample finishes ABORTED, and check_no_aborted then drops the whole group. Measured on a 16-node GLM-5.2 run (job 2403): ~65 aborts per update = the decoding population, 38% of all samples ABORTED (a July build that never calls this endpoint: 0%), 11-18 groups discarded per step, two thirds of rollout production thrown away, train_wait_time 137-340s per step. The abort_all sweep racing waiter wake-ups is also the source of the rid_to_state KeyErrors seen during updates. Passing abort_all_requests=False restores the July behavior: requests survive the update and finish under the new weights, as in_place intends.
yueming-yuan
force-pushed
the
yueming/update-weight-version-keep-requests
branch
from
August 17, 2026 22:56
a45ab04 to
83267f8
Compare
guapisolo
approved these changes
Aug 17, 2026
yushengsu-thu
added a commit
that referenced
this pull request
Aug 18, 2026
Conflicts/integration: - miles/backends/sglang_utils/sglang_engine.py: update_weight_version — main's #2589 made 'never abort in-flight requests on a version bump' unconditional, subsuming this branch's multi-LoRA-only opt-out (one tenant's publish must not abort another tenant's sampling). Resolved to main's unconditional payload and updated the branch's abort-policy test to expect the metadata-only bump for both deployment shapes. - tests/fast-gpu/test_layerwise_expert_dependencies.py: main's CI label policy now requires a domain label on every register_cuda_ci(); this branch-only registration predated it (labels=[] failed collection post-merge) — labeled 'lora' to match its LayerWise expert-LoRA scope.
yushengsu-thu
added a commit
that referenced
this pull request
Aug 18, 2026
yushengsu-thu
added a commit
to yushengsu-thu/miles
that referenced
this pull request
Aug 18, 2026
… bump, CI label fix)
yushengsu-thu
added a commit
that referenced
this pull request
Aug 22, 2026
The abort_all_requests=False behavior was introduced on main (#2589), not by this stack, so its regression test belongs in a standalone test-only PR against main rather than riding the tinker backend; the file returns to its main-tree content.
yushengsu-thu
added a commit
that referenced
this pull request
Aug 24, 2026
…uted sync LoRA sync sends only adapter tensors and never refills base weights; opening the session anyway makes begin/end_weight_update restore and re-pack the quantized base buffers with nothing loaded in between, corrupting the frozen base (reproduced on Kimi-K2.5 W4A16, TP8). Also re-adds the update_weight_version abort_all_requests=False wire pin so main #2589's no-abort behavior cannot silently regress. Absorbed from closed PRs #2715 and #2713.
fzyzcjy
added a commit
that referenced
this pull request
Aug 24, 2026
4 tasks
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.
Problem
Every weight update ends by aborting all in-flight rollout requests. The chain:
_finalize_and_resume_engines(mixin.py) callsengine.update_weight_version(new_version)— added so full-param and LoRA updates share one version bump. The July-era code passedweight_versioninside theupdate_weights_from_distributedpayload instead and never hit this endpoint.{"new_version": ...}(sglang_engine.py).UpdateWeightVersionReqInputdefaultsabort_all_requests: bool = True, and the/update_weight_versionhandler then runsabort_request(abort_all=True)— one step beforecontinue_generation.Under
pause_generation_mode=in_placethis defeats the pause itself: the scheduler's in-place pause deliberately leaves all running requests intact so they resume decoding under the new weights, and the version bump then destroys them.Downstream, each aborted turn returns a partial message with
finish_reason: "abort". The agent extracts no command from the partial and ends the episode, the sample finishes ABORTED, andcheck_no_aborteddrops the entire group.Measured impact (16-node GLM-5.2 agentic run, job 2403; July build as control)
train_wait_time137–340s per step.abort_allsweep racing_wait_one_responsewake-ups is also the source of therid_to_stateKeyErrors logged during updates (343 in this run).Fix
Pass
abort_all_requests: Falsefrom the wrapper. The pause mode has already decided the in-flight requests' fate by the time the version is bumped; underin_placethey must stay alive. This restores the July behavior, under which a 99-step run trained with turns routinely finishing under mixed weights (the per-turn rollout log-probs record the actual sampling distribution either way, so importance correction is unaffected).A follow-up candidate (not in this PR): the agent loop in
examples/experimental/openenvdoes not inspectfinish_reason, so any residual aborted turn still ends its episode; the session server's retry rollback (prepare_pretokenized) already supports re-issuing such a turn.