Support colocated joins when a partition holds no segments - #19166
Support colocated joins when a partition holds no segments#19166yashmayya wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #19166 +/- ##
============================================
+ Coverage 66.57% 66.68% +0.11%
Complexity 1423 1423
============================================
Files 3441 3445 +4
Lines 218320 219011 +691
Branches 34737 34886 +149
============================================
+ Hits 145341 146046 +705
+ Misses 61251 61226 -25
- Partials 11728 11739 +11
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:
|
151db80 to
5a48ddb
Compare
There was a problem hiding this comment.
Pull request overview
Enables colocated joins to handle empty partitions while preserving partition-to-worker alignment and reducing fan-out.
Changes:
- Adds shared partition-class assignment and empty-worker padding.
- Adds deferred-partition and multi-cluster safeguards.
- Expands planner, broker, mailbox, and integration coverage.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
pinot-query-planner/.../WorkerManagerTest.java |
Covers partition assignment and padding. |
pinot-query-planner/.../ColocationGroupAnalyzerTest.java |
Tests reducible group detection. |
pinot-query-planner/.../QueryEnvironmentTestBase.java |
Updates partition metadata construction. |
pinot-query-planner/.../PinotDispatchPlannerTest.java |
Tests spooled-stage assignment. |
pinot-query-planner/.../MailboxAssignmentVisitorTest.java |
Tests direct-exchange class agreement. |
pinot-query-planner/.../DispatchableSubPlanTest.java |
Tests segment-map preservation. |
pinot-query-planner/.../WorkerManager.java |
Implements class reduction and padding. |
pinot-query-planner/.../LeafPartitionHints.java |
Centralizes partition-hint parsing. |
pinot-query-planner/.../ColocationGroupAnalyzer.java |
Identifies colocated fragment groups. |
pinot-query-planner/.../MailboxAssignmentVisitor.java |
Validates direct-exchange mappings. |
pinot-query-planner/.../DispatchablePlanMetadata.java |
Stores broker-local class metadata. |
pinot-query-planner/.../DispatchablePlanFragment.java |
Preserves segment assignments when copied. |
pinot-query-planner/.../DispatchablePlanContext.java |
Caches partition metadata per query. |
pinot-integration-tests/.../ColocatedJoinEmptyPartitionTest.java |
Adds end-to-end join coverage. |
pinot-core/.../TablePartitionReplicatedServersInfo.java |
Publishes deferred partitions. |
pinot-broker/.../SegmentPartitionMetadataManagerTest.java |
Tests deferred-partition detection. |
pinot-broker/.../MultiClusterRoutingManagerTest.java |
Tests cross-cluster partition metadata. |
pinot-broker/.../SegmentPartitionMetadataManager.java |
Tracks deferred-only partitions. |
pinot-broker/.../MultiClusterRoutingManager.java |
Avoids combining partition arrays. |
Suppressed comments (2)
pinot-broker/src/main/java/org/apache/pinot/broker/routing/manager/MultiClusterRoutingManager.java:239
- A failed remote lookup leaves it unknown whether that cluster also serves the table. Continuing can return another cluster's partition array and enable a partition-aware plan over a partial view, which can silently drop remote-only partitions. Return
nullon this failure so the caller falls back instead.
} catch (Exception e) {
LOGGER.error("Error getting table partition info from remote cluster routing manager for table {}",
tableNameWithType, e);
continue;
pinot-broker/src/main/java/org/apache/pinot/broker/routing/manager/MultiClusterRoutingManager.java:243
- A
nullpartition-info response does not mean this cluster lacks the table; a routing entry can exist without usable partition metadata. Treating that cluster as absent can return the local array for a table spread across clusters and silently omit its partitions. CheckroutingExistsand returnnullwhen the remote cluster serves the table.
if (remotePartitionInfo == null) {
continue;
}
| partitionsWithOnlyDeferredSegments.removeIf( | ||
| partitionId -> partitionId < partitionInfoMap.length && partitionInfoMap[partitionId] != null); |
| TablePartitionReplicatedServersInfo partitionInfo = | ||
| _localClusterRoutingManager.getTablePartitionReplicatedServersInfo(tableNameWithType); |
A colocated join failed outright when a partition of one of its tables held no segments, with "Failed to find any segment for table: X, partition: N". Worker ids came from a running counter over the partitions that held data, so skipping an empty one would shift every later partition down a slot. Two tables each dropping a different empty partition could then end up with equal worker counts and be wired 1-to-1 onto mismatched partitions, losing rows with no error, which is why the assignment refused to continue at all. The stages tied together by direct exchanges now share one ordered list of partition classes, dropping only the classes that hold no data on any member. A class the group keeps but a member holds no data for gets a worker with no segments, placed on a server borrowed from a member that does hold that class so the exchange stays in process. The two sides of every direct exchange assert that they agree on the list. The broker publishes the partitions whose only segments are new and have no online replica. Those hold data that no server can serve as a whole, so they keep failing rather than being read as empty. A worker with no segments is charged against the query thread estimate like any other worker: it is dispatched and does run a leaf operator. The estimate therefore over-counts by two threads per such worker, which is conservative and only affects colocated joins over a partition space that is largely unpopulated. Those queries failed outright before, so there is no earlier estimate to compare against. Aggregation merge identity is deliberately not covered here. A leaf that scans nothing emits one identity row for an aggregation with no GROUP BY, but that predates this change: a worker whose segments are all pruned on the server already does the same. Padding raises how many such rows reach the merge without introducing the dependency, and the one aggregation that is not a true merge identity fails only when every worker is empty, which this change does not newly reach.
5a48ddb to
45447e7
Compare
Why
A colocated join fails today when a partition of one of its tables holds no segments:
This is easy to hit and hard to avoid:
The assignment cannot simply skip the empty partition. Worker ids come from a counter over the partitions that hold data, so skipping one shifts every later partition down a slot. Two tables that each drop a different empty partition then end up with equal worker counts, get wired one-to-one, and join mismatched partitions. That loses rows and reports no error, which is why the assignment refuses to continue at all.
What changes
A side effect is less fan-out. A table that declares 8 partitions and populates 3 runs 3 workers, so the broker sends fewer requests and waits on fewer servers.
What now fails on purpose
Broker pruning comes next
Broker pruning is off for a colocated join today, and this change is what makes it possible.
A follow-up PR adds it.
Testing
Two things left out on purpose
Aggregation merge identity is not covered here. A leaf that scans nothing emits one identity row for an aggregation with no
GROUP BY. That predates this change: a worker whose segments are all pruned on the server already does the same. This change raises how many such rows reach the merge without introducing the dependency. The one aggregation that is not a true merge identity,PERCENTILEKLL, fails only when every worker is empty, which this change does not newly reach. It is tracked separately.A worker with no segments is charged against the query thread estimate like any other worker. It is dispatched and does run a leaf operator. The estimate over-counts by two threads per such worker. That is conservative, and it only affects colocated joins over a partition space that is largely unpopulated. Those queries failed outright before, so there is no earlier estimate to compare against.
Labels
Applied:
bug,backward-incompat,release-notes,multi-stage,query