fix: align skipIntervals to segmentGranularity in DataSourceCompactibleSegmentIterator

[cecemei]

Description

Follow-up to #20007, addressing #20007 (comment).

---

This PR has:

• been self-reviewed.
  • using the concurrency checklist (Remove this item if the PR doesn't have any relation to concurrency.)
• added documentation for new or modified features or behaviors.
• a release note entry in the PR description.
• added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
• added or updated version, license, or notice information in licenses.yaml
• added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
• added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
• added integration tests.
• been tested in a test Druid cluster.

[FrankChen021]

Severity	Findings
P0	0
P1	0
P2	1
P3	1
Total	2

Reviewed 3 of 3 changed files.

Validation: git diff --check 1e1ee8cc454985197de5c6ceaf31a4d55e3ca39f..a5117d3ac085947f2dab1d2549408efaf9384573 passed. Builds and tests not run.

---

This is an automated review by Codex GPT-5.6-Luna(max)

[kfaraz]

Thanks for the changes, @cecemei !

I was under the impression that the code already ignores segments that cross the skip interval boundaries.

druid/server/src/main/java/org/apache/druid/server/compaction/DataSourceCompactibleSegmentIterator.java

Lines 418 to 424 in cea0d99

	final List<DataSegment> segments = timeline
	.findNonOvershadowedObjectsInInterval(lookupInterval, Partitions.ONLY_COMPLETE)
	.stream()
	// findNonOvershadowedObjectsInInterval() may return segments merely intersecting with lookupInterval, while
	// we are interested only in segments fully lying within lookupInterval here.
	.filter(segment -> lookupInterval.contains(segment.getInterval()))
	.toList();

The above snippet is from the method DataSourceCompactibleSegmentIterator.findInitialSearchInterval().

This method uses the SegmentTimeline (which is already aligned to the target segment granularity) to look up segments that need to be compacted. If any segment does not lie fully within the lookup interval, that segment will not be picked for compaction.

But since the lookup interval has no overlap with any of the skip intervals (due to the filterSkipIntervals step performed earlier), we can be sure that none of the segments that fully lie in the lookup interval have any overlap with any of the skip intervals.

@cecemei , did you encounter/write up a test case which currently fails due to misaligned granularities?
I think it would be worth checking that out first.

[cecemei]

Thanks for the changes, @cecemei !

I was under the impression that the code already ignores segments that cross the skip interval boundaries.

druid/server/src/main/java/org/apache/druid/server/compaction/DataSourceCompactibleSegmentIterator.java

Lines 418 to 424 in cea0d99

	final List<DataSegment> segments = timeline
	.findNonOvershadowedObjectsInInterval(lookupInterval, Partitions.ONLY_COMPLETE)
	.stream()
	// findNonOvershadowedObjectsInInterval() may return segments merely intersecting with lookupInterval, while
	// we are interested only in segments fully lying within lookupInterval here.
	.filter(segment -> lookupInterval.contains(segment.getInterval()))
	.toList();

The above snippet is from the method DataSourceCompactibleSegmentIterator.findInitialSearchInterval().

This method uses the SegmentTimeline (which is already aligned to the target segment granularity) to look up segments that need to be compacted. If any segment does not lie fully within the lookup interval, that segment will not be picked for compaction.

But since the lookup interval has no overlap with any of the skip intervals (due to the filterSkipIntervals step performed earlier), we can be sure that none of the segments that fully lie in the lookup interval have any overlap with any of the skip intervals.

@cecemei , did you encounter/write up a test case which currently fails due to misaligned granularities? I think it would be worth checking that out first.

The test case i added in this PR would fail without the change made. The case here is that we could have a skip interval in the middle of the day (10am - 12pm), and there're segments overlapping (0am - 10am) which get passed into findAndEnqueueSegmentsToCompact, so only the exact skip interval is not compacted but some segments during the day is still being compacted.

[kfaraz]

The test case i added in this PR would fail without the change made. The case here is that we could have a skip interval in the middle of the day (10am - 12pm), and there're segments overlapping (0am - 10am) which get passed into findAndEnqueueSegmentsToCompact, so only the exact skip interval is not compacted but some segments during the day is still being compacted.

Thanks for adding the test! The problem is that the segment.getInterval() still returns the original segment interval (which makes sense) rather than the interval aligned to the segment granularity.

[kfaraz]

Leaving minor suggestions to simplify the compaction skip reason.

[kfaraz]

The test makes sense, thanks!

[FrankChen021]

Severity	Findings
P0	0
P1	0
P2	1
P3	0
Total	1

Reviewed 3 of 3 changed files.

Validation: git diff --check 1e1ee8cc454985197de5c6ceaf31a4d55e3ca39f4 97fc0a842e3152edc00eb6de3df6523442ac99d4 passed. Builds and tests were not run.

---

This is an automated review by Codex GPT-5.6-Luna(max)

[FrankChen021]

[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.

[kfaraz]

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.

[kfaraz]

LGTM