Skip to content

fix(offload): make budgeted priority save admission mandatory - #2246

Closed
yhl-amd wants to merge 5 commits into
mainfrom
fix/dsv4-save-admission
Closed

yhl-amd wants to merge 5 commits into
mainfrom
fix/dsv4-save-admission

Conversation

@yhl-amd

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

Copy link
Copy Markdown
Contributor

Summary

  • make value-ranked priority admission the only chunked LMCache save policy; remove OFFLOAD_SAVE_POLICY and all save-queue round-robin state/branches
  • keep the existing process-wide OFFLOAD_MAX_PENDING_SAVES guard, counting a DSV4 PAGE+SLOT checkpoint as one worker operation
  • rank candidates using rank-local prefix demand, dirty-save cost, aging, and finished-request release value
  • bind each offload scheduler to its local BlockManager and enforce a real physical PAGE-block budget:
    • OFFLOAD_SAVE_MAX_PINNED_RATIO (default 0.20, maximum 0.30)
    • optional OFFLOAD_SAVE_MAX_PINNED_BLOCKS
    • effective budget = min(floor(local_blocks × ratio), absolute_limit)
  • allow a higher-scored candidate to atomically replace one or more lower-scored, undispatched committed saves
  • never evict an inflight PAGE or SLOT save; PAGE+SLOT admission, replacement, and cleanup remain atomic
  • count shared physical block IDs once and conservatively charge DSV4's full retained block table because this branch does not yet have partial PAGE source release
  • export save candidates, reservations, pinned usage, rejection/eviction/oversized counters, wait time, and priority score through engine and Prometheus metrics
  • drop a finished save that cannot be admitted instead of pinning its KV indefinitely

This ports the reusable priority/budget work into #2246 without merging the #2250-only native DSV4/LMCache-MP state scheduler changes.

Why

The original DSV4 fix bounded the number of scheduler-dispatched saves, eliminating worker-side rejection storms, but a count-only queue still lets a few large finished requests retain an unbounded fraction of the GPU KV pool. It also treats every save as equally valuable.

The admission layer bounds actual scheduler-local physical blocks and spends that budget on the saves with the highest expected reuse value. A newly valuable prefix can replace lower-value work only while that work is still committed and undispatched. Victims are simulated as a complete set before state is mutated, so a candidate that still cannot fit evicts nothing.

Round-robin admission has been removed rather than retained as a compatibility mode. This avoids maintaining two different completion, deferral, and cleanup semantics: every chunked offload save now follows the same candidate -> committed -> inflight lifecycle.

The budget is DP-local. TP ranks shard the contents of the same logical block IDs, so the capacity is neither aggregated across DP pools nor multiplied by TP world size.

Existing DSV4 admission result

The count-bound change already removed the observed save rejection/rollback storm in the TP8 A/B run:

Metric Offload off Offload on after count-bound fix
Successful requests in strict 300 s window 91 90
Request throughput 0.3033 req/s 0.3000 req/s
Total token throughput 24,544.45 token/s 24,203.56 token/s
TTFT p50 0.673 s 0.850 s
TTFT p95 1.731 s 1.799 s
TPOT p50 8.37 ms 9.50 ms
TPOT p95 12.30 ms 18.78 ms
Prompt cache-read fraction 93.54% 93.46%

After that fix: 417 scheduler save operations, 10,659,072 saved tokens, zero max_pending_saves rejections, zero PAGE watermark rollbacks, and pending saves mean/peak 0.38/2. The run recorded no LMCache loads, so it validates removal of the save storm rather than a CPU-load speedup.

Validation

  • priority/budget/PAGE+SLOT admission tests: 23 passed
  • Prometheus exporter tests: 4 passed, 1 deselected
  • python3 -m py_compile: passed
  • Ruff: passed
  • Black: passed
  • git diff --check: passed

The current local host does not provide CPU PyTorch, so the full repository suite is left to PR CI; the targeted connector tests were run with import-only dependency stubs.

@github-actions

Copy link
Copy Markdown
Contributor

🏷️ CI Guide

Runs automatically on every eligible PR before approval:

  • ✅ Pre Checkin: Black, Ruff, catalog schema validation, non-GPU unit tests

Heavy model tests:

  • ✅ Run after the PR is approved and Pre Checkin passes
  • ✅ Run immediately when an approval review is submitted
  • ✅ Can be requested before approval with labels
Label Tests
ci:full Run all heavy PR model tests: native ATOM, vLLM, and SGLang
ci:atom Run native ATOM model accuracy tests
ci:vllm Run ATOM vLLM OOT model accuracy tests
ci:sglang Run ATOM SGLang model accuracy tests

Heavy jobs are skipped when the PR is not approved and no matching ci:* label is present.
Add labels via the sidebar or gh pr edit 2246 --add-label <label>

@yhl-amd yhl-amd changed the title fix(offload): bound DSV4 save admission fix(offload): add budgeted priority save admission for DSV4 Sep 21, 2026
@yhl-amd yhl-amd changed the title fix(offload): add budgeted priority save admission for DSV4 fix(offload): make budgeted priority save admission mandatory Sep 21, 2026
yhl-amd added a commit that referenced this pull request Sep 22, 2026
# Conflicts:
#	atom/kv_transfer/offload/chunked_scheduler.py
@yhl-amd

yhl-amd commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #2250. The complete budgeted priority save-admission implementation has been merged into #2250, which now targets main directly.

@yhl-amd yhl-amd closed this Sep 22, 2026
@yhl-amd
yhl-amd deleted the fix/dsv4-save-admission branch September 22, 2026 07:04
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.

1 participant