[shipiyouniao]

Concurrent TP/PP KV growth can leave one worker mapped while another rejects a reservation or loses its reply. This PR coordinates prepare/commit/finalize outcomes, retains confirmed mappings for the same page batch, and fails closed when bounded state queries cannot establish an outcome. Successful peers do not physically unmap their mappings as compensation. Native reservations continue to use #487's per-GPU grants/refunds.

A generic native commit exception remains terminal unless the protocol has established its outcome; it is not assumed to be a retained partial map. Completed operations survive response loss through the registry. Mapping and device synchronization remain outside shared budget bookkeeping.

The merge with current main routes resize through RuntimeControl. RetainedResizeError keeps the published revision pending and permits allocation to adopt the retained batch before retrying. This applies both when consuming a revision and when completing a deferred shrink. It neither acknowledges the revision early nor reports the recoverable batch as quarantined. A newer publication can replace the pending revision.

Before this refresh, the PR's retry handling lived in the legacy watcher path:

flowchart LR
  W[Legacy resize watcher] --> M[Manager applies target]
  M --> R[Allocator resize]
  R -->|Retained batch| P[Keep target retryable]
  P --> A[Allocation adopts same batch]
  A --> M

Loading

After integrating main's runtime-control authority:

flowchart TD
  P[Per-GPU revision publication] --> C[RuntimeControl polls current revision]
  C --> R[Resize or deferred shrink completion]
  R -->|Retained batch| W[Leave revision pending and allow allocation]
  W --> A[Adopt the same retained batch]
  A --> C
  R -->|Completed| K[Acknowledge the executing revision]
  R -->|Quarantined pages| F[Report failure for the executing revision]

Loading

The refresh preserves main's control-worker commands, logical limits, lifecycle/readiness state, offload copy pins, retirement fences, pending-copy clear protection, and the charged null page under a zero quota. Both test manifests retain their additions. Six constructed regressions cover retained resize during initial application, an existing shrink and deferred completion, with and without a newer revision.

Tests from #574 remain the independent commit 2cc3240, preserving their original author and co-author. #488 and #489 remain optional successors.

Private validation of b0fb7857480208fae76a2f69417e99340b42deda: Python 3.10, 3.11, 3.12 and 3.13 CPU runs each passed 1,670 tests with 15 skips. The rebuilt native extension passed 112 tests on one T4; three tests requiring two GPUs were skipped. C++ checks, all-file pre-commit and MyPy targets 3.10-3.13 passed. The source archive was verified byte-for-byte against this commit.

Dedicated Codex review identified two pre-existing P2 availability concerns for further review: idle resize does not itself drive retained-batch recovery, and a retained batch that cannot fit may block a smaller fresh allocation. They are unchanged by this conflict refresh. Constructed single-GPU probes used simulated peers and stopped preallocation after creating the retained batch; they support the local code paths, not a production or multi-GPU reproduction. These concerns remain unresolved, and #488/#489 are not claimed to cover them.

#467's controlled same-parameter A/B is historical evidence for the original design's performance parity. It is not performance or multi-GPU TP/PP acceptance evidence for this head.

Closes #467

[Copilot]

Pull request overview

This PR adds a cross-process serialization boundary around physical KV growth so that concurrent engines sharing the same physical GPU don’t admit overlapping growth transactions from the same stale free-memory snapshot. It keys a file lock by the GPU PCI bus ID, refreshes mem_get_info under that lock, and keeps the lock held across the full batch mapping call; it also documents the shared lock-directory requirement for multi-container deployments.

Changes:

• Add a GPU-PCI-bus-ID keyed file lock to serialize physical KV growth admissions across processes.
• Re-check free memory headroom under the lock immediately before mapping, and hold the lock through the full batch map.
• Document KVCACHED_PHYSICAL_GROWTH_LOCK_DIR usage/constraints in the example.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
examples/01_simple_two_models/README.md	Documents the new lock directory env var and the shared-filesystem requirement.
csrc/inc/gpu_vmm.hpp	Exposes a backend-agnostic wrapper for querying GPU PCI bus ID.
csrc/inc/allocator.hpp	Tracks bytes-per-offset to compute per-transaction required physical growth.
csrc/allocator.cpp	Implements the physical-growth guard (file lock + headroom admission) and applies it around batch mapping.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[RixinLiu]

The problem is real. I reproduced it on an A100 through the normal KVCacheManager.alloc() path: a peer takes 25 GiB mid-allocation, csrc/page.cpp:14 reports out of memory, the process exits 134, and an except BaseException wrapped around alloc() never runs. But I don't think a pre-admission lock is the right shape for the fix.

Below is my understanding, feel free to correct me if sth is wrong:

1. Right now a failure during allocation has two possible endings. If it raises, alloc_page() catches it, _alloc() rolls back the partial blocks and returns None, and fix(vllm): report an exhausted KV pool as a scheduling miss, not a crash #453 turns that into a scheduling miss, the engine keeps serving. If it's physical exhaustion, the process just dies. The only thing separating those two endings is that csrc/page.cpp:14 calls CHECK_GPU(mem_create(...)), which aborts, instead of raising.
2. And the lock doesn't save that rollback anyway. PhysicalGrowthGuard's constructor throws from the top of map_to_kv_tensors(); GPUPage's constructor throws from the middle of the same function. Both end up in the same catch in alloc_page() and the same _rollback_partial_alloc(). So what the lock actually saves is one failed cuMemCreate call, and what it costs is an open + flock + set_device + mem_get_info on every single mapping, including all the ones that would have succeeded.
3. So my suggestion is: land the abort → raise change first, then measure the colocated case again. If failures turn out to be frequent enough to hurt throughput (which then can be called as a starvation problem), let's talk about admission control then.

[shipiyouniao]

Thanks, I agree with the sequencing and cost concern. Your reproduction also confirms that the fatal gap is not the Python rollback itself, but CHECK_GPU(mem_create(...)) aborting before that rollback can run.

The abort-to-exception part is already covered by #418 together with the VMM batch rollback it requires: failures from physical allocation, mapping, or access setup restore the zero-page mapping and roll back earlier mappings in the batch. The exception can then reach PageAllocator / KVCacheManager, and #453 turns exhaustion into a scheduling miss. Changing only page.cpp without the batch transaction would still leave partially mapped layers or offsets.

I will close this admission-lock PR and treat #418 followed by #453 as the first-line fix. After #418 lands, admission control should only be reconsidered if colocated pressure tests show repeated allocation misses causing measurable starvation or throughput loss. This keeps open / flock / set_device / mem_get_info off every successful mapping.

[shipiyouniao]

Reopened this as a draft and collected the new correctness, starvation, and performance evidence in #467. The rollback-only path still reaches repeated physical-page OOM under sustained colocated pressure, while the narrower admission variants we tested regress throughput or fairness. I am keeping the original transaction boundary here as the best current reference design, not as a merge-ready conclusion.

[shipiyouniao]

@RixinLiu This has been substantially reworked and is ready for another protocol review.

Your earlier concerns are now addressed as follows:

• The abort-to-exception and batch rollback path from fix: make KV tensor VMM batches transactional #418 remains the first-line failure handling. Admission is not a substitute for rollback; it is based on the reproduced sustained allocation-miss and starvation case documented in Design: safe cross-process admission for concurrent physical KV growth #467.
• The fair cross-process turn now covers only a fresh physical-capacity check and CUDA physical-handle reservation. Mapping, synchronization, and inference are outside the critical section. A same-offset orphan adoption that needs no new physical page now skips the admission lock and memory query entirely.
• All TP/PP targets reserve before mappings are committed concurrently. Partial or outcome-unknown successes are retained and adopted by a same-offset retry instead of being physically unmapped online.
• The two earlier validation details were rechecked: non-finite utilization values are rejected, and lock/ticket files use no-follow opens.

The strict same-configuration A/B result is 4034.74 versus 4098.53 tok/s aggregate makespan throughput (+1.58%), with 822/822 successful requests. The two fault-injection cases also completed without restarts, CUDA errors, or unreconciled mappings. The exact final commit was rebuilt on a T4 and passed the C++ suite, 231 CPU tests, MyPy on Python 3.9-3.13, 83 applicable GPU/VMM tests (1 skipped), and a same-GPU two-process reserve/map/unmap smoke.

The PR remains a draft only because its prerequisite changes are still open; the current transaction and admission design is ready for review.