Repository navigation
Conversation
Co-authored-by: OpenAI Codex <codex@openai.com> Signed-off-by: liofranxx <liofranxx@99-1-180-7.lightspeed.clmasc.sbcglobal.net>
Resolve readiness-health conflicts while preserving upstream KV event cleanup.\n\nCo-authored-by: OpenAI Codex <codex@openai.com>\nSigned-off-by: liofranxx <liofranxx@99-1-180-7.lightspeed.clmasc.sbcglobal.net> Signed-off-by: liofranxx <liofranxx@99-1-180-7.lightspeed.clmasc.sbcglobal.net>
DP>1 fixes: - Only count a DP dummy batch as ready progress when the rank has no local unfinished requests. A rank that holds requests but schedules nothing (e.g. vllm-project#45388) keeps the wave alive with dummy batches and was previously reported as making progress. - Run the idle readiness probe for DP>1 instead of skipping it. Dense DP ranks are independent engines and run the dummy-batch probe as with DP=1. MoE DP engines answer with a liveness-only ping, since a dummy batch outside the wave loop could deadlock against peer collectives; a busy loop wedged in a control RPC now times out the probe. Not-ready details: - /ready returns a JSON body for every not-ready case: {"status": "not_ready", "reason": ..., "message": ..., <details>, "in_progress": [...]} with reason one of engine_dead, sleeping, stalled, probe_timeout or probe_failed. - in_progress lists the utility call each EngineCore is executing, with its rank and elapsed time. EngineCore publishes the operation on the ready-progress broadcast before running a utility and clears it afterwards, so every API server reports it regardless of which one issued the call. Tests: - Unit tests for the DP probe, reason/in-progress reporting and the engine-side operation broadcast. - Fault-injection E2E tests (tests/v1/engine/readiness) using --scheduler-cls / --worker-extension-cls: scheduler stall, latent CUDA fault while idle and no-false-positive checks on a single engine, and MoE/dense DP=2 variants including a rank wedged in a mismatched DP collective. Wired into the V1 Engine and Distributed DP Basic steps. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
Contributor
|
Documentation preview: https://vllm--60505.org.readthedocs.build/en/60505/ |
mergify
Bot
added
documentation
Contributor
|
This pull request has merge conflicts that must be resolved before it can be |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
SumanthRH
marked this pull request as ready for review
October 8, 2026 01:45
SumanthRH
requested review from
AndreasKaratzas,
DarkLight1337,
Harry-Chen,
NickLucche,
aarnphm,
jperezdealgaba,
khluu,
njhill and
robertgshaw2-redhat
as code owners
October 8, 2026 01:45
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
Author
|
/ci run |
|
❌ @SumanthRH, A reviewer with write access must run |
Contributor
Author
|
RIP |
|
✅ @SumanthRH, CI is now available for this PR.
|
Contributor
|
/ci run |
Collaborator
CI selector (shadow): 152 test steps (213 jobs) instead of 86 (106 jobs)Shadow mode: this changes nothing about what CI runs. It shows what the evidence-based selector would pick for this PR, next to today's rules. How it works. Feedback welcome: reply here if it would skip a step this change needs, or runs something unrelated.
Selector would run (152)
Would skip (today's rules run them) (9)
Would add (today's rules do not run them) (75)
AMD mirrors: would skip (8)
AMD mirrors: would add (61)
21 changed files · base |
ShuoleiWang
reviewed
Oct 8, 2026
| if progress.operation is not None | ||
| ] | ||
|
|
||
| def _mark_engine_busy(self, engine: EngineIdentity) -> None: |
Contributor
There was a problem hiding this comment.
This mark has no undo path, so /ready can stay at stalled on an idle, healthy server.
If the EngineCore input thread drops the request (preprocess_add_request raising, or a P0/P1 multimodal cache miss), it never reaches the busy loop, the error output carries no readiness fields, and the engine publishes nothing. _check_ready then sees BUSY, skips the idle probe, and returns stalled after VLLM_READY_STALL_TIMEOUT_S. It stays that way until another request or utility call comes in. Behind a Kubernetes readiness probe it never recovers: the pod is pulled, so no request arrives to fix the state.
Suggestion: update _ready_progress only from engine messages and drop this mark. The early return in _check_ready currently relies on it to catch a wedged idle rank while another DP rank is busy, so this should go together with per-rank evaluation: progress check for BUSY ranks, idle probe for IDLE ranks.
EngineCore now broadcasts readiness progress to every frontend. The Rust client decoded EngineCoreOutputs as a fixed 8-field tuple and rejected readiness-only messages, so the decode error closed all request and utility registries on the first IDLE->BUSY transition. - Decode ready_progress_seq, ready_state and ready_operation. - Classify readiness-only messages as EngineCoreOutputs::Readiness; the fields are ignored on request, utility and DP control payloads. - The output dispatcher ignores readiness until the Rust frontend serves /ready. - Mirror the fields in python_compat.py and check that Python-encoded readiness-only and readiness-carrying request frames decode. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
The frontend marked an engine BUSY when it sent a request. If the engine dropped the request before the busy loop (preprocessing error, MM cache miss), nothing undid the mark and /ready reported stalled until another request arrived, which never happens behind a readiness probe. /ready also skipped the idle probe as soon as any rank was BUSY, so a dense DP rank wedged while IDLE went unnoticed. - Track readiness only from EngineCore messages; drop _mark_engine_busy. The engine-side guard in check_health_gpu already makes a probe that races a new request a no-op. - Check BUSY ranks for stalls and probe every IDLE rank, sharing one in-flight probe per rank. Probe failures report the engine ranks. - Ignore readiness from engines that are no longer tracked, so ranks paused on their way out of an elastic EP scale-down cannot leave /ready sleeping. - Drop the probe-task lock; task creation never suspends. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
…or it The idle probe reused execute_dummy_batch, whose LoRA warmup context adds dummy adapters and calls remove_all_adapters() on exit, so polling /ready on an idle --enable-lora server evicted every loaded adapter. The probe also returned once its kernels were queued, so a fault in the forward pass (or one queued just before it) was not reported by that probe. - Add Worker.check_health_gpu: a dummy batch under keep_loaded_loras(), which skips dummy adapter setup and removal, followed by a device synchronize. Executors expose it and EngineCore's probe uses it; DP wave dummy batches are unchanged. - The latent fault E2E test now requires the first /ready after the fault to fail, with a variant that faults inside the probe's forward pass. Both fail without the synchronize. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
Scheduler.has_requests() stays true for finished requests awaiting delayed KV connector frees and for pending connector pushes, so such an engine stays BUSY while scheduling no tokens and /ready reported it as stalled, e.g. a P/D prefill node waiting for its decode peer to pull. Count a step as progress when no unfinished requests remain. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
Readiness fields were stamped onto request outputs in addition to the standalone broadcast, which is always queued first and reaches every frontend. Drop the stamping so request outputs keep their original shape. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
…by GPU count
- Out-of-tree workers have no check_health_gpu; WorkerBase now raises
NotImplementedError so /ready reports that instead of an AttributeError.
- Rename the readiness E2E tests to test_ready_single_gpu.py and
test_ready_multi_gpu.py so the 2-GPU CI step runs one file.
Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
griffe warned that the documented `details` parameter had no annotation, which fails `mkdocs build --strict`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
The multiproc executor read the probe's reply only from the output rank (TP0 of the last PP stage). PP dummy runs do not communicate across stages, so a CUDA fault on an earlier stage left /ready at 200 until the next request failed. Every rank now replies. Add a PP=2 E2E test with the fault on the non-output stage. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
ShuoleiWang
reviewed
Oct 10, 2026
ShuoleiWang
left a comment
ShuoleiWang
left a comment
Contributor
There was a problem hiding this comment.
Non-blocking: the tests could be shorter without losing coverage, e.g. a shared EngineCoreProc setup in test_async_llm_health.py, one parametrized test for the busy/sleeping/DP-wave cases that skip the dummy batch, and the three mocked /ready handler tests in test_basic.py merged into one parametrized test that still checks reason and the JSON body.
| def is_ready(self) -> bool: | ||
| return self._is_ready and not self._shutdown_event.is_set() | ||
|
|
||
| async def check_ready(self) -> bool: |
Contributor
There was a problem hiding this comment.
When the supervisor runs Rust children (VLLM_USE_RUST_FRONTEND), every child returns 404 for /ready, so the supervisor's existing /ready and /readyz stay at 503 even when all children are healthy. The new test_check_ready in test_basic.py hits the same gap in the Rust frontend CI job, which runs that file with VLLM_USE_RUST_FRONTEND=1.
Since Rust /ready is planned as a follow-up anyway, the simplest fix is to leave both out of this PR: keep the supervisor's /ready as it was, and drop test_check_ready, which the readiness e2e tests already cover. Supervisor aggregation can come with the Rust endpoint.
6 of 8 tasks
Contributor
|
This pull request has merge conflicts that must be resolved before it can be |
Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
The Rust frontend has no /ready yet, so supervisor aggregation reported 503 with Rust children, and test_check_ready failed in the Rust frontend CI job. Both move to the Rust /ready follow-up; the readiness E2E tests already cover a healthy server returning 200. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
The DP supervisor already serves /ready and /readyz to mean "the process is up". /health_generate says what the check does and matches SGLang's name for the same probe. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
SumanthRH
changed the title
/ready/health_generate
Contributor
|
This pull request has merge conflicts that must be resolved before it can be |
21 of 87 tasks
…ing state Address review feedback on the core footprint and test size. - EngineCore.check_health_gpu(cache_ttl_s, stall_timeout_s) now returns the rank's health state: sleeping, stalled, busy, or ok after the GPU probe. A last-progress timestamp, restarted whenever the engine becomes busy, replaces the progress counter. - get_current_operation is answered by the EngineCore input thread (like handle_fault_tolerance), so in_progress is reported while the busy loop is blocked inside a utility call. - Remove the readiness broadcast, the EngineCoreOutputs readiness fields, the per-rank client state and its DP rank syncing, and the Rust changes. - AsyncLLM asks every engine rank and aggregates the answers. - Use WorkerBase.synchronize_device() in the GPU worker probe. - Collapse the unit and handler tests into parametrized tests, and add an E2E test for a blocked utility call. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
6 tasks done
|
Context: while evaluating this design I prototyped a much smaller opt-in variant, and I'm deliberately not opening a competing PR (repo policy on duplicate work) — posting it here in case any of it is useful for this PR. Branch: https://github.com/dunjin/vllm/tree/feat/health-generate-probe Approach:
Known trade-off, same as the #24491 discussion: the probe enqueues a request, so a deep queue can produce a false 503 — that's exactly why it is opt-in and separate from Happy to fold any part into this PR, or drop it entirely — your call. |
6 tasks done
6 tasks done
This branch has not been deployed
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
Adds a generation health check at
/health_generateThe implementation is based on PR #59179, with improvements that cover more edge cases with DP > 1 and report why the server is not ready. I've added the author of #59179 as a committer on this PR. Please see #59179 for context on the initial design.
This does not duplicate other work: it continues #59179, and I'm not aware of another open PR adding a generation health check.
Background
For long running inference deployments for online serving as well as RL use-cases, there can be a variety of inference engine failures or hangs. Broadly there are two sources of failures / hangs:
a. Probabilistic, recoverable: Errors that happen due to low-probability events such as:
b. Irrecoverable bugs: Bugs that are deterministic failures in certain state.
A generation health check helps users detect and recover from failures of class 1. and 2.a.
List of failures to handle
Here are a few relevant issues for vLLM server failures / hangs:
max_model_lenwhen DP ranks disagree about whether the drafter path should run.reduce_scatteror a related collective.pause_generation,sleep(),collective_rpc, or online weight update. The older issue covers a lateSTART_DP_WAVEre-arming a paused engine; the later PR fixes the coordinator assuming engines started merely because it forwarded a wake.FIRST_REQ, or ranks can enter mismatched collectives during weight-sync/control RPCs.running=0,scheduled=0, waiting requests remain stuck, and throughput stays at zero.What kind of health checks will detect such failures / hangs?
/healthendpoint/health_generatein SGLang/health_generatein vLLM (SGLang's check plus busy-stall detection)One failure that is not caught by any of the methods above is #53130. I would argue that it is out of scope for the current effort, since the issue is a bit unclear on the exact root cause for the bug. A part of the issue also mentions that the hang occurs when the scheduler is already busy and is able to make progress on running prefill inputs, which means that the engine is alive and healthy.
Note that we haven't discussed any recoverable errors from infrastructure failures. Typical failures here lead to collective timeouts or EngineCore process death, which are of a similar nature to the failures above.
Changes on top of #59179
Readiness evaluation
Per rank. Each engine rank is checked separately:
sleeping;So a busy DP rank no longer hides an idle rank that is stuck in a control RPC.
Progress comes only from the engines. The frontend no longer marks a rank busy when it sends a request. That mark could leave the server reporting
stalledwhile idle, if the engine dropped the request before scheduling it.Connector cleanup is not a stall. Steps that only drain KV-transfer cleanup for finished requests (e.g. a P/D prefill node waiting on a remote pull) count as progress.
A dedicated broadcast. EngineCore publishes readiness to every API server in its own message. Nothing is attached to request outputs any more.
The idle GPU probe
check_health_gpuon the worker:WorkerBase.check_health_gpuraisesNotImplementedError, so platforms that don't implement it reportprobe_failed.VLLM_READY_IDLE_PROBE_CACHE_TTL_S.DP > 1
Not-ready details
{"status": "not_ready", "reason": ..., "message": ..., <details>, "in_progress": [...]}reasonis one ofengine_dead,sleeping,stalled,probe_timeoutorprobe_failed.engine_deadreturns onlystatusandreason.in_progresslists the utility call each EngineCore is running, with its rank and elapsed time. EngineCore publishes the operation before running a utility and clears it afterwards, so every API server reports it, whichever one issued the call.Configuration
VLLM_HEALTH_CHECK_GPU_TIMEOUTprobe_timeout.VLLM_READY_STALL_TIMEOUT_Sstalled.0disables the check.VLLM_READY_IDLE_PROBE_CACHE_TTL_S0disables the cache.Scope
/health_generateendpoint, and aggregating it in the DP supervisor, will come in a follow-up PR./readyand/readyzare unchanged.Why report additional not-ready details?
Downstream applications, such as a load balancer or a health monitor daemon, decide whether to tear down a server based on the response. A 503 from
/health_generatedoes not mean the server is broken; it means the server can't serve generation requests right now. For example, it returns 503 while waiting for a weight sync to complete.Comparison with the
/fault_tolerance/APIsThe current fault tolerance APIs can detect failures/hangs during the forward pass in DPEP MoE deployments. This PR targets a broader set of failures/hangs, which is why it adds a separate
/health_generateendpoint.Known limitations
VLLM_EXECUTE_MODEL_TIMEOUT_SECONDS(300 s) and then completes, its late reply stays queued. The next executor RPC reads it, and the engine fails. The same applies to any timed-outcollective_rpcutility call today.VLLM_HEALTH_CHECK_GPU_TIMEOUT <= 0makes every idle probe reportprobe_timeout./health_generateis not served yet (see Scope above).Test Plan
Apart from the unit tests added in #59179, I've added real E2E tests that cover failure modes from the list above. I think these E2E tests are important to guarantee failure recovery at the engine level.
New tests:
in_progressreporting;tests/v1/engine/readiness, using--scheduler-clsand--worker-extension-cls, split by GPU count:test_ready_single_gpu.py(1 GPU): scheduler stall, a latent CUDA fault while idle (including a fault in the probe's own forward pass), and no false positives.test_ready_multi_gpu.py(2 GPUs): MoE and dense DP = 2 variants, including a rank stuck in a mismatched DP collective, and a PP = 2 CUDA fault on the stage that doesn't return outputs.Test Result
tests/v1/engine/test_async_llm_health.pytests/entrypoints/serve/instrumentator/test_basic.pyserver_loaddeselected, as in CI)tests/v1/engine/readiness/test_ready_single_gpu.pytests/v1/engine/readiness/test_ready_multi_gpu.pytests/entrypoints/launchers/test_dp_supervisor.pyvllm-engine-core-clientAI assistance (Claude Code) was used for this PR. I have reviewed every changed line and ran the tests above.
🤖 Generated with Claude Code
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.