From 6c5ac734633abc11d943c883c3b2eff5c7acf3a8 Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Wed, 14 Jun 2023 22:45:31 +0530 Subject: [PATCH 01/18] Allow empty tier for load rules --- .../coordinator/rules/ForeverLoadRule.java | 3 +- .../coordinator/rules/IntervalLoadRule.java | 3 +- .../server/coordinator/rules/LoadRule.java | 15 +++++-- .../coordinator/rules/PeriodLoadRule.java | 3 +- .../rules/ForeverLoadRuleTest.java | 25 +++-------- .../rules/IntervalLoadRuleTest.java | 44 +++++++++++-------- .../coordinator/rules/PeriodLoadRuleTest.java | 31 +++++++++++-- 7 files changed, 73 insertions(+), 51 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/ForeverLoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/ForeverLoadRule.java index ee5da0bdbb20..feda6c038059 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/ForeverLoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/ForeverLoadRule.java @@ -22,7 +22,6 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.collect.ImmutableMap; -import org.apache.druid.client.DruidServer; import org.apache.druid.timeline.DataSegment; import org.joda.time.DateTime; import org.joda.time.Interval; @@ -41,7 +40,7 @@ public ForeverLoadRule( @JsonProperty("tieredReplicants") Map tieredReplicants ) { - this.tieredReplicants = tieredReplicants == null ? ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS) : tieredReplicants; + this.tieredReplicants = tieredReplicants == null ? ImmutableMap.of() : tieredReplicants; validateTieredReplicants(this.tieredReplicants); } diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/IntervalLoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/IntervalLoadRule.java index 2e944bf28544..9518bee4ac23 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/IntervalLoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/IntervalLoadRule.java @@ -22,7 +22,6 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.collect.ImmutableMap; -import org.apache.druid.client.DruidServer; import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.timeline.DataSegment; import org.joda.time.DateTime; @@ -45,7 +44,7 @@ public IntervalLoadRule( @JsonProperty("tieredReplicants") Map tieredReplicants ) { - this.tieredReplicants = tieredReplicants == null ? ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS) : tieredReplicants; + this.tieredReplicants = tieredReplicants == null ? ImmutableMap.of() : tieredReplicants; validateTieredReplicants(this.tieredReplicants); this.interval = interval; } diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java index 933b63e47263..e3f62601c886 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java @@ -23,6 +23,7 @@ import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap; import it.unimi.dsi.fastutil.objects.Object2LongMap; import it.unimi.dsi.fastutil.objects.Object2LongOpenHashMap; +import org.apache.druid.client.DruidServer; import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.java.util.emitter.EmittingLogger; @@ -58,6 +59,7 @@ public abstract class LoadRule implements Rule static final String DROPPED_COUNT = "droppedCount"; public final String NON_PRIMARY_ASSIGNED_COUNT = "totalNonPrimaryReplicantsLoaded"; public static final String REQUIRED_CAPACITY = "requiredCapacity"; + public static final int DEFAULT_LOAD_RULE_NUM_REPLICANTS = 0; private final Object2IntMap targetReplicants = new Object2IntOpenHashMap<>(); private final Object2IntMap currentReplicants = new Object2IntOpenHashMap<>(); @@ -74,7 +76,12 @@ public CoordinatorStats run( { try { // get the "snapshots" of targetReplicants and currentReplicants for assignments. - targetReplicants.putAll(getTieredReplicants()); + Map tieredReplicants = getTieredReplicants(); + if (tieredReplicants.isEmpty()) { + targetReplicants.put(DruidServer.DEFAULT_TIER, DEFAULT_LOAD_RULE_NUM_REPLICANTS); + } else { + targetReplicants.putAll(getTieredReplicants()); + } currentReplicants.putAll(params.getSegmentReplicantLookup().getClusterTiers(segment.getId())); final CoordinatorStats stats = new CoordinatorStats(); @@ -545,15 +552,15 @@ private static int dropSegmentFromServers( protected static void validateTieredReplicants(final Map tieredReplicants) { - if (tieredReplicants.size() == 0) { - throw new IAE("A rule with empty tiered replicants is invalid"); + if (tieredReplicants == null || tieredReplicants.isEmpty()) { + return; } for (Map.Entry entry : tieredReplicants.entrySet()) { if (entry.getValue() == null) { throw new IAE("Replicant value cannot be empty"); } if (entry.getValue() < 0) { - throw new IAE("Replicant value [%d] is less than 0, which is not allowed", entry.getValue()); + throw new IAE("Replicant value [%d] is less than 1, which is not allowed", entry.getValue()); } } } diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java index 0d6e2e099a74..fdeffc927597 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java @@ -22,7 +22,6 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.collect.ImmutableMap; -import org.apache.druid.client.DruidServer; import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.timeline.DataSegment; import org.joda.time.DateTime; @@ -49,7 +48,7 @@ public PeriodLoadRule( @JsonProperty("tieredReplicants") Map tieredReplicants ) { - this.tieredReplicants = tieredReplicants == null ? ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS) : tieredReplicants; + this.tieredReplicants = tieredReplicants == null ? ImmutableMap.of() : tieredReplicants; validateTieredReplicants(this.tieredReplicants); this.period = period; this.includeFuture = includeFuture == null ? DEFAULT_INCLUDE_FUTURE : includeFuture; diff --git a/server/src/test/java/org/apache/druid/server/coordinator/rules/ForeverLoadRuleTest.java b/server/src/test/java/org/apache/druid/server/coordinator/rules/ForeverLoadRuleTest.java index c9be7f63ddbe..146fdd94f034 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/rules/ForeverLoadRuleTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/rules/ForeverLoadRuleTest.java @@ -43,28 +43,16 @@ public void testSerdeNullTieredReplicants() throws Exception Rule reread = jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); Assert.assertEquals(rule.getTieredReplicants(), ((ForeverLoadRule) reread).getTieredReplicants()); - Assert.assertEquals(ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), rule.getTieredReplicants()); + Assert.assertEquals(ImmutableMap.of(), rule.getTieredReplicants()); } - @Test - public void testMappingNullTieredReplicants() throws Exception + @Test(expected = IAE.class) + public void testCreatingNegativeTieredReplicants() { - String inputJson = "{\n" - + " \"type\": \"loadForever\"\n" - + "}"; - String expectedJson = " {\n" - + " \"tieredReplicants\": {\n" - + " \"" + DruidServer.DEFAULT_TIER + "\": " + DruidServer.DEFAULT_NUM_REPLICANTS + "\n" - + " },\n" - + " \"type\": \"loadForever\"\n" - + " }"; - ObjectMapper jsonMapper = new DefaultObjectMapper(); - ForeverLoadRule inputForeverLoadRule = jsonMapper.readValue(inputJson, ForeverLoadRule.class); - ForeverLoadRule expectedForeverLoadRule = jsonMapper.readValue(expectedJson, ForeverLoadRule.class); - Assert.assertEquals(expectedForeverLoadRule.getTieredReplicants(), inputForeverLoadRule.getTieredReplicants()); + ForeverLoadRule foreverLoadRule = new ForeverLoadRule(ImmutableMap.of(DruidServer.DEFAULT_TIER, -1)); } - @Test(expected = IAE.class) + @Test public void testEmptyTieredReplicants() throws Exception { ForeverLoadRule rule = new ForeverLoadRule( @@ -72,7 +60,8 @@ public void testEmptyTieredReplicants() throws Exception ); ObjectMapper jsonMapper = new DefaultObjectMapper(); - Rule reread = jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); + LoadRule reread = (LoadRule) jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); + Assert.assertEquals(ImmutableMap.of(), reread.getTieredReplicants()); } @Test(expected = IAE.class) diff --git a/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java b/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java index 52e892dd8cff..3e17544de8a6 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java @@ -23,11 +23,16 @@ import com.google.common.collect.ImmutableMap; import org.apache.druid.client.DruidServer; import org.apache.druid.jackson.DefaultObjectMapper; +import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.Intervals; import org.junit.Assert; import org.junit.Test; +import java.util.HashMap; +import java.util.Map; + /** + * Unit tests for {@link IntervalLoadRule} */ public class IntervalLoadRuleTest { @@ -56,29 +61,30 @@ public void testSerdeNullTieredReplicants() throws Exception Rule reread = jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); Assert.assertEquals(rule, reread); - Assert.assertEquals( - ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), - rule.getTieredReplicants() + Assert.assertEquals(ImmutableMap.of(), rule.getTieredReplicants()); + } + + @Test(expected = IAE.class) + public void testCreatingNegativeTieredReplicants() + { + IntervalLoadRule intervalLoadRule = new IntervalLoadRule( + Intervals.of("0/3000"), + ImmutableMap.of(DruidServer.DEFAULT_TIER, -1) ); } - @Test - public void testMappingNullTieredReplicants() throws Exception + @Test(expected = IAE.class) + public void testEmptyReplicantValue() throws Exception { - String inputJson = " {\n" - + " \"interval\": \"0000-01-01T00:00:00.000-05:50:36/3000-01-01T00:00:00.000-06:00\",\n" - + " \"type\": \"loadByInterval\"\n" - + " }"; - String expectedJson = "{\n" - + " \"interval\": \"0000-01-01T00:00:00.000-05:50:36/3000-01-01T00:00:00.000-06:00\",\n" - + " \"tieredReplicants\": {\n" - + " \"" + DruidServer.DEFAULT_TIER + "\": " + DruidServer.DEFAULT_NUM_REPLICANTS + "\n" - + " },\n" - + " \"type\": \"loadByInterval\"\n" - + " }"; + // Immutable map does not allow null values + Map tieredReplicants = new HashMap<>(); + tieredReplicants.put("tier", null); + IntervalLoadRule rule = new IntervalLoadRule( + Intervals.of("0/3000"), + tieredReplicants + ); + ObjectMapper jsonMapper = new DefaultObjectMapper(); - IntervalLoadRule inputIntervalLoadRule = jsonMapper.readValue(inputJson, IntervalLoadRule.class); - IntervalLoadRule expectedIntervalLoadRule = jsonMapper.readValue(expectedJson, IntervalLoadRule.class); - Assert.assertEquals(expectedIntervalLoadRule, inputIntervalLoadRule); + Rule reread = jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); } } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java b/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java index 84b72614000f..398e79abfe43 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java @@ -24,6 +24,7 @@ import org.apache.druid.client.DruidServer; import org.apache.druid.jackson.DefaultObjectMapper; import org.apache.druid.java.util.common.DateTimes; +import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.Intervals; import org.apache.druid.timeline.DataSegment; import org.apache.druid.timeline.partition.NoneShardSpec; @@ -33,6 +34,9 @@ import org.junit.Assert; import org.junit.Test; +import java.util.HashMap; +import java.util.Map; + /** */ public class PeriodLoadRuleTest @@ -158,7 +162,7 @@ public void testSerdeNull() throws Exception Assert.assertEquals(rule.isIncludeFuture(), ((PeriodLoadRule) reread).isIncludeFuture()); Assert.assertEquals(PeriodLoadRule.DEFAULT_INCLUDE_FUTURE, rule.isIncludeFuture()); Assert.assertEquals(rule.getTieredReplicants(), ((PeriodLoadRule) reread).getTieredReplicants()); - Assert.assertEquals(ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), rule.getTieredReplicants()); + Assert.assertEquals(ImmutableMap.of(), rule.getTieredReplicants()); } /** @@ -174,9 +178,6 @@ public void testMappingNull() throws Exception String expectedJson = "{\n" + " \"period\": \"P1D\",\n" + " \"includeFuture\": " + PeriodLoadRule.DEFAULT_INCLUDE_FUTURE + ",\n" - + " \"tieredReplicants\": {\n" - + " \"" + DruidServer.DEFAULT_TIER + "\": " + DruidServer.DEFAULT_NUM_REPLICANTS + "\n" - + " },\n" + " \"type\": \"loadByPeriod\"\n" + " }"; ObjectMapper jsonMapper = new DefaultObjectMapper(); @@ -186,4 +187,26 @@ public void testMappingNull() throws Exception Assert.assertEquals(expectedPeriodLoadRule.getPeriod(), inputPeriodLoadRule.getPeriod()); Assert.assertEquals(expectedPeriodLoadRule.isIncludeFuture(), inputPeriodLoadRule.isIncludeFuture()); } + + @Test(expected = IAE.class) + public void testCreatingNegativeTieredReplicants() + { + PeriodLoadRule periodLoadRule = new PeriodLoadRule(Period.days(1), true, ImmutableMap.of(DruidServer.DEFAULT_TIER, -1)); + } + + @Test(expected = IAE.class) + public void testEmptyReplicantValue() throws Exception + { + // Immutable map does not allow null values + Map tieredReplicants = new HashMap<>(); + tieredReplicants.put("tier", null); + PeriodLoadRule rule = new PeriodLoadRule( + Period.days(1), + true, + tieredReplicants + ); + + ObjectMapper jsonMapper = new DefaultObjectMapper(); + Rule reread = jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); + } } From ccb981c77949c52ec821505d9e24f4d9d8596f94 Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Sat, 17 Jun 2023 21:16:27 +0530 Subject: [PATCH 02/18] Address review comments --- .../apache/druid/server/coordinator/rules/LoadRule.java | 2 +- .../server/coordinator/rules/ForeverLoadRuleTest.java | 4 ++-- .../server/coordinator/rules/IntervalLoadRuleTest.java | 7 ++----- .../druid/server/coordinator/rules/PeriodLoadRuleTest.java | 4 ++-- 4 files changed, 7 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java index e3f62601c886..0e72e21c036d 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java @@ -560,7 +560,7 @@ protected static void validateTieredReplicants(final Map tiered throw new IAE("Replicant value cannot be empty"); } if (entry.getValue() < 0) { - throw new IAE("Replicant value [%d] is less than 1, which is not allowed", entry.getValue()); + throw new IAE("Replicant value [%d] is less than 0, which is not allowed", entry.getValue()); } } } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/rules/ForeverLoadRuleTest.java b/server/src/test/java/org/apache/druid/server/coordinator/rules/ForeverLoadRuleTest.java index 146fdd94f034..3c01081de829 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/rules/ForeverLoadRuleTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/rules/ForeverLoadRuleTest.java @@ -49,7 +49,7 @@ public void testSerdeNullTieredReplicants() throws Exception @Test(expected = IAE.class) public void testCreatingNegativeTieredReplicants() { - ForeverLoadRule foreverLoadRule = new ForeverLoadRule(ImmutableMap.of(DruidServer.DEFAULT_TIER, -1)); + new ForeverLoadRule(ImmutableMap.of(DruidServer.DEFAULT_TIER, -1)); } @Test @@ -75,6 +75,6 @@ public void testEmptyReplicantValue() throws Exception ); ObjectMapper jsonMapper = new DefaultObjectMapper(); - Rule reread = jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); + jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); } } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java b/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java index 3e17544de8a6..4b04017f4df5 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java @@ -67,10 +67,7 @@ public void testSerdeNullTieredReplicants() throws Exception @Test(expected = IAE.class) public void testCreatingNegativeTieredReplicants() { - IntervalLoadRule intervalLoadRule = new IntervalLoadRule( - Intervals.of("0/3000"), - ImmutableMap.of(DruidServer.DEFAULT_TIER, -1) - ); + new IntervalLoadRule(Intervals.of("0/3000"), ImmutableMap.of(DruidServer.DEFAULT_TIER, -1)); } @Test(expected = IAE.class) @@ -85,6 +82,6 @@ public void testEmptyReplicantValue() throws Exception ); ObjectMapper jsonMapper = new DefaultObjectMapper(); - Rule reread = jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); + jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); } } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java b/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java index 398e79abfe43..caa48e50a797 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java @@ -191,7 +191,7 @@ public void testMappingNull() throws Exception @Test(expected = IAE.class) public void testCreatingNegativeTieredReplicants() { - PeriodLoadRule periodLoadRule = new PeriodLoadRule(Period.days(1), true, ImmutableMap.of(DruidServer.DEFAULT_TIER, -1)); + new PeriodLoadRule(Period.days(1), true, ImmutableMap.of(DruidServer.DEFAULT_TIER, -1)); } @Test(expected = IAE.class) @@ -207,6 +207,6 @@ public void testEmptyReplicantValue() throws Exception ); ObjectMapper jsonMapper = new DefaultObjectMapper(); - Rule reread = jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); + jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); } } From 62a9012f463c73cf7ddd4a19b41465da9d6ebc2f Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Mon, 19 Jun 2023 11:32:30 +0530 Subject: [PATCH 03/18] Add flag for backward compatibility --- .../coordinator/rules/ForeverLoadRule.java | 24 ++++- .../coordinator/rules/IntervalLoadRule.java | 24 ++++- .../coordinator/rules/PeriodLoadRule.java | 32 ++++++- .../rules/ForeverLoadRuleTest.java | 62 +++++++++++-- .../rules/IntervalLoadRuleTest.java | 41 +++++++- .../coordinator/rules/PeriodLoadRuleTest.java | 93 +++++++++++++++---- 6 files changed, 245 insertions(+), 31 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/ForeverLoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/ForeverLoadRule.java index feda6c038059..6d7dbc372135 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/ForeverLoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/ForeverLoadRule.java @@ -22,6 +22,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.collect.ImmutableMap; +import org.apache.druid.client.DruidServer; import org.apache.druid.timeline.DataSegment; import org.joda.time.DateTime; import org.joda.time.Interval; @@ -34,16 +35,35 @@ public class ForeverLoadRule extends LoadRule { private final Map tieredReplicants; + private final boolean allowEmptyTieredReplicants; + + public ForeverLoadRule(Map tieredReplicants) + { + this(tieredReplicants, null); + } @JsonCreator public ForeverLoadRule( - @JsonProperty("tieredReplicants") Map tieredReplicants + @JsonProperty("tieredReplicants") Map tieredReplicants, + @JsonProperty("allowEmptyTieredReplicants") Boolean allowEmptyTieredReplicants ) { - this.tieredReplicants = tieredReplicants == null ? ImmutableMap.of() : tieredReplicants; + this.allowEmptyTieredReplicants = allowEmptyTieredReplicants != null && allowEmptyTieredReplicants; + + if (!this.allowEmptyTieredReplicants) { + this.tieredReplicants = tieredReplicants == null ? ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS) : tieredReplicants; + } else { + this.tieredReplicants = tieredReplicants == null ? ImmutableMap.of() : tieredReplicants; + } validateTieredReplicants(this.tieredReplicants); } + @JsonProperty + public boolean isAllowEmptyTieredReplicants() + { + return allowEmptyTieredReplicants; + } + @Override @JsonProperty public String getType() diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/IntervalLoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/IntervalLoadRule.java index 9518bee4ac23..b7c17eb77ce3 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/IntervalLoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/IntervalLoadRule.java @@ -22,6 +22,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.collect.ImmutableMap; +import org.apache.druid.client.DruidServer; import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.timeline.DataSegment; import org.joda.time.DateTime; @@ -37,14 +38,27 @@ public class IntervalLoadRule extends LoadRule private final Interval interval; private final Map tieredReplicants; + private final boolean allowEmptyTieredReplicants; + + public IntervalLoadRule(Interval interval, Map tieredReplicants) + { + this(interval, tieredReplicants, null); + } @JsonCreator public IntervalLoadRule( @JsonProperty("interval") Interval interval, - @JsonProperty("tieredReplicants") Map tieredReplicants + @JsonProperty("tieredReplicants") Map tieredReplicants, + @JsonProperty("allowEmptyTieredReplicants") Boolean allowEmptyTieredReplicants ) { - this.tieredReplicants = tieredReplicants == null ? ImmutableMap.of() : tieredReplicants; + this.allowEmptyTieredReplicants = allowEmptyTieredReplicants != null && allowEmptyTieredReplicants; + + if (!this.allowEmptyTieredReplicants) { + this.tieredReplicants = tieredReplicants == null ? ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS) : tieredReplicants; + } else { + this.tieredReplicants = tieredReplicants == null ? ImmutableMap.of() : tieredReplicants; + } validateTieredReplicants(this.tieredReplicants); this.interval = interval; } @@ -63,6 +77,12 @@ public Map getTieredReplicants() return tieredReplicants; } + @JsonProperty + public boolean isAllowEmptyTieredReplicants() + { + return allowEmptyTieredReplicants; + } + @Override public int getNumReplicants(String tier) { diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java index fdeffc927597..9ccfb8a468bf 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java @@ -22,6 +22,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.collect.ImmutableMap; +import org.apache.druid.client.DruidServer; import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.timeline.DataSegment; import org.joda.time.DateTime; @@ -40,15 +41,36 @@ public class PeriodLoadRule extends LoadRule private final Period period; private final boolean includeFuture; private final Map tieredReplicants; + /** + * Compatibility flag for load rules. If the flag is false or not present, null or empty {@link #tieredReplicants} + * will be understood as the default load rule of {@link DruidServer#DEFAULT_NUM_REPLICANTS} replicants + * for @{@link DruidServer#DEFAULT_TIER}. + *
+ * If the flag is true, it will be kept as an empty map. This will enable the new behaviour of not loading segments + * to a historical unless the tier and number of replicants are explicitly specified. + */ + private final boolean allowEmptyTieredReplicants; + + public PeriodLoadRule(Period period, Boolean includeFuture, Map tieredReplicants) + { + this(period, includeFuture, tieredReplicants, null); + } @JsonCreator public PeriodLoadRule( @JsonProperty("period") Period period, @JsonProperty("includeFuture") Boolean includeFuture, - @JsonProperty("tieredReplicants") Map tieredReplicants + @JsonProperty("tieredReplicants") Map tieredReplicants, + @JsonProperty("allowEmptyTieredReplicants") Boolean allowEmptyTieredReplicants ) { - this.tieredReplicants = tieredReplicants == null ? ImmutableMap.of() : tieredReplicants; + this.allowEmptyTieredReplicants = allowEmptyTieredReplicants != null && allowEmptyTieredReplicants; + + if (!this.allowEmptyTieredReplicants) { + this.tieredReplicants = tieredReplicants == null ? ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS) : tieredReplicants; + } else { + this.tieredReplicants = tieredReplicants == null ? ImmutableMap.of() : tieredReplicants; + } validateTieredReplicants(this.tieredReplicants); this.period = period; this.includeFuture = includeFuture == null ? DEFAULT_INCLUDE_FUTURE : includeFuture; @@ -87,6 +109,12 @@ public int getNumReplicants(String tier) return retVal == null ? 0 : retVal; } + @JsonProperty + public boolean isAllowEmptyTieredReplicants() + { + return allowEmptyTieredReplicants; + } + @Override public boolean appliesTo(DataSegment segment, DateTime referenceTimestamp) { diff --git a/server/src/test/java/org/apache/druid/server/coordinator/rules/ForeverLoadRuleTest.java b/server/src/test/java/org/apache/druid/server/coordinator/rules/ForeverLoadRuleTest.java index 3c01081de829..07f631feb72b 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/rules/ForeverLoadRuleTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/rules/ForeverLoadRuleTest.java @@ -33,11 +33,21 @@ public class ForeverLoadRuleTest { @Test - public void testSerdeNullTieredReplicants() throws Exception + public void testSerdeNullTieredReplicantWithoutAllowEmptyFlag() throws Exception { - ForeverLoadRule rule = new ForeverLoadRule( - null - ); + ForeverLoadRule rule = new ForeverLoadRule(null, null); + + ObjectMapper jsonMapper = new DefaultObjectMapper(); + Rule reread = jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); + + Assert.assertEquals(rule.getTieredReplicants(), ((ForeverLoadRule) reread).getTieredReplicants()); + Assert.assertEquals(ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), rule.getTieredReplicants()); + } + + @Test + public void testSerdeNullTieredReplicantWithAllowEmptyFlag() throws Exception + { + ForeverLoadRule rule = new ForeverLoadRule(null, true); ObjectMapper jsonMapper = new DefaultObjectMapper(); Rule reread = jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); @@ -49,14 +59,14 @@ public void testSerdeNullTieredReplicants() throws Exception @Test(expected = IAE.class) public void testCreatingNegativeTieredReplicants() { - new ForeverLoadRule(ImmutableMap.of(DruidServer.DEFAULT_TIER, -1)); + new ForeverLoadRule(ImmutableMap.of(DruidServer.DEFAULT_TIER, -1), null); } @Test public void testEmptyTieredReplicants() throws Exception { - ForeverLoadRule rule = new ForeverLoadRule( - ImmutableMap.of() + ForeverLoadRule rule = new ForeverLoadRule(ImmutableMap.of(), + true ); ObjectMapper jsonMapper = new DefaultObjectMapper(); @@ -71,10 +81,46 @@ public void testEmptyReplicantValue() throws Exception Map tieredReplicants = new HashMap<>(); tieredReplicants.put("tier", null); ForeverLoadRule rule = new ForeverLoadRule( - tieredReplicants + tieredReplicants, + true ); ObjectMapper jsonMapper = new DefaultObjectMapper(); jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); } + + @Test + public void testWithoutAllowEmptyReplicantShouldCreateDefaultTier() throws Exception + { + String inputJson = " {\n" + + " \"type\": \"loadForever\"\n" + + " }"; + String expectedJson = " {\n" + + " \"tieredReplicants\": {\n" + + " \"" + DruidServer.DEFAULT_TIER + "\": " + DruidServer.DEFAULT_NUM_REPLICANTS + "\n" + + " },\n" + + " \"type\": \"loadForever\"\n" + + " }"; + ObjectMapper jsonMapper = new DefaultObjectMapper(); + ForeverLoadRule inputForeverLoadRule = jsonMapper.readValue(inputJson, ForeverLoadRule.class); + ForeverLoadRule expectedForeverLoadRule = jsonMapper.readValue(expectedJson, ForeverLoadRule.class); + Assert.assertEquals(expectedForeverLoadRule.getTieredReplicants(), inputForeverLoadRule.getTieredReplicants()); + } + + @Test + public void testWithAllowEmptyReplicantShouldDefaultToEmpty() throws Exception + { + String inputJson = " {\n" + + " \"type\": \"loadForever\"\n," + + " \"allowEmptyTieredReplicants\": \"true\"\n" + + " }"; + String expectedJson = " {\n" + + " \"tieredReplicants\": {},\n" + + " \"type\": \"loadForever\"\n" + + " }"; + ObjectMapper jsonMapper = new DefaultObjectMapper(); + ForeverLoadRule inputForeverLoadRule = jsonMapper.readValue(inputJson, ForeverLoadRule.class); + ForeverLoadRule expectedForeverLoadRule = jsonMapper.readValue(expectedJson, ForeverLoadRule.class); + Assert.assertEquals(expectedForeverLoadRule.getTieredReplicants(), inputForeverLoadRule.getTieredReplicants()); + } } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java b/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java index 4b04017f4df5..d4f53da04f5f 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java @@ -54,7 +54,7 @@ public void testSerde() throws Exception public void testSerdeNullTieredReplicants() throws Exception { IntervalLoadRule rule = new IntervalLoadRule( - Intervals.of("0/3000"), null + Intervals.of("0/3000"), null, true ); ObjectMapper jsonMapper = new DefaultObjectMapper(); @@ -84,4 +84,43 @@ public void testEmptyReplicantValue() throws Exception ObjectMapper jsonMapper = new DefaultObjectMapper(); jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); } + + @Test + public void testWithoutAllowEmptyReplicantShouldCreateDefaultTier() throws Exception + { + String inputJson = " {\n" + + " \"interval\": \"0000-01-01T00:00:00.000-05:50:36/3000-01-01T00:00:00.000-06:00\",\n" + + " \"type\": \"loadByInterval\"\n" + + " }"; + String expectedJson = " {\n" + + " \"interval\": \"0000-01-01T00:00:00.000-05:50:36/3000-01-01T00:00:00.000-06:00\",\n" + + " \"tieredReplicants\": {\n" + + " \"" + DruidServer.DEFAULT_TIER + "\": " + DruidServer.DEFAULT_NUM_REPLICANTS + "\n" + + " },\n" + + " \"type\": \"loadByInterval\"\n" + + " }"; + ObjectMapper jsonMapper = new DefaultObjectMapper(); + IntervalLoadRule inputForeverLoadRule = jsonMapper.readValue(inputJson, IntervalLoadRule.class); + IntervalLoadRule expectedForeverLoadRule = jsonMapper.readValue(expectedJson, IntervalLoadRule.class); + Assert.assertEquals(expectedForeverLoadRule.getTieredReplicants(), inputForeverLoadRule.getTieredReplicants()); + } + + @Test + public void testWithAllowEmptyReplicantShouldDefaultToEmpty() throws Exception + { + String inputJson = " {\n" + + " \"interval\": \"0000-01-01T00:00:00.000-05:50:36/3000-01-01T00:00:00.000-06:00\",\n" + + " \"type\": \"loadByInterval\",\n" + + " \"allowEmptyTieredReplicants\": \"true\"\n" + + " }"; + String expectedJson = " {\n" + + " \"interval\": \"0000-01-01T00:00:00.000-05:50:36/3000-01-01T00:00:00.000-06:00\",\n" + + " \"tieredReplicants\": {},\n" + + " \"type\": \"loadByInterval\"\n" + + " }"; + ObjectMapper jsonMapper = new DefaultObjectMapper(); + IntervalLoadRule inputForeverLoadRule = jsonMapper.readValue(inputJson, IntervalLoadRule.class); + IntervalLoadRule expectedForeverLoadRule = jsonMapper.readValue(expectedJson, IntervalLoadRule.class); + Assert.assertEquals(expectedForeverLoadRule.getTieredReplicants(), inputForeverLoadRule.getTieredReplicants()); + } } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java b/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java index caa48e50a797..382ddc2c8455 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java @@ -84,7 +84,7 @@ public void testAppliesToPeriod() Assert.assertFalse( rule.appliesTo( BUILDER.interval(new Interval(now.plusDays(1), now.plusDays(2))) - .build(), + .build(), now ) ); @@ -95,24 +95,24 @@ public void testAppliesToPartialOverlap() { DateTime now = DateTimes.of("2012-12-31T01:00:00"); PeriodLoadRule rule = new PeriodLoadRule( - new Period("P1M"), - false, - ImmutableMap.of("", 0) + new Period("P1M"), + false, + ImmutableMap.of("", 0) ); Assert.assertTrue( - rule.appliesTo( - BUILDER.interval(new Interval(now.minusWeeks(1), now.plusWeeks(1))).build(), - now - ) + rule.appliesTo( + BUILDER.interval(new Interval(now.minusWeeks(1), now.plusWeeks(1))).build(), + now + ) ); Assert.assertTrue( - rule.appliesTo( - BUILDER.interval( - new Interval(now.minusMonths(1).minusWeeks(1), now.minusMonths(1).plusWeeks(1)) - ).build(), - now - ) + rule.appliesTo( + BUILDER.interval( + new Interval(now.minusMonths(1).minusWeeks(1), now.minusMonths(1).plusWeeks(1)) + ).build(), + now + ) ); } @@ -149,10 +149,27 @@ public void testIncludeFuture() * test serialize/deserilize null values of {@link PeriodLoadRule#tieredReplicants} and {@link PeriodLoadRule#includeFuture} */ @Test - public void testSerdeNull() throws Exception + public void testSerdeNullTieredReplicantWithoutAllowEmptyFlag() throws Exception + { + PeriodLoadRule rule = new PeriodLoadRule( + new Period("P1D"), null, null, null + ); + + ObjectMapper jsonMapper = new DefaultObjectMapper(); + Rule reread = jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); + + Assert.assertEquals(rule.getPeriod(), ((PeriodLoadRule) reread).getPeriod()); + Assert.assertEquals(rule.isIncludeFuture(), ((PeriodLoadRule) reread).isIncludeFuture()); + Assert.assertEquals(PeriodLoadRule.DEFAULT_INCLUDE_FUTURE, rule.isIncludeFuture()); + Assert.assertEquals(rule.getTieredReplicants(), ((PeriodLoadRule) reread).getTieredReplicants()); + Assert.assertEquals(ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), rule.getTieredReplicants()); + } + + @Test + public void testSerdeNullTieredReplicantWithAllowEmptyFlag() throws Exception { PeriodLoadRule rule = new PeriodLoadRule( - new Period("P1D"), null, null + new Period("P1D"), null, null, true ); ObjectMapper jsonMapper = new DefaultObjectMapper(); @@ -209,4 +226,48 @@ public void testEmptyReplicantValue() throws Exception ObjectMapper jsonMapper = new DefaultObjectMapper(); jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); } + + + @Test + public void testWithoutAllowEmptyReplicantShouldCreateDefaultTier() throws Exception + { + String inputJson = " {\n" + + " \"period\": \"P1D\",\n" + + " \"includeFuture\": " + PeriodLoadRule.DEFAULT_INCLUDE_FUTURE + ",\n" + + " \"type\": \"loadByPeriod\"\n" + + " }"; + String expectedJson = " {\n" + + " \"period\": \"P1D\",\n" + + " \"includeFuture\": " + PeriodLoadRule.DEFAULT_INCLUDE_FUTURE + ",\n" + + " \"tieredReplicants\": {\n" + + " \"" + DruidServer.DEFAULT_TIER + "\": " + DruidServer.DEFAULT_NUM_REPLICANTS + "\n" + + " },\n" + + " \"type\": \"loadByPeriod\"\n" + + " }"; + ObjectMapper jsonMapper = new DefaultObjectMapper(); + PeriodLoadRule inputPeriodLoadRule = jsonMapper.readValue(inputJson, PeriodLoadRule.class); + PeriodLoadRule expectedPeriodLoadRule = jsonMapper.readValue(expectedJson, PeriodLoadRule.class); + Assert.assertEquals(expectedPeriodLoadRule.getTieredReplicants(), inputPeriodLoadRule.getTieredReplicants()); + } + + @Test + public void testWithAllowEmptyReplicantShouldDefaultToEmpty() throws Exception + { + String inputJson = " {\n" + + " \"period\": \"P1D\",\n" + + " \"includeFuture\": " + PeriodLoadRule.DEFAULT_INCLUDE_FUTURE + ",\n" + + " \"allowEmptyTieredReplicants\": \"true\",\n" + + " \"type\": \"loadByPeriod\"\n" + + " }"; + String expectedJson = " {\n" + + " \"period\": \"P1D\",\n" + + " \"includeFuture\": " + PeriodLoadRule.DEFAULT_INCLUDE_FUTURE + ",\n" + + " \"tieredReplicants\": {},\n" + + " \"type\": \"loadByPeriod\"\n" + + " }"; + ObjectMapper jsonMapper = new DefaultObjectMapper(); + PeriodLoadRule inputPeriodLoadRule = jsonMapper.readValue(inputJson, PeriodLoadRule.class); + PeriodLoadRule expectedPeriodLoadRule = jsonMapper.readValue(expectedJson, PeriodLoadRule.class); + Assert.assertEquals(expectedPeriodLoadRule.getTieredReplicants(), inputPeriodLoadRule.getTieredReplicants()); + } } From 3fed565f242667460a3a564494fcb740d1c5519a Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Mon, 19 Jun 2023 11:42:07 +0530 Subject: [PATCH 04/18] Remove constructor --- .../metadata/SQLMetadataRuleManager.java | 3 +- .../coordinator/rules/ForeverLoadRule.java | 5 -- .../coordinator/rules/IntervalLoadRule.java | 5 -- .../coordinator/rules/PeriodLoadRule.java | 5 -- .../metadata/SQLMetadataRuleManagerTest.java | 22 ++++-- .../coordinator/BalanceSegmentsProfiler.java | 2 +- .../coordinator/DruidCoordinatorTest.java | 6 +- .../server/coordinator/RunRulesTest.java | 74 ++++++++++++------- .../duty/UnloadUnusedSegmentsTest.java | 6 +- .../rules/IntervalLoadRuleTest.java | 8 +- .../coordinator/rules/PeriodLoadRuleTest.java | 20 +++-- .../CoordinatorSimulationBaseTest.java | 2 +- .../simulate/TestMetadataRuleManager.java | 2 +- .../server/http/DataSourcesResourceTest.java | 2 +- .../router/CoordinatorRuleManagerTest.java | 12 +-- .../router/TieredBrokerHostSelectorTest.java | 7 +- 16 files changed, 104 insertions(+), 77 deletions(-) diff --git a/server/src/main/java/org/apache/druid/metadata/SQLMetadataRuleManager.java b/server/src/main/java/org/apache/druid/metadata/SQLMetadataRuleManager.java index b3d0fa9b27f1..b7cdb2f7ec91 100644 --- a/server/src/main/java/org/apache/druid/metadata/SQLMetadataRuleManager.java +++ b/server/src/main/java/org/apache/druid/metadata/SQLMetadataRuleManager.java @@ -96,7 +96,8 @@ public Void withHandle(Handle handle) throws Exception ImmutableMap.of( DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS - ) + ), + null ) ); final String version = DateTimes.nowUtc().toString(); diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/ForeverLoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/ForeverLoadRule.java index 6d7dbc372135..dab067c7c370 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/ForeverLoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/ForeverLoadRule.java @@ -37,11 +37,6 @@ public class ForeverLoadRule extends LoadRule private final Map tieredReplicants; private final boolean allowEmptyTieredReplicants; - public ForeverLoadRule(Map tieredReplicants) - { - this(tieredReplicants, null); - } - @JsonCreator public ForeverLoadRule( @JsonProperty("tieredReplicants") Map tieredReplicants, diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/IntervalLoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/IntervalLoadRule.java index b7c17eb77ce3..d83ef5aadeea 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/IntervalLoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/IntervalLoadRule.java @@ -40,11 +40,6 @@ public class IntervalLoadRule extends LoadRule private final Map tieredReplicants; private final boolean allowEmptyTieredReplicants; - public IntervalLoadRule(Interval interval, Map tieredReplicants) - { - this(interval, tieredReplicants, null); - } - @JsonCreator public IntervalLoadRule( @JsonProperty("interval") Interval interval, diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java index 9ccfb8a468bf..0129b3b2ea31 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java @@ -51,11 +51,6 @@ public class PeriodLoadRule extends LoadRule */ private final boolean allowEmptyTieredReplicants; - public PeriodLoadRule(Period period, Boolean includeFuture, Map tieredReplicants) - { - this(period, includeFuture, tieredReplicants, null); - } - @JsonCreator public PeriodLoadRule( @JsonProperty("period") Period period, diff --git a/server/src/test/java/org/apache/druid/metadata/SQLMetadataRuleManagerTest.java b/server/src/test/java/org/apache/druid/metadata/SQLMetadataRuleManagerTest.java index 61bc5d9080db..20ffdb81188a 100644 --- a/server/src/test/java/org/apache/druid/metadata/SQLMetadataRuleManagerTest.java +++ b/server/src/test/java/org/apache/druid/metadata/SQLMetadataRuleManagerTest.java @@ -108,7 +108,8 @@ public void testRuleInsert() List rules = Collections.singletonList( new IntervalLoadRule( Intervals.of("2015-01-01/2015-02-01"), - ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS) + ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), + null ) ); ruleManager.overrideRule(DATASOURCE, rules, createAuditInfo("override rule")); @@ -171,7 +172,8 @@ public void testAuditEntryCreated() throws Exception List rules = Collections.singletonList( new IntervalLoadRule( Intervals.of("2015-01-01/2015-02-01"), - ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS) + ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), + null ) ); final AuditInfo auditInfo = createAuditInfo("create audit entry"); @@ -200,9 +202,10 @@ public void testFetchAuditEntriesForAllDataSources() throws Exception List rules = Collections.singletonList( new IntervalLoadRule( Intervals.of("2015-01-01/2015-02-01"), ImmutableMap.of( - DruidServer.DEFAULT_TIER, - DruidServer.DEFAULT_NUM_REPLICANTS - ) + DruidServer.DEFAULT_TIER, + DruidServer.DEFAULT_NUM_REPLICANTS + ), + null ) ); final AuditInfo auditInfo = createAuditInfo("test_comment"); @@ -232,7 +235,8 @@ public void testRemoveRulesOlderThanWithNonExistenceDatasourceAndOlderThanTimest List rules = ImmutableList.of( new IntervalLoadRule( Intervals.of("2015-01-01/2015-02-01"), - ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS) + ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), + null ) ); ruleManager.overrideRule(DATASOURCE, rules, createAuditInfo("test")); @@ -258,7 +262,8 @@ public void testRemoveRulesOlderThanWithNonExistenceDatasourceAndNewerThanTimest List rules = ImmutableList.of( new IntervalLoadRule( Intervals.of("2015-01-01/2015-02-01"), - ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS) + ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), + null ) ); ruleManager.overrideRule(DATASOURCE, rules, createAuditInfo("update rules")); @@ -286,7 +291,8 @@ public void testRemoveRulesOlderThanWithActiveDatasourceShouldNotDelete() throws List rules = ImmutableList.of( new IntervalLoadRule( Intervals.of("2015-01-01/2015-02-01"), - ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS) + ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), + null ) ); ruleManager.overrideRule(DATASOURCE, rules, createAuditInfo("update rules")); diff --git a/server/src/test/java/org/apache/druid/server/coordinator/BalanceSegmentsProfiler.java b/server/src/test/java/org/apache/druid/server/coordinator/BalanceSegmentsProfiler.java index 06c43e9a68e1..b115064d3b86 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/BalanceSegmentsProfiler.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/BalanceSegmentsProfiler.java @@ -56,7 +56,7 @@ public class BalanceSegmentsProfiler List segments = new ArrayList<>(); ServiceEmitter emitter; MetadataRuleManager manager; - PeriodLoadRule loadRule = new PeriodLoadRule(new Period("P5000Y"), null, ImmutableMap.of("normal", 3)); + PeriodLoadRule loadRule = new PeriodLoadRule(new Period("P5000Y"), null, ImmutableMap.of("normal", 3), null); List rules = ImmutableList.of(loadRule); @Before diff --git a/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorTest.java b/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorTest.java index 4076ce9d3a87..9d6c1667a0e1 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorTest.java @@ -307,7 +307,7 @@ public void testCoordinatorRun() throws Exception String tier = "hot"; // Setup MetadataRuleManager - Rule foreverLoadRule = new ForeverLoadRule(ImmutableMap.of(tier, 2)); + Rule foreverLoadRule = new ForeverLoadRule(ImmutableMap.of(tier, 2), null); EasyMock.expect(metadataRuleManager.getRulesWithDefault(EasyMock.anyString())) .andReturn(ImmutableList.of(foreverLoadRule)).atLeastOnce(); EasyMock.expect(metadataRuleManager.getAllRules()) @@ -424,8 +424,8 @@ public void testCoordinatorRun() throws Exception public void testCoordinatorTieredRun() throws Exception { final String dataSource = "dataSource", hotTierName = "hot", coldTierName = "cold"; - final Rule hotTier = new IntervalLoadRule(Intervals.of("2018-01-01/P1M"), ImmutableMap.of(hotTierName, 1)); - final Rule coldTier = new ForeverLoadRule(ImmutableMap.of(coldTierName, 1)); + final Rule hotTier = new IntervalLoadRule(Intervals.of("2018-01-01/P1M"), ImmutableMap.of(hotTierName, 1), null); + final Rule coldTier = new ForeverLoadRule(ImmutableMap.of(coldTierName, 1), null); final String loadPathCold = "/druid/loadqueue/cold:1234"; final DruidServer hotServer = new DruidServer("hot", "hot", null, 5L, ServerType.HISTORICAL, hotTierName, 0); final DruidServer coldServer = new DruidServer("cold", "cold", null, 5L, ServerType.HISTORICAL, coldTierName, 0); diff --git a/server/src/test/java/org/apache/druid/server/coordinator/RunRulesTest.java b/server/src/test/java/org/apache/druid/server/coordinator/RunRulesTest.java index 3256a0ad766b..c9fe01dcb854 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/RunRulesTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/RunRulesTest.java @@ -114,7 +114,8 @@ public void testOneTierTwoReplicantsWithStrictReplicantLimit() Collections.singletonList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z"), - ImmutableMap.of("normal", 2) + ImmutableMap.of("normal", 2), + null ) )).atLeastOnce(); EasyMock.replay(databaseRuleManager); @@ -172,7 +173,8 @@ public void testTwoTiersTwoReplicantsWithStrictReplicantLimit() Collections.singletonList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z"), - ImmutableMap.of("hot", 2, "normal", 2) + ImmutableMap.of("hot", 2, "normal", 2), + null ) )).atLeastOnce(); EasyMock.replay(databaseRuleManager); @@ -242,15 +244,18 @@ public void testRunThreeTiersOneReplicant() Lists.newArrayList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T06:00:00.000Z"), - ImmutableMap.of("hot", 1) + ImmutableMap.of("hot", 1), + null ), new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T12:00:00.000Z"), - ImmutableMap.of("normal", 1) + ImmutableMap.of("normal", 1), + null ), new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z"), - ImmutableMap.of("cold", 1) + ImmutableMap.of("cold", 1), + null ) )).atLeastOnce(); EasyMock.replay(databaseRuleManager); @@ -353,11 +358,13 @@ public void testRunTwoTiersTwoReplicants() Lists.newArrayList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T06:00:00.000Z"), - ImmutableMap.of("hot", 2) + ImmutableMap.of("hot", 2), + null ), new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z"), - ImmutableMap.of("cold", 1) + ImmutableMap.of("cold", 1), + null ) ) ).atLeastOnce(); @@ -422,11 +429,13 @@ public void testRunTwoTiersWithExistingSegments() Lists.newArrayList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T12:00:00.000Z"), - ImmutableMap.of("hot", 1) + ImmutableMap.of("hot", 1), + null ), new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z"), - ImmutableMap.of("normal", 1) + ImmutableMap.of("normal", 1), + null ) ) ).atLeastOnce(); @@ -488,11 +497,13 @@ public void testRunTwoTiersTierDoesNotExist() Lists.newArrayList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T12:00:00.000Z"), - ImmutableMap.of("hot", 1) + ImmutableMap.of("hot", 1), + null ), new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z"), - ImmutableMap.of("normal", 1) + ImmutableMap.of("normal", 1), + null ) ) ).atLeastOnce(); @@ -538,7 +549,8 @@ public void testRunRuleDoesNotExist() Collections.singletonList( new IntervalLoadRule( Intervals.of("2012-01-02T00:00:00.000Z/2012-01-03T00:00:00.000Z"), - ImmutableMap.of("normal", 1) + ImmutableMap.of("normal", 1), + null ) ) ) @@ -585,7 +597,8 @@ public void testDropRemove() Lists.newArrayList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T12:00:00.000Z"), - ImmutableMap.of("normal", 1) + ImmutableMap.of("normal", 1), + null ), new IntervalDropRule(Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z")) ) @@ -634,7 +647,8 @@ public void testDropTooManyInSameTier() Lists.newArrayList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T12:00:00.000Z"), - ImmutableMap.of("normal", 1) + ImmutableMap.of("normal", 1), + null ), new IntervalDropRule(Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z")) ) @@ -698,7 +712,8 @@ public void testDropTooManyInDifferentTiers() Lists.newArrayList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T12:00:00.000Z"), - ImmutableMap.of("hot", 1) + ImmutableMap.of("hot", 1), + null ), new IntervalDropRule(Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z")) ) @@ -751,7 +766,8 @@ public void testDontDropInDifferentTiers() Lists.newArrayList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T12:00:00.000Z"), - ImmutableMap.of("hot", 1) + ImmutableMap.of("hot", 1), + null ), new IntervalDropRule(Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z")) ) @@ -801,7 +817,8 @@ public void testDropServerActuallyServesSegment() Collections.singletonList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T01:00:00.000Z"), - ImmutableMap.of("normal", 0) + ImmutableMap.of("normal", 0), + null ) ) ) @@ -879,7 +896,8 @@ public void testReplicantThrottle() Collections.singletonList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2013-01-01T00:00:00.000Z"), - ImmutableMap.of("hot", 2) + ImmutableMap.of("hot", 2), + null ) ) ) @@ -981,7 +999,8 @@ public void testReplicantThrottleAcrossTiers() ImmutableMap.of( "hot", 1, DruidServer.DEFAULT_TIER, 1 - ) + ), + null ) ) ) @@ -1046,7 +1065,8 @@ public void testDropReplicantThrottle() .andReturn( Collections.singletonList(new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2013-01-02T00:00:00.000Z"), - ImmutableMap.of("normal", 1) + ImmutableMap.of("normal", 1), + null ) ) ) @@ -1144,7 +1164,7 @@ public void testRulesRunOnNonOvershadowedSegmentsOnly() mockEmptyPeon(); EasyMock.expect(databaseRuleManager.getRulesWithDefault(EasyMock.anyObject())).andReturn( - Collections.singletonList(new ForeverLoadRule(ImmutableMap.of(DruidServer.DEFAULT_TIER, 1)))).atLeastOnce(); + Collections.singletonList(new ForeverLoadRule(ImmutableMap.of(DruidServer.DEFAULT_TIER, 1), null))).atLeastOnce(); EasyMock.replay(databaseRuleManager); DruidCluster druidCluster = DruidClusterBuilder @@ -1210,7 +1230,8 @@ public void testTwoNodesOneTierThreeReplicantsRandomStrategyNotEnoughNodes() EasyMock.expect(databaseRuleManager.getRulesWithDefault(EasyMock.anyObject())).andReturn( Collections.singletonList( new ForeverLoadRule( - ImmutableMap.of(DruidServer.DEFAULT_TIER, 3) + ImmutableMap.of(DruidServer.DEFAULT_TIER, 3), + null ) )).atLeastOnce(); EasyMock.replay(databaseRuleManager); @@ -1278,7 +1299,8 @@ public void testOneNodesOneTierOneReplicantRandomStrategyEnoughSpace() EasyMock.expect(databaseRuleManager.getRulesWithDefault(EasyMock.anyObject())).andReturn( Collections.singletonList( new ForeverLoadRule( - ImmutableMap.of(DruidServer.DEFAULT_TIER, 1) + ImmutableMap.of(DruidServer.DEFAULT_TIER, 1), + null ) )).atLeastOnce(); EasyMock.replay(databaseRuleManager); @@ -1337,7 +1359,8 @@ public void testOneNodesOneTierOneReplicantRandomStrategyNotEnoughSpace() EasyMock.expect(databaseRuleManager.getRulesWithDefault(EasyMock.anyObject())).andReturn( Collections.singletonList( new ForeverLoadRule( - ImmutableMap.of(DruidServer.DEFAULT_TIER, numReplicants) + ImmutableMap.of(DruidServer.DEFAULT_TIER, numReplicants), + null ) )).atLeastOnce(); EasyMock.replay(databaseRuleManager); @@ -1397,7 +1420,8 @@ public void testOneNodesOneTierOneReplicantCostBalancerStrategyNotEnoughSpace() EasyMock.expect(databaseRuleManager.getRulesWithDefault(EasyMock.anyObject())).andReturn( Collections.singletonList( new ForeverLoadRule( - ImmutableMap.of(DruidServer.DEFAULT_TIER, numReplicants) + ImmutableMap.of(DruidServer.DEFAULT_TIER, numReplicants), + null ) )).atLeastOnce(); EasyMock.replay(databaseRuleManager); diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/UnloadUnusedSegmentsTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/UnloadUnusedSegmentsTest.java index f74762c7ad9d..9bb3b1caf0fb 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/UnloadUnusedSegmentsTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/UnloadUnusedSegmentsTest.java @@ -340,7 +340,8 @@ private static void mockRuleManager(MetadataRuleManager metadataRuleManager) ImmutableMap.of( DruidServer.DEFAULT_TIER, 1, "tier2", 1 - ) + ), + null ) )).anyTimes(); @@ -350,7 +351,8 @@ private static void mockRuleManager(MetadataRuleManager metadataRuleManager) ImmutableMap.of( DruidServer.DEFAULT_TIER, 1, "tier2", 1 - ) + ), + null ) )).anyTimes(); diff --git a/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java b/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java index d4f53da04f5f..32b3b231a802 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java @@ -41,7 +41,8 @@ public void testSerde() throws Exception { IntervalLoadRule rule = new IntervalLoadRule( Intervals.of("0/3000"), - ImmutableMap.of(DruidServer.DEFAULT_TIER, 2) + ImmutableMap.of(DruidServer.DEFAULT_TIER, 2), + null ); ObjectMapper jsonMapper = new DefaultObjectMapper(); @@ -67,7 +68,7 @@ public void testSerdeNullTieredReplicants() throws Exception @Test(expected = IAE.class) public void testCreatingNegativeTieredReplicants() { - new IntervalLoadRule(Intervals.of("0/3000"), ImmutableMap.of(DruidServer.DEFAULT_TIER, -1)); + new IntervalLoadRule(Intervals.of("0/3000"), ImmutableMap.of(DruidServer.DEFAULT_TIER, -1), null); } @Test(expected = IAE.class) @@ -78,7 +79,8 @@ public void testEmptyReplicantValue() throws Exception tieredReplicants.put("tier", null); IntervalLoadRule rule = new IntervalLoadRule( Intervals.of("0/3000"), - tieredReplicants + tieredReplicants, + null ); ObjectMapper jsonMapper = new DefaultObjectMapper(); diff --git a/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java b/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java index 382ddc2c8455..94aadfc5aad3 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java @@ -55,7 +55,8 @@ public void testAppliesToAll() PeriodLoadRule rule = new PeriodLoadRule( new Period("P5000Y"), false, - ImmutableMap.of("", 0) + ImmutableMap.of("", 0), + null ); Assert.assertTrue(rule.appliesTo(BUILDER.interval(Intervals.of("2012-01-01/2012-12-31")).build(), now)); @@ -70,7 +71,8 @@ public void testAppliesToPeriod() PeriodLoadRule rule = new PeriodLoadRule( new Period("P1M"), false, - ImmutableMap.of("", 0) + ImmutableMap.of("", 0), + null ); Assert.assertTrue(rule.appliesTo(BUILDER.interval(new Interval(now.minusWeeks(1), now)).build(), now)); @@ -97,7 +99,8 @@ public void testAppliesToPartialOverlap() PeriodLoadRule rule = new PeriodLoadRule( new Period("P1M"), false, - ImmutableMap.of("", 0) + ImmutableMap.of("", 0), + null ); Assert.assertTrue( @@ -123,12 +126,14 @@ public void testIncludeFuture() PeriodLoadRule includeFutureRule = new PeriodLoadRule( new Period("P2D"), true, - ImmutableMap.of("", 0) + ImmutableMap.of("", 0), + null ); PeriodLoadRule notIncludeFutureRule = new PeriodLoadRule( new Period("P2D"), false, - ImmutableMap.of("", 0) + ImmutableMap.of("", 0), + null ); Assert.assertTrue( @@ -208,7 +213,7 @@ public void testMappingNull() throws Exception @Test(expected = IAE.class) public void testCreatingNegativeTieredReplicants() { - new PeriodLoadRule(Period.days(1), true, ImmutableMap.of(DruidServer.DEFAULT_TIER, -1)); + new PeriodLoadRule(Period.days(1), true, ImmutableMap.of(DruidServer.DEFAULT_TIER, -1), true); } @Test(expected = IAE.class) @@ -220,7 +225,8 @@ public void testEmptyReplicantValue() throws Exception PeriodLoadRule rule = new PeriodLoadRule( Period.days(1), true, - tieredReplicants + tieredReplicants, + true ); ObjectMapper jsonMapper = new DefaultObjectMapper(); diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java index 84c6886d9d40..9b3b76c0332a 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBaseTest.java @@ -289,7 +289,7 @@ Load andOn(String tier, int numReplicas) Rule forever() { - return new ForeverLoadRule(tieredReplicants); + return new ForeverLoadRule(tieredReplicants, null); } } } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestMetadataRuleManager.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestMetadataRuleManager.java index 9ca037b0cfbf..9bb877112cf6 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestMetadataRuleManager.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/TestMetadataRuleManager.java @@ -40,7 +40,7 @@ public TestMetadataRuleManager() { rules.put( DEFAULT_DATASOURCE, - Collections.singletonList(new ForeverLoadRule(null)) + Collections.singletonList(new ForeverLoadRule(null, null)) ); } diff --git a/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java b/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java index e98aaa0eaddf..c360f841baa0 100644 --- a/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java +++ b/server/src/test/java/org/apache/druid/server/http/DataSourcesResourceTest.java @@ -631,7 +631,7 @@ public void testMarkAsUnusedAllSegmentsInDataSource() public void testIsHandOffComplete() { MetadataRuleManager databaseRuleManager = EasyMock.createMock(MetadataRuleManager.class); - Rule loadRule = new IntervalLoadRule(Intervals.of("2013-01-02T00:00:00Z/2013-01-03T00:00:00Z"), null); + Rule loadRule = new IntervalLoadRule(Intervals.of("2013-01-02T00:00:00Z/2013-01-03T00:00:00Z"), null, null); Rule dropRule = new IntervalDropRule(Intervals.of("2013-01-01T00:00:00Z/2013-01-02T00:00:00Z")); DataSourcesResource dataSourcesResource = new DataSourcesResource(inventoryView, null, databaseRuleManager, null, null, null); diff --git a/services/src/test/java/org/apache/druid/server/router/CoordinatorRuleManagerTest.java b/services/src/test/java/org/apache/druid/server/router/CoordinatorRuleManagerTest.java index 89741c0c706b..553b6ef7b278 100644 --- a/services/src/test/java/org/apache/druid/server/router/CoordinatorRuleManagerTest.java +++ b/services/src/test/java/org/apache/druid/server/router/CoordinatorRuleManagerTest.java @@ -49,7 +49,7 @@ public class CoordinatorRuleManagerTest private static final String DATASOURCE1 = "datasource1"; private static final String DATASOURCE2 = "datasource2"; private static final List DEFAULT_RULES = ImmutableList.of( - new ForeverLoadRule(ImmutableMap.of("__default", 2)) + new ForeverLoadRule(ImmutableMap.of("__default", 2), null) ); @org.junit.Rule @@ -109,7 +109,7 @@ public void testGetRulesWithKnownDatasourceReturningAllRulesWithDefaultRule() manager.poll(); final List rules = manager.getRulesWithDefault(DATASOURCE2); final List expectedRules = new ArrayList<>(); - expectedRules.add(new ForeverLoadRule(null)); + expectedRules.add(new ForeverLoadRule(null, null)); expectedRules.add(new IntervalDropRule(Intervals.of("2020-01-01/2020-01-02"))); expectedRules.addAll(DEFAULT_RULES); Assert.assertEquals(expectedRules, rules); @@ -119,16 +119,16 @@ private DruidLeaderClient mockClient() { final Map> rules = ImmutableMap.of( DATASOURCE1, - ImmutableList.of(new ForeverLoadRule(null)), + ImmutableList.of(new ForeverLoadRule(null, null)), DATASOURCE2, - ImmutableList.of(new ForeverLoadRule(null), new IntervalDropRule(Intervals.of("2020-01-01/2020-01-02"))), + ImmutableList.of(new ForeverLoadRule(null, null), new IntervalDropRule(Intervals.of("2020-01-01/2020-01-02"))), "datasource3", ImmutableList.of( - new PeriodLoadRule(new Period("P1M"), true, null), + new PeriodLoadRule(new Period("P1M"), true, null, null), new ForeverDropRule() ), TieredBrokerConfig.DEFAULT_RULE_NAME, - ImmutableList.of(new ForeverLoadRule(ImmutableMap.of("__default", 2))) + ImmutableList.of(new ForeverLoadRule(ImmutableMap.of("__default", 2), null)) ); final StringFullResponseHolder holder = EasyMock.niceMock(StringFullResponseHolder.class); EasyMock.expect(holder.getStatus()) diff --git a/services/src/test/java/org/apache/druid/server/router/TieredBrokerHostSelectorTest.java b/services/src/test/java/org/apache/druid/server/router/TieredBrokerHostSelectorTest.java index 670b61702e2f..8dbf03292360 100644 --- a/services/src/test/java/org/apache/druid/server/router/TieredBrokerHostSelectorTest.java +++ b/services/src/test/java/org/apache/druid/server/router/TieredBrokerHostSelectorTest.java @@ -418,11 +418,12 @@ public boolean isStarted() public List getRulesWithDefault(String dataSource) { return Arrays.asList( - new IntervalLoadRule(Intervals.of("2013/2014"), ImmutableMap.of("hot", 1)), - new IntervalLoadRule(Intervals.of("2012/2013"), ImmutableMap.of("medium", 1)), + new IntervalLoadRule(Intervals.of("2013/2014"), ImmutableMap.of("hot", 1), null), + new IntervalLoadRule(Intervals.of("2012/2013"), ImmutableMap.of("medium", 1), null), new IntervalLoadRule( Intervals.of("2011/2012"), - ImmutableMap.of(DruidServer.DEFAULT_TIER, 1) + ImmutableMap.of(DruidServer.DEFAULT_TIER, 1), + null ) ); } From 2a5dc1a6662280691434cbccfad286274f0dc8d2 Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Mon, 19 Jun 2023 12:42:31 +0530 Subject: [PATCH 05/18] Correct validation --- .../coordinator/rules/ForeverLoadRule.java | 2 +- .../coordinator/rules/IntervalLoadRule.java | 2 +- .../server/coordinator/rules/LoadRule.java | 6 +-- .../coordinator/rules/PeriodLoadRule.java | 2 +- .../rules/ForeverLoadRuleTest.java | 30 ++------------ .../rules/IntervalLoadRuleTest.java | 18 +-------- .../coordinator/rules/LoadRuleTest.java | 7 ++++ .../coordinator/rules/PeriodLoadRuleTest.java | 39 ++----------------- 8 files changed, 21 insertions(+), 85 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/ForeverLoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/ForeverLoadRule.java index dab067c7c370..73c03bcbfaee 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/ForeverLoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/ForeverLoadRule.java @@ -50,7 +50,7 @@ public ForeverLoadRule( } else { this.tieredReplicants = tieredReplicants == null ? ImmutableMap.of() : tieredReplicants; } - validateTieredReplicants(this.tieredReplicants); + validateTieredReplicants(this.tieredReplicants, this.allowEmptyTieredReplicants); } @JsonProperty diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/IntervalLoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/IntervalLoadRule.java index d83ef5aadeea..58e77731e24c 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/IntervalLoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/IntervalLoadRule.java @@ -54,7 +54,7 @@ public IntervalLoadRule( } else { this.tieredReplicants = tieredReplicants == null ? ImmutableMap.of() : tieredReplicants; } - validateTieredReplicants(this.tieredReplicants); + validateTieredReplicants(this.tieredReplicants, this.allowEmptyTieredReplicants); this.interval = interval; } diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java index 0e72e21c036d..61fabd9beda6 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java @@ -550,10 +550,10 @@ private static int dropSegmentFromServers( return numToDrop; } - protected static void validateTieredReplicants(final Map tieredReplicants) + protected static void validateTieredReplicants(final Map tieredReplicants, boolean allowEmptyTieredReplicants) { - if (tieredReplicants == null || tieredReplicants.isEmpty()) { - return; + if (tieredReplicants.size() == 0 && !allowEmptyTieredReplicants) { + throw new IAE("A rule with empty tiered replicants is invalid unless \"allowEmptyTieredReplicants\" is set to true."); } for (Map.Entry entry : tieredReplicants.entrySet()) { if (entry.getValue() == null) { diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java index 0129b3b2ea31..db16e85bbcbb 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java @@ -66,7 +66,7 @@ public PeriodLoadRule( } else { this.tieredReplicants = tieredReplicants == null ? ImmutableMap.of() : tieredReplicants; } - validateTieredReplicants(this.tieredReplicants); + validateTieredReplicants(this.tieredReplicants, this.allowEmptyTieredReplicants); this.period = period; this.includeFuture = includeFuture == null ? DEFAULT_INCLUDE_FUTURE : includeFuture; } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/rules/ForeverLoadRuleTest.java b/server/src/test/java/org/apache/druid/server/coordinator/rules/ForeverLoadRuleTest.java index 07f631feb72b..19bdc63597dd 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/rules/ForeverLoadRuleTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/rules/ForeverLoadRuleTest.java @@ -33,7 +33,7 @@ public class ForeverLoadRuleTest { @Test - public void testSerdeNullTieredReplicantWithoutAllowEmptyFlag() throws Exception + public void testSerde() throws Exception { ForeverLoadRule rule = new ForeverLoadRule(null, null); @@ -44,18 +44,6 @@ public void testSerdeNullTieredReplicantWithoutAllowEmptyFlag() throws Exception Assert.assertEquals(ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), rule.getTieredReplicants()); } - @Test - public void testSerdeNullTieredReplicantWithAllowEmptyFlag() throws Exception - { - ForeverLoadRule rule = new ForeverLoadRule(null, true); - - ObjectMapper jsonMapper = new DefaultObjectMapper(); - Rule reread = jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); - - Assert.assertEquals(rule.getTieredReplicants(), ((ForeverLoadRule) reread).getTieredReplicants()); - Assert.assertEquals(ImmutableMap.of(), rule.getTieredReplicants()); - } - @Test(expected = IAE.class) public void testCreatingNegativeTieredReplicants() { @@ -95,16 +83,9 @@ public void testWithoutAllowEmptyReplicantShouldCreateDefaultTier() throws Excep String inputJson = " {\n" + " \"type\": \"loadForever\"\n" + " }"; - String expectedJson = " {\n" - + " \"tieredReplicants\": {\n" - + " \"" + DruidServer.DEFAULT_TIER + "\": " + DruidServer.DEFAULT_NUM_REPLICANTS + "\n" - + " },\n" - + " \"type\": \"loadForever\"\n" - + " }"; ObjectMapper jsonMapper = new DefaultObjectMapper(); ForeverLoadRule inputForeverLoadRule = jsonMapper.readValue(inputJson, ForeverLoadRule.class); - ForeverLoadRule expectedForeverLoadRule = jsonMapper.readValue(expectedJson, ForeverLoadRule.class); - Assert.assertEquals(expectedForeverLoadRule.getTieredReplicants(), inputForeverLoadRule.getTieredReplicants()); + Assert.assertEquals(ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), inputForeverLoadRule.getTieredReplicants()); } @Test @@ -114,13 +95,8 @@ public void testWithAllowEmptyReplicantShouldDefaultToEmpty() throws Exception + " \"type\": \"loadForever\"\n," + " \"allowEmptyTieredReplicants\": \"true\"\n" + " }"; - String expectedJson = " {\n" - + " \"tieredReplicants\": {},\n" - + " \"type\": \"loadForever\"\n" - + " }"; ObjectMapper jsonMapper = new DefaultObjectMapper(); ForeverLoadRule inputForeverLoadRule = jsonMapper.readValue(inputJson, ForeverLoadRule.class); - ForeverLoadRule expectedForeverLoadRule = jsonMapper.readValue(expectedJson, ForeverLoadRule.class); - Assert.assertEquals(expectedForeverLoadRule.getTieredReplicants(), inputForeverLoadRule.getTieredReplicants()); + Assert.assertEquals(ImmutableMap.of(), inputForeverLoadRule.getTieredReplicants()); } } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java b/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java index 32b3b231a802..4f305fe724d9 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java @@ -94,17 +94,9 @@ public void testWithoutAllowEmptyReplicantShouldCreateDefaultTier() throws Excep + " \"interval\": \"0000-01-01T00:00:00.000-05:50:36/3000-01-01T00:00:00.000-06:00\",\n" + " \"type\": \"loadByInterval\"\n" + " }"; - String expectedJson = " {\n" - + " \"interval\": \"0000-01-01T00:00:00.000-05:50:36/3000-01-01T00:00:00.000-06:00\",\n" - + " \"tieredReplicants\": {\n" - + " \"" + DruidServer.DEFAULT_TIER + "\": " + DruidServer.DEFAULT_NUM_REPLICANTS + "\n" - + " },\n" - + " \"type\": \"loadByInterval\"\n" - + " }"; ObjectMapper jsonMapper = new DefaultObjectMapper(); IntervalLoadRule inputForeverLoadRule = jsonMapper.readValue(inputJson, IntervalLoadRule.class); - IntervalLoadRule expectedForeverLoadRule = jsonMapper.readValue(expectedJson, IntervalLoadRule.class); - Assert.assertEquals(expectedForeverLoadRule.getTieredReplicants(), inputForeverLoadRule.getTieredReplicants()); + Assert.assertEquals(ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), inputForeverLoadRule.getTieredReplicants()); } @Test @@ -115,14 +107,8 @@ public void testWithAllowEmptyReplicantShouldDefaultToEmpty() throws Exception + " \"type\": \"loadByInterval\",\n" + " \"allowEmptyTieredReplicants\": \"true\"\n" + " }"; - String expectedJson = " {\n" - + " \"interval\": \"0000-01-01T00:00:00.000-05:50:36/3000-01-01T00:00:00.000-06:00\",\n" - + " \"tieredReplicants\": {},\n" - + " \"type\": \"loadByInterval\"\n" - + " }"; ObjectMapper jsonMapper = new DefaultObjectMapper(); IntervalLoadRule inputForeverLoadRule = jsonMapper.readValue(inputJson, IntervalLoadRule.class); - IntervalLoadRule expectedForeverLoadRule = jsonMapper.readValue(expectedJson, IntervalLoadRule.class); - Assert.assertEquals(expectedForeverLoadRule.getTieredReplicants(), inputForeverLoadRule.getTieredReplicants()); + Assert.assertEquals(ImmutableMap.of(), inputForeverLoadRule.getTieredReplicants()); } } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/rules/LoadRuleTest.java b/server/src/test/java/org/apache/druid/server/coordinator/rules/LoadRuleTest.java index 642823d186c2..99e1c2fa52ac 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/rules/LoadRuleTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/rules/LoadRuleTest.java @@ -29,6 +29,7 @@ import org.apache.druid.client.ImmutableDruidServer; import org.apache.druid.jackson.DefaultObjectMapper; import org.apache.druid.java.util.common.DateTimes; +import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.Intervals; import org.apache.druid.java.util.common.JodaUtils; import org.apache.druid.java.util.common.concurrent.Execs; @@ -892,6 +893,12 @@ public void testRedundantReplicaDropDuringDecommissioning() EasyMock.verify(throttler); } + @Test(expected = IAE.class) + public void testValidateTieredReplicantsEmptyTierReplicantsWithoutAllowEmptyFlag() + { + new ForeverLoadRule(ImmutableMap.of(), null); + } + private DataSegment createDataSegment(String dataSource) { return new DataSegment( diff --git a/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java b/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java index 94aadfc5aad3..c9211a8f0504 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java @@ -154,7 +154,7 @@ public void testIncludeFuture() * test serialize/deserilize null values of {@link PeriodLoadRule#tieredReplicants} and {@link PeriodLoadRule#includeFuture} */ @Test - public void testSerdeNullTieredReplicantWithoutAllowEmptyFlag() throws Exception + public void testSerdeNull() throws Exception { PeriodLoadRule rule = new PeriodLoadRule( new Period("P1D"), null, null, null @@ -170,23 +170,6 @@ public void testSerdeNullTieredReplicantWithoutAllowEmptyFlag() throws Exception Assert.assertEquals(ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), rule.getTieredReplicants()); } - @Test - public void testSerdeNullTieredReplicantWithAllowEmptyFlag() throws Exception - { - PeriodLoadRule rule = new PeriodLoadRule( - new Period("P1D"), null, null, true - ); - - ObjectMapper jsonMapper = new DefaultObjectMapper(); - Rule reread = jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); - - Assert.assertEquals(rule.getPeriod(), ((PeriodLoadRule) reread).getPeriod()); - Assert.assertEquals(rule.isIncludeFuture(), ((PeriodLoadRule) reread).isIncludeFuture()); - Assert.assertEquals(PeriodLoadRule.DEFAULT_INCLUDE_FUTURE, rule.isIncludeFuture()); - Assert.assertEquals(rule.getTieredReplicants(), ((PeriodLoadRule) reread).getTieredReplicants()); - Assert.assertEquals(ImmutableMap.of(), rule.getTieredReplicants()); - } - /** * test mapping null values of {@link PeriodLoadRule#tieredReplicants} and {@link PeriodLoadRule#includeFuture} */ @@ -242,18 +225,9 @@ public void testWithoutAllowEmptyReplicantShouldCreateDefaultTier() throws Excep + " \"includeFuture\": " + PeriodLoadRule.DEFAULT_INCLUDE_FUTURE + ",\n" + " \"type\": \"loadByPeriod\"\n" + " }"; - String expectedJson = " {\n" - + " \"period\": \"P1D\",\n" - + " \"includeFuture\": " + PeriodLoadRule.DEFAULT_INCLUDE_FUTURE + ",\n" - + " \"tieredReplicants\": {\n" - + " \"" + DruidServer.DEFAULT_TIER + "\": " + DruidServer.DEFAULT_NUM_REPLICANTS + "\n" - + " },\n" - + " \"type\": \"loadByPeriod\"\n" - + " }"; ObjectMapper jsonMapper = new DefaultObjectMapper(); PeriodLoadRule inputPeriodLoadRule = jsonMapper.readValue(inputJson, PeriodLoadRule.class); - PeriodLoadRule expectedPeriodLoadRule = jsonMapper.readValue(expectedJson, PeriodLoadRule.class); - Assert.assertEquals(expectedPeriodLoadRule.getTieredReplicants(), inputPeriodLoadRule.getTieredReplicants()); + Assert.assertEquals(ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), inputPeriodLoadRule.getTieredReplicants()); } @Test @@ -265,15 +239,8 @@ public void testWithAllowEmptyReplicantShouldDefaultToEmpty() throws Exception + " \"allowEmptyTieredReplicants\": \"true\",\n" + " \"type\": \"loadByPeriod\"\n" + " }"; - String expectedJson = " {\n" - + " \"period\": \"P1D\",\n" - + " \"includeFuture\": " + PeriodLoadRule.DEFAULT_INCLUDE_FUTURE + ",\n" - + " \"tieredReplicants\": {},\n" - + " \"type\": \"loadByPeriod\"\n" - + " }"; ObjectMapper jsonMapper = new DefaultObjectMapper(); PeriodLoadRule inputPeriodLoadRule = jsonMapper.readValue(inputJson, PeriodLoadRule.class); - PeriodLoadRule expectedPeriodLoadRule = jsonMapper.readValue(expectedJson, PeriodLoadRule.class); - Assert.assertEquals(expectedPeriodLoadRule.getTieredReplicants(), inputPeriodLoadRule.getTieredReplicants()); + Assert.assertEquals(ImmutableMap.of(), inputPeriodLoadRule.getTieredReplicants()); } } From 264f6639dcbf4ff380230c5ac3267a4b252de69f Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Mon, 19 Jun 2023 16:08:56 +0530 Subject: [PATCH 06/18] Reover RunRulesTest --- .../server/coordinator/RunRulesTest.java | 74 +++++++------------ 1 file changed, 25 insertions(+), 49 deletions(-) diff --git a/server/src/test/java/org/apache/druid/server/coordinator/RunRulesTest.java b/server/src/test/java/org/apache/druid/server/coordinator/RunRulesTest.java index c9fe01dcb854..3256a0ad766b 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/RunRulesTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/RunRulesTest.java @@ -114,8 +114,7 @@ public void testOneTierTwoReplicantsWithStrictReplicantLimit() Collections.singletonList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z"), - ImmutableMap.of("normal", 2), - null + ImmutableMap.of("normal", 2) ) )).atLeastOnce(); EasyMock.replay(databaseRuleManager); @@ -173,8 +172,7 @@ public void testTwoTiersTwoReplicantsWithStrictReplicantLimit() Collections.singletonList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z"), - ImmutableMap.of("hot", 2, "normal", 2), - null + ImmutableMap.of("hot", 2, "normal", 2) ) )).atLeastOnce(); EasyMock.replay(databaseRuleManager); @@ -244,18 +242,15 @@ public void testRunThreeTiersOneReplicant() Lists.newArrayList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T06:00:00.000Z"), - ImmutableMap.of("hot", 1), - null + ImmutableMap.of("hot", 1) ), new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T12:00:00.000Z"), - ImmutableMap.of("normal", 1), - null + ImmutableMap.of("normal", 1) ), new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z"), - ImmutableMap.of("cold", 1), - null + ImmutableMap.of("cold", 1) ) )).atLeastOnce(); EasyMock.replay(databaseRuleManager); @@ -358,13 +353,11 @@ public void testRunTwoTiersTwoReplicants() Lists.newArrayList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T06:00:00.000Z"), - ImmutableMap.of("hot", 2), - null + ImmutableMap.of("hot", 2) ), new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z"), - ImmutableMap.of("cold", 1), - null + ImmutableMap.of("cold", 1) ) ) ).atLeastOnce(); @@ -429,13 +422,11 @@ public void testRunTwoTiersWithExistingSegments() Lists.newArrayList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T12:00:00.000Z"), - ImmutableMap.of("hot", 1), - null + ImmutableMap.of("hot", 1) ), new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z"), - ImmutableMap.of("normal", 1), - null + ImmutableMap.of("normal", 1) ) ) ).atLeastOnce(); @@ -497,13 +488,11 @@ public void testRunTwoTiersTierDoesNotExist() Lists.newArrayList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T12:00:00.000Z"), - ImmutableMap.of("hot", 1), - null + ImmutableMap.of("hot", 1) ), new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z"), - ImmutableMap.of("normal", 1), - null + ImmutableMap.of("normal", 1) ) ) ).atLeastOnce(); @@ -549,8 +538,7 @@ public void testRunRuleDoesNotExist() Collections.singletonList( new IntervalLoadRule( Intervals.of("2012-01-02T00:00:00.000Z/2012-01-03T00:00:00.000Z"), - ImmutableMap.of("normal", 1), - null + ImmutableMap.of("normal", 1) ) ) ) @@ -597,8 +585,7 @@ public void testDropRemove() Lists.newArrayList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T12:00:00.000Z"), - ImmutableMap.of("normal", 1), - null + ImmutableMap.of("normal", 1) ), new IntervalDropRule(Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z")) ) @@ -647,8 +634,7 @@ public void testDropTooManyInSameTier() Lists.newArrayList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T12:00:00.000Z"), - ImmutableMap.of("normal", 1), - null + ImmutableMap.of("normal", 1) ), new IntervalDropRule(Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z")) ) @@ -712,8 +698,7 @@ public void testDropTooManyInDifferentTiers() Lists.newArrayList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T12:00:00.000Z"), - ImmutableMap.of("hot", 1), - null + ImmutableMap.of("hot", 1) ), new IntervalDropRule(Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z")) ) @@ -766,8 +751,7 @@ public void testDontDropInDifferentTiers() Lists.newArrayList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T12:00:00.000Z"), - ImmutableMap.of("hot", 1), - null + ImmutableMap.of("hot", 1) ), new IntervalDropRule(Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z")) ) @@ -817,8 +801,7 @@ public void testDropServerActuallyServesSegment() Collections.singletonList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T01:00:00.000Z"), - ImmutableMap.of("normal", 0), - null + ImmutableMap.of("normal", 0) ) ) ) @@ -896,8 +879,7 @@ public void testReplicantThrottle() Collections.singletonList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2013-01-01T00:00:00.000Z"), - ImmutableMap.of("hot", 2), - null + ImmutableMap.of("hot", 2) ) ) ) @@ -999,8 +981,7 @@ public void testReplicantThrottleAcrossTiers() ImmutableMap.of( "hot", 1, DruidServer.DEFAULT_TIER, 1 - ), - null + ) ) ) ) @@ -1065,8 +1046,7 @@ public void testDropReplicantThrottle() .andReturn( Collections.singletonList(new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2013-01-02T00:00:00.000Z"), - ImmutableMap.of("normal", 1), - null + ImmutableMap.of("normal", 1) ) ) ) @@ -1164,7 +1144,7 @@ public void testRulesRunOnNonOvershadowedSegmentsOnly() mockEmptyPeon(); EasyMock.expect(databaseRuleManager.getRulesWithDefault(EasyMock.anyObject())).andReturn( - Collections.singletonList(new ForeverLoadRule(ImmutableMap.of(DruidServer.DEFAULT_TIER, 1), null))).atLeastOnce(); + Collections.singletonList(new ForeverLoadRule(ImmutableMap.of(DruidServer.DEFAULT_TIER, 1)))).atLeastOnce(); EasyMock.replay(databaseRuleManager); DruidCluster druidCluster = DruidClusterBuilder @@ -1230,8 +1210,7 @@ public void testTwoNodesOneTierThreeReplicantsRandomStrategyNotEnoughNodes() EasyMock.expect(databaseRuleManager.getRulesWithDefault(EasyMock.anyObject())).andReturn( Collections.singletonList( new ForeverLoadRule( - ImmutableMap.of(DruidServer.DEFAULT_TIER, 3), - null + ImmutableMap.of(DruidServer.DEFAULT_TIER, 3) ) )).atLeastOnce(); EasyMock.replay(databaseRuleManager); @@ -1299,8 +1278,7 @@ public void testOneNodesOneTierOneReplicantRandomStrategyEnoughSpace() EasyMock.expect(databaseRuleManager.getRulesWithDefault(EasyMock.anyObject())).andReturn( Collections.singletonList( new ForeverLoadRule( - ImmutableMap.of(DruidServer.DEFAULT_TIER, 1), - null + ImmutableMap.of(DruidServer.DEFAULT_TIER, 1) ) )).atLeastOnce(); EasyMock.replay(databaseRuleManager); @@ -1359,8 +1337,7 @@ public void testOneNodesOneTierOneReplicantRandomStrategyNotEnoughSpace() EasyMock.expect(databaseRuleManager.getRulesWithDefault(EasyMock.anyObject())).andReturn( Collections.singletonList( new ForeverLoadRule( - ImmutableMap.of(DruidServer.DEFAULT_TIER, numReplicants), - null + ImmutableMap.of(DruidServer.DEFAULT_TIER, numReplicants) ) )).atLeastOnce(); EasyMock.replay(databaseRuleManager); @@ -1420,8 +1397,7 @@ public void testOneNodesOneTierOneReplicantCostBalancerStrategyNotEnoughSpace() EasyMock.expect(databaseRuleManager.getRulesWithDefault(EasyMock.anyObject())).andReturn( Collections.singletonList( new ForeverLoadRule( - ImmutableMap.of(DruidServer.DEFAULT_TIER, numReplicants), - null + ImmutableMap.of(DruidServer.DEFAULT_TIER, numReplicants) ) )).atLeastOnce(); EasyMock.replay(databaseRuleManager); From f0486a84d23b3c1d22c1e1324fab233232b11052 Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Mon, 19 Jun 2023 16:17:58 +0530 Subject: [PATCH 07/18] Resolve errors during merge --- .../server/coordinator/rules/LoadRule.java | 32 ++------- .../server/coordinator/duty/RunRulesTest.java | 72 ++++++++++++------- .../coordinator/rules/LoadRuleTest.java | 2 +- 3 files changed, 55 insertions(+), 51 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java index 55fe71ce9bc5..f95ae572fd88 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java @@ -19,35 +19,12 @@ package org.apache.druid.server.coordinator.rules; -import it.unimi.dsi.fastutil.objects.Object2IntMap; -import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap; -import it.unimi.dsi.fastutil.objects.Object2LongMap; -import it.unimi.dsi.fastutil.objects.Object2LongOpenHashMap; +import com.google.common.collect.ImmutableMap; import org.apache.druid.client.DruidServer; import org.apache.druid.java.util.common.IAE; -import org.apache.druid.java.util.common.StringUtils; -import org.apache.druid.java.util.emitter.EmittingLogger; -import org.apache.druid.server.coordinator.BalancerStrategy; -import org.apache.druid.server.coordinator.CoordinatorStats; -import org.apache.druid.server.coordinator.DruidCluster; -import org.apache.druid.server.coordinator.DruidCoordinator; -import org.apache.druid.server.coordinator.DruidCoordinatorRuntimeParams; -import org.apache.druid.server.coordinator.ReplicationThrottler; -import org.apache.druid.server.coordinator.SegmentReplicantLookup; -import org.apache.druid.server.coordinator.ServerHolder; import org.apache.druid.timeline.DataSegment; -import org.apache.druid.timeline.SegmentId; -import javax.annotation.Nullable; -import java.util.Collections; -import java.util.HashMap; -import java.util.Iterator; -import java.util.List; import java.util.Map; -import java.util.NavigableSet; -import java.util.TreeSet; -import java.util.function.Predicate; -import java.util.stream.Collectors; /** * LoadRules indicate the number of replicants a segment should have in a given tier. @@ -57,7 +34,12 @@ public abstract class LoadRule implements Rule @Override public void run(DataSegment segment, SegmentActionHandler handler) { - handler.replicateSegment(segment, getTieredReplicants()); + Map tieredReplicants = getTieredReplicants(); + if (tieredReplicants.isEmpty()) { + handler.replicateSegment(segment, ImmutableMap.of(DruidServer.DEFAULT_TIER, 0)); + } else { + handler.replicateSegment(segment, tieredReplicants); + } } protected static void validateTieredReplicants(final Map tieredReplicants, boolean allowEmptyTieredReplicants) diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/RunRulesTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/RunRulesTest.java index abc218d89460..cd9c1e228da1 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/RunRulesTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/RunRulesTest.java @@ -129,7 +129,8 @@ public void testOneTierTwoReplicantsWithStrictReplicantLimit() Collections.singletonList( new IntervalLoadRule( Intervals.of("2012-01-01/2012-01-02"), - ImmutableMap.of("normal", 2) + ImmutableMap.of("normal", 2), + null ) )).atLeastOnce(); EasyMock.replay(databaseRuleManager); @@ -185,7 +186,8 @@ public void testTwoTiersTwoReplicantsWithStrictReplicantLimit() Collections.singletonList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z"), - ImmutableMap.of("hot", 2, "normal", 2) + ImmutableMap.of("hot", 2, "normal", 2), + null ) )).atLeastOnce(); EasyMock.replay(databaseRuleManager); @@ -248,15 +250,18 @@ public void testRunThreeTiersOneReplicant() Lists.newArrayList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T06:00:00.000Z"), - ImmutableMap.of("hot", 1) + ImmutableMap.of("hot", 1), + null ), new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T12:00:00.000Z"), - ImmutableMap.of("normal", 1) + ImmutableMap.of("normal", 1), + null ), new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z"), - ImmutableMap.of("cold", 1) + ImmutableMap.of("cold", 1), + null ) )).atLeastOnce(); EasyMock.replay(databaseRuleManager); @@ -353,11 +358,13 @@ public void testRunTwoTiersTwoReplicants() Lists.newArrayList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T06:00:00.000Z"), - ImmutableMap.of("hot", 2) + ImmutableMap.of("hot", 2), + null ), new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z"), - ImmutableMap.of("cold", 1) + ImmutableMap.of("cold", 1), + null ) ) ).atLeastOnce(); @@ -399,11 +406,13 @@ public void testRunTwoTiersWithExistingSegments() Lists.newArrayList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T12:00:00.000Z"), - ImmutableMap.of("hot", 1) + ImmutableMap.of("hot", 1), + null ), new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z"), - ImmutableMap.of("normal", 1) + ImmutableMap.of("normal", 1), + null ) ) ).atLeastOnce(); @@ -445,11 +454,13 @@ public void testRunTwoTiersTierDoesNotExist() Lists.newArrayList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T12:00:00.000Z"), - ImmutableMap.of("hot", 1) + ImmutableMap.of("hot", 1), + null ), new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z"), - ImmutableMap.of("normal", 1) + ImmutableMap.of("normal", 1), + null ) ) ).atLeastOnce(); @@ -480,7 +491,8 @@ public void testRunRuleDoesNotExist() Collections.singletonList( new IntervalLoadRule( Intervals.of("2012-01-02T00:00:00.000Z/2012-01-03T00:00:00.000Z"), - ImmutableMap.of("normal", 1) + ImmutableMap.of("normal", 1), + null ) ) ) @@ -527,7 +539,8 @@ public void testDropRemove() Lists.newArrayList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T12:00:00.000Z"), - ImmutableMap.of("normal", 1) + ImmutableMap.of("normal", 1), + null ), new IntervalDropRule(Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z")) ) @@ -564,7 +577,8 @@ public void testDropTooManyInSameTier() Lists.newArrayList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T12:00:00.000Z"), - ImmutableMap.of("normal", 1) + ImmutableMap.of("normal", 1), + null ), new IntervalDropRule(Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z")) ) @@ -620,7 +634,8 @@ public void testDropTooManyInDifferentTiers() Lists.newArrayList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T12:00:00.000Z"), - ImmutableMap.of("hot", 1) + ImmutableMap.of("hot", 1), + null ), new IntervalDropRule(Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z")) ) @@ -663,7 +678,8 @@ public void testDontDropInDifferentTiers() Lists.newArrayList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T12:00:00.000Z"), - ImmutableMap.of("hot", 1) + ImmutableMap.of("hot", 1), + null ), new IntervalDropRule(Intervals.of("2012-01-01T00:00:00.000Z/2012-01-02T00:00:00.000Z")) ) @@ -703,7 +719,8 @@ public void testDropServerActuallyServesSegment() Collections.singletonList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2012-01-01T01:00:00.000Z"), - ImmutableMap.of("normal", 0) + ImmutableMap.of("normal", 0), + null ) ) ) @@ -768,7 +785,8 @@ public void testNoThrottleWhenSegmentNotLoadedInTier() Collections.singletonList( new IntervalLoadRule( Intervals.of("2012-01-01T00:00:00.000Z/2013-01-01T00:00:00.000Z"), - ImmutableMap.of("hot", 2) + ImmutableMap.of("hot", 2), + null ) ) ) @@ -847,7 +865,8 @@ public void testReplicantThrottleAcrossTiers() Collections.singletonList( new IntervalLoadRule( Intervals.of("2012-01-01/2013-01-01"), - ImmutableMap.of("hot", 1, DruidServer.DEFAULT_TIER, 1) + ImmutableMap.of("hot", 1, DruidServer.DEFAULT_TIER, 1), + null ) ) ) @@ -899,7 +918,8 @@ public void testDropReplicantThrottle() Collections.singletonList( new IntervalLoadRule( Intervals.of("2012-01-01/2013-01-02"), - ImmutableMap.of("normal", 1) + ImmutableMap.of("normal", 1), + null ) ) ) @@ -982,7 +1002,7 @@ public void testRulesRunOnNonOvershadowedSegmentsOnly() mockEmptyPeon(); EasyMock.expect(databaseRuleManager.getRulesWithDefault(EasyMock.anyObject())).andReturn( - Collections.singletonList(new ForeverLoadRule(ImmutableMap.of(DruidServer.DEFAULT_TIER, 1)))).atLeastOnce(); + Collections.singletonList(new ForeverLoadRule(ImmutableMap.of(DruidServer.DEFAULT_TIER, 1), null))).atLeastOnce(); EasyMock.replay(databaseRuleManager); DruidCluster druidCluster = DruidCluster.builder().add( @@ -1019,7 +1039,7 @@ public void testTwoNodesOneTierThreeReplicantsRandomStrategyNotEnoughNodes() EasyMock.expect(databaseRuleManager.getRulesWithDefault(EasyMock.anyObject())).andReturn( Collections.singletonList( - new ForeverLoadRule(ImmutableMap.of(DruidServer.DEFAULT_TIER, 3)) + new ForeverLoadRule(ImmutableMap.of(DruidServer.DEFAULT_TIER, 3), null) )).atLeastOnce(); EasyMock.replay(databaseRuleManager); @@ -1081,7 +1101,7 @@ public void testOneNodesOneTierOneReplicantRandomStrategyEnoughSpace() EasyMock.expect(databaseRuleManager.getRulesWithDefault(EasyMock.anyObject())).andReturn( Collections.singletonList( - new ForeverLoadRule(ImmutableMap.of(DruidServer.DEFAULT_TIER, 1)) + new ForeverLoadRule(ImmutableMap.of(DruidServer.DEFAULT_TIER, 1), null) )).atLeastOnce(); EasyMock.replay(databaseRuleManager); @@ -1133,7 +1153,8 @@ public void testOneNodesOneTierOneReplicantRandomStrategyNotEnoughSpace() EasyMock.expect(databaseRuleManager.getRulesWithDefault(EasyMock.anyObject())).andReturn( Collections.singletonList( new ForeverLoadRule( - ImmutableMap.of(DruidServer.DEFAULT_TIER, numReplicants) + ImmutableMap.of(DruidServer.DEFAULT_TIER, numReplicants), + null ) )).atLeastOnce(); EasyMock.replay(databaseRuleManager); @@ -1194,7 +1215,8 @@ public void testOneNodesOneTierOneReplicantCostBalancerStrategyNotEnoughSpace() EasyMock.expect(databaseRuleManager.getRulesWithDefault(EasyMock.anyObject())).andReturn( Collections.singletonList( new ForeverLoadRule( - ImmutableMap.of(DruidServer.DEFAULT_TIER, numReplicants) + ImmutableMap.of(DruidServer.DEFAULT_TIER, numReplicants), + null ) )).atLeastOnce(); EasyMock.replay(databaseRuleManager); diff --git a/server/src/test/java/org/apache/druid/server/coordinator/rules/LoadRuleTest.java b/server/src/test/java/org/apache/druid/server/coordinator/rules/LoadRuleTest.java index 6a891c2e6ba0..c7190c1514a1 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/rules/LoadRuleTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/rules/LoadRuleTest.java @@ -712,7 +712,7 @@ private DataSegment createDataSegment(String dataSource) private static LoadRule loadForever(final Map tieredReplicants) { - return new ForeverLoadRule(tieredReplicants); + return new ForeverLoadRule(tieredReplicants, null); } private static LoadQueuePeon createEmptyPeon() From e3f32462b1acd401ba8b65bdfe529cc9de322601 Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Mon, 19 Jun 2023 21:47:25 +0530 Subject: [PATCH 08/18] Rename parameter and refactor code --- .../coordinator/rules/ForeverLoadRule.java | 27 ++++++--------- .../coordinator/rules/IntervalLoadRule.java | 25 ++++++-------- .../server/coordinator/rules/LoadRule.java | 27 +++++++++++++-- .../coordinator/rules/PeriodLoadRule.java | 33 ++++++------------- .../rules/ForeverLoadRuleTest.java | 20 ++++++++--- .../rules/IntervalLoadRuleTest.java | 29 +++++++++++----- .../coordinator/rules/PeriodLoadRuleTest.java | 20 +++++++++-- 7 files changed, 108 insertions(+), 73 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/ForeverLoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/ForeverLoadRule.java index 90abac94213e..b9cf134b9e1d 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/ForeverLoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/ForeverLoadRule.java @@ -21,12 +21,12 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; -import com.google.common.collect.ImmutableMap; -import org.apache.druid.client.DruidServer; +import org.apache.druid.common.config.Configs; import org.apache.druid.timeline.DataSegment; import org.joda.time.DateTime; import org.joda.time.Interval; +import javax.annotation.Nullable; import java.util.Map; import java.util.Objects; @@ -35,30 +35,23 @@ public class ForeverLoadRule extends LoadRule { private final Map tieredReplicants; - private final boolean allowEmptyTieredReplicants; + private final boolean useDefaultTierForNull; @JsonCreator public ForeverLoadRule( @JsonProperty("tieredReplicants") Map tieredReplicants, - @JsonProperty("allowEmptyTieredReplicants") Boolean allowEmptyTieredReplicants + @JsonProperty("useDefaultTierForNull") @Nullable Boolean useDefaultTierForNull ) { - this.allowEmptyTieredReplicants = allowEmptyTieredReplicants != null && allowEmptyTieredReplicants; - - if (!this.allowEmptyTieredReplicants) { - this.tieredReplicants = tieredReplicants == null - ? ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS) - : tieredReplicants; - } else { - this.tieredReplicants = tieredReplicants == null ? ImmutableMap.of() : tieredReplicants; - } - validateTieredReplicants(this.tieredReplicants, this.allowEmptyTieredReplicants); + this.useDefaultTierForNull = Configs.valueOrDefault(useDefaultTierForNull, true); + this.tieredReplicants = createTieredReplicants(tieredReplicants, this.useDefaultTierForNull); + validateTieredReplicants(this.tieredReplicants, this.useDefaultTierForNull); } - @JsonProperty - public boolean isAllowEmptyTieredReplicants() + @JsonProperty("useDefaultTierForNull") + public boolean useDefaultTierForNull() { - return allowEmptyTieredReplicants; + return useDefaultTierForNull; } @Override diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/IntervalLoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/IntervalLoadRule.java index 58e77731e24c..f0b0884d9a66 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/IntervalLoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/IntervalLoadRule.java @@ -21,13 +21,13 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; -import com.google.common.collect.ImmutableMap; -import org.apache.druid.client.DruidServer; +import org.apache.druid.common.config.Configs; import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.timeline.DataSegment; import org.joda.time.DateTime; import org.joda.time.Interval; +import javax.annotation.Nullable; import java.util.Map; /** @@ -38,23 +38,18 @@ public class IntervalLoadRule extends LoadRule private final Interval interval; private final Map tieredReplicants; - private final boolean allowEmptyTieredReplicants; + private final boolean useDefaultTierForNull; @JsonCreator public IntervalLoadRule( @JsonProperty("interval") Interval interval, @JsonProperty("tieredReplicants") Map tieredReplicants, - @JsonProperty("allowEmptyTieredReplicants") Boolean allowEmptyTieredReplicants + @JsonProperty("useDefaultTierForNull") @Nullable Boolean useDefaultTierForNull ) { - this.allowEmptyTieredReplicants = allowEmptyTieredReplicants != null && allowEmptyTieredReplicants; - - if (!this.allowEmptyTieredReplicants) { - this.tieredReplicants = tieredReplicants == null ? ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS) : tieredReplicants; - } else { - this.tieredReplicants = tieredReplicants == null ? ImmutableMap.of() : tieredReplicants; - } - validateTieredReplicants(this.tieredReplicants, this.allowEmptyTieredReplicants); + this.useDefaultTierForNull = Configs.valueOrDefault(useDefaultTierForNull, true); + this.tieredReplicants = createTieredReplicants(tieredReplicants, this.useDefaultTierForNull); + validateTieredReplicants(this.tieredReplicants, this.useDefaultTierForNull); this.interval = interval; } @@ -72,10 +67,10 @@ public Map getTieredReplicants() return tieredReplicants; } - @JsonProperty - public boolean isAllowEmptyTieredReplicants() + @JsonProperty("useDefaultTierForNull") + public boolean useDefaultTierForNull() { - return allowEmptyTieredReplicants; + return useDefaultTierForNull; } @Override diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java index f95ae572fd88..ca26c7382be4 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java @@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableMap; import org.apache.druid.client.DruidServer; +import org.apache.druid.common.config.Configs; import org.apache.druid.java.util.common.IAE; import org.apache.druid.timeline.DataSegment; @@ -42,10 +43,30 @@ public void run(DataSegment segment, SegmentActionHandler handler) } } - protected static void validateTieredReplicants(final Map tieredReplicants, boolean allowEmptyTieredReplicants) + /** + * Function to create a tiered replicants map choosing default values, in case the map is null. + * {@code useDefaultTierForNull} decides the default value. + *
+ * If the boolean is true, the default value is a singleton map with key {@link DruidServer#DEFAULT_NUM_REPLICANTS} + * and value @{@link DruidServer#DEFAULT_TIER}. + *
+ * If the boolean is false, the default value is an empty map. This will enable the new behaviour of not loading + * segments to a historical unless the tier and number of replicants are explicitly specified. + */ + protected static Map createTieredReplicants(final Map tieredReplicants, boolean useDefaultTierForNull) { - if (tieredReplicants.size() == 0 && !allowEmptyTieredReplicants) { - throw new IAE("A rule with empty tiered replicants is invalid unless \"allowEmptyTieredReplicants\" is set to true."); + if (useDefaultTierForNull) { + return Configs.valueOrDefault(tieredReplicants, ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS)); + } else { + return Configs.valueOrDefault(tieredReplicants, ImmutableMap.of()); + } + } + + protected static void validateTieredReplicants(final Map tieredReplicants, boolean useDefaultTierForNull) + { + if (tieredReplicants.size() == 0 && useDefaultTierForNull) { + // If useDefaultTierForNull is true, null is translated to a default tier, and an empty replicant tier map is not allowed. + throw new IAE("A rule with empty tiered replicants is invalid unless \"useDefaultTierForNull\" is set to false."); } for (Map.Entry entry : tieredReplicants.entrySet()) { if (entry.getValue() == null) { diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java index db16e85bbcbb..5ad73165c93b 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java @@ -21,14 +21,14 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; -import com.google.common.collect.ImmutableMap; -import org.apache.druid.client.DruidServer; +import org.apache.druid.common.config.Configs; import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.timeline.DataSegment; import org.joda.time.DateTime; import org.joda.time.Interval; import org.joda.time.Period; +import javax.annotation.Nullable; import java.util.Map; /** @@ -41,32 +41,19 @@ public class PeriodLoadRule extends LoadRule private final Period period; private final boolean includeFuture; private final Map tieredReplicants; - /** - * Compatibility flag for load rules. If the flag is false or not present, null or empty {@link #tieredReplicants} - * will be understood as the default load rule of {@link DruidServer#DEFAULT_NUM_REPLICANTS} replicants - * for @{@link DruidServer#DEFAULT_TIER}. - *
- * If the flag is true, it will be kept as an empty map. This will enable the new behaviour of not loading segments - * to a historical unless the tier and number of replicants are explicitly specified. - */ - private final boolean allowEmptyTieredReplicants; + private final boolean useDefaultTierForNull; @JsonCreator public PeriodLoadRule( @JsonProperty("period") Period period, @JsonProperty("includeFuture") Boolean includeFuture, @JsonProperty("tieredReplicants") Map tieredReplicants, - @JsonProperty("allowEmptyTieredReplicants") Boolean allowEmptyTieredReplicants + @JsonProperty("useDefaultTierForNull") @Nullable Boolean useDefaultTierForNull ) { - this.allowEmptyTieredReplicants = allowEmptyTieredReplicants != null && allowEmptyTieredReplicants; - - if (!this.allowEmptyTieredReplicants) { - this.tieredReplicants = tieredReplicants == null ? ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS) : tieredReplicants; - } else { - this.tieredReplicants = tieredReplicants == null ? ImmutableMap.of() : tieredReplicants; - } - validateTieredReplicants(this.tieredReplicants, this.allowEmptyTieredReplicants); + this.useDefaultTierForNull = Configs.valueOrDefault(useDefaultTierForNull, true); + this.tieredReplicants = createTieredReplicants(tieredReplicants, this.useDefaultTierForNull); + validateTieredReplicants(this.tieredReplicants, this.useDefaultTierForNull); this.period = period; this.includeFuture = includeFuture == null ? DEFAULT_INCLUDE_FUTURE : includeFuture; } @@ -104,10 +91,10 @@ public int getNumReplicants(String tier) return retVal == null ? 0 : retVal; } - @JsonProperty - public boolean isAllowEmptyTieredReplicants() + @JsonProperty("useDefaultTierForNull") + public boolean useDefaultTierForNull() { - return allowEmptyTieredReplicants; + return useDefaultTierForNull; } @Override diff --git a/server/src/test/java/org/apache/druid/server/coordinator/rules/ForeverLoadRuleTest.java b/server/src/test/java/org/apache/druid/server/coordinator/rules/ForeverLoadRuleTest.java index 19bdc63597dd..41e1bd9ee618 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/rules/ForeverLoadRuleTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/rules/ForeverLoadRuleTest.java @@ -54,7 +54,7 @@ public void testCreatingNegativeTieredReplicants() public void testEmptyTieredReplicants() throws Exception { ForeverLoadRule rule = new ForeverLoadRule(ImmutableMap.of(), - true + false ); ObjectMapper jsonMapper = new DefaultObjectMapper(); @@ -78,7 +78,7 @@ public void testEmptyReplicantValue() throws Exception } @Test - public void testWithoutAllowEmptyReplicantShouldCreateDefaultTier() throws Exception + public void testShouldCreateDefaultTier() throws Exception { String inputJson = " {\n" + " \"type\": \"loadForever\"\n" @@ -89,11 +89,23 @@ public void testWithoutAllowEmptyReplicantShouldCreateDefaultTier() throws Excep } @Test - public void testWithAllowEmptyReplicantShouldDefaultToEmpty() throws Exception + public void testUseDefaultTierAsTrueShouldCreateDefaultTier() throws Exception + { + String inputJson = " {\n" + + " \"type\": \"loadForever\"\n," + + " \"useDefaultTierForNull\": \"true\"\n" + + " }"; + ObjectMapper jsonMapper = new DefaultObjectMapper(); + ForeverLoadRule inputForeverLoadRule = jsonMapper.readValue(inputJson, ForeverLoadRule.class); + Assert.assertEquals(ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), inputForeverLoadRule.getTieredReplicants()); + } + + @Test + public void testUseDefaultTierAsFalseShouldCreateEmptyMap() throws Exception { String inputJson = " {\n" + " \"type\": \"loadForever\"\n," - + " \"allowEmptyTieredReplicants\": \"true\"\n" + + " \"useDefaultTierForNull\": \"false\"\n" + " }"; ObjectMapper jsonMapper = new DefaultObjectMapper(); ForeverLoadRule inputForeverLoadRule = jsonMapper.readValue(inputJson, ForeverLoadRule.class); diff --git a/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java b/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java index 4f305fe724d9..35e3475ee109 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java @@ -55,7 +55,7 @@ public void testSerde() throws Exception public void testSerdeNullTieredReplicants() throws Exception { IntervalLoadRule rule = new IntervalLoadRule( - Intervals.of("0/3000"), null, true + Intervals.of("0/3000"), null, false ); ObjectMapper jsonMapper = new DefaultObjectMapper(); @@ -88,27 +88,40 @@ public void testEmptyReplicantValue() throws Exception } @Test - public void testWithoutAllowEmptyReplicantShouldCreateDefaultTier() throws Exception + public void testShouldCreateDefaultTier() throws Exception { String inputJson = " {\n" + " \"interval\": \"0000-01-01T00:00:00.000-05:50:36/3000-01-01T00:00:00.000-06:00\",\n" + " \"type\": \"loadByInterval\"\n" + " }"; ObjectMapper jsonMapper = new DefaultObjectMapper(); - IntervalLoadRule inputForeverLoadRule = jsonMapper.readValue(inputJson, IntervalLoadRule.class); - Assert.assertEquals(ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), inputForeverLoadRule.getTieredReplicants()); + IntervalLoadRule inputIntervalLoadRule = jsonMapper.readValue(inputJson, IntervalLoadRule.class); + Assert.assertEquals(ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), inputIntervalLoadRule.getTieredReplicants()); } @Test - public void testWithAllowEmptyReplicantShouldDefaultToEmpty() throws Exception + public void testUseDefaultTierAsTrueShouldCreateDefaultTier() throws Exception { String inputJson = " {\n" + " \"interval\": \"0000-01-01T00:00:00.000-05:50:36/3000-01-01T00:00:00.000-06:00\",\n" + " \"type\": \"loadByInterval\",\n" - + " \"allowEmptyTieredReplicants\": \"true\"\n" + + " \"useDefaultTierForNull\": \"true\"\n" + " }"; ObjectMapper jsonMapper = new DefaultObjectMapper(); - IntervalLoadRule inputForeverLoadRule = jsonMapper.readValue(inputJson, IntervalLoadRule.class); - Assert.assertEquals(ImmutableMap.of(), inputForeverLoadRule.getTieredReplicants()); + IntervalLoadRule inputIntervalLoadRule = jsonMapper.readValue(inputJson, IntervalLoadRule.class); + Assert.assertEquals(ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), inputIntervalLoadRule.getTieredReplicants()); + } + + @Test + public void testUseDefaultTierAsFalseShouldCreateEmptyMap() throws Exception + { + String inputJson = " {\n" + + " \"interval\": \"0000-01-01T00:00:00.000-05:50:36/3000-01-01T00:00:00.000-06:00\",\n" + + " \"type\": \"loadByInterval\",\n" + + " \"useDefaultTierForNull\": \"false\"\n" + + " }"; + ObjectMapper jsonMapper = new DefaultObjectMapper(); + IntervalLoadRule inputIntervalLoadRule = jsonMapper.readValue(inputJson, IntervalLoadRule.class); + Assert.assertEquals(ImmutableMap.of(), inputIntervalLoadRule.getTieredReplicants()); } } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java b/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java index c9211a8f0504..ed5191c4ed1c 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java @@ -218,7 +218,7 @@ public void testEmptyReplicantValue() throws Exception @Test - public void testWithoutAllowEmptyReplicantShouldCreateDefaultTier() throws Exception + public void testShouldCreateDefaultTier() throws Exception { String inputJson = " {\n" + " \"period\": \"P1D\",\n" @@ -231,12 +231,26 @@ public void testWithoutAllowEmptyReplicantShouldCreateDefaultTier() throws Excep } @Test - public void testWithAllowEmptyReplicantShouldDefaultToEmpty() throws Exception + public void testUseDefaultTierAsTrueShouldCreateDefaultTier() throws Exception + { + String inputJson = " {\n" + + " \"period\": \"P1D\",\n" + + " \"includeFuture\": " + PeriodLoadRule.DEFAULT_INCLUDE_FUTURE + ",\n" + + " \"useDefaultTierForNull\": \"true\",\n" + + " \"type\": \"loadByPeriod\"\n" + + " }"; + ObjectMapper jsonMapper = new DefaultObjectMapper(); + PeriodLoadRule inputPeriodLoadRule = jsonMapper.readValue(inputJson, PeriodLoadRule.class); + Assert.assertEquals(ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), inputPeriodLoadRule.getTieredReplicants()); + } + + @Test + public void testUseDefaultTierAsFalseShouldCreateEmptyMap() throws Exception { String inputJson = " {\n" + " \"period\": \"P1D\",\n" + " \"includeFuture\": " + PeriodLoadRule.DEFAULT_INCLUDE_FUTURE + ",\n" - + " \"allowEmptyTieredReplicants\": \"true\",\n" + + " \"useDefaultTierForNull\": \"false\",\n" + " \"type\": \"loadByPeriod\"\n" + " }"; ObjectMapper jsonMapper = new DefaultObjectMapper(); From 035bd035fb74de1a1fdaabb827894c35536c6a8c Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Mon, 19 Jun 2023 22:45:26 +0530 Subject: [PATCH 09/18] Revert unneeded change --- .../apache/druid/server/coordinator/rules/LoadRule.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java index ca26c7382be4..df4258560ec1 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java @@ -35,12 +35,7 @@ public abstract class LoadRule implements Rule @Override public void run(DataSegment segment, SegmentActionHandler handler) { - Map tieredReplicants = getTieredReplicants(); - if (tieredReplicants.isEmpty()) { - handler.replicateSegment(segment, ImmutableMap.of(DruidServer.DEFAULT_TIER, 0)); - } else { - handler.replicateSegment(segment, tieredReplicants); - } + handler.replicateSegment(segment, getTieredReplicants()); } /** From 7ffd6f3fa1a5a2ef1a5777997810d3af4d66297e Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Tue, 20 Jun 2023 15:21:36 +0530 Subject: [PATCH 10/18] Refactor code, address review comments --- .../coordinator/rules/ForeverLoadRule.java | 21 +---- .../coordinator/rules/IntervalLoadRule.java | 20 +---- .../server/coordinator/rules/LoadRule.java | 53 ++++++++----- .../coordinator/rules/PeriodLoadRule.java | 20 +---- .../rules/ForeverLoadRuleTest.java | 70 +++++++++++------ .../rules/IntervalLoadRuleTest.java | 72 +++++++++++------ .../coordinator/rules/PeriodLoadRuleTest.java | 77 +++++++++++++------ 7 files changed, 188 insertions(+), 145 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/ForeverLoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/ForeverLoadRule.java index b9cf134b9e1d..585a6848997b 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/ForeverLoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/ForeverLoadRule.java @@ -21,7 +21,6 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; -import org.apache.druid.common.config.Configs; import org.apache.druid.timeline.DataSegment; import org.joda.time.DateTime; import org.joda.time.Interval; @@ -34,24 +33,13 @@ */ public class ForeverLoadRule extends LoadRule { - private final Map tieredReplicants; - private final boolean useDefaultTierForNull; - @JsonCreator public ForeverLoadRule( @JsonProperty("tieredReplicants") Map tieredReplicants, @JsonProperty("useDefaultTierForNull") @Nullable Boolean useDefaultTierForNull ) { - this.useDefaultTierForNull = Configs.valueOrDefault(useDefaultTierForNull, true); - this.tieredReplicants = createTieredReplicants(tieredReplicants, this.useDefaultTierForNull); - validateTieredReplicants(this.tieredReplicants, this.useDefaultTierForNull); - } - - @JsonProperty("useDefaultTierForNull") - public boolean useDefaultTierForNull() - { - return useDefaultTierForNull; + super(tieredReplicants, useDefaultTierForNull); } @Override @@ -61,13 +49,6 @@ public String getType() return "loadForever"; } - @Override - @JsonProperty - public Map getTieredReplicants() - { - return tieredReplicants; - } - @Override public int getNumReplicants(String tier) { diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/IntervalLoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/IntervalLoadRule.java index f0b0884d9a66..765c8f898d46 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/IntervalLoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/IntervalLoadRule.java @@ -21,7 +21,6 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; -import org.apache.druid.common.config.Configs; import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.timeline.DataSegment; import org.joda.time.DateTime; @@ -37,8 +36,6 @@ public class IntervalLoadRule extends LoadRule private static final Logger log = new Logger(IntervalLoadRule.class); private final Interval interval; - private final Map tieredReplicants; - private final boolean useDefaultTierForNull; @JsonCreator public IntervalLoadRule( @@ -47,9 +44,7 @@ public IntervalLoadRule( @JsonProperty("useDefaultTierForNull") @Nullable Boolean useDefaultTierForNull ) { - this.useDefaultTierForNull = Configs.valueOrDefault(useDefaultTierForNull, true); - this.tieredReplicants = createTieredReplicants(tieredReplicants, this.useDefaultTierForNull); - validateTieredReplicants(this.tieredReplicants, this.useDefaultTierForNull); + super(tieredReplicants, useDefaultTierForNull); this.interval = interval; } @@ -60,19 +55,6 @@ public String getType() return "loadByInterval"; } - @Override - @JsonProperty - public Map getTieredReplicants() - { - return tieredReplicants; - } - - @JsonProperty("useDefaultTierForNull") - public boolean useDefaultTierForNull() - { - return useDefaultTierForNull; - } - @Override public int getNumReplicants(String tier) { diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java index df4258560ec1..220438827371 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java @@ -19,10 +19,11 @@ package org.apache.druid.server.coordinator.rules; +import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.collect.ImmutableMap; import org.apache.druid.client.DruidServer; import org.apache.druid.common.config.Configs; -import org.apache.druid.java.util.common.IAE; +import org.apache.druid.error.InvalidInput; import org.apache.druid.timeline.DataSegment; import java.util.Map; @@ -32,6 +33,28 @@ */ public abstract class LoadRule implements Rule { + protected final Map tieredReplicants; + protected final boolean useDefaultTierForNull; + + protected LoadRule(Map tieredReplicants, Boolean useDefaultTierForNull) + { + this.useDefaultTierForNull = Configs.valueOrDefault(useDefaultTierForNull, true); + this.tieredReplicants = handleNullTieredReplicants(tieredReplicants, this.useDefaultTierForNull); + validateTieredReplicants(this.tieredReplicants); + } + + @JsonProperty + public Map getTieredReplicants() + { + return tieredReplicants; + } + + @JsonProperty + public boolean useDefaultTierForNull() + { + return useDefaultTierForNull; + } + @Override public void run(DataSegment segment, SegmentActionHandler handler) { @@ -39,16 +62,14 @@ public void run(DataSegment segment, SegmentActionHandler handler) } /** - * Function to create a tiered replicants map choosing default values, in case the map is null. - * {@code useDefaultTierForNull} decides the default value. - *
- * If the boolean is true, the default value is a singleton map with key {@link DruidServer#DEFAULT_NUM_REPLICANTS} - * and value @{@link DruidServer#DEFAULT_TIER}. - *
- * If the boolean is false, the default value is an empty map. This will enable the new behaviour of not loading - * segments to a historical unless the tier and number of replicants are explicitly specified. + * Returns the given {@code tieredReplicants} map unchanged if it is non-null (including empty). + * Returns the following default values if the given map is null. + *
    + *
  • If {@code useDefaultTierForNull} is true, returns a singleton map from {@link DruidServer#DEFAULT_TIER} to {@link DruidServer#DEFAULT_NUM_REPLICANTS}.
    • + *
  • If {@code useDefaultTierForNull} is false, returns an empty map. This causes segments to have a replication factor of 0 and not get assigned to any historical.
    • + *
*/ - protected static Map createTieredReplicants(final Map tieredReplicants, boolean useDefaultTierForNull) + protected static Map handleNullTieredReplicants(final Map tieredReplicants, boolean useDefaultTierForNull) { if (useDefaultTierForNull) { return Configs.valueOrDefault(tieredReplicants, ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS)); @@ -57,24 +78,18 @@ protected static Map createTieredReplicants(final Map tieredReplicants, boolean useDefaultTierForNull) + protected static void validateTieredReplicants(final Map tieredReplicants) { - if (tieredReplicants.size() == 0 && useDefaultTierForNull) { - // If useDefaultTierForNull is true, null is translated to a default tier, and an empty replicant tier map is not allowed. - throw new IAE("A rule with empty tiered replicants is invalid unless \"useDefaultTierForNull\" is set to false."); - } for (Map.Entry entry : tieredReplicants.entrySet()) { if (entry.getValue() == null) { - throw new IAE("Replicant value cannot be empty"); + throw InvalidInput.exception("Invalid number of replicas for tier [%s]. Value must not be null.", entry.getKey()); } if (entry.getValue() < 0) { - throw new IAE("Replicant value [%d] is less than 0, which is not allowed", entry.getValue()); + throw InvalidInput.exception("Invalid number of replicas for tier [%s]. Value [%d] must be positive.", entry.getKey(), entry.getValue()); } } } - public abstract Map getTieredReplicants(); - public abstract int getNumReplicants(String tier); } diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java index 5ad73165c93b..63cb9d1e1740 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java @@ -21,7 +21,6 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; -import org.apache.druid.common.config.Configs; import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.timeline.DataSegment; import org.joda.time.DateTime; @@ -40,8 +39,6 @@ public class PeriodLoadRule extends LoadRule private final Period period; private final boolean includeFuture; - private final Map tieredReplicants; - private final boolean useDefaultTierForNull; @JsonCreator public PeriodLoadRule( @@ -51,9 +48,7 @@ public PeriodLoadRule( @JsonProperty("useDefaultTierForNull") @Nullable Boolean useDefaultTierForNull ) { - this.useDefaultTierForNull = Configs.valueOrDefault(useDefaultTierForNull, true); - this.tieredReplicants = createTieredReplicants(tieredReplicants, this.useDefaultTierForNull); - validateTieredReplicants(this.tieredReplicants, this.useDefaultTierForNull); + super(tieredReplicants, useDefaultTierForNull); this.period = period; this.includeFuture = includeFuture == null ? DEFAULT_INCLUDE_FUTURE : includeFuture; } @@ -77,13 +72,6 @@ public boolean isIncludeFuture() return includeFuture; } - @Override - @JsonProperty - public Map getTieredReplicants() - { - return tieredReplicants; - } - @Override public int getNumReplicants(String tier) { @@ -91,12 +79,6 @@ public int getNumReplicants(String tier) return retVal == null ? 0 : retVal; } - @JsonProperty("useDefaultTierForNull") - public boolean useDefaultTierForNull() - { - return useDefaultTierForNull; - } - @Override public boolean appliesTo(DataSegment segment, DateTime referenceTimestamp) { diff --git a/server/src/test/java/org/apache/druid/server/coordinator/rules/ForeverLoadRuleTest.java b/server/src/test/java/org/apache/druid/server/coordinator/rules/ForeverLoadRuleTest.java index 41e1bd9ee618..cdd5809c17f1 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/rules/ForeverLoadRuleTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/rules/ForeverLoadRuleTest.java @@ -22,8 +22,12 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableMap; import org.apache.druid.client.DruidServer; +import org.apache.druid.error.DruidException; +import org.apache.druid.error.DruidExceptionMatcher; import org.apache.druid.jackson.DefaultObjectMapper; -import org.apache.druid.java.util.common.IAE; +import org.apache.druid.java.util.common.StringUtils; +import org.hamcrest.MatcherAssert; +import org.hamcrest.Matchers; import org.junit.Assert; import org.junit.Test; @@ -32,6 +36,8 @@ public class ForeverLoadRuleTest { + private static final ObjectMapper OBJECT_MAPPER = new DefaultObjectMapper(); + @Test public void testSerde() throws Exception { @@ -44,37 +50,60 @@ public void testSerde() throws Exception Assert.assertEquals(ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), rule.getTieredReplicants()); } - @Test(expected = IAE.class) + @Test public void testCreatingNegativeTieredReplicants() { - new ForeverLoadRule(ImmutableMap.of(DruidServer.DEFAULT_TIER, -1), null); + MatcherAssert.assertThat( + Assert.assertThrows(DruidException.class, () -> + new ForeverLoadRule( + ImmutableMap.of(DruidServer.DEFAULT_TIER, -1), + null + ) + ), + DruidExceptionMatcher.invalidInput().expectMessage( + Matchers.containsString( + StringUtils.format( + "Invalid number of replicas for tier [%s]. Value [%d] must be positive.", + DruidServer.DEFAULT_TIER, + -1 + ) + ) + ) + ); } @Test public void testEmptyTieredReplicants() throws Exception { - ForeverLoadRule rule = new ForeverLoadRule(ImmutableMap.of(), - false - ); + ForeverLoadRule rule = new ForeverLoadRule(ImmutableMap.of(), false); - ObjectMapper jsonMapper = new DefaultObjectMapper(); - LoadRule reread = (LoadRule) jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); + LoadRule reread = (LoadRule) OBJECT_MAPPER.readValue(OBJECT_MAPPER.writeValueAsString(rule), Rule.class); Assert.assertEquals(ImmutableMap.of(), reread.getTieredReplicants()); } - @Test(expected = IAE.class) - public void testEmptyReplicantValue() throws Exception + @Test + public void testNullReplicantValue() { // Immutable map does not allow null values Map tieredReplicants = new HashMap<>(); tieredReplicants.put("tier", null); - ForeverLoadRule rule = new ForeverLoadRule( - tieredReplicants, - true - ); - ObjectMapper jsonMapper = new DefaultObjectMapper(); - jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); + MatcherAssert.assertThat( + Assert.assertThrows(DruidException.class, () -> + new ForeverLoadRule( + tieredReplicants, + true + ) + ), + DruidExceptionMatcher.invalidInput().expectMessage( + Matchers.containsString( + StringUtils.format( + "Invalid number of replicas for tier [%s]. Value must not be null.", + "tier" + ) + ) + ) + ); } @Test @@ -83,8 +112,7 @@ public void testShouldCreateDefaultTier() throws Exception String inputJson = " {\n" + " \"type\": \"loadForever\"\n" + " }"; - ObjectMapper jsonMapper = new DefaultObjectMapper(); - ForeverLoadRule inputForeverLoadRule = jsonMapper.readValue(inputJson, ForeverLoadRule.class); + ForeverLoadRule inputForeverLoadRule = OBJECT_MAPPER.readValue(inputJson, ForeverLoadRule.class); Assert.assertEquals(ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), inputForeverLoadRule.getTieredReplicants()); } @@ -95,8 +123,7 @@ public void testUseDefaultTierAsTrueShouldCreateDefaultTier() throws Exception + " \"type\": \"loadForever\"\n," + " \"useDefaultTierForNull\": \"true\"\n" + " }"; - ObjectMapper jsonMapper = new DefaultObjectMapper(); - ForeverLoadRule inputForeverLoadRule = jsonMapper.readValue(inputJson, ForeverLoadRule.class); + ForeverLoadRule inputForeverLoadRule = OBJECT_MAPPER.readValue(inputJson, ForeverLoadRule.class); Assert.assertEquals(ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), inputForeverLoadRule.getTieredReplicants()); } @@ -107,8 +134,7 @@ public void testUseDefaultTierAsFalseShouldCreateEmptyMap() throws Exception + " \"type\": \"loadForever\"\n," + " \"useDefaultTierForNull\": \"false\"\n" + " }"; - ObjectMapper jsonMapper = new DefaultObjectMapper(); - ForeverLoadRule inputForeverLoadRule = jsonMapper.readValue(inputJson, ForeverLoadRule.class); + ForeverLoadRule inputForeverLoadRule = OBJECT_MAPPER.readValue(inputJson, ForeverLoadRule.class); Assert.assertEquals(ImmutableMap.of(), inputForeverLoadRule.getTieredReplicants()); } } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java b/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java index 35e3475ee109..dc809dffd963 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java @@ -22,9 +22,13 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableMap; import org.apache.druid.client.DruidServer; +import org.apache.druid.error.DruidException; +import org.apache.druid.error.DruidExceptionMatcher; import org.apache.druid.jackson.DefaultObjectMapper; -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.hamcrest.MatcherAssert; +import org.hamcrest.Matchers; import org.junit.Assert; import org.junit.Test; @@ -36,6 +40,8 @@ */ public class IntervalLoadRuleTest { + private static final ObjectMapper OBJECT_MAPPER = new DefaultObjectMapper(); + @Test public void testSerde() throws Exception { @@ -45,8 +51,7 @@ public void testSerde() throws Exception null ); - ObjectMapper jsonMapper = new DefaultObjectMapper(); - Rule reread = jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); + Rule reread = OBJECT_MAPPER.readValue(OBJECT_MAPPER.writeValueAsString(rule), Rule.class); Assert.assertEquals(rule, reread); } @@ -58,33 +63,59 @@ public void testSerdeNullTieredReplicants() throws Exception Intervals.of("0/3000"), null, false ); - ObjectMapper jsonMapper = new DefaultObjectMapper(); - Rule reread = jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); + Rule reread = OBJECT_MAPPER.readValue(OBJECT_MAPPER.writeValueAsString(rule), Rule.class); Assert.assertEquals(rule, reread); Assert.assertEquals(ImmutableMap.of(), rule.getTieredReplicants()); } - @Test(expected = IAE.class) + @Test public void testCreatingNegativeTieredReplicants() { - new IntervalLoadRule(Intervals.of("0/3000"), ImmutableMap.of(DruidServer.DEFAULT_TIER, -1), null); + MatcherAssert.assertThat( + Assert.assertThrows(DruidException.class, () -> + new IntervalLoadRule( + Intervals.of("0/3000"), + ImmutableMap.of(DruidServer.DEFAULT_TIER, -1), + null + ) + ), + DruidExceptionMatcher.invalidInput().expectMessage( + Matchers.containsString( + StringUtils.format( + "Invalid number of replicas for tier [%s]. Value [%d] must be positive.", + DruidServer.DEFAULT_TIER, + -1 + ) + ) + ) + ); } - @Test(expected = IAE.class) - public void testEmptyReplicantValue() throws Exception + @Test + public void testNullReplicantValue() { // Immutable map does not allow null values Map tieredReplicants = new HashMap<>(); tieredReplicants.put("tier", null); - IntervalLoadRule rule = new IntervalLoadRule( - Intervals.of("0/3000"), - tieredReplicants, - null - ); - ObjectMapper jsonMapper = new DefaultObjectMapper(); - jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); + MatcherAssert.assertThat( + Assert.assertThrows(DruidException.class, () -> + new IntervalLoadRule( + Intervals.of("0/3000"), + tieredReplicants, + null + ) + ), + DruidExceptionMatcher.invalidInput().expectMessage( + Matchers.containsString( + StringUtils.format( + "Invalid number of replicas for tier [%s]. Value must not be null.", + "tier" + ) + ) + ) + ); } @Test @@ -94,8 +125,7 @@ public void testShouldCreateDefaultTier() throws Exception + " \"interval\": \"0000-01-01T00:00:00.000-05:50:36/3000-01-01T00:00:00.000-06:00\",\n" + " \"type\": \"loadByInterval\"\n" + " }"; - ObjectMapper jsonMapper = new DefaultObjectMapper(); - IntervalLoadRule inputIntervalLoadRule = jsonMapper.readValue(inputJson, IntervalLoadRule.class); + IntervalLoadRule inputIntervalLoadRule = OBJECT_MAPPER.readValue(inputJson, IntervalLoadRule.class); Assert.assertEquals(ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), inputIntervalLoadRule.getTieredReplicants()); } @@ -107,8 +137,7 @@ public void testUseDefaultTierAsTrueShouldCreateDefaultTier() throws Exception + " \"type\": \"loadByInterval\",\n" + " \"useDefaultTierForNull\": \"true\"\n" + " }"; - ObjectMapper jsonMapper = new DefaultObjectMapper(); - IntervalLoadRule inputIntervalLoadRule = jsonMapper.readValue(inputJson, IntervalLoadRule.class); + IntervalLoadRule inputIntervalLoadRule = OBJECT_MAPPER.readValue(inputJson, IntervalLoadRule.class); Assert.assertEquals(ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), inputIntervalLoadRule.getTieredReplicants()); } @@ -120,8 +149,7 @@ public void testUseDefaultTierAsFalseShouldCreateEmptyMap() throws Exception + " \"type\": \"loadByInterval\",\n" + " \"useDefaultTierForNull\": \"false\"\n" + " }"; - ObjectMapper jsonMapper = new DefaultObjectMapper(); - IntervalLoadRule inputIntervalLoadRule = jsonMapper.readValue(inputJson, IntervalLoadRule.class); + IntervalLoadRule inputIntervalLoadRule = OBJECT_MAPPER.readValue(inputJson, IntervalLoadRule.class); Assert.assertEquals(ImmutableMap.of(), inputIntervalLoadRule.getTieredReplicants()); } } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java b/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java index ed5191c4ed1c..57733a24652e 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java @@ -22,12 +22,16 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableMap; import org.apache.druid.client.DruidServer; +import org.apache.druid.error.DruidException; +import org.apache.druid.error.DruidExceptionMatcher; import org.apache.druid.jackson.DefaultObjectMapper; import org.apache.druid.java.util.common.DateTimes; -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.timeline.DataSegment; import org.apache.druid.timeline.partition.NoneShardSpec; +import org.hamcrest.MatcherAssert; +import org.hamcrest.Matchers; import org.joda.time.DateTime; import org.joda.time.Interval; import org.joda.time.Period; @@ -41,6 +45,8 @@ */ public class PeriodLoadRuleTest { + private static final ObjectMapper OBJECT_MAPPER = new DefaultObjectMapper(); + private static final DataSegment.Builder BUILDER = DataSegment .builder() .dataSource("test") @@ -160,8 +166,7 @@ public void testSerdeNull() throws Exception new Period("P1D"), null, null, null ); - ObjectMapper jsonMapper = new DefaultObjectMapper(); - Rule reread = jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); + Rule reread = OBJECT_MAPPER.readValue(OBJECT_MAPPER.writeValueAsString(rule), Rule.class); Assert.assertEquals(rule.getPeriod(), ((PeriodLoadRule) reread).getPeriod()); Assert.assertEquals(rule.isIncludeFuture(), ((PeriodLoadRule) reread).isIncludeFuture()); @@ -185,35 +190,62 @@ public void testMappingNull() throws Exception + " \"includeFuture\": " + PeriodLoadRule.DEFAULT_INCLUDE_FUTURE + ",\n" + " \"type\": \"loadByPeriod\"\n" + " }"; - ObjectMapper jsonMapper = new DefaultObjectMapper(); - PeriodLoadRule inputPeriodLoadRule = jsonMapper.readValue(inputJson, PeriodLoadRule.class); - PeriodLoadRule expectedPeriodLoadRule = jsonMapper.readValue(expectedJson, PeriodLoadRule.class); + PeriodLoadRule inputPeriodLoadRule = OBJECT_MAPPER.readValue(inputJson, PeriodLoadRule.class); + PeriodLoadRule expectedPeriodLoadRule = OBJECT_MAPPER.readValue(expectedJson, PeriodLoadRule.class); Assert.assertEquals(expectedPeriodLoadRule.getTieredReplicants(), inputPeriodLoadRule.getTieredReplicants()); Assert.assertEquals(expectedPeriodLoadRule.getPeriod(), inputPeriodLoadRule.getPeriod()); Assert.assertEquals(expectedPeriodLoadRule.isIncludeFuture(), inputPeriodLoadRule.isIncludeFuture()); } - @Test(expected = IAE.class) + @Test public void testCreatingNegativeTieredReplicants() { - new PeriodLoadRule(Period.days(1), true, ImmutableMap.of(DruidServer.DEFAULT_TIER, -1), true); + MatcherAssert.assertThat( + Assert.assertThrows(DruidException.class, () -> + new PeriodLoadRule( + Period.days(1), + true, + ImmutableMap.of(DruidServer.DEFAULT_TIER, -1), + true + ) + ), + DruidExceptionMatcher.invalidInput().expectMessage( + Matchers.containsString( + StringUtils.format( + "Invalid number of replicas for tier [%s]. Value [%d] must be positive.", + DruidServer.DEFAULT_TIER, + -1 + ) + ) + ) + ); } - @Test(expected = IAE.class) - public void testEmptyReplicantValue() throws Exception + @Test + public void testNullReplicantValue() { // Immutable map does not allow null values Map tieredReplicants = new HashMap<>(); tieredReplicants.put("tier", null); - PeriodLoadRule rule = new PeriodLoadRule( - Period.days(1), - true, - tieredReplicants, - true - ); - ObjectMapper jsonMapper = new DefaultObjectMapper(); - jsonMapper.readValue(jsonMapper.writeValueAsString(rule), Rule.class); + MatcherAssert.assertThat( + Assert.assertThrows(DruidException.class, () -> + new PeriodLoadRule( + Period.days(1), + true, + tieredReplicants, + true + ) + ), + DruidExceptionMatcher.invalidInput().expectMessage( + Matchers.containsString( + StringUtils.format( + "Invalid number of replicas for tier [%s]. Value must not be null.", + "tier" + ) + ) + ) + ); } @@ -225,8 +257,7 @@ public void testShouldCreateDefaultTier() throws Exception + " \"includeFuture\": " + PeriodLoadRule.DEFAULT_INCLUDE_FUTURE + ",\n" + " \"type\": \"loadByPeriod\"\n" + " }"; - ObjectMapper jsonMapper = new DefaultObjectMapper(); - PeriodLoadRule inputPeriodLoadRule = jsonMapper.readValue(inputJson, PeriodLoadRule.class); + PeriodLoadRule inputPeriodLoadRule = OBJECT_MAPPER.readValue(inputJson, PeriodLoadRule.class); Assert.assertEquals(ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), inputPeriodLoadRule.getTieredReplicants()); } @@ -239,8 +270,7 @@ public void testUseDefaultTierAsTrueShouldCreateDefaultTier() throws Exception + " \"useDefaultTierForNull\": \"true\",\n" + " \"type\": \"loadByPeriod\"\n" + " }"; - ObjectMapper jsonMapper = new DefaultObjectMapper(); - PeriodLoadRule inputPeriodLoadRule = jsonMapper.readValue(inputJson, PeriodLoadRule.class); + PeriodLoadRule inputPeriodLoadRule = OBJECT_MAPPER.readValue(inputJson, PeriodLoadRule.class); Assert.assertEquals(ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS), inputPeriodLoadRule.getTieredReplicants()); } @@ -253,8 +283,7 @@ public void testUseDefaultTierAsFalseShouldCreateEmptyMap() throws Exception + " \"useDefaultTierForNull\": \"false\",\n" + " \"type\": \"loadByPeriod\"\n" + " }"; - ObjectMapper jsonMapper = new DefaultObjectMapper(); - PeriodLoadRule inputPeriodLoadRule = jsonMapper.readValue(inputJson, PeriodLoadRule.class); + PeriodLoadRule inputPeriodLoadRule = OBJECT_MAPPER.readValue(inputJson, PeriodLoadRule.class); Assert.assertEquals(ImmutableMap.of(), inputPeriodLoadRule.getTieredReplicants()); } } From e3d97b5c44b05397eb07b5f60c120dac27c4f049 Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Tue, 20 Jun 2023 17:12:04 +0530 Subject: [PATCH 11/18] Remove unneeded test --- .../druid/server/coordinator/rules/LoadRuleTest.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/server/src/test/java/org/apache/druid/server/coordinator/rules/LoadRuleTest.java b/server/src/test/java/org/apache/druid/server/coordinator/rules/LoadRuleTest.java index c7190c1514a1..1deca43b2a42 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/rules/LoadRuleTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/rules/LoadRuleTest.java @@ -26,7 +26,6 @@ import org.apache.druid.client.DruidServer; import org.apache.druid.client.ImmutableDruidServer; import org.apache.druid.java.util.common.DateTimes; -import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.Intervals; import org.apache.druid.java.util.common.concurrent.Execs; import org.apache.druid.java.util.common.granularity.Granularities; @@ -689,12 +688,6 @@ public void testRedundantReplicaDropDuringDecommissioning() Assert.assertEquals(0, mockPeon3.getSegmentsToDrop().size()); } - @Test(expected = IAE.class) - public void testValidateTieredReplicantsEmptyTierReplicantsWithoutAllowEmptyFlag() - { - new ForeverLoadRule(ImmutableMap.of(), null); - } - private DataSegment createDataSegment(String dataSource) { return new DataSegment( From 114de1dba0f394b66f0749b76ea14b24f58d9869 Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Tue, 20 Jun 2023 18:55:36 +0530 Subject: [PATCH 12/18] Refactor code --- .../coordinator/rules/ForeverLoadRule.java | 26 -------------- .../coordinator/rules/IntervalLoadRule.java | 24 +++---------- .../server/coordinator/rules/LoadRule.java | 36 ++++++++++++++++--- .../coordinator/rules/PeriodLoadRule.java | 30 ++++++++++++---- 4 files changed, 59 insertions(+), 57 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/ForeverLoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/ForeverLoadRule.java index 585a6848997b..35f22fa555f8 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/ForeverLoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/ForeverLoadRule.java @@ -27,7 +27,6 @@ import javax.annotation.Nullable; import java.util.Map; -import java.util.Objects; /** */ @@ -49,13 +48,6 @@ public String getType() return "loadForever"; } - @Override - public int getNumReplicants(String tier) - { - Integer retVal = tieredReplicants.get(tier); - return (retVal == null) ? 0 : retVal; - } - @Override public boolean appliesTo(DataSegment segment, DateTime referenceTimestamp) { @@ -68,22 +60,4 @@ public boolean appliesTo(Interval interval, DateTime referenceTimestamp) return true; } - @Override - public boolean equals(Object o) - { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - ForeverLoadRule that = (ForeverLoadRule) o; - return Objects.equals(tieredReplicants, that.tieredReplicants); - } - - @Override - public int hashCode() - { - return Objects.hash(tieredReplicants); - } } diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/IntervalLoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/IntervalLoadRule.java index 765c8f898d46..209f5c24d1e4 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/IntervalLoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/IntervalLoadRule.java @@ -28,6 +28,7 @@ import javax.annotation.Nullable; import java.util.Map; +import java.util.Objects; /** */ @@ -55,13 +56,6 @@ public String getType() return "loadByInterval"; } - @Override - public int getNumReplicants(String tier) - { - final Integer retVal = tieredReplicants.get(tier); - return retVal == null ? 0 : retVal; - } - @JsonProperty public Interval getInterval() { @@ -89,24 +83,16 @@ public boolean equals(Object o) if (o == null || getClass() != o.getClass()) { return false; } - - IntervalLoadRule that = (IntervalLoadRule) o; - - if (interval != null ? !interval.equals(that.interval) : that.interval != null) { - return false; - } - if (tieredReplicants != null ? !tieredReplicants.equals(that.tieredReplicants) : that.tieredReplicants != null) { + if (!super.equals(o)) { return false; } - - return true; + IntervalLoadRule that = (IntervalLoadRule) o; + return Objects.equals(interval, that.interval); } @Override public int hashCode() { - int result = interval != null ? interval.hashCode() : 0; - result = 31 * result + (tieredReplicants != null ? tieredReplicants.hashCode() : 0); - return result; + return Objects.hash(super.hashCode(), interval); } } diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java index 220438827371..4344e191b81f 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java @@ -27,14 +27,15 @@ import org.apache.druid.timeline.DataSegment; import java.util.Map; +import java.util.Objects; /** * LoadRules indicate the number of replicants a segment should have in a given tier. */ public abstract class LoadRule implements Rule { - protected final Map tieredReplicants; - protected final boolean useDefaultTierForNull; + private final Map tieredReplicants; + private final boolean useDefaultTierForNull; protected LoadRule(Map tieredReplicants, Boolean useDefaultTierForNull) { @@ -69,7 +70,7 @@ public void run(DataSegment segment, SegmentActionHandler handler) *
  • If {@code useDefaultTierForNull} is false, returns an empty map. This causes segments to have a replication factor of 0 and not get assigned to any historical.
    • * */ - protected static Map handleNullTieredReplicants(final Map tieredReplicants, boolean useDefaultTierForNull) + private static Map handleNullTieredReplicants(final Map tieredReplicants, boolean useDefaultTierForNull) { if (useDefaultTierForNull) { return Configs.valueOrDefault(tieredReplicants, ImmutableMap.of(DruidServer.DEFAULT_TIER, DruidServer.DEFAULT_NUM_REPLICANTS)); @@ -78,7 +79,7 @@ protected static Map handleNullTieredReplicants(final Map tieredReplicants) + private static void validateTieredReplicants(final Map tieredReplicants) { for (Map.Entry entry : tieredReplicants.entrySet()) { if (entry.getValue() == null) { @@ -90,6 +91,31 @@ protected static void validateTieredReplicants(final Map tiered } } - public abstract int getNumReplicants(String tier); + public int getNumReplicants(String tier) + { + Integer retVal = getTieredReplicants().get(tier); + return (retVal == null) ? 0 : retVal; + } + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + LoadRule loadRule = (LoadRule) o; + return useDefaultTierForNull == loadRule.useDefaultTierForNull && Objects.equals( + tieredReplicants, + loadRule.tieredReplicants + ); + } + + @Override + public int hashCode() + { + return Objects.hash(tieredReplicants, useDefaultTierForNull); + } } diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java index 63cb9d1e1740..1d2b4e187716 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/PeriodLoadRule.java @@ -29,6 +29,7 @@ import javax.annotation.Nullable; import java.util.Map; +import java.util.Objects; /** */ @@ -72,13 +73,6 @@ public boolean isIncludeFuture() return includeFuture; } - @Override - public int getNumReplicants(String tier) - { - final Integer retVal = tieredReplicants.get(tier); - return retVal == null ? 0 : retVal; - } - @Override public boolean appliesTo(DataSegment segment, DateTime referenceTimestamp) { @@ -90,4 +84,26 @@ public boolean appliesTo(Interval interval, DateTime referenceTimestamp) { return Rules.eligibleForLoad(period, interval, referenceTimestamp, includeFuture); } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + if (!super.equals(o)) { + return false; + } + PeriodLoadRule that = (PeriodLoadRule) o; + return includeFuture == that.includeFuture && Objects.equals(period, that.period); + } + + @Override + public int hashCode() + { + return Objects.hash(super.hashCode(), period, includeFuture); + } } From f63569db9619cef805719f35d61a1397a3633d65 Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Tue, 20 Jun 2023 21:37:18 +0530 Subject: [PATCH 13/18] Address review comments --- .../rules/ForeverLoadRuleTest.java | 21 ++++--------------- .../rules/IntervalLoadRuleTest.java | 21 ++++--------------- .../coordinator/rules/PeriodLoadRuleTest.java | 21 ++++--------------- 3 files changed, 12 insertions(+), 51 deletions(-) diff --git a/server/src/test/java/org/apache/druid/server/coordinator/rules/ForeverLoadRuleTest.java b/server/src/test/java/org/apache/druid/server/coordinator/rules/ForeverLoadRuleTest.java index cdd5809c17f1..c1497afe899b 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/rules/ForeverLoadRuleTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/rules/ForeverLoadRuleTest.java @@ -25,9 +25,7 @@ import org.apache.druid.error.DruidException; import org.apache.druid.error.DruidExceptionMatcher; import org.apache.druid.jackson.DefaultObjectMapper; -import org.apache.druid.java.util.common.StringUtils; import org.hamcrest.MatcherAssert; -import org.hamcrest.Matchers; import org.junit.Assert; import org.junit.Test; @@ -60,14 +58,8 @@ public void testCreatingNegativeTieredReplicants() null ) ), - DruidExceptionMatcher.invalidInput().expectMessage( - Matchers.containsString( - StringUtils.format( - "Invalid number of replicas for tier [%s]. Value [%d] must be positive.", - DruidServer.DEFAULT_TIER, - -1 - ) - ) + DruidExceptionMatcher.invalidInput().expectMessageContains( + "Invalid number of replicas for tier [_default_tier]. Value [-1] must be positive." ) ); } @@ -95,13 +87,8 @@ public void testNullReplicantValue() true ) ), - DruidExceptionMatcher.invalidInput().expectMessage( - Matchers.containsString( - StringUtils.format( - "Invalid number of replicas for tier [%s]. Value must not be null.", - "tier" - ) - ) + DruidExceptionMatcher.invalidInput().expectMessageContains( + "Invalid number of replicas for tier [tier]. Value must not be null." ) ); } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java b/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java index dc809dffd963..5eae342d2eda 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/rules/IntervalLoadRuleTest.java @@ -26,9 +26,7 @@ import org.apache.druid.error.DruidExceptionMatcher; import org.apache.druid.jackson.DefaultObjectMapper; import org.apache.druid.java.util.common.Intervals; -import org.apache.druid.java.util.common.StringUtils; import org.hamcrest.MatcherAssert; -import org.hamcrest.Matchers; import org.junit.Assert; import org.junit.Test; @@ -80,14 +78,8 @@ public void testCreatingNegativeTieredReplicants() null ) ), - DruidExceptionMatcher.invalidInput().expectMessage( - Matchers.containsString( - StringUtils.format( - "Invalid number of replicas for tier [%s]. Value [%d] must be positive.", - DruidServer.DEFAULT_TIER, - -1 - ) - ) + DruidExceptionMatcher.invalidInput().expectMessageContains( + "Invalid number of replicas for tier [_default_tier]. Value [-1] must be positive." ) ); } @@ -107,13 +99,8 @@ public void testNullReplicantValue() null ) ), - DruidExceptionMatcher.invalidInput().expectMessage( - Matchers.containsString( - StringUtils.format( - "Invalid number of replicas for tier [%s]. Value must not be null.", - "tier" - ) - ) + DruidExceptionMatcher.invalidInput().expectMessageContains( + "Invalid number of replicas for tier [tier]. Value must not be null." ) ); } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java b/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java index 57733a24652e..ebb8dee76481 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java @@ -27,11 +27,9 @@ 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.StringUtils; import org.apache.druid.timeline.DataSegment; import org.apache.druid.timeline.partition.NoneShardSpec; import org.hamcrest.MatcherAssert; -import org.hamcrest.Matchers; import org.joda.time.DateTime; import org.joda.time.Interval; import org.joda.time.Period; @@ -209,14 +207,8 @@ public void testCreatingNegativeTieredReplicants() true ) ), - DruidExceptionMatcher.invalidInput().expectMessage( - Matchers.containsString( - StringUtils.format( - "Invalid number of replicas for tier [%s]. Value [%d] must be positive.", - DruidServer.DEFAULT_TIER, - -1 - ) - ) + DruidExceptionMatcher.invalidInput().expectMessageContains( + "Invalid number of replicas for tier [_default_tier]. Value [-1] must be positive." ) ); } @@ -237,13 +229,8 @@ public void testNullReplicantValue() true ) ), - DruidExceptionMatcher.invalidInput().expectMessage( - Matchers.containsString( - StringUtils.format( - "Invalid number of replicas for tier [%s]. Value must not be null.", - "tier" - ) - ) + DruidExceptionMatcher.invalidInput().expectMessageContains( + "Invalid number of replicas for tier [tier]. Value must not be null." ) ); } From e83bcc71b912e959da201756403e782fb69406bb Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Wed, 21 Jun 2023 14:50:04 +0530 Subject: [PATCH 14/18] Add documentation --- docs/operations/rule-configuration.md | 52 +++++++++++++++++++ .../server/coordinator/rules/LoadRule.java | 3 ++ .../coordinator/rules/LoadRuleTest.java | 10 ++++ .../coordinator/rules/PeriodLoadRuleTest.java | 10 ++++ 4 files changed, 75 insertions(+) diff --git a/docs/operations/rule-configuration.md b/docs/operations/rule-configuration.md index 0d75cf54e89a..f4116a458356 100644 --- a/docs/operations/rule-configuration.md +++ b/docs/operations/rule-configuration.md @@ -181,6 +181,58 @@ Set the following properties: - `interval`: the load interval specified as an [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601) range encoded as a string. - `tieredReplicants`: a map of tier names to the number of segment replicas for that tier. +## Default Value for tiered replicants + +`useDefaultTierForNull` is an optional parameter that can be passed to a Load Rule. This parameter determines the default value of `tieredReplicants` only has an effect if the field is not present. The default value of `useDefaultTierForNull` is true. + +If this parameter is true, a missing `tieredReplicant` in the load rule is assumed to mean that the segment matching this rule should be loaded on historicals be default. This will initilaize `tieredReplicants` with `"tieredReplicants": { "_default_tier": 2 }` + +If this parameter is false, a missing `tieredReplicants` in the load rule is assumed to mean that the segment matching this rule does not need to be loaded on any historical by default. This will mean that these segments are excluded from most queries as well. This will initilaize `tieredReplicants` as an empty map. + +Example: + +With `useDefaultTierForNull` as true: + +```json +{ + "type": "loadForever", + "useDefaultTierForNull": true +} +``` + +is converted to + +```json +{ + "type": "loadForever", + "useDefaultTierForNull": true, + "tieredReplicants": { + "_default_tier": 2 + } +} +``` + +With `useDefaultTierForNull` as false: + +```json +{ + "type": "loadByInterval", + "useDefaultTierForNull": false, + "interval": "2012-01-01/2013-01-01" +} +``` + +is converted to + +```json +{ + "type": "loadByInterval", + "useDefaultTierForNull": false, + "interval": "2012-01-01/2013-01-01", + "tieredReplicants": {} +} +``` + ## Drop rules Drop rules define when Druid drops segments from the cluster. Druid keeps dropped data in deep storage. Note that if you enable automatic cleanup of unused segments, or you run a kill task, Druid deletes the data from deep storage. See [Data deletion](../data-management/delete.md) for more information on deleting data. diff --git a/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java b/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java index 4344e191b81f..5d7b724c8451 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/rules/LoadRule.java @@ -35,6 +35,9 @@ public abstract class LoadRule implements Rule { private final Map tieredReplicants; + /** + * Used to determing the default value if tieredReplicants is null in {@link #handleNullTieredReplicants}. + */ private final boolean useDefaultTierForNull; protected LoadRule(Map tieredReplicants, Boolean useDefaultTierForNull) diff --git a/server/src/test/java/org/apache/druid/server/coordinator/rules/LoadRuleTest.java b/server/src/test/java/org/apache/druid/server/coordinator/rules/LoadRuleTest.java index 1deca43b2a42..668013f7ff34 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/rules/LoadRuleTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/rules/LoadRuleTest.java @@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.util.concurrent.ListeningExecutorService; import com.google.common.util.concurrent.MoreExecutors; +import nl.jqno.equalsverifier.EqualsVerifier; import org.apache.druid.client.DruidServer; import org.apache.druid.client.ImmutableDruidServer; import org.apache.druid.java.util.common.DateTimes; @@ -764,4 +765,13 @@ private static class Tier static final String T1 = "tier1"; static final String T2 = "tier2"; } + + @Test + public void testEquals() + { + EqualsVerifier.forClass(LoadRule.class) + .withNonnullFields("tieredReplicants") + .usingGetClass() + .verify(); + } } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java b/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java index ebb8dee76481..7b1dd2085f02 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/rules/PeriodLoadRuleTest.java @@ -21,6 +21,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableMap; +import nl.jqno.equalsverifier.EqualsVerifier; import org.apache.druid.client.DruidServer; import org.apache.druid.error.DruidException; import org.apache.druid.error.DruidExceptionMatcher; @@ -273,4 +274,13 @@ public void testUseDefaultTierAsFalseShouldCreateEmptyMap() throws Exception PeriodLoadRule inputPeriodLoadRule = OBJECT_MAPPER.readValue(inputJson, PeriodLoadRule.class); Assert.assertEquals(ImmutableMap.of(), inputPeriodLoadRule.getTieredReplicants()); } + + @Test + public void testEquals() + { + EqualsVerifier.forClass(PeriodLoadRule.class) + .withNonnullFields("tieredReplicants") + .usingGetClass() + .verify(); + } } From dba1605c327809a56cb2b7d5b866a12406938724 Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Wed, 21 Jun 2023 15:25:08 +0530 Subject: [PATCH 15/18] Update docs --- docs/operations/rule-configuration.md | 100 ++++++++++++++------------ 1 file changed, 55 insertions(+), 45 deletions(-) diff --git a/docs/operations/rule-configuration.md b/docs/operations/rule-configuration.md index f4116a458356..75d9c4771712 100644 --- a/docs/operations/rule-configuration.md +++ b/docs/operations/rule-configuration.md @@ -109,7 +109,58 @@ In the web console you can use the up and down arrows on the right side of the i Load rules define how Druid assigns segments to [historical process tiers](./mixed-workloads.md#historical-tiering), and how many replicas of a segment exist in each tier. -If you have a single tier, Druid automatically names the tier `_default` and loads all segments onto it. If you define an additional tier, you must define a load rule to specify which segments to load on that tier. Until you define a load rule, your new tier remains empty. +If you have a single tier, Druid automatically names the tier `_default`. If you define an additional tier, you must define a load rule to specify which segments to load on that tier. Until you define a load rule, your new tier remains empty. + +`useDefaultTierForNull` is an optional parameter that can be passed to a Load Rule. This parameter determines the default value of `tieredReplicants` only has an effect if the field is not present. The default value of `useDefaultTierForNull` is true. + +If this parameter is true, a missing `tieredReplicant` in the load rule is assumed to mean that the segment matching this rule should be loaded on historicals by default. This will initilaize `tieredReplicants` with the default tier `"tieredReplicants": { "_default_tier": 2 }`. + +If this parameter is false, a missing `tieredReplicants` in the load rule is assumed to mean that the segment matching this rule does not need to be loaded on any historical by default. This will mean that these segments are excluded from most queries as well. This will initilaize `tieredReplicants` as an empty map. + +Example: + +With `useDefaultTierForNull` as true: + +```json +{ + "type": "loadForever", + "useDefaultTierForNull": true +} +``` + +is converted to + +```json +{ + "type": "loadForever", + "useDefaultTierForNull": true, + "tieredReplicants": { + "_default_tier": 2 + } +} +``` + +With `useDefaultTierForNull` as false: + +```json +{ + "type": "loadByInterval", + "useDefaultTierForNull": false, + "interval": "2012-01-01/2013-01-01" +} +``` + +is converted to + +```json +{ + "type": "loadByInterval", + "useDefaultTierForNull": false, + "interval": "2012-01-01/2013-01-01", + "tieredReplicants": {} +} +``` + ### Forever load rule @@ -130,6 +181,7 @@ The following example places one replica of each segment on a custom tier named Set the following property: - `tieredReplicants`: a map of tier names to the number of segment replicas for that tier. +- `useDefaultTierForNull`: This parameter determines the default value of `tieredReplicants` and only has an effect if the field is not present. The default value of `useDefaultTierForNull` is true. ### Period load rule @@ -158,6 +210,7 @@ Set the following properties: You can use this property to load segments with future start and end dates, where "future" is relative to the time when the Coordinator evaluates data against the rule. Defaults to `true`. - `tieredReplicants`: a map of tier names to the number of segment replicas for that tier. +- `useDefaultTierForNull`: This parameter determines the default value of `tieredReplicants` and only has an effect if the field is not present. The default value of `useDefaultTierForNull` is true. ### Interval load rule @@ -180,6 +233,7 @@ Set the following properties: - `interval`: the load interval specified as an [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601) range encoded as a string. - `tieredReplicants`: a map of tier names to the number of segment replicas for that tier. +- `useDefaultTierForNull`: This parameter determines the default value of `tieredReplicants` and only has an effect if the field is not present. The default value of `useDefaultTierForNull` is true. ## Default Value for tiered replicants @@ -189,50 +243,6 @@ If this parameter is true, a missing `tieredReplicant` in the load rule is assum If this parameter is false, a missing `tieredReplicants` in the load rule is assumed to mean that the segment matching this rule does not need to be loaded on any historical by default. This will mean that these segments are excluded from most queries as well. This will initilaize `tieredReplicants` as an empty map. -Example: - -With `useDefaultTierForNull` as true: - -```json -{ - "type": "loadForever", - "useDefaultTierForNull": true -} -``` - -is converted to - -```json -{ - "type": "loadForever", - "useDefaultTierForNull": true, - "tieredReplicants": { - "_default_tier": 2 - } -} -``` - -With `useDefaultTierForNull` as false: - -```json -{ - "type": "loadByInterval", - "useDefaultTierForNull": false, - "interval": "2012-01-01/2013-01-01" -} -``` - -is converted to - -```json -{ - "type": "loadByInterval", - "useDefaultTierForNull": false, - "interval": "2012-01-01/2013-01-01", - "tieredReplicants": {} -} -``` - ## Drop rules Drop rules define when Druid drops segments from the cluster. Druid keeps dropped data in deep storage. Note that if you enable automatic cleanup of unused segments, or you run a kill task, Druid deletes the data from deep storage. See [Data deletion](../data-management/delete.md) for more information on deleting data. From 70caf984d5397ad0f94a81ecba14610a70fdb1c9 Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Wed, 21 Jun 2023 15:34:16 +0530 Subject: [PATCH 16/18] Update docs --- docs/operations/rule-configuration.md | 8 -------- 1 file changed, 8 deletions(-) diff --git a/docs/operations/rule-configuration.md b/docs/operations/rule-configuration.md index 75d9c4771712..8943ce881416 100644 --- a/docs/operations/rule-configuration.md +++ b/docs/operations/rule-configuration.md @@ -235,14 +235,6 @@ Set the following properties: - `tieredReplicants`: a map of tier names to the number of segment replicas for that tier. - `useDefaultTierForNull`: This parameter determines the default value of `tieredReplicants` and only has an effect if the field is not present. The default value of `useDefaultTierForNull` is true. -## Default Value for tiered replicants - -`useDefaultTierForNull` is an optional parameter that can be passed to a Load Rule. This parameter determines the default value of `tieredReplicants` only has an effect if the field is not present. The default value of `useDefaultTierForNull` is true. - -If this parameter is true, a missing `tieredReplicant` in the load rule is assumed to mean that the segment matching this rule should be loaded on historicals be default. This will initilaize `tieredReplicants` with `"tieredReplicants": { "_default_tier": 2 }` - -If this parameter is false, a missing `tieredReplicants` in the load rule is assumed to mean that the segment matching this rule does not need to be loaded on any historical by default. This will mean that these segments are excluded from most queries as well. This will initilaize `tieredReplicants` as an empty map. - ## Drop rules Drop rules define when Druid drops segments from the cluster. Druid keeps dropped data in deep storage. Note that if you enable automatic cleanup of unused segments, or you run a kill task, Druid deletes the data from deep storage. See [Data deletion](../data-management/delete.md) for more information on deleting data. From 3c8ad5f24d7a502fd12c12c7d5fb4951a610bacd Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Wed, 21 Jun 2023 16:39:46 +0530 Subject: [PATCH 17/18] Fix spelling --- docs/operations/rule-configuration.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/operations/rule-configuration.md b/docs/operations/rule-configuration.md index 8943ce881416..7191b811b42b 100644 --- a/docs/operations/rule-configuration.md +++ b/docs/operations/rule-configuration.md @@ -113,9 +113,9 @@ If you have a single tier, Druid automatically names the tier `_default`. If you `useDefaultTierForNull` is an optional parameter that can be passed to a Load Rule. This parameter determines the default value of `tieredReplicants` only has an effect if the field is not present. The default value of `useDefaultTierForNull` is true. -If this parameter is true, a missing `tieredReplicant` in the load rule is assumed to mean that the segment matching this rule should be loaded on historicals by default. This will initilaize `tieredReplicants` with the default tier `"tieredReplicants": { "_default_tier": 2 }`. +If this parameter is true, a missing `tieredReplicant` in the load rule is assumed to mean that the segment matching this rule should be loaded on historicals by default. This will initialize `tieredReplicants` with the default tier `"tieredReplicants": { "_default_tier": 2 }`. -If this parameter is false, a missing `tieredReplicants` in the load rule is assumed to mean that the segment matching this rule does not need to be loaded on any historical by default. This will mean that these segments are excluded from most queries as well. This will initilaize `tieredReplicants` as an empty map. +If this parameter is false, a missing `tieredReplicants` in the load rule is assumed to mean that the segment matching this rule does not need to be loaded on any historical by default. This will mean that these segments are excluded from most queries as well. This will initialize `tieredReplicants` as an empty map. Example: From 553aea98fad9ca5598fc5fb7364cf490b282b3fa Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Thu, 22 Jun 2023 00:13:02 +0530 Subject: [PATCH 18/18] Update docs --- docs/operations/rule-configuration.md | 54 +++------------------------ 1 file changed, 6 insertions(+), 48 deletions(-) diff --git a/docs/operations/rule-configuration.md b/docs/operations/rule-configuration.md index 7191b811b42b..0bf803355aad 100644 --- a/docs/operations/rule-configuration.md +++ b/docs/operations/rule-configuration.md @@ -111,56 +111,14 @@ Load rules define how Druid assigns segments to [historical process tiers](./mix If you have a single tier, Druid automatically names the tier `_default`. If you define an additional tier, you must define a load rule to specify which segments to load on that tier. Until you define a load rule, your new tier remains empty. -`useDefaultTierForNull` is an optional parameter that can be passed to a Load Rule. This parameter determines the default value of `tieredReplicants` only has an effect if the field is not present. The default value of `useDefaultTierForNull` is true. +All load rules can have these properties: -If this parameter is true, a missing `tieredReplicant` in the load rule is assumed to mean that the segment matching this rule should be loaded on historicals by default. This will initialize `tieredReplicants` with the default tier `"tieredReplicants": { "_default_tier": 2 }`. - -If this parameter is false, a missing `tieredReplicants` in the load rule is assumed to mean that the segment matching this rule does not need to be loaded on any historical by default. This will mean that these segments are excluded from most queries as well. This will initialize `tieredReplicants` as an empty map. - -Example: - -With `useDefaultTierForNull` as true: - -```json -{ - "type": "loadForever", - "useDefaultTierForNull": true -} -``` - -is converted to - -```json -{ - "type": "loadForever", - "useDefaultTierForNull": true, - "tieredReplicants": { - "_default_tier": 2 - } -} -``` - -With `useDefaultTierForNull` as false: - -```json -{ - "type": "loadByInterval", - "useDefaultTierForNull": false, - "interval": "2012-01-01/2013-01-01" -} -``` - -is converted to - -```json -{ - "type": "loadByInterval", - "useDefaultTierForNull": false, - "interval": "2012-01-01/2013-01-01", - "tieredReplicants": {} -} -``` +|Property|Description|Required|Default value| +|---------|-----------|---------|-------------| +| `tieredReplicants`| Map from tier names to the respective number of segment replicas to be loaded on those tiers. The number of replicas for each tier must be either 0 or a positive integer.| No | When `useDefaultTierForNull` is `true`, the default value is `{"_default_tier": 2}` i.e. 2 replicas to be loaded on the `_default_tier`.

    When `useDefaultTierForNull` is `false`, the default value is `{}` i.e. no replicas to be loaded on any tier. | +|`useDefaultTierForNull`|Determines the default value of `tieredReplicants` if it is not specified or set to `null`.| No | `true`| +Specific types of load rules discussed below may have other properties too. ### Forever load rule