fix(checkpoint): wait for restore admission under capacity pressure (#100 + review fixes) - #103
Merged
voipmonitor merged 5 commits intoSep 28, 2026
Conversation
Signed-off-by: Costa Tsaousis <costa@netdata.cloud>
Signed-off-by: Costa Tsaousis <costa@netdata.cloud>
Follow-up review fixes for #100: - Detect vLLM restore admission reservations once, when the bridge is created (the admission methods and the reserve_admission parameter). Without them, log one warning and keep the previous fallback: a restore without free GPU blocks is admitted and retried while unadmitted, one without a copy slot recomputes, and no missing method is ever called (finish_request no longer raises on every finished request). - Look a selected checkpoint up again only when no local checkpoint at least as long is cached, not whenever a different one replaced it. - Log a capacity wait when it starts and every 30 s with its age, the free GPU blocks and the blocks the checkpoint needs (or the copies in flight); report_status() counts current waits, the longest wait and the waits started. Every way out of a wait releases vLLM's waiter entry. - A restore that retains its answer for half the lookup timeout sends the lookup again without waiting for it, keeping its pages recent; the reply replaces the retained answer, and an empty one recomputes the prompt. - A complete local hit again bypasses a pending directory lookup, except behind the request's own reserved copy, and settles the lookup so a later eviction looks the prefix up again. - Validate an answered manifest once per answer, and log an invalidated publication without the "missed; looking up a shorter checkpoint" line. - Tests run on vLLM with and without the admission API: reservation-only cases skip with a reason, the fallback is tested through an allocator without the API, and exits assert that no restore stays queued. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Scope the fragment to the request-boundary recurrent models, document the fallback with an older vLLM, the per-reply lookup timeout (up to four replies for a request whose restores fail), wait logging, lookup refresh and the selection and local-hit rules. It still requires the paired vLLM fragment vllm-restore-admission. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
voipmonitor
marked this pull request as ready for review
September 28, 2026 16:05
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is #100 by @ktsaou (commits unchanged) plus fixes for our review findings. It pairs with vLLM LMCache#929.
What #100 does. When a restore cannot reserve its GPU pages, the bridge keeps the answered manifest and waits for capacity. Before, it admitted the request to recompute its whole prompt ("admitted without the restore").
Review fixes on top:
can_admit_external_boundary_request,external_boundary_admission_ready,release_external_boundary_admission, and thereserve_admissionparameter. Without them it logs one WARNING and keeps the previous behaviour. Before this, an older vLLM hit an AttributeError on every finished request, which killed the engine.report_status()counts waits, and the waiter entry is cleared on every exit path.lookup_timeoutsends the lookup again, so the checkpoint's pages stay recent.Tests (CPU).
test_checkpoint_multimodal_roots) fails on the base too.E2E (with vLLM LMCache#929; setup and table in vllm#929): 8 agents whose contexts outgrow the KV pool.
The bridge logged 648 "waiting for GPU capacity" lines and no "admitted without the restore".
🤖 Generated with Claude Code