Add ideal state replica ID information for query routing #17619
Conversation
yashmayya
commented
Feb 2, 2026
yashmayya
commented
- Useful for query routing strategies that require knowing the ideal state replica ID for each segment's instance candidates.
...ker/src/main/java/org/apache/pinot/broker/routing/instanceselector/BaseInstanceSelector.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/apache/pinot/broker/routing/instanceselector/SegmentInstanceCandidate.java
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #17619 +/- ##
============================================
- Coverage 63.15% 63.14% -0.01%
- Complexity 1479 1498 +19
============================================
Files 3173 3173
Lines 189917 190109 +192
Branches 29064 29070 +6
============================================
+ Hits 119935 120044 +109
- Misses 60658 60747 +89
+ Partials 9324 9318 -6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Jackie-Jiang
left a comment
Jackie-Jiang
left a comment
There was a problem hiding this comment.
LGTM. Let's try to reduce the overhead of map access
...ker/src/main/java/org/apache/pinot/broker/routing/instanceselector/BaseInstanceSelector.java
Show resolved
Hide resolved
| // Build mapping from instance to position in ideal state (ideal state replica ID) | ||
| Map<String, Integer> instanceToIdealStateReplicaId = new HashMap<>(); | ||
| int idealStateReplicaId = 0; | ||
| for (String instance : sortedIdealStateMap.keySet()) { |
There was a problem hiding this comment.
Since we always loop over IS, we should be able to reduce map lookup by tracking the iteration while looping over IS
There was a problem hiding this comment.
We don't always loop over IS though - see these two cases:
(externalViewInstanceStateMap == null)and(newSegmentCreationTimeMs == null)(externalViewInstanceStateMap != null)and(newSegmentCreationTimeMs == null)(only loop over online instances).
There was a problem hiding this comment.
onlineInstances are also computed via looping over IS
There was a problem hiding this comment.
Hm yes but that would duplicate the logic in three different places for (IMO) little benefit (since these maps would be tiny in size - just the # of replicas per segment). Also this logic is not in the query hot path.
There was a problem hiding this comment.
It will introduce a per segment short-lived map and one extra map lookup per segment-instance pair. Let me try if it is easy to avoid
There was a problem hiding this comment.
Fair point, I've updated the logic to avoid the short-lived maps per segment and re-use the same loops.
...-broker/src/main/java/org/apache/pinot/broker/routing/instanceselector/InstanceSelector.java
Outdated
Show resolved
Hide resolved
yashmayya
force-pushed
the
ideal-state-replica-group-routing
branch
from
822df08 to
8a2f985
Compare
3 participants
