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