andygrove opened a new pull request, #25806: URL: https://github.com/apache/datafusion/pull/25806
## Which issue does this PR close? - Part of #25758 - Part of #25804 ## Rationale for this change When `ExternalSorter` spills, it frees its memory reservation before writing the sorted batches to the spill file, while those batches are still in memory. Until the writes finish, the memory pool can hand the same bytes to another consumer, so the process can use more memory than the pool allows. This is finding 3 in #25804. ## What changes are included in this PR? This PR backports the reservation fix from #24923 (@Phoenix500526) to the `branch-55` line. `ExternalSorter::consume_and_spill_append` now moves the reservation into a local that is released once the writes finish, on success or error, instead of freeing it before writing. #24923 itself adds an async spill-writing API, which is a new feature and not eligible for a patch release, so only this change is backported. ## Are these changes tested? Yes. `test_spill_reservation_held_during_write` uses a custom spill backend that records the pool's reserved bytes while batch data is written. Without the fix it fails (0 bytes are reserved during every write); with the fix it passes. On this branch I ran: - `cargo test -p datafusion-physical-plan --lib` - `cargo test -p datafusion --test core_integration memory_limit` - `cargo test -p datafusion --features extended_tests --test fuzz` for `sort_fuzz` and `spilling_fuzz` - `cargo clippy --all-targets --all-features -- -D warnings` ## Are there any user-facing changes? No. -- 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]
