[GitHub] [pinot] mqliang commented on a change in pull request #7557: Clean up controller-table related metrics in ControllerPeriodicTask

[GitBox <gi...@apache.org>]

mqliang commented on a change in pull request #7557:
URL: https://github.com/apache/pinot/pull/7557#discussion_r726786292



##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/validation/RealtimeSegmentValidationManager.java
##########
@@ -124,6 +124,16 @@ private void runSegmentLevelValidation(TableConfig tableConfig) {
     }
   }
 
+  @Override
+  protected void nonLeaderCleanup(List<String> tableNamesWithType) {
+    for (String tableNameWithType : tableNamesWithType) {
+      TableType tableType = TableNameBuilder.getTableTypeFromTableName(tableNameWithType);
+      if (tableType == TableType.REALTIME) {
+        _validationMetrics.cleanupTotalDocumentCountGauge(tableNameWithType);

Review comment:
       Same here, change as
   ```
   _validationMetrics.removeGauge(tableNameWithType, "TotalDocumentCount");
   
   ```

##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/validation/OfflineSegmentIntervalChecker.java
##########
@@ -136,6 +136,20 @@ private void validateOfflineSegmentPush(TableConfig tableConfig) {
     _validationMetrics.updateSegmentCountGauge(offlineTableName, numSegments);
   }
 
+  @Override
+  protected void nonLeaderCleanup(List<String> tableNamesWithType) {
+    for (String tableNameWithType : tableNamesWithType) {
+      TableType tableType = TableNameBuilder.getTableTypeFromTableName(tableNameWithType);
+      if (tableType == TableType.OFFLINE) {
+        _validationMetrics.cleanupMissingSegmentCountGauge(tableNameWithType);
+        _validationMetrics.cleanupOfflineSegmentDelayGauge(tableNameWithType);
+        _validationMetrics.cleanupLastPushTimeGauge(tableNameWithType);
+        _validationMetrics.cleanupTotalDocumentCountGauge(tableNameWithType);
+        _validationMetrics.cleanupSegmentCountGauge(tableNameWithType);

Review comment:
       Too many code duplication in ValidationMetrics.java IMO. Remove all cleanupXXXGauge() functions in ValidationMetrics.java, change `removeGauge()` in ValidationMetrics.java as:
   ```
     public void removeGauge(final String resourceName, final String gaugeName) {
       final String fullGaugeName = makeGaugeName(resourceName, gaugeName);
       PinotMetricName pinotMetricName = makeMetricName(fullGaugeName);
       PinotMetricUtils.removeMetric(_metricsRegistry, pinotMetricName);
       _metricNames.remove(pinotMetricName);
       _gaugeValues.remove(fullGaugeName);
     }
   
   ```
   
   then change L144-L148 here as:
   ```
   _validationMetrics.removeGauge(tableNameWithType, "missingSegmentCount");
   _validationMetrics.removeGauge(tableNameWithType, "offlineSegmentDelayHours");
   _validationMetrics.removeGauge(tableNameWithType, "lastPushTimeDelayHours");
   _validationMetrics.removeGauge(tableNameWithType, "TotalDocumentCount");
   _validationMetrics.removeGauge(tableNameWithType, "SegmentCount");
   
   ```
   
   At the same time, as gauge names ("missingSegmentCount", "offlineSegmentDelayHours", etc) are used at multiple places, define them as static variable.




-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org