kumarUjjawal commented on code in PR #23619:
URL: https://github.com/apache/datafusion/pull/23619#discussion_r3735903111


##########
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:
   My concern is specific to the multi-column path, ReusableRows still owns 
both `Arc<Rows>` buffers, so dropping `RowValues` releases its reservation 
without freeing the spare buffer. The lower `max_peak` therefore shows lower 
accounting while actual row-buffer memory is unchanged.



-- 
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]

Reply via email to