Skip to content

feat(allocator): add independent atomic budget observations - #590

Open
shipiyouniao wants to merge 1 commit into
ovg-project:mainfrom
shipiyouniao:codex/coordinator-observation-20261019
Open

shipiyouniao wants to merge 1 commit into
ovg-project:mainfrom
shipiyouniao:codex/coordinator-observation-20261019

Conversation

@shipiyouniao

@shipiyouniao shipiyouniao commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

Adds independent coordinator observations on top of #487, now merged into main at d100fa7471d1825f693d7ba95d3174d2ab2c726d. This PR contains one commit and seven files.

Coordinator observation should not wait for the accounting lock or trigger a driver sample. The existing allocator-bound snapshot is unsuitable for that boundary, and simply removing its lock would race ordinary shared-field writes.

This adds kvcached.vmm_ops.observe_physical_budget(physical_device), taking the allocator's PCI bus ID. The reader opens an existing ledger read-only, maps it with PROT_READ, copies atomic fields, and closes the mapping before returning. It does not initialize CUDA, take the ledger flock, create or repair a ledger, or sample memory. The file pathname and layout remain private implementation details.

Publication and invalidation use atomics. Epoch-local generations bracket pool and instance identity reads so a slot reused during observation is omitted and incomplete is set, without retrying. Nonblocking OFD initialization/epoch leases protect mapping lifetime independently of the ledger lock. Missing, initializing, replaced or incompatible ledgers raise an error. This is not a claim that the entire path has no OS locks or a hard latency bound.

Fields can reflect different updates, including limit and revision. Even incomplete=False does not promise a coherent inventory. Charges include reservations and unconfirmed releases, not measured physical occupancy. Observation is not admission authority: admission checks and debits retain their existing common short lock.

Before: allocator-bound observation

sequenceDiagram
    participant C as Coordinator
    participant A as Allocator
    participant L as Ledger
    C->>A: Request existing budget state
    A->>L: Acquire accounting flock
    L-->>A: Read snapshot under lock
    A->>L: Release accounting flock
    A-->>C: Return snapshot
Loading

After: independent reader

sequenceDiagram
    participant C as Coordinator
    participant R as KVCached reader
    participant L as Ledger
    C->>R: observe_physical_budget with PCI ID
    R->>L: Open read-only and retain live epoch
    Note over R,L: Nonblocking OFD lifetime leases
    R->>L: Map read-only and load atomic fields
    R->>R: Check identities and omit changed slots
    R->>L: Unmap and release leases
    R-->>C: Return observations without cross-field consistency
    Note over R,L: No accounting flock and no sample
Loading

Compatibility decision

This implementation changes the private shared layout and magic. Old and new allocators cannot share a live ledger: incompatible connections are rejected. All old allocator workers sharing that ledger must exit before the new version initializes a fresh epoch. No manual file deletion is needed, and this is not a machine-wide or cluster-wide reboot requirement. The new epoch clears the old charges, limits and revisions, which are established again through startup registration and configuration. An old binary must not win the new-epoch initialization race.

@RixinLiu Are you OK with requiring a coordinated restart of the instances sharing this ledger for this layout change, or should this PR preserve compatibility with the pre-observation layout? This is the current implementation choice, pending your confirmation.

Validation

The native budget regressions, ASan/UBSan run, full extension build and compiled Python binding smoke passed in a private Linux environment. These include reading while another process holds the ledger lock, no sample or file mutation, slot reuse, concurrent lifecycle changes, unavailable/invalid-ledger errors, and confirming CUDA stays uninitialized during the binding smoke. These are constructed regressions, not a serving reproduction. A dedicated private Codex source review completed with no actionable findings.

Before the rebase, for commit ed864a9, the full GPU-free C++ suite, full pre-commit checks, strict MyPy for Python 3.10-3.13, extension build and Python smoke passed. Python 3.11 CPU tests passed with 1,234 passed and 12 skipped, and test classification passed. Checks left the source unchanged. The final commit a7f9964 rebases the identical observation patch onto merged #487. Its code and test tree is identical to the earlier PR merge-test tree, with only a documentation deletion inherited from main. Formatting, syntax and patch-equivalence checks passed after the rebase. The cached actionlint environment uses the configured version with the existing system-toolchain cache.

Final-head CI for a7f9964 passed all nine jobs: the Python 3.10-3.13 CPU matrix and classification, plus pre-commit/native tests and four MyPy versions. GPU serving, performance and the full supported engine/runtime matrix have not been tested for this change.

Refs #585, especially the requested observation boundary.

@shipiyouniao
shipiyouniao force-pushed the codex/coordinator-observation-20261019 branch from ed864a9 to a7f9964 Compare October 19, 2026 03:12
@shipiyouniao
shipiyouniao marked this pull request as ready for review October 19, 2026 03:18
@RixinLiu
RixinLiu self-requested a review October 20, 2026 03:58
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.

1 participant