mcvsubbu commented on a change in pull request #7368:
URL: https://github.com/apache/pinot/pull/7368#discussion_r719564642
##########
File path:
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/mergerollup/MergeRollupTaskGenerator.java
##########
@@ -463,4 +478,134 @@ private long getWatermarkMs(long minStartTimeMs, long
bucketMs, String mergeLeve
return pinotTaskConfigs;
}
+
+ private long getMergeRollupTaskDelayInNumTimeBuckets(long watermarkMs, long
bufferTimeMs, long bucketTimeMs) {
Review comment:
int should be ok for numBuckets?
##########
File path:
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/mergerollup/MergeRollupTaskGenerator.java
##########
@@ -94,6 +96,9 @@
private static final String REFRESH = "REFRESH";
private static final Logger LOGGER =
LoggerFactory.getLogger(MergeRollupTaskGenerator.class);
+ // Metrics
+ private static final String MERGE_ROLLUP_TASK_DELAY_IN_NUM_BUCKETS =
"mergeRollupTaskDelayInNumBuckets";
Review comment:
+1, also try to include some explanation of what action may need to be
taken if the metric goes above/below whatever levels. It is not obvious from
the name of the metric.
Thanks.
##########
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:
Do we want to throw an exception here? Or, do we want to just log an
error message and not generate any tasks and bump some metric that we can watch
for non-zero?
--
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]
