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

Reply via email to