Skip to content

Refactor: Ensure that CompactionCandidate.currentStatus is non-null#19058

Open
kfaraz wants to merge 5 commits intoapache:masterfrom
kfaraz:make_status_nonnull
Open

Refactor: Ensure that CompactionCandidate.currentStatus is non-null#19058
kfaraz wants to merge 5 commits intoapache:masterfrom
kfaraz:make_status_nonnull

Conversation

@kfaraz
Copy link
Contributor

@kfaraz kfaraz commented Feb 26, 2026

Description

Inspired by #18968 , this PR tries to ensure that the currentStatus inside a CompactionCandidate is always non-null,
with a relatively smaller changeset.


This PR has:

  • been self-reviewed.
  • 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.

@kfaraz kfaraz requested a review from cecemei February 26, 2026 01:00
Copy link
Contributor

@cecemei cecemei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks more clear with a non-null compaction status, left a few nits.

{
final Set<Interval> segmentIntervals =
segments.stream().map(DataSegment::getInterval).collect(Collectors.toSet());
final Interval umbrellaInterval = JodaUtils.umbrellaInterval(segmentIntervals);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wondering if umbrellaInterval would be different from compactionInterval? seems like DataSourceCompactibleSegmentIterator.populateQueue would create DataSegment with updated interval and shard spec, if config.getSegmentGranularity is set.

@@ -68,7 +86,7 @@ public static CompactionCandidate from(
umbrellaInterval,
compactionInterval,
segmentIntervals.size(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: could use CompactionStatistics to wrap numIntervals, totalBytes and numSegments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants