From 4dc9c5bad4a5e887309fed5c4af2a8bd7b84e055 Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Tue, 26 Jan 2021 01:41:06 -0800 Subject: [PATCH 01/31] Support segmentGranularity for auto-compaction --- .../NewestSegmentFirstPolicyBenchmark.java | 1 + .../indexing/common/task/CompactionTask.java | 24 ++++- .../ClientCompactionTaskQuerySerdeTest.java | 2 + .../duty/ITAutoCompactionTest.java | 1 + .../indexing/ClientCompactionTaskQuery.java | 15 ++- ...entCompactionTaskQueryGranularitySpec.java | 95 +++++++++++++++++++ .../indexing/HttpIndexingServiceClient.java | 2 + .../indexing/IndexingServiceClient.java | 1 + .../granularity/UniformGranularitySpec.java | 7 +- .../DataSourceCompactionConfig.java | 33 +++++++ .../coordinator/duty/CompactSegments.java | 6 ++ .../indexing/NoopIndexingServiceClient.java | 1 + .../DataSourceCompactionConfigTest.java | 4 + .../coordinator/duty/CompactSegmentsTest.java | 1 + .../duty/NewestSegmentFirstIteratorTest.java | 9 ++ .../duty/NewestSegmentFirstPolicyTest.java | 1 + 16 files changed, 198 insertions(+), 5 deletions(-) create mode 100644 server/src/main/java/org/apache/druid/client/indexing/ClientCompactionTaskQueryGranularitySpec.java diff --git a/benchmarks/src/test/java/org/apache/druid/server/coordinator/NewestSegmentFirstPolicyBenchmark.java b/benchmarks/src/test/java/org/apache/druid/server/coordinator/NewestSegmentFirstPolicyBenchmark.java index 91c1409b4071..e744bf92d2b9 100644 --- a/benchmarks/src/test/java/org/apache/druid/server/coordinator/NewestSegmentFirstPolicyBenchmark.java +++ b/benchmarks/src/test/java/org/apache/druid/server/coordinator/NewestSegmentFirstPolicyBenchmark.java @@ -99,6 +99,7 @@ public void setup() null, null, null, + null, null ) ); diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java index 5cbaa3529c4a..e57dd723aa07 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java @@ -141,6 +141,8 @@ public class CompactionTask extends AbstractBatchIndexTask @Nullable private final Granularity segmentGranularity; @Nullable + private final GranularitySpec granularitySpec; + @Nullable private final ParallelIndexTuningConfig tuningConfig; @JsonIgnore private final SegmentProvider segmentProvider; @@ -172,7 +174,8 @@ public CompactionTask( @JsonProperty("dimensions") @Nullable final DimensionsSpec dimensions, @JsonProperty("dimensionsSpec") @Nullable final DimensionsSpec dimensionsSpec, @JsonProperty("metricsSpec") @Nullable final AggregatorFactory[] metricsSpec, - @JsonProperty("segmentGranularity") @Nullable final Granularity segmentGranularity, + @JsonProperty("segmentGranularity") @Deprecated @Nullable final Granularity segmentGranularity, + @JsonProperty("granularitySpec") @Nullable final GranularitySpec granularitySpec, @JsonProperty("tuningConfig") @Nullable final TuningConfig tuningConfig, @JsonProperty("context") @Nullable final Map context, @JacksonInject SegmentLoaderFactory segmentLoaderFactory, @@ -202,6 +205,16 @@ public CompactionTask( this.dimensionsSpec = dimensionsSpec == null ? dimensions : dimensionsSpec; this.metricsSpec = metricsSpec; this.segmentGranularity = segmentGranularity; + if (granularitySpec == null && segmentGranularity != null) { + this.granularitySpec = new UniformGranularitySpec( + segmentGranularity, + UniformGranularitySpec.DEFAULT_QUERY_GRANULARITY, + UniformGranularitySpec.DEFAULT_ROLLUP, + null + ); + } else { + this.granularitySpec = granularitySpec; + } this.tuningConfig = tuningConfig != null ? getTuningConfig(tuningConfig) : null; this.segmentProvider = new SegmentProvider(dataSource, this.ioConfig.getInputSpec()); this.partitionConfigurationManager = new PartitionConfigurationManager(this.tuningConfig); @@ -891,6 +904,8 @@ public static class Builder @Nullable private Granularity segmentGranularity; @Nullable + private GranularitySpec granularitySpec; + @Nullable private TuningConfig tuningConfig; @Nullable private Map context; @@ -940,6 +955,12 @@ public Builder segmentGranularity(Granularity segmentGranularity) return this; } + public Builder granularitySpec(GranularitySpec granularitySpec) + { + this.granularitySpec = granularitySpec; + return this; + } + public Builder tuningConfig(TuningConfig tuningConfig) { this.tuningConfig = tuningConfig; @@ -965,6 +986,7 @@ public CompactionTask build() dimensionsSpec, metricsSpec, segmentGranularity, + granularitySpec, tuningConfig, context, segmentLoaderFactory, diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/ClientCompactionTaskQuerySerdeTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/ClientCompactionTaskQuerySerdeTest.java index a25ae37648ae..589772187c9b 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/ClientCompactionTaskQuerySerdeTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/ClientCompactionTaskQuerySerdeTest.java @@ -113,6 +113,7 @@ public void testClientCompactionTaskQueryToCompactionTask() throws IOException 1000, 100 ), + null, ImmutableMap.of("key", "value") ); @@ -283,6 +284,7 @@ public void testCompactionTaskToClientCompactionTaskQuery() throws IOException 1000, 100 ), + null, new HashMap<>() ); diff --git a/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java b/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java index 5d1d55ba7b63..839f8202889c 100644 --- a/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java +++ b/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java @@ -348,6 +348,7 @@ private void submitCompactionConfig( null, 1 ), + null, null ); compactionResource.submitCompactionConfig(compactionConfig); diff --git a/server/src/main/java/org/apache/druid/client/indexing/ClientCompactionTaskQuery.java b/server/src/main/java/org/apache/druid/client/indexing/ClientCompactionTaskQuery.java index d0dd1756f028..4745046abbdc 100644 --- a/server/src/main/java/org/apache/druid/client/indexing/ClientCompactionTaskQuery.java +++ b/server/src/main/java/org/apache/druid/client/indexing/ClientCompactionTaskQuery.java @@ -22,6 +22,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Preconditions; +import org.apache.druid.segment.indexing.granularity.GranularitySpec; import java.util.Map; import java.util.Objects; @@ -38,6 +39,7 @@ public class ClientCompactionTaskQuery implements ClientTaskQuery private final String dataSource; private final ClientCompactionIOConfig ioConfig; private final ClientCompactionTaskQueryTuningConfig tuningConfig; + private final ClientCompactionTaskQueryGranularitySpec granularitySpec; private final Map context; @JsonCreator @@ -46,6 +48,7 @@ public ClientCompactionTaskQuery( @JsonProperty("dataSource") String dataSource, @JsonProperty("ioConfig") ClientCompactionIOConfig ioConfig, @JsonProperty("tuningConfig") ClientCompactionTaskQueryTuningConfig tuningConfig, + @JsonProperty("granularitySpec") ClientCompactionTaskQueryGranularitySpec granularitySpec, @JsonProperty("context") Map context ) { @@ -53,6 +56,7 @@ public ClientCompactionTaskQuery( this.dataSource = dataSource; this.ioConfig = ioConfig; this.tuningConfig = tuningConfig; + this.granularitySpec = granularitySpec; this.context = context; } @@ -89,12 +93,19 @@ public ClientCompactionTaskQueryTuningConfig getTuningConfig() return tuningConfig; } + @JsonProperty + public ClientCompactionTaskQueryGranularitySpec getGranularitySpec() + { + return granularitySpec; + } + @JsonProperty public Map getContext() { return context; } + @Override public boolean equals(Object o) { @@ -109,13 +120,14 @@ public boolean equals(Object o) Objects.equals(dataSource, that.dataSource) && Objects.equals(ioConfig, that.ioConfig) && Objects.equals(tuningConfig, that.tuningConfig) && + Objects.equals(granularitySpec, that.granularitySpec) && Objects.equals(context, that.context); } @Override public int hashCode() { - return Objects.hash(id, dataSource, ioConfig, tuningConfig, context); + return Objects.hash(id, dataSource, ioConfig, tuningConfig, granularitySpec, context); } @Override @@ -126,6 +138,7 @@ public String toString() ", dataSource='" + dataSource + '\'' + ", ioConfig=" + ioConfig + ", tuningConfig=" + tuningConfig + + ", granularitySpec=" + granularitySpec + ", context=" + context + '}'; } diff --git a/server/src/main/java/org/apache/druid/client/indexing/ClientCompactionTaskQueryGranularitySpec.java b/server/src/main/java/org/apache/druid/client/indexing/ClientCompactionTaskQueryGranularitySpec.java new file mode 100644 index 000000000000..1739301da162 --- /dev/null +++ b/server/src/main/java/org/apache/druid/client/indexing/ClientCompactionTaskQueryGranularitySpec.java @@ -0,0 +1,95 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.client.indexing; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import org.apache.druid.java.util.common.granularity.Granularity; +import org.apache.druid.segment.indexing.granularity.UniformGranularitySpec; + +import java.util.Objects; + +public class ClientCompactionTaskQueryGranularitySpec +{ + private final Granularity segmentGranularity; + private final Granularity queryGranularity; + private final Boolean rollup; + + @JsonCreator + public ClientCompactionTaskQueryGranularitySpec( + @JsonProperty("segmentGranularity") Granularity segmentGranularity, + @JsonProperty("queryGranularity") Granularity queryGranularity, + @JsonProperty("rollup") Boolean rollup + ) + { + this.queryGranularity = queryGranularity == null ? UniformGranularitySpec.DEFAULT_QUERY_GRANULARITY : queryGranularity; + this.rollup = rollup == null ? UniformGranularitySpec.DEFAULT_ROLLUP : rollup; + this.segmentGranularity = segmentGranularity == null ? UniformGranularitySpec.DEFAULT_SEGMENT_GRANULARITY : segmentGranularity; + } + + @JsonProperty + public Granularity getSegmentGranularity() + { + return segmentGranularity; + } + + @JsonProperty + public Granularity getQueryGranularity() + { + return queryGranularity; + } + + @JsonProperty + public Boolean getRollup() + { + return rollup; + } + + @Override + public String toString() + { + return "ClientCompactionTaskQueryGranularitySpec{" + + "segmentGranularity=" + segmentGranularity + + ", queryGranularity=" + queryGranularity + + ", rollup=" + rollup + + '}'; + } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + ClientCompactionTaskQueryGranularitySpec that = (ClientCompactionTaskQueryGranularitySpec) o; + return Objects.equals(segmentGranularity, that.segmentGranularity) && + Objects.equals(queryGranularity, that.queryGranularity) && + Objects.equals(rollup, that.rollup); + } + + @Override + public int hashCode() + { + return Objects.hash(segmentGranularity, queryGranularity, rollup); + } +} diff --git a/server/src/main/java/org/apache/druid/client/indexing/HttpIndexingServiceClient.java b/server/src/main/java/org/apache/druid/client/indexing/HttpIndexingServiceClient.java index 2b6e47b1f3c8..546438d668a9 100644 --- a/server/src/main/java/org/apache/druid/client/indexing/HttpIndexingServiceClient.java +++ b/server/src/main/java/org/apache/druid/client/indexing/HttpIndexingServiceClient.java @@ -79,6 +79,7 @@ public String compactSegments( List segments, int compactionTaskPriority, ClientCompactionTaskQueryTuningConfig tuningConfig, + ClientCompactionTaskQueryGranularitySpec granularitySpec, @Nullable Map context ) { @@ -99,6 +100,7 @@ public String compactSegments( dataSource, new ClientCompactionIOConfig(ClientCompactionIntervalSpec.fromSegments(segments)), tuningConfig, + granularitySpec, context ); return runTask(taskId, taskQuery); diff --git a/server/src/main/java/org/apache/druid/client/indexing/IndexingServiceClient.java b/server/src/main/java/org/apache/druid/client/indexing/IndexingServiceClient.java index 14d43596a088..8eb803434678 100644 --- a/server/src/main/java/org/apache/druid/client/indexing/IndexingServiceClient.java +++ b/server/src/main/java/org/apache/druid/client/indexing/IndexingServiceClient.java @@ -40,6 +40,7 @@ String compactSegments( List segments, int compactionTaskPriority, @Nullable ClientCompactionTaskQueryTuningConfig tuningConfig, + @Nullable ClientCompactionTaskQueryGranularitySpec granularitySpec, @Nullable Map context ); diff --git a/server/src/main/java/org/apache/druid/segment/indexing/granularity/UniformGranularitySpec.java b/server/src/main/java/org/apache/druid/segment/indexing/granularity/UniformGranularitySpec.java index fae680aba9bc..3fe748d0ce00 100644 --- a/server/src/main/java/org/apache/druid/segment/indexing/granularity/UniformGranularitySpec.java +++ b/server/src/main/java/org/apache/druid/segment/indexing/granularity/UniformGranularitySpec.java @@ -35,8 +35,9 @@ public class UniformGranularitySpec implements GranularitySpec { - private static final Granularity DEFAULT_SEGMENT_GRANULARITY = Granularities.DAY; - private static final Granularity DEFAULT_QUERY_GRANULARITY = Granularities.NONE; + public static final Granularity DEFAULT_SEGMENT_GRANULARITY = Granularities.DAY; + public static final Granularity DEFAULT_QUERY_GRANULARITY = Granularities.NONE; + public static final Boolean DEFAULT_ROLLUP = Boolean.TRUE; private final Granularity segmentGranularity; private final Granularity queryGranularity; @@ -53,7 +54,7 @@ public UniformGranularitySpec( ) { this.queryGranularity = queryGranularity == null ? DEFAULT_QUERY_GRANULARITY : queryGranularity; - this.rollup = rollup == null ? Boolean.TRUE : rollup; + this.rollup = rollup == null ? DEFAULT_ROLLUP : rollup; this.segmentGranularity = segmentGranularity == null ? DEFAULT_SEGMENT_GRANULARITY : segmentGranularity; if (inputIntervals != null) { diff --git a/server/src/main/java/org/apache/druid/server/coordinator/DataSourceCompactionConfig.java b/server/src/main/java/org/apache/druid/server/coordinator/DataSourceCompactionConfig.java index c5a1263dc241..676142a329f5 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/DataSourceCompactionConfig.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/DataSourceCompactionConfig.java @@ -22,6 +22,8 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Preconditions; +import org.apache.druid.segment.indexing.granularity.GranularitySpec; +import org.apache.druid.segment.indexing.granularity.UniformGranularitySpec; import org.joda.time.Period; import javax.annotation.Nullable; @@ -46,6 +48,7 @@ public class DataSourceCompactionConfig private final Integer maxRowsPerSegment; private final Period skipOffsetFromLatest; private final UserCompactionTaskQueryTuningConfig tuningConfig; + private final GranularitySpec granularitySpec; private final Map taskContext; @JsonCreator @@ -56,6 +59,7 @@ public DataSourceCompactionConfig( @JsonProperty("maxRowsPerSegment") @Deprecated @Nullable Integer maxRowsPerSegment, @JsonProperty("skipOffsetFromLatest") @Nullable Period skipOffsetFromLatest, @JsonProperty("tuningConfig") @Nullable UserCompactionTaskQueryTuningConfig tuningConfig, + @JsonProperty("granularitySpec") @Nullable GranularitySpec granularitySpec, @JsonProperty("taskContext") @Nullable Map taskContext ) { @@ -69,6 +73,24 @@ public DataSourceCompactionConfig( this.maxRowsPerSegment = maxRowsPerSegment; this.skipOffsetFromLatest = skipOffsetFromLatest == null ? DEFAULT_SKIP_OFFSET_FROM_LATEST : skipOffsetFromLatest; this.tuningConfig = tuningConfig; + if (granularitySpec != null) { + Preconditions.checkArgument( + granularitySpec instanceof UniformGranularitySpec, + "Auto compaction granularitySpec only supports uniform type" + ); + Preconditions.checkArgument( + granularitySpec.isRollup() == UniformGranularitySpec.DEFAULT_ROLLUP, + "Auto compaction granularitySpec only supports default rollup value" + ); + Preconditions.checkArgument( + granularitySpec.getQueryGranularity().equals(UniformGranularitySpec.DEFAULT_QUERY_GRANULARITY), + "Auto compaction granularitySpec only supports default query granularity value"); + Preconditions.checkArgument( + granularitySpec.inputIntervals().isEmpty() && !granularitySpec.bucketIntervals().isPresent(), + "Auto compaction granularitySpec does not supports interval value" + ); + } + this.granularitySpec = granularitySpec; this.taskContext = taskContext; } @@ -111,6 +133,13 @@ public UserCompactionTaskQueryTuningConfig getTuningConfig() return tuningConfig; } + @JsonProperty + @Nullable + public GranularitySpec getGranularitySpec() + { + return granularitySpec; + } + @JsonProperty @Nullable public Map getTaskContext() @@ -131,8 +160,10 @@ public boolean equals(Object o) return taskPriority == that.taskPriority && inputSegmentSizeBytes == that.inputSegmentSizeBytes && Objects.equals(dataSource, that.dataSource) && + Objects.equals(maxRowsPerSegment, that.maxRowsPerSegment) && Objects.equals(skipOffsetFromLatest, that.skipOffsetFromLatest) && Objects.equals(tuningConfig, that.tuningConfig) && + Objects.equals(granularitySpec, that.granularitySpec) && Objects.equals(taskContext, that.taskContext); } @@ -143,8 +174,10 @@ public int hashCode() dataSource, taskPriority, inputSegmentSizeBytes, + maxRowsPerSegment, skipOffsetFromLatest, tuningConfig, + granularitySpec, taskContext ); } diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/CompactSegments.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/CompactSegments.java index 3b7ee31dab8c..bc178e5c749d 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/CompactSegments.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/CompactSegments.java @@ -24,6 +24,7 @@ import com.google.common.collect.Maps; import com.google.inject.Inject; import org.apache.druid.client.indexing.ClientCompactionTaskQuery; +import org.apache.druid.client.indexing.ClientCompactionTaskQueryGranularitySpec; import org.apache.druid.client.indexing.ClientCompactionTaskQueryTuningConfig; import org.apache.druid.client.indexing.IndexingServiceClient; import org.apache.druid.client.indexing.TaskPayloadResponse; @@ -295,6 +296,11 @@ private CoordinatorStats doRun( segmentsToCompact, config.getTaskPriority(), ClientCompactionTaskQueryTuningConfig.from(config.getTuningConfig(), config.getMaxRowsPerSegment()), + new ClientCompactionTaskQueryGranularitySpec( + config.getGranularitySpec().getSegmentGranularity(), + config.getGranularitySpec().getQueryGranularity(), + config.getGranularitySpec().isRollup() + ), newAutoCompactionContext(config.getTaskContext()) ); diff --git a/server/src/test/java/org/apache/druid/client/indexing/NoopIndexingServiceClient.java b/server/src/test/java/org/apache/druid/client/indexing/NoopIndexingServiceClient.java index 34800cc2dd7e..09f4c693fb1f 100644 --- a/server/src/test/java/org/apache/druid/client/indexing/NoopIndexingServiceClient.java +++ b/server/src/test/java/org/apache/druid/client/indexing/NoopIndexingServiceClient.java @@ -50,6 +50,7 @@ public String compactSegments( List segments, int compactionTaskPriority, @Nullable ClientCompactionTaskQueryTuningConfig tuningConfig, + @Nullable ClientCompactionTaskQueryGranularitySpec granularitySpec, @Nullable Map context ) { diff --git a/server/src/test/java/org/apache/druid/server/coordinator/DataSourceCompactionConfigTest.java b/server/src/test/java/org/apache/druid/server/coordinator/DataSourceCompactionConfigTest.java index 96bfcc126cc6..c2bb8503d5e2 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/DataSourceCompactionConfigTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/DataSourceCompactionConfigTest.java @@ -56,6 +56,7 @@ public void testSerdeBasic() throws IOException null, new Period(3600), null, + null, ImmutableMap.of("key", "val") ); final String json = OBJECT_MAPPER.writeValueAsString(config); @@ -80,6 +81,7 @@ public void testSerdeWithMaxRowsPerSegment() throws IOException 30, new Period(3600), null, + null, ImmutableMap.of("key", "val") ); final String json = OBJECT_MAPPER.writeValueAsString(config); @@ -122,6 +124,7 @@ public void testSerdeWithMaxTotalRows() throws IOException null, null ), + null, ImmutableMap.of("key", "val") ); final String json = OBJECT_MAPPER.writeValueAsString(config); @@ -164,6 +167,7 @@ public void testSerdeMaxTotalRowsWithMaxRowsPerSegment() throws IOException null, null ), + null, ImmutableMap.of("key", "val") ); diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/CompactSegmentsTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/CompactSegmentsTest.java index c8ef83dac463..9bd8a347851c 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/CompactSegmentsTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/CompactSegmentsTest.java @@ -831,6 +831,7 @@ private List createCompactionConfigs(@Nullable Integ null, null ), + null, null ) ); diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIteratorTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIteratorTest.java index 76a84bc29dd9..d2bda07d348a 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIteratorTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIteratorTest.java @@ -90,6 +90,7 @@ public void testFindPartitionsSpecFromConfigWithNullTuningConfigReturnDynamicPar null, null, null, + null, null ); Assert.assertEquals( @@ -128,6 +129,7 @@ public void testFindPartitionsSpecFromConfigWithNullMaxTotalRowsReturnLongMaxVal null, null ), + null, null ); Assert.assertEquals( @@ -166,6 +168,7 @@ public void testFindPartitionsSpecFromConfigWithNonNullMaxTotalRowsReturnGivenVa null, null ), + null, null ); Assert.assertEquals( @@ -204,6 +207,7 @@ public void testFindPartitionsSpecFromConfigWithNonNullMaxRowsPerSegmentReturnGi null, null ), + null, null ); Assert.assertEquals( @@ -242,6 +246,7 @@ public void testFindPartitionsSpecFromConfigWithDeprecatedMaxRowsPerSegmentAndMa null, null ), + null, null ); Assert.assertEquals( @@ -280,6 +285,7 @@ public void testFindPartitionsSpecFromConfigWithDeprecatedMaxRowsPerSegmentAndPa null, null ), + null, null ); Assert.assertEquals( @@ -318,6 +324,7 @@ public void testFindPartitionsSpecFromConfigWithDeprecatedMaxTotalRowsAndPartiti null, null ), + null, null ); Assert.assertEquals( @@ -356,6 +363,7 @@ public void testFindPartitionsSpecFromConfigWithHashPartitionsSpec() null, null ), + null, null ); Assert.assertEquals( @@ -394,6 +402,7 @@ public void testFindPartitionsSpecFromConfigWithRangePartitionsSpec() null, null ), + null, null ); Assert.assertEquals( diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstPolicyTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstPolicyTest.java index 749f9d7c89f6..92c9f779d09a 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstPolicyTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstPolicyTest.java @@ -556,6 +556,7 @@ private DataSourceCompactionConfig createCompactionConfig( null, skipOffsetFromLatest, null, + null, null ); } From 43df955c497dd65996dc596aabb5658885de71fc Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Tue, 26 Jan 2021 01:44:00 -0800 Subject: [PATCH 02/31] Support segmentGranularity for auto-compaction --- .../apache/druid/client/indexing/ClientCompactionTaskQuery.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/main/java/org/apache/druid/client/indexing/ClientCompactionTaskQuery.java b/server/src/main/java/org/apache/druid/client/indexing/ClientCompactionTaskQuery.java index 4745046abbdc..ea4155525db3 100644 --- a/server/src/main/java/org/apache/druid/client/indexing/ClientCompactionTaskQuery.java +++ b/server/src/main/java/org/apache/druid/client/indexing/ClientCompactionTaskQuery.java @@ -22,7 +22,6 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Preconditions; -import org.apache.druid.segment.indexing.granularity.GranularitySpec; import java.util.Map; import java.util.Objects; From 77d953ce1c02b0aa61f6551edf5ba07f964664f9 Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Tue, 26 Jan 2021 02:51:31 -0800 Subject: [PATCH 03/31] Support segmentGranularity for auto-compaction --- .../druid/indexing/common/task/CompactionTask.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java index e57dd723aa07..037d9c452bcd 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java @@ -300,7 +300,14 @@ public AggregatorFactory[] getMetricsSpec() @Override public Granularity getSegmentGranularity() { - return segmentGranularity; + return granularitySpec == null ? null : granularitySpec.getSegmentGranularity(); + } + + @JsonProperty + @Nullable + public GranularitySpec getGranularitySpec() + { + return granularitySpec; } @Nullable @@ -360,7 +367,7 @@ public TaskStatus runTask(TaskToolbox toolbox) throws Exception partitionConfigurationManager, dimensionsSpec, metricsSpec, - segmentGranularity, + granularitySpec == null ? null : granularitySpec.getSegmentGranularity(), toolbox.getCoordinatorClient(), segmentLoaderFactory, retryPolicyFactory From 064a43fa0d3786fe75400c73e29b17b309dc2233 Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Wed, 3 Feb 2021 03:00:28 -0800 Subject: [PATCH 04/31] Support segmentGranularity for auto-compaction --- .../druid/timeline/CompactionState.java | 19 +++++-- .../druid/timeline/DataSegmentTest.java | 4 +- .../common/task/AbstractBatchIndexTask.java | 6 ++- .../druid/indexing/common/task/IndexTask.java | 3 +- .../parallel/ParallelIndexSupervisorTask.java | 3 +- .../task/CompactionTaskParallelRunTest.java | 12 +++-- .../common/task/CompactionTaskRunTest.java | 7 ++- .../granularity/ArbitraryGranularitySpec.java | 12 +++++ .../indexing/granularity/GranularitySpec.java | 4 ++ .../granularity/UniformGranularitySpec.java | 12 +++++ .../coordinator/duty/CompactSegments.java | 23 +++++++- .../duty/NewestSegmentFirstIterator.java | 53 ++++++++++++++++--- .../coordinator/duty/CompactSegmentsTest.java | 9 ++-- .../duty/NewestSegmentFirstIteratorTest.java | 1 + .../sql/calcite/schema/SystemSchemaTest.java | 3 +- 15 files changed, 143 insertions(+), 28 deletions(-) diff --git a/core/src/main/java/org/apache/druid/timeline/CompactionState.java b/core/src/main/java/org/apache/druid/timeline/CompactionState.java index c30f427c19ca..85887179e2c1 100644 --- a/core/src/main/java/org/apache/druid/timeline/CompactionState.java +++ b/core/src/main/java/org/apache/druid/timeline/CompactionState.java @@ -43,15 +43,20 @@ public class CompactionState // org.apache.druid.segment.IndexSpec cannot be used here because it's in the 'processing' module which // has a dependency on the 'core' module where this class is. private final Map indexSpec; + // org.apache.druid.segment.indexing.granularity.GranularitySpec cannot be used here because it's in the + // 'server' module which has a dependency on the 'core' module where this class is. + private final Map granularitySpec; @JsonCreator public CompactionState( @JsonProperty("partitionsSpec") PartitionsSpec partitionsSpec, - @JsonProperty("indexSpec") Map indexSpec + @JsonProperty("indexSpec") Map indexSpec, + @JsonProperty("granularitySpec") Map granularitySpec ) { this.partitionsSpec = partitionsSpec; this.indexSpec = indexSpec; + this.granularitySpec = granularitySpec; } @JsonProperty @@ -66,6 +71,12 @@ public Map getIndexSpec() return indexSpec; } + @JsonProperty + public Map getGranularitySpec() + { + return granularitySpec; + } + @Override public boolean equals(Object o) { @@ -77,13 +88,14 @@ public boolean equals(Object o) } CompactionState that = (CompactionState) o; return Objects.equals(partitionsSpec, that.partitionsSpec) && - Objects.equals(indexSpec, that.indexSpec); + Objects.equals(indexSpec, that.indexSpec) && + Objects.equals(granularitySpec, that.granularitySpec); } @Override public int hashCode() { - return Objects.hash(partitionsSpec, indexSpec); + return Objects.hash(partitionsSpec, indexSpec, granularitySpec); } @Override @@ -92,6 +104,7 @@ public String toString() return "CompactionState{" + "partitionsSpec=" + partitionsSpec + ", indexSpec=" + indexSpec + + ", granularitySpec=" + granularitySpec + '}'; } } diff --git a/core/src/test/java/org/apache/druid/timeline/DataSegmentTest.java b/core/src/test/java/org/apache/druid/timeline/DataSegmentTest.java index 54836421f0b5..66c3400cfd29 100644 --- a/core/src/test/java/org/apache/druid/timeline/DataSegmentTest.java +++ b/core/src/test/java/org/apache/druid/timeline/DataSegmentTest.java @@ -120,6 +120,7 @@ public void testV1Serialization() throws Exception new NumberedShardSpec(3, 0), new CompactionState( new HashedPartitionsSpec(100000, null, ImmutableList.of("dim1")), + ImmutableMap.of(), ImmutableMap.of() ), TEST_VERSION, @@ -231,7 +232,8 @@ public void testWithLastCompactionState() { final CompactionState compactionState = new CompactionState( new DynamicPartitionsSpec(null, null), - Collections.singletonMap("test", "map") + Collections.singletonMap("test", "map"), + Collections.singletonMap("test2", "map2") ); final DataSegment segment1 = DataSegment.builder() .dataSource("foo") diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/AbstractBatchIndexTask.java b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/AbstractBatchIndexTask.java index f9c725a294ba..362e02da3aec 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/AbstractBatchIndexTask.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/AbstractBatchIndexTask.java @@ -489,12 +489,14 @@ public static boolean isGuaranteedRollup(IndexIOConfig ioConfig, IndexTuningConf public static Function, Set> compactionStateAnnotateFunction( boolean storeCompactionState, TaskToolbox toolbox, - IndexTuningConfig tuningConfig + IndexTuningConfig tuningConfig, + GranularitySpec granularitySpec ) { if (storeCompactionState) { final Map indexSpecMap = tuningConfig.getIndexSpec().asMap(toolbox.getJsonMapper()); - final CompactionState compactionState = new CompactionState(tuningConfig.getPartitionsSpec(), indexSpecMap); + final Map granularitySpecMap = granularitySpec.asMap(toolbox.getJsonMapper()); + final CompactionState compactionState = new CompactionState(tuningConfig.getPartitionsSpec(), indexSpecMap, granularitySpecMap); return segments -> segments .stream() .map(s -> s.withLastCompactionState(compactionState)) diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/IndexTask.java b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/IndexTask.java index 78d469957bd9..920461a43313 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/IndexTask.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/IndexTask.java @@ -902,7 +902,8 @@ private TaskStatus generateAndPublishSegments( compactionStateAnnotateFunction( storeCompactionState, toolbox, - ingestionSchema.getTuningConfig() + ingestionSchema.getTuningConfig(), + ingestionSchema.getDataSchema().getGranularitySpec() ); // Probably we can publish atomicUpdateGroup along with segments. diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/ParallelIndexSupervisorTask.java b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/ParallelIndexSupervisorTask.java index c9989e96d40f..7aebbba6dc17 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/ParallelIndexSupervisorTask.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/ParallelIndexSupervisorTask.java @@ -928,7 +928,8 @@ private void publishSegments( final Function, Set> annotateFunction = compactionStateAnnotateFunction( storeCompactionState, toolbox, - ingestionSchema.getTuningConfig() + ingestionSchema.getTuningConfig(), + ingestionSchema.getDataSchema().getGranularitySpec() ); final TransactionalSegmentPublisher publisher = (segmentsToBeOverwritten, segmentsToPublish, commitMetadata) -> toolbox.getTaskActionClient().submit( diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskParallelRunTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskParallelRunTest.java index 7b7005bcbd60..b9b46e42493b 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskParallelRunTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskParallelRunTest.java @@ -146,7 +146,8 @@ public void testRunParallelWithDynamicPartitioningMatchCompactionState() final Set compactedSegments = runTask(compactionTask); final CompactionState expectedState = new CompactionState( new DynamicPartitionsSpec(null, Long.MAX_VALUE), - compactionTask.getTuningConfig().getIndexSpec().asMap(getObjectMapper()) + compactionTask.getTuningConfig().getIndexSpec().asMap(getObjectMapper()), + ImmutableMap.of() ); for (DataSegment segment : compactedSegments) { Assert.assertSame( @@ -178,7 +179,8 @@ public void testRunParallelWithHashPartitioningMatchCompactionState() final Set compactedSegments = runTask(compactionTask); final CompactionState expectedState = new CompactionState( new HashedPartitionsSpec(null, 3, null), - compactionTask.getTuningConfig().getIndexSpec().asMap(getObjectMapper()) + compactionTask.getTuningConfig().getIndexSpec().asMap(getObjectMapper()), + ImmutableMap.of() ); for (DataSegment segment : compactedSegments) { // Expect compaction state to exist as store compaction state by default @@ -207,7 +209,8 @@ public void testRunParallelWithRangePartitioning() final Set compactedSegments = runTask(compactionTask); final CompactionState expectedState = new CompactionState( new SingleDimensionPartitionsSpec(7, null, "dim", false), - compactionTask.getTuningConfig().getIndexSpec().asMap(getObjectMapper()) + compactionTask.getTuningConfig().getIndexSpec().asMap(getObjectMapper()), + ImmutableMap.of() ); for (DataSegment segment : compactedSegments) { // Expect compaction state to exist as store compaction state by default @@ -236,7 +239,8 @@ public void testRunParallelWithRangePartitioningWithSingleTask() final Set compactedSegments = runTask(compactionTask); final CompactionState expectedState = new CompactionState( new SingleDimensionPartitionsSpec(7, null, "dim", false), - compactionTask.getTuningConfig().getIndexSpec().asMap(getObjectMapper()) + compactionTask.getTuningConfig().getIndexSpec().asMap(getObjectMapper()), + ImmutableMap.of() ); for (DataSegment segment : compactedSegments) { // Expect compaction state to exist as store compaction state by default diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskRunTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskRunTest.java index b466f3546c47..f1f8d33d1636 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskRunTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskRunTest.java @@ -23,6 +23,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.jsontype.NamedType; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Ordering; import com.google.common.io.Files; import org.apache.druid.client.coordinator.CoordinatorClient; @@ -193,7 +194,8 @@ public static void setupClass() throws JsonProcessingException DEFAULT_COMPACTION_STATE = new CompactionState( new DynamicPartitionsSpec(5000000, Long.MAX_VALUE), - mapper.readValue(mapper.writeValueAsString(new IndexSpec()), Map.class) + mapper.readValue(mapper.writeValueAsString(new IndexSpec()), Map.class), + ImmutableMap.of() ); } @@ -312,7 +314,8 @@ public void testRunWithHashPartitioning() throws Exception Assert.assertEquals(6, segments.size()); final CompactionState expectedState = new CompactionState( new HashedPartitionsSpec(null, 3, null), - compactionTask.getTuningConfig().getIndexSpec().asMap(getObjectMapper()) + compactionTask.getTuningConfig().getIndexSpec().asMap(getObjectMapper()), + ImmutableMap.of() ); for (int i = 0; i < 3; i++) { diff --git a/server/src/main/java/org/apache/druid/segment/indexing/granularity/ArbitraryGranularitySpec.java b/server/src/main/java/org/apache/druid/segment/indexing/granularity/ArbitraryGranularitySpec.java index b2e18318b62c..9378e7077b51 100644 --- a/server/src/main/java/org/apache/druid/segment/indexing/granularity/ArbitraryGranularitySpec.java +++ b/server/src/main/java/org/apache/druid/segment/indexing/granularity/ArbitraryGranularitySpec.java @@ -21,6 +21,8 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.base.Optional; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterators; @@ -36,6 +38,7 @@ import javax.annotation.Nullable; import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.SortedSet; import java.util.TreeSet; @@ -180,4 +183,13 @@ public GranularitySpec withIntervals(List inputIntervals) { return new ArbitraryGranularitySpec(queryGranularity, rollup, inputIntervals); } + + @Override + public Map asMap(ObjectMapper objectMapper) + { + return objectMapper.convertValue( + this, + new TypeReference>() {} + ); + } } diff --git a/server/src/main/java/org/apache/druid/segment/indexing/granularity/GranularitySpec.java b/server/src/main/java/org/apache/druid/segment/indexing/granularity/GranularitySpec.java index 9272f6961e49..697c9b0cff01 100644 --- a/server/src/main/java/org/apache/druid/segment/indexing/granularity/GranularitySpec.java +++ b/server/src/main/java/org/apache/druid/segment/indexing/granularity/GranularitySpec.java @@ -21,12 +21,14 @@ import com.fasterxml.jackson.annotation.JsonSubTypes; import com.fasterxml.jackson.annotation.JsonTypeInfo; +import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.base.Optional; import org.apache.druid.java.util.common.granularity.Granularity; import org.joda.time.DateTime; import org.joda.time.Interval; import java.util.List; +import java.util.Map; import java.util.SortedSet; /** @@ -70,4 +72,6 @@ public interface GranularitySpec Granularity getQueryGranularity(); GranularitySpec withIntervals(List inputIntervals); + + Map asMap(ObjectMapper objectMapper); } diff --git a/server/src/main/java/org/apache/druid/segment/indexing/granularity/UniformGranularitySpec.java b/server/src/main/java/org/apache/druid/segment/indexing/granularity/UniformGranularitySpec.java index 3fe748d0ce00..309013a2fa0e 100644 --- a/server/src/main/java/org/apache/druid/segment/indexing/granularity/UniformGranularitySpec.java +++ b/server/src/main/java/org/apache/druid/segment/indexing/granularity/UniformGranularitySpec.java @@ -21,6 +21,8 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.base.Optional; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; @@ -31,6 +33,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.SortedSet; public class UniformGranularitySpec implements GranularitySpec @@ -189,4 +192,13 @@ public GranularitySpec withIntervals(List inputIntervals) { return new UniformGranularitySpec(segmentGranularity, queryGranularity, rollup, inputIntervals); } + + @Override + public Map asMap(ObjectMapper objectMapper) + { + return objectMapper.convertValue( + this, + new TypeReference>() {} + ); + } } diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/CompactSegments.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/CompactSegments.java index bc178e5c749d..6429a73e9801 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/CompactSegments.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/CompactSegments.java @@ -31,6 +31,7 @@ import org.apache.druid.indexer.TaskStatusPlus; import org.apache.druid.indexer.partitions.SingleDimensionPartitionsSpec; import org.apache.druid.java.util.common.ISE; +import org.apache.druid.java.util.common.granularity.Granularity; import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.server.coordinator.AutoCompactionSnapshot; import org.apache.druid.server.coordinator.CompactionStatistics; @@ -124,11 +125,29 @@ public DruidCoordinatorRuntimeParams run(DruidCoordinatorRuntimeParams params) } if (COMPACTION_TASK_TYPE.equals(response.getPayload().getType())) { final ClientCompactionTaskQuery compactionTaskQuery = (ClientCompactionTaskQuery) response.getPayload(); - final Interval interval = compactionTaskQuery.getIoConfig().getInputSpec().getInterval(); - compactionTaskIntervals.computeIfAbsent(status.getDataSource(), k -> new ArrayList<>()).add(interval); numEstimatedNonCompleteCompactionTasks += findMaxNumTaskSlotsUsedByOneCompactionTask( compactionTaskQuery.getTuningConfig() ); + DataSourceCompactionConfig dataSourceCompactionConfig = compactionConfigs.get(status.getDataSource()); + if (dataSourceCompactionConfig != null && dataSourceCompactionConfig.getGranularitySpec() != null) { + Granularity configuredSegmentGranularity = dataSourceCompactionConfig.getGranularitySpec().getSegmentGranularity(); + if (configuredSegmentGranularity != null + && compactionTaskQuery.getGranularitySpec() != null + && !configuredSegmentGranularity.equals(compactionTaskQuery.getGranularitySpec().getSegmentGranularity())) { + // We will cancel active compaction task if segmentGranularity changes and we will need to + // re-compact the interval + LOG.info("Canceled task[%s] as task segmentGranularity is [%s] but compaction config " + + "segmentGranularity is [%s]", + status.getId(), + compactionTaskQuery.getGranularitySpec().getSegmentGranularity(), + configuredSegmentGranularity); + indexingServiceClient.cancelTask(status.getId()); + continue; + } + } + // Skip interval as the current active compaction task is satisfactory + final Interval interval = compactionTaskQuery.getIoConfig().getInputSpec().getInterval(); + compactionTaskIntervals.computeIfAbsent(status.getDataSource(), k -> new ArrayList<>()).add(interval); } else { throw new ISE("task[%s] is not a compactionTask", status.getId()); } diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIterator.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIterator.java index c03c4b4254a4..ae150473e170 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIterator.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIterator.java @@ -28,11 +28,14 @@ import org.apache.druid.indexer.partitions.PartitionsSpec; import org.apache.druid.java.util.common.DateTimes; import org.apache.druid.java.util.common.ISE; +import org.apache.druid.java.util.common.Intervals; import org.apache.druid.java.util.common.JodaUtils; +import org.apache.druid.java.util.common.granularity.Granularity; import org.apache.druid.java.util.common.guava.Comparators; import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.segment.IndexSpec; import org.apache.druid.segment.SegmentUtils; +import org.apache.druid.segment.indexing.granularity.GranularitySpec; import org.apache.druid.server.coordinator.CompactionStatistics; import org.apache.druid.server.coordinator.DataSourceCompactionConfig; import org.apache.druid.timeline.CompactionState; @@ -57,6 +60,7 @@ import java.util.NoSuchElementException; import java.util.Objects; import java.util.PriorityQueue; +import java.util.Set; import java.util.stream.Collectors; /** @@ -93,10 +97,21 @@ public class NewestSegmentFirstIterator implements CompactionSegmentIterator dataSources.forEach((String dataSource, VersionedIntervalTimeline timeline) -> { final DataSourceCompactionConfig config = compactionConfigs.get(dataSource); - + Granularity configuredSegmentGranularity = null; if (config != null && !timeline.isEmpty()) { + if (config.getGranularitySpec() != null && config.getGranularitySpec().getSegmentGranularity() != null) { + configuredSegmentGranularity = config.getGranularitySpec().getSegmentGranularity(); + VersionedIntervalTimeline timelineWithConfiguredSegmentGranularity = new VersionedIntervalTimeline<>(Comparator.naturalOrder()); + Set segments = timeline.findNonOvershadowedObjectsInInterval(Intervals.ETERNITY, Partitions.ONLY_COMPLETE); + for (DataSegment segment : segments) { + for (Interval interval : configuredSegmentGranularity.getIterable(segment.getInterval())) { + timelineWithConfiguredSegmentGranularity.add(interval, segment.getVersion(), segment.getShardSpec().createChunk(segment)); + } + } + timeline = timelineWithConfiguredSegmentGranularity; + } final List searchIntervals = - findInitialSearchInterval(timeline, config.getSkipOffsetFromLatest(), skipIntervals.get(dataSource)); + findInitialSearchInterval(timeline, config.getSkipOffsetFromLatest(), configuredSegmentGranularity, skipIntervals.get(dataSource)); if (!searchIntervals.isEmpty()) { timelineIterators.put(dataSource, new CompactibleTimelineObjectHolderCursor(timeline, searchIntervals)); } @@ -257,10 +272,11 @@ static PartitionsSpec findPartitinosSpecFromConfig(ClientCompactionTaskQueryTuni } } - private boolean needsCompaction(ClientCompactionTaskQueryTuningConfig tuningConfig, SegmentsToCompact candidates) + private boolean needsCompaction(DataSourceCompactionConfig config, SegmentsToCompact candidates) { Preconditions.checkState(!candidates.isEmpty(), "Empty candidates"); - + final ClientCompactionTaskQueryTuningConfig tuningConfig = + ClientCompactionTaskQueryTuningConfig.from(config.getTuningConfig(), config.getMaxRowsPerSegment()); final PartitionsSpec partitionsSpecFromConfig = findPartitinosSpecFromConfig(tuningConfig); final CompactionState lastCompactionState = candidates.segments.get(0).getLastCompactionState(); if (lastCompactionState == null) { @@ -313,6 +329,20 @@ private boolean needsCompaction(ClientCompactionTaskQueryTuningConfig tuningConf ); needsCompaction = true; } + // + + final GranularitySpec segmentGranularitySpec = lastCompactionState.getGranularitySpec() != null ? + objectMapper.convertValue(lastCompactionState.getGranularitySpec(), GranularitySpec.class) : + null; + + if (config.getGranularitySpec() != null && !config.getGranularitySpec().equals(segmentGranularitySpec)) { + log.info( + "Configured granularitySpec[%s] is different from the one[%s] of segments. Needs compaction", + config.getGranularitySpec(), + segmentGranularitySpec + ); + needsCompaction = true; + } return needsCompaction; } @@ -339,7 +369,7 @@ private SegmentsToCompact findSegmentsToCompact( if (!candidates.isEmpty()) { final boolean isCompactibleSize = candidates.getTotalSize() <= inputSegmentSize; final boolean needsCompaction = needsCompaction( - ClientCompactionTaskQueryTuningConfig.from(config.getTuningConfig(), config.getMaxRowsPerSegment()), + config, candidates ); @@ -396,6 +426,7 @@ private void collectSegmentStatistics( private static List findInitialSearchInterval( VersionedIntervalTimeline timeline, Period skipOffset, + Granularity configuredSegmentGranularity, @Nullable List skipIntervals ) { @@ -407,6 +438,7 @@ private static List findInitialSearchInterval( final List fullSkipIntervals = sortAndAddSkipIntervalFromLatest( last.getInterval().getEnd(), skipOffset, + configuredSegmentGranularity, skipIntervals ); @@ -447,19 +479,27 @@ private static List findInitialSearchInterval( static List sortAndAddSkipIntervalFromLatest( DateTime latest, Period skipOffset, + Granularity configuredSegmentGranularity, @Nullable List skipIntervals ) { final List nonNullSkipIntervals = skipIntervals == null ? new ArrayList<>(1) : new ArrayList<>(skipIntervals.size()); + final Interval skipFromLatest; + if (configuredSegmentGranularity != null) { + DateTime skipFromLastest = new DateTime(latest).minus(skipOffset); + DateTime skipOffsetBucketToSegmentGranularity = configuredSegmentGranularity.bucketStart(skipFromLastest); + skipFromLatest = new Interval(skipOffsetBucketToSegmentGranularity, latest); + } else { + skipFromLatest = new Interval(skipOffset, latest); + } if (skipIntervals != null) { final List sortedSkipIntervals = new ArrayList<>(skipIntervals); sortedSkipIntervals.sort(Comparators.intervalsByStartThenEnd()); final List overlapIntervals = new ArrayList<>(); - final Interval skipFromLatest = new Interval(skipOffset, latest); for (Interval interval : sortedSkipIntervals) { if (interval.overlaps(skipFromLatest)) { @@ -476,7 +516,6 @@ static List sortAndAddSkipIntervalFromLatest( nonNullSkipIntervals.add(skipFromLatest); } } else { - final Interval skipFromLatest = new Interval(skipOffset, latest); nonNullSkipIntervals.add(skipFromLatest); } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/CompactSegmentsTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/CompactSegmentsTest.java index 9bd8a347851c..fe85ab84ebe2 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/CompactSegmentsTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/CompactSegmentsTest.java @@ -356,12 +356,12 @@ public void testMakeStatsForDataSourceWithCompactedIntervalBetweenNonCompactedIn DataSegment afterNoon = createSegment(dataSourceName, j, false, k); if (j == 3) { // Make two intervals on this day compacted (two compacted intervals back-to-back) - beforeNoon = beforeNoon.withLastCompactionState(new CompactionState(partitionsSpec, ImmutableMap.of())); - afterNoon = afterNoon.withLastCompactionState(new CompactionState(partitionsSpec, ImmutableMap.of())); + beforeNoon = beforeNoon.withLastCompactionState(new CompactionState(partitionsSpec, ImmutableMap.of(), ImmutableMap.of())); + afterNoon = afterNoon.withLastCompactionState(new CompactionState(partitionsSpec, ImmutableMap.of(), ImmutableMap.of())); } if (j == 1) { // Make one interval on this day compacted - afterNoon = afterNoon.withLastCompactionState(new CompactionState(partitionsSpec, ImmutableMap.of())); + afterNoon = afterNoon.withLastCompactionState(new CompactionState(partitionsSpec, ImmutableMap.of(), ImmutableMap.of())); } segments.add(beforeNoon); segments.add(afterNoon); @@ -985,7 +985,8 @@ private void compactSegments( "lz4", "longEncoding", "longs" - ) + ), + ImmutableMap.of() ), 1, segmentSize diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIteratorTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIteratorTest.java index d2bda07d348a..b32a7b7eb818 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIteratorTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIteratorTest.java @@ -71,6 +71,7 @@ public void testAddSkipIntervalFromLatestAndSort() final List fullSkipIntervals = NewestSegmentFirstIterator.sortAndAddSkipIntervalFromLatest( DateTimes.of("2019-01-01"), new Period(72, 0, 0, 0), + null, ImmutableList.of( Intervals.of("2018-12-30/2018-12-31"), Intervals.of("2018-12-24/2018-12-25") diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/schema/SystemSchemaTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/schema/SystemSchemaTest.java index 4c37c97abb16..cc159f93e612 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/schema/SystemSchemaTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/schema/SystemSchemaTest.java @@ -279,7 +279,8 @@ public Authorizer getAuthorizer(String name) private final CompactionState expectedCompactionState = new CompactionState( new DynamicPartitionsSpec(null, null), - Collections.singletonMap("test", "map") + Collections.singletonMap("test", "map"), + Collections.singletonMap("test2", "map2") ); private final DataSegment publishedCompactedSegment1 = new DataSegment( From 7c6d17a2f25d136633e6deb6c719e0aba31d552f Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Wed, 3 Feb 2021 03:31:26 -0800 Subject: [PATCH 05/31] resolve conflict --- .../druid/server/coordinator/DataSourceCompactionConfig.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/apache/druid/server/coordinator/DataSourceCompactionConfig.java b/server/src/main/java/org/apache/druid/server/coordinator/DataSourceCompactionConfig.java index 676142a329f5..31598d679acc 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/DataSourceCompactionConfig.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/DataSourceCompactionConfig.java @@ -86,7 +86,7 @@ public DataSourceCompactionConfig( granularitySpec.getQueryGranularity().equals(UniformGranularitySpec.DEFAULT_QUERY_GRANULARITY), "Auto compaction granularitySpec only supports default query granularity value"); Preconditions.checkArgument( - granularitySpec.inputIntervals().isEmpty() && !granularitySpec.bucketIntervals().isPresent(), + granularitySpec.inputIntervals().isEmpty(), "Auto compaction granularitySpec does not supports interval value" ); } From 778685144a59b6786c3a957be5347b60e3e97a88 Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Thu, 4 Feb 2021 00:27:39 -0800 Subject: [PATCH 06/31] Support segmentGranularity for auto-compaction --- .../duty/NewestSegmentFirstIterator.java | 63 ++++++++++++++++--- 1 file changed, 54 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIterator.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIterator.java index ae150473e170..17cd9ef4ac56 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIterator.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIterator.java @@ -41,9 +41,13 @@ import org.apache.druid.timeline.CompactionState; import org.apache.druid.timeline.DataSegment; import org.apache.druid.timeline.Partitions; +import org.apache.druid.timeline.SegmentId; import org.apache.druid.timeline.TimelineObjectHolder; import org.apache.druid.timeline.VersionedIntervalTimeline; +import org.apache.druid.timeline.partition.NumberedPartitionChunk; +import org.apache.druid.timeline.partition.NumberedShardSpec; import org.apache.druid.timeline.partition.PartitionChunk; +import org.apache.druid.timeline.partition.ShardSpec; import org.apache.druid.utils.Streams; import org.joda.time.DateTime; import org.joda.time.Interval; @@ -54,6 +58,7 @@ import java.util.Collections; import java.util.Comparator; import java.util.HashMap; +import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -79,6 +84,7 @@ public class NewestSegmentFirstIterator implements CompactionSegmentIterator // searchIntervals keeps track of the current state of which interval should be considered to search segments to // compact. private final Map timelineIterators; + private final Map> originalShardSpecDatasourceMap = new HashMap<>(); private final PriorityQueue queue = new PriorityQueue<>( (o1, o2) -> Comparators.intervalsByStartThenEnd().compare(o2.interval, o1.interval) @@ -100,12 +106,42 @@ public class NewestSegmentFirstIterator implements CompactionSegmentIterator Granularity configuredSegmentGranularity = null; if (config != null && !timeline.isEmpty()) { if (config.getGranularitySpec() != null && config.getGranularitySpec().getSegmentGranularity() != null) { + Map> IntervalToPartitionMap = new HashMap<>(); configuredSegmentGranularity = config.getGranularitySpec().getSegmentGranularity(); + // Create a new timeline to hold segments in the new configured segment granularity VersionedIntervalTimeline timelineWithConfiguredSegmentGranularity = new VersionedIntervalTimeline<>(Comparator.naturalOrder()); Set segments = timeline.findNonOvershadowedObjectsInInterval(Intervals.ETERNITY, Partitions.ONLY_COMPLETE); for (DataSegment segment : segments) { + // Convert original segmentGranularity to new granularities bucket by configuredSegmentGranularity + // For example, if the original is interval of 2020-01-28/2020-02-03 with WEEK granularity + // and the configuredSegmentGranularity is MONTH, the segment will be split to two segments + // of 2020-01/2020-02 and 2020-02/2020-03. for (Interval interval : configuredSegmentGranularity.getIterable(segment.getInterval())) { - timelineWithConfiguredSegmentGranularity.add(interval, segment.getVersion(), segment.getShardSpec().createChunk(segment)); + IntervalToPartitionMap.computeIfAbsent(interval, k -> new HashSet<>()).add(segment); + } + } + Map originalShardSpecs = originalShardSpecDatasourceMap.computeIfAbsent(dataSource, k -> new HashMap<>()); + for (Map.Entry> partitionsPerInterval : IntervalToPartitionMap.entrySet()) { + Interval interval = partitionsPerInterval.getKey(); + int partitionNum = 0; + Set segmentSet = partitionsPerInterval.getValue(); + 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 + // However, partition in the new timeline (timelineWithConfiguredSegmentGranularity) can be hold multiple + // partitions of the original timeline (when the new segmentGranularity is larger than the original + // segmentGranularity). Hence, we group all the segments of the original timeline into intervals bucket + // by the new configuredSegmentGranularity. We then convert each segment into a new partition space so that + // there is no duplicate partitionNum across all segments of each new Interval. We will have to save the + // original ShardSpec to convert the segment back when returning from the iterator. + originalShardSpecs.put(segmentsForCompact.getId(), segment.getShardSpec()); + timelineWithConfiguredSegmentGranularity.add( + interval, + segmentsForCompact.getVersion(), + NumberedPartitionChunk.make(partitionNum, partitions, segmentsForCompact) + ); + partitionNum += 1; } } timeline = timelineWithConfiguredSegmentGranularity; @@ -329,17 +365,19 @@ private boolean needsCompaction(DataSourceCompactionConfig config, SegmentsToCom ); needsCompaction = true; } - // - final GranularitySpec segmentGranularitySpec = lastCompactionState.getGranularitySpec() != null ? - objectMapper.convertValue(lastCompactionState.getGranularitySpec(), GranularitySpec.class) : - null; + // Only checks for segmentGranularity as auto compaction currently only supports segmentGranularity + final Granularity segmentGranularity = lastCompactionState.getGranularitySpec() != null ? + objectMapper.convertValue(lastCompactionState.getGranularitySpec(), GranularitySpec.class).getSegmentGranularity() : + null; - if (config.getGranularitySpec() != null && !config.getGranularitySpec().equals(segmentGranularitySpec)) { + if (config.getGranularitySpec() != null && + config.getGranularitySpec().getSegmentGranularity() != null && + !config.getGranularitySpec().getSegmentGranularity().equals(segmentGranularity)) { log.info( "Configured granularitySpec[%s] is different from the one[%s] of segments. Needs compaction", config.getGranularitySpec(), - segmentGranularitySpec + segmentGranularity ); needsCompaction = true; } @@ -364,8 +402,15 @@ private SegmentsToCompact findSegmentsToCompact( final long inputSegmentSize = config.getInputSegmentSizeBytes(); while (compactibleTimelineObjectHolderCursor.hasNext()) { - final SegmentsToCompact candidates = new SegmentsToCompact(compactibleTimelineObjectHolderCursor.next()); - + List segments = compactibleTimelineObjectHolderCursor.next(); + Map originalShardSpec = originalShardSpecDatasourceMap.get(dataSourceName); + // Convert segment back to original ShardSpec if the datasource has configuredSegmentGranularity set + if (originalShardSpec != null) { + segments = segments.stream() + .map(segment -> segment.withShardSpec(originalShardSpec.get(segment.getId()))) + .collect(Collectors.toList()); + } + final SegmentsToCompact candidates = new SegmentsToCompact(segments); if (!candidates.isEmpty()) { final boolean isCompactibleSize = candidates.getTotalSize() <= inputSegmentSize; final boolean needsCompaction = needsCompaction( From ac1290300188f4fb4b7910ac22f16a8b79a3154e Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Thu, 4 Feb 2021 00:28:53 -0800 Subject: [PATCH 07/31] Support segmentGranularity for auto-compaction --- .../server/coordinator/duty/NewestSegmentFirstIterator.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIterator.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIterator.java index 17cd9ef4ac56..bcaf2fbb49f0 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIterator.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIterator.java @@ -106,7 +106,7 @@ public class NewestSegmentFirstIterator implements CompactionSegmentIterator Granularity configuredSegmentGranularity = null; if (config != null && !timeline.isEmpty()) { if (config.getGranularitySpec() != null && config.getGranularitySpec().getSegmentGranularity() != null) { - Map> IntervalToPartitionMap = new HashMap<>(); + Map> intervalToPartitionMap = new HashMap<>(); configuredSegmentGranularity = config.getGranularitySpec().getSegmentGranularity(); // Create a new timeline to hold segments in the new configured segment granularity VersionedIntervalTimeline timelineWithConfiguredSegmentGranularity = new VersionedIntervalTimeline<>(Comparator.naturalOrder()); @@ -117,11 +117,11 @@ public class NewestSegmentFirstIterator implements CompactionSegmentIterator // and the configuredSegmentGranularity is MONTH, the segment will be split to two segments // of 2020-01/2020-02 and 2020-02/2020-03. for (Interval interval : configuredSegmentGranularity.getIterable(segment.getInterval())) { - IntervalToPartitionMap.computeIfAbsent(interval, k -> new HashSet<>()).add(segment); + intervalToPartitionMap.computeIfAbsent(interval, k -> new HashSet<>()).add(segment); } } Map originalShardSpecs = originalShardSpecDatasourceMap.computeIfAbsent(dataSource, k -> new HashMap<>()); - for (Map.Entry> partitionsPerInterval : IntervalToPartitionMap.entrySet()) { + for (Map.Entry> partitionsPerInterval : intervalToPartitionMap.entrySet()) { Interval interval = partitionsPerInterval.getKey(); int partitionNum = 0; Set segmentSet = partitionsPerInterval.getValue(); From 4994ca7780b0d74e1a417d966505151845781c93 Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Thu, 4 Feb 2021 02:28:40 -0800 Subject: [PATCH 08/31] fix tests --- .../indexing/HttpIndexingServiceClient.java | 4 ++-- .../coordinator/duty/CompactSegments.java | 17 ++++++++++++----- .../duty/NewestSegmentFirstIterator.java | 2 +- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/apache/druid/client/indexing/HttpIndexingServiceClient.java b/server/src/main/java/org/apache/druid/client/indexing/HttpIndexingServiceClient.java index 546438d668a9..321831feb7cb 100644 --- a/server/src/main/java/org/apache/druid/client/indexing/HttpIndexingServiceClient.java +++ b/server/src/main/java/org/apache/druid/client/indexing/HttpIndexingServiceClient.java @@ -78,8 +78,8 @@ public String compactSegments( String idPrefix, List segments, int compactionTaskPriority, - ClientCompactionTaskQueryTuningConfig tuningConfig, - ClientCompactionTaskQueryGranularitySpec granularitySpec, + @Nullable ClientCompactionTaskQueryTuningConfig tuningConfig, + @Nullable ClientCompactionTaskQueryGranularitySpec granularitySpec, @Nullable Map context ) { diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/CompactSegments.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/CompactSegments.java index 6429a73e9801..c486e87a415e 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/CompactSegments.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/CompactSegments.java @@ -309,17 +309,24 @@ private CoordinatorStats doRun( snapshotBuilder.incrementSegmentCountCompacted(segmentsToCompact.size()); final DataSourceCompactionConfig config = compactionConfigs.get(dataSourceName); + ClientCompactionTaskQueryGranularitySpec queryGranularitySpec; + if (config.getGranularitySpec() != null) { + queryGranularitySpec = new ClientCompactionTaskQueryGranularitySpec( + config.getGranularitySpec().getSegmentGranularity(), + config.getGranularitySpec().getQueryGranularity(), + config.getGranularitySpec().isRollup() + ); + } else { + queryGranularitySpec = null; + } + // make tuningConfig final String taskId = indexingServiceClient.compactSegments( "coordinator-issued", segmentsToCompact, config.getTaskPriority(), ClientCompactionTaskQueryTuningConfig.from(config.getTuningConfig(), config.getMaxRowsPerSegment()), - new ClientCompactionTaskQueryGranularitySpec( - config.getGranularitySpec().getSegmentGranularity(), - config.getGranularitySpec().getQueryGranularity(), - config.getGranularitySpec().isRollup() - ), + queryGranularitySpec, newAutoCompactionContext(config.getTaskContext()) ); diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIterator.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIterator.java index bcaf2fbb49f0..807e42a94f16 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIterator.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIterator.java @@ -533,7 +533,7 @@ static List sortAndAddSkipIntervalFromLatest( : new ArrayList<>(skipIntervals.size()); final Interval skipFromLatest; if (configuredSegmentGranularity != null) { - DateTime skipFromLastest = new DateTime(latest).minus(skipOffset); + DateTime skipFromLastest = new DateTime(latest, latest.getZone()).minus(skipOffset); DateTime skipOffsetBucketToSegmentGranularity = configuredSegmentGranularity.bucketStart(skipFromLastest); skipFromLatest = new Interval(skipOffsetBucketToSegmentGranularity, latest); } else { From 46e34161c4f9e6f419e0c2f66e68dd84591189d6 Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Thu, 4 Feb 2021 03:38:12 -0800 Subject: [PATCH 09/31] fix more tests --- .../timeline/partition/PartitionChunk.java | 2 +- .../task/CompactionTaskParallelRunTest.java | 88 ++++++++++++++----- .../common/task/CompactionTaskRunTest.java | 80 ++++++++++++----- 3 files changed, 125 insertions(+), 45 deletions(-) diff --git a/core/src/main/java/org/apache/druid/timeline/partition/PartitionChunk.java b/core/src/main/java/org/apache/druid/timeline/partition/PartitionChunk.java index 10b43b811794..54aaf03406b8 100644 --- a/core/src/main/java/org/apache/druid/timeline/partition/PartitionChunk.java +++ b/core/src/main/java/org/apache/druid/timeline/partition/PartitionChunk.java @@ -58,7 +58,7 @@ public interface PartitionChunk extends Comparable> * Returns true if this chunk is the end of the partition. Most commonly, that means it represents the range * [X, infinity] for some concrete X. * - * @return true if the chunk is the beginning of the partition + * @return true if the chunk is the end of the partition */ boolean isEnd(); diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskParallelRunTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskParallelRunTest.java index b9b46e42493b..7fe878121c4e 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskParallelRunTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskParallelRunTest.java @@ -129,7 +129,7 @@ public void setup() throws IOException } @Test - public void testRunParallelWithDynamicPartitioningMatchCompactionState() + public void testRunParallelWithDynamicPartitioningMatchCompactionState() throws Exception { runIndexTask(null, true); @@ -144,23 +144,33 @@ public void testRunParallelWithDynamicPartitioningMatchCompactionState() .build(); final Set compactedSegments = runTask(compactionTask); - final CompactionState expectedState = new CompactionState( - new DynamicPartitionsSpec(null, Long.MAX_VALUE), - compactionTask.getTuningConfig().getIndexSpec().asMap(getObjectMapper()), - ImmutableMap.of() - ); for (DataSegment segment : compactedSegments) { Assert.assertSame( lockGranularity == LockGranularity.TIME_CHUNK ? NumberedShardSpec.class : NumberedOverwriteShardSpec.class, segment.getShardSpec().getClass() ); // Expect compaction state to exist as store compaction state by default + CompactionState expectedState = new CompactionState( + new DynamicPartitionsSpec(null, Long.MAX_VALUE), + compactionTask.getTuningConfig().getIndexSpec().asMap(getObjectMapper()), + getObjectMapper().readValue( + getObjectMapper().writeValueAsString( + new UniformGranularitySpec( + Granularities.HOUR, + Granularities.NONE, + true, + ImmutableList.of(segment.getInterval()) + ) + ), + Map.class + ) + ); Assert.assertEquals(expectedState, segment.getLastCompactionState()); } } @Test - public void testRunParallelWithHashPartitioningMatchCompactionState() + public void testRunParallelWithHashPartitioningMatchCompactionState() throws Exception { // Hash partitioning is not supported with segment lock yet Assume.assumeFalse(lockGranularity == LockGranularity.SEGMENT); @@ -177,20 +187,30 @@ public void testRunParallelWithHashPartitioningMatchCompactionState() .build(); final Set compactedSegments = runTask(compactionTask); - final CompactionState expectedState = new CompactionState( - new HashedPartitionsSpec(null, 3, null), - compactionTask.getTuningConfig().getIndexSpec().asMap(getObjectMapper()), - ImmutableMap.of() - ); for (DataSegment segment : compactedSegments) { // Expect compaction state to exist as store compaction state by default Assert.assertSame(HashBasedNumberedShardSpec.class, segment.getShardSpec().getClass()); + CompactionState expectedState = new CompactionState( + new HashedPartitionsSpec(null, 3, null), + compactionTask.getTuningConfig().getIndexSpec().asMap(getObjectMapper()), + getObjectMapper().readValue( + getObjectMapper().writeValueAsString( + new UniformGranularitySpec( + Granularities.HOUR, + Granularities.NONE, + true, + ImmutableList.of(segment.getInterval()) + ) + ), + Map.class + ) + ); Assert.assertEquals(expectedState, segment.getLastCompactionState()); } } @Test - public void testRunParallelWithRangePartitioning() + public void testRunParallelWithRangePartitioning() throws Exception { // Range partitioning is not supported with segment lock yet Assume.assumeFalse(lockGranularity == LockGranularity.SEGMENT); @@ -207,20 +227,30 @@ public void testRunParallelWithRangePartitioning() .build(); final Set compactedSegments = runTask(compactionTask); - final CompactionState expectedState = new CompactionState( - new SingleDimensionPartitionsSpec(7, null, "dim", false), - compactionTask.getTuningConfig().getIndexSpec().asMap(getObjectMapper()), - ImmutableMap.of() - ); for (DataSegment segment : compactedSegments) { // Expect compaction state to exist as store compaction state by default Assert.assertSame(SingleDimensionShardSpec.class, segment.getShardSpec().getClass()); + CompactionState expectedState = new CompactionState( + new SingleDimensionPartitionsSpec(7, null, "dim", false), + compactionTask.getTuningConfig().getIndexSpec().asMap(getObjectMapper()), + getObjectMapper().readValue( + getObjectMapper().writeValueAsString( + new UniformGranularitySpec( + Granularities.HOUR, + Granularities.NONE, + true, + ImmutableList.of(segment.getInterval()) + ) + ), + Map.class + ) + ); Assert.assertEquals(expectedState, segment.getLastCompactionState()); } } @Test - public void testRunParallelWithRangePartitioningWithSingleTask() + public void testRunParallelWithRangePartitioningWithSingleTask() throws Exception { // Range partitioning is not supported with segment lock yet Assume.assumeFalse(lockGranularity == LockGranularity.SEGMENT); @@ -237,14 +267,24 @@ public void testRunParallelWithRangePartitioningWithSingleTask() .build(); final Set compactedSegments = runTask(compactionTask); - final CompactionState expectedState = new CompactionState( - new SingleDimensionPartitionsSpec(7, null, "dim", false), - compactionTask.getTuningConfig().getIndexSpec().asMap(getObjectMapper()), - ImmutableMap.of() - ); for (DataSegment segment : compactedSegments) { // Expect compaction state to exist as store compaction state by default Assert.assertSame(SingleDimensionShardSpec.class, segment.getShardSpec().getClass()); + CompactionState expectedState = new CompactionState( + new SingleDimensionPartitionsSpec(7, null, "dim", false), + compactionTask.getTuningConfig().getIndexSpec().asMap(getObjectMapper()), + getObjectMapper().readValue( + getObjectMapper().writeValueAsString( + new UniformGranularitySpec( + Granularities.HOUR, + Granularities.NONE, + true, + ImmutableList.of(segment.getInterval()) + ) + ), + Map.class + ) + ); Assert.assertEquals(expectedState, segment.getLastCompactionState()); } } diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskRunTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskRunTest.java index 30a303777731..48292479eed6 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskRunTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskRunTest.java @@ -55,6 +55,7 @@ import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.java.util.common.concurrent.Execs; import org.apache.druid.java.util.common.granularity.Granularities; +import org.apache.druid.java.util.common.granularity.Granularity; import org.apache.druid.java.util.common.guava.Sequence; import org.apache.druid.query.aggregation.AggregatorFactory; import org.apache.druid.query.aggregation.LongSumAggregatorFactory; @@ -132,9 +133,6 @@ public class CompactionTaskRunTest extends IngestionTestBase 0 ); - // Expecte compaction state to exist after compaction as we store compaction state by default - private static CompactionState DEFAULT_COMPACTION_STATE; - private static final List TEST_ROWS = ImmutableList.of( "2014-01-01T00:00:10Z,a,1\n", "2014-01-01T00:00:10Z,b,2\n", @@ -187,15 +185,26 @@ public Collection fetchUsedSegmentsInDataSourceForIntervals( this.lockGranularity = lockGranularity; } - @BeforeClass - public static void setupClass() throws JsonProcessingException + public static CompactionState getDefaultCompactionState(Granularity segmentGranularity, + Granularity queryGranularity, + List intervals) throws JsonProcessingException { ObjectMapper mapper = new DefaultObjectMapper(); - - DEFAULT_COMPACTION_STATE = new CompactionState( + // Expected compaction state to exist after compaction as we store compaction state by default + return new CompactionState( new DynamicPartitionsSpec(5000000, Long.MAX_VALUE), mapper.readValue(mapper.writeValueAsString(new IndexSpec()), Map.class), - ImmutableMap.of() + mapper.readValue( + mapper.writeValueAsString( + new UniformGranularitySpec( + segmentGranularity, + queryGranularity, + true, + intervals + ) + ), + Map.class + ) ); } @@ -240,7 +249,10 @@ public void testRunWithDynamicPartitioning() throws Exception Intervals.of("2014-01-01T0%d:00:00/2014-01-01T0%d:00:00", i, i + 1), segments.get(i).getInterval() ); - Assert.assertEquals(DEFAULT_COMPACTION_STATE, segments.get(i).getLastCompactionState()); + Assert.assertEquals( + getDefaultCompactionState(Granularities.HOUR, Granularities.NONE, ImmutableList.of(Intervals.of("2014-01-01T0%d:00:00/2014-01-01T0%d:00:00", i, i + 1))), + segments.get(i).getLastCompactionState() + ); if (lockGranularity == LockGranularity.SEGMENT) { Assert.assertEquals( new NumberedOverwriteShardSpec(32768, 0, 2, (short) 1, (short) 1), @@ -313,11 +325,6 @@ public void testRunWithHashPartitioning() throws Exception final List segments = resultPair.rhs; Assert.assertEquals(6, segments.size()); - final CompactionState expectedState = new CompactionState( - new HashedPartitionsSpec(null, 3, null), - compactionTask.getTuningConfig().getIndexSpec().asMap(getObjectMapper()), - ImmutableMap.of() - ); for (int i = 0; i < 3; i++) { final Interval interval = Intervals.of("2014-01-01T0%d:00:00/2014-01-01T0%d:00:00", i, i + 1); @@ -327,6 +334,21 @@ public void testRunWithHashPartitioning() throws Exception interval, segments.get(segmentIdx).getInterval() ); + CompactionState expectedState = new CompactionState( + new HashedPartitionsSpec(null, 3, null), + compactionTask.getTuningConfig().getIndexSpec().asMap(getObjectMapper()), + getObjectMapper().readValue( + getObjectMapper().writeValueAsString( + new UniformGranularitySpec( + Granularities.HOUR, + Granularities.NONE, + true, + ImmutableList.of(Intervals.of("2014-01-01T0%d:00:00/2014-01-01T0%d:00:00", i, i + 1)) + ) + ), + Map.class + ) + ); Assert.assertEquals(expectedState, segments.get(segmentIdx).getLastCompactionState()); Assert.assertSame(HashBasedNumberedShardSpec.class, segments.get(segmentIdx).getShardSpec().getClass()); } @@ -364,7 +386,10 @@ public void testRunCompactionTwice() throws Exception Intervals.of("2014-01-01T0%d:00:00/2014-01-01T0%d:00:00", i, i + 1), segments.get(i).getInterval() ); - Assert.assertEquals(DEFAULT_COMPACTION_STATE, segments.get(i).getLastCompactionState()); + Assert.assertEquals( + getDefaultCompactionState(Granularities.HOUR, Granularities.NONE, ImmutableList.of(Intervals.of("2014-01-01T0%d:00:00/2014-01-01T0%d:00:00", i, i + 1))), + segments.get(i).getLastCompactionState() + ); if (lockGranularity == LockGranularity.SEGMENT) { Assert.assertEquals( new NumberedOverwriteShardSpec(PartitionIds.NON_ROOT_GEN_START_PARTITION_ID, 0, 2, (short) 1, (short) 1), @@ -391,7 +416,10 @@ public void testRunCompactionTwice() throws Exception Intervals.of("2014-01-01T0%d:00:00/2014-01-01T0%d:00:00", i, i + 1), segments.get(i).getInterval() ); - Assert.assertEquals(DEFAULT_COMPACTION_STATE, segments.get(i).getLastCompactionState()); + Assert.assertEquals( + getDefaultCompactionState(Granularities.HOUR, Granularities.NONE, ImmutableList.of(Intervals.of("2014-01-01T0%d:00:00/2014-01-01T0%d:00:00", i, i + 1))), + segments.get(i).getLastCompactionState() + ); if (lockGranularity == LockGranularity.SEGMENT) { Assert.assertEquals( new NumberedOverwriteShardSpec( @@ -490,7 +518,10 @@ public void testRunIndexAndCompactAtTheSameTimeForDifferentInterval() throws Exc Intervals.of("2014-01-01T0%d:00:00/2014-01-01T0%d:00:00", i, i + 1), segments.get(i).getInterval() ); - Assert.assertEquals(DEFAULT_COMPACTION_STATE, segments.get(i).getLastCompactionState()); + Assert.assertEquals( + getDefaultCompactionState(Granularities.HOUR, Granularities.NONE, ImmutableList.of(Intervals.of("2014-01-01T0%d:00:00/2014-01-01T0%d:00:00", i, i + 1))), + segments.get(i).getLastCompactionState() + ); if (lockGranularity == LockGranularity.SEGMENT) { Assert.assertEquals( new NumberedOverwriteShardSpec(PartitionIds.NON_ROOT_GEN_START_PARTITION_ID, 0, 2, (short) 1, (short) 1), @@ -529,7 +560,10 @@ public void testWithSegmentGranularity() throws Exception Assert.assertEquals(Intervals.of("2014-01-01/2014-01-02"), segments.get(0).getInterval()); Assert.assertEquals(new NumberedShardSpec(0, 1), segments.get(0).getShardSpec()); - Assert.assertEquals(DEFAULT_COMPACTION_STATE, segments.get(0).getLastCompactionState()); + Assert.assertEquals( + getDefaultCompactionState(Granularities.DAY, Granularities.NONE, ImmutableList.of(Intervals.of("2014-01-01T00:00:00/2014-01-01T03:00:00"))), + segments.get(0).getLastCompactionState() + ); // hour segmentGranularity final CompactionTask compactionTask2 = builder @@ -547,7 +581,10 @@ public void testWithSegmentGranularity() throws Exception for (int i = 0; i < 3; i++) { Assert.assertEquals(Intervals.of("2014-01-01T0%d:00:00/2014-01-01T0%d:00:00", i, i + 1), segments.get(i).getInterval()); Assert.assertEquals(new NumberedShardSpec(0, 1), segments.get(i).getShardSpec()); - Assert.assertEquals(DEFAULT_COMPACTION_STATE, segments.get(i).getLastCompactionState()); + Assert.assertEquals( + getDefaultCompactionState(Granularities.HOUR, Granularities.NONE, ImmutableList.of(Intervals.of("2014-01-01/2014-01-02"))), + segments.get(i).getLastCompactionState() + ); } } @@ -784,7 +821,10 @@ public void testRunRegularIndexTaskWithIngestSegmentFirehose() throws Exception Intervals.of("2014-01-01T0%d:00:00/2014-01-01T0%d:00:00", i, i + 1), segments.get(i).getInterval() ); - Assert.assertEquals(DEFAULT_COMPACTION_STATE, segments.get(i).getLastCompactionState()); + Assert.assertEquals( + getDefaultCompactionState(Granularities.HOUR, Granularities.MINUTE, ImmutableList.of()), + segments.get(i).getLastCompactionState() + ); if (lockGranularity == LockGranularity.SEGMENT) { Assert.assertEquals( new NumberedOverwriteShardSpec(32768, 0, 2, (short) 1, (short) 1), From 9a0fb8b720b01b6efbe4358e9689c5cabf798d53 Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Thu, 4 Feb 2021 03:40:03 -0800 Subject: [PATCH 10/31] fix checkstyle --- .../druid/indexing/common/task/CompactionTaskRunTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskRunTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskRunTest.java index 48292479eed6..bd1f8193417d 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskRunTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskRunTest.java @@ -23,7 +23,6 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.jsontype.NamedType; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.Ordering; import com.google.common.io.Files; import org.apache.druid.client.coordinator.CoordinatorClient; @@ -90,7 +89,6 @@ import org.junit.After; import org.junit.Assert; import org.junit.Before; -import org.junit.BeforeClass; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; From 5a221629a56a5e567c3cfe894ab3bc267997b03d Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Fri, 5 Feb 2021 00:52:53 -0800 Subject: [PATCH 11/31] add unit tests --- .../indexing/common/task/CompactionTask.java | 6 +- .../ClientCompactionTaskQuerySerdeTest.java | 21 +- .../common/task/CompactionTaskTest.java | 37 +++ ...entCompactionTaskQueryGranularitySpec.java | 11 +- .../granularity/BaseGranularitySpec.java | 6 +- .../granularity/UniformGranularitySpec.java | 4 - .../DataSourceCompactionConfig.java | 5 +- .../coordinator/duty/CompactSegments.java | 9 +- .../granularity/ArbitraryGranularityTest.java | 27 ++ .../granularity/UniformGranularityTest.java | 33 ++- .../DataSourceCompactionConfigTest.java | 95 +++++++ .../coordinator/duty/CompactSegmentsTest.java | 233 +++++++++++++++++- .../duty/NewestSegmentFirstPolicyTest.java | 201 +++++++++++++-- 13 files changed, 645 insertions(+), 43 deletions(-) diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java index e0f29d3e60dd..5d971c3ebfb2 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java @@ -208,8 +208,8 @@ public CompactionTask( if (granularitySpec == null && segmentGranularity != null) { this.granularitySpec = new UniformGranularitySpec( segmentGranularity, - UniformGranularitySpec.DEFAULT_QUERY_GRANULARITY, - UniformGranularitySpec.DEFAULT_ROLLUP, + null, + null, null ); } else { @@ -368,7 +368,7 @@ public TaskStatus runTask(TaskToolbox toolbox) throws Exception partitionConfigurationManager, dimensionsSpec, metricsSpec, - granularitySpec == null ? null : granularitySpec.getSegmentGranularity(), + getSegmentGranularity(), toolbox.getCoordinatorClient(), segmentLoaderFactory, retryPolicyFactory diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/ClientCompactionTaskQuerySerdeTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/ClientCompactionTaskQuerySerdeTest.java index 11735691d5ef..c3419452cee8 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/ClientCompactionTaskQuerySerdeTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/ClientCompactionTaskQuerySerdeTest.java @@ -28,6 +28,7 @@ import org.apache.druid.client.indexing.ClientCompactionIOConfig; import org.apache.druid.client.indexing.ClientCompactionIntervalSpec; import org.apache.druid.client.indexing.ClientCompactionTaskQuery; +import org.apache.druid.client.indexing.ClientCompactionTaskQueryGranularitySpec; import org.apache.druid.client.indexing.ClientCompactionTaskQueryTuningConfig; import org.apache.druid.client.indexing.ClientTaskQuery; import org.apache.druid.client.indexing.IndexingServiceClient; @@ -45,11 +46,14 @@ import org.apache.druid.jackson.DefaultObjectMapper; import org.apache.druid.java.util.common.HumanReadableBytes; import org.apache.druid.java.util.common.Intervals; +import org.apache.druid.java.util.common.granularity.Granularities; +import org.apache.druid.java.util.common.granularity.Granularity; import org.apache.druid.segment.IndexSpec; import org.apache.druid.segment.data.BitmapSerde.DefaultBitmapSerdeFactory; import org.apache.druid.segment.data.CompressionFactory.LongEncodingStrategy; import org.apache.druid.segment.data.CompressionStrategy; import org.apache.druid.segment.incremental.RowIngestionMetersFactory; +import org.apache.druid.segment.indexing.granularity.UniformGranularitySpec; import org.apache.druid.segment.realtime.appenderator.AppenderatorsManager; import org.apache.druid.segment.realtime.firehose.ChatHandlerProvider; import org.apache.druid.segment.realtime.firehose.NoopChatHandlerProvider; @@ -113,7 +117,7 @@ public void testClientCompactionTaskQueryToCompactionTask() throws IOException 1000, 100 ), - null, + new ClientCompactionTaskQueryGranularitySpec(Granularities.DAY, Granularities.HOUR, true), ImmutableMap.of("key", "value") ); @@ -187,6 +191,18 @@ public void testClientCompactionTaskQueryToCompactionTask() throws IOException query.getTuningConfig().getTotalNumMergeTasks().intValue(), task.getTuningConfig().getTotalNumMergeTasks() ); + Assert.assertEquals( + query.getGranularitySpec().getQueryGranularity(), + task.getGranularitySpec().getQueryGranularity() + ); + Assert.assertEquals( + query.getGranularitySpec().getSegmentGranularity(), + task.getGranularitySpec().getSegmentGranularity() + ); + Assert.assertEquals( + query.getGranularitySpec().isRollup(), + task.getGranularitySpec().isRollup() + ); Assert.assertEquals(query.getContext(), task.getContext()); } @@ -244,6 +260,7 @@ public void testCompactionTaskToClientCompactionTaskQuery() throws IOException null ) ) + .granularitySpec(new UniformGranularitySpec(Granularities.DAY, Granularities.HOUR, null)) .build(); final ClientCompactionTaskQuery expected = new ClientCompactionTaskQuery( @@ -285,7 +302,7 @@ public void testCompactionTaskToClientCompactionTaskQuery() throws IOException 1000, 100 ), - null, + new ClientCompactionTaskQueryGranularitySpec(Granularities.DAY, Granularities.HOUR, true), new HashMap<>() ); diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskTest.java index 21b7f7397b3a..3512903a3922 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskTest.java @@ -355,6 +355,43 @@ public void setup() segmentLoaderFactory = new SegmentLoaderFactory(testIndexIO, OBJECT_MAPPER); } + @Test + public void testCreateCompactionTaskWithGranularitySpec() + { + final CompactionTask taskCreatedWithSegmentGranularity = new Builder( + DATA_SOURCE, + segmentLoaderFactory, + RETRY_POLICY_FACTORY + ).inputSpec(new CompactionIntervalSpec(COMPACTION_INTERVAL, SegmentUtils.hashIds(SEGMENTS))) + .tuningConfig(createTuningConfig()) + .segmentGranularity(Granularities.HOUR) + .build(); + final CompactionTask taskCreatedWithGranularitySpec = new Builder( + DATA_SOURCE, + segmentLoaderFactory, + RETRY_POLICY_FACTORY + ).inputSpec(new CompactionIntervalSpec(COMPACTION_INTERVAL, SegmentUtils.hashIds(SEGMENTS))) + .tuningConfig(createTuningConfig()) + .granularitySpec(new UniformGranularitySpec(Granularities.HOUR, Granularities.DAY, null)) + .build(); + Assert.assertEquals(taskCreatedWithGranularitySpec.getSegmentGranularity(), taskCreatedWithSegmentGranularity.getSegmentGranularity()); + } + + @Test + public void testCreateCompactionTaskWithGranularitySpecOverrideSegmentGranularity() + { + final CompactionTask taskCreatedWithSegmentGranularity = new Builder( + DATA_SOURCE, + segmentLoaderFactory, + RETRY_POLICY_FACTORY + ).inputSpec(new CompactionIntervalSpec(COMPACTION_INTERVAL, SegmentUtils.hashIds(SEGMENTS))) + .tuningConfig(createTuningConfig()) + .segmentGranularity(Granularities.HOUR) + .granularitySpec(new UniformGranularitySpec(Granularities.MINUTE, Granularities.DAY, null)) + .build(); + Assert.assertEquals(Granularities.MINUTE, taskCreatedWithSegmentGranularity.getSegmentGranularity()); + } + @Test public void testSerdeWithInterval() throws IOException { diff --git a/server/src/main/java/org/apache/druid/client/indexing/ClientCompactionTaskQueryGranularitySpec.java b/server/src/main/java/org/apache/druid/client/indexing/ClientCompactionTaskQueryGranularitySpec.java index 1739301da162..580275a6a0b0 100644 --- a/server/src/main/java/org/apache/druid/client/indexing/ClientCompactionTaskQueryGranularitySpec.java +++ b/server/src/main/java/org/apache/druid/client/indexing/ClientCompactionTaskQueryGranularitySpec.java @@ -22,6 +22,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import org.apache.druid.java.util.common.granularity.Granularity; +import org.apache.druid.segment.indexing.granularity.BaseGranularitySpec; import org.apache.druid.segment.indexing.granularity.UniformGranularitySpec; import java.util.Objects; @@ -30,7 +31,7 @@ public class ClientCompactionTaskQueryGranularitySpec { private final Granularity segmentGranularity; private final Granularity queryGranularity; - private final Boolean rollup; + private final boolean rollup; @JsonCreator public ClientCompactionTaskQueryGranularitySpec( @@ -39,9 +40,9 @@ public ClientCompactionTaskQueryGranularitySpec( @JsonProperty("rollup") Boolean rollup ) { - this.queryGranularity = queryGranularity == null ? UniformGranularitySpec.DEFAULT_QUERY_GRANULARITY : queryGranularity; - this.rollup = rollup == null ? UniformGranularitySpec.DEFAULT_ROLLUP : rollup; - this.segmentGranularity = segmentGranularity == null ? UniformGranularitySpec.DEFAULT_SEGMENT_GRANULARITY : segmentGranularity; + this.queryGranularity = queryGranularity == null ? BaseGranularitySpec.DEFAULT_QUERY_GRANULARITY : queryGranularity; + this.rollup = rollup == null ? BaseGranularitySpec.DEFAULT_ROLLUP : rollup; + this.segmentGranularity = segmentGranularity == null ? BaseGranularitySpec.DEFAULT_SEGMENT_GRANULARITY : segmentGranularity; } @JsonProperty @@ -57,7 +58,7 @@ public Granularity getQueryGranularity() } @JsonProperty - public Boolean getRollup() + public boolean isRollup() { return rollup; } diff --git a/server/src/main/java/org/apache/druid/segment/indexing/granularity/BaseGranularitySpec.java b/server/src/main/java/org/apache/druid/segment/indexing/granularity/BaseGranularitySpec.java index defff1330713..cc617b0a9e70 100644 --- a/server/src/main/java/org/apache/druid/segment/indexing/granularity/BaseGranularitySpec.java +++ b/server/src/main/java/org/apache/druid/segment/indexing/granularity/BaseGranularitySpec.java @@ -24,6 +24,8 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterators; import org.apache.druid.java.util.common.DateTimes; +import org.apache.druid.java.util.common.granularity.Granularities; +import org.apache.druid.java.util.common.granularity.Granularity; import org.apache.druid.java.util.common.guava.Comparators; import org.joda.time.DateTime; import org.joda.time.Interval; @@ -33,9 +35,11 @@ import java.util.List; import java.util.TreeSet; -abstract class BaseGranularitySpec implements GranularitySpec +public abstract class BaseGranularitySpec implements GranularitySpec { public static final Boolean DEFAULT_ROLLUP = Boolean.TRUE; + public static final Granularity DEFAULT_SEGMENT_GRANULARITY = Granularities.DAY; + public static final Granularity DEFAULT_QUERY_GRANULARITY = Granularities.NONE; protected List inputIntervals; protected final Boolean rollup; diff --git a/server/src/main/java/org/apache/druid/segment/indexing/granularity/UniformGranularitySpec.java b/server/src/main/java/org/apache/druid/segment/indexing/granularity/UniformGranularitySpec.java index a52790e5814d..b9d5653b7fb8 100644 --- a/server/src/main/java/org/apache/druid/segment/indexing/granularity/UniformGranularitySpec.java +++ b/server/src/main/java/org/apache/druid/segment/indexing/granularity/UniformGranularitySpec.java @@ -23,7 +23,6 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; -import org.apache.druid.java.util.common.granularity.Granularities; import org.apache.druid.java.util.common.granularity.Granularity; import org.apache.druid.java.util.common.granularity.IntervalsByGranularity; import org.joda.time.Interval; @@ -33,9 +32,6 @@ public class UniformGranularitySpec extends BaseGranularitySpec { - public static final Granularity DEFAULT_SEGMENT_GRANULARITY = Granularities.DAY; - public static final Granularity DEFAULT_QUERY_GRANULARITY = Granularities.NONE; - private final Granularity segmentGranularity; private final Granularity queryGranularity; private final IntervalsByGranularity intervalsByGranularity; diff --git a/server/src/main/java/org/apache/druid/server/coordinator/DataSourceCompactionConfig.java b/server/src/main/java/org/apache/druid/server/coordinator/DataSourceCompactionConfig.java index 31598d679acc..692eb81d23b6 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/DataSourceCompactionConfig.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/DataSourceCompactionConfig.java @@ -22,6 +22,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Preconditions; +import org.apache.druid.segment.indexing.granularity.BaseGranularitySpec; import org.apache.druid.segment.indexing.granularity.GranularitySpec; import org.apache.druid.segment.indexing.granularity.UniformGranularitySpec; import org.joda.time.Period; @@ -79,11 +80,11 @@ public DataSourceCompactionConfig( "Auto compaction granularitySpec only supports uniform type" ); Preconditions.checkArgument( - granularitySpec.isRollup() == UniformGranularitySpec.DEFAULT_ROLLUP, + granularitySpec.isRollup() == BaseGranularitySpec.DEFAULT_ROLLUP, "Auto compaction granularitySpec only supports default rollup value" ); Preconditions.checkArgument( - granularitySpec.getQueryGranularity().equals(UniformGranularitySpec.DEFAULT_QUERY_GRANULARITY), + granularitySpec.getQueryGranularity().equals(BaseGranularitySpec.DEFAULT_QUERY_GRANULARITY), "Auto compaction granularitySpec only supports default query granularity value"); Preconditions.checkArgument( granularitySpec.inputIntervals().isEmpty(), diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/CompactSegments.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/CompactSegments.java index c486e87a415e..dcd7fab8df41 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/CompactSegments.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/CompactSegments.java @@ -125,9 +125,6 @@ public DruidCoordinatorRuntimeParams run(DruidCoordinatorRuntimeParams params) } if (COMPACTION_TASK_TYPE.equals(response.getPayload().getType())) { final ClientCompactionTaskQuery compactionTaskQuery = (ClientCompactionTaskQuery) response.getPayload(); - numEstimatedNonCompleteCompactionTasks += findMaxNumTaskSlotsUsedByOneCompactionTask( - compactionTaskQuery.getTuningConfig() - ); DataSourceCompactionConfig dataSourceCompactionConfig = compactionConfigs.get(status.getDataSource()); if (dataSourceCompactionConfig != null && dataSourceCompactionConfig.getGranularitySpec() != null) { Granularity configuredSegmentGranularity = dataSourceCompactionConfig.getGranularitySpec().getSegmentGranularity(); @@ -145,9 +142,13 @@ public DruidCoordinatorRuntimeParams run(DruidCoordinatorRuntimeParams params) continue; } } - // Skip interval as the current active compaction task is satisfactory + // Skip interval as the current active compaction task is good final Interval interval = compactionTaskQuery.getIoConfig().getInputSpec().getInterval(); compactionTaskIntervals.computeIfAbsent(status.getDataSource(), k -> new ArrayList<>()).add(interval); + // Since we keep the current active compaction task running, we count the active task slots + numEstimatedNonCompleteCompactionTasks += findMaxNumTaskSlotsUsedByOneCompactionTask( + compactionTaskQuery.getTuningConfig() + ); } else { throw new ISE("task[%s] is not a compactionTask", status.getId()); } diff --git a/server/src/test/java/org/apache/druid/segment/indexing/granularity/ArbitraryGranularityTest.java b/server/src/test/java/org/apache/druid/segment/indexing/granularity/ArbitraryGranularityTest.java index a0154228a615..5d3b460d6304 100644 --- a/server/src/test/java/org/apache/druid/segment/indexing/granularity/ArbitraryGranularityTest.java +++ b/server/src/test/java/org/apache/druid/segment/indexing/granularity/ArbitraryGranularityTest.java @@ -32,6 +32,7 @@ import org.junit.Test; import java.util.List; +import java.util.Map; public class ArbitraryGranularityTest { @@ -217,6 +218,32 @@ public void testJson() } } + @Test + public void testAsMap() throws Exception + { + final GranularitySpec spec = new ArbitraryGranularitySpec(Granularities.NONE, Lists.newArrayList( + Intervals.of("2012-01-08T00Z/2012-01-11T00Z"), + Intervals.of("2012-02-01T00Z/2012-03-01T00Z"), + Intervals.of("2012-01-07T00Z/2012-01-08T00Z"), + Intervals.of("2012-01-03T00Z/2012-01-04T00Z"), + Intervals.of("2012-01-01T00Z/2012-01-03T00Z") + )); + + Map map = spec.asMap(JSON_MAPPER); + final GranularitySpec rtSpec = JSON_MAPPER.convertValue(map, GranularitySpec.class); + Assert.assertEquals( + "Round-trip", + ImmutableList.copyOf(spec.sortedBucketIntervals()), + ImmutableList.copyOf(rtSpec.sortedBucketIntervals()) + ); + Assert.assertEquals( + "Round-trip", + ImmutableList.copyOf(spec.inputIntervals()), + ImmutableList.copyOf(rtSpec.inputIntervals()) + ); + Assert.assertEquals(spec, rtSpec); + } + @Test public void testNullInputIntervals() { diff --git a/server/src/test/java/org/apache/druid/segment/indexing/granularity/UniformGranularityTest.java b/server/src/test/java/org/apache/druid/segment/indexing/granularity/UniformGranularityTest.java index 27760f63c187..d39f36b7a8a4 100644 --- a/server/src/test/java/org/apache/druid/segment/indexing/granularity/UniformGranularityTest.java +++ b/server/src/test/java/org/apache/druid/segment/indexing/granularity/UniformGranularityTest.java @@ -38,10 +38,11 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Map; public class UniformGranularityTest { - private static final ObjectMapper JOSN_MAPPER = new DefaultObjectMapper(); + private static final ObjectMapper JSON_MAPPER = new DefaultObjectMapper(); @Test public void testSimple() @@ -161,7 +162,7 @@ public void testJson() ); try { - final GranularitySpec rtSpec = JOSN_MAPPER.readValue(JOSN_MAPPER.writeValueAsString(spec), GranularitySpec.class); + final GranularitySpec rtSpec = JSON_MAPPER.readValue(JSON_MAPPER.writeValueAsString(spec), GranularitySpec.class); Assert.assertEquals( "Round-trip sortedBucketIntervals", ImmutableList.copyOf(spec.sortedBucketIntervals()), @@ -178,6 +179,34 @@ public void testJson() } } + @Test + public void testAsMap() throws Exception + { + final GranularitySpec spec = new UniformGranularitySpec( + Granularities.DAY, + null, + Lists.newArrayList( + Intervals.of("2012-01-08T00Z/2012-01-11T00Z"), + Intervals.of("2012-01-07T00Z/2012-01-08T00Z"), + Intervals.of("2012-01-03T00Z/2012-01-04T00Z"), + Intervals.of("2012-01-01T00Z/2012-01-03T00Z") + ) + ); + Map map = spec.asMap(JSON_MAPPER); + final GranularitySpec rtSpec = JSON_MAPPER.convertValue(map, GranularitySpec.class); + Assert.assertEquals( + "Round-trip sortedBucketIntervals", + ImmutableList.copyOf(spec.sortedBucketIntervals()), + ImmutableList.copyOf(rtSpec.sortedBucketIntervals().iterator()) + ); + Assert.assertEquals( + "Round-trip granularity", + spec.getSegmentGranularity(), + rtSpec.getSegmentGranularity() + ); + Assert.assertEquals(spec, rtSpec); + } + @Test public void testEquals() { diff --git a/server/src/test/java/org/apache/druid/server/coordinator/DataSourceCompactionConfigTest.java b/server/src/test/java/org/apache/druid/server/coordinator/DataSourceCompactionConfigTest.java index c2bb8503d5e2..7401c9a57525 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/DataSourceCompactionConfigTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/DataSourceCompactionConfigTest.java @@ -20,15 +20,21 @@ package org.apache.druid.server.coordinator; import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import jdk.nashorn.internal.ir.annotations.Immutable; import org.apache.druid.data.input.SegmentsSplitHintSpec; import org.apache.druid.indexer.partitions.DynamicPartitionsSpec; import org.apache.druid.jackson.DefaultObjectMapper; import org.apache.druid.java.util.common.HumanReadableBytes; +import org.apache.druid.java.util.common.Intervals; +import org.apache.druid.java.util.common.granularity.Granularities; import org.apache.druid.segment.IndexSpec; import org.apache.druid.segment.data.BitmapSerde.DefaultBitmapSerdeFactory; import org.apache.druid.segment.data.CompressionFactory.LongEncodingStrategy; import org.apache.druid.segment.data.CompressionStrategy; +import org.apache.druid.segment.indexing.granularity.ArbitraryGranularitySpec; +import org.apache.druid.segment.indexing.granularity.UniformGranularitySpec; import org.apache.druid.segment.writeout.TmpFileSegmentWriteOutMediumFactory; import org.joda.time.Duration; import org.joda.time.Period; @@ -69,6 +75,7 @@ public void testSerdeBasic() throws IOException Assert.assertEquals(config.getSkipOffsetFromLatest(), fromJson.getSkipOffsetFromLatest()); Assert.assertEquals(config.getTuningConfig(), fromJson.getTuningConfig()); Assert.assertEquals(config.getTaskContext(), fromJson.getTaskContext()); + Assert.assertEquals(config.getGranularitySpec(), fromJson.getGranularitySpec()); } @Test @@ -221,4 +228,92 @@ public void testSerdeUserCompactionTuningConfig() throws IOException OBJECT_MAPPER.readValue(json, UserCompactionTaskQueryTuningConfig.class); Assert.assertEquals(tuningConfig, fromJson); } + + @Test + public void testSerdeGranularitySpec() throws IOException + { + final DataSourceCompactionConfig config = new DataSourceCompactionConfig( + "dataSource", + null, + 500L, + null, + new Period(3600), + null, + new UniformGranularitySpec(Granularities.HOUR, null, null), + ImmutableMap.of("key", "val") + ); + final String json = OBJECT_MAPPER.writeValueAsString(config); + final DataSourceCompactionConfig fromJson = OBJECT_MAPPER.readValue(json, DataSourceCompactionConfig.class); + + Assert.assertEquals(config.getDataSource(), fromJson.getDataSource()); + Assert.assertEquals(25, fromJson.getTaskPriority()); + Assert.assertEquals(config.getInputSegmentSizeBytes(), fromJson.getInputSegmentSizeBytes()); + Assert.assertEquals(config.getMaxRowsPerSegment(), fromJson.getMaxRowsPerSegment()); + Assert.assertEquals(config.getSkipOffsetFromLatest(), fromJson.getSkipOffsetFromLatest()); + Assert.assertEquals(config.getTuningConfig(), fromJson.getTuningConfig()); + Assert.assertEquals(config.getTaskContext(), fromJson.getTaskContext()); + Assert.assertEquals(config.getGranularitySpec(), fromJson.getGranularitySpec()); + } + + @Test(expected = IllegalArgumentException.class) + public void testFailIfGranularitySpecContainsNonDefaultQueryGranularity() + { + new DataSourceCompactionConfig( + "dataSource", + null, + 500L, + null, + new Period(3600), + null, + new UniformGranularitySpec(Granularities.HOUR, Granularities.MONTH, null), + ImmutableMap.of("key", "val") + ); + } + + @Test(expected = IllegalArgumentException.class) + public void testFailIfGranularitySpecContainsNonDefaultRollup() + { + new DataSourceCompactionConfig( + "dataSource", + null, + 500L, + null, + new Period(3600), + null, + new UniformGranularitySpec(Granularities.HOUR, Granularities.MONTH, false, null), + ImmutableMap.of("key", "val") + ); + } + + @Test(expected = IllegalArgumentException.class) + public void testFailIfGranularitySpecContainsNonEmptyInterval() + { + new DataSourceCompactionConfig( + "dataSource", + null, + 500L, + null, + new Period(3600), + null, + new UniformGranularitySpec(Granularities.HOUR, Granularities.MONTH, ImmutableList.of(Intervals.of("2012-01-08T00Z/2012-01-11T00Z"))), + ImmutableMap.of("key", "val") + ); + } + + @Test(expected = IllegalArgumentException.class) + public void testFailIfGranularitySpecIsNotUniform() + { + new DataSourceCompactionConfig( + "dataSource", + null, + 500L, + null, + new Period(3600), + null, + new ArbitraryGranularitySpec(null, null, null), + ImmutableMap.of("key", "val") + ); + } + + } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/CompactSegmentsTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/CompactSegmentsTest.java index fe85ab84ebe2..bdeb9454831f 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/CompactSegmentsTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/CompactSegmentsTest.java @@ -28,16 +28,25 @@ import junitparams.converters.Nullable; import org.apache.commons.lang3.mutable.MutableInt; import org.apache.druid.client.DataSourcesSnapshot; +import org.apache.druid.client.indexing.ClientCompactionIOConfig; +import org.apache.druid.client.indexing.ClientCompactionIntervalSpec; import org.apache.druid.client.indexing.ClientCompactionTaskQuery; +import org.apache.druid.client.indexing.ClientCompactionTaskQueryGranularitySpec; import org.apache.druid.client.indexing.ClientCompactionTaskQueryTuningConfig; import org.apache.druid.client.indexing.ClientTaskQuery; import org.apache.druid.client.indexing.HttpIndexingServiceClient; import org.apache.druid.client.indexing.IndexingWorker; import org.apache.druid.client.indexing.IndexingWorkerInfo; +import org.apache.druid.client.indexing.TaskPayloadResponse; +import org.apache.druid.client.indexing.TaskStatus; import org.apache.druid.discovery.DruidLeaderClient; import org.apache.druid.discovery.DruidNodeDiscovery; import org.apache.druid.discovery.DruidNodeDiscoveryProvider; import org.apache.druid.discovery.NodeRole; +import org.apache.druid.indexer.RunnerTaskState; +import org.apache.druid.indexer.TaskLocation; +import org.apache.druid.indexer.TaskState; +import org.apache.druid.indexer.TaskStatusPlus; import org.apache.druid.indexer.partitions.DynamicPartitionsSpec; import org.apache.druid.indexer.partitions.HashedPartitionsSpec; import org.apache.druid.indexer.partitions.PartitionsSpec; @@ -47,8 +56,10 @@ import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.Intervals; import org.apache.druid.java.util.common.StringUtils; +import org.apache.druid.java.util.common.granularity.Granularities; import org.apache.druid.java.util.http.client.Request; import org.apache.druid.java.util.http.client.response.StringFullResponseHolder; +import org.apache.druid.segment.indexing.granularity.UniformGranularitySpec; import org.apache.druid.server.DruidNode; import org.apache.druid.server.coordinator.AutoCompactionSnapshot; import org.apache.druid.server.coordinator.CoordinatorCompactionConfig; @@ -59,6 +70,7 @@ import org.apache.druid.server.coordinator.UserCompactionTaskQueryTuningConfig; import org.apache.druid.timeline.CompactionState; import org.apache.druid.timeline.DataSegment; +import org.apache.druid.timeline.Partitions; import org.apache.druid.timeline.TimelineObjectHolder; import org.apache.druid.timeline.VersionedIntervalTimeline; import org.apache.druid.timeline.partition.HashBasedNumberedShardSpec; @@ -81,6 +93,8 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; +import org.mockito.ArgumentCaptor; +import org.mockito.ArgumentMatchers; import org.mockito.Mockito; import java.io.IOException; @@ -89,6 +103,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.function.BiFunction; @@ -101,6 +116,7 @@ public class CompactSegmentsTest { private static final ObjectMapper JSON_MAPPER = new DefaultObjectMapper(); private static final String DATA_SOURCE_PREFIX = "dataSource_"; + private static final int PARTITION_PER_TIME_INTERVAL = 4; // Each dataSource starts with 440 byte, 44 segments, and 11 intervals needing compaction private static final int TOTAL_BYTE_PER_DATASOURCE = 440; private static final int TOTAL_SEGMENT_PER_DATASOURCE = 44; @@ -144,6 +160,7 @@ public static Collection constructorFeeder() private final BiFunction shardSpecFactory; private Map> dataSources; + Map> datasourceToSegments = new HashMap<>(); public CompactSegmentsTest(PartitionsSpec partitionsSpec, BiFunction shardSpecFactory) { @@ -154,18 +171,23 @@ public CompactSegmentsTest(PartitionsSpec partitionsSpec, BiFunction segments = new ArrayList<>(); + List allSegments = new ArrayList<>(); for (int i = 0; i < 3; i++) { final String dataSource = DATA_SOURCE_PREFIX + i; for (int j : new int[]{0, 1, 2, 3, 7, 8}) { - for (int k = 0; k < 4; k++) { - segments.add(createSegment(dataSource, j, true, k)); - segments.add(createSegment(dataSource, j, false, k)); + for (int k = 0; k < PARTITION_PER_TIME_INTERVAL; k++) { + List segmentForDatasource = datasourceToSegments.computeIfAbsent(dataSource, key -> new ArrayList<>()); + DataSegment dataSegment = createSegment(dataSource, j, true, k); + allSegments.add(dataSegment); + segmentForDatasource.add(dataSegment); + dataSegment = createSegment(dataSource, j, false, k); + allSegments.add(dataSegment); + segmentForDatasource.add(dataSegment); } } } dataSources = DataSourcesSnapshot - .fromUsedSegments(segments, ImmutableMap.of()) + .fromUsedSegments(allSegments, ImmutableMap.of()) .getUsedSegmentsTimelinesPerDataSource(); } @@ -351,7 +373,7 @@ public void testMakeStatsForDataSourceWithCompactedIntervalBetweenNonCompactedIn String dataSourceName = DATA_SOURCE_PREFIX + 1; List segments = new ArrayList<>(); for (int j : new int[]{0, 1, 2, 3, 7, 8}) { - for (int k = 0; k < 4; k++) { + for (int k = 0; k < PARTITION_PER_TIME_INTERVAL; k++) { DataSegment beforeNoon = createSegment(dataSourceName, j, true, k); DataSegment afterNoon = createSegment(dataSourceName, j, false, k); if (j == 3) { @@ -538,6 +560,205 @@ public void testRunMultipleCompactionTaskSlots() Assert.assertEquals(3, stats.getGlobalStat(CompactSegments.COMPACTION_TASK_COUNT)); } + @Test + public void testCompactWithoutGranularitySpec() + { + final HttpIndexingServiceClient mockIndexingServiceClient = Mockito.mock(HttpIndexingServiceClient.class); + final CompactSegments compactSegments = new CompactSegments(JSON_MAPPER, mockIndexingServiceClient); + final List compactionConfigs = new ArrayList<>(); + final String dataSource = DATA_SOURCE_PREFIX + 0; + compactionConfigs.add( + new DataSourceCompactionConfig( + dataSource, + 0, + 500L, + null, + new Period("PT0H"), // smaller than segment interval + new UserCompactionTaskQueryTuningConfig( + null, + null, + null, + null, + partitionsSpec, + null, + null, + null, + null, + null, + 3, + null, + null, + null, + null, + null, + null + ), + null, + null + ) + ); + doCompactSegments(compactSegments, compactionConfigs); + ArgumentCaptor> segmentsCaptor = ArgumentCaptor.forClass(List.class); + ArgumentCaptor granularitySpecArgumentCaptor = ArgumentCaptor.forClass(ClientCompactionTaskQueryGranularitySpec.class); + Mockito.verify(mockIndexingServiceClient).compactSegments( + ArgumentMatchers.anyString(), + segmentsCaptor.capture(), + ArgumentMatchers.anyInt(), + ArgumentMatchers.any(), + granularitySpecArgumentCaptor.capture(), + ArgumentMatchers.any() + ); + // Only the same amount of segments as the original PARTITION_PER_TIME_INTERVAL since segment granulartity is the same + Assert.assertEquals(PARTITION_PER_TIME_INTERVAL, segmentsCaptor.getValue().size()); + Assert.assertNull(granularitySpecArgumentCaptor.getValue()); + } + + @Test + public void testCompactWithGranularitySpec() + { + final HttpIndexingServiceClient mockIndexingServiceClient = Mockito.mock(HttpIndexingServiceClient.class); + final CompactSegments compactSegments = new CompactSegments(JSON_MAPPER, mockIndexingServiceClient); + final List compactionConfigs = new ArrayList<>(); + final String dataSource = DATA_SOURCE_PREFIX + 0; + compactionConfigs.add( + new DataSourceCompactionConfig( + dataSource, + 0, + 500L, + null, + new Period("PT0H"), // smaller than segment interval + new UserCompactionTaskQueryTuningConfig( + null, + null, + null, + null, + partitionsSpec, + null, + null, + null, + null, + null, + 3, + null, + null, + null, + null, + null, + null + ), + new UniformGranularitySpec(Granularities.YEAR, null, null), + null + ) + ); + doCompactSegments(compactSegments, compactionConfigs); + ArgumentCaptor> segmentsCaptor = ArgumentCaptor.forClass(List.class); + ArgumentCaptor granularitySpecArgumentCaptor = ArgumentCaptor.forClass(ClientCompactionTaskQueryGranularitySpec.class); + Mockito.verify(mockIndexingServiceClient).compactSegments( + ArgumentMatchers.anyString(), + segmentsCaptor.capture(), + ArgumentMatchers.anyInt(), + ArgumentMatchers.any(), + granularitySpecArgumentCaptor.capture(), + ArgumentMatchers.any() + ); + // All segments is compact at the same time since we changed the segment granularity to YEAR and all segment + // are within the same year + Assert.assertEquals(datasourceToSegments.get(dataSource).size(), segmentsCaptor.getValue().size()); + Assert.assertEquals(Granularities.YEAR, granularitySpecArgumentCaptor.getValue().getSegmentGranularity()); + } + + @Test + public void testCompactWithGranularitySpecConflictWithActiveCompactionTask() + { + final String dataSource = DATA_SOURCE_PREFIX + 0; + final String conflictTaskId = "taskIdDummy"; + final HttpIndexingServiceClient mockIndexingServiceClient = Mockito.mock(HttpIndexingServiceClient.class); + TaskStatusPlus runningConflictCompactionTask = new TaskStatusPlus( + conflictTaskId, + "groupId", + "compact", + DateTimes.EPOCH, + DateTimes.EPOCH, + TaskState.RUNNING, + RunnerTaskState.RUNNING, + -1L, + TaskLocation.unknown(), + dataSource, + null + ); + TaskPayloadResponse runningConflictCompactionTaskPayload = new TaskPayloadResponse( + conflictTaskId, + new ClientCompactionTaskQuery( + conflictTaskId, + dataSource, + new ClientCompactionIOConfig( + new ClientCompactionIntervalSpec( + Intervals.of("2000/2099"), + "testSha256OfSortedSegmentIds" + ) + ), + null, + new ClientCompactionTaskQueryGranularitySpec(Granularities.DAY, null, null), + null + ) + ); + Mockito.when(mockIndexingServiceClient.getActiveTasks()).thenReturn(ImmutableList.of(runningConflictCompactionTask)); + Mockito.when(mockIndexingServiceClient.getTaskPayload(ArgumentMatchers.eq(conflictTaskId))).thenReturn(runningConflictCompactionTaskPayload); + + final CompactSegments compactSegments = new CompactSegments(JSON_MAPPER, mockIndexingServiceClient); + final List compactionConfigs = new ArrayList<>(); + compactionConfigs.add( + new DataSourceCompactionConfig( + dataSource, + 0, + 500L, + null, + new Period("PT0H"), // smaller than segment interval + new UserCompactionTaskQueryTuningConfig( + null, + null, + null, + null, + partitionsSpec, + null, + null, + null, + null, + null, + 3, + null, + null, + null, + null, + null, + null + ), + new UniformGranularitySpec(Granularities.YEAR, null, null), + null + ) + ); + doCompactSegments(compactSegments, compactionConfigs); + // Verify that conflict task was canceled + Mockito.verify(mockIndexingServiceClient).cancelTask(conflictTaskId); + // The active conflict task has interval of 2000/2099 + // Make sure that we do not skip interval of conflict task. + // Since we cancel the task and will have to compact those intervals with the new segmentGranulartity + ArgumentCaptor> segmentsCaptor = ArgumentCaptor.forClass(List.class); + ArgumentCaptor granularitySpecArgumentCaptor = ArgumentCaptor.forClass(ClientCompactionTaskQueryGranularitySpec.class); + Mockito.verify(mockIndexingServiceClient).compactSegments( + ArgumentMatchers.anyString(), + segmentsCaptor.capture(), + ArgumentMatchers.anyInt(), + ArgumentMatchers.any(), + granularitySpecArgumentCaptor.capture(), + ArgumentMatchers.any() + ); + // All segments is compact at the same time since we changed the segment granularity to YEAR and all segment + // are within the same year + Assert.assertEquals(datasourceToSegments.get(dataSource).size(), segmentsCaptor.getValue().size()); + Assert.assertEquals(Granularities.YEAR, granularitySpecArgumentCaptor.getValue().getSegmentGranularity()); + } + @Test public void testRunParallelCompactionMultipleCompactionTaskSlots() { diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstPolicyTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstPolicyTest.java index 92c9f779d09a..bdf0da2b9fea 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstPolicyTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstPolicyTest.java @@ -22,10 +22,17 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; +import org.apache.commons.collections.IteratorUtils; import org.apache.druid.jackson.DefaultObjectMapper; import org.apache.druid.java.util.common.DateTimes; import org.apache.druid.java.util.common.Intervals; +import org.apache.druid.java.util.common.granularity.Granularities; +import org.apache.druid.java.util.common.granularity.Granularity; import org.apache.druid.java.util.common.guava.Comparators; +import org.apache.druid.segment.indexing.granularity.GranularitySpec; +import org.apache.druid.segment.indexing.granularity.UniformGranularitySpec; import org.apache.druid.server.coordinator.DataSourceCompactionConfig; import org.apache.druid.timeline.DataSegment; import org.apache.druid.timeline.Partitions; @@ -42,6 +49,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.Comparator; +import java.util.HashSet; import java.util.List; import java.util.stream.Collectors; @@ -58,7 +66,7 @@ public void testLargeOffsetAndSmallSegmentInterval() { final Period segmentPeriod = new Period("PT1H"); final CompactionSegmentIterator iterator = policy.reset( - ImmutableMap.of(DATA_SOURCE, createCompactionConfig(10000, new Period("P2D"))), + ImmutableMap.of(DATA_SOURCE, createCompactionConfig(10000, new Period("P2D"), null)), ImmutableMap.of( DATA_SOURCE, createTimeline( @@ -83,7 +91,7 @@ public void testSmallOffsetAndLargeSegmentInterval() { final Period segmentPeriod = new Period("PT1H"); final CompactionSegmentIterator iterator = policy.reset( - ImmutableMap.of(DATA_SOURCE, createCompactionConfig(10000, new Period("PT1M"))), + ImmutableMap.of(DATA_SOURCE, createCompactionConfig(10000, new Period("PT1M"), null)), ImmutableMap.of( DATA_SOURCE, createTimeline( @@ -116,7 +124,7 @@ public void testLargeGapInData() { final Period segmentPeriod = new Period("PT1H"); final CompactionSegmentIterator iterator = policy.reset( - ImmutableMap.of(DATA_SOURCE, createCompactionConfig(10000, new Period("PT1H1M"))), + ImmutableMap.of(DATA_SOURCE, createCompactionConfig(10000, new Period("PT1H1M"), null)), ImmutableMap.of( DATA_SOURCE, createTimeline( @@ -149,7 +157,7 @@ public void testLargeGapInData() public void testHugeShard() { final CompactionSegmentIterator iterator = policy.reset( - ImmutableMap.of(DATA_SOURCE, createCompactionConfig(10000, new Period("P1D"))), + ImmutableMap.of(DATA_SOURCE, createCompactionConfig(10000, new Period("P1D"), null)), ImmutableMap.of( DATA_SOURCE, createTimeline( @@ -199,7 +207,7 @@ public void testHugeShard() public void testManySegmentsPerShard() { final CompactionSegmentIterator iterator = policy.reset( - ImmutableMap.of(DATA_SOURCE, createCompactionConfig(800000, new Period("P1D"))), + ImmutableMap.of(DATA_SOURCE, createCompactionConfig(800000, new Period("P1D"), null)), ImmutableMap.of( DATA_SOURCE, createTimeline( @@ -259,9 +267,9 @@ public void testSkipUnknownDataSource() final CompactionSegmentIterator iterator = policy.reset( ImmutableMap.of( unknownDataSource, - createCompactionConfig(10000, new Period("P2D")), + createCompactionConfig(10000, new Period("P2D"), null), DATA_SOURCE, - createCompactionConfig(10000, new Period("P2D")) + createCompactionConfig(10000, new Period("P2D"), null) ), ImmutableMap.of( DATA_SOURCE, @@ -307,7 +315,7 @@ public void testClearSegmentsToCompactWhenSkippingSegments() ) ); final CompactionSegmentIterator iterator = policy.reset( - ImmutableMap.of(DATA_SOURCE, createCompactionConfig(inputSegmentSizeBytes, new Period("P0D"))), + ImmutableMap.of(DATA_SOURCE, createCompactionConfig(inputSegmentSizeBytes, new Period("P0D"), null)), ImmutableMap.of(DATA_SOURCE, timeline), Collections.emptyMap() ); @@ -340,7 +348,7 @@ public void testIfFirstSegmentIsInSkipOffset() ); final CompactionSegmentIterator iterator = policy.reset( - ImmutableMap.of(DATA_SOURCE, createCompactionConfig(40000, new Period("P1D"))), + ImmutableMap.of(DATA_SOURCE, createCompactionConfig(40000, new Period("P1D"), null)), ImmutableMap.of(DATA_SOURCE, timeline), Collections.emptyMap() ); @@ -361,7 +369,7 @@ public void testIfFirstSegmentOverlapsSkipOffset() ); final CompactionSegmentIterator iterator = policy.reset( - ImmutableMap.of(DATA_SOURCE, createCompactionConfig(40000, new Period("P1D"))), + ImmutableMap.of(DATA_SOURCE, createCompactionConfig(40000, new Period("P1D"), null)), ImmutableMap.of(DATA_SOURCE, timeline), Collections.emptyMap() ); @@ -369,12 +377,91 @@ public void testIfFirstSegmentOverlapsSkipOffset() Assert.assertFalse(iterator.hasNext()); } + @Test + public void testIfSegmentsSkipOffsetWithConfiguredSegmentGranularityEqual() + { + final VersionedIntervalTimeline timeline = createTimeline( + new SegmentGenerateSpec(Intervals.of("2017-11-30T23:00:00/2017-12-03T00:00:00"), new Period("P1D")), + new SegmentGenerateSpec(Intervals.of("2017-10-14T00:00:00/2017-10-15T00:00:00"), new Period("P1D")) + ); + + final CompactionSegmentIterator iterator = policy.reset( + ImmutableMap.of(DATA_SOURCE, createCompactionConfig( + 40000, new Period("P1D"), new UniformGranularitySpec(Granularities.DAY, null, null))), + ImmutableMap.of(DATA_SOURCE, timeline), + Collections.emptyMap() + ); + + // We should only get segments in Oct + final List expectedSegmentsToCompact = new ArrayList<>( + timeline.findNonOvershadowedObjectsInInterval(Intervals.of("2017-10-14T00:00:00/2017-12-02T00:00:00"), Partitions.ONLY_COMPLETE) + ); + + Assert.assertTrue(iterator.hasNext()); + Assert.assertEquals(ImmutableSet.copyOf(expectedSegmentsToCompact), ImmutableSet.copyOf(Iterables.concat(ImmutableSet.copyOf(iterator)))); + } + + @Test + public void testIfSegmentsSkipOffsetWithConfiguredSegmentGranularityLarger() + { + final VersionedIntervalTimeline timeline = createTimeline( + // This contains segment that + // - Cross between month boundary of latest month (starts in Nov and ends in Dec). This should be skipped + // - Fully in latest month (starts in Dec and ends in Dec). This should be skipped + // - Does not overlap latest month (starts in Oct and ends in Oct). This should not be skipped + new SegmentGenerateSpec(Intervals.of("2017-11-30T23:00:00/2017-12-03T00:00:00"), new Period("PT5H")), + new SegmentGenerateSpec(Intervals.of("2017-10-14T00:00:00/2017-10-15T00:00:00"), new Period("PT5H")) + ); + + final CompactionSegmentIterator iterator = policy.reset( + ImmutableMap.of(DATA_SOURCE, createCompactionConfig( + 40000, new Period("P1D"), new UniformGranularitySpec(Granularities.MONTH, null, null))), + ImmutableMap.of(DATA_SOURCE, timeline), + Collections.emptyMap() + ); + + // We should only get segments in Oct + final List expectedSegmentsToCompact = new ArrayList<>( + timeline.findNonOvershadowedObjectsInInterval(Intervals.of("2017-10-14T00:00:00/2017-10-15T00:00:00"), Partitions.ONLY_COMPLETE) + ); + + Assert.assertTrue(iterator.hasNext()); + List actual = iterator.next(); + Assert.assertEquals(expectedSegmentsToCompact.size(), actual.size()); + Assert.assertEquals(ImmutableSet.copyOf(expectedSegmentsToCompact), ImmutableSet.copyOf(actual)); + Assert.assertFalse(iterator.hasNext()); + } + + @Test + public void testIfSegmentsSkipOffsetWithConfiguredSegmentGranularitySmaller() + { + final VersionedIntervalTimeline timeline = createTimeline( + new SegmentGenerateSpec(Intervals.of("2017-12-01T23:00:00/2017-12-03T00:00:00"), new Period("PT5H")), + new SegmentGenerateSpec(Intervals.of("2017-10-14T00:00:00/2017-10-15T00:00:00"), new Period("PT5H")) + ); + + final CompactionSegmentIterator iterator = policy.reset( + ImmutableMap.of(DATA_SOURCE, createCompactionConfig( + 40000, new Period("P1D"), new UniformGranularitySpec(Granularities.MINUTE, null, null))), + ImmutableMap.of(DATA_SOURCE, timeline), + Collections.emptyMap() + ); + + // We should only get segments in Oct + final List expectedSegmentsToCompact = new ArrayList<>( + timeline.findNonOvershadowedObjectsInInterval(Intervals.of("2017-10-14T00:00:00/2017-10-15T00:00:00"), Partitions.ONLY_COMPLETE) + ); + + Assert.assertTrue(iterator.hasNext()); + Assert.assertEquals(ImmutableSet.copyOf(expectedSegmentsToCompact), ImmutableSet.copyOf(Iterables.concat(ImmutableSet.copyOf(iterator)))); + } + @Test public void testWithSkipIntervals() { final Period segmentPeriod = new Period("PT1H"); final CompactionSegmentIterator iterator = policy.reset( - ImmutableMap.of(DATA_SOURCE, createCompactionConfig(10000, new Period("P1D"))), + ImmutableMap.of(DATA_SOURCE, createCompactionConfig(10000, new Period("P1D"), null)), ImmutableMap.of( DATA_SOURCE, createTimeline( @@ -414,7 +501,7 @@ public void testHoleInSearchInterval() { final Period segmentPeriod = new Period("PT1H"); final CompactionSegmentIterator iterator = policy.reset( - ImmutableMap.of(DATA_SOURCE, createCompactionConfig(10000, new Period("PT1H"))), + ImmutableMap.of(DATA_SOURCE, createCompactionConfig(10000, new Period("PT1H"), null)), ImmutableMap.of( DATA_SOURCE, createTimeline( @@ -455,6 +542,91 @@ public void testHoleInSearchInterval() ); } + @Test + public void testIteratorReturnsSegmentsInConfiguredSegmentGranularity() + { + final VersionedIntervalTimeline timeline = createTimeline( + // Segments with day interval from Oct to Dec + new SegmentGenerateSpec(Intervals.of("2017-10-01T00:00:00/2017-12-31T00:00:00"), new Period("P1D")) + ); + + final CompactionSegmentIterator iterator = policy.reset( + ImmutableMap.of(DATA_SOURCE, createCompactionConfig( + 130000, new Period("P0D"), new UniformGranularitySpec(Granularities.MONTH, null, null))), + ImmutableMap.of(DATA_SOURCE, timeline), + Collections.emptyMap() + ); + + // We should get all segments in timeline back since skip offset is P0D. + // However, we only need to iterator 3 times (once for each month) since the new configured segmentGranularity is MONTH. + // and hence iterator would return all segments bucketed to the configured segmentGranularity + // Month of Dec + Assert.assertTrue(iterator.hasNext()); + List expectedSegmentsToCompact = new ArrayList<>( + timeline.findNonOvershadowedObjectsInInterval(Intervals.of("2017-12-01T00:00:00/2017-12-31T00:00:00"), Partitions.ONLY_COMPLETE) + ); + Assert.assertEquals( + ImmutableSet.copyOf(expectedSegmentsToCompact), + ImmutableSet.copyOf(iterator.next()) + ); + // Month of Nov + Assert.assertTrue(iterator.hasNext()); + expectedSegmentsToCompact = new ArrayList<>( + timeline.findNonOvershadowedObjectsInInterval(Intervals.of("2017-11-01T00:00:00/2017-12-01T00:00:00"), Partitions.ONLY_COMPLETE) + ); + Assert.assertEquals( + ImmutableSet.copyOf(expectedSegmentsToCompact), + ImmutableSet.copyOf(iterator.next()) + ); + // Month of Oct + Assert.assertTrue(iterator.hasNext()); + expectedSegmentsToCompact = new ArrayList<>( + timeline.findNonOvershadowedObjectsInInterval(Intervals.of("2017-10-01T00:00:00/2017-11-01T00:00:00"), Partitions.ONLY_COMPLETE) + ); + Assert.assertEquals( + ImmutableSet.copyOf(expectedSegmentsToCompact), + ImmutableSet.copyOf(iterator.next()) + ); + // No more + Assert.assertFalse(iterator.hasNext()); + } + + @Test + public void testIteratorReturnsSegmentsInMultipleIntervalIfConfiguredSegmentGranularityCrossBoundary() + { + final VersionedIntervalTimeline timeline = createTimeline( + // Single segment starts in Jan and ends in Feb + new SegmentGenerateSpec(Intervals.of("2020-01-28/2020-02-03"), new Period("P7D")) + ); + + final CompactionSegmentIterator iterator = policy.reset( + ImmutableMap.of(DATA_SOURCE, createCompactionConfig( + 130000, new Period("P0D"), new UniformGranularitySpec(Granularities.MONTH, null, null))), + ImmutableMap.of(DATA_SOURCE, timeline), + Collections.emptyMap() + ); + + List expectedSegmentsToCompact = new ArrayList<>( + timeline.findNonOvershadowedObjectsInInterval(Intervals.of("2020-01-28/2020-02-03"), Partitions.ONLY_COMPLETE) + ); + + // We should get the same segment back twice when the iterator returns for Jan and when the iterator returns for Feb + // Month of Jan + Assert.assertTrue(iterator.hasNext()); + Assert.assertEquals( + expectedSegmentsToCompact, + iterator.next() + ); + // Month of Feb + Assert.assertTrue(iterator.hasNext()); + Assert.assertEquals( + expectedSegmentsToCompact, + iterator.next() + ); + // No more + Assert.assertFalse(iterator.hasNext()); + } + private static void assertCompactSegmentIntervals( CompactionSegmentIterator iterator, Period segmentPeriod, @@ -546,7 +718,8 @@ private static VersionedIntervalTimeline createTimeline( private DataSourceCompactionConfig createCompactionConfig( long inputSegmentSizeBytes, - Period skipOffsetFromLatest + Period skipOffsetFromLatest, + GranularitySpec granularitySpec ) { return new DataSourceCompactionConfig( @@ -556,7 +729,7 @@ private DataSourceCompactionConfig createCompactionConfig( null, skipOffsetFromLatest, null, - null, + granularitySpec, null ); } From 092868f5c6bcd24b74ddb866c88961193a89c33a Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Fri, 5 Feb 2021 01:00:05 -0800 Subject: [PATCH 12/31] fix checkstyle --- ...ientCompactionTaskQueryGranularitySpec.java | 1 - .../DataSourceCompactionConfigTest.java | 1 - .../coordinator/duty/CompactSegmentsTest.java | 2 -- .../duty/NewestSegmentFirstPolicyTest.java | 18 +++++------------- 4 files changed, 5 insertions(+), 17 deletions(-) diff --git a/server/src/main/java/org/apache/druid/client/indexing/ClientCompactionTaskQueryGranularitySpec.java b/server/src/main/java/org/apache/druid/client/indexing/ClientCompactionTaskQueryGranularitySpec.java index 580275a6a0b0..6f12ea771331 100644 --- a/server/src/main/java/org/apache/druid/client/indexing/ClientCompactionTaskQueryGranularitySpec.java +++ b/server/src/main/java/org/apache/druid/client/indexing/ClientCompactionTaskQueryGranularitySpec.java @@ -23,7 +23,6 @@ import com.fasterxml.jackson.annotation.JsonProperty; import org.apache.druid.java.util.common.granularity.Granularity; import org.apache.druid.segment.indexing.granularity.BaseGranularitySpec; -import org.apache.druid.segment.indexing.granularity.UniformGranularitySpec; import java.util.Objects; diff --git a/server/src/test/java/org/apache/druid/server/coordinator/DataSourceCompactionConfigTest.java b/server/src/test/java/org/apache/druid/server/coordinator/DataSourceCompactionConfigTest.java index 7401c9a57525..01cfabb2c40b 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/DataSourceCompactionConfigTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/DataSourceCompactionConfigTest.java @@ -22,7 +22,6 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import jdk.nashorn.internal.ir.annotations.Immutable; import org.apache.druid.data.input.SegmentsSplitHintSpec; import org.apache.druid.indexer.partitions.DynamicPartitionsSpec; import org.apache.druid.jackson.DefaultObjectMapper; diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/CompactSegmentsTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/CompactSegmentsTest.java index bdeb9454831f..c8928103c095 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/CompactSegmentsTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/CompactSegmentsTest.java @@ -38,7 +38,6 @@ import org.apache.druid.client.indexing.IndexingWorker; import org.apache.druid.client.indexing.IndexingWorkerInfo; import org.apache.druid.client.indexing.TaskPayloadResponse; -import org.apache.druid.client.indexing.TaskStatus; import org.apache.druid.discovery.DruidLeaderClient; import org.apache.druid.discovery.DruidNodeDiscovery; import org.apache.druid.discovery.DruidNodeDiscoveryProvider; @@ -70,7 +69,6 @@ import org.apache.druid.server.coordinator.UserCompactionTaskQueryTuningConfig; import org.apache.druid.timeline.CompactionState; import org.apache.druid.timeline.DataSegment; -import org.apache.druid.timeline.Partitions; import org.apache.druid.timeline.TimelineObjectHolder; import org.apache.druid.timeline.VersionedIntervalTimeline; import org.apache.druid.timeline.partition.HashBasedNumberedShardSpec; diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstPolicyTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstPolicyTest.java index bdf0da2b9fea..0c7e6ea1c340 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstPolicyTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstPolicyTest.java @@ -24,12 +24,10 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; -import org.apache.commons.collections.IteratorUtils; import org.apache.druid.jackson.DefaultObjectMapper; import org.apache.druid.java.util.common.DateTimes; import org.apache.druid.java.util.common.Intervals; import org.apache.druid.java.util.common.granularity.Granularities; -import org.apache.druid.java.util.common.granularity.Granularity; import org.apache.druid.java.util.common.guava.Comparators; import org.apache.druid.segment.indexing.granularity.GranularitySpec; import org.apache.druid.segment.indexing.granularity.UniformGranularitySpec; @@ -49,7 +47,6 @@ import java.util.Arrays; import java.util.Collections; import java.util.Comparator; -import java.util.HashSet; import java.util.List; import java.util.stream.Collectors; @@ -386,8 +383,7 @@ public void testIfSegmentsSkipOffsetWithConfiguredSegmentGranularityEqual() ); final CompactionSegmentIterator iterator = policy.reset( - ImmutableMap.of(DATA_SOURCE, createCompactionConfig( - 40000, new Period("P1D"), new UniformGranularitySpec(Granularities.DAY, null, null))), + ImmutableMap.of(DATA_SOURCE, createCompactionConfig(40000, new Period("P1D"), new UniformGranularitySpec(Granularities.DAY, null, null))), ImmutableMap.of(DATA_SOURCE, timeline), Collections.emptyMap() ); @@ -414,8 +410,7 @@ public void testIfSegmentsSkipOffsetWithConfiguredSegmentGranularityLarger() ); final CompactionSegmentIterator iterator = policy.reset( - ImmutableMap.of(DATA_SOURCE, createCompactionConfig( - 40000, new Period("P1D"), new UniformGranularitySpec(Granularities.MONTH, null, null))), + ImmutableMap.of(DATA_SOURCE, createCompactionConfig(40000, new Period("P1D"), new UniformGranularitySpec(Granularities.MONTH, null, null))), ImmutableMap.of(DATA_SOURCE, timeline), Collections.emptyMap() ); @@ -441,8 +436,7 @@ public void testIfSegmentsSkipOffsetWithConfiguredSegmentGranularitySmaller() ); final CompactionSegmentIterator iterator = policy.reset( - ImmutableMap.of(DATA_SOURCE, createCompactionConfig( - 40000, new Period("P1D"), new UniformGranularitySpec(Granularities.MINUTE, null, null))), + ImmutableMap.of(DATA_SOURCE, createCompactionConfig(40000, new Period("P1D"), new UniformGranularitySpec(Granularities.MINUTE, null, null))), ImmutableMap.of(DATA_SOURCE, timeline), Collections.emptyMap() ); @@ -551,8 +545,7 @@ public void testIteratorReturnsSegmentsInConfiguredSegmentGranularity() ); final CompactionSegmentIterator iterator = policy.reset( - ImmutableMap.of(DATA_SOURCE, createCompactionConfig( - 130000, new Period("P0D"), new UniformGranularitySpec(Granularities.MONTH, null, null))), + ImmutableMap.of(DATA_SOURCE, createCompactionConfig(130000, new Period("P0D"), new UniformGranularitySpec(Granularities.MONTH, null, null))), ImmutableMap.of(DATA_SOURCE, timeline), Collections.emptyMap() ); @@ -600,8 +593,7 @@ public void testIteratorReturnsSegmentsInMultipleIntervalIfConfiguredSegmentGran ); final CompactionSegmentIterator iterator = policy.reset( - ImmutableMap.of(DATA_SOURCE, createCompactionConfig( - 130000, new Period("P0D"), new UniformGranularitySpec(Granularities.MONTH, null, null))), + ImmutableMap.of(DATA_SOURCE, createCompactionConfig(130000, new Period("P0D"), new UniformGranularitySpec(Granularities.MONTH, null, null))), ImmutableMap.of(DATA_SOURCE, timeline), Collections.emptyMap() ); From ace42815d9d16d8d4ce27cf5194794b2d2770c99 Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Fri, 5 Feb 2021 01:01:44 -0800 Subject: [PATCH 13/31] fix checkstyle --- .../task/ClientCompactionTaskQuerySerdeTest.java | 1 - .../indexing/common/task/CompactionTaskTest.java | 14 +++++++------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/ClientCompactionTaskQuerySerdeTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/ClientCompactionTaskQuerySerdeTest.java index c3419452cee8..1e9628f96a71 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/ClientCompactionTaskQuerySerdeTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/ClientCompactionTaskQuerySerdeTest.java @@ -47,7 +47,6 @@ import org.apache.druid.java.util.common.HumanReadableBytes; import org.apache.druid.java.util.common.Intervals; import org.apache.druid.java.util.common.granularity.Granularities; -import org.apache.druid.java.util.common.granularity.Granularity; import org.apache.druid.segment.IndexSpec; import org.apache.druid.segment.data.BitmapSerde.DefaultBitmapSerdeFactory; import org.apache.druid.segment.data.CompressionFactory.LongEncodingStrategy; diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskTest.java index 3512903a3922..7b3b76425136 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskTest.java @@ -363,9 +363,9 @@ public void testCreateCompactionTaskWithGranularitySpec() segmentLoaderFactory, RETRY_POLICY_FACTORY ).inputSpec(new CompactionIntervalSpec(COMPACTION_INTERVAL, SegmentUtils.hashIds(SEGMENTS))) - .tuningConfig(createTuningConfig()) - .segmentGranularity(Granularities.HOUR) - .build(); + .tuningConfig(createTuningConfig()) + .segmentGranularity(Granularities.HOUR) + .build(); final CompactionTask taskCreatedWithGranularitySpec = new Builder( DATA_SOURCE, segmentLoaderFactory, @@ -385,10 +385,10 @@ public void testCreateCompactionTaskWithGranularitySpecOverrideSegmentGranularit segmentLoaderFactory, RETRY_POLICY_FACTORY ).inputSpec(new CompactionIntervalSpec(COMPACTION_INTERVAL, SegmentUtils.hashIds(SEGMENTS))) - .tuningConfig(createTuningConfig()) - .segmentGranularity(Granularities.HOUR) - .granularitySpec(new UniformGranularitySpec(Granularities.MINUTE, Granularities.DAY, null)) - .build(); + .tuningConfig(createTuningConfig()) + .segmentGranularity(Granularities.HOUR) + .granularitySpec(new UniformGranularitySpec(Granularities.MINUTE, Granularities.DAY, null)) + .build(); Assert.assertEquals(Granularities.MINUTE, taskCreatedWithSegmentGranularity.getSegmentGranularity()); } From 5873318584bbf842edf26c0cdc0e69f0ee5bdafa Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Fri, 5 Feb 2021 01:05:44 -0800 Subject: [PATCH 14/31] fix checkstyle --- .../common/task/CompactionTaskTest.java | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskTest.java index 7b3b76425136..71b603eab517 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskTest.java @@ -358,37 +358,41 @@ public void setup() @Test public void testCreateCompactionTaskWithGranularitySpec() { - final CompactionTask taskCreatedWithSegmentGranularity = new Builder( + final Builder builder = new Builder( DATA_SOURCE, segmentLoaderFactory, RETRY_POLICY_FACTORY - ).inputSpec(new CompactionIntervalSpec(COMPACTION_INTERVAL, SegmentUtils.hashIds(SEGMENTS))) - .tuningConfig(createTuningConfig()) - .segmentGranularity(Granularities.HOUR) - .build(); - final CompactionTask taskCreatedWithGranularitySpec = new Builder( + ); + builder.inputSpec(new CompactionIntervalSpec(COMPACTION_INTERVAL, SegmentUtils.hashIds(SEGMENTS))); + builder.tuningConfig(createTuningConfig()); + builder.segmentGranularity(Granularities.HOUR); + final CompactionTask taskCreatedWithSegmentGranularity = builder.build(); + + final Builder builder2 = new Builder( DATA_SOURCE, segmentLoaderFactory, RETRY_POLICY_FACTORY - ).inputSpec(new CompactionIntervalSpec(COMPACTION_INTERVAL, SegmentUtils.hashIds(SEGMENTS))) - .tuningConfig(createTuningConfig()) - .granularitySpec(new UniformGranularitySpec(Granularities.HOUR, Granularities.DAY, null)) - .build(); + ); + builder2.inputSpec(new CompactionIntervalSpec(COMPACTION_INTERVAL, SegmentUtils.hashIds(SEGMENTS))); + builder2.tuningConfig(createTuningConfig()); + builder2.granularitySpec(new UniformGranularitySpec(Granularities.HOUR, Granularities.DAY, null)); + final CompactionTask taskCreatedWithGranularitySpec = builder2.build(); Assert.assertEquals(taskCreatedWithGranularitySpec.getSegmentGranularity(), taskCreatedWithSegmentGranularity.getSegmentGranularity()); } @Test public void testCreateCompactionTaskWithGranularitySpecOverrideSegmentGranularity() { - final CompactionTask taskCreatedWithSegmentGranularity = new Builder( + final Builder builder = new Builder( DATA_SOURCE, segmentLoaderFactory, RETRY_POLICY_FACTORY - ).inputSpec(new CompactionIntervalSpec(COMPACTION_INTERVAL, SegmentUtils.hashIds(SEGMENTS))) - .tuningConfig(createTuningConfig()) - .segmentGranularity(Granularities.HOUR) - .granularitySpec(new UniformGranularitySpec(Granularities.MINUTE, Granularities.DAY, null)) - .build(); + ); + builder.inputSpec(new CompactionIntervalSpec(COMPACTION_INTERVAL, SegmentUtils.hashIds(SEGMENTS))); + builder.tuningConfig(createTuningConfig()); + builder.segmentGranularity(Granularities.HOUR); + builder.granularitySpec(new UniformGranularitySpec(Granularities.MINUTE, Granularities.DAY, null)); + final CompactionTask taskCreatedWithSegmentGranularity = builder.build(); Assert.assertEquals(Granularities.MINUTE, taskCreatedWithSegmentGranularity.getSegmentGranularity()); } From a09deeac8434081fe99ffba636c38384acd4d1ed Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Fri, 5 Feb 2021 03:06:11 -0800 Subject: [PATCH 15/31] add unit tests --- .../duty/NewestSegmentFirstIterator.java | 50 +++++++++++++----- .../duty/NewestSegmentFirstPolicyTest.java | 52 ++++++++++++++----- 2 files changed, 75 insertions(+), 27 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIterator.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIterator.java index 807e42a94f16..98c1440e1d4d 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIterator.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIterator.java @@ -30,6 +30,7 @@ import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.Intervals; import org.apache.druid.java.util.common.JodaUtils; +import org.apache.druid.java.util.common.Pair; import org.apache.druid.java.util.common.granularity.Granularity; import org.apache.druid.java.util.common.guava.Comparators; import org.apache.druid.java.util.common.logger.Logger; @@ -84,7 +85,10 @@ public class NewestSegmentFirstIterator implements CompactionSegmentIterator // searchIntervals keeps track of the current state of which interval should be considered to search segments to // compact. private final Map timelineIterators; - private final Map> originalShardSpecDatasourceMap = new HashMap<>(); + // 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> intervalCompactedForDatasource = new HashMap<>(); private final PriorityQueue queue = new PriorityQueue<>( (o1, o2) -> Comparators.intervalsByStartThenEnd().compare(o2.interval, o1.interval) @@ -105,6 +109,7 @@ public class NewestSegmentFirstIterator implements CompactionSegmentIterator final DataSourceCompactionConfig config = compactionConfigs.get(dataSource); Granularity configuredSegmentGranularity = null; if (config != null && !timeline.isEmpty()) { + Map, ShardSpec> originalShardSpecs = new HashMap<>(); if (config.getGranularitySpec() != null && config.getGranularitySpec().getSegmentGranularity() != null) { Map> intervalToPartitionMap = new HashMap<>(); configuredSegmentGranularity = config.getGranularitySpec().getSegmentGranularity(); @@ -120,7 +125,6 @@ public class NewestSegmentFirstIterator implements CompactionSegmentIterator intervalToPartitionMap.computeIfAbsent(interval, k -> new HashSet<>()).add(segment); } } - Map originalShardSpecs = originalShardSpecDatasourceMap.computeIfAbsent(dataSource, k -> new HashMap<>()); for (Map.Entry> partitionsPerInterval : intervalToPartitionMap.entrySet()) { Interval interval = partitionsPerInterval.getKey(); int partitionNum = 0; @@ -135,7 +139,7 @@ public class NewestSegmentFirstIterator implements CompactionSegmentIterator // by the new configuredSegmentGranularity. We then convert each segment into a new partition space so that // there is no duplicate partitionNum across all segments of each new Interval. We will have to save the // original ShardSpec to convert the segment back when returning from the iterator. - originalShardSpecs.put(segmentsForCompact.getId(), segment.getShardSpec()); + originalShardSpecs.put(new Pair<>(interval, segmentsForCompact.getId()), segment.getShardSpec()); timelineWithConfiguredSegmentGranularity.add( interval, segmentsForCompact.getVersion(), @@ -149,7 +153,7 @@ public class NewestSegmentFirstIterator implements CompactionSegmentIterator final List searchIntervals = findInitialSearchInterval(timeline, config.getSkipOffsetFromLatest(), configuredSegmentGranularity, skipIntervals.get(dataSource)); if (!searchIntervals.isEmpty()) { - timelineIterators.put(dataSource, new CompactibleTimelineObjectHolderCursor(timeline, searchIntervals)); + timelineIterators.put(dataSource, new CompactibleTimelineObjectHolderCursor(timeline, searchIntervals, originalShardSpecs)); } } }); @@ -238,10 +242,12 @@ private void updateQueue(String dataSourceName, DataSourceCompactionConfig confi private static class CompactibleTimelineObjectHolderCursor implements Iterator> { private final List> holders; + private final Map, ShardSpec> originalShardSpecs; CompactibleTimelineObjectHolderCursor( VersionedIntervalTimeline timeline, - List totalIntervalsToSearch + List totalIntervalsToSearch, + Map, ShardSpec> originalShardSpecs ) { this.holders = totalIntervalsToSearch @@ -252,6 +258,7 @@ private static class CompactibleTimelineObjectHolderCursor implements Iterator isCompactibleHolder(interval, holder)) ) .collect(Collectors.toList()); + this.originalShardSpecs = originalShardSpecs; } private boolean isCompactibleHolder(Interval interval, TimelineObjectHolder holder) @@ -271,6 +278,14 @@ private boolean isCompactibleHolder(Interval interval, TimelineObjectHolder 0; } + private DataSegment transformShardSpecIfNeeded(DataSegment dataSegment, Interval interval) + { + if (originalShardSpecs != null && !originalShardSpecs.isEmpty()) { + return dataSegment.withShardSpec(originalShardSpecs.get(new Pair<>(interval, dataSegment.getId()))); + } + return dataSegment; + } + @Override public boolean hasNext() { @@ -283,8 +298,10 @@ public List next() if (holders.isEmpty()) { throw new NoSuchElementException(); } - return Streams.sequentialStreamFrom(holders.remove(holders.size() - 1).getObject()) + TimelineObjectHolder timelineObjectHolder = holders.remove(holders.size() - 1); + return Streams.sequentialStreamFrom(timelineObjectHolder.getObject()) .map(PartitionChunk::getObject) + .map(dataSegment -> transformShardSpecIfNeeded(dataSegment, timelineObjectHolder.getTrueInterval())) .collect(Collectors.toList()); } } @@ -403,13 +420,6 @@ private SegmentsToCompact findSegmentsToCompact( while (compactibleTimelineObjectHolderCursor.hasNext()) { List segments = compactibleTimelineObjectHolderCursor.next(); - Map originalShardSpec = originalShardSpecDatasourceMap.get(dataSourceName); - // Convert segment back to original ShardSpec if the datasource has configuredSegmentGranularity set - if (originalShardSpec != null) { - segments = segments.stream() - .map(segment -> segment.withShardSpec(originalShardSpec.get(segment.getId()))) - .collect(Collectors.toList()); - } final SegmentsToCompact candidates = new SegmentsToCompact(segments); if (!candidates.isEmpty()) { final boolean isCompactibleSize = candidates.getTotalSize() <= inputSegmentSize; @@ -419,6 +429,15 @@ private SegmentsToCompact findSegmentsToCompact( ); if (isCompactibleSize && needsCompaction) { + if (config.getGranularitySpec() != null && config.getGranularitySpec().getSegmentGranularity() != null) { + Interval interval = candidates.getUmbrellaInterval(); + Set intervalsCompacted = intervalCompactedForDatasource.computeIfAbsent(dataSourceName, k -> new HashSet<>()); + // Skip this candidates if we have compacted the interval already + if (intervalsCompacted.contains(interval)) { + continue; + } + intervalsCompacted.add(interval); + } return candidates; } else { if (!needsCompaction) { @@ -663,6 +682,11 @@ private long getNumberOfSegments() return segments.size(); } + private Interval getUmbrellaInterval() + { + return JodaUtils.umbrellaInterval(segments.stream().map(DataSegment::getInterval).collect(Collectors.toList())); + } + private long getNumberOfIntervals() { return segments.stream().map(DataSegment::getInterval).distinct().count(); diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstPolicyTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstPolicyTest.java index 0c7e6ea1c340..cc23f6a4ab32 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstPolicyTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstPolicyTest.java @@ -588,8 +588,9 @@ public void testIteratorReturnsSegmentsInConfiguredSegmentGranularity() public void testIteratorReturnsSegmentsInMultipleIntervalIfConfiguredSegmentGranularityCrossBoundary() { final VersionedIntervalTimeline timeline = createTimeline( - // Single segment starts in Jan and ends in Feb - new SegmentGenerateSpec(Intervals.of("2020-01-28/2020-02-03"), new Period("P7D")) + new SegmentGenerateSpec(Intervals.of("2020-01-01/2020-01-08"), new Period("P7D")), + new SegmentGenerateSpec(Intervals.of("2020-01-28/2020-02-03"), new Period("P7D")), + new SegmentGenerateSpec(Intervals.of("2020-02-08/2020-02-15"), new Period("P7D")) ); final CompactionSegmentIterator iterator = policy.reset( @@ -597,28 +598,51 @@ public void testIteratorReturnsSegmentsInMultipleIntervalIfConfiguredSegmentGran ImmutableMap.of(DATA_SOURCE, timeline), Collections.emptyMap() ); + // We should get the segment of "2020-01-28/2020-02-03" back twice when the iterator returns for Jan and when the + // iterator returns for Feb. + // Month of Jan List expectedSegmentsToCompact = new ArrayList<>( - timeline.findNonOvershadowedObjectsInInterval(Intervals.of("2020-01-28/2020-02-03"), Partitions.ONLY_COMPLETE) + timeline.findNonOvershadowedObjectsInInterval(Intervals.of("2020-01-28/2020-02-15"), Partitions.ONLY_COMPLETE) ); - - // We should get the same segment back twice when the iterator returns for Jan and when the iterator returns for Feb - // Month of Jan Assert.assertTrue(iterator.hasNext()); - Assert.assertEquals( - expectedSegmentsToCompact, - iterator.next() - ); + List actual = iterator.next(); + Assert.assertEquals(expectedSegmentsToCompact.size(), actual.size()); + Assert.assertEquals(ImmutableSet.copyOf(expectedSegmentsToCompact), ImmutableSet.copyOf(actual)); // Month of Feb - Assert.assertTrue(iterator.hasNext()); - Assert.assertEquals( - expectedSegmentsToCompact, - iterator.next() + expectedSegmentsToCompact = new ArrayList<>( + timeline.findNonOvershadowedObjectsInInterval(Intervals.of("2020-01-01/2020-02-03"), Partitions.ONLY_COMPLETE) ); + Assert.assertTrue(iterator.hasNext()); + actual = iterator.next(); + Assert.assertEquals(expectedSegmentsToCompact.size(), actual.size()); + Assert.assertEquals(ImmutableSet.copyOf(expectedSegmentsToCompact), ImmutableSet.copyOf(actual)); // No more Assert.assertFalse(iterator.hasNext()); } + @Test + public void testIteratorDoesNotReturnCompactedInterval() + { + final VersionedIntervalTimeline timeline = createTimeline( + new SegmentGenerateSpec(Intervals.of("2017-12-01T00:00:00/2017-12-02T00:00:00"), new Period("P1D")) + ); + + final CompactionSegmentIterator iterator = policy.reset( + ImmutableMap.of(DATA_SOURCE, createCompactionConfig(40000, new Period("P0D"), new UniformGranularitySpec(Granularities.MINUTE, null, null))), + ImmutableMap.of(DATA_SOURCE, timeline), + Collections.emptyMap() + ); + + final List expectedSegmentsToCompact = new ArrayList<>( + timeline.findNonOvershadowedObjectsInInterval(Intervals.of("2017-12-01T00:00:00/2017-12-02T00:00:00"), Partitions.ONLY_COMPLETE) + ); + Assert.assertTrue(iterator.hasNext()); + Assert.assertEquals(ImmutableSet.copyOf(expectedSegmentsToCompact), ImmutableSet.copyOf(iterator.next())); + // Iterator should return only once since all the "minute" interval of the iterator contains the same interval + Assert.assertFalse(iterator.hasNext()); + } + private static void assertCompactSegmentIntervals( CompactionSegmentIterator iterator, Period segmentPeriod, From 9ed2c618cc9b3c6851d26b029e4b5d563dbb1480 Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Fri, 5 Feb 2021 03:22:30 -0800 Subject: [PATCH 16/31] add integration tests --- .../duty/ITAutoCompactionTest.java | 51 +++++++++++++++++-- .../tests/indexer/ITCompactionTaskTest.java | 26 +++++++--- ...compaction_task_with_granularity_spec.json | 17 +++++++ ...paction_task_with_segment_granularity.json | 15 ++++++ 4 files changed, 98 insertions(+), 11 deletions(-) create mode 100644 integration-tests/src/test/resources/indexer/wikipedia_compaction_task_with_granularity_spec.json create mode 100644 integration-tests/src/test/resources/indexer/wikipedia_compaction_task_with_segment_granularity.json diff --git a/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java b/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java index 839f8202889c..d68ad8d5640a 100644 --- a/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java +++ b/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java @@ -28,7 +28,11 @@ import org.apache.druid.indexer.partitions.SingleDimensionPartitionsSpec; import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.StringUtils; +import org.apache.druid.java.util.common.granularity.Granularities; +import org.apache.druid.java.util.common.granularity.Granularity; import org.apache.druid.java.util.common.logger.Logger; +import org.apache.druid.segment.indexing.granularity.GranularitySpec; +import org.apache.druid.segment.indexing.granularity.UniformGranularitySpec; import org.apache.druid.server.coordinator.AutoCompactionSnapshot; import org.apache.druid.server.coordinator.CoordinatorCompactionConfig; import org.apache.druid.server.coordinator.DataSourceCompactionConfig; @@ -160,7 +164,7 @@ public void testAutoCompactionDutyCanUpdateCompactionConfig() throws Exception LOG.info("Auto compaction test with hash partitioning"); final HashedPartitionsSpec hashedPartitionsSpec = new HashedPartitionsSpec(null, 3, null); - submitCompactionConfig(hashedPartitionsSpec, NO_SKIP_OFFSET, 1); + submitCompactionConfig(hashedPartitionsSpec, NO_SKIP_OFFSET, 1, null); // 2 segments published per day after compaction. forceTriggerAutoCompaction(4); verifyQuery(INDEX_QUERIES_RESOURCE); @@ -175,7 +179,7 @@ public void testAutoCompactionDutyCanUpdateCompactionConfig() throws Exception "city", false ); - submitCompactionConfig(rangePartitionsSpec, NO_SKIP_OFFSET, 1); + submitCompactionConfig(rangePartitionsSpec, NO_SKIP_OFFSET, 1, null); forceTriggerAutoCompaction(2); verifyQuery(INDEX_QUERIES_RESOURCE); verifySegmentsCompacted(rangePartitionsSpec, 2); @@ -278,6 +282,37 @@ public void testAutoCompactionDutyCanUpdateTaskSlots() throws Exception } } + @Test + public void testAutoCompactionDutyWithSegmentGranularity() throws Exception + { + loadData(INDEX_TASK); + try (final Closeable ignored = unloader(fullDatasourceName)) { + final List intervalsBeforeCompaction = coordinator.getSegmentIntervals(fullDatasourceName); + intervalsBeforeCompaction.sort(null); + // 4 segments across 2 days (4 total)... + verifySegmentsCount(4); + verifyQuery(INDEX_QUERIES_RESOURCE); + + submitCompactionConfig(1000, NO_SKIP_OFFSET, new UniformGranularitySpec(Granularities.MONTH, null, null)); + + LOG.info("Auto compaction test with MONTH segment granularity"); + + forceTriggerAutoCompaction(1); + verifyQuery(INDEX_QUERIES_RESOURCE); + verifySegmentsCompacted(1, 1000); + checkCompactionIntervals(intervalsBeforeCompaction); + + LOG.info("Auto compaction test with DAY segment granularity"); + + submitCompactionConfig(1000, NO_SKIP_OFFSET, new UniformGranularitySpec(Granularities.DAY, null, null)); + // 2 segments published per day after compaction. + forceTriggerAutoCompaction(2); + verifyQuery(INDEX_QUERIES_RESOURCE); + verifySegmentsCompacted(2, 1000); + checkCompactionIntervals(intervalsBeforeCompaction); + } + } + private void loadData(String indexTask) throws Exception { String taskSpec = getResourceAsString(indexTask); @@ -314,13 +349,19 @@ private void verifyQuery(String queryResource) throws Exception private void submitCompactionConfig(Integer maxRowsPerSegment, Period skipOffsetFromLatest) throws Exception { - submitCompactionConfig(new DynamicPartitionsSpec(maxRowsPerSegment, null), skipOffsetFromLatest, 1); + submitCompactionConfig(maxRowsPerSegment, skipOffsetFromLatest,null); + } + + private void submitCompactionConfig(Integer maxRowsPerSegment, Period skipOffsetFromLatest, GranularitySpec granularitySpec) throws Exception + { + submitCompactionConfig(new DynamicPartitionsSpec(maxRowsPerSegment, null), skipOffsetFromLatest, 1, granularitySpec); } private void submitCompactionConfig( PartitionsSpec partitionsSpec, Period skipOffsetFromLatest, - int maxNumConcurrentSubTasks + int maxNumConcurrentSubTasks, + GranularitySpec granularitySpec ) throws Exception { DataSourceCompactionConfig compactionConfig = new DataSourceCompactionConfig( @@ -348,7 +389,7 @@ private void submitCompactionConfig( null, 1 ), - null, + granularitySpec, null ); compactionResource.submitCompactionConfig(compactionConfig); diff --git a/integration-tests/src/test/java/org/apache/druid/tests/indexer/ITCompactionTaskTest.java b/integration-tests/src/test/java/org/apache/druid/tests/indexer/ITCompactionTaskTest.java index ed7b44e984ed..f82f07cc2ec8 100644 --- a/integration-tests/src/test/java/org/apache/druid/tests/indexer/ITCompactionTaskTest.java +++ b/integration-tests/src/test/java/org/apache/druid/tests/indexer/ITCompactionTaskTest.java @@ -49,6 +49,8 @@ public class ITCompactionTaskTest extends AbstractIndexerTest private static final String INDEX_DATASOURCE = "wikipedia_index_test"; private static final String COMPACTION_TASK = "/indexer/wikipedia_compaction_task.json"; + private static final String COMPACTION_TASK_WITH_SEGMENT_GRANULARITY = "/indexer/wikipedia_compaction_task_with_segment_granularity.json"; + private static final String COMPACTION_TASK_WITH_GRANULARITY_SPEC = "/indexer/wikipedia_compaction_task_with_granularity_spec.json"; private static final String INDEX_TASK_WITH_TIMESTAMP = "/indexer/wikipedia_with_timestamp_index_task.json"; @@ -66,16 +68,28 @@ public void setFullDatasourceName(Method method) @Test public void testCompaction() throws Exception { - loadDataAndCompact(INDEX_TASK, INDEX_QUERIES_RESOURCE); + loadDataAndCompact(INDEX_TASK, INDEX_QUERIES_RESOURCE, COMPACTION_TASK); + } + + @Test + public void testCompactionWithSegmentGranularity() throws Exception + { + loadDataAndCompact(INDEX_TASK, INDEX_QUERIES_RESOURCE, COMPACTION_TASK_WITH_SEGMENT_GRANULARITY); + } + + @Test + public void testCompactionWithGranularitySpec() throws Exception + { + loadDataAndCompact(INDEX_TASK, INDEX_QUERIES_RESOURCE, COMPACTION_TASK_WITH_GRANULARITY_SPEC); } @Test public void testCompactionWithTimestampDimension() throws Exception { - loadDataAndCompact(INDEX_TASK_WITH_TIMESTAMP, INDEX_QUERIES_RESOURCE); + loadDataAndCompact(INDEX_TASK_WITH_TIMESTAMP, INDEX_QUERIES_RESOURCE, COMPACTION_TASK); } - private void loadDataAndCompact(String indexTask, String queriesResource) throws Exception + private void loadDataAndCompact(String indexTask, String queriesResource, String compactionResource) throws Exception { loadData(indexTask); @@ -102,7 +116,7 @@ private void loadDataAndCompact(String indexTask, String queriesResource) throws queryHelper.testQueriesFromString(queryResponseTemplate); - compactData(); + compactData(compactionResource); // The original 4 segments should be compacted into 2 new segments checkNumberOfSegments(2); @@ -124,9 +138,9 @@ private void loadData(String indexTask) throws Exception ); } - private void compactData() throws Exception + private void compactData(String compactionResource) throws Exception { - final String template = getResourceAsString(COMPACTION_TASK); + final String template = getResourceAsString(compactionResource); String taskSpec = StringUtils.replace(template, "%%DATASOURCE%%", fullDatasourceName); final String taskID = indexer.submitTask(taskSpec); diff --git a/integration-tests/src/test/resources/indexer/wikipedia_compaction_task_with_granularity_spec.json b/integration-tests/src/test/resources/indexer/wikipedia_compaction_task_with_granularity_spec.json new file mode 100644 index 000000000000..2a24dd6709e5 --- /dev/null +++ b/integration-tests/src/test/resources/indexer/wikipedia_compaction_task_with_granularity_spec.json @@ -0,0 +1,17 @@ +{ + "type" : "compact", + "dataSource" : "%%DATASOURCE%%", + "ioConfig" : { + "type": "compact", + "inputSpec": { + "type": "interval", + "interval": "2013-08-31/2013-09-02" + } + }, + "granularitySpec": { + "segmentGranularity": "MONTH" + }, + "context" : { + "storeCompactionState" : true + } +} \ No newline at end of file diff --git a/integration-tests/src/test/resources/indexer/wikipedia_compaction_task_with_segment_granularity.json b/integration-tests/src/test/resources/indexer/wikipedia_compaction_task_with_segment_granularity.json new file mode 100644 index 000000000000..3fa7f801b61f --- /dev/null +++ b/integration-tests/src/test/resources/indexer/wikipedia_compaction_task_with_segment_granularity.json @@ -0,0 +1,15 @@ +{ + "type" : "compact", + "dataSource" : "%%DATASOURCE%%", + "ioConfig" : { + "type": "compact", + "inputSpec": { + "type": "interval", + "interval": "2013-08-31/2013-09-02" + } + }, + "segmentGranularity": "MONTH", + "context" : { + "storeCompactionState" : true + } +} \ No newline at end of file From 020566a889f9241391d89be58e9361fab77e3e04 Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Fri, 5 Feb 2021 03:23:38 -0800 Subject: [PATCH 17/31] fix checkstyle --- .../druid/tests/coordinator/duty/ITAutoCompactionTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java b/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java index d68ad8d5640a..8ed54642a532 100644 --- a/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java +++ b/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java @@ -349,7 +349,7 @@ private void verifyQuery(String queryResource) throws Exception private void submitCompactionConfig(Integer maxRowsPerSegment, Period skipOffsetFromLatest) throws Exception { - submitCompactionConfig(maxRowsPerSegment, skipOffsetFromLatest,null); + submitCompactionConfig(maxRowsPerSegment, skipOffsetFromLatest, null); } private void submitCompactionConfig(Integer maxRowsPerSegment, Period skipOffsetFromLatest, GranularitySpec granularitySpec) throws Exception From 4f031b6ef84e330ecbd8c493131f4f875bba3698 Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Fri, 5 Feb 2021 03:24:21 -0800 Subject: [PATCH 18/31] fix checkstyle --- .../druid/tests/coordinator/duty/ITAutoCompactionTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java b/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java index 8ed54642a532..0749e70abdda 100644 --- a/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java +++ b/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java @@ -29,7 +29,6 @@ import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.java.util.common.granularity.Granularities; -import org.apache.druid.java.util.common.granularity.Granularity; import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.segment.indexing.granularity.GranularitySpec; import org.apache.druid.segment.indexing.granularity.UniformGranularitySpec; From 2dfdcd4bbfa022798ba64e83445358cb9b03a94f Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Fri, 5 Feb 2021 11:51:18 -0800 Subject: [PATCH 19/31] fix failing tests --- .travis.yml | 2 +- .../segment/indexing/granularity/ArbitraryGranularityTest.java | 2 +- .../segment/indexing/granularity/UniformGranularityTest.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index ffb6c1dbd996..8d4fd9ac1428 100644 --- a/.travis.yml +++ b/.travis.yml @@ -133,7 +133,7 @@ jobs: install: skip before_script: *setup_generate_license script: > - MAVEN_OPTS='-Xmx3000m' ${MVN} clean install -Prat -Pdist -Pbundle-contrib-exts --fail-at-end + MAVEN_OPTS='-Xmx5000m' ${MVN} clean install -Prat -Pdist -Pbundle-contrib-exts --fail-at-end -pl '!benchmarks' ${MAVEN_SKIP} ${MAVEN_SKIP_TESTS} -Ddruid.console.skip=false -T1C - <<: *package diff --git a/server/src/test/java/org/apache/druid/segment/indexing/granularity/ArbitraryGranularityTest.java b/server/src/test/java/org/apache/druid/segment/indexing/granularity/ArbitraryGranularityTest.java index 5d3b460d6304..25bf848d8b1c 100644 --- a/server/src/test/java/org/apache/druid/segment/indexing/granularity/ArbitraryGranularityTest.java +++ b/server/src/test/java/org/apache/druid/segment/indexing/granularity/ArbitraryGranularityTest.java @@ -219,7 +219,7 @@ public void testJson() } @Test - public void testAsMap() throws Exception + public void testAsMap() { final GranularitySpec spec = new ArbitraryGranularitySpec(Granularities.NONE, Lists.newArrayList( Intervals.of("2012-01-08T00Z/2012-01-11T00Z"), diff --git a/server/src/test/java/org/apache/druid/segment/indexing/granularity/UniformGranularityTest.java b/server/src/test/java/org/apache/druid/segment/indexing/granularity/UniformGranularityTest.java index d39f36b7a8a4..0c19724883e4 100644 --- a/server/src/test/java/org/apache/druid/segment/indexing/granularity/UniformGranularityTest.java +++ b/server/src/test/java/org/apache/druid/segment/indexing/granularity/UniformGranularityTest.java @@ -180,7 +180,7 @@ public void testJson() } @Test - public void testAsMap() throws Exception + public void testAsMap() { final GranularitySpec spec = new UniformGranularitySpec( Granularities.DAY, From ad11616116d04f6c4b4c72ba19370a42057c04c3 Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Fri, 5 Feb 2021 19:04:29 -0800 Subject: [PATCH 20/31] address comments --- .../granularity/ArbitraryGranularitySpec.java | 9 --------- .../indexing/granularity/BaseGranularitySpec.java | 12 ++++++++++++ .../indexing/granularity/UniformGranularitySpec.java | 9 --------- 3 files changed, 12 insertions(+), 18 deletions(-) diff --git a/server/src/main/java/org/apache/druid/segment/indexing/granularity/ArbitraryGranularitySpec.java b/server/src/main/java/org/apache/druid/segment/indexing/granularity/ArbitraryGranularitySpec.java index b81d5554042f..05994fed29d0 100644 --- a/server/src/main/java/org/apache/druid/segment/indexing/granularity/ArbitraryGranularitySpec.java +++ b/server/src/main/java/org/apache/druid/segment/indexing/granularity/ArbitraryGranularitySpec.java @@ -141,15 +141,6 @@ public GranularitySpec withIntervals(List inputIntervals) return new ArbitraryGranularitySpec(queryGranularity, rollup, inputIntervals); } - @Override - public Map asMap(ObjectMapper objectMapper) - { - return objectMapper.convertValue( - this, - new TypeReference>() {} - ); - } - @Override protected LookupIntervalBuckets getLookupTableBuckets() { diff --git a/server/src/main/java/org/apache/druid/segment/indexing/granularity/BaseGranularitySpec.java b/server/src/main/java/org/apache/druid/segment/indexing/granularity/BaseGranularitySpec.java index cc617b0a9e70..779952b57793 100644 --- a/server/src/main/java/org/apache/druid/segment/indexing/granularity/BaseGranularitySpec.java +++ b/server/src/main/java/org/apache/druid/segment/indexing/granularity/BaseGranularitySpec.java @@ -20,6 +20,8 @@ package org.apache.druid.segment.indexing.granularity; import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.base.Optional; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterators; @@ -33,6 +35,7 @@ import java.util.Collections; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.TreeSet; public abstract class BaseGranularitySpec implements GranularitySpec @@ -82,6 +85,15 @@ public TreeSet materializedBucketIntervals() protected abstract LookupIntervalBuckets getLookupTableBuckets(); + @Override + public Map asMap(ObjectMapper objectMapper) + { + return objectMapper.convertValue( + this, + new TypeReference>() {} + ); + } + /** * This is a helper class to facilitate sharing the code for sortedBucketIntervals among * the various GranularitySpec implementations. In particular, the UniformGranularitySpec diff --git a/server/src/main/java/org/apache/druid/segment/indexing/granularity/UniformGranularitySpec.java b/server/src/main/java/org/apache/druid/segment/indexing/granularity/UniformGranularitySpec.java index b9d5653b7fb8..3554df5ad85d 100644 --- a/server/src/main/java/org/apache/druid/segment/indexing/granularity/UniformGranularitySpec.java +++ b/server/src/main/java/org/apache/druid/segment/indexing/granularity/UniformGranularitySpec.java @@ -138,15 +138,6 @@ public GranularitySpec withIntervals(List inputIntervals) return new UniformGranularitySpec(segmentGranularity, queryGranularity, rollup, inputIntervals); } - @Override - public Map asMap(ObjectMapper objectMapper) - { - return objectMapper.convertValue( - this, - new TypeReference>() {} - ); - } - @Override protected LookupIntervalBuckets getLookupTableBuckets() { From 94ee53248e4bc625e60a650024da871ea5c682b8 Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Fri, 5 Feb 2021 19:05:24 -0800 Subject: [PATCH 21/31] address comments --- .../segment/indexing/granularity/ArbitraryGranularitySpec.java | 3 --- .../segment/indexing/granularity/UniformGranularitySpec.java | 3 --- 2 files changed, 6 deletions(-) diff --git a/server/src/main/java/org/apache/druid/segment/indexing/granularity/ArbitraryGranularitySpec.java b/server/src/main/java/org/apache/druid/segment/indexing/granularity/ArbitraryGranularitySpec.java index 05994fed29d0..8bf6396bf3ea 100644 --- a/server/src/main/java/org/apache/druid/segment/indexing/granularity/ArbitraryGranularitySpec.java +++ b/server/src/main/java/org/apache/druid/segment/indexing/granularity/ArbitraryGranularitySpec.java @@ -21,8 +21,6 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; -import com.fasterxml.jackson.core.type.TypeReference; -import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.Iterators; import com.google.common.collect.PeekingIterator; import org.apache.druid.java.util.common.IAE; @@ -32,7 +30,6 @@ import javax.annotation.Nullable; import java.util.List; -import java.util.Map; public class ArbitraryGranularitySpec extends BaseGranularitySpec { diff --git a/server/src/main/java/org/apache/druid/segment/indexing/granularity/UniformGranularitySpec.java b/server/src/main/java/org/apache/druid/segment/indexing/granularity/UniformGranularitySpec.java index 3554df5ad85d..cd7c7a4d0d74 100644 --- a/server/src/main/java/org/apache/druid/segment/indexing/granularity/UniformGranularitySpec.java +++ b/server/src/main/java/org/apache/druid/segment/indexing/granularity/UniformGranularitySpec.java @@ -21,14 +21,11 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; -import com.fasterxml.jackson.core.type.TypeReference; -import com.fasterxml.jackson.databind.ObjectMapper; import org.apache.druid.java.util.common.granularity.Granularity; import org.apache.druid.java.util.common.granularity.IntervalsByGranularity; import org.joda.time.Interval; import java.util.List; -import java.util.Map; public class UniformGranularitySpec extends BaseGranularitySpec { From 343a07b527340cdeda1b5603478d755f811bc06d Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Wed, 10 Feb 2021 02:51:33 -0800 Subject: [PATCH 22/31] fix tests --- .travis.yml | 2 +- .../duty/ITAutoCompactionTest.java | 12 ++--- .../tests/indexer/ITCompactionTaskTest.java | 50 +++++++++++++++---- ...compaction_task_with_granularity_spec.json | 2 +- ...paction_task_with_segment_granularity.json | 2 +- .../duty/NewestSegmentFirstIterator.java | 7 +-- 6 files changed, 53 insertions(+), 22 deletions(-) diff --git a/.travis.yml b/.travis.yml index 8d4fd9ac1428..ffb6c1dbd996 100644 --- a/.travis.yml +++ b/.travis.yml @@ -133,7 +133,7 @@ jobs: install: skip before_script: *setup_generate_license script: > - MAVEN_OPTS='-Xmx5000m' ${MVN} clean install -Prat -Pdist -Pbundle-contrib-exts --fail-at-end + MAVEN_OPTS='-Xmx3000m' ${MVN} clean install -Prat -Pdist -Pbundle-contrib-exts --fail-at-end -pl '!benchmarks' ${MAVEN_SKIP} ${MAVEN_SKIP_TESTS} -Ddruid.console.skip=false -T1C - <<: *package diff --git a/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java b/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java index 0749e70abdda..82006328fb18 100644 --- a/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java +++ b/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java @@ -296,18 +296,18 @@ public void testAutoCompactionDutyWithSegmentGranularity() throws Exception LOG.info("Auto compaction test with MONTH segment granularity"); - forceTriggerAutoCompaction(1); + forceTriggerAutoCompaction(2); verifyQuery(INDEX_QUERIES_RESOURCE); - verifySegmentsCompacted(1, 1000); + verifySegmentsCompacted(2, 1000); checkCompactionIntervals(intervalsBeforeCompaction); - LOG.info("Auto compaction test with DAY segment granularity"); + LOG.info("Auto compaction test with HOUR segment granularity"); - submitCompactionConfig(1000, NO_SKIP_OFFSET, new UniformGranularitySpec(Granularities.DAY, null, null)); + submitCompactionConfig(1000, NO_SKIP_OFFSET, new UniformGranularitySpec(Granularities.HOUR, null, null)); // 2 segments published per day after compaction. - forceTriggerAutoCompaction(2); + forceTriggerAutoCompaction(48); verifyQuery(INDEX_QUERIES_RESOURCE); - verifySegmentsCompacted(2, 1000); + verifySegmentsCompacted(48, 1000); checkCompactionIntervals(intervalsBeforeCompaction); } } diff --git a/integration-tests/src/test/java/org/apache/druid/tests/indexer/ITCompactionTaskTest.java b/integration-tests/src/test/java/org/apache/druid/tests/indexer/ITCompactionTaskTest.java index f82f07cc2ec8..2f5aa6963544 100644 --- a/integration-tests/src/test/java/org/apache/druid/tests/indexer/ITCompactionTaskTest.java +++ b/integration-tests/src/test/java/org/apache/druid/tests/indexer/ITCompactionTaskTest.java @@ -23,11 +23,16 @@ import org.apache.commons.io.IOUtils; import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.StringUtils; +import org.apache.druid.java.util.common.granularity.Granularities; +import org.apache.druid.java.util.common.granularity.Granularity; +import org.apache.druid.java.util.common.granularity.PeriodGranularity; import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.testing.IntegrationTestingConfig; import org.apache.druid.testing.guice.DruidTestModuleFactory; import org.apache.druid.testing.utils.ITRetryUtil; import org.apache.druid.tests.TestNGGroup; +import org.joda.time.DateTime; +import org.joda.time.Interval; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Guice; import org.testng.annotations.Test; @@ -37,6 +42,7 @@ import java.io.InputStream; import java.lang.reflect.Method; import java.nio.charset.StandardCharsets; +import java.util.ArrayList; import java.util.List; @Test(groups = {TestNGGroup.COMPACTION, TestNGGroup.QUICKSTART_COMPATIBLE}) @@ -68,36 +74,41 @@ public void setFullDatasourceName(Method method) @Test public void testCompaction() throws Exception { - loadDataAndCompact(INDEX_TASK, INDEX_QUERIES_RESOURCE, COMPACTION_TASK); + loadDataAndCompact(INDEX_TASK, INDEX_QUERIES_RESOURCE, COMPACTION_TASK, null); } @Test public void testCompactionWithSegmentGranularity() throws Exception { - loadDataAndCompact(INDEX_TASK, INDEX_QUERIES_RESOURCE, COMPACTION_TASK_WITH_SEGMENT_GRANULARITY); + loadDataAndCompact(INDEX_TASK, INDEX_QUERIES_RESOURCE, COMPACTION_TASK_WITH_SEGMENT_GRANULARITY, Granularities.MONTH); } @Test public void testCompactionWithGranularitySpec() throws Exception { - loadDataAndCompact(INDEX_TASK, INDEX_QUERIES_RESOURCE, COMPACTION_TASK_WITH_GRANULARITY_SPEC); + loadDataAndCompact(INDEX_TASK, INDEX_QUERIES_RESOURCE, COMPACTION_TASK_WITH_GRANULARITY_SPEC, Granularities.MONTH); } @Test public void testCompactionWithTimestampDimension() throws Exception { - loadDataAndCompact(INDEX_TASK_WITH_TIMESTAMP, INDEX_QUERIES_RESOURCE, COMPACTION_TASK); + loadDataAndCompact(INDEX_TASK_WITH_TIMESTAMP, INDEX_QUERIES_RESOURCE, COMPACTION_TASK, null); } - private void loadDataAndCompact(String indexTask, String queriesResource, String compactionResource) throws Exception + private void loadDataAndCompact( + String indexTask, + String queriesResource, + String compactionResource, + Granularity newSegmentGranularity + ) throws Exception { loadData(indexTask); // 4 segments across 2 days checkNumberOfSegments(4); - final List intervalsBeforeCompaction = coordinator.getSegmentIntervals(fullDatasourceName); - intervalsBeforeCompaction.sort(null); + List expectedIntervalAfterCompaction = coordinator.getSegmentIntervals(fullDatasourceName); + expectedIntervalAfterCompaction.sort(null); try (final Closeable ignored = unloader(fullDatasourceName)) { String queryResponseTemplate; try { @@ -116,12 +127,24 @@ private void loadDataAndCompact(String indexTask, String queriesResource, String queryHelper.testQueriesFromString(queryResponseTemplate); - compactData(compactionResource); + compactData(compactionResource, newSegmentGranularity); // The original 4 segments should be compacted into 2 new segments checkNumberOfSegments(2); queryHelper.testQueriesFromString(queryResponseTemplate); - checkCompactionIntervals(intervalsBeforeCompaction); + + + if (newSegmentGranularity != null) { + List newIntervals = new ArrayList<>(); + for (String interval : expectedIntervalAfterCompaction) { + for (Interval newinterval : newSegmentGranularity.getIterable(new Interval(interval))) + { + newIntervals.add(newinterval.toString()); + } + } + expectedIntervalAfterCompaction = newIntervals; + } + checkCompactionIntervals(expectedIntervalAfterCompaction); } } private void loadData(String indexTask) throws Exception @@ -138,10 +161,17 @@ private void loadData(String indexTask) throws Exception ); } - private void compactData(String compactionResource) throws Exception + private void compactData(String compactionResource, Granularity newSegmentGranularity) throws Exception { final String template = getResourceAsString(compactionResource); String taskSpec = StringUtils.replace(template, "%%DATASOURCE%%", fullDatasourceName); + if (newSegmentGranularity != null) { + taskSpec = StringUtils.replace( + template, + "%%SEGMENTGRANULARITY%%", + ((PeriodGranularity) Granularities.MONTH).getPeriod().toString() + ); + } final String taskID = indexer.submitTask(taskSpec); LOG.info("TaskID for compaction task %s", taskID); diff --git a/integration-tests/src/test/resources/indexer/wikipedia_compaction_task_with_granularity_spec.json b/integration-tests/src/test/resources/indexer/wikipedia_compaction_task_with_granularity_spec.json index 2a24dd6709e5..828579b6e46b 100644 --- a/integration-tests/src/test/resources/indexer/wikipedia_compaction_task_with_granularity_spec.json +++ b/integration-tests/src/test/resources/indexer/wikipedia_compaction_task_with_granularity_spec.json @@ -9,7 +9,7 @@ } }, "granularitySpec": { - "segmentGranularity": "MONTH" + "segmentGranularity": "%%SEGMENTGRANULARITY%%" }, "context" : { "storeCompactionState" : true diff --git a/integration-tests/src/test/resources/indexer/wikipedia_compaction_task_with_segment_granularity.json b/integration-tests/src/test/resources/indexer/wikipedia_compaction_task_with_segment_granularity.json index 3fa7f801b61f..254926f224c5 100644 --- a/integration-tests/src/test/resources/indexer/wikipedia_compaction_task_with_segment_granularity.json +++ b/integration-tests/src/test/resources/indexer/wikipedia_compaction_task_with_segment_granularity.json @@ -8,7 +8,7 @@ "interval": "2013-08-31/2013-09-02" } }, - "segmentGranularity": "MONTH", + "segmentGranularity": "%%SEGMENTGRANULARITY%%", "context" : { "storeCompactionState" : true } diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIterator.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIterator.java index 98c1440e1d4d..90bfb664faa2 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIterator.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIterator.java @@ -86,8 +86,9 @@ public class NewestSegmentFirstIterator implements CompactionSegmentIterator // compact. private final Map timelineIterators; // 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 + // If configured segmentGranularity in config is finer than current segmentGranularity, the same set of segments + // can belong to multiple intervals in the timeline. We keep track of the compacted intervals between each + // run of the compaction job and skip any interval that was already previously compacted. private final Map> intervalCompactedForDatasource = new HashMap<>(); private final PriorityQueue queue = new PriorityQueue<>( @@ -132,7 +133,7 @@ public class NewestSegmentFirstIterator implements CompactionSegmentIterator 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 + // PartitionHolder can only holds chunks of one partition space // However, partition in the new timeline (timelineWithConfiguredSegmentGranularity) can be hold multiple // partitions of the original timeline (when the new segmentGranularity is larger than the original // segmentGranularity). Hence, we group all the segments of the original timeline into intervals bucket From a506287588dabc6c4dc5750f7ebd2db2d3f3d97d Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Wed, 10 Feb 2021 02:52:55 -0800 Subject: [PATCH 23/31] fix tests --- .../org/apache/druid/tests/indexer/ITCompactionTaskTest.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/integration-tests/src/test/java/org/apache/druid/tests/indexer/ITCompactionTaskTest.java b/integration-tests/src/test/java/org/apache/druid/tests/indexer/ITCompactionTaskTest.java index 2f5aa6963544..970ae4d265fa 100644 --- a/integration-tests/src/test/java/org/apache/druid/tests/indexer/ITCompactionTaskTest.java +++ b/integration-tests/src/test/java/org/apache/druid/tests/indexer/ITCompactionTaskTest.java @@ -31,7 +31,6 @@ import org.apache.druid.testing.guice.DruidTestModuleFactory; import org.apache.druid.testing.utils.ITRetryUtil; import org.apache.druid.tests.TestNGGroup; -import org.joda.time.DateTime; import org.joda.time.Interval; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Guice; @@ -137,8 +136,7 @@ private void loadDataAndCompact( if (newSegmentGranularity != null) { List newIntervals = new ArrayList<>(); for (String interval : expectedIntervalAfterCompaction) { - for (Interval newinterval : newSegmentGranularity.getIterable(new Interval(interval))) - { + for (Interval newinterval : newSegmentGranularity.getIterable(new Interval(interval))) { newIntervals.add(newinterval.toString()); } } From da018dc620ae670af46504767696ac39026e745d Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Wed, 10 Feb 2021 03:28:52 -0800 Subject: [PATCH 24/31] fix test --- .../service-supervisords/zookeeper.conf | 2 +- .../duty/ITAutoCompactionTest.java | 16 ++++++------- .../tests/indexer/ITCompactionTaskTest.java | 24 +++++++++---------- 3 files changed, 20 insertions(+), 22 deletions(-) diff --git a/integration-tests/docker/service-supervisords/zookeeper.conf b/integration-tests/docker/service-supervisords/zookeeper.conf index 6039e2630484..99fcd880617c 100644 --- a/integration-tests/docker/service-supervisords/zookeeper.conf +++ b/integration-tests/docker/service-supervisords/zookeeper.conf @@ -1,5 +1,5 @@ [program:zookeeper] -command=/usr/local/zookeeper-%(ENV_ZK_VERSION)s/bin/zkServer.sh start-foreground +command=/usr/local/zookeeper-3.5/bin/zkServer.sh start-foreground user=daemon priority=0 stdout_logfile=/shared/logs/zookeeper.log diff --git a/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java b/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java index 82006328fb18..b93d5a29b6c2 100644 --- a/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java +++ b/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java @@ -292,22 +292,22 @@ public void testAutoCompactionDutyWithSegmentGranularity() throws Exception verifySegmentsCount(4); verifyQuery(INDEX_QUERIES_RESOURCE); - submitCompactionConfig(1000, NO_SKIP_OFFSET, new UniformGranularitySpec(Granularities.MONTH, null, null)); + submitCompactionConfig(1000, NO_SKIP_OFFSET, new UniformGranularitySpec(Granularities.YEAR, null, null)); - LOG.info("Auto compaction test with MONTH segment granularity"); + LOG.info("Auto compaction test with YEAR segment granularity"); - forceTriggerAutoCompaction(2); + forceTriggerAutoCompaction(1); verifyQuery(INDEX_QUERIES_RESOURCE); - verifySegmentsCompacted(2, 1000); + verifySegmentsCompacted(1, 1000); checkCompactionIntervals(intervalsBeforeCompaction); - LOG.info("Auto compaction test with HOUR segment granularity"); + LOG.info("Auto compaction test with DAY segment granularity"); - submitCompactionConfig(1000, NO_SKIP_OFFSET, new UniformGranularitySpec(Granularities.HOUR, null, null)); + submitCompactionConfig(1000, NO_SKIP_OFFSET, new UniformGranularitySpec(Granularities.DAY, null, null)); // 2 segments published per day after compaction. - forceTriggerAutoCompaction(48); + forceTriggerAutoCompaction(2); verifyQuery(INDEX_QUERIES_RESOURCE); - verifySegmentsCompacted(48, 1000); + verifySegmentsCompacted(2, 1000); checkCompactionIntervals(intervalsBeforeCompaction); } } diff --git a/integration-tests/src/test/java/org/apache/druid/tests/indexer/ITCompactionTaskTest.java b/integration-tests/src/test/java/org/apache/druid/tests/indexer/ITCompactionTaskTest.java index 970ae4d265fa..3162c5e6287d 100644 --- a/integration-tests/src/test/java/org/apache/druid/tests/indexer/ITCompactionTaskTest.java +++ b/integration-tests/src/test/java/org/apache/druid/tests/indexer/ITCompactionTaskTest.java @@ -23,9 +23,7 @@ import org.apache.commons.io.IOUtils; import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.StringUtils; -import org.apache.druid.java.util.common.granularity.Granularities; -import org.apache.druid.java.util.common.granularity.Granularity; -import org.apache.druid.java.util.common.granularity.PeriodGranularity; +import org.apache.druid.java.util.common.granularity.GranularityType; import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.testing.IntegrationTestingConfig; import org.apache.druid.testing.guice.DruidTestModuleFactory; @@ -79,13 +77,13 @@ public void testCompaction() throws Exception @Test public void testCompactionWithSegmentGranularity() throws Exception { - loadDataAndCompact(INDEX_TASK, INDEX_QUERIES_RESOURCE, COMPACTION_TASK_WITH_SEGMENT_GRANULARITY, Granularities.MONTH); + loadDataAndCompact(INDEX_TASK, INDEX_QUERIES_RESOURCE, COMPACTION_TASK_WITH_SEGMENT_GRANULARITY, GranularityType.MONTH); } @Test public void testCompactionWithGranularitySpec() throws Exception { - loadDataAndCompact(INDEX_TASK, INDEX_QUERIES_RESOURCE, COMPACTION_TASK_WITH_GRANULARITY_SPEC, Granularities.MONTH); + loadDataAndCompact(INDEX_TASK, INDEX_QUERIES_RESOURCE, COMPACTION_TASK_WITH_GRANULARITY_SPEC, GranularityType.MONTH); } @Test @@ -98,7 +96,7 @@ private void loadDataAndCompact( String indexTask, String queriesResource, String compactionResource, - Granularity newSegmentGranularity + GranularityType newSegmentGranularity ) throws Exception { loadData(indexTask); @@ -136,7 +134,7 @@ private void loadDataAndCompact( if (newSegmentGranularity != null) { List newIntervals = new ArrayList<>(); for (String interval : expectedIntervalAfterCompaction) { - for (Interval newinterval : newSegmentGranularity.getIterable(new Interval(interval))) { + for (Interval newinterval : newSegmentGranularity.getDefaultGranularity().getIterable(new Interval(interval))) { newIntervals.add(newinterval.toString()); } } @@ -159,19 +157,19 @@ private void loadData(String indexTask) throws Exception ); } - private void compactData(String compactionResource, Granularity newSegmentGranularity) throws Exception + private void compactData(String compactionResource, GranularityType newSegmentGranularity) throws Exception { - final String template = getResourceAsString(compactionResource); - String taskSpec = StringUtils.replace(template, "%%DATASOURCE%%", fullDatasourceName); + String template = getResourceAsString(compactionResource); + template = StringUtils.replace(template, "%%DATASOURCE%%", fullDatasourceName); if (newSegmentGranularity != null) { - taskSpec = StringUtils.replace( + template = StringUtils.replace( template, "%%SEGMENTGRANULARITY%%", - ((PeriodGranularity) Granularities.MONTH).getPeriod().toString() + newSegmentGranularity.name() ); } - final String taskID = indexer.submitTask(taskSpec); + final String taskID = indexer.submitTask(template); LOG.info("TaskID for compaction task %s", taskID); indexer.waitUntilTaskCompletes(taskID); From 5dd9e5064ecd302fbebc873441ece23066c2f914 Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Wed, 10 Feb 2021 03:37:07 -0800 Subject: [PATCH 25/31] fix test --- integration-tests/docker/service-supervisords/zookeeper.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration-tests/docker/service-supervisords/zookeeper.conf b/integration-tests/docker/service-supervisords/zookeeper.conf index 99fcd880617c..6039e2630484 100644 --- a/integration-tests/docker/service-supervisords/zookeeper.conf +++ b/integration-tests/docker/service-supervisords/zookeeper.conf @@ -1,5 +1,5 @@ [program:zookeeper] -command=/usr/local/zookeeper-3.5/bin/zkServer.sh start-foreground +command=/usr/local/zookeeper-%(ENV_ZK_VERSION)s/bin/zkServer.sh start-foreground user=daemon priority=0 stdout_logfile=/shared/logs/zookeeper.log From d9783b43b41434e06ac04b428d98075bd40f046a Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Wed, 10 Feb 2021 03:59:33 -0800 Subject: [PATCH 26/31] fix test --- .../duty/ITAutoCompactionTest.java | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java b/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java index b93d5a29b6c2..22f2beb0467c 100644 --- a/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java +++ b/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java @@ -29,6 +29,8 @@ import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.java.util.common.granularity.Granularities; +import org.apache.druid.java.util.common.granularity.Granularity; +import org.apache.druid.java.util.common.granularity.GranularityType; import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.segment.indexing.granularity.GranularitySpec; import org.apache.druid.segment.indexing.granularity.UniformGranularitySpec; @@ -44,6 +46,7 @@ import org.apache.druid.tests.indexer.AbstractITBatchIndexTest; import org.apache.druid.tests.indexer.AbstractIndexerTest; import org.apache.druid.timeline.DataSegment; +import org.joda.time.Interval; import org.joda.time.Period; import org.testng.Assert; import org.testng.annotations.BeforeMethod; @@ -292,23 +295,38 @@ public void testAutoCompactionDutyWithSegmentGranularity() throws Exception verifySegmentsCount(4); verifyQuery(INDEX_QUERIES_RESOURCE); - submitCompactionConfig(1000, NO_SKIP_OFFSET, new UniformGranularitySpec(Granularities.YEAR, null, null)); + Granularity newGranularity = Granularities.YEAR; + submitCompactionConfig(1000, NO_SKIP_OFFSET, new UniformGranularitySpec(newGranularity, null, null)); LOG.info("Auto compaction test with YEAR segment granularity"); + List expectedIntervalAfterCompaction = new ArrayList<>(); + for (String interval : intervalsBeforeCompaction) { + for (Interval newinterval : newGranularity.getIterable(new Interval(interval))) { + expectedIntervalAfterCompaction.add(newinterval.toString()); + } + } forceTriggerAutoCompaction(1); verifyQuery(INDEX_QUERIES_RESOURCE); verifySegmentsCompacted(1, 1000); - checkCompactionIntervals(intervalsBeforeCompaction); + checkCompactionIntervals(expectedIntervalAfterCompaction); + + newGranularity = Granularities.DAY; + submitCompactionConfig(1000, NO_SKIP_OFFSET, new UniformGranularitySpec(newGranularity, null, null)); LOG.info("Auto compaction test with DAY segment granularity"); - submitCompactionConfig(1000, NO_SKIP_OFFSET, new UniformGranularitySpec(Granularities.DAY, null, null)); + expectedIntervalAfterCompaction = new ArrayList<>(); + for (String interval : intervalsBeforeCompaction) { + for (Interval newinterval : newGranularity.getIterable(new Interval(interval))) { + expectedIntervalAfterCompaction.add(newinterval.toString()); + } + } // 2 segments published per day after compaction. forceTriggerAutoCompaction(2); verifyQuery(INDEX_QUERIES_RESOURCE); verifySegmentsCompacted(2, 1000); - checkCompactionIntervals(intervalsBeforeCompaction); + checkCompactionIntervals(expectedIntervalAfterCompaction); } } From 4405fc6c71392b8aab1ed6186ee512f25f7f7e34 Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Wed, 10 Feb 2021 04:00:45 -0800 Subject: [PATCH 27/31] fix test --- .../druid/tests/coordinator/duty/ITAutoCompactionTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java b/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java index 22f2beb0467c..305711061193 100644 --- a/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java +++ b/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java @@ -30,7 +30,6 @@ import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.java.util.common.granularity.Granularities; import org.apache.druid.java.util.common.granularity.Granularity; -import org.apache.druid.java.util.common.granularity.GranularityType; import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.segment.indexing.granularity.GranularitySpec; import org.apache.druid.segment.indexing.granularity.UniformGranularitySpec; From 06060f9a175c5098b9d3e375afbbb4131f363a39 Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Wed, 10 Feb 2021 11:29:30 -0800 Subject: [PATCH 28/31] fix test --- .../druid/tests/coordinator/duty/ITAutoCompactionTest.java | 5 +++-- .../org/apache/druid/tests/indexer/ITCompactionTaskTest.java | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java b/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java index 305711061193..92dc4d41aab3 100644 --- a/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java +++ b/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java @@ -47,6 +47,7 @@ import org.apache.druid.timeline.DataSegment; import org.joda.time.Interval; import org.joda.time.Period; +import org.joda.time.chrono.ISOChronology; import org.testng.Assert; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Guice; @@ -301,7 +302,7 @@ public void testAutoCompactionDutyWithSegmentGranularity() throws Exception List expectedIntervalAfterCompaction = new ArrayList<>(); for (String interval : intervalsBeforeCompaction) { - for (Interval newinterval : newGranularity.getIterable(new Interval(interval))) { + for (Interval newinterval : newGranularity.getIterable(new Interval(interval, ISOChronology.getInstanceUTC()))) { expectedIntervalAfterCompaction.add(newinterval.toString()); } } @@ -317,7 +318,7 @@ public void testAutoCompactionDutyWithSegmentGranularity() throws Exception expectedIntervalAfterCompaction = new ArrayList<>(); for (String interval : intervalsBeforeCompaction) { - for (Interval newinterval : newGranularity.getIterable(new Interval(interval))) { + for (Interval newinterval : newGranularity.getIterable(new Interval(interval, ISOChronology.getInstanceUTC()))) { expectedIntervalAfterCompaction.add(newinterval.toString()); } } diff --git a/integration-tests/src/test/java/org/apache/druid/tests/indexer/ITCompactionTaskTest.java b/integration-tests/src/test/java/org/apache/druid/tests/indexer/ITCompactionTaskTest.java index 3162c5e6287d..a0cd4a5d61d0 100644 --- a/integration-tests/src/test/java/org/apache/druid/tests/indexer/ITCompactionTaskTest.java +++ b/integration-tests/src/test/java/org/apache/druid/tests/indexer/ITCompactionTaskTest.java @@ -30,6 +30,7 @@ import org.apache.druid.testing.utils.ITRetryUtil; import org.apache.druid.tests.TestNGGroup; import org.joda.time.Interval; +import org.joda.time.chrono.ISOChronology; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Guice; import org.testng.annotations.Test; @@ -134,7 +135,7 @@ private void loadDataAndCompact( if (newSegmentGranularity != null) { List newIntervals = new ArrayList<>(); for (String interval : expectedIntervalAfterCompaction) { - for (Interval newinterval : newSegmentGranularity.getDefaultGranularity().getIterable(new Interval(interval))) { + for (Interval newinterval : newSegmentGranularity.getDefaultGranularity().getIterable(new Interval(interval, ISOChronology.getInstanceUTC()))) { newIntervals.add(newinterval.toString()); } } From e6e90306d770cf18a2297cba949eb14acbb20fa5 Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Wed, 10 Feb 2021 14:18:03 -0800 Subject: [PATCH 29/31] fix test --- .../tests/coordinator/duty/ITAutoCompactionTest.java | 10 +++++++--- .../druid/tests/indexer/ITCompactionTaskTest.java | 12 +++++++----- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java b/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java index 92dc4d41aab3..a722b771fa75 100644 --- a/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java +++ b/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java @@ -58,8 +58,10 @@ import java.io.InputStream; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.UUID; @Test(groups = {TestNGGroup.COMPACTION}) @@ -474,11 +476,13 @@ private void verifySegmentsCount(int numExpectedSegments) private void checkCompactionIntervals(List expectedIntervals) { + Set expectedIntervalsSet = new HashSet<>(expectedIntervals); ITRetryUtil.retryUntilTrue( () -> { - final List actualIntervals = coordinator.getSegmentIntervals(fullDatasourceName); - actualIntervals.sort(null); - return actualIntervals.equals(expectedIntervals); + final Set actualIntervals = new HashSet<>(coordinator.getSegmentIntervals(fullDatasourceName)); + System.out.println("ACTUAL: " + actualIntervals); + System.out.println("EXPECTED: " + expectedIntervalsSet); + return actualIntervals.equals(expectedIntervalsSet); }, "Compaction interval check" ); diff --git a/integration-tests/src/test/java/org/apache/druid/tests/indexer/ITCompactionTaskTest.java b/integration-tests/src/test/java/org/apache/druid/tests/indexer/ITCompactionTaskTest.java index a0cd4a5d61d0..f4ee55952dde 100644 --- a/integration-tests/src/test/java/org/apache/druid/tests/indexer/ITCompactionTaskTest.java +++ b/integration-tests/src/test/java/org/apache/druid/tests/indexer/ITCompactionTaskTest.java @@ -41,7 +41,9 @@ import java.lang.reflect.Method; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; +import java.util.Set; @Test(groups = {TestNGGroup.COMPACTION, TestNGGroup.QUICKSTART_COMPATIBLE}) @Guice(moduleFactory = DruidTestModuleFactory.class) @@ -194,13 +196,13 @@ private void checkNumberOfSegments(int numExpectedSegments) private void checkCompactionIntervals(List expectedIntervals) { + Set expectedIntervalsSet = new HashSet<>(expectedIntervals); ITRetryUtil.retryUntilTrue( () -> { - final List intervalsAfterCompaction = coordinator.getSegmentIntervals(fullDatasourceName); - intervalsAfterCompaction.sort(null); - System.out.println("AFTER: " + intervalsAfterCompaction); - System.out.println("EXPECTED: " + expectedIntervals); - return intervalsAfterCompaction.equals(expectedIntervals); + final Set intervalsAfterCompaction = new HashSet<>(coordinator.getSegmentIntervals(fullDatasourceName)); + System.out.println("ACTUAL: " + intervalsAfterCompaction); + System.out.println("EXPECTED: " + expectedIntervalsSet); + return intervalsAfterCompaction.equals(expectedIntervalsSet); }, "Compaction interval check" ); From 578d433366340192cfc6ad29518905d70898caa6 Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Wed, 10 Feb 2021 17:38:32 -0800 Subject: [PATCH 30/31] fix test --- .../coordinator/duty/ITAutoCompactionTest.java | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java b/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java index a722b771fa75..62d75560f852 100644 --- a/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java +++ b/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java @@ -315,20 +315,6 @@ public void testAutoCompactionDutyWithSegmentGranularity() throws Exception newGranularity = Granularities.DAY; submitCompactionConfig(1000, NO_SKIP_OFFSET, new UniformGranularitySpec(newGranularity, null, null)); - - LOG.info("Auto compaction test with DAY segment granularity"); - - expectedIntervalAfterCompaction = new ArrayList<>(); - for (String interval : intervalsBeforeCompaction) { - for (Interval newinterval : newGranularity.getIterable(new Interval(interval, ISOChronology.getInstanceUTC()))) { - expectedIntervalAfterCompaction.add(newinterval.toString()); - } - } - // 2 segments published per day after compaction. - forceTriggerAutoCompaction(2); - verifyQuery(INDEX_QUERIES_RESOURCE); - verifySegmentsCompacted(2, 1000); - checkCompactionIntervals(expectedIntervalAfterCompaction); } } From 7784f70fd13a43d67abcd2029f4225429eb27130 Mon Sep 17 00:00:00 2001 From: Maytas Monsereenusorn Date: Wed, 10 Feb 2021 17:48:23 -0800 Subject: [PATCH 31/31] fix test --- .../coordinator/duty/ITAutoCompactionTest.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java b/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java index 62d75560f852..91bac02b5c97 100644 --- a/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java +++ b/integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionTest.java @@ -315,6 +315,23 @@ public void testAutoCompactionDutyWithSegmentGranularity() throws Exception newGranularity = Granularities.DAY; submitCompactionConfig(1000, NO_SKIP_OFFSET, new UniformGranularitySpec(newGranularity, null, null)); + + LOG.info("Auto compaction test with DAY segment granularity"); + + // The earlier segment with YEAR granularity is still 'used' as it’s not fully overshaowed. + // This is because we only have newer version on 2013-08-31 to 2013-09-01 and 2013-09-01 to 2013-09-02. + // The version for the YEAR segment is still the latest for 2013-01-01 to 2013-08-31 and 2013-09-02 to 2014-01-01. + // Hence, all three segments are available and the expected intervals are combined from the DAY and YEAR segment granularities + // (which are 2013-08-31 to 2013-09-01, 2013-09-01 to 2013-09-02 and 2013-01-01 to 2014-01-01) + for (String interval : intervalsBeforeCompaction) { + for (Interval newinterval : newGranularity.getIterable(new Interval(interval, ISOChronology.getInstanceUTC()))) { + expectedIntervalAfterCompaction.add(newinterval.toString()); + } + } + forceTriggerAutoCompaction(3); + verifyQuery(INDEX_QUERIES_RESOURCE); + verifySegmentsCompacted(3, 1000); + checkCompactionIntervals(expectedIntervalAfterCompaction); } }