Skip to content

perf(scheduler): merge a task's sorted partitions before the shuffle write - #2335

Draft
Dandandan wants to merge 6 commits into
apache:mainfrom
Dandandan:proto/task-local-ordered-merge
Draft

perf(scheduler): merge a task's sorted partitions before the shuffle write#2335
Dandandan wants to merge 6 commits into
apache:mainfrom
Dandandan:proto/task-local-ordered-merge

Conversation

@Dandandan

@Dandandan Dandandan commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

What

A sorted passthrough shuffle stage is read back by an ordering-preserving merge.
This does that merge inside the producing task instead, with the per-partition
TopK's row limit: each task writes one file of at most n rows instead of one
file per partition holding n each. The consumer then merges T × n rows from
T sources rather than P × n from P.

The rewrite runs on the per-task plan — in the stage plan the writer would take
its partitioning from the merge and the stage would collapse to one task. Tasks
report output partition 0, which file_id already disambiguates.

Taking the limit is sound because a per-partition TopK(n) only exists when a
global limit of n sits above the consuming merge, so any row in the global top
n that a task holds is in that task's own top n.

Only sound when the consumer merges a partition's locations in order, so the AQE
adapter makes the call and marks the writer — build_reader plants
RangeShuffleReaderExec for that same exchange under the same conditions. The
static planner never marks; it plants the arrival-order reader, and concatenating
separately sorted files is not sorted.
ballista.shuffle.merge_ordered_passthrough (default true) turns it off.

Measured

TPC-H SF10, --partitions 64, 2 executors × 4 vcores. All 22 queries verified
against single-process DataFusion.

Rows the consumer's ordered reader merges — each result is exactly T × n
(16 tasks × the query's limit):

query off on
q2 4,670 1,600 −66%
q3 497 160 −68%
q10 1,100 320 −71%
q21 4,010 1,600 −60%
q18 624 624 limit not binding

Passthrough shuffle files across those queries: 709 → 469, i.e. P → T per stage.

Wall clock does not change for TPC-H (at SF=10 at least)

… write

A sorted passthrough stage is read back by an ordering-preserving reader that
merges on the same key. Merging inside the producing task moves that work
upstream: the task writes one file instead of one per partition, and the
consumer opens one source per task instead of one per partition.

The rewrite runs on the per-task plan, not the stage plan. In the stage plan
the writer would derive its partitioning from the merge and the stage would
collapse to a single task.

Off by default behind ballista.shuffle.merge_ordered_passthrough.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DZS73f3mPrwQ8EPbswFJVa
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Aug 17, 2026
Dandandan and others added 3 commits August 17, 2026 11:56
Enabling the merge by default surfaced wrong row order in the sort-shuffle
client tests. Those run on the static planner, which always plants the
arrival-order reader; concatenating separately sorted files is not sorted.
Adaptive planning plants RangeShuffleReaderExec for exactly the condition the
rewrite checks, so gate the rewrite on adaptive planning being in use.

Also enable the merge by default.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DZS73f3mPrwQ8EPbswFJVa
The gate inferred a property of the consumer stage from a session flag, and
that inference was wrong: a physical-plan submission builds a
StaticExecutionGraph while adaptive planning stays enabled, so a statically
planned job — which always gets the arrival-order reader — would have had its
partitions merged and returned rows out of order.

The AQE adapter is the only place that knows both sides: build_reader plants
RangeShuffleReaderExec for the same exchange under the same conditions. It now
marks the writer, and the per-task rewrite reads the mark. The static planner
never marks, so it cannot be affected.

Collapses the two duplicated call sites into one helper, and drops the
per-task BallistaConfig clone the gate used to pay.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DZS73f3mPrwQ8EPbswFJVa
A per-partition TopK(n) only exists because a global limit of n sits above the
merge that consumes the stage, so the task's merge can stop at n as well: any
row in the global top n that a task holds is in that task's own top n.

Without the limit the merge relocated work without removing any. With it the
consumer's reader merges T*n rows instead of P*n — measured on TPC-H SF10 at 64
partitions: q2 4,670 -> 1,600, q3 497 -> 160, q10 1,100 -> 320, q21 4,010 ->
1,600. q18 is unchanged because its input is smaller than the limit allows.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DZS73f3mPrwQ8EPbswFJVa

@milenkovicm milenkovicm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Dandandan one question regarding planning logic

/// Must run after [`restrict_plan_to_partitions`], which treats a
/// `SortPreservingMergeExec` as a collapse and gives leaves below one the full
/// upstream — merging first would make every task read the whole stage input.
pub fn merge_task_partitions_before_write(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder could this be moved to AQE planner rule instead of having planning logic at this point ?

@avantgardnerio avantgardnerio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I ran this with h2o q8 with parallel windows enabled, it hung. PTAL

Kicking off CI again, since it should trigger there too.

@Dandandan
Dandandan marked this pull request as draft August 21, 2026 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants