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


##########
pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManagerTest.java:
##########
@@ -3032,6 +3032,43 @@ public void 
testRepairSegmentsInErrorStateCountsFailedRepairsAsUnrecoverable()
     assertTrue(exception.getMessage().contains(donePartialErrorSegment), 
exception.getMessage());
     verify(mockControllerMetrics).setOrUpdateTableGauge(REALTIME_TABLE_NAME,
         ControllerGauge.PAUSELESS_SEGMENTS_IN_UNRECOVERABLE_ERROR_COUNT, 2L);
+    // The failed re-ingestion trigger and the failed reset are each metered
+    verify(mockControllerMetrics, 
times(2)).addMeteredTableValue(REALTIME_TABLE_NAME,
+        ControllerMeter.LLC_ERROR_SEGMENT_REPAIR_FAILURES, 1L);
+  }
+
+  @Test
+  public void 
testRepairSegmentsInErrorStateMetersFailedResetsOnNonPauselessTable() {

Review Comment:
   Optional coverage improvement: include a three-segment reset batch with two 
failed segments and assert a metered value of 2L. Both tests currently exercise 
a failed-reset set of size one, so they would still pass if 
segmentsFailedToReset.size() were accidentally replaced with 1.



##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/ControllerMeter.java:
##########
@@ -59,6 +59,8 @@ public enum ControllerMeter implements AbstractMetrics.Meter {
   
LLC_SEGMENTS_DEEP_STORE_UPLOAD_RETRY_SUCCESS("LLCSegmentDeepStoreUploadRetrySuccess",
 false),
   
LLC_SEGMENTS_DEEP_STORE_UPLOAD_RETRY_ERROR("LLCSegmentDeepStoreUploadRetryError",
 false),
   SEGMENT_MISSING_DEEP_STORE_LINK("RealtimeSegmentMissingDeepStoreLink", 
false),
+  // Number of failed repair attempts (segment reset or re-ingestion trigger) 
in the periodic error-segment repair

Review Comment:
   Non-blocking: please clarify that this counts failures to initiate repairs, 
once per affected segment per invocation. It does not observe eventual 
asynchronous repair failures or count skipped repairs, and a failed attempt on 
a later pass increments it again. LLC_ERROR_SEGMENT_REPAIR_FAILURES is a 
reasonable name with that contract made explicit.



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

Reply via email to