kosiew commented on code in PR #24820:
URL: https://github.com/apache/datafusion/pull/24820#discussion_r4011722333
##########
datafusion/physical-plan/src/joins/nested_loop_join.rs:
##########
@@ -1945,8 +2042,9 @@ impl FallbackCoordinator {
}
let _build_timer = build_time.timer();
- let merged_batch = concat_batches(&left_schema, &pending_batches)?;
- let n_rows = merged_batch.num_rows();
+ // Every batch of the pass is reserved above, so compacting it here
stays within budget.
+ let chunks = coalesce_chunks(pending_batches, &left_schema,
target_batch_size)?;
Review Comment:
I think this can exceed the configured memory limit during replay.
`load_one_chunk` keeps reserving decoded input batches until `try_grow`
fails, and those batches are still live when they are passed to
`coalesce_chunks`. The coalescing step then allocates the concatenated output
without acquiring an additional reservation for that copy.
For a chunk made up of many small or wide batches, this means we can
temporarily hold roughly both the fully reserved inputs and another full copied
output. That brings back the same kind of memory amplification the earlier
spill handling was trying to avoid, just on the replay side.
Could we either preserve the replay batches at their existing granularity,
or explicitly limit or reserve the coalescing allocation before materializing
the combined batch?
--
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]
