pepijnve commented on code in PR #23522:
URL: https://github.com/apache/datafusion/pull/23522#discussion_r3593513151


##########
datafusion/physical-plan/src/spill/spill_pool.rs:
##########
@@ -476,6 +469,42 @@ pub fn channel(
     (writer, Box::pin(reader))
 }
 
+/// Creates a paired writer and reader for a spill pool with MPSC 
(multi-producer,
+/// single-consumer) semantics. See [`channel`] for the general architecture 
description
+/// of the spill pool.
+///
+/// Additional writers can be created by cloning the returned 
[`SharedSpillPoolWriter`].
+///
+/// In contrast to [`channel`], this implementation provides no guarantees 
regarding
+/// the read order of the returned [`SendableRecordBatchStream`].
+///
+/// If you need strict end-to-end FIFO (a single writer whose batches are read 
back in exact
+/// write order), use [`channel`] instead.
+///
+/// # File Management
+///
+/// The shared channel uses the same size-based rotation trigger as the 
[single producer channel](channel).
+/// All writers share the same pool of write files and coordinate file 
rotation. The number of open
+/// files is kept as small as possible. When more writes occur concurrently 
than there are open write
+/// files an additional file will be opened to write to. This prevents 
multiple writers from blocking
+/// each other.
+///
+/// When the last writer clone is dropped, it finalizes any remaining open 
write files so that all
+/// written data can be accessed by the reader.
+///
+/// # Returns
+///
+/// A tuple of `(SharedSpillPoolWriter, SendableRecordBatchStream)` that share 
the same
+/// underlying pool. The reader is returned as a stream for immediate use with
+/// async stream combinators. The writer can be cloned to create additional 
writers.
+pub fn shared_channel(

Review Comment:
   So... I completely forgot about backwards compatibility for a moment. The 
`spill_pool` module, `channel` function, and `SpillPoolWriter` type were all 
public. The changes we're making here are breaking for existing code. Should I 
reshuffle things a bit to maintain the MPSC capable variant under the original 
names and introduce new ones for the SPSC only variant instead?
   
   edit: I've gone ahead and done this already. Could use some help on naming 
these things. Best I could come up with is `SpillPoolWriter` for the MP version 
and `SpillPoolSink` for the non-clonable one. A bit too generic to my liking, 
but I didn't have anything better right away.



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