Add mods/fix-kv-offload-disk-tier: EAGLE store filter + multi-node promoted-row re-sync - #373
Add mods/fix-kv-offload-disk-tier: EAGLE store filter + multi-node promoted-row re-sync#373yaro-tal wants to merge 2 commits into
Conversation
Two bugs in OffloadingConnector + TieringOffloadingSpec with an fs secondary
tier. Shipped as one mod because neither fix is useful alone: apply only the
first and a multi-node cluster gets a working cache that corrupts; apply only
the second and there is nothing to correct, because the cache never hits.
1. EAGLE/MTP groups can never certify a hit, so the tier returns ZERO hits.
The store path keeps only the trailing `tail` chunks of each alignment
segment. But _sliding_window_lookup finds `tail` chunks and an unverified
EAGLE group then pops one (num_hit_chunks -= 1), so it needs tail + 1
CONSECUTIVE chunks and the store filter has guaranteed it can never see
them. Since _lookup() ANDs across groups, that permanent zero vetoes every
other group too. Measured here: 502 GB written, 0 bytes ever read back.
Fix: also keep the segment-head chunk for eagle groups, making the retained
set {0} u {acc-tail .. acc-1} -- a run of exactly tail + 1 consecutive
chunks across the segment boundary.
2. Tensor parallelism across nodes silently serves WRONG KV.
SharedOffloadRegion is a per-node /dev/shm mmap. On one node the
scheduler-side region (rank=None) and every worker region (rank=r) are the
same file -- rank only selects a slot within a row -- so a promotion the
tier manager performs is visible to every worker for free. That assumption
is undocumented and false across nodes: each node has its own mmap, and only
the rank co-located with the manager runs a secondary tier.
GPU->CPU stores stay symmetric, but disk->CPU promotions land only on the
manager's rank, and the following CPU->GPU load reads each rank's OWN
region. Every other rank feeds its GPU stale bytes. Nothing raises.
Hashing the same row index in both nodes' mmaps, before the fix:
rows written by GPU->CPU stores 112/112 identical
rows written by disk->CPU promotion 0/112 identical
After: 112/112. Every divergent row was a promoted row, and vice versa.
Fix: the manager records rows a completed promotion filled,
build_connector_meta() drains them into
OffloadingConnectorMetadata.promoted_rows, and every rank re-syncs them from
the manager's rank over the existing TP group before any load is submitted.
Read once and transfer over the link, rather than having every rank read the
tier: a second reader pulls the same bytes over the same link anyway and
hits the disk twice. A welcome consequence is that the tier no longer needs
to be shared between nodes at all.
Gated on the KV cache being replicated across TP ranks (all groups MLA),
because copying one rank's rows onto a head-sharded cache would corrupt it
just as thoroughly. VLLM_OFFLOAD_KV_REPLICATED overrides. A one-shot check
after the first broadcast confirms the rows really landed identically. On
single-node deployments the whole thing is a no-op.
Verified against vLLM e2666d9a65f41fc376607531453cbd57c4c71016 on
DeepSeek-V4-Flash-0731 across two DGX Sparks (TP=2, one GPU per node).
Status update: this mod is incomplete for the case it targets, and I know why nowSix days of production data later, I want to be straight about a limitation rather than leave it for someone to discover. The two patches here make the disk tier correct. They do not make it useful on a prefix larger than your primary tier. If you apply this mod and your prefixes are long, you will most likely see the tier do nothing at all — no error, no warning, just persistent zero hits. That is not this mod failing to apply; it is a third bug underneath it. The bug
return LookupResult.MISS if not promoted else LookupResult.RETRYThe matching walk promotes one primary-tier row per queried key, just to confirm the key is there. Once the primary tier is full, every subsequent key — including keys that are sitting on disk — comes back It is self-reinforcing: the walk eats the same rows Why this will hit essentially everyone using this imageThe primary tier is sized out of host RAM, and we are all on GB10 boxes with 128 GB total. With a large model resident there is very little left — our How to tell in 30 secondsOn a vetoed lookup, sum the per-group RETRY counts. If the sum pins at exactly your primary tier's row count, you are hitting this and your data is on disk, not missing. You can confirm the row count independently from metric granularity: Do not read a high miss count as disk absence without doing this first. That misreading cost us about two weeks. Work in progressThe fix is separating matching from staging — match with
Across 40 requests: 88.3% of prompt tokens covered, 1,349,632 tokens not re-prefilled. I would rather land that here as a third patch than leave the mod half-useful, so please treat this PR as not-ready-to-merge for now unless you want the correctness fixes on their own. I will follow up with the patch. Two things are still unverified and I will not claim otherwise: the second rank's GPU-side KV is confirmed only at the CPU-row mirror, and the multi-wave path has never actually executed (see below). A second-order note for anyone tuning: with the ceiling gone, the remaining pressure is concurrency. One 440k-token prefix needs ~222 of our 498 rows, so two concurrent large requests fit and three do not. An admission gate is written but deliberately switched off until it has been exercised. Related
ProvenanceWritten with AI assistance and verified on a live two-node cluster. These patches are a collaboration between Claude Opus 5 and DeepSeek-V4-Flash, each reviewing the other's work and keeping the other honest; every number above is measured from a real deployment rather than asserted by a model. Flagging it so any policy you have on AI-assisted contributions is your call, not my assumption. 🤖 Generated with Claude Code |
Patches 01 and 02 make the disk tier CORRECT. They do not make it USABLE on a
prefix larger than the primary tier. This is the patch that does.
TieringOffloadingManager.lookup ends:
return LookupResult.MISS if not promoted else LookupResult.RETRY
The matching walk promoted one primary-tier row per queried key, purely to
confirm the key existed. Once the tier filled, every further key -- including
keys present on disk -- returned MISS, indistinguishable from absence, and the
cross-group AND discarded the whole external hit. Self-reinforcing: the walk ate
the rows prepare_store needed, so the tier stopped being written too.
This will hit most users of this image. The tier is sized from host RAM on
128 GB boxes, so a few GiB buys a few hundred rows; a 242k-token prefix queries
12,434 keys.
Diagnosis in 30 seconds: sum the per-group RETRY counts on a vetoed lookup. If
they pin at exactly your tier's row count, your data IS on disk. Row count is
confirmable from metric granularity (kv_offload_cpu_cache_usage_perc only ever
takes k/rows values).
Fix: match with promote=False, then stage the confirmed hit in waves. Three new
stdlib-only modules with host-runnable tests, plus the driver in scheduler.py
and the promote flag in manager.py.
Measured on a live 2-node TP=2 DeepSeek-V4-Flash-0731 cluster:
summed RETRY per probe 498 -> 0
exit=ZERO 14 -> 0
cold 294,186-tok prompt ext=0 -> 290,816 (98.9%)
Since then: 23 multi-wave loads at num_waves=2/3 with exact slicing and blocks
closing end to end; both ranks confirmed job-for-job; a 1 GiB (124-row) tier
served a cold 348,000-token prompt at 99.5%, which is monolithically impossible
and can only be done by streaming; and next-token distributions from tier-served
KV sit within the engine's own noise floor, equal to vLLM's own GPU prefix cache.
Parking works (16 slot events, all released) but ships OFF by default; treat it
as the least-exercised part.
Scope, stated plainly: this also carries ~70 lines of env-gated diagnostics,
inert unless their env var is set. They are the instruments that found the bug;
hand-removing them would ship code we have not run.
run.sh also gains a whole-set idempotency guard. The per-patch reverse-check
broke once a third overlapping patch existed -- reversing 01 alone fails while
03's edits to the same regions are present, so a second run errored instead of
skipping. Verified: applies cleanly from stock upstream, all files compile, and
re-running now skips.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Patch 03 is in, and this is ready for review nowFollowing up on my earlier comment asking you to hold this: the missing piece has landed as What 03 doesMatching promoted one primary-tier row per queried key just to confirm the key existed. Once the tier filled, keys that were sitting on disk returned Fix: match with Results, all measured on a live 2-node TP=2 cluster
Since the earlier comment, four things I would not claim without evidence:
Zero asserts, zero aborts, zero Things I want to be upfront about
Verified before pushing: applies cleanly to a from-scratch reconstruction of stock vLLM ProvenanceWritten with AI assistance and verified on the hardware described. A collaboration between Claude Opus 5 and DeepSeek-V4-Flash, each reviewing the other's work and keeping the other honest. Every number here is measured on a real deployment, not asserted by a model — including the ones that came out inconvenient. 🤖 Generated with Claude Code |
11-hour soak updateThe numbers in my previous comment came from short windows. Here they are after 11 hours of continuous real workload (agent coding sessions, 250-350k-token prefixes), at
Two things this firms up specifically:
Nothing in the patch or the README changes as a result — posting because the earlier evidence was thinner than I would want anyone to merge on. |
1 participant
Two bugs in
OffloadingConnector+TieringOffloadingSpecwith anfssecondary tier. They're in one mod because neither fix is useful alone: apply
only the first and a multi-node cluster gets a working cache that corrupts;
apply only the second and there's nothing to correct, because the cache never
hits.
Found while running DeepSeek-V4-Flash-0731 across two DGX Sparks (TP=2, one GPU
per node). Verified against vLLM
e2666d9a65f41fc376607531453cbd57c4c71016.1. EAGLE/MTP groups can never certify a hit — the tier returns zero hits
The store path drops sliding-window chunks no lookup could reach, keeping only
the trailing
tailchunks of each alignment segment. But_sliding_window_lookupfindstailchunks and an unverified EAGLE groupthen pops one (
num_hit_chunks -= 1), so it needstail + 1consecutivechunks — which the store filter has guaranteed it can never see.
Because
_lookup()ANDs the per-group results, that group's permanent zerovetoes every other group as well. The tier reports a 0% hit rate while happily
writing hundreds of GB. Measured here: 502 GB stored, 0 bytes ever read back.
Fix: also keep the segment-head chunk for eagle groups, so the retained set is
{0} ∪ {acc-tail .. acc-1}— a run of exactlytail + 1consecutive chunksacross the segment boundary. The pop lands on a chunk at
0 (mod acc), so theresulting length stays a multiple of the full-attention chunk size.
2. Multi-node TP silently serves wrong KV
SharedOffloadRegionis a per-node/dev/shmmmap. On a single node thescheduler-side region (
rank=None) and every worker region (rank=r) are thesame file —
rankonly selects a slot within a row — so a promotion the tiermanager performs is visible to every worker for free. That assumption is
undocumented, and false as soon as TP spans nodes: each node has its own mmap,
and only the rank co-located with the manager runs a secondary tier at all.
So every other rank feeds its GPU whatever stale bytes occupied that row.
Nothing raises, nothing warns, no checksum fails.
Hashing the same row index in both nodes' mmaps:
Over a longer run the correspondence was exact: every divergent row was a
promoted row, and every promoted row was divergent.
The symptom depends only on what was in the stale row, which is why it presents
as several unrelated bugs — zeros (fresh mmap) give coherent output with
confabulated later content; a recycled row gives token soup and other
sessions' content bleeding into the response. Hits served entirely from the
CPU tier are always correct, because no promotion is involved, which is what
makes it hard to catch.
Fix: the manager records the rows a completed promotion filled,
build_connector_meta()drains them intoOffloadingConnectorMetadata.promoted_rows, and every rank re-syncs them fromthe manager's rank over the existing TP group before any load is submitted.
Ordering matters:
build_connector_meta()runson_schedule_end()→completed-job processing first, so a promotion that lands in a step ships its
row ids in that same step's metadata. All ranks get an identical list, so they
issue the same collectives in the same order.
Read once and transfer over the link, rather than having every rank read the
tier — a second reader pulls the same bytes over the same link anyway and
hits the disk twice. Consequence worth having: the tier no longer needs to be
shared between nodes, so it can be node-local with no NFS.
Safety gate
Re-syncing one rank's rows onto another is only correct for a replicated KV
cache. MLA stores one compressed latent per token and is replicated by
construction; a head-sharded cache (GQA/MHA) or per-rank recurrent state
genuinely differs per rank, and copying over it would corrupt it just as
thoroughly as the bug. So the mod does nothing unless every KV group is
known-replicated, and logs which way it decided.
VLLM_OFFLOAD_KV_REPLICATED=0|1overrides. A one-shot check immediately afterthe first broadcast confirms the rows landed identically — race-free, since
equality there holds by construction.
On single-node deployments patch 2 is a no-op, so it's safe to leave applied.
Testing
compileallclean,import vllmOK, andrun.shis idempotent on a second run.prompts, load it back from disk. Before: multilingual token soup. After: all
checkpoints correct, with the row diff going 0/112 → 112/112 identical.
fsreads), not a CPU-tier hit —reset_prefix_cachedoes not drain the CPU tier atblocks_per_chunk > 1,which makes naive tests measure memory instead.
Notes
PYTHONHASHSEEDmust be set to the same fixed value everywhere orNONE_HASHis reseeded per process and nothing on disk is ever found again. vLLM already
warns about this; the tier makes it expensive. Worth knowing when checking:
/proc/<pid>/environis unreliable forVLLM::EngineCore, which callssetproctitleand clobbers that region — it cost us a wrong diagnosis.Happy to split this into two mods if you'd rather, though they only produce a
working tier together.