Skip to content

fix(control): preserve quota watching and clear recovery - #559

Merged
RixinLiu merged 3 commits into
ovg-project:mainfrom
shipiyouniao:fix/zero-resize-target
Oct 22, 2026
Merged

RixinLiu merged 3 commits into
ovg-project:mainfrom
shipiyouniao:fix/zero-resize-target

Conversation

@shipiyouniao

@shipiyouniao shipiyouniao commented Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator

After syncing with main, this PR fixes two remaining lifecycle gaps: quota updates are not observed when page preallocation is disabled, and a retained zero quota can block clear() while it rebuilds the mandatory null block.

Validation on cac1459:

  • GitHub Actions: 9/9 checks passed; Python 3.10–3.13 each reported 1,514 passed and 14 skipped. (CPU matrix, pre-commit/MyPy)
  • T4 with a rebuilt extension: 118 passed and 3 dual-GPU cases skipped. Tests cover 16 combinations of preallocation, live blocks, null-block reservation and API takeover, with three zero/restore cycles each.
  • Private Codex review found no actionable regressions.

GPU validation covers single-device allocator/control behavior, not TP/PP serving or inference throughput. No repository documentation changes.

@rishabhsinha17 rishabhsinha17 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the project needs both fixes. with KVCACHED_PAGE_PREALLOC_ENABLED=false nothing on main watches the kvctl segment, and clear() under a retained zero (the value the native watcher keeps once kvctl publishes it) hangs. a cpu build of csrc with stub cuda shows main keeping get_resize_target at -1 with no watcher thread, while this head starts one watcher without the preallocation worker, keeps the value while stopped, picks up the next on restart and joins it on destruction with no stop. zero on the real manager over a page-level fake refuses the next page, keeps the live page with its free blocks reusable, and a raised quota grows the pool again, as on main since #552. clear() with a null block (block 0 kept for padding) hangs at main because resize(0) runs inside the reservation and the loop then waits for a page with the watcher stopped, here it finishes with null_block [0] and in_shrink True and recovers once kvctl raises the quota.

before merge it needs main, #552 merged the same >= 0 helper and the six -1 fakes, so the merge has one conflict in _alloc_impl plus a duplicate _apply_resize_target. i kept main's helper, #552's reclaim call after the branch and your placement inside it, cpu manifest 1323 passed on that tree with the failure set identical to main. second, test_quota_watcher_without_preallocation_stops_on_destruction asserts the /dev/shm file is gone after gc, and since #546 the owner only closes its fd and leaves the file for kvctl delete, test_native_close_preserves_idle_segment pins that. my cpu build of the merged csrc keeps the file after destruction and the gpu runner will fail that line. assert it still exists and unlink it, the exit code already covers the thread. #564's cap and #558 compose, the end of clear is one more pass through the helper. gpu files stay on your T4.

@shipiyouniao shipiyouniao changed the title fix(control): honor zero quotas without page preallocation fix(control): preserve quota watching and clear recovery Oct 20, 2026

@jeff3071 jeff3071 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR keeps quota watching active when page preallocation is disabled, cleans up the watcher when the allocator is destroyed, and reapplies pending resize targets after manager initialization and after clear() rebuilds the optional null block.

Tested on an A2000. Work as expected.

@RixinLiu RixinLiu left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shipiyouniao Floor the target at one page when the pool reserves a null block. The null page is always in use, so a target below one page can never be reached.
In the current implementation, the kvctl limit got deferred, so the shrink does not happen. So when clear restarts the pre-alloc thread. It will find there are empty pages in the free list, so it will pre-allocates some. But if we floor the limit to 1, the shrink can really happen, then the pre-allocates can't do anything

@shipiyouniao

Copy link
Copy Markdown
Collaborator Author

Fixed in c4b1390 (diff from the previous head). Pools with a null block now have a one-page physical shrink floor. The requested zero limit stays zero, and status still reports overage/deferred while the null mapping exceeds it. Deferred resize uses whole pages, avoiding truncation when block size does not divide page size.

On a T4, with 2 MiB pages, 2 layers and 2 KV buffers, the old code rebounded to 4 pages (32 MiB charged) after clear() restarted preallocation. The fix stays at 1 page (8 MiB charged), with free/reserved pages both zero.

  • All 32 zero-limit combinations passed; native tests: 134 passed, 3 dual-GPU cases skipped on the single T4.
  • Official CPU CI: Python 3.10-3.13 each had 1523 passed and 14 skipped. All nine automatic checks passed, including pre-commit/MyPy.
  • Dedicated Codex review completed with no actionable findings.

Official GPU serving CI was not run for this revision. Ready for another look.

@RixinLiu RixinLiu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@RixinLiu
RixinLiu merged commit 6fc5f58 into ovg-project:main Oct 22, 2026
9 checks passed
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.

4 participants