Skip to content

fix: account fair-pool memory across sibling reservations - #5847

Draft
sunchao wants to merge 2 commits into
apache:mainfrom
sunchao:codex/upstream-consumer-accounting
Draft

fix: account fair-pool memory across sibling reservations#5847
sunchao wants to merge 2 commits into
apache:mainfrom
sunchao:codex/upstream-consumer-accounting

Conversation

@sunchao

@sunchao sunchao commented Sep 10, 2026

Copy link
Copy Markdown
Member

Which issue does this PR close?

Addresses the fair-memory-pool accounting in #5212. Overlaps #5466 and needs merge-order coordination with #5613.

Rationale for this change

The fair pool can force operators to spill even when they have room within their own allowance. Consider a 32 MiB pool shared by two consumers, each entitled to 16 MiB. If one holds 10 MiB and the other holds 6 MiB, the second should be able to acquire another 10 MiB. Today, Comet compares the pool's combined 16 MiB usage against that consumer's 16 MiB allowance and rejects the request. With more consumers, this can leave an increasingly large part of the configured pool unusable.

The correction also needs to distinguish a consumer, which owns the allowance, from its reservations, which account for individual buffers. One operator can hold several reservations under the same consumer registration. Checking only the reservation making the request would still let a consumer whose sibling reservations already hold 10 MiB and 6 MiB acquire more through an empty sibling, despite having exhausted its 16 MiB share.

There is a separate total-capacity constraint. An operator might fill the pool before a second consumer registers. The newcomer has a fair share on paper, but cannot allocate until some existing memory is released.

What changes are included in this PR?

The pool now treats all reservations belonging to one consumer as a single account. Growth is admitted only when it fits both that consumer's fair share and the space remaining in the whole pool. Splitting a reservation, moving its contents, or creating an empty sibling preserves the existing charge; releasing memory restores the consumer's available allowance. The pool also reports its configured finite capacity.

This preserves the current policy of dividing the pool among all registered consumers; changing how spillable consumers participate remains in #5465. This overlaps the fair-share correction in #5466 and extends it to cover siblings and total capacity. #5613 separately changes locking around JVM memory calls. These changes need an agreed merge order so its admission and rollback paths retain the same accounting guarantees.

How are these changes tested?

Seven regression tests exercise the actual pool admission logic with DataFusion reservations and simulated JVM grants. They cover the examples above, reservation splitting and transfer, release and retry, mixed consumers, oversized requests, and failed or partial grants.

The original PR revision passed native Rust CI. Its only failed test was a shared decimal codegen coverage assertion, which is corrected by upstream #5849 and now included in this branch. The assertion was reproduced before the correction and passed afterward in a focused local run; local formatting checks also passed.

See fresh CI for revision 865529d4 for validation of the updated branch. A dedicated Spark off-heap integration run has not been performed.

@github-actions github-actions Bot added bug Something isn't working area:memory Memory pools, reservations, OOM handling labels Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:memory Memory pools, reservations, OOM handling bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant