kumarUjjawal commented on code in PR #23619:
URL: https://github.com/apache/datafusion/pull/23619#discussion_r3735722372
##########
datafusion/core/tests/fuzz_cases/spilling_fuzz_in_memory_constrained_env.rs:
##########
@@ -524,13 +505,7 @@ async fn
run_sort_preserving_merge_peak_memory_with_spilled_input(
// BatchBuilder needs to hold 3 Record batches simultaneously to merge two
// streams (because a stream can cross a record batch boundary)
// there is also one cursor needed per stream
- let mut max_peak = 3 * ipc_batch_size + 2 * cursor_unit + converter_size;
-
- // with round robin enabled, 2 extra cursors live in memory
- // see https://github.com/apache/datafusion/issues/23604
- if round_robin {
- max_peak += 2 * cursor_unit;
- };
+ let max_peak = 3 * ipc_batch_size + 2 * cursor_unit + converter_size;
Review Comment:
Could we reduce ReusableRows to one slot here, keep the spare buffer
reserved, or scope this optimization and test to the single-column path?
##########
datafusion/physical-plan/src/sorts/merge.rs:
##########
@@ -435,7 +436,7 @@ impl<C: CursorValues> SortPreservingMergeStream<C> {
// Take the current cursor, leaving `None` in its place
let taken = self.cursors[stream_idx].take();
if let Some(prev_cursors) = &mut self.prev_cursors {
- prev_cursors[stream_idx] = taken;
+ prev_cursors[stream_idx] = taken.map(|c| c.last_value());
Review Comment:
should we reserve the retained value before storing it and make the
allocation path fallible?
##########
datafusion/core/tests/fuzz_cases/spilling_fuzz_in_memory_constrained_env.rs:
##########
Review Comment:
we should keep at least the single-column no-round-robin case added by
#23606? Removing all three disabled-mode cases drops the only direct
peak-memory regression coverage for that fix
--
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]
