Do we need bounded channels in RepartitionExec?

[adriangb]

Now that we have a good spilling implementation in #18207, do we still want bounded channels for the in-memory data? This was first introduced in #4867.

My feeling is that we could probably drop it. The one situation I am worried about is when we fill RepartitionExec's buffers and consume the entire memory budget then the query fails. i.e. if we could do "cooperative" spilling RepartitionExec would be the ideal candidate to spill and this would not be a problem (an upstream GroupBy could ask other operators to spill, RepartitionExec would spill easily and free up memory). But today that's not the case.

One way to collect more information is to run ClickBench (and other benchmarks) w/o the Distribution infrastructure and compare runtimes, peak memory use and behavior under constrained memory budgets.

cc @Dandandan @alamb @crepererum @2010YOUY01

[alamb]

Another important property of the current channel implementation is that it has back pressure on the producers (e.g. scans) if the consumers aren't consuming at a fast enough rate

So specifically if one consumer is consuming all its data very fast but others are not, it is important not to drive the producers unless it is strictly required to make progress.

[adriangb]

Right I understand that, but I wonder if it wouldn't be better to just let everything spill?

[crepererum]

What about users that don't have a disk to spill to?

[adriangb]

What about users that don't have a disk to spill to?

Yeah that's a very good point.

I think I'm going to close this for now - it seems there are enough good reasons to keep the current behavior