Repository navigation
[Bugfix][KV Offload] Drain unfetched P2P stores before cache reset - #63988
Conversation
|
Thanks for picking this up. Ran the repros from #63948 on this branch: both pass now and fail on main. Small thing: |
|
This pull request has merge conflicts that must be resolved before it can be |
liranschour
left a comment
There was a problem hiding this comment.
Thanks for fixing #63948. The direction looks right, but I think there are a few correctness issues in the drain path before this can go in, mainly the interaction with fetches that arrive mid-drain and with symmetric (lookup-supplied) rounds. Details are inline. The last three comments are pre-existing or nits.
813cdfb to
b666d32
Compare
Fail unbound and session-bound stores waiting for a decoder, withdraw their block references, and reject late fetches for invalidated handoffs. Keep in-flight transfers pinned until their transport reports completion. Add regressions for queued stores, late requests, in-flight success/failure, primary-slot reuse, and a real LLM cache reset with an absent decoder. Fixes vllm-project#63948 Signed-off-by: maithilijoshi20 <maithilij2003@gmail.com>
b666d32 to
0b9a70f
Compare
…drain Withdraw existing supply once and reject fetches arriving during drain without leaving producer bindings. Retain empty symmetric lookup rounds for prompt late-fetch failure and limit request tombstones to PD rounds. Use pending-abort-aware finalization and reuse the unbound reaping path. Move the GPU regression into the existing engine-test module with automatic port allocation. Use the host page size for primary-region tests, and cover drain races and fresh symmetric rounds. Signed-off-by: maithilijoshi20 <maithilij2003@gmail.com>
0b9a70f to
5952f80
Compare
Signed-off-by: maithilijoshi20 <maithilij2003@gmail.com>
9fc46ad to
f548437
Compare
|
/ci run |
|
✅ Triggered Buildkite CI #99191 for commit ℹ️ This PR is 181 commits behind upstream |
|
✅ @maithilijoshi20, CI is now available for this PR.
|
#63988 added the test with a 16-token offload block while the V1 KV Connectors job was disabled; #63231 then raised the default GPU block size to 64, which the offload block must divide. Pin block_size=16 as #63231 did for test_example_connector.py. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: khluu <khluu000@gmail.com>
Overview
Fix cache reset on P2P prefill nodes when store jobs are waiting for a decoder to fetch their KV blocks.
Fixes #63948.
Claims
Validation
Tested on WSL Ubuntu 24.04 with Python 3.12, PyTorch 2.14.0+cu132, NIXL 1.5.1/UCX, and an RTX 3050 8 GB.
The OPT-125m test waits for an active cascade with an absent decoder, calls
LLM.reset_prefix_cache(reset_connector=True), and checks that the next request has zero cached tokens and identical greedy output. Unit tests cover queued stores, mid-drain fetches, symmetric lookup rounds, active transfer pins, and abort acknowledgements. Pre-commit checks pass for the changed files.Without the fix, the issue's producer-reset reproducer fails at
assert not self._jobsinTieringOffloadingManager.reset_cache(); with the fix, both unbound and session-bound stores drain successfully, as confirmed in the issue author's PR feedback.Details
A prefill node can finish storing KV before its decoder sends a fetch. These jobs hold primary-tier slots but are not counted as active transfers, so waiting only for transfers leaves jobs behind when the parent resets its cache.
Drain fails unbound stores through the existing expiry path and withdraws queued session supply once. Fetches received during polling are rejected without binding their request IDs. In-flight writes retain their pins until the transport reports completion.
Withdrawn symmetric lookup rounds retain enough state to reject a late fetch without invalidating a fresh round under the same request ID. Aborted rounds use the existing acknowledgement path to avoid duplicate terminal messages.
This addresses producer stores waiting for decoder demand. Related #61667 covers consumer write quarantine. No duplicate fix was found when opening this PR. AI assistance was used for review, and validation.
Pull Request Checklist
/pr-checklistskill.