ariel-miculas opened a new issue, #23604:
URL: https://github.com/apache/datafusion/issues/23604

   ### Describe the bug
   
   prev_cursors in Sort Preserving Merge keeps an additional Cursor alive 
during the merge operation;
   This is particularly problematic in the context of a single-column sort when 
merging streams read from spill files in IPC format, because in this case, due 
to https://github.com/apache/arrow-rs/issues/6363, the single sort column kept 
alive by prev_cursors prevents an entire Record batch from being dropped. This 
is particularly problematic with large Record batches, leading to an excess in 
the observed peak memory.
   
   ###  Practical experiment:
   with round robin tie-breaking feature disabled and this patch:
   ```
     /// Advances the actual cursor. If it reaches its end, update the
       /// previous cursor with it.
       ///
       /// If the given partition is not exhausted, the function returns `true`.
       fn advance_cursors(&mut self, stream_idx: usize) -> bool {
           if let Some(cursor) = &mut self.cursors[stream_idx] {
               let _ = cursor.advance();
               if cursor.is_finished() {
                   if self.enable_round_robin_tie_breaker {
                       self.prev_cursors[stream_idx] = 
self.cursors[stream_idx].take();
                   } else {
                       self.cursors[stream_idx] = None;
                   }
               }
               true
           } else {
               false
           }
       }
   ```
   
   Observed peak memory (measured directly from the Rust allocator, not by 
jemalloc or other allocator stats) goes down from 425 MiBs to 347 MiBs, which 
is two Record batches worth of memory (each record batch is 39 MiBs).
   In this experiment I'm always merging two streams read from the spill files 
via multi level merge.
   
   ### To Reproduce
   
   _No response_
   
   ### Expected behavior
   
   prev_cursors only needs the last row from the cursor, so why does it have to 
keep the entire cursor?
   
   ### Additional context
   
   Another similar issue, where extra Record Batches lead to increase in peak 
memory: https://github.com/apache/datafusion/issues/23559


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