[MSE] Support sender-side sorting for sort exchanges - #19396
Open
xiangfu0 wants to merge 2 commits into
Open
Conversation
PinotLogicalSortExchange carried an isSortOnSender flag that nothing honored: MailboxSendOperator always streamed its input unchanged and SortedMailboxReceiveOperator always buffered every row of every sender before sorting them all at once. MailboxSendOperator now sorts its input by the exchange collation when MailboxSendNode.isSort() is set, using the same SortUtils.SortComparator the receiver used so direction and null ordering are unchanged, and sends the result in bounded blocks. Every destination then receives a subsequence of that order, because each exchange routes the rows of a block to its destinations without reordering them. SortedMailboxReceiveOperator merges the senders when the plan says they sorted, emitting the smallest head row as soon as every unfinished sender has one, so rows flow downstream while the exchange is still running. It reads whichever mailbox has a block ready rather than blocking on the sender it needs next: blocking on one sender deadlocks when that sender is itself blocked on a full mailbox of another receiver that is waiting on this one. BlockingMultiStreamConsumer gains getLastReadStream() and isStreamLive() for that. PinotWindowExchangeNodeInsertRule now creates its sort exchanges with isSortOnSender=true, which is what its TODOs described. The plain ORDER BY and aggregate exchanges keep sorting on the receiver only, so non-window plans are unchanged. Rolling upgrade: upgrade servers before brokers. A server that has not been upgraded ignores the sort flag, and an upgraded receiver told the senders sorted would merge unsorted input.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #19396 +/- ##
============================================
+ Coverage 57.71% 67.59% +9.88%
- Complexity 7 1430 +1423
============================================
Files 2686 3487 +801
Lines 163987 224441 +60454
Branches 26627 35424 +8797
============================================
+ Hits 94640 151716 +57076
+ Misses 61352 60685 -667
- Partials 7995 12040 +4045
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Carry an explicit per-block sort confirmation through both mailbox transports so upgraded receivers can fall back safely when a legacy sender ignores the plan flag. Preserve tentative merge rows during fallback, make fan-in bookkeeping constant-time, and add active-deadline/resource sampling plus focused transport and merge regressions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #19395.
PinotLogicalSortExchangealready carried sender/receiver sort flags, but the runtime did not honor them: senders streamed unsorted input andSortedMailboxReceiveOperatorbuffered and sorted the complete exchange. This change implements sender-side sorting for window exchanges and lets receivers k-way merge the sorted streams, so a window withORDER BYcan begin producing rows without retaining the full exchanged input at one receiver.Runtime behavior
MailboxSendOperatorsorts the complete sender input with the exchange collation, including direction and null ordering, then emits sequential blocks of at most 10k rows. Existing exchange implementations preserve the order of each destination's subsequence.SortedMailboxReceiveOperatormerges one cursor per sender when the plan requests sender sorting. It drains whichever mailbox is ready to avoid cross-receiver backpressure deadlocks and emits merged blocks of at most 10k rows.ORDER BYexchanges and any sender that does not confirm the new protocol.PinotWindowExchangeNodeInsertRuleenables sender sorting for ordered window exchanges. Plain sort and aggregate exchanges retain receiver-side sorting.Mixed-version safety
Every data block from a sorting sender now carries a
sortedOnSenderconfirmation through both in-memory and gRPC mailboxes. The gRPC confirmation is transport metadata rather than part of the serialized data block.An upgraded receiver tentatively merges only confirmed streams. Before it can emit its first row it must have a head row from every non-empty live sender; if any sender is old (or uses a mailbox implementation that cannot carry the confirmation), that sender's first data block has no marker and the receiver folds all tentatively buffered rows into the existing full buffer-and-sort path. The confirmation is checked on every data block. An old empty sender is harmless because it contributes no rows.
Older receivers ignore the additional metadata and continue to full-sort. The new
SendingMailbox.send(data, sortedOnSender)method is a default method, so existing mailbox implementations remain source- and binary-compatible and cause the safe fallback. No server-before-broker upgrade ordering is required.Review fixes
The review also tightened the implementation beyond the initial PR:
and sender errors.
Tests and hygiene
MailboxSendOperatorfrom a realMailboxSendNode, verify sorted output reaches the terminalSendingMailboxthroughsend(data, true), reject the legacy one-argument path, and cover the nested multi-receiver exchange../mvnw -pl pinot-query-runtime -am test: 20-module reactor success; planner 1,539 tests, runtime 4,613 tests, 0 failures/errors (18 runtime skips), followed by the final runtime-only suite at 4,616 tests, 0 failures/errors (18 skips).spotless:apply,checkstyle:check,license:format, andlicense:checkpass forpinot-query-planner,pinot-query-runtime,pinot-perf.Base/head benchmark
Exact comparison:
bbbed251444e00d58bdcbe59a6f233902eb7e79cafe7a5db057776482049fdec6f93b15695043cealegacystats mode, with JFR and memory-pool profilers.Both point deltas are small relative to the reported uncertainty, and the intervals overlap heavily, so this run does not support a latency improvement or regression claim. Peak process RSS was 3,269,705,728 bytes for the base and 3,323,641,856 bytes for the head (+1.65%). The recordings cover one row scale and a fixed two-sender topology. JFR captured allocation behavior, but this JDK/JMH combination did not provide a reliable normalized bytes/op result. Both benchmark forks completed their measurements and results before JMH forcibly stopped leftover integration-test threads during teardown.