jtao15 commented on code in PR #9890:
URL: https://github.com/apache/pinot/pull/9890#discussion_r1040111143
##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/mergerollup/MergeRollupTaskGenerator.java:
##########
@@ -61,13 +63,26 @@
/**
* A {@link PinotTaskGenerator} implementation for generating tasks of type
{@link MergeRollupTask}
*
- * TODO: Add the support for realtime table
+ * Assumptions:
+ * - When the MergeRollupTask starts the first time, records older than the
bufferTimeMs have already been ingested.
Review Comment:
`getValidBucketEndTimeMsForSegment` is only used for the delay metrics. When
we validate the target bucket in `isValidBucketEndTime`, we do the following:
```
// Check that bucketEndMs <= now - bufferMs
if (bucketEndMs > System.currentTimeMillis() - bufferMs) {
return false;
}
```
If we use maxEndTimeMs of segments instead of `now`, we won't have the issue
that `latest watermarks advanced too far before records are ingested`?
--
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
