jayzhan211 commented on code in PR #25250:
URL: https://github.com/apache/datafusion/pull/25250#discussion_r3998875639
##########
datafusion/physical-plan/src/joins/sort_merge_join/exec.rs:
##########
@@ -606,7 +606,11 @@ impl ExecutionPlan for SortMergeJoinExec {
let buffered = buffered.execute(partition, Arc::clone(&context))?;
let batch_size = context.session_config().batch_size();
+ // The stream spills its buffered batches when it cannot grow, so a
+ // pool that budgets spillable and unspillable consumers differently
+ // (`FairSpillPool`) has to know it can.
let reservation =
MemoryConsumer::new(format!("SMJStream[{partition}]"))
+ .with_can_spill(true)
Review Comment:
**What if we keep it false**
Under GreedyMemoryPool or UnboundedMemoryPool: nothing. Neither reads the
flag. A plain memory limit gives you the greedy pool, so most users would never
notice.
Under FairSpillPool, three things go wrong, all from one cause. The pool
budgets a consumer that cannot spill differently from one that can: it lets it
take free memory first come, first served, and subtracts whatever it holds
before dividing the rest evenly among the spillable consumers.
1. The join is not counted as a spillable consumer, so the sorts feeding it
split the pool as if the join needed nothing.
2. Every batch the join buffers is booked as unspillable, so it shrinks
every other spillable operator's share as it grows.
3. The join is never asked to spill until the whole pool is allocated.
What that looks like in practice. Take a 300 MB pool with two sorts feeding
the join. Registered as spillable, each of the three gets 100 MB. Registered as
unspillable, the sorts each get half of whatever the join has not taken. If the
join buffers 280 MB of one large key group, the sorts are down to 10 MB each,
which is the default sort_spill_reservation_bytes a sort must hold to merge its
spill files. Below that, the sort fails with Resources exhausted, and the query
fails inside an operator that was behaving correctly, when the join could have
spilled instead. Short of failure, the sorts spill more than necessary.
It also makes the outcome depend on timing. If the sorts hold memory first,
the join sees only the leftovers and spills early and often. If the join
buffers first, the sorts starve. Removing exactly that order dependence is what
FairSpillPool exists for.
For the fallback specifically, this is the shape every fallen-back partition
has: two external sorts plus this stream, sharing one pool. Keeping it false
would make the fallback's memory behavior under the fair pool unfair in the
same way.
--
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]
