Repository navigation
Conversation
… copy Fixes vllm-project#42489 When a resumable streaming request receives a same-request streaming_update, vLLM rebuilds the live prompt from: - already-computed output tokens folded back into the prompt - the new chunk's prompt_token_ids This renewed prompt can exceed max_model_len even though the original request was within bounds. When it did, gpu_input_batch.add_request() would crash with an opaque NumPy broadcast error because the fixed-width token_ids_cpu buffer is shaped (max_num_reqs, max_model_len). Fix: - Add a length guard at the top of _update_request_as_session() that returns True (rejected) when renewed_prompt_len >= max_model_len. The method now returns bool so callers can act on the result. - Both call sites (_handle_stopped_request and add_request) finish the request with FINISHED_LENGTH_CAPPED on rejection, matching existing overlong-prompt semantics (FinishReason.LENGTH). - Add a defensive bounds check in GPUInputBatch.add_request() before the NumPy write to give a clear error message if another path regresses. Testing: - test_streaming_update_overflow_rejected_by_update_request_as_session - test_streaming_update_overflow_boundary - test_streaming_update_overflow_handle_stopped_request - test_streaming_update_overflow_add_request_waiting_for_streaming All four are CPU-only scheduler-unit tests with max_model_len=8/10. Signed-off-by: KrxGu <krishom70@gmail.com>
There was a problem hiding this comment.
Code Review
This pull request implements a guard against max_model_len overflow for resumable streaming updates. It modifies the scheduler to check if a renewed prompt length will exceed the maximum allowed length before applying updates, ensuring requests are finished with a FINISHED_LENGTH_CAPPED status when necessary. Additionally, defensive checks were added to the GPU input batch processing to catch potential regressions, and comprehensive unit tests were included to verify the new logic. I have no feedback to provide.
|
This PR is ready for review. |
|
This pull request has merge conflicts that must be resolved before it can be |
|
Thanks for putting this up. This is exactly where we ended up too (RFC #51948). The one thing we'd ask to be softer is |
sawsa307
left a comment
There was a problem hiding this comment.
Thank you @KrxGu for the PR! A few things before this can land:
- Rebase: the PR conflicts with current main, and the tests reference
skipped_waiting, which #58947 removed (nowkv_holding_waiting/deferred_waiting). The tests would also fit better intests/v1/streaming_input/test_scheduler_streaming.pyalongside the sibling coverage there. - V2 runner: the defensive bounds check in
gpu_input_batch.add_requestcovers MRV1 only. MRV2 has the same fixed-width[max_num_reqs, max_model_len]buffer with an unguarded write invllm/v1/worker/gpu/states.py(all_token_ids.stage_writeinadd_request). The scheduler-side guard covers V2, so this is belt-and-braces — but either add the V2 counterpart or note it in the PR body. - Encoder-token growth: the prompt-length check misses pure encoder-token accumulation (the audio/realtime case from #39996 that #40072 covered). Could you fold the encoder-token accounting into the same pre-mutation check and add ianliuy as co-author?
A streaming-input session merged the next chunk into its prompt without checking the accumulated length against max_model_len. Once the merged session exceeded the limit, the scheduler clamped the request's remaining prefill to zero tokens and wedged it in RUNNING with no further outputs; on older V2-runner builds the same unguarded merge produced negative num_scheduled_tokens and crashed EngineCore (vllm-project#62707, vllm-project#42489). Reject the update at merge time instead. The dropped chunk's reply ends with an empty, length-capped turn so the frontend's one-boundary-per-chunk bookkeeping stays in sync, the session parks in WAITING_FOR_STREAMING_REQ so a shorter update can continue it, and updates queued behind a dropped one are drained and applied when they fit — the softer outcome requested in the review of vllm-project#42502. Fixes vllm-project#62707 Co-authored-by: opencode <noreply@opencode.ai> Signed-off-by: potoior <2986486901@qq.com>
Fixes #42489
When a resumable request receives a streaming_update, the scheduler
rebuilds the prompt by folding in already-computed output tokens plus
the new chunk. That renewed prompt can exceed max_model_len even though
the original request was fine, and when it did, gpu_input_batch.add_request()
would crash with an opaque NumPy broadcast error:
The fix adds a length check at the start of _update_request_as_session()
before any state is mutated. If renewed_prompt_len >= max_model_len, the
method returns early and the caller finishes the request with
FINISHED_LENGTH_CAPPED (same as any other length-exceeded request).
Both call sites are covered: _handle_stopped_request() and add_request().
A defensive bounds check is also added in GPUInputBatch.add_request() so
any future regression produces a clear error instead of a NumPy traceback.
Testing: 4 CPU-only scheduler unit tests covering the overflow path,
exact boundary, and both call sites. No model or GPU required.