Jackie-Jiang commented on code in PR #13879:
URL: https://github.com/apache/pinot/pull/13879#discussion_r1744366830


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/SegmentStatusChecker.java:
##########
@@ -318,6 +321,17 @@ private void updateSegmentMetrics(String 
tableNameWithType, TableConfig tableCon
         continue;
       }
 
+      long startTimeMs = segmentZKMetadata.getStartTimeMs();
+      boolean isStartTimeMsInvalid = startTimeMs > 0 && 
!TimeUtils.timeValueInValidRange(startTimeMs);
+      long endTimeMs = segmentZKMetadata.getEndTimeMs();
+      boolean isEndTimeMsInvalid = endTimeMs > 0 && 
!TimeUtils.timeValueInValidRange(endTimeMs);
+      if (isStartTimeMsInvalid) {
+        segmentsInvalidStartTime.add(segment);
+      }
+      if (isEndTimeMsInvalid) {
+        segmentsInvalidEndTime.add(segment);
+      }

Review Comment:
   Let's only skip the `IN_PROGRESS` segments and count negative start/end time 
as invalid:
   ```
       ...
     }
   ```
   ```suggestion
         if (segmentZKMetadata.getStatus() != Status.IN_PROGRESS) {
           if 
(!TimeUtils.timeValueInValidRange(segmentZKMetadata.getStartTimeMs()) {
             segmentsInvalidStartTime.add(segment);
           }
           if 
(!TimeUtils.timeValueInValidRange(segmentZKMetadata.getEndTimeMs()) {
             segmentsInvalidEndTime.add(segment);
           }
         }
   ```



##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/ControllerGauge.java:
##########
@@ -35,6 +35,11 @@ public enum ControllerGauge implements AbstractMetrics.Gauge 
{
   PERCENT_OF_REPLICAS("percent", false),
 
   SEGMENTS_IN_ERROR_STATE("segments", false),
+  // Segment start and end time is stored in milliseconds.
+  // Invalid start/end time means the broker time pruner will not work 
correctly.
+  // Invalid end times means time retention will not happen for that segment.
+  SEGMENT_INVALID_START_TIME("segmentInvalidStartTime", false),
+  SEGMENT_INVALID_END_TIME("segmentInvalidEndTime", false),

Review Comment:
   ```suggestion
     SEGMENTS_WITH_INVALID_START_TIME("segments", false),
     SEGMENTS_WITH_INVALID_END_TIME("segments", false),
   ```



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