Skip to content

fix(checkpoint): preserve disk checkpoints after L1 allocation failure - #105

Merged
voipmonitor merged 2 commits into
local-inference-lab:integration/local-inference-labfrom
ktsaou:fix/checkpoint-restore-progress
Sep 29, 2026
Merged

voipmonitor merged 2 commits into
local-inference-lab:integration/local-inference-labfrom
ktsaou:fix/checkpoint-restore-progress

Conversation

@ktsaou

@ktsaou ktsaou commented Sep 29, 2026 •

Copy link
Copy Markdown

A failed checkpoint restore can remove an intact disk checkpoint from the directory when L1 cannot allocate its load buffers. The retry code checks free RAM after the prefetch has completed; allocation padding or a concurrent writer releasing memory can make that later sample incorrectly look sufficient.

Carry the actual reservation-failure flag with the completed prefetch result, count aligned page allocations, and check the existing admission deadline before resubmitting. Capacity failures preserve the checkpoint for a later restore. Existing bitmap-only callers consume the same result through compatibility wrappers.

Regression tests cover both filesystem adapters: seven small pages whose aligned allocations exceed available RAM, and a writer that releases RAM between failed allocation and result consumption. In each case the checkpoint remains listed and restores byte-for-byte after pressure is removed. Both scenarios fail on the integration base and pass with this patch.

Validation: 55 existing checkpoint-storage tests and 8 new parameterized cases passed using real L1 allocation and both filesystem adapters. Nine independent edge-case checks also passed, including actual missing pages, partial contention, deadline expiry and both result APIs. The final image passed a 203-test CPU suite, including 160 forced disk restores within its progress probe. CI code-quality checks passed at 8a2d97b4. GPU-gated suites were not run against production GPUs.

This change addresses false checkpoint invalidation after allocation failure. It does not claim to fix an independently observed storage prefetch that stays pending through cancellation.

@coderabbitai

coderabbitai Bot commented Sep 29, 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: 7781b2d2-58fe-488a-a82f-0e7eec943094

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.

@voipmonitor

Copy link
Copy Markdown

Thanks, the bug is real on the current head. The store only got a bitmap and dropped the L1 reservation failure, so a restore whose RAM was held briefly, or was fragmented below one page, delisted an intact disk checkpoint. Our tests reproduce both cases on 75f2b59.

Merged onto the current head, this PR needed three follow-ups. They are in #108, which carries your two commits unchanged:

  1. Moving the admission-timeout check before "repeat once when there is room" stopped retiring checkpoints with a lost page file when the first lookup answered after the timeout, or with timeout 0. The timeout now bounds only repeats after a RAM failure.
  2. After a RAM failure the lease resubmitted on every poll, at 700–900 disk lookups/s for up to 8 s. It now waits 20 ms between repeats.
  3. fix(checkpoint): a restore stuck in storage misses instead of killing the engine #106's _stall_lookups helper patched query_prefetch_status, which the store no longer calls, so 3 tests failed. The helper now patches the detailed query.

#108 also adds regression tests and the lmcache-105 fragment: 276 passed. We'll merge #108 after a GPU run, which should make this PR redundant.

voipmonitor added a commit that referenced this pull request Sep 29, 2026
…ailure-listing

fix(checkpoint): keep intact disk checkpoints listed when a restore cannot get RAM (ktsaou's #105 + follow-ups)
@voipmonitor
voipmonitor merged commit c337fc8 into local-inference-lab:integration/local-inference-lab Sep 29, 2026
3 checks passed
@voipmonitor

Copy link
Copy Markdown

Landed through #108 (820af25ff6), with your two commits unchanged and the three follow-ups described above. Thanks for finding this. The regression tests reproduce both of your cases, RAM freed before the poll and fragmented RAM, on the old head.

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.

2 participants