[Bugfix] Gate hybrid+connector divergent local-hit path on connector opt-in [necessary but NOT sufficient — predicted 0/38 refuted, see comment] - #403
Conversation
…opt-in The per-group hybrid lookup at Scheduler.schedule() reports the full-attention group's local prefix-cache hit as the model-wide computed prefix (num_new_local_computed_tokens = max(per_group_hits)) whenever ANY KV connector is attached, and delegates restoration of the lagging recurrent (Mamba/GDN) state at that boundary to the connector. Only nixl fulfils that contract (_apply_prefix_caching transfers the SSM state block unconditionally). A connector that does not restore lagging-group state (e.g. LMCacheMPConnector) leaves the Mamba/GDN recurrent state at the resume boundary uninitialized or stale: the attention KV is valid, the recurrent state is not, and generation is silently corrupted for content inside the divergence window. HTTP 200, no crash, no error. Root-caused against receipts/lmcache-reuse-test.json (four-arm ladder, identical frozen probe and thresholds): L0 (no connector, vLLM's own prefix cache only) 0/38 failed; L1cold (connector attached, cold) 7/38; L2warm (same server, warm) 7/38 (1 corruption detector fire); L3restart (fresh server over retained connector-backed L2) 38/38. Every bounded failure's needle falls inside [hit_tokens - 1600, hit_tokens), the span covered by valid full-attention KV but not yet integrated into any restored recurrent state (1600 = the connector chunk size); every bounded-arm request, passing or failing, shows the same ~0.245 mean |chosen logprob delta| against control, consistent with every non-zero hit having run over a corrupted boundary state and only requests with a scored needle in the window failing the answer check. Mirror upstream vLLM's fix for this defect class (PR vllm-project#48425 lineage, capability flag supports_divergent_local_hybrid_hits): gate the per-group divergent-hit path on the connector opting in via getattr(connector, 'supports_divergent_local_hybrid_hits', False). Connectors that do not opt in fall back to KVCacheCoordinator.get_computed_blocks() (the reconciled min-across- groups hit), which only resumes generation where every cache group's state, including the recurrent one, actually exists -- exactly the regime the clean L0 control ran under. Trigger condition (both required): (a) a hybrid model (Mamba/GDN + full-attention layers) AND (b) an external KV connector attached (LMCache, nixl, etc.). Native vLLM prefix caching with no connector attached is unaffected; this is the L0 control above and is what every currently shipped GG image and campaign serve. Tradeoff: a GG deployment using nixl P/D with hybrid models loses the FA-hit optimization (nixl already restores state unconditionally, so it is unaffected in practice today, but does not yet advertise the opt-in flag) until GG rebases the upstream capability-flag plumbing so nixl's connector can set supports_divergent_local_hybrid_hits=True. Verification: applies cleanly to dev/gilded-gnosis @ fa033bd (the head this branch is cut from, byte-identical to the pinned r34 image rootfs's scheduler.py), round-trips byte-identical, and the patched file py_compiles. Behavioral proof (four-arm ladder re-run with this gate applied, predicted to turn L1cold/L2warm 0/38) is GPU-only and is tracked as a pending, separately-gated re-run -- not run as part of this CPU-only patch production. Refs: receipts/lmcache-fix.json, receipts/lmcache-reuse-test.json Mirrors: vllm-project#48425 Signed-off-by: Michel Belleau <michel.belleau@malaiwah.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe scheduler now checks ChangesHybrid Prefix-Cache Scheduling
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This localized scheduler change is merge-ready after normal checks and review; no actionable merge-blocking risk remains. Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…al-inference-lab/vllm Files the hybrid+connector divergent-hit scheduler defect that LMCacheFix root-caused (receipts/lmcache-fix.json) on our own downstream fork, honestly scoped as CPU-proven / GPU-pending: - Issue local-inference-lab/vllm#402: root cause, four-arm ladder evidence (receipts/lmcache-reuse-test.json) reinterpreted, trigger condition, campaign-unaffected statement, duplicate search against this repo specifically, prior-art cite vllm-project/vllm#48425 - PR local-inference-lab/vllm#403 (malaiwah/vllm-voipmonitor fix/gg-scheduler-divergent-hybrid-hit-gate @ 50be10ca1, DCO signed): the reviewed patch applied verbatim (not retyped), CPU gates re-verified (apply clean, round-trips byte-identical, py_compiles), GPU behavioral proof stated explicitly as pending a separately planned ladder re-run Duplicate search covered local-inference-lab/vllm specifically (LMCacheFix had already searched vllm-project/vllm and LMCache/LMCache). Two adjacent PRs (#401 DCP hash alignment, #293 load-failure recovery) found and distinguished as not duplicates. No GPU used.
Correction from the author: this PR's predicted outcome is REFUTED by measurementI predicted in this PR's body that the gate would take the LMCache corruption from 7/38 to 0/38.
Please do not read this PR as a corruption fix. It remains, I believe, correct scheduler code — The one-line reason, and why it is not "the patch broke it"
What is downstream, now traced to a specific line rather than guessedMy first write-up blamed the retrieve path for not restoring Mamba/GDN state. That was wrong, and I
That also explains the otherwise puzzling ladder result that a poison-free L2 is unreachable: every Second hole, and it is this repo's: acceptance is keyed to attention-chunk existence and never to state Suggested dispositionKeep this PR as a scheduler-correctness change with the prediction struck, and pair it with a Full evidence: 7 arms × 38 requests, all 266 scored rows, metric deltas and overlay provenance — |
Follow-up: my own mechanism attribution above was also wrong — measured, not arguedI posted earlier in this thread that the downstream defect was store-side (null mamba block ids stored The dominant defect is fp8-KV transfer. With this PR applied, on one GPU, single writer, no
What that means for the three stories in play:
Practical consequence worth stating plainly: any deployment serving fp8 KV — which is a common This PR's disposition is unchanged from my previous comment: a scheduler-correctness change whose Three operational facts we hit that are not documented anywhere I can find, offered in case they save Five arms with server ledgers: |
Closes #402.
Provenance
Derived directly from the reviewed, sha256-pinned patch file
patches/gg-vllm-hybrid-divergent-hit-gate.patch(sha2565f9ad10bedff87c4f37ce61bfe9be5cc9ed84dce99d9d3685c54b0341529839f) inmalaiwah/qwen38-27b-exl3— applied withgit applyontodev/gilded-gnosis @ fa033bd4e, not retyped. That commit'sscheduler.pyisbyte-identical to the copy vendored in the pinned r34 serving image's rootfs
(verified by direct diff before this PR was opened), so this is exactly what
ships today.
Bug
See #402 for the full root cause and the four-arm ladder evidence
(
receipts/lmcache-reuse-test.json): the hybrid per-group local prefix-cachelookup in
Scheduler.schedule()reports the full-attention group's hit as themodel-wide computed prefix whenever any KV connector is attached, and
relies on the connector to restore the lagging recurrent (Mamba/GDN) state at
that boundary. Only nixl's
_apply_prefix_cachingdoes that unconditionally.LMCacheMPConnectordoes not, so generation resumes with an uninitialized orstale recurrent state and is silently corrupted — HTTP 200, no crash — for
content inside the divergence window (measured: L0 no-connector control 0/38
failed vs L1cold/L2warm connector-attached 7/38, L3restart 38/38).
Fix
Gate the per-group hybrid lookup on the connector opting in:
Connectors that don't set the flag (every connector on this branch today,
including LMCacheMPConnector) fall back to
KVCacheCoordinator.get_computed_blocks()— the reconciled min-across-groupshit, which only resumes where every group's state, including the recurrent
one, actually exists. That is the same regime the clean L0 control ran under.
This mirrors upstream vLLM's own fix for this exact defect class —
vllm-project#48425 (commit
229e01e9e), which introducedsupports_divergent_local_hybrid_hitsand this same gate. Ourdev/gilded-gnosislineage predates that fix and carries neither the flag northe gate; a third party independently hit the identical bug on stock upstream
0.26.0 and cited vllm-project#48425 as their fix
(LMCache/LMCache#4247).
File touched:
vllm/v1/core/sched/scheduler.py, one hunk at theif request.num_computed_tokens == 0:block insideScheduler.schedule()(currently line 726 on
dev/gilded-gnosis @ fa033bd4e; +23/-0, comment +one
getattrcondition).Status: CPU-verified only. GPU behavioral proof pending.
What is verified (CPU, this session):
dev/gilded-gnosis @ fa033bd4e(git apply --check, then applied)vllm/v1/core/sched/scheduler.pypy_compilescleanWhat is NOT yet verified (explicitly, so this isn't mistaken for a proven fix):
— the four-arm ladder in
receipts/lmcache-reuse-test.jsonre-run with thisgate applied turns L1cold/L2warm from 7/38 failing to 0/38 — is a prediction
from the root-cause analysis, not a measurement.
the gate for treating this as behaviorally proven, not just CPU-clean.
Trigger condition / who this affects
Requires both: (a) a hybrid model (Mamba/GDN + full-attention layers) and
(b) an external KV connector attached. No currently running GG serving is
affected — the TB2.1 campaign's
sr1image and in-flight AIBoss passes usenative vLLM prefix caching only, no
--kv-transfer-config, no connectorattached.
Tradeoff
A GG deployment running nixl P/D with a hybrid model loses the FA-hit
optimization until GG rebases the upstream capability-flag plumbing so nixl's
connector can set
supports_divergent_local_hybrid_hits=True(nixl alreadyrestores recurrent state unconditionally today, so it stays correct, just
un-opted-in). Not applicable to this campaign, which does not use nixl.
DCO
Single commit, signed off:
Signed-off-by: Michel Belleau <michel.belleau@malaiwah.com>.References
receipts/lmcache-fix.json,receipts/lmcache-reuse-test.json,patches/gg-vllm-hybrid-divergent-hit-gate.patch(sha2565f9ad10bedff87c4f37ce61bfe9be5cc9ed84dce99d9d3685c54b0341529839f), all inhttps://github.com/malaiwah/qwen38-27b-exl3
Summary by CodeRabbit