andygrove commented on PR #5847: URL: https://github.com/apache/datafusion-comet/pull/5847#issuecomment-5848807678
#6205 landed on the 25th, and after mbutrovich's review it became a per-consumer ledger rather than the per-reservation check its description still talks about. `CometFairPoolState.consumers` is keyed by `MemoryConsumer::id()`, so the reservations that `new_empty()`, `split()` and `take()` create all draw on one share, and `try_grow` also checks the pool total. That covers the 10+6 then +10 case, the sibling case and the late-registration case this PR set out to fix, and a refused or partial grant already leaves both counters untouched. So the ordering points in my earlier comments now belong to #5613's rebase onto the ledger rather than to this PR. As far as I can tell, what would be left after a rebase is the `memory_limit` override returning `MemoryLimit::Finite(pool_size)` and three tests that main doesn't have yet: `mixed_consumers_keep_the_existing_sharing_policy`, `failed_acquisition_does_not_change_consumer_or_pool_usage` and `overflowing_request_is_rejected_before_acquisition`. Do you want to rebase down to those, or close this one in favor of #6205? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
