mqliang commented on code in PR #14536:
URL: https://github.com/apache/pinot/pull/14536#discussion_r1857564219


##########
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:
   @sajjad-moradi  This log need to be deleted, as
   1. This log will falsely printed when online and consuming segments have 
different replica group.
   2. 
[L388-L392](https://github.com/apache/pinot/pull/14536/files#diff-9ed38679d346d267a71767f40bb6e711e18ea544a243347297dd142b9e310f0aL388-L392)
 already log the partial online segments.



-- 
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

Reply via email to