Skip to content

Add streaming k-way merge to SortedMailboxReceiveOperator - #19121

Draft
rohityadav1993 wants to merge 3 commits into
apache:masterfrom
rohityadav1993:oss/pr2-sorted-mailbox-kway-merge
Draft

Add streaming k-way merge to SortedMailboxReceiveOperator#19121
rohityadav1993 wants to merge 3 commits into
apache:masterfrom
rohityadav1993:oss/pr2-sorted-mailbox-kway-merge

Conversation

@rohityadav1993

Copy link
Copy Markdown
Contributor

SortedMailboxReceiveOperator accumulates every row from all senders and only then sorts, even when
each sender's stream is already sorted on the collation keys — flagged as a TODO in the class today.
That defeats streaming and makes the receive stage's peak memory proportional to the whole input.
This is the second bullet of Challenge 1 in #18667.

Approach

When the planner can prove each sender emits sorted output, the receiver performs a k-way heap merge
across mailboxes and emits bounded sorted blocks as data arrives.

  • BlockingMultiStreamConsumer gains a StreamHandle<T> abstraction
    (awaitDataOrTerminal() / poll()) so a consumer can hold the head of each stream without
    draining it — the primitive the heap merge needs. A consumer must use either the handles or
    readMseBlockBlocking(), never both; the mode is latched.
    relieveSiblingsOnce() does one non-blocking poll per sibling stream per refill, so a fast sender
    with a disjoint key range cannot sit blocked on a full mailbox holding an MSE worker thread.
  • SortedMailboxReceiveOperator implements the merge with a per-mailbox cursor that caches its
    head row, and verifies the precondition at runtime: each sender's rows must be non-decreasing, and
    a violation surfaces as an error block naming the offending mailbox instead of silently wrong
    output.
  • PlanFragmenter / PinotLogicalQueryPlanner / QueryEnvironment mark a leaf selection
    ORDER BY sender fragment as sorted-on-sender when the plan shape and collation match.
    This gate fails closed: resolvesToSinglePhysicalTable() rejects hybrid tables, logical
    tables, unknown tables, and a null TableCache. This matters — a scan over a hybrid table compiles
    into two ServerQueryRequests that LeafOperator runs concurrently into one mailbox, producing
    two independently sorted runs concatenated, not a sorted stream. Empty collations are also
    rejected, so a LIMIT without ORDER BY (which compiles to a collation-less LogicalSort) is
    never marked sorted.
  • BaseMailboxReceiveOperator.StatKey.K_WAY_MERGE_USED (new) reports per receive operator, in
    the response stageStats, whether the merge actually ran. Reporting is presence-based —
    StatMap drops false booleans, so kWayMergeUsed: true appears on the merge path and nothing
    appears on the accumulate-then-sort path. This is the only runtime discriminator between the two
    paths; both render as type: MAILBOX_RECEIVE.

Opt-in query options

Option Default Meaning
streamingSortedMailboxReceive false Use the k-way merge when the planner has also proven senders are sorted
streamingSortedMailboxReceiveBlockSize 10000 Rows per emitted block
sortedSelectionMergeEnabled (from the parent PR) false Additionally required only for the plain leaf-selection ORDER BY shape, where the rel plan contains no sort exchange

isSortedOnSender is set either by a rel-level sort exchange that already declares sender-side
sorting, or — for a plain leaf selection ORDER BY — by PlanFragmenter under
sortedSelectionMergeEnabled. Only the latter case needs both options.

Rolling-upgrade note

K_WAY_MERGE_USED is appended as the last StatKey constant. StatMap serializes keys by
ordinal and deserializes without a bounds check, so a peer on a build predating the key cannot
decode stage stats containing it. Because BOOLEAN keys are written only when true, and the key is
only ever true when streamingSortedMailboxReceive is on, leaving the option off (the default)
keeps a mixed-version cluster safe. This is documented on the option constant. Bounds-checking the
ordinal in StatMap.merge(DataInput) is a worthwhile separate fix.

No behaviour change when the option is off

With streamingSortedMailboxReceive unset, the existing accumulate-then-sort path runs unchanged and
no new stat is emitted. The planner-side marking is additionally a no-op under
usePhysicalOptimizer, since the v2 path does not go through PlanFragmenter.

Tests

Test class Count
SortedMailboxReceiveOperatorTest (extended) 36
PlanFragmenterTest (new) 42 cases — accept/reject sender shapes, hybrid / logical / unknown-table rejects, collation matching, option-off for every shape, null-cache fail-closed
SortOperatorTest (extended) 23
BlockingMultiStreamConsumerTest (extended) 9
full pinot-query-planner suite 1449
StreamingSortedMailboxReceiveTest (new integration test) 4

The integration test asserts kWayMergeUsed in stageStats on the merge arm and its absence on the
baseline arm, so it cannot pass with the merge path deleted.

What this buys, honestly

On a local cluster over an 87.8M-doc, 56-segment table, the merge is confirmed engaged
(kWayMergeUsed: true on both join-input receives) and results are identical to the
accumulate-then-sort path. It is not a throughput win for that workload — enabling the mailbox
merge alone was the slowest sorted variant measured. The benefit is bounded peak memory in the
receive stage and latency to first row, not wall clock. Row order among equal collation keys is
unspecified and may differ between the two paths, as with any SQL ORDER BY; the output multiset is
identical.

Stacking

Stacked on #19120. The GitHub diff for this PR includes the parent's commits until the parent merges
review only the top commit.

This PR does not compile without its parent. QueryEnvironment calls
QueryOptionsUtils.isSortedSelectionMergeEnabled(...), which the parent PR adds. Please do not
merge this one first.

Part of #18667.

An unbounded leaf-stage ORDER BY (as injected for sorted merge join inputs)
routes to MinMaxValueBasedSelectionOrderByCombineOperator, which merges every
segment's rows into a single block before returning anything. At large data
volumes this exceeds the leaf stage's CPU budget and ThreadAccountant raises
EarlyTerminationException inside SelectionOperatorUtils.mergeWithOrdering(),
surfacing to the broker as a spurious "Cancelled by sender".

This adds a streaming alternative, opt-in via the `streamingSelectionOrderBy`
query option:

- StreamingSelectionOrderByOperator emits sorted blocks incrementally for a
  segment that is physically sorted on the leading ORDER BY column, reading
  the sorted forward index in order instead of building a priority queue.
- StreamingSelectionOrderByCombineOperator performs a k-way heap merge across
  segment operators and emits bounded blocks (`streamingSelectionOrderByBlockSize`,
  default 10000) rather than one materialized result.
- SelectionPlanNode and CombinePlanNode select these operators when the option
  is set and the sortedness precondition holds; otherwise behaviour is unchanged.

Part of apache#18667.
Correctness and safety fixes on top of the streaming selection ORDER BY
combine operator:

- StreamingSelectionOrderByCombineOperator now checks the query deadline and
  termination state inside the merge loop. The merge drains its heap on the
  caller thread and, for single-block cursors, may never re-enter a child
  operator, so nothing else observed cancellation, pause, or timeout for up to
  (limit + offset) iterations. This restored parity with
  MinMaxValueBasedSelectionOrderByCombineOperator, which this operator replaces
  on the same query shape.
- Child blocks whose data schema disagrees with the merge schema are dropped and
  reported as MERGE_RESPONSE errors instead of being merged blindly, mirroring
  SelectionOrderByResultsBlockMerger. Segments on a server can disagree on
  schema mid-reload, and merging rows of differing width under a single schema
  corrupted the result rather than failing.
- Segments are released when an Error (not just an Exception) escapes the merge,
  so an OutOfMemoryError while accumulating output rows can no longer leave
  segments acquired for the lifetime of the server. In single-stage mode there
  is no start()/stop() backstop around this operator.
- StreamingSelectionOrderByOperator reports docs and entries scanned to the
  shared QueryScanCostContext, as every other selection operator does. Without
  it, scan-based query killing was blind on this path.

Also renames the query option from `streamingSelectionOrderBy` to
`sortedSelectionMergeEnabled` (and `...BlockSize` to
`sortedSelectionMergeBlockSize`). The option gates the classic single-stage
combine path as well as the MSE leaf path, where nothing streams, so the old
name misdescribed it; query option strings are effectively permanent public API.
The default block size moves to a named constant.

Part of apache#18667.
SortedMailboxReceiveOperator accumulates every row from all senders and then
sorts, even when each sender's stream is already sorted on the collation keys
(noted as a TODO in the class). That defeats streaming and makes the receive
stage's peak memory proportional to the full input.

When the sending stage is known to produce sorted output, the receiver now
performs a k-way heap merge across mailboxes and emits bounded sorted blocks
as data arrives:

- BlockingMultiStreamConsumer gains a StreamHandle<T> abstraction with
  awaitDataOrTerminal()/poll(), so a consumer can peek the head of each stream
  without draining it — the primitive the heap merge needs.
- PlanFragmenter/PinotLogicalQueryPlanner propagate sender-side collation so
  the receiver can tell whether its inputs are individually sorted.
- Opt-in via the `streamingSortedMailboxReceive` query option, with
  `streamingSortedMailboxReceiveBlockSize` controlling emitted block size.
  Without the option, the existing accumulate-then-sort path is used unchanged.

Part of apache#18667.
@codecov-commenter

codecov-commenter commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.12005% with 140 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.60%. Comparing base (a8b207e) to head (0f09d4f).
⚠️ Report is 17 commits behind head on master.

Files with missing lines Patch % Lines
...bine/StreamingSelectionOrderByCombineOperator.java 72.41% 39 Missing and 17 partials ⚠️
...runtime/operator/SortedMailboxReceiveOperator.java 84.35% 20 Missing and 8 partials ⚠️
...rator/query/StreamingSelectionOrderByOperator.java 88.26% 21 Missing and 6 partials ⚠️
...me/operator/utils/BlockingMultiStreamConsumer.java 81.81% 10 Missing and 4 partials ⚠️
...he/pinot/query/planner/logical/PlanFragmenter.java 84.44% 4 Missing and 3 partials ⚠️
...va/org/apache/pinot/core/plan/CombinePlanNode.java 50.00% 0 Missing and 4 partials ⚠️
...pinot/core/plan/maker/InstancePlanMakerImplV2.java 66.66% 1 Missing and 1 partial ⚠️
.../org/apache/pinot/core/plan/SelectionPlanNode.java 92.30% 0 Missing and 1 partial ⚠️
...y/runtime/operator/BaseMailboxReceiveOperator.java 75.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #19121      +/-   ##
============================================
+ Coverage     65.49%   65.60%   +0.10%     
  Complexity     1423     1423              
============================================
  Files          3430     3434       +4     
  Lines        218010   218881     +871     
  Branches      34648    34853     +205     
============================================
+ Hits         142784   143591     +807     
- Misses        63666    63691      +25     
- Partials      11560    11599      +39     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-25 65.60% <82.12%> (+0.10%) ⬆️
temurin 65.60% <82.12%> (+0.10%) ⬆️
unittests 65.59% <82.12%> (+0.10%) ⬆️
unittests1 57.02% <82.12%> (+0.15%) ⬆️
unittests2 37.77% <1.40%> (-0.08%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants