Conversation
#50388 recovers a KV load failure on a hybrid model by recomputing the request in full: it rewinds num_computed_tokens to 0 in place and keeps the request's blocks. A running request cannot restart that way. With overlapping batches (async scheduling, pipeline parallelism) the next step is already scheduled, so its sliding-window and Mamba groups have swapped blocks they no longer need for the shared null block, which the recompute then writes and reads back, and the in-flight step's output, computed on the failed load, is delivered as if valid. Preempt such requests instead. Re-admission allocates everything afresh and the stale in-flight output is dropped. The request's blocks leave the prefix cache first: a sync load is cached at allocation, so the retry would otherwise hit the garbage. Single-group models keep the in-place rewind and the async path already frees the blocks before retrying. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Yifan Qiao <yifanqiao@inferact.ai>
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.
Purpose
Follow-up to #50388, which stopped EngineCore from crashing on a KV load failure with a hybrid KV cache by recomputing the affected request in full. For a request in the
WAITING_FOR_REMOTE_KVSstate that works: its blocks are freed once the transfer finishes and it is re-admitted. For a running request (a synchronous load underkv_load_failure_policy="recompute") the recompute is done in place, rewindingnum_computed_tokensto 0 while the request keeps its blocks and stays inrunning. That is not a valid restart:allocate_slotsalready ranremove_skipped_blockson the loaded prefix: the sliding-window and Mamba groups now point at the shared null block for the positions they no longer needed. The recompute from 0 writes those positions' KV into the null block and the attention right after them reads it back.This PR preempts such requests instead of rewinding them in place.
_preempt_requestalready does everything a from-scratch restart needs: it frees the blocks, moves the request to the head of the waiting queue, reports it inpreempted_req_idsso the worker drops its state, and withdrop_stale_output=Truedrops the in-flight output. Before preempting, the request's blocks are evicted from the prefix cache; a sync load is cached at allocation, so the retry would otherwise hit the garbage. Single-group models keep the block-precise in-place rewind, and the async path is unchanged.Retry semantics: the re-admitted request goes through normal admission, so the connector is asked for a match again (as it is today for the request-level
_handle_failed_recvingpath). A connector that keeps offering a load it cannot deliver would retry each admission.Not a duplicate: #56135, #50742, #45497, #48216 and #54733 address the crash that #50388 fixed or work out a precise per-group truncation point; #53298 keeps the in-place rewind and repairs Model Runner V2 state around it for single-group models. None of them changes how a running hybrid request is restarted after #50388.
Test Plan
The new
test_hybrid_sync_load_failure_preempts_requestcovers a running hybrid request whose sync load fails: it is preempted with no output appended, its blocks are released and no longer cached, and the nextschedule()re-admits it from scratch withpreempted_req_idsset.Test Result
207 passed. Pre-commit (ruff, mypy) clean. Not run end to end against a KV connector that injects load failures; the unit test exercises the scheduler path only.
🤖 Developed with Claude Code; all changes reviewed and tested by the author.