[Enhancement] Eventize the spillable sort by eshishki · Pull Request #74590 · StarRocks/starrocks

eshishki · 2026-06-09T17:54:54Z

Why I'm doing:

Spillable sort (ORDER BY / TOP-N) blocks on spill IO but subscribes to no wakeup, so the busy-poller spin-polls it (the cost is in #74586, the shared base in #74587). This PR opts the sort into the event scheduler.

What I'm doing:

Stacked on #74587 (base).

The partition sort sink is a sink edge: it parks OUTPUT_FULL on a full writer (WAIT_FLUSH) or a busy spill-process channel (WAIT_CHANNEL), subscribes to the spiller's sink list, and shares SpillableFlatSinkMixin — same as the agg / join sinks.

The merge sources (single-merge and parallel-merge) park INPUT_EMPTY on a per-partition restore (WAIT_RESTORE). A sort merges all spilled partitions, so unlike the single-spiller agg source a merge source subscribes to every partition spiller's source list — one subscription per spilled partition — and any partition's restore or flush-all completion wakes it. The parallel-merge source also coordinates its merge stages through the merger's own observable, as the non-spill parallel sort already does.

Gated on enable_spill_sort_events (default false, mutable): off, the sort fragment runs on the poller as before; on, the sort drivers are woken by events instead of polled. Query results are unchanged.

Part of #74586

What type of PR is this:

Does this PR entail a change in behavior?

Yes, this PR will result in a change in behavior.
No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

Interface/UI changes: syntax, type conversion, expression evaluation, display information
Parameter changes: default values, similar parameters but with different default values
Policy changes: use new policy to replace old one, functionality automatically enabled
Feature removed
Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

I have added test cases for my bug fix or my new feature
This pr needs user documentation (for new or modified features or behaviors)
- I have added documentation for my new feature or new function
- This pr needs auto generate documentation
This is a backport pr

Bugfix cherry-pick branch check:

I have checked the version labels which the pr will be auto-backported to the target branch
- 4.1
- 4.0
- 3.5

CelerData-Reviewer · 2026-06-09T17:55:50Z

@codex review

github-actions · 2026-06-09T17:56:09Z

No new undocumented parameters detected by the param-drift check.

github-actions · 2026-06-09T18:03:24Z

🌎 Translation Required?

✅ All translation files are up to date.
Great job! No translation actions are required for this PR.

🕒 Last updated: Mon, 22 Jun 2026 08:58:48 GMT

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e92b471081

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

CelerData-Reviewer · 2026-06-09T20:59:51Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 02e632e6f1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

stdpain · 2026-06-22T07:22:42Z

@eshishki could u rebase this PR ? I want to push for this PR to be merged.

Move the spillable sort onto the pipeline event scheduler: the partition sort sink and both the single-merge and parallel-merge sources park and are woken by the partition spillers' flush-all / restore completions rather than busy-polling. Gated on enable_spill_sort_events (default false); with the flag off the spilled sort stays on the poller. - The sink folds through the shared SpillableFlatSinkMixin. The merge sources subscribe each driver's observer to every partition spiller's source list (N-subscription fan-out, since one source merges all partitions). The parallel-merge source uses a mixed park model: the restore axis names WAIT_RESTORE, the merge-coordination axis stays NONE. - Tests: per-source block_reason lockstep oracle and observer fan-out wakeups (single- and parallel-merge), plus force-spill ON/OFF result-parity e2e. - Documents enable_spill_sort_events (en/zh/ja). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Evgeniy Shishkin <eshishki@gmail.com>

CelerData-Reviewer · 2026-06-22T08:17:39Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bc0956944e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

A partition spiller's restore/flush IO task records its error via update_spilled_task_status and emits only a source wakeup; it does not flip is_cancel/has_output_data. The merge sources' has_output() (is_partition_ready, or the parallel merger's pending/stage gates) never observed it, so on the event scheduler the source would re-park WAIT_RESTORE after the error's source wakeup and sleep until query timeout instead of propagating the error. Add SortContext::spiller_task_status() -- the first non-OK task_status() across the N partition spillers -- and consult it in both merge sources' has_output() (report ready) and pull_chunk() (RETURN_IF_ERROR). This mirrors the single-spiller agg/NLJ sources' RETURN_TRUE_IF_SPILL_TASK_ERROR + task_status() pattern, but the sort source merges N partitions so the scan and the reused Status live in one helper. - Tests: the lockstep transcription gains a spiller_task_error axis asserting an error makes the source runnable (has_output() true, block_reason() NONE, never a silent WAIT_RESTORE park); the observer test asserts the recorded error is observable on a bare partition spiller and the SortContext-style scan surfaces it while the wakeup stays on the source list. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Evgeniy Shishkin <eshishki@gmail.com>

github-actions · 2026-06-23T04:05:50Z

[FE Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

github-actions · 2026-06-23T04:06:10Z

[Java-Extensions Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

github-actions · 2026-06-23T04:27:50Z

[BE Incremental Coverage Report]

✅ pass : 31 / 33 (93.94%)

file detail

	path	covered_line	new_line	coverage	not_covered_line_detail
🔵	be/src/exec/pipeline/sort/spillable_partition_sort_sink_operator.h	4	6	66.67%	[48, 50]
🔵	be/src/exec/pipeline/sort/local_parallel_merge_sort_source_operator.cpp	7	7	100.00%	[]
🔵	be/src/exec/pipeline/sort/sort_context.cpp	12	12	100.00%	[]
🔵	be/src/exec/pipeline/sort/local_merge_sort_source_operator.cpp	5	5	100.00%	[]
🔵	be/src/exec/pipeline/sort/spillable_partition_sort_sink_operator.cpp	1	1	100.00%	[]
🔵	be/src/exec/pipeline/sort/local_merge_sort_source_operator.h	1	1	100.00%	[]
🔵	be/src/exec/pipeline/sort/local_parallel_merge_sort_source_operator.h	1	1	100.00%	[]

Signed-off-by: Evgeniy Shishkin <eshishki@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Oliver Layer <o.layer@celonis.com>

eshishki requested review from a team as code owners June 9, 2026 17:54

github-actions Bot added the documentation Documentation changes label Jun 9, 2026

mergify Bot assigned eshishki Jun 9, 2026

eshishki mentioned this pull request Jun 9, 2026

[Enhancement] Move spillable pipeline operators off the busy-poller onto the event scheduler #74586

Open

github-actions Bot requested a review from stdpain June 9, 2026 17:59

chatgpt-codex-connector Bot reviewed Jun 9, 2026

View reviewed changes

Comment thread be/src/compute_env/spill/spill_metrics.cpp Outdated

eshishki force-pushed the enhance/spill-event-sort branch 7 times, most recently from 13b4297 to 02e632e Compare June 9, 2026 20:58

github-actions Bot added the docs-maintainer Picked up by the weekly docs triage workflow label Jun 9, 2026

chatgpt-codex-connector Bot reviewed Jun 9, 2026

View reviewed changes

Comment thread docs/en/administration/management/monitoring/metric_details/s.md

eshishki force-pushed the enhance/spill-event-sort branch 5 times, most recently from 1e02a64 to 773c060 Compare June 10, 2026 09:53

stdpain reviewed Jun 10, 2026

View reviewed changes

Comment thread be/src/exec/pipeline/sort/local_merge_sort_source_operator.h Outdated

eshishki force-pushed the enhance/spill-event-sort branch 2 times, most recently from d1761ef to fd63883 Compare June 10, 2026 11:59

stdpain reviewed Jun 11, 2026

View reviewed changes

Comment thread be/src/exec/pipeline/sort/spillable_partition_sort_sink_operator.h Outdated

eshishki force-pushed the enhance/spill-event-sort branch from fd63883 to 9cc0bd8 Compare June 11, 2026 07:20

eshishki force-pushed the enhance/spill-event-sort branch 12 times, most recently from 7fcfa66 to b80df57 Compare June 16, 2026 12:42

stdpain self-assigned this Jun 17, 2026

eshishki force-pushed the enhance/spill-event-sort branch from b80df57 to bc09569 Compare June 22, 2026 08:16

chatgpt-codex-connector Bot reviewed Jun 22, 2026

View reviewed changes

Comment thread be/src/exec/pipeline/sort/spillable_partition_sort_sink_operator.h

stdpain approved these changes Jun 23, 2026

View reviewed changes

trueeyu approved these changes Jun 23, 2026

View reviewed changes

stdpain merged commit 0f3815e into StarRocks:main Jun 23, 2026
93 of 94 checks passed

Uh oh!

Conversation

eshishki commented Jun 9, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why I'm doing:

What I'm doing:

What type of PR is this:

Checklist:

Bugfix cherry-pick branch check:

Uh oh!

CelerData-Reviewer commented Jun 9, 2026

Uh oh!

github-actions Bot commented Jun 9, 2026

Uh oh!

github-actions Bot commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🌎 Translation Required?

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

CelerData-Reviewer commented Jun 9, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stdpain commented Jun 22, 2026

Uh oh!

CelerData-Reviewer commented Jun 22, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

github-actions Bot commented Jun 23, 2026

[FE Incremental Coverage Report]

Uh oh!

github-actions Bot commented Jun 23, 2026

[Java-Extensions Incremental Coverage Report]

Uh oh!

github-actions Bot commented Jun 23, 2026

[BE Incremental Coverage Report]

file detail

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

eshishki commented Jun 9, 2026 •

edited by github-actions Bot

Loading

github-actions Bot commented Jun 9, 2026 •

edited

Loading