Skip to content

Commit e2339fb

Browse files
Dandandanclaude
andcommitted
Defer task spawning in CoalescePartitionsExec to first poll
Previously, CoalescePartitionsExec eagerly spawned tasks for all input partitions in execute(). This meant that even if the output stream was never polled (e.g., query cancelled), all tasks would be spawned. This changes the multi-partition path to use a lazy stream that defers spawning until first poll_next(), reducing unnecessary work when streams are created but not consumed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 4084a18 commit e2339fb

1 file changed

Lines changed: 89 additions & 28 deletions

File tree

datafusion/physical-plan/src/coalesce_partitions.rs

Lines changed: 89 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,15 @@
1919
//! into a single partition
2020
2121
use std::any::Any;
22+
use std::pin::Pin;
2223
use std::sync::Arc;
24+
use std::task::{Context, Poll};
2325

2426
use super::metrics::{BaselineMetrics, ExecutionPlanMetricsSet, MetricsSet};
2527
use super::stream::{ObservedStream, RecordBatchReceiverStream};
2628
use super::{
27-
DisplayAs, ExecutionPlanProperties, PlanProperties, SendableRecordBatchStream,
28-
Statistics,
29+
DisplayAs, ExecutionPlanProperties, PlanProperties, RecordBatchStream,
30+
SendableRecordBatchStream, Statistics,
2931
};
3032
use crate::execution_plan::{CardinalityEffect, EvaluationType, SchedulingType};
3133
use crate::filter_pushdown::{FilterDescription, FilterPushdownPhase};
@@ -34,11 +36,14 @@ use crate::sort_pushdown::SortOrderPushdownResult;
3436
use crate::{DisplayFormatType, ExecutionPlan, Partitioning, check_if_same_properties};
3537
use datafusion_physical_expr_common::sort_expr::PhysicalSortExpr;
3638

39+
use arrow::datatypes::SchemaRef;
3740
use datafusion_common::config::ConfigOptions;
3841
use datafusion_common::tree_node::TreeNodeRecursion;
3942
use datafusion_common::{Result, assert_eq_or_internal_err, internal_err};
4043
use datafusion_execution::TaskContext;
4144
use datafusion_physical_expr::PhysicalExpr;
45+
use futures::Stream;
46+
use futures::StreamExt;
4247

4348
/// Merge execution plan executes partitions in parallel and combines them into a single
4449
/// partition. No guarantees are made about the order of the resulting partition.
@@ -209,33 +214,14 @@ impl ExecutionPlan for CoalescePartitionsExec {
209214
}
210215
_ => {
211216
let baseline_metrics = BaselineMetrics::new(&self.metrics, partition);
212-
// record the (very) minimal work done so that
213-
// elapsed_compute is not reported as 0
214-
let elapsed_compute = baseline_metrics.elapsed_compute().clone();
215-
let _timer = elapsed_compute.timer();
216-
217-
// use a stream that allows each sender to put in at
218-
// least one result in an attempt to maximize
219-
// parallelism.
220-
let mut builder =
221-
RecordBatchReceiverStream::builder(self.schema(), input_partitions);
222-
223-
// spawn independent tasks whose resulting streams (of batches)
224-
// are sent to the channel for consumption.
225-
for part_i in 0..input_partitions {
226-
builder.run_input(
227-
Arc::clone(&self.input),
228-
part_i,
229-
Arc::clone(&context),
230-
);
231-
}
232-
233-
let stream = builder.build();
234-
Ok(Box::pin(ObservedStream::new(
235-
stream,
217+
Ok(Box::pin(CoalescePartitionsStream {
218+
schema: self.schema(),
219+
input: Arc::clone(&self.input),
220+
context,
236221
baseline_metrics,
237-
self.fetch,
238-
)))
222+
fetch: self.fetch,
223+
state: CoalescePartitionsStreamState::Pending,
224+
}))
239225
}
240226
}
241227
}
@@ -352,6 +338,81 @@ impl ExecutionPlan for CoalescePartitionsExec {
352338
}
353339
}
354340

341+
/// A stream that lazily spawns input partition tasks on first poll,
342+
/// rather than eagerly in `execute()`.
343+
struct CoalescePartitionsStream {
344+
schema: SchemaRef,
345+
input: Arc<dyn ExecutionPlan>,
346+
context: Arc<TaskContext>,
347+
baseline_metrics: BaselineMetrics,
348+
fetch: Option<usize>,
349+
state: CoalescePartitionsStreamState,
350+
}
351+
352+
enum CoalescePartitionsStreamState {
353+
/// Tasks have not been spawned yet.
354+
Pending,
355+
/// Tasks have been spawned, polling the merged stream.
356+
Running(Pin<Box<ObservedStream>>),
357+
}
358+
359+
impl CoalescePartitionsStream {
360+
fn start(&mut self) -> &mut Pin<Box<ObservedStream>> {
361+
let input_partitions = self.input.output_partitioning().partition_count();
362+
363+
// record the (very) minimal work done so that
364+
// elapsed_compute is not reported as 0
365+
let elapsed_compute = self.baseline_metrics.elapsed_compute().clone();
366+
let _timer = elapsed_compute.timer();
367+
368+
// use a stream that allows each sender to put in at
369+
// least one result in an attempt to maximize
370+
// parallelism.
371+
let mut builder =
372+
RecordBatchReceiverStream::builder(self.schema.clone(), input_partitions);
373+
374+
// spawn independent tasks whose resulting streams (of batches)
375+
// are sent to the channel for consumption.
376+
for part_i in 0..input_partitions {
377+
builder.run_input(
378+
Arc::clone(&self.input),
379+
part_i,
380+
Arc::clone(&self.context),
381+
);
382+
}
383+
384+
let stream = builder.build();
385+
self.state = CoalescePartitionsStreamState::Running(Box::pin(
386+
ObservedStream::new(stream, self.baseline_metrics.clone(), self.fetch),
387+
));
388+
match &mut self.state {
389+
CoalescePartitionsStreamState::Running(s) => s,
390+
_ => unreachable!(),
391+
}
392+
}
393+
}
394+
395+
impl RecordBatchStream for CoalescePartitionsStream {
396+
fn schema(&self) -> SchemaRef {
397+
self.schema.clone()
398+
}
399+
}
400+
401+
impl Stream for CoalescePartitionsStream {
402+
type Item = Result<arrow::array::RecordBatch>;
403+
404+
fn poll_next(
405+
mut self: Pin<&mut Self>,
406+
cx: &mut Context<'_>,
407+
) -> Poll<Option<Self::Item>> {
408+
let stream = match &mut self.state {
409+
CoalescePartitionsStreamState::Running(s) => s,
410+
CoalescePartitionsStreamState::Pending => self.start(),
411+
};
412+
stream.poll_next_unpin(cx)
413+
}
414+
}
415+
355416
#[cfg(test)]
356417
mod tests {
357418
use super::*;

0 commit comments

Comments
 (0)