Skip to content

fix(checkpoint): retry concurrent immutable page writers - #65

Closed
yatesdr wants to merge 8 commits into
devfrom
fix/checkpoint-concurrent-dedup-publication
Closed

yatesdr wants to merge 8 commits into
devfrom
fix/checkpoint-concurrent-dedup-publication

Conversation

@yatesdr

@yatesdr yatesdr commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fix concurrent request-boundary checkpoint publication when schema-2
generations share immutable content-addressed pages.

  • Distinguish an in-flight immutable writer from a terminal store conflict.
  • Return an explicit metadata-only busy admission without aborting the
    waiting generation.
  • Retry in the existing worker-owned background transfer with bounded
    exponential backoff, leaving LMCache RPC handler threads free to process the
    owner completion.
  • Reuse a committed page through the existing no-copy marker, or reserve it
    after the owner aborts.
  • Preserve capacity eviction/backpressure, all-rank atomic publication,
    cancellation ownership and filesystem deduplication.

Target branch: dev. Merge #62 before this PR; the retained commit ancestry carries its atomic checkpoint transport. This fixes concurrent publication for content-addressed payloads represented in #62, including #64. Merging this PR must publish to dev, not only to the #62 review branch.

Fixes local-inference-lab/vllm#725

Root cause

The R30 transfer worker permits two checkpoint copy jobs per rank. Instruction,
prompt and response generations from one request can consequently overlap.
Their unchanged full attention pages have identical schema-2 object keys.

While the first D2H copy owns a write lease, reserve_write_detailed(..., "new") reports KEY_NOT_WRITABLE to a later generation. Because the page is
not readable until commit, the later store was treated as a terminal conflict.
Rank timing differed, and one negative rank acknowledgement caused the atomic
connector to abort the entire generation.

Implementation

CheckpointPayloadStore.prepare_store() now reports
AdmissionFailure.BUSY only when every unresolved key is explicitly
KEY_NOT_WRITABLE. Missing result entries, capacity failures and invalid or
aborted generations retain their prior terminal/backpressure behavior.

The checkpoint RPC represents this state as CheckpointLeaseResponse("busy").
No SHM lease or byte slot is exposed. CheckpointTransferWorker retries the
same immutable manifest/rank off the model and RPC threads, starting at 1 ms
and capping at 10 ms under the existing RPC deadline. Shutdown or deadline
returns a negative rank result so the scheduler performs its existing atomic
abort.

Validation

  • New owner-commit and owner-abort storage regressions fail twice on pristine
    R30 and pass with this patch.
  • New real SHM/MQ two-worker regression proves the second store remains pending
    while the first copy is held, then both generations publish.
  • 41 admission and checkpoint storage/transfer tests pass.
  • 23 checkpoint identity and durable-index tests pass.
  • 5 L1 manager/partial-allocation tests pass, 1 skipped for the CPU test
    environment.
  • SPDX, device-use guards, isort, ruff, ruff-format, codespell and mypy pass.
    Rust hooks are explicitly skipped for this Python-only change on macOS.

Complete TP4/DCP4 GPU, cross-replica, restart, filesystem-write and performance
qualification will be added from the patched R30 image.

Signed-off-by: Derek Yates derek.yates@live.com

voipmonitor and others added 7 commits September 8, 2026 04:15
…d memory

Authenticate immutable model, layout and token-prefix identities in a bounded durable directory. Publish a generation only after every rank stores its complete payload. Retain SHM leases until copy completion or explicit drained cancellation; unresolved ownership stops admission. Worker-owned background transfers keep tensor payloads out of metadata RPC and require no GPU context in the sidecar.

Validation: 42 index, identity and real SHM/RPC storage tests plus 81 engine-driven transfer tests passed. Coverage includes process-restart filesystem retrieval, partial rank failures, capacity rejection, read-lock eviction, late lease and completion replies, and lost-ownership admission shutdown. Pre-commit checks passed. Status: implemented; composed GPU lifetime qualification remains a release gate.

Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: Martin Vit <martin@voipmonitor.org>
(cherry picked from commit b7977f4)
Import target, recurrent, auxiliary and draft pages through the atomic vLLM checkpoint allocator. Publish only all-rank successful copies, preserve cancelled request pins until admitted transfers drain, and prevent reused public request IDs from consuming predecessor bookkeeping. CUDA work uses existing model-worker streams and pinned SHM; ordinary aligned transfers remain separate.

Validation: eight real allocator/MQ ownership tests passed, including cancellation, reused request IDs, per-rank failure, LoRA namespace isolation and immutable revision requirements. Storage and transfer suites also passed. The GPU checkpoint copier retains the platform CUDA stream/event primitives. Complete-image qualification is pending; status: implemented.

Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: Martin Vit <martin@voipmonitor.org>
(cherry picked from commit 427d926)
Propagate the explicit HTTP force flag through the cache server and management module to L1 eviction. Non-forced clearing retains read/write-locked objects; an omitted flag and the argument-free CLEAR RPC retain their forced behavior. No wire identifier or payload changes are required.

Validation: two real SHM regressions fail before the correction and pass afterward, covering prefetch before slot exposure, active read leases, pending write leases, unlocked eviction and release. All 94 storage and HTTP tests pass; Python pre-commit checks pass. Concurrent GPU qualification is pending; status: implemented.

Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: Martin Vit <martin@voipmonitor.org>
(cherry picked from commit 6b6074c)
…ases

Reject failed begin and copy submissions with terminal ownership results, retain fatal unsafe-copy behavior, and release store admission counters even when rollback raises. Require bound worker state before emitting rank metadata. Successful copy layout, GPU kernels and durable recency semantics are unchanged. Validation: seven fault-injection cases plus 56 existing SHM, identity, index and allocator tests pass; Python pre-commit and mypy pass.
Give each checkpoint rank/storage pair a separate payload-key namespace.
Checkpoint groups use different logical page positions and group-specific
hashes, so they are not the aligned rank/group families required by ordinary
chunk-coherent LRU eviction. The manifest and complete-payload retrieval own
generation atomicity; active SHM copies retain their existing read/write leases.

Retry capacity-only store reservations with the storage manager's bounded
eviction backpressure, releasing all partial reservations before each retry.
Conflicts and invalid generations remain immediate admission failures.

Payload keys use the version-2 namespace. Version-1 filesystem objects produce
safe cache misses and recomputation, never partial or mixed-generation restores.
Model arithmetic, cache-page geometry, and ordinary chunk eviction are unchanged.

Validation: global and isolated LRU reproduce zero eviction candidates before
the fix. All 55 checkpoint storage/index and chunk-coherent eviction tests pass,
including sustained stores beyond pool capacity, pinned SHM bytes, rank-failure
cleanup, and filesystem restart. Python pre-commit checks pass; Rust hooks are
explicitly skipped because no Rust source changes.

(cherry picked from commit 6168170)
Return a negative rank completion when CUDA event creation or recording fails before copy submission. Continue draining other tasks so collective pins can be released. Unsafe in-flight copy failures remain fatal. Normal transfer dispatch is unchanged.

Validation: both event fault injections fail before this change; 58 checkpoint connector, storage and index tests pass afterward. Python pre-commit hooks pass; Rust hooks are explicitly excluded for these Python-only files.

Assisted-by: OpenAI Codex
(cherry picked from commit 617a1b4)
Signed-off-by: derek <derek.yates@live.com>
@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (1)
  • dev/*

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: b7da3a52-15c7-4029-9bc6-fbc135d4dd74

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Signed-off-by: derek <derek.yates@live.com>
@yatesdr
yatesdr force-pushed the fix/checkpoint-concurrent-dedup-publication branch from 2a09b82 to da7df92 Compare September 9, 2026 21:56
@yatesdr

yatesdr commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

Composed GPU qualification — passed

Qualified commit da7df9299ee4d02d9101b18259ffc96b65792d18 on usaubsrvai260001: two RTX PRO 6000 Blackwell TP4/DCP4/MTP3 replicas behind HAProxy, FP8 KV, 4,096 scheduler budget, request-boundary checkpoints, one shared engine-driven LMCache sidecar, 128 GiB L1 and filesystem L2 on NVMe. Auto-responsive compute-share fairness was enabled.

Correctness and ownership

  • Deterministic concurrent-writer regression: fails on pristine R30; owner-commit and owner-abort variants pass with this patch.
  • Focused CPU suites: 3 new regressions, 41 admission/storage/transfer tests, 23 identity/index tests, and 5 L1/partial-allocation tests passed (one expected CPU-environment skip).
  • Changed-file SPDX, device guards, isort, Ruff, Ruff format, codespell, and mypy checks passed.
  • Live cancellation after three streamed events drained pending generations and all store/retrieve leases to zero. The other replica restored the complete 30,020-token prompt with zero recompute.

Cross-replica and persistence

  • 50/50 fresh long-conversation trials passed across both A→B and B→A directions; zero missing generations or full recomputes.
  • Typical continuation restored 24,344–25,244 prefix tokens and computed only 13 suffix tokens.
  • Four- and eight-turn HAProxy conversations moved between replicas correctly; cross-replica turns restored 43K–48K prefixes and computed 13–18 suffix tokens.
  • After recreating both vLLM replicas and the shared sidecar, A and B each restored all 28,521 tokens from persisted L2 with zero compute.

Write amplification

  • First storage of a new 28,521-token prompt: +92 payload files, +634,208,256 bytes.
  • Identical cross-replica replay: +0 files, +0 bytes.
  • Identical post-restart replay: nvme0n1 write-sector delta = 0.
  • New 43,221-token conversation: +937,525,248 bytes. Each later 36-token turn added a constant 441,188,352 bytes / 64 objects for the two new recurrent+draft boundary bundles; the growing attention history was not recopied.
  • No stuck write locks, leases, generations, or storage tasks.

Performance

Matched local-inference-lab sustained decode, pristine R30 → patched:

C R30 patched delta
1 195.4 195.6 +0.1%
4 429.0 428.6 -0.1%
8 584.2 589.5 +0.9%
16 825.8 839.5 +1.7%
32 1173.6 1170.1 -0.3%
64 1524.5 1527.8 +0.2%

Matched cold LMCache write-through prefill: 32K 7,287 → 7,285 tok/s (-0.03%); 200K 7,044 → 7,039 tok/s (-0.07%). No attributable performance regression.

Auto fairness adapted compute share across approximately 0.31–0.52 during three 65,535-token prefill/decode collisions. Decode continued without starvation and recovered after every prefill.

The final provenance build and the tested build have identical root filesystem layer hashes; only the OCI revision label changed when the PR was rebased into this clean one-commit review.

@yatesdr

yatesdr commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

Public qualification image: ghcr.io/yatesdr/jovian-judgement-glm53-lmcache:r30.1-lmcache-pr65-da7df929

OCI index digest: sha256:6f2f098ebc54ebd75fa90cd7fe79927101f36aeb53a7e6132ba29b871df407fd

The package is public; anonymous manifest access was verified with an empty Docker credential directory.

@voipmonitor

Copy link
Copy Markdown

Validated and included in the two-layer R32 image, preserving Derek's original
authorship and signed commit. #65 remains the canonical review PR; no duplicate
implementation PR was created.

  • Independent real-storage owner-commit and owner-abort cases both fail on R31
    because the waiting generation is discarded; both pass with this patch.
  • 124 checkpoint/storage/index/identity/admission/retention tests pass against
    the installed image. Four independent ownership, deadline and shutdown
    checks also pass.
  • TP4/DCP1 MTP3, FP8 KV: exact 54,643-token answers through cold, GPU, RAM,
    filesystem and full worker/sidecar restart. Every restored prompt has zero
    recompute; RAM 0.272 s, filesystem 0.294 s, restart 0.409 s.
  • All 12 overlapping-prompt replays restore completely; distinct cache salts
    remain isolated misses. 6.95 GB of durable payloads through 4 GiB RAM retain
    a healthy allocator and restore the oldest 32K prompt without recompute.

This is one TP4 engine, not an independent reproduction of the author's
two-replica HAProxy topology. The strict bare-value formatting probe has two
format failures despite correct values. A separate MTP/JSON-schema test exposes
a grammar HTTP 500 on both R31 and R32; it is tracked in
vLLM #726 and is
not claimed fixed here.

Image: localinferencelab/vllm:jovian-judgement-community-20260909-r32.
Qualification and raw evidence.
vLLM, B12X, launchers and all 14 audited native libraries are unchanged from R31.
Use matching worker and sidecar images for the busy-admission response.

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- Pull request metadata ---'
gh pr view 65 --repo local-inference-lab/LMCache --json number,title,baseRefName,headRefName,files,url

printf '%s\n' '--- Changed files ---'
gh pr diff 65 --repo local-inference-lab/LMCache --name-only

printf '%s\n' '--- Diff stat ---'
git diff --stat "$(git merge-base HEAD "origin/$(gh pr view 65 --repo local-inference-lab/LMCache --json baseRefName --jq .baseRefName)")" HEAD

printf '%s\n' '--- Candidate checkpoint symbols ---'
rg -n -C 3 --glob '*.py' \
  'class CheckpointPayloadStore\b|def prepare_store\b|class CheckpointTransferWorker\b|CheckpointLeaseResponse\b|AdmissionFailure\b|KEY_NOT_WRITABLE\b' .

Length of output: 2540


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- Pull request diff ---'
gh pr diff 65 --repo local-inference-lab/LMCache --patch

printf '%s\n' '--- Source outlines ---'
for file in \
  lmcache/v1/distributed/admission.py \
  lmcache/v1/multiprocess/checkpoint_storage.py \
  lmcache/v1/multiprocess/checkpoint_transfer.py \
  lmcache/v1/multiprocess/modules/checkpoint.py \
  lmcache/v1/multiprocess/protocols/checkpoint.py \
  tests/v1/multiprocess/test_checkpoint_storage.py
do
  printf '\n--- %s ---\n' "$file"
  ast-grep outline "$file" --items all --type class,function --view expanded || true
done

printf '%s\n' '--- Focused source contexts ---'
rg -n -C 18 --glob '*.py' \
  'def prepare_store\b|class CheckpointPayloadStore\b|class CheckpointTransferWorker\b|def _.*store\b|CheckpointLeaseResponse\b|AdmissionFailure\b|KEY_NOT_WRITABLE\b|busy' \
  lmcache/v1/distributed/admission.py \
  lmcache/v1/multiprocess/checkpoint_storage.py \
  lmcache/v1/multiprocess/checkpoint_transfer.py \
  lmcache/v1/multiprocess/modules/checkpoint.py \
  lmcache/v1/multiprocess/protocols/checkpoint.py \
  tests/v1/multiprocess/test_checkpoint_storage.py

Length of output: 100122


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- Transfer worker implementation ---'
nl -ba lmcache/v1/multiprocess/checkpoint_transfer.py | sed -n '100,360p'

printf '%s\n' '--- Checkpoint page grouping and store admission ---'
nl -ba lmcache/v1/multiprocess/checkpoint_storage.py | sed -n '40,390p'

printf '%s\n' '--- Storage reservation API and error semantics ---'
rg -n -C 12 --glob '*.py' \
  'def reserve_write_detailed\b|reserve_write_detailed\(|class L1Error\b|KEY_NOT_WRITABLE|KEY_NOT_EXIST|OUT_OF_MEMORY' \
  lmcache tests

printf '%s\n' '--- Busy-status and store-timeout tests or handling ---'
rg -n -C 8 --glob '*.py' \
  'status == "busy"|status in \("miss", "busy"\)|AdmissionFailure\.BUSY|rpc_timeout|_DrainedCheckpointLeaseTimeout' \
  lmcache tests

Length of output: 238346


@voipmonitor I found one blocking issue.

Busy retry can exceed its deadline. In lmcache/v1/multiprocess/checkpoint_transfer.py, Line 238 calls _call() with its full self._rpc_timeout after checking remaining. A CHECKPOINT_PREPARE_STORE request that starts just before the busy-retry deadline can block for the full RPC timeout. If it times out, _call() can wait for a second full timeout to drain the request safely.

This does not meet the stated contract that deadline expiry returns a negative rank result. Pass the remaining busy-retry budget into the request wait, or prevent a retry request when it cannot finish within that budget. Keep the existing late-response reconciliation path for requests that already reached the server. Add a test where a busy retry reply is delayed past the retry deadline.


✏️ Learnings added
Learnt from: voipmonitor
URL: https://github.com/local-inference-lab/LMCache/pull/65

Timestamp: 2026-09-09T23:51:10.765Z
Learning: For `lmcache/v1` checkpoint publication, `CheckpointLeaseResponse("busy")` is a compatibility-sensitive RPC response. Deploy matching checkpoint transfer worker and sidecar images when this response is enabled.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

You are interacting with an AI system.

@voipmonitor

Copy link
Copy Markdown

Community integration receipt for #65: this exact PR head is already merged into integration/local-inference-lab through 2fa5d10e.

The original contributor commits are preserved. Exact head ancestry was verified, not inferred from equivalent file contents. Closing this dev-targeted review because its community integration is complete; this is not a claim that it merged into dev. Further community changes must target integration/local-inference-lab. GitHub cannot retarget an already-contained head because there are no new commits to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

R30 LMCache: concurrent deduplicated boundary stores lose checkpoint generations

2 participants