lnbest0707-uber commented on code in PR #12336:
URL: https://github.com/apache/pinot/pull/12336#discussion_r1486777455


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/SegmentStatusChecker.java:
##########
@@ -323,9 +330,12 @@ private void updateSegmentMetrics(String 
tableNameWithType, TableConfig tableCon
     if (nOffline > 0) {
       LOGGER.warn("Table {} has {} segments with no online replicas", 
tableNameWithType, nOffline);
     }
+    if (nLessReplicas > 0) {
+      LOGGER.info("Table {} has {} segments with no high availability", 
tableNameWithType, nLessReplicas);
+    }
     if (nReplicasExternal < nReplicasIdealMax) {
-      LOGGER.warn("Table {} has {} replicas, below replication threshold :{}", 
tableNameWithType, nReplicasExternal,
-          nReplicasIdealMax);
+      LOGGER.warn("Table {} has {} replicas, below replication threshold :{}, 
with {} segments running with less than"

Review Comment:
   It was logged as this because the above on was info not warn.
   Changed that one to warn and remove logging nLessReplicas here.
   
   > What do we mean by "Table foo has x replicas"? Can we say: "Table foo has 
at least one segment with x replicas...."
   
   Yes, correct. Made the changed.
   
   > I think we should also rename nReplicasExternal to minReplicasOfAnySegment 
to make this more readable.
   
   Agree. But did not add that in this patch as it has more reference 
occurrence to be changed. Will probably revise in another patch.



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