xiangfu0 commented on code in PR #19537:
URL: https://github.com/apache/pinot/pull/19537#discussion_r4221914955
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/routing/WorkerManager.java:
##########
@@ -920,9 +922,13 @@ private void
assignWorkersToNonPartitionedLeafFragment(PlanFragment fragment, Di
for (Map.Entry<ServerInstance, SegmentsToQuery> serverEntry :
segmentsMap.entrySet()) {
Map<String, List<String>> tableTypeToSegmentListMap =
serverInstanceToSegmentsMap.computeIfAbsent(serverEntry.getKey(),
k -> new HashMap<>());
- // TODO: support optional segments for multi-stage engine.
+ Map<String, List<String>> tableTypeToOptionalSegmentListMap =
+
serverInstanceToOptionalSegmentsMap.computeIfAbsent(serverEntry.getKey(), k ->
new HashMap<>());
Preconditions.checkState(tableTypeToSegmentListMap.put(tableType,
serverEntry.getValue().getSegments()) == null,
"Entry for server {} and table type: {} already exist!",
serverEntry.getKey(), tableType);
+ Preconditions.checkState(
+ tableTypeToOptionalSegmentListMap.put(tableType,
serverEntry.getValue().getOptionalSegments()) == null,
Review Comment:
**Empty optional lists pollute the optional maps on every query.**
`getOptionalSegments()` is usually an empty list, which is put here
unconditionally; the resulting `{OFFLINE: []}` map is non-empty, so both
`MapUtils.isEmpty(optionalSegmentMap)` below and `!optionalForWorker.isEmpty()`
in `DispatchablePlanContext` pass, and essentially every worker of every
non-partitioned leaf gets a useless `optionalTableSegmentsMap` (wasted
allocations now, wire bloat once serialization is added). Filter per-list with
`CollectionUtils.isNotEmpty` the way
`transferToServerInstanceLogicalSegmentsMap` does. Minor: the new Precondition
message uses `{}` placeholders, but Guava substitutes `%s`, not `{}` (copied
from the line above, but worth not propagating).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
