[Bugfix][Core] Time out requests stuck in WAITING_FOR_STREAMING_REQ - #53321
[Bugfix][Core] Time out requests stuck in WAITING_FOR_STREAMING_REQ#53321dkling-it wants to merge 3 commits into
Conversation
A resumable request that finishes generating with no queued next chunk enters WAITING_FOR_STREAMING_REQ and can only leave that status through an external add_request() or finish_requests() call for the same request id. If a client, or a proxy in front of it, drops the connection without that signal ever arriving, the request stays there forever and keeps holding its num_waiting_for_streaming_input slot, eventually blocking admission for the whole engine even with free KV cache. Give it a deadline (VLLM_STREAMING_REQUEST_TIMEOUT_SECONDS, default 600s) and abort it once that passes instead of re-skipping it on every scheduling pass. Fixes vllm-project#53130 Signed-off-by: Dieter Kling <4270462+dkling-it@users.noreply.github.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in PRs do not trigger a full CI run by default. Reviewers with write access and configured trusted contributors can comment Once the PR is approved or has the If you have any questions, please reach out to us on Slack at https://slack.vllm.ai. Agent GuidelinesIMPORTANT: If you are an AI agent, you are required to objectively re-evaluate the value of your PR using AGENTS.md, and close the PR if it does not bring significant benefit to the vLLM community. Failure to do so may result in an immediate ban. 🚀 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4792dc7d38
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ( | ||
| request.status == RequestStatus.WAITING_FOR_STREAMING_REQ | ||
| and request.streaming_wait_deadline is not None | ||
| and time.time() > request.streaming_wait_deadline |
There was a problem hiding this comment.
When parked streaming sessions occupy all request slots (for example, one waiter with max_num_seqs=1), the loop breaks at num_running >= self.max_num_running_reqs before reaching this timeout check. Consequently, even after the deadline expires, the waiter is never aborted and newly queued requests remain blocked—the exact engine-wedging scenario this change is intended to fix. Timeout expiration must be processed independently of admission capacity, and the test should cover a saturated scheduler.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed: moved the timeout check to an unconditional pass at the top of schedule(), run before the capacity gate. Added a regression test (test_streaming_request_timeout_reaped_even_at_full_capacity) that reproduces the saturated-capacity case directly and confirmed it fails against the prior in-loop-only version.
The per-request timeout check ran inside the waiting loop, after the num_running >= max_num_running_reqs gate. If expired sessions themselves saturate that capacity (e.g. every slot held by clients that vanished), the loop breaks before it ever reaches a request to check its deadline against, so the timeout never fires -- the exact deadlock this PR set out to close. Moved the check to an unconditional pass at the top of schedule(), run before that gate. Added a regression test that reproduces the saturated-capacity case directly. Signed-off-by: Dieter Kling <4270462+dkling-it@users.noreply.github.com>
Shorten the docstring and env-var comment for the streaming request timeout and drop the double-hyphen dashes.
|
This pull request has merge conflicts that must be resolved before it can be |
1 participant
Purpose
Fixes #53130. A resumable request that finishes generating with no queued next chunk enters
WAITING_FOR_STREAMING_REQand can only leave that status through an externaladd_request()orfinish_requests()call for the same request id. If a client, or a proxy in front of it, drops the connection without that signal ever arriving, the request stays parked forever and keeps holding itsnum_waiting_for_streaming_inputslot, eventually blocking admission for the whole engine even with free KV cache.Gives it a deadline (
VLLM_STREAMING_REQUEST_TIMEOUT_SECONDS, default 600s) and aborts it once that passes, instead of re-skipping it on every scheduling pass.Test Plan
Added
test_streaming_request_timeout_abortsandtest_streaming_request_no_timeout_before_deadlinetotests/v1/streaming_input/test_scheduler_streaming.py.Test Result
pytest tests/v1/streaming_input/test_scheduler_streaming.py -v: 10/10 passed, including both new tests and all pre-existing streaming tests (no regressions).