fix(engine): resume preempted sequences by recompute - #77
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes recompute-based resume for preempted sequences by explicitly tracking which sequences are resumed at scheduling time, and then using that information in the engine to (a) feed the correct tokens during resume-prefill and (b) slice flattened logits correctly in mixed fresh+resumed batches without corrupting metrics.
Changes:
- Scheduler: recomputes
num_tokenson preemption and threadsresumed_sequence_idsthroughScheduledBatchfor PREFILL batches. - Engine: uses
resumed_sequence_idsto feedpromptvsprompt+generated, and to slice prefill logits by per-sequence fed length while avoiding metric clobbering on resume. - Tests: adds regression coverage for token feeding, mixed-batch logit slicing, start/metrics preservation, and an end-to-end forced-preemption equivalence test.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/executor/test_scheduler.py | Adds regression tests for preemption num_tokens recompute and for resumed_sequence_ids being carried in scheduled PREFILL batches. |
| tests/executor/test_schedule_engine.py | Adds unit + end-to-end tests for resume-by-recompute behavior (token feeding, logits slicing, metrics preservation, forced-preemption output equivalence). |
| server/executor/types.py | Extends ScheduledBatch to include resumed_sequence_ids (default empty). |
| server/executor/scheduler.py | Marks resumed sequences during PREFILL scheduling and recomputes num_tokens at preemption time. |
| server/executor/engine.py | Uses scheduler-provided resumed_sequence_ids to control resume-prefill inputs, per-seq logits slicing, and to avoid resetting metrics on resume. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Fixes recompute-based resume for preempted sequences, which was previously broken end-to-end: every prefill raised
ValueError, and (once that's fixed) resumed sequences would get corrupted logits and clobbered metrics.ScheduledBatchnow carriesresumed_sequence_ids, populated inschedule()at the moment a waiting sequence's prior state (PREEMPTED) is still known — before it gets overwritten toRUNNING. Also fixes_preemptto recomputenum_tokensasprompt + generatedlength, since resume re-feeds both._prepare_prefill: usesresumed_sequence_ids(notseq.state, which the scheduler already flips toRUNNINGbefore the engine sees it) to decide whether to feed only prompt tokens (fresh) or prompt + generated tokens (resumed recompute)._post_prefill: slices the flattened logits per sequence by its actual fed length (Pfresh,P+Gresumed) instead of alwaysnum_prompt_tokens, fixing misalignment in batches mixing fresh and resumed sequences. Also skipson_prefill_started/num_prompt_tokensreassignment on resume, since resume is a second prefill for an already-admitted request and would otherwise clobberstart_ns(corruptingqueue_wait_ms/ttft_ms/total_ms).