Repository navigation
feat(allocator): add independent atomic budget observations - #590
Open
shipiyouniao wants to merge 1 commit into
Open
shipiyouniao wants to merge 1 commit into
shipiyouniao wants to merge 1 commit into
Conversation
shipiyouniao
force-pushed
the
codex/coordinator-observation-20261019
branch
from
October 19, 2026 03:12
ed864a9 to
a7f9964
Compare
shipiyouniao
marked this pull request as ready for review
October 19, 2026 03:18
RixinLiu
self-requested a review
October 20, 2026 03:58
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.
Adds independent coordinator observations on top of #487, now merged into
mainatd100fa7471d1825f693d7ba95d3174d2ab2c726d. 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 withPROT_READ, copies atomic fields, and closes the mapping before returning. It does not initialize CUDA, take the ledgerflock, 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
incompleteis 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=Falsedoes 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 snapshotAfter: 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 sampleCompatibility 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 commita7f9964rebases 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
a7f9964passed 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.