Skip to content

feat(offload): support native DSv4 checkpoints with LMCache MP - #2250

Merged
valarLip merged 29 commits into
mainfrom
feat/dsv4-lmcache-mp
Sep 28, 2026
Merged

valarLip merged 29 commits into
mainfrom
feat/dsv4-lmcache-mp

Conversation

@yhl-amd

@yhl-amd yhl-amd commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Add native DSv4 PAGE/STATE checkpoint support through a standalone LMCache multiprocess server.

The integration saves and restores PAGE KV together with the matching recurrent STATE checkpoint while active requests retain fixed SLOTs. State sources are represented by immutable checkpoint PAGE units, and each emitted save acquires an independent checkpoint lease before request teardown. Restore reserves fresh PAGE units and copies the native checkpoint image back into the request SLOT before the request is allowed to resume.

Sparse STATE lookup requires a complete checkpoint at the same boundary as PAGE KV. Full-prompt lookup truncates tokens before querying. ATOM uses the same server-wide absent marker for every registered group: PAGE IDs are nonnegative, including valid block 0, while missing STATE entries are -1. Start the matching LMCache server with --null-block-id -1 --separate-object-groups. Native layout/model revision namespaces and exact operation generations reject incompatible or stale transfers. Uncertain DMA completion retains its lease.

The partial PAGE-release path remains fail-closed for recurrent-state requests. Only the native-state MP scheduler explicitly opts in after it is bound to the PAGE checkpoint coordinator; connectors without that guarantee keep the whole request deferred.

Scope

This PR is focused on the LMCache MP path. The existing standalone DSv4 and Kimi LMCache connector behavior is intentionally unchanged.

Native MP keeps a simple max_pending_saves safety bound. Value-ranked admission, PAGE pin-ratio budgets, candidate replacement, and the related policy metrics are not included. If those policies are needed later, they should be designed specifically around the MP path instead of changing legacy connectors.

Dependencies

Main implementation

Start with the "LMCache Multiprocess (lmcache_mp)" section of atom/kv_transfer/offload/README.md for launch configuration, lifetime rules, and supported scope. The primary implementation is in:

  • atom/kv_transfer/offload/mp/connector.py
  • atom/kv_transfer/offload/mp/native_state_scheduler.py
  • atom/kv_transfer/offload/mp/native_state_worker.py
  • atom/kv_transfer/offload/mp/native_state_layout.py

Cleanups on this branch:

  • Every ATOM-owned offload knob (OFFLOAD_*, LMCACHE_MP_TRANSFER_MODE) is read through atom/utils/envs.py; kv_connector_extra_config overrides still take precedence.
  • Adopted transfer units are published by PageUnitCheckpointStore.adopt_units; chunk ranges and PAGE source-safe completions come from shared helpers in mp/backend.py; _checkpoint_descriptor_buffer lives once on AttentionMetadataBuilder; native-state budget charges and refunds are paired; finished requests carry an _offload_finished flag instead of a copied block table.
  • A backend whose state_transfer() copies PAGE-backed checkpoints but publishes no paged_state_checkpoint_spec / execute_paged_state_copies (Kimi K3 today) is refused at lmcache_mp registration, instead of running a PAGE-only worker under a native-state scheduler whose lookups never hit.
  • The README's MP server example passes --eviction-policy LRU, which the pinned LMCache requires (reported by @kvnloo).

Validation

  • full repository Black check
  • Ruff on all changed Python files
  • Python syntax compilation
  • CPU/unit tests selected by offload, LMCache, DSv4, Kimi, scheduler, checkpoint, GDN and attention keywords, rebased tree vs main at 68e0df5 in rocm/atom-dev:nightly_202609161445: 3452 passed; no test fails only on this branch (one main failure, the SeqView slot scan, passes here).
  • DSv4-Pro TP8 end to end on 8×MI355X at 62893d7 with the pinned LMCache 0.5.6.dev98, in three passes: cold (empty HBM and LMCache), restore (fresh ATOM, same LMCache L1), and control (fresh ATOM, empty LMCache). Restore loaded 460,032 tokens from LMCache (offload_hit 0.86, 0 load failures, 352/352 retrieves finished); an 18,095-token prompt restored 0:17920 across the 8192 and 16384 checkpoints. GSM8K (64-shot, 50 samples) scored 0.94 on all three passes, and cold-vs-restore answers matched 47/50, the same as the cold-vs-control floor (FP8 with dynamic batching is not bit-reproducible run to run).
  • Keep the LMCache server's default worker reap/registration-grace settings. With --worker-registration-grace-seconds 30, the restarted workers were reaped before their first heartbeat, and every restore silently fell back to prefill while accuracy still looked normal.

Logit-level equivalence and performance measurements remain outstanding.

@yhl-amd yhl-amd changed the title feat(offload): support native DSv4 checkpoints with LMCache MP feat(offload): add budgeted native DSv4 checkpoints with LMCache MP Sep 22, 2026
@yhl-amd
yhl-amd changed the base branch from fix/dsv4-save-admission to main September 22, 2026 07:03
@yhl-amd yhl-amd changed the title feat(offload): add budgeted native DSv4 checkpoints with LMCache MP feat(offload): support native DSv4 checkpoints with LMCache MP Sep 22, 2026
@yhl-amd
yhl-amd marked this pull request as ready for review September 24, 2026 16:20
@kvnloo

kvnloo commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

While tracing the native MP path, could we add --eviction-policy LRU to the existing Running the MP server example?

In the pinned LMCache 05fc77a, ServerCommand.add_arguments() includes add_storage_manager_args(), which requires --eviction-policy; the command currently omits it. This is a source-level finding, I haven’t run the full CLI or GPU path.

I also noticed the PR description still points to mp/README.md, while the guide is now in the parent offload README. Would a one-line recipe correction plus updating that pointer be the right scope? I’d keep the topology, checkpoint and lifetime design unchanged.

AI-assisted source review and drafting; no GPU results claimed.

kvnloo commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

Follow-up with a one-line patch and real-parser validation.

Against ATOM 9c64bea and LMCache 05fc77a, the exact README arguments reject with exit 2 for missing --eviction-policy. Adding only --eviction-policy LRU parses successfully; removing it restores the same rejection. The existing host, port, chunk size, null-block marker, group separation, transfer mode and memory-size argument remain unchanged.

The check uses the unmodified ServerCommand argument parser and real configuration modules, with the required CPU native extensions built from the pinned source. It never calls execute(), starts a server or runs a GPU test. The evidence summary retains the initial environment failure separately from the successful comparison.

The patch is just the README line for your existing branch—none of the downstream experiment machinery. LRU is an explicit choice for this example, not a runtime-default or policy change. No additional testing requested from you for this finding.

AI-assisted preparation and evidence review; no GPU results claimed.

Save and restore DSv4 PAGE KV together with the matching recurrent STATE
checkpoint through a standalone LMCache multiprocess server. The lmcache_mp
connector selects its implementation from backend capabilities: backends
that publish a PagedStateCheckpointSpec and execute_paged_state_copies use
the native PAGE/STATE path, others keep the PAGE-only transport. Saves
lease immutable READY checkpoint PAGE units instead of snapshotting the
live SLOT; restores reserve fresh units and adopt them as a local READY
checkpoint.

Also:
- allow single-host DP and DP-attention, scoping MP sessions per replica;
- release source PAGE blocks per chunk and reacquire a finished request's
  still-resident prefix at save admission;
- allow partial release of recurrent-state requests only when the
  connector guarantees an independent state lease;
- read offload env knobs through atom.utils.envs;
- refuse PAGE-copied state backends that lack the native contract.

Requires LMCache dev@05fc77a (LMCache/LMCache#5132), pinned on main by
#2395. Start the server with --null-block-id -1 --separate-object-groups.

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Honglie Yi and others added 2 commits September 26, 2026 03:24
The pinned LMCache (05fc77a) requires --eviction-policy on `lmcache server`;
without it the documented command exits 2 during argument parsing.

Reported-by: kvnloo
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Native MP stored every READY checkpoint boundary, including whole short
prompts, although a prefix below OFFLOAD_MIN_LOAD_TOKENS (8192 by default,
equal to OFFLOAD_MIN_SAVE_TOKENS) can never be loaded back. On DSv4-Pro TP8
with a 1K/1K workload at concurrency 64, offload stored 257 such prefixes and
cost 12.4% output throughput; skipping them stores none and leaves 1.6-2.2%,
within run-to-run noise. 16K prompts still save as before.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@valarLip

Copy link
Copy Markdown
Collaborator

Review — native DSv4 checkpoints with LMCache MP

Reviewed ad850b5ee against merge base 68e0df5e7 in a detached worktree: 44 files, +4888/−447.

The native-state path is a real capability and the checkpoint/lease accounting around it is careful work — _charge_state_image, the PAGE/STATE split in the layout builder, and the deferred-free protocol all read like someone who has thought about the lifetimes. Two things concern me, and they are of different kinds.

The first is a boundary:

The mechanism this PR deletes worked in both harnesses; the mechanism that replaces it is ATOM-engine-only. Early block release was previously safe because protected_block_ids leased the unemitted final save's source blocks and _offload_finished_block_ids froze the table. Both are gone, replaced by a reacquire-at-admission path that needs an ATOM BlockManager — which does not exist on the vLLM plugin side, where early block release is the thing that actually runs.

The second is a shape that recurs:

Four separate defects are one change applied to some but not all of a set of siblings — a signature swept to two of three implementations, a dtype fix swept to one of two halves of the same registration, a DP guard relaxed without the namespace that made it safe, and one env var read in two different units by two schedulers that cite the same justification for it.

Provenance: [verified] means I read the deciding lines in the PR-head worktree myself. [reported] means the shape matches the code but I did not trace it end to end.


1. The late-save path cannot run on the vLLM plugin, and is silently wrong there instead of loudly [verified]

These are two findings that only make sense together, because the second is currently masking the first.

protected_block_ids no longer protects the [saved, aligned) suffix. Its docstring is explicit about this being deliberate:

"This includes a final save that has not been emitted yet: request teardown freezes its token identity and computed frontier, but protects no physical PAGE until admission reacquires the still-canonical prefix."

So the whole safety argument rests on the reacquire at chunked_scheduler.py:582:

if getattr(seq, "_offload_finished", False) and not seq.block_table:
    late_source = self._late_save_source(seq, saved, aligned)
else:
    block_ids = list(seq.block_table)

On the plugin path not seq.block_table is never true. SeqView.block_table is cleared in exactly one place — reset_for_preemption (plugin/vllm/kv_transfer/seq_view.py:111) — and _seqs.drop() (:153) only pops the registry entry. A finished request keeps a populated table, so the else branch runs list(seq.block_table) over blocks vLLM has already freed and handed to other requests. The store then writes another request's KV under this request's token hashes, and a later prefix hit serves it. No crash, no log line, wrong tokens.

Now the masking. If that gate were fixed, the request would reach _late_save_source, whose first line is:

if self._block_manager is None:
    raise RuntimeError("late offload save requires a bound block manager")

grep -rn bind_block_manager atom/ has zero hits under atom/plugin/; the only production caller is model_engine/scheduler.py:626. plugin/vllm/kv_transfer/connector.py:292 constructs DenseOffloadScheduler(self._config) directly, bypassing the offload/connector.py:180 shell that would forward the bind. So _block_manager is None for the process lifetime and the fix turns a silent corruption into a hard RuntimeError out of the vLLM scheduler step.

This is not a missing wire. _late_save_source unconditionally calls self._block_manager.acquire_offload_prefix(...) and free_leased_blocks(...), and there is no ATOM BlockManager on the plugin side to bind — vLLM owns the blocks. The mechanism has no implementation there.

Worth deciding before merge: either the plugin keeps a lease-based protection (what was deleted), or _supports_early_block_release is forced False on that path. Note tests/test_offload_early_block_release.py only covers the ATOM path, where BlockManager.deallocate does clear block_table, and tests/plugin is --ignored by .github/scripts/run_unit_tests.sh — so no CI shape can reach this.

2. Both new hash-chain walks seed from a hardcoded -1 instead of seq.cache_seed [verified]

native_state_scheduler.py:168 and block_manager.py:2572 (acquire_offload_prefix) both start the chain at -1. Every other chain walk in the codebase seeds from the sequence:

site seed
native_state_scheduler.py:168 chain[-1] if chain else **-1**
block_manager.py:2572 parent_hash = **-1**
block_manager.py:864 chain[-1] if chain else **seq.cache_seed**
block_manager.py:1584 return **seq.cache_seed**

The comment directly above the first one says "Use its exact public hashing algorithm and token slices" — the algorithm and the slices were copied; the seed was not.

The blast radius is precise: sequence.py:268 sets cache_seed = -1 and only overwrites it when multimodal_data is not None, and compute_hash mixes the prefix in only when prefix != -1. So the two are equivalent for text requests and diverge from block 0 onward for multimodal ones. On a multimodal DSv4 request self._checkpoints.contains(self._boundary_hash(...)) never matches, every native save is silently skipped, and adopt_transfer_units(op, prefix_hash) publishes a restored checkpoint under a key no loader will ever look up. In acquire_offload_prefix the same seed makes kv.lookup(block_hash) miss block 0, so the late save degrades to "nothing resident".

This is the live path, not a corner: page_unit_checkpoint.py:849 declares readable_midstep = False, and block_manager.py:1967 is if not self.state.readable_midstep or ...: seq.block_hashes = [] — for exactly the model family this PR targets, the empty-chain fallback is the normal case. _boundary_hash's two branches also use different seeds for the same boundary and are only accidentally equal today. No offload test uses a multimodal sequence, and acquire_offload_prefix's one test reference (test_offload_early_block_release.py:385) is a lambda stub.

3. A failed submit() wedges the engine permanently, and every recovery path is switched off [reported]

native_state_worker.py:255 catches, logs, and returns — leaving the pending entry holding _UncertainSubmission, whose query() is return False. The operation can never become terminal, so _complete_native_save — the only caller of _refund_state_image and settle_offload_store — is never reached.

What makes this worse than a leak is that all four escapes are deliberately disabled: abandon_save returns None (:511), reclaim_stale_leases returns [] (:516), the pin was taken timeout_reclaimable=False (page_unit_checkpoint.py:600) so reclaim_stale_offload_pins skips it, and _reconcile_stalled_deferred_saves exits on its first line because save_abandon_timeout_s() is 0.0 (backend.py:1137, scheduler.py:1276).

One ZMQ hiccup or LMCache server restart therefore costs, permanently: units_per_checkpoint PAGE units pinned, _pinned_state_bytes charged (after _max_pending_saves such events _has_state_budget() is False and all native saves and loads stop being admitted), the request's blocks never freed because should_defer_free stays True, and has_pending_work() stuck True so EngineCore busy-loops over idle GPUs. len(pending) also never drops, so the worker's own bound at :219 starts rejecting every subsequent save.

tests/test_lmcache_mp_native_worker.py:208 asserts the lease is retained — so the wedge is the tested contract. Retaining the lease is right; what is missing is any bounded path out of it.

4. OFFLOAD_MIN_SAVE_TOKENS is read in two different units [reported]

The base scheduler compares it against the save increment; the native scheduler compares it against the absolute boundary:

# chunked_scheduler.py:453
if target - saved < self._min_save_tokens:          # increment
# native_state_scheduler.py
floor = max(exhausted, self._min_save_tokens - 1, 0)  # absolute

Both cite the same justification, and it is only true for the absolute reading. At the default 8192, a 40k-token request that has already saved 32k finishes with a sub-8k remainder; the base scheduler frees the reacquired blocks and pops the request from _save_tracker, so the tail of every long request is silently never persisted and later hits stop 32k in — while the native scheduler on the same knob would have accepted that boundary. Pre-PR the base had no _min_save_tokens at all and emitted whenever aligned > saved, so this also silently changes dense/hybrid behaviour; the README still documents the threshold only under "Native MP" while it now lives in the shared base.

The same knob has a second failure at the other end. With OFFLOAD_MIN_SAVE_TOKENS=0 (reachable — envs.py clamps with max(0, ...)) and a fully-evicted prefix, acquire_offload_prefix breaks on the first block, so target == saved, the early-out is skipped, and a zero-length save is emitted forever: entry[1] = aligned leaves the watermark unchanged and the retirement _save_tracker.pop(sid) at :585 only runs when late_source is None, which now never happens. On the native path the same shape is worse — _build_save_request calls _boundary_hash(seq, aligned=0), which raises ValueError("native checkpoint boundary must align to hash blocks") straight out of build_connector_meta.

Both new fixtures set OFFLOAD_MIN_SAVE_TOKENS=0 (test_offload_early_block_release.py:47, test_lmcache_mp_native_scheduler.py:83), so the default-valued behaviour in the first half is never exercised and the fully-evicted case in the second half is never reached.

5. Three sweeps that stopped short

descriptor_slot was added to two of three implementations [reported]. backends.py:341 declares execute_paged_state_copies(self, store_ops, restore_ops, descriptor_slot: int = 0); deepseek_v4_attn.py:968 and gdn_attn.py:815 were updated; deepseek_v41/backend.py:263 still has def execute_paged_state_copies(self, stores, restores). The native worker always calls it with the keyword (native_state_worker.py:285-295), so the moment DSv4.1 is reached the TypeError lands inside _begin_restore's except Exception and is reported as "LMCache MP native restore failed" rather than failing loudly.

The uint8 fix was applied to one of two halves of the same registration [reported]. backend.py:433 now registers view.view(torch.uint8) with a comment explaining that LMCache's ROCm raw-pointer fallback "cannot express FP8 through the CUDA array interface". native_state_layout.py:162 hands tensors = list(page_views) to the same register_kv_caches in native dtype — while the STATE aliases built eleven lines below do retype via page_view.view(torch.uint8). A native-state model publishing FP8 block views hits the bug the sibling path just fixed: wrong bit patterns after a cache hit, not a crash.

The DP guard was relaxed without the namespace that made it safe [reported]. The blanket if dp_size != 1 or enable_dp_attention: raise NotImplementedError("TP-only") became a multi-node-only if dp_size_local != dp_size:. But _model_namespace hashes only model/layout/checkpoint with no DP rank, _parallel_strategy returns worker_id = rank_in_group // replication so every replica registers worker_id 0, and _server_urls returns the one configured URL for all of them. Four processes register four different sets of GPU KV tensors under an identical (model_name, worker_id, world_size) identity. Only _mp_session_id was DP-scoped, which fixes per-request routing but not the registration namespace. The deleted cases are the tell: ({"dp": 2}, "TP-only") and ({"enable_dp_attention": True}, "TP-only") were removed from tests/test_lmcache_mp.py, and enable_dp_attention no longer appears anywhere under offload/mp/.

6. Two selectors that encode the wrong predicate [reported]

is not None is standing in for "enabled". mp/connector.py:144 picks the native-state scheduler whenever block_manager.paged_state_checkpoints is not None. But block_manager.py:240 constructs that coordinator with enabled=self.enable_prefix_caching and self.num_state_slots > 0 — the attribute is non-None regardless, and enabled gates only applies() (page_unit_checkpoint.py:893); acquire_checkpoint_source and reserve_transfer_units never consult it. DSv4 served with prefix caching off therefore binds the native scheduler, whose _save_frontier queries a store that can never hold a READY image, and no offload happens at all where the PAGE-only scheduler would have worked. tests/test_lmcache_mp_shell.py only ever passes object() or None, so the disabled case has no coverage.

can_partially_deallocate_state is fail-open while the safety net it cites is fail-closed. multi_connector.py:656 returns True if any sub says True; protected_block_ids, sixteen lines above, returns None if any sub cannot answer. With a native-state MP sub plus a dense PAGE sub both deferring on the same finished sequence, the composite says True and the state slot is recycled while the non-declaring sub is still deferring. The docstring's claimed mitigation does not reach it: protected_block_ids narrows PAGE blocks only, and a dense sub legitimately returns frozenset() there while still needing the request alive. tests/test_multi_connector.py:291 passes under either ANY or ALL, so it has no discriminating power here.

7. Lifetimes and ordering [reported]

The MP session is ended before the late save is submitted. This PR sets _supports_early_block_release = True on the MP scheduler, where the base had it False precisely so no save could outlive request_finished. Now: request_finished → end_session(_mp_session_id(config, seq.id)); then a later build_connector_meta reacquires the prefix and emits the final save; then the worker submits it under that same, already-ended session id. Either the server drops the store — losing the end-of-prompt checkpoint, the most valuable one and the one a later full-prompt lookup depends on — or it resurrects a torn-down session. The native scheduler's own request_finished (:465) defers only when a dispatched load lease exists, not for a pending save.

The restore side stream is never fenced against the compute stream. grep -E 'wait_stream|wait_event|record_stream' across atom/kv_transfer/offload/mp/ and deepseek_v4_attn.py returns nothing. The same execute_paged_state_copies runs on the compute stream from AttentionMetadataBuilder.build() and on self._restore_stream from _begin_restore on the connector thread, with no handshake. Read-after-write is safe (the worker host-polls restore_event.query()), but write-after-read is not: relocate_state_slots runs on the compute stream from batch.state_maintenance_ops.relocations and can move slots while a parked restore is in flight, and the previous occupant's still-enqueued decode work is unfenced. Only suspend_queued_restore removes one competitor. Symptom: non-deterministic garbage after a cache hit that vanishes under HIP_LAUNCH_BLOCKING=1. tests/test_lmcache_mp_native_worker.py:360 monkeypatches torch.cuda.stream to a no-op, so it pins the descriptor-slot plumbing and enshrines the missing sync.

DSv4 + MTP cannot start. deepseek_v4_attn.py:1986 validates sum(region.unit_bytes) == checkpoint_spec.page_unit_bytes over the target's regions; model_runner.py:2107 then extends block_regions and block_tensor_views with the draft's; native_state_layout.py:159 re-sums over all regions and raises ValueError("PAGE regions do not cover the native PAGE unit") inside register_kv_caches. Were the check looser, the per-ordinal image loop would fold draft KV rows into the checkpoint image. Separately, line 2117 gcds tp_replication_factor down to 1 but leaves native_state_tp_replication_factor at tp_size, so two declarations describing the same physical object disagree and are validated independently. The PR's e2e was DSv4-Pro TP8, presumably with no draft builder.

A pin and a budget charge leak on the exception path. native_state_scheduler.py:258 charges the state budget and takes the checkpoint PAGE lease, then calls super()._build_save_request(...) and builds NativeStateTransfer with no try/except around the remainder. If anything after the acquire raises, the caller at chunked_scheduler.py:594 frees only the late-acquired KV blocks and re-raises; nothing ever reaches release_offload_store_source / settle_offload_store / _refund_state_image, because _native_saves[operation] was never written. The pin is timeout_reclaimable=False, so the reclaimer skips it forever: one whole checkpoint image leaves the pool for the process lifetime. _charge_state_image is carefully exception-safe for the acquire itself, but its unwind scope ends at acquire(). Same shape at _decide_load_after_alloc:350-369, where reserve_transfer_units precedes a suspend_queued_restore that can assert and a _boundary_hash that can raise.

8. Cleanliness, cost, docs

  • _save_frontier now runs an O(prompt/chunk) hash-and-lookup scan per tracked request per scheduler step.
  • Descriptor slots ≥1 lazily allocate pinned memory on the connector thread mid-serving, while warmup_per_req_cache warms only slot 0 — contradicting that function's own docstring.
  • _begin_restore's exception path leaks a descriptor slot.
  • 15 new env vars are absent from docs/environment_variables.md.
  • chunked_scheduler.py:1162 still cites the "frozen block table" this PR deleted.

9. The tests encode several of these defects as the contract

This is the part I would act on first, because it is why the PR is green:

test what it pins
test_lmcache_mp_native_worker.py:208 asserts the lease is retained — makes §3's permanent wedge the contract
test_lmcache_mp_native_worker.py:360 torch.cuda.stream monkeypatched to a no-op — pins §7's missing sync
test_multi_connector.py:291 passes under ANY or ALL — no discriminating power for §6
test_lmcache_mp_shell.py only object() or None — §6's disabled case uncovered
both new fixtures OFFLOAD_MIN_SAVE_TOKENS=0 — §4's default behaviour never runs
run_unit_tests.sh --ignore=tests/plugin §1 is unreachable by CI by construction

None of these are wrong tests. Each asserts something true about the code as written; the gap is that the thing asserted is the symptom.


Checked and not upheld

Recorded so they are not re-raised: a suspected KeyError in _new_load_operation and a suspected stride collapse in _build_cache_views are both properly guarded, and a suspected late-save lease leak with _early_release off is unreachable — _offload_finished is only stamped under early release.

Honglie Yi and others added 4 commits September 27, 2026 03:44
…e threshold

Review follow-ups (#2250 §1, §4).

- Unbound schedulers (the vLLM plugin, where vLLM owns the blocks) cannot
  reacquire a finished request's prefix, so teardown again freezes the block
  table and leases the unemitted suffix, and the final save reads exactly
  those leased blocks. The late-save reacquire path now runs only when a
  BlockManager is bound, instead of keying off an empty block table that the
  plugin never clears.
- The shared base no longer reads OFFLOAD_MIN_SAVE_TOKENS, so dense and hybrid
  behave as before this PR. A late save with nothing savable resident now
  retires the request instead of emitting an empty save forever.
- Native MP applies OFFLOAD_MIN_SAVE_TOKENS to the absolute boundary for
  normal and late saves alike, so a long request's short tail is still
  stored, and the floor can never reach boundary 0.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Review follow-ups (#2250 §3, §7).

- A submission that raises, a restore that raises, or a restore event whose
  query raises may still be touching engine memory, so its lease is kept -- but
  no longer forever. _UncertainSubmission now fails the transfer after
  lmcache.mp.uncertain_transfer_timeout_s (default: twice lmcache.mp.mq_timeout)
  with a warning, so the save or load settles and its lease, budget, pending
  slot and descriptor slot are released. Before, one ZMQ hiccup could stop all
  native saves and loads and keep EngineCore busy-looping.
- PAGE-only MP treats a raising save submission the same way instead of as a
  definite failure. The failure reported no quiescence, so under #2339 the save
  was never retried and its lease was never released. The now-dead
  _immediate_save_failures set is removed.
- Native restores are fenced both ways: the restore stream waits for work
  already on the compute stream (write-after-read on the SLOT's previous
  occupant), and every step's compute stream waits on in-flight restore
  events (read-after-write, including SLOT relocations in build()). The fence
  is on the GPU, so the host does not block.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ssions

Review follow-ups (#2250 §7).

- The MP scheduler ended a request's session in request_finished, but with
  early release its final save is emitted and submitted under that session
  afterwards. Session ends are now deferred until no save of the request is
  tracked or in flight. They are swept on retirement and at every metadata
  build.
- Native save admission returns the checkpoint lease and budget charge if
  anything after the acquire raises; the pin is never timeout-reclaimable, so
  it used to leak for the process lifetime. Load admission computes the
  boundary hash before reserving units, and releases the units, the budget
  and any suspended local restore if the rest raises.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Review follow-ups (#2250 §5, §7).

- DSv4 now publishes paged_state_region_count, the number of its own PAGE
  regions. A draft with a pool of its own appends regions after them, and the
  native layout validates checkpoint coverage and builds STATE aliases from
  the leading regions only. Draft rows are registered as ordinary PAGE KV and
  never folded into the checkpoint image. Before, DSv4 with such a draft
  failed registration with "PAGE regions do not cover the native PAGE unit".
- Auto rank collapse is off when a DSpark draft's backend owns a KV pool:
  that draft publishes per-rank PAGE regions, so the complete PAGE object is
  not replicated and registration would otherwise reject the requested
  collapse.
- The native layout registers its PAGE group as uint8 views, like the
  PAGE-only path and like its own STATE aliases. LMCache's ROCm raw-pointer
  fallback cannot express FP8 through the CUDA array interface.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Honglie Yi and others added 2 commits September 27, 2026 04:07
…ts early

Review follow-ups (#2250 §5, §8).

- Native _save_frontier walked the prompt checkpoint by checkpoint for every
  tracked request on every scheduler step. PageUnitCheckpointStore now has a
  generation that is bumped whenever the READY set can change, and the answer
  is cached per request on (frontier, floor, generation).
- Restore descriptor slots >= 1 were first allocated as pinned memory on the
  connector thread mid-serving. The native worker now reserves them at
  registration through the new builder hook reserve_checkpoint_descriptors
  (base builders allocate their descriptor buffers, DSv4.1 its per-slot
  staging).
- Document why single-host DP replicas share one (model_name, worker_id,
  world_size) identity. It is the content-addressed storage namespace, while
  the server registers GPU memory per instance_id, refcounts layout
  descriptors per (model_name, world_size), and _mp_session_id scopes
  sessions per replica.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Review follow-ups (#2250 §8, §9).

- docs/environment_variables.md listed only OFFLOAD_MAX_PENDING_SAVES and the
  LMCache pin timeout, and still said they bypass atom.utils.envs. All 16
  ATOM-owned offload knobs are now documented with type, default, precedence
  and invalid-value behavior. A new test fails if an OFFLOAD_*/LMCACHE_* var
  registered in envs.py is missing from the reference.
- README and envs.py describe OFFLOAD_MIN_SAVE_TOKENS as the native absolute
  boundary for normal and late saves, and the README documents the
  uncertain-transfer bound.
- Test that a restore raising after it took a descriptor slot fails the load
  and returns the slot once the uncertainty bound expires.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@yhl-amd

yhl-amd commented Sep 27, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review. Every point is addressed in ec3fe93..2f0d606 (9 commits on top of ad850b5). Two of them (§5c, §6a) are resolved differently than suggested, with the reasoning below.

§1 Late save without a BlockManager — ec3fe93
Unbound schedulers (the vLLM plugin) are back on the lease-based protection this PR had removed. Teardown freezes the block table and leases the unemitted suffix, and the final save reads exactly those blocks. The reacquire path now runs only when a BlockManager is bound, instead of keying off not seq.block_table, which the plugin never clears. test_unbound_teardown_leases_the_blocks_its_final_save_reads lives in tests/ (not tests/plugin), so CI reaches it. The #2339 retired-failure test is back to its main version.

§2 Hash-chain seed — 971823c
The native _boundary_hash fallback now extends its chain through BlockManager.prefix_hash_chain, the manager's own _chain_to, so seed, algorithm and slicing are shared and both branches agree by construction. acquire_offload_prefix seeds from seq.cache_seed. Tests cover a multimodal-seeded chain on both sides.

§3 Wedge on an unprovable submission — c944f61
The lease is still retained, but only for a bounded time. _UncertainSubmission reports a failed terminal result after lmcache.mp.uncertain_transfer_timeout_s (default 2 × lmcache.mp.mq_timeout = 600 s) and logs a warning. The save or load then settles through the normal path, which releases the pin, budget, pending slot and descriptor slot. The same bound covers a raising restore and a restore event whose query() raises; the latter used to continue forever. The PAGE-only path had the same shape under #2339: an immediate failure with no quiescence report, so the save never retried and its lease was never released. It now uses the same bounded uncertainty. test_uncertain_remote_submission_retains_lease is joined by tests for the expiry.

§4 One knob, two units — ec3fe93
The shared base no longer reads OFFLOAD_MIN_SAVE_TOKENS, so dense/hybrid behave as before this PR. Native MP applies it to the absolute boundary for normal and late saves alike, which means the short tail of a long request is stored. The floor can never reach boundary 0. A late save with nothing savable resident now retires the request instead of emitting an empty save forever. Tests run with the default value (unset env) as well as with explicit ones.

§5

  • a. descriptor_slot in DSv4.1 — 13721f0: StateCopies keeps an independent staging buffer and upload fence per slot. A new AST sweep test fails if any execute_paged_state_copies under model_ops/attentions lacks the parameter.
  • b. uint8 — a608c55: the native PAGE group registers byte views, like the PAGE-only path and the STATE aliases.
  • c. DP identity — documented in 17918ab, no logic change. In LMCache (05fc77a), register_kv_cache keys GPU memory by the unique instance_id. LayoutDescRegistry.register refcounts (model_name, world_size) and is written for multiple registrants of one identity, and all replicas publish the same descriptor. (model_name, worker_id, world_size) is the content-addressed storage namespace, so sharing it deduplicates identical prefixes across replicas by design, as the README states. Sessions and their lookup locks are per replica via _mp_session_id. The removed TP-only cases were replaced by the single-host DP/DP-attention acceptance, multi-node rejection and shared-namespace/per-replica-session tests that are already in test_lmcache_mp.py.

§6 — 7cdef48

  • a. Disabled coordinator: this now fails at bind with a message naming --enable-prefix-caching, rather than falling back to PAGE-only. For DSv4, PAGE-only would restore KV under stale recurrent state, which is the silent-wrong-output case the in-process path already refuses for GDN.
  • b. can_partially_deallocate_state is all-of over still-deferring subs (a missing method vetoes, nothing deferring is False). The multi-connector test now separates ANY from ALL.

§7

  • a. Session lifetime — e93d0ef: end_session is deferred until no save of the request is tracked or in flight. It is swept on retirement and at every metadata build.
  • b. Restore fencing — c944f61: the restore stream wait_streams the compute stream before copying (WAR on the SLOT's previous occupant). Every step's start_load_kv makes the compute stream wait_event on in-flight restore events (RAW, including relocations in build()). Both fences are on the GPU, so the host does not block. The stream test now asserts both fences instead of no-op'ing the stream.
  • c. DSv4 + draft pool — a608c55: DSv4 publishes paged_state_region_count. The native layout validates coverage and builds image aliases from the leading (target) regions only, and draft rows are registered as ordinary PAGE. Auto rank collapse is also off when a DSpark draft's backend owns a KV pool, since draft_kv.py declares factor 1; otherwise registration would reject the collapse.
  • d. Admission unwind — e93d0ef: save admission returns the lease and budget if anything after the acquire raises. Load admission hashes before reserving and, on error, releases units, budget and any suspended local restore.

§8

  • The per-step frontier scan is cached per request on (frontier, floor, store.generation). The store bumps its generation whenever the READY set can change — 17918ab.
  • Restore descriptor slots are reserved at registration through the new builder hook reserve_checkpoint_descriptors, not first touched mid-serving — 17918ab.
  • The _begin_restore slot is returned via the §3 bound, and a test covers it — 2f0d606.
  • All 16 ATOM-owned offload env vars are in docs/environment_variables.md, with a test guarding it — 2f0d606.
  • The stale "frozen block table" comment is rewritten — ec3fe93.

§9 Each test in your table now asserts the property rather than the symptom (see above). About 20 tests were added.

Validation

  • Black and Ruff are clean on every changed file.
  • Offload/LMCache/scheduler/attention-selected suite vs main 68e0df5: 3488 passed, with no test failing only on this branch. The one main failure among them, the SeqView slot scan, passes here.
  • DSv4-Pro TP8 end to end at 2f0d606 with the pinned LMCache: cold / restore / control (empty LMCache). The restore pass loaded 428,544 tokens from LMCache (offload_hit 0.80) with 0 load failures, and an 18K-token prompt restored 0:17920. GSM8K accuracy was 0.96 / 0.94 / 0.92, and answers matched cold 49/50 for both restore and control.

On a CPU-only torch, torch.cuda.Event is a dummy class that raises when it is
built, which happens before the restore takes its descriptor slot, so the test's
"slot is held until the bound" assertion failed in CI. Stub the Event so the
restore fails deterministically at the stream fence, after the slot is taken,
on every runner.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@valarLip

Copy link
Copy Markdown
Collaborator

Second pass — 94c074129

Re-reviewed at 94c074129, ten commits on from the head my previous comment covered (ad850b5ee).

First, the part that deserves saying plainly: every section of that review was answered, and the fixes land on the right lines. ec3fe93bb keeps early release safe without a BlockManager and collapses the two save thresholds into one; 971823cb0 seeds both chains from cache_seed; 13721f0df takes the descriptor slot in DSv4.1; a608c55c1 keeps draft PAGE regions out of the native image; e93d0ef63 ends sessions after the last save and unwinds failed admissions; 7cdef48d0 fails closed on a disabled coordinator and on composite state release; c944f617f bounds unprovable transfers and fences native restores; 17918abdd and 2f0d60605 take the cost and docs items. The FP8 sweep landed too — native_state_layout.py:173 is now [view.view(torch.uint8) for view in page_views] [verified].

On the one point I raised that was not changed, the code argues back and I think it wins: config.py:429 documents replica-local worker ids as deliberate — "id i means shard i of the model, which holds the same bytes in every replica, so replicas sharing a disk/remote backend share entries instead of duplicating them" [verified]. That answers the cache-key half of what I asked; I'm withdrawing it.

So this pass is not a second list of the same kind. It is one observation:

Several of these fixes introduce the next defect, and they do it for one reason: the PR still has not decided what settles a transfer whose outcome is unprovable. It now answers that question three different ways in the same file — never (save_abandon_timeout_s() -> 0.0, timeout_reclaimable=False), after 600 s (_UncertainSubmission), and never, unbounded (_terminal_future_result's blanket exception swallow). My last review only pushed on the "never" side; c944f617f supplied the "after 600 s" side; the two now coexist and contradict each other on the same physical blocks.

Settling that one policy collapses §1, §2, §3 and §4 below.

Provenance: [verified] = I read the deciding lines at this head. [reported] = the shape matches the code but I did not trace it end to end.


1. The load path now frees blocks on a timeout that the save path explicitly refuses to trust [reported]

page_unit_checkpoint.py:605 states the rule and the reason: checkpoint pins are timeout_reclaimable=False because "a missing report cannot prove that another process has stopped its DMA."

The new uncertainty bound does not carry that rule to the load side. _submit_native(loading=True) swallows a transport exception and leaves _UncertainSubmission installed (native_state_worker.py:245). After lmcache.mp.uncertain_transfer_timeout_s (default 2x mq_timeout = 600 s) query() returns True with result() False, get_finished reports failed_loading, and _finish_native_load(succeeded=False) → _release_native_load → release_transfer_units → BlockPool.release_units → free(block_id) puts the destination units straight back on the allocatable free list — while the server may still be DMA-writing into them. The next request allocated those blocks gets its KV overwritten mid-flight.

The same inference appears on the source side (backend.py:1125): at the deadline, get_finished emits _source_safe_completions(..., _chunk_ranges(pending.start, pending.end, chunk_size)) with no result check, so _drain_source_safe_releases → free_leased_blocks → kv.free releases HBM a remote process may still be reading. Same pattern again at native_state_worker.py:373.

Either the load and source-safe paths adopt the save path's rule (no release without a report), or all three move to a quarantine list that is only recycled on a positive signal. What they cannot be is opposite policies for the same unprovable fact, one file apart.

2. save_abandon_timeout_s() -> 0.0 switches off two mechanisms this connector does not own [verified]

Scheduler._save_abandon_timeout_s() feeds three call sites, not one:

scheduler.py:1276  _reconcile_stalled_deferred_saves   -> returns 0 immediately
scheduler.py:3880  bm.reclaim_stale_state_store_pins   -> no-op at timeout <= 0
scheduler.py:3887  bm.reconcile_orphan_load_slots      -> no-op at timeout <= 0

Before this PR the MP connector inherited OffloadSchedulerMixin.save_abandon_timeout_s (LMCache pin + 30 s). Returning 0.0 is a defensible statement about this connector's saves; it is not a statement about orphan load slots or stale state-store pins. reconcile_orphan_load_slots guards a mechanism whose own docstring warns the state pool wedges "with no fault to point at", and _reconcile_stalled_deferred_saves is the only drain for deferred_free_blocks. A connector opting out of clock-based reclamation should express that in its own reclaimer, not by zeroing a scheduler-wide knob.

3. The new bound does not cover a future that keeps raising [reported]

backend.py:549 — _terminal_future_result catches every exception and reports "not terminal", with no attempt counter and no deadline. On a dead socket or torn-down IPC context, future.query() raises on every poll and the operation is pending for the process lifetime, holding its source lease, its _save_inflight slot, its admission budget and (native) its checkpoint pin.

Every real future is stored unwrapped, and none of the three _UncertainSubmission install sites fires on poll failure — they all fire at or before submission. So the class of failure the new bound was written for is bounded, and its nearest neighbour is not. tests/test_lmcache_mp_native_worker.py:294 pins the retention as intended for source safety, which is right; the gap is that it is unbounded where every sibling now has a bound.

4. A native STORE terminal that never arrives still has no recovery path [reported]

This is the native twin of the case c944f617f fixed, and it was not carried across. At native_state_scheduler.py:540: abandon_save returns None, reclaim_stale_leases returns [], the inherited save_abandon_timeout_s() is 0.0, and reclaim_stale_offload_pins skips acquire_checkpoint_source pins because they are timeout_reclaimable=False. If a worker rank dies, TP quorum is never reached, or a completion is dropped, _native_saves[operation] is never popped — so release_offload_store_source / settle_offload_store / _refund_state_image never run. One checkpoint image leaves BlockPool permanently, _pinned_state_bytes never refunds (after max_pending_saves such events nothing saves or loads again), _save_inflight[sid] never clears so the request's blocks sit in deferred_free_blocks forever, and has_pending_work() stays True so EngineCore busy-loops over idle GPUs.

5. ec3fe93bb drops the last chunk of every early-released request when prefix caching is off [reported]

The protected.update(table[start_block:end_block]) that origin/main ran unconditionally now runs only under if self._block_manager is None (chunked_scheduler.py:694), so with a BlockManager bound the unemitted final save relies entirely on acquire_offload_prefix — which has no prefix-caching guard.

BlockManager.hash_blocks returns immediately when prefix caching is off (block_manager.py:1629), and all three kv.publish sites sit behind the same guard, so _hash_to_block_id is permanently empty. acquire_offload_prefix breaks on the first lookup, _late_save_source returns None, and build_connector_meta does self._save_tracker.pop(sid); continue — no warning, no counter. DenseOffloadScheduler sets _supports_early_block_release = True unconditionally and nothing rejects the combination, while offload/README.md:1215 ships --no-enable_prefix_caching alongside lmcache_offload. Every test in tests/test_offload_early_block_release.py passes enable_prefix_caching=True.

Even with prefix caching on, a block freed at teardown can be re-allocated and unindexed before the next metadata build, truncating the save.

6. 13721f0df fixed the production signature; the test doubles still cannot take the keyword — and no test runs a successful native restore at all [reported]

_begin_restore calls self._native_copy((), (...,), descriptor_slot=descriptor_slot). The doubles are lambda *_: None (tests/test_lmcache_mp_native_worker.py:107, tests/test_lmcache_mp_shell.py:87) and lambda stores, restores: None (tests/test_lmcache_mp_native_layout.py:50) — positional-only. The call would raise TypeError: got an unexpected keyword argument, which _begin_restore's bare except Exception swallows into restore_succeeded = False.

The suite stays green only because the restore-success tests (:262, :281) replace _begin_restore outright and line 279 sets pending.restore_succeeded = True by hand. tests/test_paged_state_copy_signature.py AST-checks production signatures but not the doubles — so the exact drift 13721f0df just fixed in DSv4.1 remains undetectable in the test doubles, and the happy path of the feature this PR adds has no end-to-end coverage.

7. Smaller, independent [reported]

  • protected_block_ids sets seq._offload_finished = True before the scheduler decides whether partial deallocation is allowed (chunked_scheduler.py:679). Scheduler calls _connector_protected_block_ids(seq) at scheduler.py:3442 and computes state_safe afterwards; when state_safe is False the request keeps its whole block table, but the flag is already set, so the next build_connector_meta tests only _offload_finished and self._block_manager is not None and takes a second kv.claim per block it still owns. Reachable on any per-request-state model whose backend forks rather than copies (qwen3_next, glm5_next_text, kimi_linear) under the PAGE-only MP scheduler, which sets _supports_early_block_release = True and defines no can_partially_deallocate_state. tests/test_scheduler.py:2043 stubs protected_block_ids, so the ordering is untested.
  • The STATE image lease is released on a PAGE signal (native_state_worker.py:322). _emit_native_source_safe treats any(end >= boundary) from take_completed_ranges() — which reports PAGE token ranges — as proof the STATE engine groups (1+ordinal, recurrent_state=True) are also done being read, and the scheduler then drops the pin on checkpoint PAGE units that _native_block_ids proves are disjoint from the KV pages. Soundness requires LMCache to emit a chunk completion only after every engine group registered at that chunk has been read. Nothing in the repo asserts, validates or documents that; the only take_completed_ranges implementations in tree are test fakes. Either state the required guarantee and assert it at registration, or fence the STATE release on its own signal.
  • Worker-local backpressure is reported as a terminal save failure (native_state_worker.py:220). When len(pending) >= self._max_pending_saves the worker parks _NativePending(req, None); _terminal_future_result(None) returns (True, None), a STORE completion with succeeded=False is emitted, and _complete_native_save increments _save_failures[sid]. After three such events _save_frontier raises the floor past that boundary and the prefix is never stored again. Nothing was submitted and nothing was read — this is pure admission control being charged against a failure budget. The comment claims the scheduler enforces the same bound, but the two count different things: the scheduler bounds len(self._save_inflight) (per request id) while the worker bounds len(self._native_saves) (per operation, including the refused and immediate_success placeholders that linger until the next sweep). Any transient divergence is permanent for that prefix.
  • Restore descriptor slots are sized off a save-queue bound (native_state_worker.py:143). Slots come from _max_pending_saves; concurrent native loads are governed by the independent lmcache.mp.max_pinned_state_bytes. With 8 images of budget and the default OFFLOAD_MAX_PENDING_SAVES=2, the scheduler admits 8 loads against slots [1, 2]; the third terminal load hits if not self._restore_descriptor_slots: return False and continues, re-polling a future whose result was already consumed. Separately, neither except handler in the load loop (:398-407, :412-422) returns pending.descriptor_slot, parking a slot for the full 600 s window — a one-line fix in both.
  • _finish_native_load discards adopt_transfer_units' return value (native_state_scheduler.py:453). adopt_units' own docstring documents that boolean as the caller's race signal. When two requests share a prefix and B loses the publish race, B's units are released and False returned; _finish_native_load ignores it, calls release_suspended_restore(lease.local_restore) — discarding the queued restore that would have refilled B's SLOT — and reports load_finished. B paid a full image reservation and budget charge for nothing, and its recurrent SLOT may hold whatever the discarded restore was meant to replace. No log, no metric, no retry.
  • adopt_transfer_units pops before it commits (page_unit_checkpoint.py:1082). units = self._transfer_units.pop(owner, None) runs first; if store.adopt_units → BlockPool.rekey_units raises its AssertionError on an owner mismatch, the exception propagates out of _finish_native_load with no try/finally anywhere on that path, leaving units_per_checkpoint blocks allocated, absent from _transfer_units, absent from records, and unreachable by any release path. Every occurrence permanently shrinks the KV pool. Pop after the transfer succeeds, or wrap it.
  • _clear_pending_load resolves by sid alone (native_state_scheduler.py:431), while _finish_native_load, request_finished and cancel_pending_load all guard with lease.seq is seq. _decide_load_after_alloc returns native_load_id_busy exactly when lease.seq is not seq, and build_connector_meta then clears by sid for the new seq. I could not construct a live step sequence that reaches it — _new_load_operation flips dispatched in the same step — so this is latent, but it is the odd one out of four.
  • _finite_float_env raises on an empty value (envs.py:99), diverging from the policy _flag_env declares two functions above. OFFLOAD_PUBLICATION_TIMEOUT_S= — the documented way to clear a knob inline — makes float('') raise out of DSV4OffloadConnector.__init__ and kill the worker at startup. The four readers now sitting side by side disagree: _flag_env reads empty as off, _optional_int_env as None, _int_env_or_default warns and defaults, only _finite_float_env propagates. OFFLOAD_COPY_WORKERS / OFFLOAD_LOAD_WORKERS were left on bare int(...) and missed the sweep, so a non-integer dies with invalid literal for int() while docs/environment_variables.md:299 documents them as plain int | 1.

8. Structure

Mostly carried from the last pass, plus what the new code added:

  • mp/backend.py is 1332 lines against the repo's 800-line ceiling.
  • build_native_state_mp_layout is 150 lines and duplicates the PAGE validation in backend._build_cache_views — and the two copies have already diverged on contiguity and byte checks.
  • native_state_worker.get_finished is 91 lines at 6 levels of nesting with three near-identical recovery blocks. Three of the findings above live inside it.
  • _DescriptorStaging is a third copy of the buffer helper this PR just consolidated for DSV4 and GDN.
  • tests/test_paged_state_copy_signature.py and tests/test_lmcache_mp_native_layout.py:223 parse and exec production source. A test that reads the text of the code it tests passes for reasons unrelated to behaviour — and in this case it is also what let §6 through.
  • envs.py:943 still lists engine_driven as a valid transfer mode that the code rejects.
  • The _save_frontier memo added in 17918abdd is keyed on a store-global generation that any request's checkpoint traffic bumps.

Checked and not upheld

Recorded so they are not re-raised: the scheduler/worker native-vs-PAGE-only selection mismatch (checkpoint_spec is not None ⟺ transfer.copies, and state_backend is the same builder); the activate_block_leases double-claim (chunk size is enforced divisible by virtual block size); a "wedged forever" variant of the failed-save path (chunked_scheduler.py:938 and :962 recover in both completion orders); _invalidate_pool_caches discarding reserved descriptors (startup-only); and the as_strided storage-offset hazard (covered by tests/test_lmcache_mp_native_layout.py:173).

Honglie Yi and others added 13 commits September 28, 2026 03:28
…at a deadline

The MP path answered "what settles a transfer whose outcome is unprovable"
three ways: never (native checkpoint pins, a zeroed abandon timeout), after
600 s (the uncertainty bound, which then freed load destinations and PAGE
sources a server might still be using), and never-unbounded (a future whose
poll keeps raising). One rule now: memory under an MP transfer is released
only on a terminal report, and a transfer still not terminal after
`lmcache.mp.transfer_deadline_s` (default 1200 s) raises
`LMCacheTransferUnprovable`, stopping the engine.

- Worker: every pending PAGE-only and native load/save carries its start
  time and is checked against the deadline, which bounds a raising
  submission, a future whose poll keeps raising, and a restore whose event
  cannot be queried. A descriptor that cannot be built is provably unsent
  and still fails immediately; only the transport call is unprovable.
- Scheduler: a watchdog over dispatched saves, loads and native checkpoint
  sources catches a completion that never arrives (lost report, silent TP
  rank), with a margin so the worker fails first.
- `save_abandon_timeout_s` is inherited again, so the engine-wide stalled
  save, state-pin and orphan-load-slot reclaimers stay on; MP opts out
  through its own `abandon_save` / `reclaim_stale_leases` instead.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The late final save reacquired a finished request's prefix through the hash
index whenever a BlockManager was bound. With prefix caching off nothing is
indexed, so the lookup found nothing and the save was dropped without a
warning -- the combination the offload README ships for `lmcache_offload`.
Hash reacquire now requires a bound manager with prefix caching; otherwise
teardown leases the unemitted suffix of the frozen table, as on main. A
late save that finds part of its prefix evicted is counted in
`truncated_late_saves`.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
`protected_block_ids` marked the request finished before the scheduler
decided whether it could release it partially. When per-request state made
that unsafe, the request was deferred whole and kept its table, but the next
metadata build still took the late-save path and claimed every block a
second time through the hash index (dropping the save if a hash was
missing). `activate_block_leases`, which only the partial-release branch
calls, now marks the release, and only that mark selects the reacquire path.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…erminal

The worker released a save's checkpoint image as soon as any PAGE chunk
milestone reached the boundary. Chunk milestones report PAGE token ranges;
nothing in LMCache promises that a chunk completes only after every engine
group registered at it, STATE groups included, has been read. The pinned
LMCache emits no chunk events, so this path never ran, but it would have
been unsound on the first server that did. The STATE source-safe channel is
removed: the image pin is released at the STORE terminal, and chunk
milestones release PAGE leases only.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
`OFFLOAD_PUBLICATION_TIMEOUT_S=` made `float('')` raise out of connector
init, and `OFFLOAD_COPY_WORKERS` / `OFFLOAD_LOAD_WORKERS` died with a bare
`invalid literal for int()`. The offload section now states one policy:
unset or empty is the default; a set but unusable value warns and falls back
for knobs that only tune reuse, and is rejected at startup, naming the
variable, for widths, sizes and timeouts. The transfer-mode comment no
longer lists `engine_driven` as valid.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…invariants

- `adopt_transfer_units` popped the transfer record before adopting; if
  adoption raised, the units were reserved but unreachable by any release
  path. The record is now removed only after adoption returns.
- A native restore that loses the publish race to an identical image was
  dropped silently; it is now logged as a deduplication.
- The worker's own pending-save refusal is unreachable while the scheduler
  enforces the same bound; if it ever fires it now logs an error instead of
  passing as an ordinary save failure.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…y doubles

The restore-copy doubles were positional-only, so the production call with
`descriptor_slot=` would have raised inside `_begin_restore` and been
reported as a failed restore; the success tests replaced `_begin_restore`
outright, so no test ran the happy path. The doubles now take the production
signature, and a new test drives a terminal retrieve through `get_finished`
into the real `_begin_restore`, then asserts the copy's descriptor slot, that
the load finishes only once the restore event is done, and that the slot is
returned. The CUDA stand-ins are shared with the fence test.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…n checkpoints

The per-request `_save_frontier` memo was keyed on the store-wide
generation, so any request's publish or eviction forced every tracked
request to rescan its prompt on the next step. The store now keeps a
bounded log of which prefix hash each generation bump touched; a request
rescans only when a change hits one of the boundaries it scanned (its answer
or anything above it), or when the log cannot tell.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…int copier

DSv4.1 kept its own `_DescriptorStaging`, a third copy of the per-slot
pinned descriptor pool the base builder shares with DSv4 and GDN, and the
only one that fenced reuse: a non-blocking H2D reads its pinned rows when
the stream reaches it, so refilling a buffer whose last upload is still
queued rewrites the descriptor under an earlier copy. `DescriptorStaging`
in `pool_layout/paged_state_copy.py` now owns the per-slot buffers and the
fence, and the base builder (DSv4, GDN) and DSv4.1's `StateCopies` both use
it, so the base path gains the fence too.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
`build_native_state_mp_layout` and `_build_cache_views` each validated the
backend-published PAGE views, and had already drifted: PAGE-only required
full contiguity and compared total bytes against the view, native checked
inner contiguity plus the block stride against the region. Both now call
`validate_page_views` (new `mp/page_views.py`), which applies the stricter
union -- shape, tight block-major stride, unit and total bytes, aliasing,
forward indexing, one device -- and, for native, that a block's physical
slots divide the block size. Each builder keeps only what is its own:
PAGE-only rejects stateful fields; native checks the image coverage.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
`get_finished` was 91 lines at six levels of nesting, with three
near-identical recovery blocks. It is now a loop over `_poll_native_save`
and `_poll_native_load`; the load's retrieve-then-restore progression lives
in `_advance_native_load`, and both unprovable-restore paths share
`_hold_unprovable_restore`. No behaviour change.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
`mp/backend.py` had grown to 1332 lines holding configuration, PAGE view
validation, transfer bookkeeping, lookups and both connector halves. It is
split without behaviour change into:

- `deployment.py`: config validation, TP/DP topology and rank collapse,
  model namespace, server adapters
- `transfer.py`: operation identity, terminal detection, the transfer
  deadline and `LMCacheTransferUnprovable`
- `lookup.py`: the scheduler's lookup client and read-lock bookkeeping
- `page_views.py`: PAGE-only `_build_cache_views`, beside the shared view
  validation it uses
- `worker.py` / `scheduler.py`: the PAGE-only connector halves

The native modules, the public shell and the tests import from the new
homes; tests patch each name where it is looked up.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ate saves

- With the abandon window inherited again, a slow but legitimate MP save
  deferred past it was "abandoned" (a no-op for MP) and, a minute later,
  logged as a wedged P/D send -- both long before MP's own transfer
  deadline. A connector can now answer `waits_for_transfer_report(seq)`;
  the stalled-save reclaim skips such requests. LMCache MP answers True,
  the in-process shell forwards (default False), and the composite answers
  True only if every sub still deferring the request does, so P/D sends and
  in-process saves keep their abandon path.
- A late save that finds part of its prefix evicted now logs a warning with
  the running count, not a debug line: early release returns the tail's
  blocks at teardown, and this is the cost of that trade.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@yhl-amd

yhl-amd commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the second pass. You were right that the three answers to "what settles an unprovable transfer" were the root; the fix starts there. Everything is in 83f515a..ecffc99 (13 commits on top of 94c0741). Below, "not changed" items include the reason.

The policy — 83f515a
Memory under an MP transfer (PAGE sources, restore destinations, checkpoint pins, descriptor slots) is released only on a terminal report. No clock frees it. A transfer still not terminal after lmcache.mp.transfer_deadline_s (default 1200 s) raises LMCacheTransferUnprovable and the engine stops: fail-stop instead of either corrupting blocks or wedging the pool silently. This collapses §1–§4:

  • §1 _UncertainSubmission is gone. A raising submission becomes _UnprovableSubmission, which never turns terminal on its own, so load destinations, PAGE sources and the native source-safe path are never released on a timer. A descriptor that cannot be built is provably unsent, so it still fails immediately. Only the transport call is treated as unprovable, and that now applies to PAGE-only loads too; before, a raising load submit was an immediate failure.
  • §2 save_abandon_timeout_s is inherited again, so the stalled-save, state-pin and orphan-load-slot reclaimers keep running. MP opts out through its own abandon_save / reclaim_stale_leases, which release nothing. So that a slow MP save is not first logged as "abandoned" and then as a wedged P/D send long before its deadline, a connector can answer waits_for_transfer_report(seq) and the stalled-save reclaim skips that request (ecffc99). MP answers True. The composite answers True only if every sub still deferring the request does, so P/D sends and in-process saves keep their abandon path.
  • §3 Every pending operation carries its start time. The deadline is checked wherever it is still not terminal, so a future whose query() keeps raising is bounded like everything else.
  • §4 A scheduler-side watchdog covers dispatched saves, loads and native checkpoint sources (_native_saves). It catches a completion that never arrives (lost report, silent TP rank). It uses the deadline plus a 60 s margin, so the worker, which can name the exact transfer, fails first.

§5 — 5cc76a8: The hash reacquire now requires a bound manager with prefix caching. Otherwise teardown leases the unemitted suffix, as on main. New test with enable_prefix_caching=False. On your second paragraph (prefix caching on, a tail block evicted between teardown and admission): that is the price of early release, which returns the tail's blocks at teardown. Protecting them would undo the saving. So it is made visible rather than prevented: a warning with the running truncated_late_saves count (ecffc99).

§6 — 45943d1: The doubles take the production signature. A new test drives a terminal retrieve through get_finished into the real _begin_restore. It asserts the copy received descriptor_slot, that the load finishes only once the restore event is done, and that the slot comes back. (For accuracy: the three lambdas you listed were never called, because the worker fixture bypasses registration. The real gap was the missing happy-path test, as you said.)

§7

  • Flag before decision — eaac7eb: activate_block_leases, which only the partial-release branch calls, now sets _offload_released, and only that selects the reacquire path. A request deferred whole saves from the table it still owns. The test fails if it reacquires.
  • STATE on a PAGE signal — 818a555: The STATE source-safe channel is removed. The image pin is released at the STORE terminal only. (The pinned LMCache has no chunk events, so the per-range path never ran; it would have been unsound on the first server that had them.)
  • Worker backpressure — 9b8f779: Not changed as a failure-budget problem, because it cannot diverge. Every save the worker holds is still in the scheduler's _save_inflight: that is popped only on the aggregated STORE terminal, which needs every rank, and each rank deletes its entry in the same sweep that emits it. Placeholders are removed in that sweep too, and non-writers return before the check. If it ever fires, it now logs an error instead of passing as an ordinary save failure.
  • Restore slots vs loads — not changed. By default max_pinned_state_bytes = max_pending_saves × image_bytes, and saves and loads share it, so admitted loads never outnumber slots. With a raised budget, an extra load waits for a slot (_begin_restore → False). Re-polling is safe: LMCache futures memoize result(). On the two except paths, keeping the slot is deliberate: the restore may already have queued an H2D that reads the slot's staging. Under the new policy it stays held until the deadline stops the engine.
  • adopt_transfer_units return — 9b8f779: Now logged as a deduplication. The loser's units are released by adopt_units itself, and the SLOT already holds the identical restored image, so releasing the suspended local restore is correct.
  • Pop before commit — 9b8f779: The transfer record is removed only after adoption returns. Test included.
  • _clear_pending_load by sid — not changed. In ATOM a sid is a process-unique Sequence.counter value and preemption reuses the object, so a sid never maps to two seqs. The plugin, where ids come from the client, never gets the native scheduler.
  • envs — c8156e8: Unset or empty now always means the default. A set but unusable value warns and falls back for reuse-tuning knobs, and is rejected at startup, naming the variable, for widths, sizes and timeouts. That policy is written at the top of the offload section. The engine_driven comment is fixed. (All three behaviours predate this PR, but they are cheap to fix here.)

§8

  • mp/backend.py is split into deployment.py, transfer.py, lookup.py, page_views.py, worker.py and scheduler.py (largest now 490 lines) — ac5f2dd.
  • The PAGE validation is one validate_page_views, used by both registrations, with a test that breaks the same view both ways — ec7e2ec.
  • Native get_finished is split into _poll_native_save / _poll_native_load / _advance_native_load, and the three recovery blocks are now one _hold_unprovable_restore — cf1fea4.
  • _DescriptorStaging became the shared DescriptorStaging in pool_layout/paged_state_copy.py, fence included, so DSv4 and GDN gain the upload fence as well — 28dda85.
  • The frontier memo is invalidated per request: the store logs which hash each generation bump touched, and a request rescans only when one of its own scanned boundaries changed — 4f0eeda.
  • The source-parsing tests stay. The CPU unit-test job has no aiter, so the attention modules cannot be imported there. That is the same reason test_deepseek_v4_transfer_regions.py and test_bind_walk_rows.py on main extract and run production source. The §6 gap is now closed by the behavioural restore test rather than by those.

Validation

  • Black and Ruff are clean. The MP, offload, scheduler, envs and checkpoint suites pass (865 tests).
  • The CPU-container failures that remain match 94c0741 exactly (no GPU arch in the container).
  • DSv4-Pro TP8 e2e at ac5f2dd (cold / restore / control with an empty LMCache): passed.
    • The restore pass loaded 429,056 tokens from LMCache (offload_hit 0.80), and all 328 retrieves finished.
    • The 18K-token probe read from LMCache on restore and read nothing on control.
    • GSM8K accuracy was 0.92 / 0.90 / 0.92, and answers matched cold 46/50 (restore) vs 47/50 (control).
    • No LMCacheTransferUnprovable and no load failures.
    • ecffc99 changes only scheduler reclaim and logging, and is covered by CPU tests.

Honglie Yi and others added 3 commits September 28, 2026 06:57
…ether

Every backend kept `block_regions` and `block_tensor_views` as two lists
paired by hand (DSV4 even collected a third list of sources to zip later).
`KVTransferTensors.add_block_region(tensor, semantic_role=...)` now appends
both from one tensor: the region's addresses and a zero-copy
`uint8 [num_units, 1, unit_bytes]` alias of exactly those bytes, cut from a
larger allocation when `total_bytes` says so.

DSV4, MHA, the MHA draft and MLA all publish through it. MHA and the draft
already published this shape. MLA's views change from `[n, block_size, width]`
to the same byte form; LMCache stores the same bytes in the same order either
way, and neither its object key nor ATOM's model namespace depends on view
shape, so existing cache entries stay valid. The MLA builder test's module
stubs had drifted since #2399 (`atom.utils.block_tables`); they are fixed so
the test runs again.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…w together

`add_block_region` paired a region and its view at one call site, but the
pair was only a convention: both lists stayed public and mutable, the
constructor still took them separately, DSV4 and MLA used a throwaway
`KVTransferTensors` as a scratch builder, the draft merge extended the two
lists by hand, and tensor code lived in the torch-free `types` contract.

- `PageRegion(region, view)` is a frozen value; `KVTransferTensors.pages`
  holds them. `block_regions` and `block_tensor_views` are read-only tuples
  derived from it, so no code can add to one without the other.
- `page_region(tensor, ...)` in the new `disaggregation/page_region.py`
  builds one from the owning tensor (zero-copy byte view, contiguity and
  size checks); `types.py` no longer touches torch.
- Builders pass `pages=[...]` straight to the final object: no scratch
  instance. The address-only producer (Qwen4 exp) publishes
  `PageRegion(region)` with no view, which LMCache MP refuses as before.
- `merge_pages(other)` replaces the draft merge in `ModelRunner`, carrying
  the gcd replication rule with it.

`validate_page_views` stays on the LMCache MP side: it is the consumer
checking what it is handed, not a second copy of construction.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Resolves docs/environment_variables.md: #2414's ATOM_KV_OFFLOAD and
ATOM_KV_OFFLOAD_EXTRA_CONFIG rows join the rewritten LMCache offload table.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants