peterxcli opened a new pull request, #5466: URL: https://github.com/apache/datafusion-comet/pull/5466
## Which issue does this PR close? Addresses finding 1 and the fair-pool portion of finding 16 in #5212. Follow-up #5465 tracks spillable versus unspillable consumer accounting separately. ## Rationale for this change `CometFairMemoryPool` calculates a per-consumer fair share but compares it with pool-wide usage. With a 32 MiB pool and two consumers, one reservation holding 10 MiB and the requester holding 6 MiB make total usage equal the 16 MiB fair share. Main then rejects any positive growth from the requester, even though that requester has 10 MiB of fair-share headroom. The fair pool also has a fixed configured capacity, so reporting an unknown memory limit is inaccurate. ## What changes are included in this PR? - Compare the fair share with the requesting reservation's size plus the requested growth. - Report `MemoryLimit::Finite(pool_size)` for `CometFairMemoryPool`. - Add a focused regression test for the 10 MiB + 6 MiB reservation case and finite-limit reporting. - Preserve the existing policy of counting all registered consumers. Restricting the count to spillable consumers is the separate policy and annotation change tracked by #5465. ## How are these changes tested? Focused native test: ```shell DYLD_LIBRARY_PATH=/opt/homebrew/Cellar/openjdk/26.0.1/libexec/openjdk.jdk/Contents/Home/lib/server RUSTFLAGS=' -Ctarget-cpu=native' cargo test --release -p datafusion-comet fair_share_uses_requesting_reservation_and_reports_pool_limit --lib ``` Formatting and diff checks: ```shell cargo fmt --all -- --check git diff --check ``` A warmed local A/B used `local[1]`, a 32 MiB fair pool, one sort partition, three warmups, and seven measured runs: | Workload | Spills | Native spilled bytes | Wall time, median (range) | Max task, median (range) | | --- | ---: | ---: | ---: | ---: | | 200K rows | 5 → 2 | 13,014,520 → 13,013,008 | 92 (78–121) → 99 (87–115) ms | 51 (44–65) → 54 (51–76) ms | | 2M rows | 43 → 16 | 232,467,592 → 130,125,664 | 406 (372–507) → 369 (326–632) ms | 371 (338–481) → 341 (300–603) ms | Spill counts and bytes were identical across the seven runs for each build. Runtime ranges overlap, so this PR makes no speed claim; the demonstrated benefit is restoring the requesting consumer's fair-share capacity and avoiding premature spills. -- 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]
