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 because
no segments will be scheduled for this table. I think that this would be good
enough to start with.
--
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]
