Repository navigation
fix(offload): read PP/TP from parallel_config for LMCache world size - #2282
Merged
valarLip merged 1 commit intoSep 18, 2026
Merged
Conversation
lmcache_replica_world_size() read pipeline_parallel_size and
tensor_parallel_size off the top level of the config object. That is
correct for ATOM's own config, which carries them directly, but on the
vLLM plugin path `config` is a VllmConfig and both live under
config.parallel_config. Both getattr calls fell back to 1, so a TP4
replica reported world=1.
world=1 then puts the all-rank lookup scope out of range. With
LMCACHE_LOOKUP_SERVER_WORKER_IDS=0,1,2,3
the four per-rank lookup servers start normally, but the scheduler-side
client fails to build and dense/connector.py swallows it with a bare
`except Exception`, continuing with lookup_client=None. The server comes
up healthy and nothing in the run looks wrong, yet
ChunkedOffloadSchedulerBase can never learn lmcache_cached_tokens, never
builds a LoadSpec, and _reqs_need_recv stays empty: the offload tier is
written and never read.
Measured on MiniMax-M3 TP4, plugin backend, concurrency 40, 1 h window:
Retrieved 0 (native, same config: 1760)
Double unpin 0 (native: 878384)
Stored 3508 -- the save path is fine
vllm:external_prefix_cache_hits_total 0.0
vllm:external_prefix_cache_queries_total 1.3e+07
i.e. an "LMCache on" arm paying the full store cost for zero reuse. The
same build on the ATOM-native backend logs (world=4) and retrieves
normally, which is the direct proof that the config shape is what
differs.
This stayed hidden because the default lookup scope is [0], which is in
range even when world is wrongly 1 -- so arms that left
LMCACHE_LOOKUP_SERVER_WORKER_IDS unset built a rank-0 client and did
retrieve. It only fires in the "plugin backend + all-rank lookup" cell,
which is what the MiniMax-M3 agentic recipe prescribes at 48c.
The fix keeps reading the top level first and only falls back to
parallel_config, so ATOM's native path is bit-for-bit unchanged. Three
call sites are affected: dense, hybrid/dsv4 and mp/backend.
After the fix, the same arm logs (world=4) and the three discriminators
go non-zero: Retrieved 4 / Double unpin 476 /
external_prefix_cache_hits_total 30464.0, the last matching the first
"Retrieved 30464 out of 30464 required tokens" log line exactly.
Independently confirmed on a second model and a second connector.
GLM-5.2-MXFP4 TP4, vLLM plugin backend with ATOM's own
AtomLMCacheOffloadConnector, AgentX replay at concurrency 32, 64 GiB/rank
tier, LMCACHE_LOOKUP_SERVER_WORKER_IDS=0,1,2,3:
[atom] LMCache offload scheduler: lookup client on
atom-offload-dp0 (world=4)
which is the line the old code printed as (world=1). Over the run the
tier was read on all four ranks -- 50,048 tokens / 2.22 GiB per rank at
7.0-12.1 GB/s, and vllm:external_prefix_cache_hits_total ended at 50,048,
matching the Retrieved log lines exactly. That arm was run with the fix
already cherry-picked, so it is confirmation that world=4 is reached on a
second config shape, not a second before/after comparison.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
🏷️ CI GuideRuns automatically on every eligible PR before approval:
Heavy model tests:
|
Contributor
Author
|
Related: #2283 ports the slow-tier admission / lookup-skip gates from the same MiniMax-M3 line. Independent of this fix — different files, no dependency either way. |
PerryZhang01
changed the title
PerryZhang01
force-pushed
the
perzhang/lmcache-world-size-parallel-config-20260917
branch
2 times, most recently
from
September 18, 2026 08:20
0bb89f0 to
03c6944
Compare
PerryZhang01
changed the title
yhl-amd
approved these changes
Sep 18, 2026
valarLip
deleted the
perzhang/lmcache-world-size-parallel-config-20260917
branch
September 18, 2026 09:53
Contributor
|
Post-merge review: I did not find an additional correctness issue in this change. The helper preserves the native ATOM top-level PP/TP values and only falls back to |
PerryZhang01
pushed a commit
that referenced
this pull request
Sep 20, 2026
GLM-5.3 (GlmMoeDsaForCausalLM) serves on AtomLMCacheOffloadConnector with no code of its own. The mapping added for GLM-5.2 in #2231 folds `<p>.indexer.k_cache` onto `<p>.attn` by name and never reads a model identity, and GLM-5.3 registers the identical entries: measured at TP=4, `registered 78 layers` (not 99) with the same `(64, 576)` MLA and `(64, 132)` indexer tails. Every KV-relevant config field matches GLM-5.2 across GLM-5.3-MXFP4 and GLM-5.3-FP8. The only addition is `moe_router_dtype: float32`, which selects MoE routing precision; nothing under atom/plugin/vllm/kv_transfer or atom/kv_transfer/offload reads it. Tests: rename the layout module to test_vllm_glm_dsa_kv_cache_layout.py, since it covers the architecture rather than one release, and pin each checkpoint's KV config fields in `_MODELS`. The new test derives the byte geometry from those fields instead of restating 47,700, so a future GLM that moves kv_lora_rank or index_head_dim fails here rather than offloading at the wrong stride. A second test guards the invariant that made GLM-5.3 free -- the mapping takes no model or config argument -- which no other test in the file would notice being broken. Recipe: a short GLM-5.3 page that defers to the GLM-5.2 one for mechanism, sizing and gotchas, and records only what differs or was measured here: * pass `--max-model-len` alongside `--num-gpu-blocks-override`, or startup aborts on GLM-5.3's 1,048,576-token declared context * check that every rank built a live LMCache client (`worker_ids: [0,1,2,3]`), the signal that would have caught the pre-#2282 world-size collapse * two-pass restore: 159,936 of 160,200 pass-2 prompt tokens (99.84%) served from the tier, marker recall 8/8, 32 full `Retrieved` lines across all four ranks, zero alloc or load failures; both metric identities close exactly on the drained server * the arm is its own control: `prefix_cache_hits` is 0 over the whole run, so the restored tokens came through LMCache and nowhere else -- no tier-off arm needed * no throughput number, because a concurrent job held the other socket's DRAM and PCIe; GLM-5.2's +50.4% is the best estimate but is not GLM-5.3's measurement Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
valarLip
pushed a commit
that referenced
this pull request
Sep 21, 2026
… and fix the slot that would break it on main (#2291) * docs(offload): record GLM-5.3 on the LMCache byte-offload connector GLM-5.3 (GlmMoeDsaForCausalLM) serves on AtomLMCacheOffloadConnector with no code of its own. The mapping added for GLM-5.2 in #2231 folds `<p>.indexer.k_cache` onto `<p>.attn` by name and never reads a model identity, and GLM-5.3 registers the identical entries: measured at TP=4, `registered 78 layers` (not 99) with the same `(64, 576)` MLA and `(64, 132)` indexer tails. Every KV-relevant config field matches GLM-5.2 across GLM-5.3-MXFP4 and GLM-5.3-FP8. The only addition is `moe_router_dtype: float32`, which selects MoE routing precision; nothing under atom/plugin/vllm/kv_transfer or atom/kv_transfer/offload reads it. Tests: rename the layout module to test_vllm_glm_dsa_kv_cache_layout.py, since it covers the architecture rather than one release, and pin each checkpoint's KV config fields in `_MODELS`. The new test derives the byte geometry from those fields instead of restating 47,700, so a future GLM that moves kv_lora_rank or index_head_dim fails here rather than offloading at the wrong stride. A second test guards the invariant that made GLM-5.3 free -- the mapping takes no model or config argument -- which no other test in the file would notice being broken. Recipe: a short GLM-5.3 page that defers to the GLM-5.2 one for mechanism, sizing and gotchas, and records only what differs or was measured here: * pass `--max-model-len` alongside `--num-gpu-blocks-override`, or startup aborts on GLM-5.3's 1,048,576-token declared context * check that every rank built a live LMCache client (`worker_ids: [0,1,2,3]`), the signal that would have caught the pre-#2282 world-size collapse * two-pass restore: 159,936 of 160,200 pass-2 prompt tokens (99.84%) served from the tier, marker recall 8/8, 32 full `Retrieved` lines across all four ranks, zero alloc or load failures; both metric identities close exactly on the drained server * the arm is its own control: `prefix_cache_hits` is 0 over the whole run, so the restored tokens came through LMCache and nowhere else -- no tier-off arm needed * no throughput number, because a concurrent job held the other socket's DRAM and PCIe; GLM-5.2's +50.4% is the best estimate but is not GLM-5.3's measurement Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(offload): measure GLM-5.3-FP8 on the same offload arm The GLM-5.3 page reported one hardware arm (MXFP4) and carried GLM-5.3-FP8 in the test's `_MODELS` table on config evidence alone. Serve it and close that gap: FP8 at TP=4, same host, same seeded prompt set, differing from the MXFP4 launch only in the `online_quant_config` GLM-5.2-FP8 already uses. Every figure came out identical, which is what "quantization is a weight-side choice that does not reach the KV cache" predicts: * `registered 78 layers` with `(64, 576)` x78 and `(64, 132)` x21 on all four ranks -- byte-for-byte the MXFP4 and GLM-5.2 tails * two-pass restore 159,936 of 160,200 (99.84%), marker recall 8/8, `Retrieved` distributed 8/8/8/8 across Worker_TP0-3 * `prefix_cache_hits` 0, so the self-control argument holds on this arm too; both metric identities close exactly The prompt set is seeded, so both arms were handed the same 160,200 tokens -- the columns agreeing is a comparison, not a coincidence. `identical_text_pass2_ vs_pass1` differs (1/8 MXFP4, 4/8 FP8) and the recipe now says explicitly that this field is noise in both directions rather than a quality difference. Also add the per-rank `Retrieved` histogram as check 5 in the verification section. It is the stronger half of the world-size probe -- check 3 proves each rank built an LMCache client, check 5 proves each rank read through one -- but a zero histogram is underdetermined: a tier that is merely empty (early warmup, or an arm whose HBM hit rate leaves the connector only a short miss tail) looks identical to ranks that never built a client. Documented as such, because zero is not positive evidence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(offload): show what actually separates the MXFP4 and FP8 arms The FP8 section reported that every figure matched MXFP4 and left it there. Those figures cannot do the work that reading implies: layer count is fixed by the architecture, token counts by the seeded prompt set, and the Retrieved histogram by the hit structure, so all three are invariant under a change of weight dtype by construction. They would read identically if the second arm had never been restarted, which makes "identical" unable to witness that the arms differ -- and invites the much stronger reading that quantization was shown not to matter. Add the weight-side quantities from the same two logs, which do separate them: quantization=quark vs fp8, 112.46 vs 187.96 GB peak per rank, and the block counts vLLM sized before the override (48,709 vs 21,269). That last row is the point worth keeping. --num-gpu-blocks-override is what makes the KV comparison apples-to-apples, and it is also what erases the obvious witness: both arms then report a 524,288-token pool by construction. The witness survives one line earlier, in the pre-override sizing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(offload): separate peak from steady HBM in the arm-witness table The previous commit listed the loader's peak figure and then wrote that the 1.7x weight difference "carries over as" a 2.3x difference in blocks. Those are two quantities and the sentence chains them as one. A peak includes transients the loader releases; the block count is set by what is still resident when vLLM profiles. Here the two even point opposite ways -- steady consumed memory reads higher than the reported peak, because it is measured later and includes non-torch overhead the loader's counter never saw. Report both rows, labelled, and state the actual mechanism: the relationship is a subtraction, not a ratio. FP8 consumes 78.6 GiB more, so 78.0 GiB less is left for KV, and that difference is the entire 2.29x in blocks. The matching ratio was a coincidence of this pair of checkpoints. Also adds the per-rank "Available KV cache memory" row, which is the quantity the block count is computed from and was the missing step in the argument. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(offload): measured GLM-5.3 throughput, tier residency, and sizing The GLM-5.3 recipe said "No throughput number for GLM-5.3." It now has one, from a matched ON/OFF pair run back-to-back in the same slot on tree b6e22c479 (atom_dirty_lines=0), amd/GLM-5.3-MXFP4, TP=4, --num-gpu-blocks-override 8192, LMCACHE_MAX_LOCAL_CPU_SIZE=90, concurrency 8, 64-prefix pool, 28,672-token prefix + 4,096-token unique tail, 600 s, one seed for both arms: request throughput 0.5176 -> 0.6225 req/s +20.3% TTFT p50 4295.7 -> 3062.2 ms -28.7% prompt tokens served from the tier 0 -> 40.97% Limits are reported next to the numbers: the ON arm is n=1, the OFF arm's 0.54% repeat range must not be borrowed for it, and the two run-order asymmetries both favour OFF. Three other corrections fall out of the same pair: - `Staging buffers: 300 allocated (90.0 GiB, ...)` is NOT the KV tier. The OFF arm prints it on all four workers with LMCache entirely absent, and a tier=64 arm prints the same 90.0. It is ATOM MoE expert staging, and on a tier=90 run its value collides byte-for-byte with the tier setting. Added as a Trap, with RssAnon as the instrument that does measure the tier: F = 9.7 GiB/worker read directly off the tier=0 arm, tier residency (401.7-38.8)/4 = 90.7 GiB/rank against a declared 90.0. - Sizing no longer says "use LMCACHE_MAX_LOCAL_CPU_SIZE=180". The reusable working set is 64 prefixes x 1.2737 GiB = 81.5 GiB/rank and 90 ran it with zero `no memory is available`. The tier does not have to hold the run's whole byte traffic -- it evicts the cache-bust tails, which are never reused. Swept into the GLM-5.2 recipe, which carries the same 180. - `Retrieved > 0` is a property of (tree, HBM pool), not of the tree: the same healthy tree gives Retrieved=8 at --num-gpu-blocks-override 8192 and 0 at 16896. The zero-histogram paragraph now lists all three causes and names the discriminator for each, EngineDeadError for the third. The NUMA paragraph loses two overstatements: --membind is not a policy choice when TP x tier exceeds a node's free memory, and "pinned memory does not reclaim page cache" is too strong -- Cached fell 470.3 GiB during this run. Gate on F + TP x tier against measured per-node free memory. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(offload): give SeqView the slot the shared scheduler writes on a load `ChunkedOffloadSchedulerBase` is shared between the two paths. The native path hands it an ATOM `Sequence`, which has a `__dict__` and absorbs any attribute; the plugin path hands it `SeqView`, which is slotted and raises instead. #2154 (P/D disaggregation) added chunked_scheduler.py:341: seq.offload_load_start_tokens = hbm with no matching slot here. On the plugin path that raises `AttributeError` on the *first successful tier load* -- the one path the connector exists for, and one that no boot or smoke test reaches, so it survives CI. This branch predates that commit and does not crash today; it would the moment it lands on main. The fix is the slot, not the removal of the write: `scheduler.py:2031` reads the value back to decide where suffix prefill resumes, so line 341 is the only real producer. Initialised to `None` in `__init__` to mirror ATOM's own `Sequence`, because the reader uses `getattr(seq, ..., None)` and an unset slot would make "no load was emitted" and "the slot was never declared" the same observation. The regression test parses `chunked_scheduler.py` and asserts every `seq.<attr> = ...` in it is something `SeqView` can hold. Enumerating the writes from the source is the point -- a hand-written list of names would only re-encode today's bug, while this fails on the next such commit. Verified to fail on the combination it targets (origin/main's scheduler x the pre-fix `__slots__`) and to pass after. It also guards its own scanner: if a refactor renames the loop variable the scan finds nothing, so an empty result is an error rather than a pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(offload): name the total the token percentages are against aiperf's client-side total_usage_prompt_tokens is 13 lower than the server's vllm:prompt_tokens_total on both arms (the startup health-check request). The table's three components close against the server total, so a reader recomputing from the aiperf export would find the sums off by 13. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(offload): follow the scheduler's MRO instead of one hand-picked file The SeqView slot scan read `chunked_scheduler.py` and nothing else. That is already too narrow on the live plugin path: `ChunkedOffloadSchedulerBase` inherits `OffloadSchedulerMixin` from `_offload_common.py`, whose `_maybe_start_unaligned_handoff` writes `seq.offload_loaded_tokens` and `seq.offload_handoff_boundary_tokens` from a file the old scan never opened. Both happen to be slotted, so the scan was passing by luck rather than by coverage -- the failure mode a scan is supposed to remove. Derive the entry point instead of naming it: parse `connector.py` for the class it assigns to `self._scheduler`, resolve it through that file's own imports, then scan every module in its MRO that lives under `atom/`. Scope now follows the wiring -- add a dsv4 branch to the connector and `hybrid/dsv4/connector.py`'s five seq writes come into scope on their own (three are unslotted today) with no edit to this test. Controls: the scan now reports `_offload_common.py:728`, which the old one could not see; deleting that slot fails the test naming that file and line; and against main's own sources the pair behaves as it must -- main's writes with main's slots report `offload_load_start_tokens` missing, main's writes with this branch's slots are clean. The remaining assumption is stated in the docstring rather than left implicit: only writes whose base name is literally `seq` are seen. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(offload): guard the scope derivation, not just the write scan Deriving the scan's file set from the connector's wiring removed one silent failure and introduced another one layer up: if the wiring stops being statically resolvable -- a factory, a conditional, a rename -- the derivation yields nothing and every assertion below passes vacuously. That is the same "passes while checking nothing" shape the write-scan self-guard already covers, so give the derivation its own. Check a known writer by name rather than only that the set is non-empty: the MRO always contributes the scheduler's own module, so non-empty is nearly free and would not have caught a scope that quietly lost chunked_scheduler.py. Three controls, each failing with its own message: rewiring the assignment through a factory trips the entry-point guard; a path filter that matches nothing trips the empty-set guard; dropping chunked_scheduler.py from the walk trips the known-writer guard and prints what was scanned. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(offload): say what the residency sample counts are counting n=51 and n=42 are 10 s samples of a plateau, not independent witnesses: the OFF arm's spread of 0.00 GiB is itself the evidence, since 42 byte-identical readings are one value read 42 times. Left as-is, the counts read as replication and invite borrowing a dispersion that only bounds within-run sampler stability. Each arm is still n=1 in runs, as the throughput section already says of itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(offload): stop calling an unrepeated field "sampling noise" The paragraph argued that identical_text_pass2_vs_pass1 cannot be read as a regression, which is right, but reached it by calling the field "sampling noise in both directions" and citing a GLM-5.2 tier-off arm scoring 0/8. That borrows a dispersion across model, path and arm, and naming the source does not undo the borrow -- the sentence still executes it. The failure direction is toward more confidence in the null: "is noise" is a positive claim, while nothing here was repeated on GLM-5.3 at all. Keep the argument that actually carries the conclusion -- a prefix-cache hit is not bit-reproducible against a cold run, so the field cannot measure quality however often it is sampled -- and demote the GLM-5.2 datum to what it is: one demonstration that 0/8 is compatible with correct output. State plainly that the 1/8-vs-4/8 gap is unexplained rather than shown to be noise, and that marker recall, the criterion that does carry the claim, was measured here on both arms. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(offload): claim only the half the structural reason licenses The previous wording said this field "cannot measure quality in either direction". The structure it leans on -- a prefix-cache hit is not bit-reproducible against a cold run -- licenses only one half of that: text that differs is expected whether or not anything is wrong. An *identical* pass would still be weak evidence that nothing broke, so the field is not symmetric, and claiming it carries no information at all is a stronger statement than the reason supports, in the same over-confident-in-the-null direction as the borrowed dispersion this paragraph already dropped. State the half that holds, and say it is a half. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(offload): add the default-pool GLM-5.3 working point and how it was driven The throughput pair already in this file pins the HBM pool with --num-gpu-blocks-override 8192. That makes its KV comparison apples-to-apples, but 524,288 tokens is small enough that almost any reuse distance falls outside it, and it is not the pool a deployment gets. This adds a second matched ON/OFF pair on the same tree with the override dropped (vLLM sized 3,440,832 tokens), tier 256 GiB/rank, chunk 256, ATOM_SLRU, a 128-prefix pool and 1800 s per arm -- plus the aiperf invocation, so the method is reproducible rather than described. What the section has to say that a bare table would not: - The reuse distance has to be placed inside [hbm_pool, tier) *before* the run. Pool 64 gives 2,064,384 tokens, below the pool, so the tier's hit rate is zero by construction and a bigger tier cannot help. - The headline +4.94% is a function of the window. The tier needs ~650 s to fill, so the full-window number mixes a transient with a steady state; at t >= 600 s it is +22.54%. The OFF arm is flat across every cut (0.6721/0.6889/0.6888/0.6900 req/s), which is what licenses the start-cut. - There is no warmup phase, and the ramp is not what a warmup would fix: cold requests spread over the first 600 s by coupon collection, and the OFF arm never stops producing them because the working set does not fit HBM. - HBM hit rate is *not* what improved (61.13% OFF vs 60.14% ON). On the plugin path vLLM asks the connector only about the miss tail, so the two percentages have different denominators and must not be summed. - aiperf does not export a theoretical hit rate; its overall_usage_prompt_cache_read_pct is the measured combined rate. The ceiling column is computed here, and the stricter "the tail is cache-busted so it is never reusable" form is wrong -- it yields 78.69%, below the measured 82.55%, because --cache-bust first-turn-suffix leaves 252 of 1272 requests hitting the whole prompt. Each arm is n=1 in runs; no dispersion is quoted and the OFF arm's flatness bounds within-run drift, not run-to-run variance. The sibling heading is renamed to name its working point so the two pairs read as two configurations rather than one result and an afterthought. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * [Doc] Reorganize the GLM-5.3 LMCache recipe around the measured pair The recipe had grown into a record of the investigation. Cut it back to what someone deploying this needs: what GLM-5.3 is on this connector, the server line, the client line, and the numbers those two produced. - drop "Why nothing had to change" and the evidence sections that supported it (registration dump, two-pass restore, host residency) -- that material is the PR's argument, not the recipe's, and the mechanism it rests on is already written out in full in the GLM-5.2 recipe - replace the launch block with the configuration that was actually measured: no --num-gpu-blocks-override, utilisation 0.95, tier 256 GiB/rank, chunk 256, ATOM_SLRU, lookup ids left unset. The old block pinned the pool at 8192 blocks and a 90 GiB tier, which is not the pair reported below it - add the client line verbatim, with the four flags that are load-bearing (--random-seed, PYTHONHASHSEED, --use-server-token-count, --osl-stddev 0) and the reuse-distance arithmetic that fixes the pool at 128 - keep one working point: the 1800 s ON/OFF pair at the default pool. The earlier pinned-pool run is dropped rather than carried alongside it - GLM-5.2 recipe: retarget its #sizing anchor, which this reorganization moved Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: perzhang <perzhang@amd.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This was referenced Sep 24, 2026
Closed
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
4 participants
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.
lmcache_replica_world_size()collapses to 1 on the vLLM plugin path, so an all-rank lookup client never builds and the slow tier is written but never read. This PR only fixes that config-shape bug. Native ATOM is bit-for-bit unchanged.Applies cleanly on
1c4caef48. Depends on nothing outstanding (#2231 is merged).Problem
It reads
pipeline_parallel_size/tensor_parallel_sizeoff the top level of the config object:That is right for ATOM's own config, which carries them directly. On the vLLM plugin path
configis aVllmConfig, where both live underconfig.parallel_config. Neithergetattrfinds anything, both default to 1, and a TP4 replica reports world=1.Why that is not a cosmetic off-by-one
world=1puts the all-rank lookup scope out of range. With the scope the MiniMax-M3 agentic recipe prescribes at 48c:the four per-rank lookup servers start normally, but the scheduler-side lookup client fails to build.
dense/connector.pycatches that with a bareexcept Exceptionand continues withlookup_client=None:— with an empty exception message. The server then comes up healthy and nothing about the run looks wrong, but
ChunkedOffloadSchedulerBasecan never learnlmcache_cached_tokens, never builds aLoadSpec, and_reqs_need_recvstays empty. The offload tier is written and never read, for the whole run.Measured on MiniMax-M3 MXFP4, TP4, plugin backend, concurrency 40, 1 h window:
RetrievedDouble unpinStoredvllm:external_prefix_cache_hits_totalvllm:external_prefix_cache_queries_totalAn "LMCache on" arm paying the full store cost for zero reuse. The direct proof that the config shape is what differs is the log line itself — same build, two backends:
Storedproceeding normally is not evidence the tier works: the save path does not use the lookup client. The discriminators areRetrieved,Double unpinandexternal_prefix_cache_hits_total.Why it stayed hidden
The default lookup scope is
[0], which is in range even when world is wrongly 1. Every arm that leftLMCACHE_LOOKUP_SERVER_WORKER_IDSunset built a rank-0-only client and did retrieve. The bug only fires in the cell "plugin backend + all-rank lookup" — which is exactly what the MiniMax-M3 agentic recipe prescribes for 48c, where it reports the all-rank scope taking the prompt cache hit rate from 82.86% to 95.48%. That improvement is not reproducible on the plugin path without this fix.Fix
Read the top level first and fall back to
parallel_config. The ordering matters: ATOM's native config always has correct top-level values, andVllmConfighas nothing there, so this is strictly additive — the native path is bit-for-bit unchanged, including the case whereparallel_configexists but only carries ranks (ATOM's own shape), which must not shadow the top level.The helper lives in
atom/kv_transfer/offload/config.py. Call sites indense/,hybrid/dsv4/andmp/backend.pypick up the new value with no further change.Verification
test_replica_world_size_reads_vllm_parallel_configcovers the vLLM shape (TP4 → 4, PP2×TP4 → 8) and re-asserts that an ATOM-shapedparallel_configcarrying only ranks does not shadow the top level. It fails on main withassert 1 == 4and passes with the fix.(world=4), all four lookup servers come up, and the three discriminators go non-zero —Retrieved4 /Double unpin476 /external_prefix_cache_hits_total30464.0, the last matching the firstRetrieved 30464 out of 30464 required tokenslog line exactly.AtomLMCacheOffloadConnector, AgentX replay at concurrency 32, 64 GiB/rank tier,LMCACHE_LOOKUP_SERVER_WORKER_IDS=0,1,2,3. Boot logslookup client on atom-offload-dp0 (world=4)— the line the old code printed as(world=1)— and the tier is read on all four ranks: 50,048 tokens / 2.22 GiB per rank at 7.0–12.1 GB/s, withvllm:external_prefix_cache_hits_totalending at 50,048, matching theRetrievedlog lines exactly.On that GLM-5.2 arm the tier supplied only 0.0529% of prompt tokens. That is a structural limit of the plugin path (vLLM asks the connector only about the HBM-miss tail) and is not something this fix addresses. Confirmed bit-exact in that run by
prefix_cache_queries - prefix_cache_hits = 95,172,276 - 16,337,856 = 78,834,420 = external_prefix_cache_queries. This fix makes the tier reachable at all; how much it can then supply on the plugin path is a separate problem.Not in this PR
except Exceptionindense/connector.pythat turned a fatal misconfiguration into a silent write-only tier. Out-of-range lookup scope should arguably fail the launch rather than warn with an empty message.OFFLOAD_SAVE_ADMISSION,OFFLOAD_LOOKUP_SKIP_UNLOADABLE). Those skip lookups whose answer cannot change the current request, but they leave the save floor at the HBM frontier and therefore starve the slow tier when the HBM pool is tighter than the working set (MiniMax-M3, 16612 blocks: −3.8 throughput points). If that tuning is still wanted as a default-off experiment, it belongs in perf(offload): port the slow-tier admission and lookup-skip gates to main #2283, not here.Tests
tests/test_lmcache_offload_connector.py: newtest_replica_world_size_reads_vllm_parallel_config..github/scripts/run_unit_tests.shpasses--ignore=tests/plugin, sotests/plugin/is not covered by CI unit tests.