Jackie-Jiang commented on code in PR #17619:
URL: https://github.com/apache/pinot/pull/17619#discussion_r2756351942
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/instanceselector/SegmentInstanceCandidate.java:
##########
@@ -32,6 +32,7 @@ public class SegmentInstanceCandidate {
private final String _instance;
private final boolean _online;
private final int _pool;
+ private int _idealStateReplicaId = -1;
Review Comment:
Make it final. Also add some comment explaining what it is, e.g. index of
the server within the ideal state segment assignment
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/instanceselector/BaseInstanceSelector.java:
##########
@@ -492,8 +502,9 @@ int getPool(String instanceID) {
* Selects the server instances for the given segments based on the request
id and segment states. Returns two maps
* from segment to selected server instance hosting the segment. The 2nd map
is for optional segments. The optional
* segments are used to get the new segments that are not online yet.
Instead of simply skipping them by broker at
- * routing time, we can send them to servers and let servers decide how to
handle them.
+ * routing time, we can send them to servers and let servers decide how to
handle them. List of unavailable segments
+ * can also be included.
*/
- protected abstract Pair<Map<String, String>, Map<String, String>/*optional
segments*/> select(List<String> segments,
+ protected abstract SelectionResult select(List<String> segments,
Review Comment:
Trying to understand the abstraction change here. Does this mean instance
selection can introduce new unavailable segments? If so, should we move
unavailable segments computation into this method?
Right now the unavailable segments update logic is spread in 2 places, and
becomes hard to manage
--
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]
