Repository navigation
[Bugfix][NIXL] Verify pull reads against producer lease slots - #62065
Open
vadiklyutiy wants to merge 2 commits into
Open
vadiklyutiy wants to merge 2 commits into
vadiklyutiy wants to merge 2 commits into
Conversation
## Overview Fixes vllm-project#61229: a pull-mode decoder could decode another request's KV when the producer's lease lapsed before or during the READ. The producer now exports a per-request lease slot that the decoder reads after the transfer, and the KV is used only if the producer still held the blocks. Also fixes a second path to the same result: when a request id is reused on the producer (request id randomization disabled and the router retries with the same id), a late read notification for the earlier attempt was counted against the retry and released its blocks while the retry's decoder had not read them yet. Read notifications now carry the lease nonce, and the producer ignores those of an earlier lease. ## Claims - A pull whose producer released the KV blocks before the READ completed fails the KV load (`kv_load_failure_policy` applies) instead of returning another request's tokens. This covers a read that starts after the lease lapsed and a lease that lapses mid-read; the check uses no clocks or heartbeat accounting. - The producer ignores read notifications that carry the lease nonce of an earlier request with the same id, so a late reader can no longer release blocks that the current reader still needs. - With `kv_lease_verification` on (the default), a pull that cannot be verified fails as well (no lease fields in `kv_transfer_params`, slot READ failed), and an instance that cannot register its lease slots in DRAM fails to start. `kv_lease_verification=false` restores the old behavior. - TTFT and ITL unchanged at concurrency 1; at concurrency 16 median TTFT +1.3 ms (about one decoder step), ITL and throughput unchanged. ## Validation **Reproducer** (1P1D on 2x B200, Qwen2.5-0.5B, default UCX, P limited to 1000 KV blocks so that freed blocks are reused): prefill on P, wait 34 s so the 30 s lease lapses (D has not seen the request, so no heartbeats), send 40 other prompts to P, then send the original request to D with the saved `kv_transfer_params`; compare D's output with P computing the same prompt locally. Each prompt carries a unique tag so D cannot hit its own prefix cache. | Configuration | No wait (control) | Wait 34 s + reuse | | -------------------------------------- | ----------------- | ------------------------------------------------------------------------------------------------ | | Base (`75e3b17c05`) | matches | HTTP 200 with the other request's text ("Fact 100 about volcano eruption 32..."), 2/2 | | This PR, `kv_load_failure_policy=fail` | matches | HTTP 500, 2/2; D logs "Declining KV pulled ... released the KV blocks before the read completed" | | This PR, `recompute` | matches | matches the reference, 2/2 | | This PR, P TP1 / D TP2, `fail` | matches | HTTP 500; both D ranks decline | | This PR, P TP2 / D TP1, `fail` | matches | HTTP 500 | **Request id reuse** (P with `VLLM_DISABLE_REQUEST_ID_RANDOMIZATION=1`, same `X-Request-Id` for both attempts): prefill attempt 1, wait 34 s, prefill attempt 2 with another prompt, send attempt 1's params to D (a late reader), flood P, then send attempt 2's params to D. | Configuration | Late reader of attempt 1 | Reader of attempt 2 | | ------------- | ------------------------------------------------------------------------- | ------------------------------------------- | | Base | HTTP 200; its read notification releases attempt 2's blocks | HTTP 200 with the other request's text, 2/2 | | This PR | HTTP 500 (lease check fails); P ignores its notification (nonce mismatch) | matches the reference, 2/2 | The DRAM lease slots were readable with the default UCX transports. **Unit tests:** `pytest tests/v1/kv_connector/unit/{test_nixl_connector,test_nixl_heartbeat,test_nixl_connector_hma,test_bidirectional_kv_transfer,test_nixl_push_connector,test_nixl_desc_geometry,test_nixl_simple_cpu_offload,test_multi_connector}.py` on B200: 677 passed, 2 failed (base commit: 667 passed, 2 failed). The same 2 fail on the base commit in the same environment (gated `google/gemma-3-1b-it` in `test_fewer_blocks_with_hma`, `test_multi_example_connector_consistency`). New tests cover slot reuse within a step, the exported lease, verified and released pulls, a failed slot READ, pulls without a lease with verification on and off, slot zeroing on both release paths, and stale notifications. `create_vllm_config` in the test utils turns verification off by default, since the fake remotes export no lease slots. **Serving benchmark** (1P1D through `toy_proxy_server.py`, Qwen2.5-0.5B, random 1024 in / 32 out, `--ignore-eos`, CUDA graphs, FlashInfer attention; median of 3 runs): | | Concurrency 1, base / PR | Concurrency 16, base / PR | | ------------------ | ------------------------ | ------------------------- | | Median TTFT (ms) | 25.1 / 25.0 | 43.8 / 45.1 | | P90 TTFT (ms) | 26.7 / 26.6 | 57.1 / 57.8 | | Median ITL (ms) | 1.14 / 1.13 | 1.53 / 1.51 | | Throughput (req/s) | 16.6 / 16.6 | 165.4 / 166.1 | ```bash vllm bench serve --backend vllm --model Qwen/Qwen2.5-0.5B --base-url http://127.0.0.1:8000 \ --endpoint /v1/completions --dataset-name random --random-input-len 1024 \ --random-output-len 32 --ignore-eos --seed 0 --num-prompts 800 --max-concurrency 16 ``` ## Details **Root cause.** In pull mode P frees a request's blocks when its lease expires, without knowing whether D is about to read them (the READ is one-sided). D reads by block number, so after the blocks are reused the READ still succeeds and D decodes the new owner's KV: 1. P finishes the prefill, holds the blocks and grants a 30 s lease. 2. Heartbeats stop (D stalled, network delay) or never start (the request has not reached D yet). 3. P's reaper releases the blocks; another prefill reuses them. 4. D posts the READ with the old block ids; it completes normally and D generates from foreign KV. **Fix.** Each P worker registers an int64 array in DRAM with NIXL and advertises it in the handshake metadata. 1. When a `do_remote_decode` request is first scheduled, the P scheduler assigns a slot and a nonce; workers write the nonce in that step, before the request can finish and export its lease. 2. `kv_transfer_params` carry `remote_lease_slot` and `remote_lease_nonce`. 3. A P worker zeroes the slot before reporting the request in `finished_sending` (read notification or lease expiry); the scheduler reuses a slot only after `finished_sending`, and workers apply clears before writes within a step. 4. D submits the block READs without a completion notification. Once they completed, it reads the request's lease slot on every remote rank it read from (the checks of all requests that complete in the same step share one READ per remote rank), and reports the request as received only if every slot still holds the nonce. Only then does it send the read notification, which now carries the nonce. Zeroing precedes any reuse of the blocks and the slot READ starts after the block READs completed, so a matching nonce proves P held the blocks for the whole READ. Declined pulls are counted in `vllm:nixl_num_lease_check_failures`. **Tradeoffs and limitations.** - The check delays the KV load by about one decoder step. A bounded spin on the slot READ in the same step could remove it; left out to keep the change smaller. - `NIXL_CONNECTOR_VERSION` goes from 14 to 15 (new handshake fields, new `kv_transfer_params` fields, notification format `req_id:consumers:nonce`), so P and D must be upgraded together. - Turn-2 readbacks from D in bidirectional mode are not verified yet; `_is_turn2_read_expired` still guards them. - Push mode is unchanged. - When a declined pull fails the request (`kv_load_failure_policy=fail`), the serving layer's existing cleanup sends P one more read notification after D's own. P ignores it (stale nonce, or the lease is already gone), so the only effect is an extra warning or "unrecognized request" log on P. D still sends its own notification because under `recompute` there is no cleanup, and other P ranks would otherwise hold their blocks until the lease expires. - Lease slots live in DRAM, so the NIXL backend must transfer DRAM between the instances; with verification on, a failed registration stops the instance at startup and failed slot READs fail their pulls. - `kv_lease_slots` defaults to KV blocks + `max_num_seqs`: every scheduled or held prefill owns a block of its own, and preempted ones keep their lease. A request that finds no free slot gets no lease, so verifying decoders decline it. - Deployments whose router rebuilds `kv_transfer_params` from a fixed set of fields must forward `remote_lease_slot` and `remote_lease_nonce`, or set `kv_lease_verification=false`. The vLLM toy proxy, llm-d and Dynamo forward the params verbatim. - [vllm-project#59953](vllm-project#59953) moves the opposite way (a native notification on every READ, dropping the deferred notification queue). This PR relies on deferring the notification until the lease check, so the two need to be reconciled. - The Rust frontend decodes NIXL stats with `deny_unknown_fields` and already lacks two existing fields ([vllm-project#60856](vllm-project#60856)); the new counter needs the same treatment there. **Duplicate check.** [vllm-project#61549](vllm-project#61549) (draft) estimates the producer lease on D from clocks and heartbeat timing; it cannot rule out a heartbeat that was sent but not delivered, or clock-offset error. [vllm-project#61578](vllm-project#61578) adds opt-in KV checksums. This PR is an exact check that is on by default and costs one small READ per remote rank. --- Pull Request Checklist - [x] I used vLLM's `/pr-checklist` skill. (Mandatory for agents, optional for humans). - [x] AI assistance was used during the creation of this PR. - [ ] **Design Fit:** Minimizes impact on core components, reuses existing functionality, and justifies added complexity. - [ ] **Testing and Validation:** Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI. - [ ] **Code Quality and Style:** Keeps code and comments clear and concise, and updates relevant documentation and examples. - [ ] **Pull Request Contents:** Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Vadim Gimpelson <vadim.gimpelson@gmail.com>
vadiklyutiy
requested review from
ApostaC,
NickLucche,
ivanium,
orozery,
wzhao18 and
xuechendi
as code owners
October 16, 2026 18:13
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
|
Documentation preview: https://vllm--62065.org.readthedocs.build/en/62065/ |
mergify
Bot
added
documentation
Contributor
|
This pull request has merge conflicts that must be resolved before it can be |
…ification Co-authored-by: Claude Signed-off-by: Vadim Gimpelson <vadim.gimpelson@gmail.com>
Contributor
|
This pull request has merge conflicts that must be resolved before it can be |
|
Independent check on one H100 80GB: vLLM 0.32.0, Qwen/Qwen3-0.6B, NIXL 1.4.1 pull mode over same-host UCX, prefix caching off, and a 4-second lease. I replayed one saved schedule for the delayed-read condition in #61229 on stock and the runtime-only 42b8f23 backport. Stock: 8/8 failure trials reached the condition and recorded stale read submissions; 0/8 answers matched the clean reference. These were 3 discovery trials plus 5 confirmatory replays. Backport: 5/5 reached the condition. All 5 had request-ID-matched lease_mismatch rejections, and 5/5 client answers exactly matched the reference with the recompute policy. Both control cases passed in both arms with no observed declines. Fixed controls recorded 0 lease-check failures; stock did not expose that counter. Stock output was garbled. No other request's canary word appeared, so this demonstrates corruption rather than cross-request text disclosure. I can provide the saved results and schedule, and rerun another configuration if helpful. |
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.
Overview
Fixes #61229: a pull-mode decoder could decode another request's KV when the producer's lease lapsed before or during the READ. The producer now exports a per-request lease slot that the decoder reads after the transfer, and the KV is used only if the producer still held the blocks.
Also fixes a second path to the same result: when a request id is reused on the producer (request id randomization disabled and the router retries with the same id), a late read notification for the earlier attempt was counted against the retry and released its blocks while the retry's decoder had not read them yet. Read notifications now carry the lease nonce, and the producer ignores those of an earlier lease.
Claims
kv_load_failure_policyapplies) instead of returning another request's tokens. This covers a read that starts after the lease lapsed and a lease that lapses mid-read; the check uses no clocks or heartbeat accounting.kv_lease_verificationon (the default), a pull that cannot be verified fails as well (no lease fields inkv_transfer_params, slot READ failed), and an instance that cannot register its lease slots in DRAM fails to start.kv_lease_verification=falserestores the old behavior.Validation
Reproducer (1P1D on 2x B200, Qwen2.5-0.5B, default UCX, P limited to 1000 KV blocks so that freed blocks are reused): prefill on P, wait 34 s so the 30 s lease lapses (D has not seen the request, so no heartbeats), send 40 other prompts to P, then send the original request to D with the saved
kv_transfer_params; compare D's output with P computing the same prompt locally. Each prompt carries a unique tag so D cannot hit its own prefix cache.75e3b17c05)kv_load_failure_policy=failrecomputefailfailRequest id reuse (P with
VLLM_DISABLE_REQUEST_ID_RANDOMIZATION=1, sameX-Request-Idfor both attempts): prefill attempt 1, wait 34 s, prefill attempt 2 with another prompt, send attempt 1's params to D (a late reader), flood P, then send attempt 2's params to D.The DRAM lease slots were readable with the default UCX transports.
Unit tests:
pytest tests/v1/kv_connector/unit/{test_nixl_connector,test_nixl_heartbeat,test_nixl_connector_hma,test_bidirectional_kv_transfer,test_nixl_push_connector,test_nixl_desc_geometry,test_nixl_simple_cpu_offload,test_multi_connector}.pyon B200: 677 passed, 2 failed (base commit: 667 passed, 2 failed). The same 2 fail on the base commit in the same environment (gatedgoogle/gemma-3-1b-itintest_fewer_blocks_with_hma,test_multi_example_connector_consistency). New tests cover slot reuse within a step, the exported lease, verified and released pulls, a failed slot READ, pulls without a lease with verification on and off, slot zeroing on both release paths, and stale notifications.create_vllm_configin the test utils turns verification off by default, since the fake remotes export no lease slots.Serving benchmark (1P1D through
toy_proxy_server.py, Qwen2.5-0.5B, random 1024 in / 32 out,--ignore-eos, CUDA graphs, FlashInfer attention; median of 3 runs):Details
Root cause. In pull mode P frees a request's blocks when its lease expires, without knowing whether D is about to read them (the READ is one-sided). D reads by block number, so after the blocks are reused the READ still succeeds and D decodes the new owner's KV:
Fix. Each P worker registers an int64 array in DRAM with NIXL and advertises it in the handshake metadata.
do_remote_decoderequest is first scheduled, the P scheduler assigns a slot and a nonce; workers write the nonce in that step, before the request can finish and export its lease.kv_transfer_paramscarryremote_lease_slotandremote_lease_nonce.finished_sending(read notification or lease expiry); the scheduler reuses a slot only afterfinished_sending, and workers apply clears before writes within a step.Zeroing precedes any reuse of the blocks and the slot READ starts after the block READs completed, so a matching nonce proves P held the blocks for the whole READ. Declined pulls are counted in
vllm:nixl_num_lease_check_failures.Tradeoffs and limitations.
_is_turn2_read_expiredstill guards them.Duplicate check. #61549 (draft) estimates the producer lease on D from clocks and heartbeat timing; it cannot rule out a heartbeat that was sent but not delivered, or clock-offset error. #61578 adds opt-in KV checksums. This PR is an exact check that is on by default and costs one small READ per remote rank.
Pull Request Checklist
Pull Request ChecklistI used vLLM's
/pr-checklistskill. (Mandatory for agents, optional for humans).AI assistance was used during the creation of this PR.
Design Fit: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.
Testing and Validation: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI.
Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.
Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.