kfaraz commented on code in PR #20027:
URL: https://github.com/apache/druid/pull/20027#discussion_r3804028700
##########
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:
Yeah, I guess we can add a sanity check in the constructor itself and if any
of the skip intervals is eternity, we just skip compaction of all the segments.
--
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]
