Support segmentGranularity for auto-compaction by maytasm · Pull Request #10843 · apache/druid

maytasm · 2021-02-03T11:16:51Z

Support segmentGranularity for auto-compaction

Description

The auto-compaction configuration should support a segmentGranularity option like manual compaction task.

Storing granularitySpec in CompactionState. This is so that auto compaction can determine if compaction needs to be done or not (when segmentGranularity in auto compaction config changes)
Adds granularitySpec inDataSourceCompactionConfig. This allows segmentGranularity to be set in auto compaction config. Currently, only segmentGranularity of granularitySpec is supported by auto compaction and setting other values (i.e. queryGranularity) will fail.
In CompactSegments, pass granularitySpec to indexingServiceClient.compactSegments. This is to pass the segmentGranularity value inside granularitySpec into the ingestionSpec of the compact task. Note that we also add granularitySpec field in CompactionTask and deprecates segmentGranularity field in CompactionTask. This is so that in the future we can support queryGranularity and rollup in compaction task. As a result, in CompactionTask we uses the segmentGranularity inside granularitySpec instead of the deprecated top level segmentGranularity field.
needsCompaction in NewestSegmentFirstIterator needs to take into account changing granularitySpec. We will have to compact segments when segmentGranularity changes.
When segmentGranularity changes, we also cancel any active compaction task that have different segmentGranularity and re-compact those intervals with the new segmentGranularity.
NewestSegmentFirstIterator will also possibly return multiple segments of the same bucket for the new segmentGranularity (if segmentGranularity given). For example, if the original segment granularity is day then NewestSegmentFirstIterator currently will return a single day for each iteration. But if the new segmentGranularity in the auto compaction config is larger such as week, we cannot just compact a single day with segment granularity = week (Basically we cannot send a compact task with interval of a single day). Hence, the NewestSegmentFirstIterator will have to take into account the new segmentGranularity (week) and combine 7 days and return a period of seven days.

This PR has:

been self-reviewed.
added documentation for new or modified features or behaviors.
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.

Key changed/added classes in this PR

CompactSegments
NewestSegmentFirstIterator
DataSourceCompactionConfig
CompactionState

loquisgon · 2021-02-05T23:23:02Z

  @Nullable
  private final Granularity segmentGranularity;
  @Nullable
+  private final GranularitySpec granularitySpec;


You can remove this since it is no longer used

What is no longer used?

loquisgon · 2021-02-05T23:25:40Z

+          null
+      );
+    } else {
+      this.granularitySpec = granularitySpec;


What happens if both segmentGranularity & granularitySpec are non-null?

granularitySpec takes priority since segmentGranularity is deprecated.

I'm not sure if we have a consistent stance on this but I think several places throw an exception if both the deprecated and new property are specified (such as Checks.checkOneNotNullOrEmpty usages).

I don't feel strongly about it, so I'm just mentioning this in case you feel that's a better approach.

loquisgon · 2021-02-06T00:50:00Z

    return new UniformGranularitySpec(segmentGranularity, queryGranularity, rollup, inputIntervals);
  }

+  @Override


Is it possible to move this to the abstract base class?

Good idea. Done.

jon-wei · 2021-02-08T23:54:18Z

+  // This is needed for datasource that has segmentGranularity configured
+  // If configured segmentGranularity is finer than current segmentGranularity, the same set of segments
+  // can belong to multiple intervals in the timeline. We keep track of the
+  private final Map<String, Set<Interval>> intervalCompactedForDatasource = new HashMap<>();


Did you mean to add more to the comment?

jon-wei · 2021-02-09T00:56:35Z

+            int partitions = segmentSet.size();
+            for (DataSegment segment : segmentSet) {
+              DataSegment segmentsForCompact = segment.withShardSpec(new NumberedShardSpec(partitionNum, partitions));
+              // PartitionHolder can only holds chucks of one partition space


chucks -> chunks

maytasm added 20 commits January 26, 2021 01:41

Support segmentGranularity for auto-compaction

4dc9c5b

Support segmentGranularity for auto-compaction

43df955

Support segmentGranularity for auto-compaction

77d953c

Support segmentGranularity for auto-compaction

064a43f

resolve conflict

4182bd5

resolve conflict

7c6d17a

Support segmentGranularity for auto-compaction

7786851

Support segmentGranularity for auto-compaction

ac12903

fix tests

4994ca7

fix more tests

46e3416

fix checkstyle

9a0fb8b

add unit tests

5a22162

fix checkstyle

092868f

fix checkstyle

ace4281

fix checkstyle

5873318

add unit tests

a09deea

add integration tests

9ed2c61

fix checkstyle

020566a

fix checkstyle

4f031b6

fix failing tests

2dfdcd4

loquisgon reviewed Feb 5, 2021

View reviewed changes

loquisgon reviewed Feb 6, 2021

View reviewed changes

maytasm added 2 commits February 5, 2021 19:04

address comments

ad11616

address comments

94ee532

maytasm added Area - Batch Ingestion Release Notes labels Feb 6, 2021

jon-wei reviewed Feb 9, 2021

View reviewed changes

maytasm added 2 commits February 10, 2021 02:51

fix tests

343a07b

fix tests

a506287

maytasm added 8 commits February 10, 2021 03:28

fix test

da018dc

fix test

5dd9e50

fix test

d9783b4

fix test

4405fc6

fix test

06060f9

fix test

e6e9030

fix test

578d433

fix test

7784f70

jon-wei approved these changes Feb 12, 2021

View reviewed changes

maytasm merged commit 6541178 into apache:master Feb 12, 2021

maytasm deleted the IMPLY-4334 branch February 12, 2021 11:03

suneet-s mentioned this pull request Mar 16, 2021

CompactionTask throws exception on conflicting segmentGranularity #10996

Merged

6 tasks

jihoonson mentioned this pull request Mar 16, 2021

Auto compaction can fail to find segments for compaction when segment versions are mixed in the same time chunk based on new segment granularity #10999

Closed

jihoonson mentioned this pull request May 20, 2021

Kafka ingestion ignores segment granularities that aren't "simple" granularities #11179

Open

clintropolis added this to the 0.22.0 milestone Aug 12, 2021

clintropolis mentioned this pull request Sep 3, 2021

[Draft] 0.22.0 Release Notes #11657

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support segmentGranularity for auto-compaction#10843

Support segmentGranularity for auto-compaction#10843
maytasm merged 32 commits intoapache:masterfrom
maytasm:IMPLY-4334

maytasm commented Feb 3, 2021 •

edited

Loading

Uh oh!

loquisgon Feb 5, 2021

Uh oh!

maytasm Feb 6, 2021

Uh oh!

loquisgon Feb 5, 2021

Uh oh!

maytasm Feb 6, 2021

Uh oh!

jon-wei Feb 12, 2021

Uh oh!

loquisgon Feb 6, 2021 •

edited

Loading

Uh oh!

maytasm Feb 6, 2021

Uh oh!

jon-wei Feb 8, 2021

Uh oh!

maytasm Feb 10, 2021

Uh oh!

jon-wei Feb 9, 2021

Uh oh!

maytasm Feb 10, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

maytasm commented Feb 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Key changed/added classes in this PR

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

loquisgon Feb 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

maytasm commented Feb 3, 2021 •

edited

Loading

loquisgon Feb 6, 2021 •

edited

Loading