snleee commented on code in PR #10026: URL: https://github.com/apache/pinot/pull/10026#discussion_r1055977814
########## pinot-common/src/main/java/org/apache/pinot/common/metrics/ControllerGauge.java: ########## @@ -53,6 +53,8 @@ public enum ControllerGauge implements AbstractMetrics.Gauge { DISABLED_TABLE_COUNT("TableCount", true), PERIODIC_TASK_NUM_TABLES_PROCESSED("PeriodicTaskNumTablesProcessed", true), TIME_MS_SINCE_LAST_MINION_TASK_METADATA_UPDATE("TimeMsSinceLastMinionTaskMetadataUpdate", false), + TIME_MS_SINCE_LAST_SUCCESSFUL_MINION_TASK_GENERATION("TimeMsSinceLastSuccessfulMinionTaskGeneration", false), + LAST_MINION_TASK_GENERATION_ENCOUNTERS_ERROR("LastMinionTaskGenerationEncountersError", false), Review Comment: What's the main goal of this event? We are trying to indicate that at certain time we had the issue with the task generation? I think that many existing error related metrics (query error, llc commit error etc) are being emitted as meter. Have you considered both? I think that the main difference is that - Gauge will show 1 from the time that we had the error until the next successful run. - Meter will show one spike in the graph and each spike will indicate the unsuccessful run. ########## pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotTaskManager.java: ########## @@ -541,20 +541,35 @@ private String scheduleTask(PinotTaskGenerator taskGenerator, List<TableConfig> generateTasks() return a list of TaskGeneratorMostRecentRunInfo for each table */ pinotTaskConfigs = taskGenerator.generateTasks(enabledTableConfigs); + long successRunTs = System.currentTimeMillis(); Review Comment: What does `Ts` stands for? `TaskSchedule`? Let's just use the full name for clarity. -- 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