jasperjiaguo commented on code in PR #14536:
URL: https://github.com/apache/pinot/pull/14536#discussion_r1859267651
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/SegmentStatusChecker.java:
##########
@@ -330,46 +334,46 @@ private void updateSegmentMetrics(String
tableNameWithType, TableConfig tableCon
}
}
- int numEVReplicas = 0;
+ int numEVReplicasUp = 0;
if (externalView != null) {
Map<String, String> stateMap = externalView.getStateMap(segment);
if (stateMap != null) {
for (Map.Entry<String, String> entry : stateMap.entrySet()) {
+ String serverInstanceId = entry.getKey();
String state = entry.getValue();
- if (state.equals(SegmentStateModel.ONLINE) ||
state.equals(SegmentStateModel.CONSUMING)) {
- numEVReplicas++;
+ if (isServerQueryable(serverInstanceId, context)
+ && (state.equals(SegmentStateModel.ONLINE) ||
state.equals(SegmentStateModel.CONSUMING))) {
+ numEVReplicasUp++;
}
if (state.equals(SegmentStateModel.ERROR)) {
errorSegments.add(Pair.of(segment, entry.getKey()));
}
}
}
}
- if (numEVReplicas == 0) {
+ if (numEVReplicasUp == 0) {
offlineSegments.add(segment);
- } else if (numEVReplicas < numISReplicas) {
+ } else if (numEVReplicasUp < numISReplicasUp) {
partialOnlineSegments.add(segment);
- } else {
- // Do not allow nReplicasEV to be larger than nReplicasIS
- numEVReplicas = numISReplicas;
}
- minEVReplicas = Math.min(minEVReplicas, numEVReplicas);
+
+ minEVReplicasUp = Math.min(minEVReplicasUp, numEVReplicasUp);
+ // Total number of replicas in ideal state (including ERROR/OFFLINE
states)
+ int numISReplicasTotal =
idealState.getInstanceStateMap(segment).entrySet().size();
+ minEVReplicasUpPercent = Math.min(minEVReplicasUpPercent,
numEVReplicasUp * 100 / numISReplicasTotal);
}
- if (maxISReplicas == Integer.MIN_VALUE) {
+ if (maxISReplicasUp == Integer.MIN_VALUE) {
try {
- maxISReplicas = Math.max(Integer.parseInt(idealState.getReplicas()),
1);
+ maxISReplicasUp = Math.max(Integer.parseInt(idealState.getReplicas()),
1);
} catch (NumberFormatException e) {
- maxISReplicas = 1;
+ maxISReplicasUp = 1;
}
}
- // Do not allow minEVReplicas to be larger than maxISReplicas
- minEVReplicas = Math.min(minEVReplicas, maxISReplicas);
- if (minEVReplicas < maxISReplicas) {
- LOGGER.warn("Table {} has at least one segment running with only {}
replicas, below replication threshold :{}",
- tableNameWithType, minEVReplicas, maxISReplicas);
- }
Review Comment:
Can we add some new logging instead? Meanwhile, it would be great if we have
a list of segments having min replicas and prent them out as well? I feel
sometimes the debuggability of alerts on this metric is not so great.
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
