From 76d281d64fe9615f022167f9d7cafbbe83a881ea Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Fri, 3 Dec 2021 14:15:05 -0800 Subject: [PATCH] Enable allocating segments at ALL granularity. (#12003) * Enable allocating segments at ALL granularity. The main change is that Granularity.granularitiesFinerThan will return ALL if ALL is passed in. Allocating segments at ALL granularity is somewhat unconventional, but there is nothing wrong with it, and it actually makes a lot of sense for tables that are meant to be used for lookups or dimensions rather than main fact tables. This change enables ALL segmentGranularity to work properly in appendToExisting mode. Also clarifies behavior in javadocs and tests. * Move tests to improve coverage. --- .../util/common/granularity/Granularity.java | 15 +-- .../java/util/common/GranularityTest.java | 69 +++++++++++++ .../actions/SegmentAllocateActionTest.java | 96 +++++++++---------- 3 files changed, 125 insertions(+), 55 deletions(-) diff --git a/core/src/main/java/org/apache/druid/java/util/common/granularity/Granularity.java b/core/src/main/java/org/apache/druid/java/util/common/granularity/Granularity.java index 31f4f8abefd..3d07d0c582e 100644 --- a/core/src/main/java/org/apache/druid/java/util/common/granularity/Granularity.java +++ b/core/src/main/java/org/apache/druid/java/util/common/granularity/Granularity.java @@ -101,18 +101,21 @@ public abstract class Granularity implements Cacheable return result; } + /** + * Returns a list of standard granularities that are equal to, or finer than, a provided granularity. + * + * 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. + */ public static List granularitiesFinerThan(final Granularity gran0) { final List retVal = new ArrayList<>(); final DateTime origin = (gran0 instanceof PeriodGranularity) ? ((PeriodGranularity) gran0).getOrigin() : null; final DateTimeZone tz = (gran0 instanceof PeriodGranularity) ? ((PeriodGranularity) gran0).getTimeZone() : null; for (GranularityType gran : GranularityType.values()) { - /** - * All and None are excluded b/c when asked to give all granularities finer - * than "TEN_MINUTE", you want the answer to be "FIVE_MINUTE, MINUTE and SECOND" - * it doesn't make sense to include ALL or None to be part of this. - */ - if (gran == GranularityType.ALL || gran == GranularityType.NONE) { + // Exclude ALL, unless we're looking for granularities finer than ALL; always exclude NONE. + if ((gran == GranularityType.ALL && !gran0.equals(Granularities.ALL)) || gran == GranularityType.NONE) { continue; } final Granularity segmentGranularity = gran.create(origin, tz); diff --git a/core/src/test/java/org/apache/druid/java/util/common/GranularityTest.java b/core/src/test/java/org/apache/druid/java/util/common/GranularityTest.java index c8a5fee5e50..bc8ac3396a0 100644 --- a/core/src/test/java/org/apache/druid/java/util/common/GranularityTest.java +++ b/core/src/test/java/org/apache/druid/java/util/common/GranularityTest.java @@ -880,6 +880,75 @@ public class GranularityTest Assert.assertTrue(Granularity.IS_FINER_THAN.compare(ALL, all) == 0); } + @Test + public void testGranularitiesFinerThanDay() + { + Assert.assertEquals( + ImmutableList.of( + Granularities.DAY, + Granularities.SIX_HOUR, + Granularities.HOUR, + Granularities.THIRTY_MINUTE, + Granularities.FIFTEEN_MINUTE, + Granularities.TEN_MINUTE, + Granularities.FIVE_MINUTE, + Granularities.MINUTE, + Granularities.SECOND + ), + Granularity.granularitiesFinerThan(Granularities.DAY) + ); + } + + @Test + public void testGranularitiesFinerThanHour() + { + Assert.assertEquals( + ImmutableList.of( + Granularities.HOUR, + Granularities.THIRTY_MINUTE, + Granularities.FIFTEEN_MINUTE, + Granularities.TEN_MINUTE, + Granularities.FIVE_MINUTE, + Granularities.MINUTE, + Granularities.SECOND + ), + Granularity.granularitiesFinerThan(Granularities.HOUR) + ); + } + + @Test + public void testGranularitiesFinerThanAll() + { + Assert.assertEquals( + ImmutableList.of( + Granularities.ALL, + Granularities.YEAR, + Granularities.QUARTER, + Granularities.MONTH, + Granularities.WEEK, + Granularities.DAY, + Granularities.SIX_HOUR, + Granularities.HOUR, + Granularities.THIRTY_MINUTE, + Granularities.FIFTEEN_MINUTE, + Granularities.TEN_MINUTE, + Granularities.FIVE_MINUTE, + Granularities.MINUTE, + Granularities.SECOND + ), + Granularity.granularitiesFinerThan(Granularities.ALL) + ); + } + + @Test + public void testGranularitiesFinerThanNone() + { + Assert.assertEquals( + ImmutableList.of(), + Granularity.granularitiesFinerThan(Granularities.NONE) + ); + } + private static class PathDate { public final String path; 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 672fa4846bf..bce550288d7 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 @@ -32,6 +32,7 @@ import org.apache.druid.indexing.common.task.NoopTask; import org.apache.druid.indexing.common.task.Task; 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.granularity.PeriodGranularity; @@ -102,42 +103,6 @@ public class SegmentAllocateActionTest EasyMock.replay(emitter); } - @Test - public void testGranularitiesFinerThanDay() - { - Assert.assertEquals( - ImmutableList.of( - Granularities.DAY, - Granularities.SIX_HOUR, - Granularities.HOUR, - Granularities.THIRTY_MINUTE, - Granularities.FIFTEEN_MINUTE, - Granularities.TEN_MINUTE, - Granularities.FIVE_MINUTE, - Granularities.MINUTE, - Granularities.SECOND - ), - Granularity.granularitiesFinerThan(Granularities.DAY) - ); - } - - @Test - public void testGranularitiesFinerThanHour() - { - Assert.assertEquals( - ImmutableList.of( - Granularities.HOUR, - Granularities.THIRTY_MINUTE, - Granularities.FIFTEEN_MINUTE, - Granularities.TEN_MINUTE, - Granularities.FIVE_MINUTE, - Granularities.MINUTE, - Granularities.SECOND - ), - Granularity.granularitiesFinerThan(Granularities.HOUR) - ); - } - @Test public void testManySegmentsSameInterval() { @@ -927,28 +892,61 @@ public class SegmentAllocateActionTest { final Task task = NoopTask.create(); taskActionTestKit.getTaskLockbox().add(task); - Granularity segmentGranularity = new PeriodGranularity(Period.hours(1), null, DateTimes.inferTzFromString("Asia/Shanghai")); + Granularity segmentGranularity = new PeriodGranularity( + Period.hours(1), + null, + DateTimes.inferTzFromString("Asia/Shanghai") + ); final SegmentIdWithShardSpec id1 = allocate( - task, - PARTY_TIME, - Granularities.MINUTE, - segmentGranularity, - "s1", - null + task, + PARTY_TIME, + Granularities.MINUTE, + segmentGranularity, + "s1", + null ); final SegmentIdWithShardSpec id2 = allocate( - task, - PARTY_TIME, - Granularities.MINUTE, - segmentGranularity, - "s2", - null + task, + PARTY_TIME, + Granularities.MINUTE, + segmentGranularity, + "s2", + null ); Assert.assertNotNull(id1); Assert.assertNotNull(id2); } + @Test + public void testAllocateAllGranularity() + { + final Task task = NoopTask.create(); + taskActionTestKit.getTaskLockbox().add(task); + + final SegmentIdWithShardSpec id1 = allocate( + task, + PARTY_TIME, + Granularities.MINUTE, + Granularities.ALL, + "s1", + null + ); + final SegmentIdWithShardSpec id2 = allocate( + task, + PARTY_TIME, + Granularities.MINUTE, + Granularities.ALL, + "s2", + null + ); + + Assert.assertNotNull(id1); + Assert.assertNotNull(id2); + Assert.assertEquals(Intervals.ETERNITY, id1.getInterval()); + Assert.assertEquals(Intervals.ETERNITY, id2.getInterval()); + } + private SegmentIdWithShardSpec allocate( final Task task, final DateTime timestamp,