FrankChen021 commented on code in PR #20027:
URL: https://github.com/apache/druid/pull/20027#discussion_r3804009638


##########
server/src/main/java/org/apache/druid/server/compaction/DataSourceCompactibleSegmentIterator.java:
##########
@@ -437,25 +422,56 @@ List<Interval> findInitialSearchInterval(SegmentTimeline 
timeline, List<Interval
           .map(segment -> segment.getId().getIntervalEnd())
           .max(Comparator.naturalOrder())
           .orElseThrow(AssertionError::new);
-      searchIntervals.add(new Interval(searchStart, searchEnd));
+
+      final Interval searchInterval = new Interval(searchStart, searchEnd);
+      final Interval overlappingSkipInterval = allSkipIntervals.stream()
+                                                                
.filter(searchInterval::overlaps)
+                                                                .findFirst()
+                                                                .orElse(null);
+
+      // Guardrail check, this should never happen
+      if (overlappingSkipInterval != null) {
+        log.warn(
+            "searchInterval[%s] for datasource[%s] unexpectedly overlaps 
skipInterval[%s]: %s, skipping it",
+            searchInterval, dataSource, overlappingSkipInterval,
+            describeSkipReason(overlappingSkipInterval, skipOffset, 
config.getSkipIntervals(), skipIntervals)
+        );
+        continue;
+      }
+      searchIntervals.add(searchInterval);
     }
 
     return searchIntervals;
   }
 
-  static Interval computeLatestSkipInterval(
-      @Nullable Granularity configuredSegmentGranularity,
-      DateTime latestDataTimestamp,
-      Period skipOffsetFromLatest
+  private static String describeSkipReason(
+      Interval skipInterval,
+      Period skipOffset,
+      List<Interval> configuredSkipIntervals,
+      List<Interval> lockedIntervals
   )
   {
-    if (configuredSegmentGranularity == null) {
-      return new Interval(skipOffsetFromLatest, latestDataTimestamp);
+    if (lockedIntervals.stream().anyMatch(skipInterval::overlaps)) {
+      return StringUtils.format("Interval[%s] locked by another task", 
skipInterval);
+    } else if 
(configuredSkipIntervals.stream().anyMatch(skipInterval::overlaps)) {
+      return StringUtils.format("Interval[%s] skipped by compaction config", 
skipInterval);
     } else {
-      DateTime skipFromLastest = new DateTime(latestDataTimestamp, 
latestDataTimestamp.getZone()).minus(skipOffsetFromLatest);
-      DateTime skipOffsetBucketToSegmentGranularity = 
configuredSegmentGranularity.bucketStart(skipFromLastest);
-      return new Interval(skipOffsetBucketToSegmentGranularity, 
latestDataTimestamp);
+      return StringUtils.format("Skip offset from latest[%s]", skipOffset);
+    }
+  }
+
+  private Interval alignToSegmentGranularity(Interval interval)
+  {
+    final Granularity segmentGranularity = config.getSegmentGranularity();
+    if (segmentGranularity == null) {
+      return interval;
     }
+    final DateTime alignedStart = 
segmentGranularity.bucketStart(interval.getStart());
+    final DateTime endBucketStart = 
segmentGranularity.bucketStart(interval.getEnd());
+    final DateTime alignedEnd = endBucketStart.isEqual(interval.getEnd())

Review Comment:
   [P2] ETERNITY skip intervals overflow during alignment
   
   If a configured or supplied skip interval is Intervals.ETERNITY and a 
segment granularity such as DAY is configured, bucketStart(MIN) moves the start 
before DateTimes.MIN and bucketEnd(MAX) moves the end after DateTimes.MAX. The 
resulting interval has a duration greater than Long.MAX_VALUE, so 
JodaUtils.condenseIntervals throws ArithmeticException from toDurationMillis() 
before compaction starts. Preserve ETERNITY or clamp aligned endpoints to the 
supported Druid bounds.



-- 
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]

Reply via email to