From c41e99e10ccabf008bf4031a2fca98126c7ba3cc Mon Sep 17 00:00:00 2001 From: AmatyaAvadhanula Date: Fri, 5 Jan 2024 12:14:52 +0530 Subject: [PATCH] Do not allocate week granular segments unless requested (#15589) * Do not allocate week granular segments unless explicitly requested --- .../quickstart/tutorial/rollup-index.json | 2 +- .../quickstart/tutorial/transform-index.json | 2 +- .../tutorial/updates-append-index.json | 2 +- .../tutorial/updates-append-index2.json | 2 +- .../tutorial/updates-init-index.json | 2 +- .../tutorial/updates-overwrite-index.json | 2 +- .../common/actions/SegmentAllocateAction.java | 12 +++- .../actions/SegmentAllocateActionTest.java | 61 +++++++++++++++++++ .../util/common/granularity/Granularity.java | 5 ++ .../java/util/common/GranularityTest.java | 22 ++++++- 10 files changed, 102 insertions(+), 10 deletions(-) diff --git a/examples/quickstart/tutorial/rollup-index.json b/examples/quickstart/tutorial/rollup-index.json index 7c0b5815d2c..a978c2a76d2 100644 --- a/examples/quickstart/tutorial/rollup-index.json +++ b/examples/quickstart/tutorial/rollup-index.json @@ -20,7 +20,7 @@ ], "granularitySpec" : { "type" : "uniform", - "segmentGranularity" : "week", + "segmentGranularity" : "day", "queryGranularity" : "minute", "intervals" : ["2018-01-01/2018-01-03"], "rollup" : true diff --git a/examples/quickstart/tutorial/transform-index.json b/examples/quickstart/tutorial/transform-index.json index caebb9ff9c5..14c6b9ae6d4 100644 --- a/examples/quickstart/tutorial/transform-index.json +++ b/examples/quickstart/tutorial/transform-index.json @@ -20,7 +20,7 @@ ], "granularitySpec" : { "type" : "uniform", - "segmentGranularity" : "week", + "segmentGranularity" : "day", "queryGranularity" : "minute", "intervals" : ["2018-01-01/2018-01-03"], "rollup" : true diff --git a/examples/quickstart/tutorial/updates-append-index.json b/examples/quickstart/tutorial/updates-append-index.json index 9ba53a0b311..9cfb3b7d07a 100644 --- a/examples/quickstart/tutorial/updates-append-index.json +++ b/examples/quickstart/tutorial/updates-append-index.json @@ -18,7 +18,7 @@ ], "granularitySpec": { "type": "uniform", - "segmentGranularity": "week", + "segmentGranularity": "day", "queryGranularity": "minute", "intervals": ["2018-01-01/2018-01-03"], "rollup": true diff --git a/examples/quickstart/tutorial/updates-append-index2.json b/examples/quickstart/tutorial/updates-append-index2.json index 921b8cf0e2d..3f97ca31712 100644 --- a/examples/quickstart/tutorial/updates-append-index2.json +++ b/examples/quickstart/tutorial/updates-append-index2.json @@ -18,7 +18,7 @@ ], "granularitySpec" : { "type" : "uniform", - "segmentGranularity" : "week", + "segmentGranularity" : "day", "queryGranularity" : "minute", "intervals" : ["2018-01-01/2018-01-03"], "rollup" : true diff --git a/examples/quickstart/tutorial/updates-init-index.json b/examples/quickstart/tutorial/updates-init-index.json index ed4b349c6e0..52c0950e1d8 100644 --- a/examples/quickstart/tutorial/updates-init-index.json +++ b/examples/quickstart/tutorial/updates-init-index.json @@ -18,7 +18,7 @@ ], "granularitySpec" : { "type" : "uniform", - "segmentGranularity" : "week", + "segmentGranularity" : "day", "queryGranularity" : "minute", "intervals" : ["2018-01-01/2018-01-03"], "rollup" : true diff --git a/examples/quickstart/tutorial/updates-overwrite-index.json b/examples/quickstart/tutorial/updates-overwrite-index.json index b2545f04dd1..59e88bc9c6d 100644 --- a/examples/quickstart/tutorial/updates-overwrite-index.json +++ b/examples/quickstart/tutorial/updates-overwrite-index.json @@ -18,7 +18,7 @@ ], "granularitySpec" : { "type" : "uniform", - "segmentGranularity" : "week", + "segmentGranularity" : "day", "queryGranularity" : "minute", "intervals" : ["2018-01-01/2018-01-03"], "rollup" : true diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/common/actions/SegmentAllocateAction.java b/indexing-service/src/main/java/org/apache/druid/indexing/common/actions/SegmentAllocateAction.java index f0fae4a8617..280f4199e7b 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/common/actions/SegmentAllocateAction.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/common/actions/SegmentAllocateAction.java @@ -52,9 +52,13 @@ import java.util.concurrent.ThreadLocalRandom; import java.util.stream.Collectors; /** - * Allocates a pending segment for a given timestamp. The preferredSegmentGranularity is used if there are no prior - * segments for the given timestamp, or if the prior segments for the given timestamp are already at the - * preferredSegmentGranularity. Otherwise, the prior segments will take precedence. + * Allocates a pending segment for a given timestamp. + * If a visible chunk of used segments contains the interval with the query granularity containing the timestamp, + * the pending segment is allocated with its interval. + * Else, if the interval with the preferred segment granularity containing the timestamp has no overlap + * with the existing used segments, the preferred segment granularity is used. + * Else, find the coarsest segment granularity, containing the interval with the query granularity for the timestamp, + * that does not overlap with the existing used segments. This granularity is used for allocation if it exists. *

* This action implicitly acquires some task locks when it allocates segments. You do not have to acquire them * beforehand, although you *do* have to release them yourself. (Note that task locks are automatically released when @@ -62,6 +66,8 @@ import java.util.stream.Collectors; *

* If this action cannot acquire an appropriate task lock, or if it cannot expand an existing segment set, it returns * null. + *

+ * Do NOT allocate WEEK granularity segments unless the preferred segment granularity is WEEK. */ public class SegmentAllocateAction implements TaskAction { diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/common/actions/SegmentAllocateActionTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/common/actions/SegmentAllocateActionTest.java index 4ccb8707750..05760fd46ca 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/common/actions/SegmentAllocateActionTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/common/actions/SegmentAllocateActionTest.java @@ -59,6 +59,7 @@ import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import java.io.IOException; +import java.time.Duration; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -995,6 +996,66 @@ public class SegmentAllocateActionTest Assert.assertEquals(Intervals.ETERNITY, id2.getInterval()); } + @Test + public void testAllocateWeekOnlyWhenWeekIsPreferred() + { + final Task task = NoopTask.create(); + taskActionTestKit.getTaskLockbox().add(task); + + final SegmentIdWithShardSpec id1 = allocate( + task, + DateTimes.of("2023-12-16"), + Granularities.MINUTE, + Granularities.HOUR, + "s1", + null + ); + + final SegmentIdWithShardSpec id2 = allocate( + task, + DateTimes.of("2023-12-18"), + Granularities.MINUTE, + Granularities.WEEK, + "s2", + null + ); + + Assert.assertNotNull(id1); + Assert.assertNotNull(id2); + Assert.assertEquals(Duration.ofHours(1).toMillis(), id1.getInterval().toDurationMillis()); + Assert.assertEquals(Duration.ofDays(7).toMillis(), id2.getInterval().toDurationMillis()); + } + + @Test + public void testAllocateDayWhenMonthNotPossible() + { + final Task task = NoopTask.create(); + taskActionTestKit.getTaskLockbox().add(task); + + final SegmentIdWithShardSpec id1 = allocate( + task, + DateTimes.of("2023-12-16"), + Granularities.MINUTE, + Granularities.HOUR, + "s1", + null + ); + + final SegmentIdWithShardSpec id2 = allocate( + task, + DateTimes.of("2023-12-18"), + Granularities.MINUTE, + Granularities.MONTH, + "s2", + null + ); + + Assert.assertNotNull(id1); + Assert.assertNotNull(id2); + Assert.assertEquals(Duration.ofHours(1).toMillis(), id1.getInterval().toDurationMillis()); + Assert.assertEquals(Duration.ofDays(1).toMillis(), id2.getInterval().toDurationMillis()); + } + private SegmentIdWithShardSpec allocate( final Task task, final DateTime timestamp, diff --git a/processing/src/main/java/org/apache/druid/java/util/common/granularity/Granularity.java b/processing/src/main/java/org/apache/druid/java/util/common/granularity/Granularity.java index 572467f7c74..b841eb54193 100644 --- a/processing/src/main/java/org/apache/druid/java/util/common/granularity/Granularity.java +++ b/processing/src/main/java/org/apache/druid/java/util/common/granularity/Granularity.java @@ -106,6 +106,8 @@ public abstract class Granularity implements Cacheable * ALL will not be returned unless the provided granularity is ALL. NONE will never be returned, even if the * provided granularity is NONE. This is because the main usage of this function in production is segment * allocation, and we do not wish to generate NONE-granular segments. + * + * The list of granularities returned contains WEEK only if the requested granularity is WEEK. */ public static List granularitiesFinerThan(final Granularity gran0) { @@ -117,6 +119,9 @@ public abstract class Granularity implements Cacheable if ((gran == GranularityType.ALL && !gran0.equals(Granularities.ALL)) || gran == GranularityType.NONE) { continue; } + if (gran == GranularityType.WEEK && !gran0.equals(Granularities.WEEK)) { + continue; + } final Granularity segmentGranularity = gran.create(origin, tz); final long segmentGranularityDurationMillis = segmentGranularity.bucket(DateTimes.EPOCH).toDurationMillis(); final long gran0DurationMillis = gran0.bucket(DateTimes.EPOCH).toDurationMillis(); diff --git a/processing/src/test/java/org/apache/druid/java/util/common/GranularityTest.java b/processing/src/test/java/org/apache/druid/java/util/common/GranularityTest.java index 392a43149fb..f4106fe99d1 100644 --- a/processing/src/test/java/org/apache/druid/java/util/common/GranularityTest.java +++ b/processing/src/test/java/org/apache/druid/java/util/common/GranularityTest.java @@ -1002,6 +1002,27 @@ public class GranularityTest ); } + @Test + public void testGranularitiesFinerThanWeek() + { + Assert.assertEquals( + ImmutableList.of( + Granularities.WEEK, + Granularities.DAY, + Granularities.EIGHT_HOUR, + Granularities.SIX_HOUR, + Granularities.HOUR, + Granularities.THIRTY_MINUTE, + Granularities.FIFTEEN_MINUTE, + Granularities.TEN_MINUTE, + Granularities.FIVE_MINUTE, + Granularities.MINUTE, + Granularities.SECOND + ), + Granularity.granularitiesFinerThan(Granularities.WEEK) + ); + } + @Test public void testGranularitiesFinerThanAll() { @@ -1011,7 +1032,6 @@ public class GranularityTest Granularities.YEAR, Granularities.QUARTER, Granularities.MONTH, - Granularities.WEEK, Granularities.DAY, Granularities.EIGHT_HOUR, Granularities.SIX_HOUR,