kpatel006 commented on code in PR #19891:
URL: https://github.com/apache/pinot/pull/19891#discussion_r4310464408
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/instanceselector/BaseInstanceSelector.java:
##########
@@ -446,6 +447,9 @@ void refreshSegmentStates() {
+ "(all candidate instances: {} are disabled, counting segment
as unavailable)", segment,
_tableNameWithType, candidateInstances);
unavailableSegments.add(segment);
+ if (candidates.isEmpty()) {
+ segmentsWithoutCandidates.add(segment);
Review Comment:
The implementation PR (https://github.com/apache/pinot/pull/19908) fixes
this by carrying the exclusion reason into the classification:
- BaseBrokerRoutingManager L505-510 snapshots the admin-enabled server set
before removing failure-detector exclusions. L529/606/655 pass it to the
selectors through the new onInstancesChange overload (BaseInstanceSelector
L568-572).
- BaseInstanceSelector L461-477 keeps only candidates in that admin-enabled
set. A segment is eligible only if at least one candidate is admin-enabled but
excluded by the failure detector. Both meters (L507-511, L644) read that
classification.
--
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]
