snleee commented on a change in pull request #7368:
URL: https://github.com/apache/pinot/pull/7368#discussion_r719652349
##########
File path:
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/mergerollup/MergeRollupTaskGenerator.java
##########
@@ -196,6 +208,7 @@ public String getTaskType() {
// Get the bucket size and buffer
long bucketMs =
TimeUtils.convertPeriodToMillis(mergeConfigs.get(MinionConstants.MergeTask.BUCKET_TIME_PERIOD_KEY));
+ Preconditions.checkState(bucketMs > 0, "Bucket time period must be
larger than 0");
Review comment:
Thanks for pointing this out. I changed to emit an error log since we
should continue to schedule for other tables (the current code will block other
tables if one table has a bad configuration).
For metrics, we will eventually get the alerts from the delay metric so I
don't think that we need to add another metric .
--
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
