Jackie-Jiang commented on code in PR #19530:
URL: https://github.com/apache/pinot/pull/19530#discussion_r4021344160
##########
pinot-broker/pom.xml:
##########
@@ -49,6 +49,10 @@
<groupId>org.apache.pinot</groupId>
<artifactId>pinot-timeseries-planner</artifactId>
</dependency>
+ <dependency>
Review Comment:
This shouldn't be required
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/instanceselector/BalancedInstanceSelector.java:
##########
@@ -45,11 +47,13 @@ public class BalancedInstanceSelector extends
BaseInstanceSelector {
@Override
public InstanceMapping select(List<String> segments, int requestId,
SegmentStates segmentStates, Map<String, String> queryOptions) {
- Map<String, String> segmentToSelectedInstanceMap = new
HashMap<>(HashUtil.getHashMapCapacity(segments.size()));
+ // Allocate the flat map only when a required segment is selected. It
avoids one map node per segment without
+ // reserving large arrays for queries whose segments are all optional or
unavailable.
+ Map<String, String> segmentToSelectedInstanceMap = null;
Review Comment:
I still think this is unnecessary. I cannot imagine a large table with all
segments optional. It doesn't really matter to pre-allocate the map for small
tables.
--
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]
