mcvsubbu commented on a change in pull request #7368:
URL: https://github.com/apache/pinot/pull/7368#discussion_r719796515



##########
File path: 
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/mergerollup/MergeRollupTaskGenerator.java
##########
@@ -90,15 +92,29 @@
  */
 @TaskGenerator
 public class MergeRollupTaskGenerator implements PinotTaskGenerator {
+  private static final Logger LOGGER = 
LoggerFactory.getLogger(MergeRollupTaskGenerator.class);
   private static final int DEFAULT_MAX_NUM_RECORDS_PER_TASK = 50_000_000;
   private static final String REFRESH = "REFRESH";
-  private static final Logger LOGGER = 
LoggerFactory.getLogger(MergeRollupTaskGenerator.class);
 
+  // This is the metric that keeps track of the task delay in the number of 
time buckets. For example, if we see this

Review comment:
       Ah nice, yes.
   
   In that case, does it make sense to make this a time-based metric (e.g. 
number of hours)? Then it is easier to explain it to admins -- set the limit to 
the number of hours you can tolerate in terms of delay.
   
   just a suggestion.




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