kosiew commented on code in PR #26264:
URL: https://github.com/apache/datafusion/pull/26264#discussion_r4452808768
##########
datafusion/physical-plan/src/sorts/multi_level_merge.rs:
##########
@@ -1476,6 +1507,71 @@ mod tests {
Ok(())
}
Review Comment:
Could we optionally add a headroom-enabled case with a `5 * batch_memory`
pool and no `MergeMemoryPool`? Each run needs `2 * batch_memory`, so please
verify that the minimum two runs are admitted, the third is rejected, and
`reservation.size()` remains `4 * batch_memory`; the existing tests do not
distinguish this minimum-merge exception under a tight budget.
##########
datafusion/physical-plan/src/sorts/multi_level_merge.rs:
##########
@@ -611,7 +629,20 @@ impl MultiLevelMergeBuilder {
&& number_of_spills_to_read_for_current_phase
< minimum_number_of_required_streams;
let check_headroom = self.reserve_replay_headroom &&
!skip_headroom;
- let admission = if check_headroom {
+ let check_fan_in_headroom = !check_headroom
+ && self.reserve_fan_in_headroom
+ && number_of_spills_to_read_for_current_phase
+ >= minimum_number_of_required_streams;
+ let admission = if check_fan_in_headroom {
+ // Ask the pool for what this merge takes beyond its own
+ // workspace, plus as much again, then return the spare bytes.
+ match
total_needed.checked_add(total_needed.saturating_sub(owned)) {
Review Comment:
Could we optionally test admission with retained `MergeMemoryPool` workspace
`W`, no other live children or loans, and an empty child reservation? For an
optional run, cover `T <= W` and `T > W` with parent free space just below and
exactly at `2 * (T - W)`, verifying fan-in, child reservation `T` for admitted
runs, and final parent charge `max(W, T)` after temporary headroom is released.
--
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]
