akshayrai commented on a change in pull request #4890: [TE] Stop producing so
many logs -- don't check for holes in the middle of timeseries for complete
cache misses
URL: https://github.com/apache/incubator-pinot/pull/4890#discussion_r354439738
##########
File path:
thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/cache/ThirdEyeCacheResponse.java
##########
@@ -127,26 +127,29 @@ public boolean isMissingEndSlice(long sliceEnd) {
*/
private void checkAndLogMissingMiddleSlices() {
- List<String> missingTimestamps = new ArrayList<>();
- long timeGranularity =
request.getRequest().getGroupByTimeGranularity().toMillis();
-
- // remember that we return the cached timeseries in sorted order,
- // but this assumption is not necessarily true if mergeSliceIntoRows() has
been called.
- for (int i = 1; i < timeSeriesRows.size(); i++) {
- long previousTimestamp = timeSeriesRows.get(i - 1).getTimestamp();
- long currentTimestamp = timeSeriesRows.get(i).getTimestamp();
-
- // add all missing timestamps between previous timestamp and current
timestamp to
- // the list of missing timestamps.
- while (previousTimestamp + timeGranularity < currentTimestamp) {
- missingTimestamps.add(String.valueOf(previousTimestamp +
timeGranularity));
- previousTimestamp += timeGranularity;
+ if (!this.hasNoRows()) {
+ List<String> missingTimestamps = new ArrayList<>();
+ long timeGranularity =
request.getRequest().getGroupByTimeGranularity().toMillis();
+
+ // remember that we return the cached timeseries in sorted order,
+ // but this assumption is not necessarily true if mergeSliceIntoRows()
has been called.
+ for (int i = 1; i < timeSeriesRows.size(); i++) {
+ long previousTimestamp = timeSeriesRows.get(i - 1).getTimestamp();
+ long currentTimestamp = timeSeriesRows.get(i).getTimestamp();
+
+ // add all missing timestamps between previous timestamp and current
timestamp to
+ // the list of missing timestamps.
+ while (previousTimestamp + timeGranularity < currentTimestamp) {
+ missingTimestamps.add(String.valueOf(previousTimestamp +
timeGranularity));
+ previousTimestamp += timeGranularity;
+ }
}
- }
- if (missingTimestamps.size() > 0) {
- LOG.info("cached time-series for metricUrn {} was missing data points in
the middle for {} timestamps: {}",
- request.getMetricUrn(), missingTimestamps.size(), String.join(",",
missingTimestamps));
+ // we will need to evaluate whether this is generating too many logs.
+ if (missingTimestamps.size() > 0) {
+ LOG.info("cached time-series for metricUrn {} was missing data points
in the middle for {} timestamps: {}",
Review comment:
Instead of logging all the missing timestamps, just log the start and end
timestamp along with the granularity (in days).
- TimeUnit.MILLISECONDS.toDays(granularity)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
