kosiew commented on code in PR #24820:
URL: https://github.com/apache/datafusion/pull/24820#discussion_r4021992843
##########
datafusion/physical-plan/src/joins/nested_loop_join.rs:
##########
@@ -1190,11 +1249,13 @@ async fn collect_left_input(
// polling the child stream above.
let build_timer = metrics.build_time.timer();
- let merged_batch = concat_batches(&schema, &batches)?;
+ // Compacted only once the whole side is reserved, so a load that spills
never has a
+ // partially built chunk to materialize while the pool is exhausted.
+ let chunks = coalesce_chunks(batches, &schema, target_batch_size)?;
Review Comment:
I think there is still a memory-budget issue here on the non-spilling path.
`coalesce_chunks` runs while `batches` still owns all of the fully reserved
input batches, but the allocation for the concatenated chunk is not separately
reserved.
As a result, a build side that fits just below the pool limit can
temporarily hold both the full input and a near-full target-sized copied chunk.
Being fully reserved tells us that the input fits, but it does not account for
the output allocation that exists at the same time during coalescing.
I reproduced this by varying the allocator test to use 126 one-row 64-KiB
strings, a 9-MiB pool, and an empty right batch so the build stays in memory
and there is no probe cross-product contribution. Peak live allocation reached
15.84 MiB, above the test's 13.50-MiB bound.
The committed spill/replay test passes, but it does not exercise this
in-memory case. Could we either preserve the in-memory batches without copying
here, or reserve and limit the output materialization before doing the copy? We
should also make sure the ownership accounting remains exact after the input
batches are dropped.
--
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]
