kpatel006 commented on code in PR #19891:
URL: https://github.com/apache/pinot/pull/19891#discussion_r4310707376
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/instanceselector/BaseInstanceSelector.java:
##########
@@ -470,7 +474,21 @@ void refreshSegmentStates() {
}
}
- _segmentStates = new SegmentStates(instanceCandidatesMap,
servingInstances, unavailableSegments);
+ SegmentStates previousSegmentStates = _segmentStates;
+ Set<String> previousUnavailableSegments =
+ previousSegmentStates != null ?
previousSegmentStates.getUnavailableSegments() : Set.of();
+ Set<String> previousSegmentsWithoutCandidates =
+ previousSegmentStates != null ?
previousSegmentStates.getSegmentsWithoutCandidates() : Set.of();
+ for (String segment : unavailableSegments) {
+ boolean wasAlreadyUnavailableWithCandidates =
+ previousUnavailableSegments.contains(segment) &&
!previousSegmentsWithoutCandidates.contains(segment);
+ if (!segmentsWithoutCandidates.contains(segment) &&
!wasAlreadyUnavailableWithCandidates) {
+ _brokerMetrics.addMeteredTableValue(_tableNameWithType,
BrokerMeter.FAIL_OPEN_ELIGIBLE_SEGMENTS, 1);
Review Comment:
Thanks! Addressed the issue
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/instanceselector/BaseInstanceSelector.java:
##########
@@ -577,16 +595,22 @@ public SelectionResult select(BrokerRequest
brokerRequest, List<String> segments
SegmentStates segmentStates = _segmentStates;
InstanceMapping mapping = select(segments, requestIdInt, segmentStates,
queryOptions);
Set<String> unavailableSegments = segmentStates.getUnavailableSegments();
+ Set<String> segmentsWithoutCandidates =
segmentStates.getSegmentsWithoutCandidates();
if (unavailableSegments.isEmpty()) {
return new SelectionResult(mapping, List.of(), 0);
} else {
List<String> unavailableSegmentsForRequest = new ArrayList<>();
+ boolean failOpenEligible = false;
for (String segment : segments) {
if (unavailableSegments.contains(segment)) {
unavailableSegmentsForRequest.add(segment);
+ failOpenEligible |= !segmentsWithoutCandidates.contains(segment);
}
}
+ if (failOpenEligible) {
+
_brokerMetrics.addMeteredGlobalValue(BrokerMeter.FAIL_OPEN_ELIGIBLE_REQUESTS,
1);
Review Comment:
Addressed
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/instanceselector/BaseInstanceSelector.java:
##########
@@ -577,16 +595,22 @@ public SelectionResult select(BrokerRequest
brokerRequest, List<String> segments
SegmentStates segmentStates = _segmentStates;
InstanceMapping mapping = select(segments, requestIdInt, segmentStates,
queryOptions);
Set<String> unavailableSegments = segmentStates.getUnavailableSegments();
+ Set<String> segmentsWithoutCandidates =
segmentStates.getSegmentsWithoutCandidates();
if (unavailableSegments.isEmpty()) {
return new SelectionResult(mapping, List.of(), 0);
} else {
List<String> unavailableSegmentsForRequest = new ArrayList<>();
+ boolean failOpenEligible = false;
for (String segment : segments) {
if (unavailableSegments.contains(segment)) {
unavailableSegmentsForRequest.add(segment);
+ failOpenEligible |= !segmentsWithoutCandidates.contains(segment);
}
}
+ if (failOpenEligible) {
+
_brokerMetrics.addMeteredGlobalValue(BrokerMeter.FAIL_OPEN_ELIGIBLE_REQUESTS,
1);
Review Comment:
Addressed
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/instanceselector/SegmentStates.java:
##########
@@ -38,12 +38,22 @@ public class SegmentStates {
private final Map<String, List<SegmentInstanceCandidate>>
_instanceCandidatesMap;
private final Set<String> _servingInstances;
private final Set<String> _unavailableSegments;
+ private final Set<String> _segmentsWithoutCandidates;
Review Comment:
Addressed
--
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]
