xiangfu0 commented on code in PR #19908:
URL: https://github.com/apache/pinot/pull/19908#discussion_r4297220794
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/instanceselector/BalancedInstanceSelector.java:
##########
@@ -44,6 +45,25 @@
/// light-weight and will do best effort to balance the number of segments
served by each selected server instance.
public class BalancedInstanceSelector extends BaseInstanceSelector {
+ @Override
+ protected InstanceMapping selectForFailOpen(List<String>
unavailableSegments, int requestId,
+ SegmentStates failOpenSegmentStates, Map<String, String> queryOptions) {
+ Map<String, String> selected = new
HashMap<>(HashUtil.getHashMapCapacity(unavailableSegments.size()));
+ Map<String, String> optional = new HashMap<>();
+ for (String segment : unavailableSegments) {
+ List<SegmentInstanceCandidate> candidates =
failOpenSegmentStates.getCandidates(segment);
+ if (candidates != null && !candidates.isEmpty()) {
+ SegmentInstanceCandidate candidate = candidates.get(requestId++ %
candidates.size());
Review Comment:
Minor defensive nit, not a live bug: this uses plain `%` on `requestId`,
while `ReplicaGroupInstanceSelector.selectServers` uses `Math.floorMod(...)`
for the same index. The request id generator is always non-negative today so
`%` is safe, but matching `floorMod` here (and at the pre-existing line 95)
would make it robust if that ever changes.
##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/BrokerMeter.java:
##########
@@ -216,6 +216,14 @@ public class BrokerMeter implements AbstractMetrics.Meter {
public static final BrokerMeter ROUTING_TABLE_UPDATE_FAILURE =
create("ROUTING_TABLE_UPDATE_FAILURE", "failures", false);
+ // Panic routing metrics
Review Comment:
Small naming nit: this comment says "Panic routing metrics", but the
feature, config key, and meter names all use "fail-open" everywhere else. Looks
like leftover naming from an earlier iteration — worth renaming to "Fail-open
routing metrics" so operators grepping for the feature find it.
--
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]
