From 4f14ccf93e724ca9ab05d6c2263c6b9a8782f5a2 Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Wed, 17 Jan 2024 17:04:26 +0530 Subject: [PATCH 01/17] Remove usage of skife from DruidCoordinatorConfig --- .../server/coordinator/DruidCoordinator.java | 27 +- .../coordinator/DruidCoordinatorConfig.java | 206 +++---- .../DruidCoordinatorConfigBackup.java | 166 ++++++ .../config/CoordinatorKillConfigs.java | 152 +++++ .../config/CoordinatorPeriodConfig.java | 57 ++ .../config/CoordinatorRunConfig.java | 54 ++ .../config/HttpLoadQueuePeonConfig.java | 71 +++ .../config/KillUnusedSegmentsConfig.java | 69 +++ .../config/MetadataCleanupConfig.java | 89 +++ .../server/coordinator/duty/KillAuditLog.java | 14 +- .../duty/KillCompactionConfig.java | 15 +- .../duty/KillDatasourceMetadata.java | 14 +- .../server/coordinator/duty/KillRules.java | 14 +- .../coordinator/duty/KillSupervisors.java | 14 +- .../duty/KillSupervisorsCustomDuty.java | 19 +- .../coordinator/duty/KillUnusedSegments.java | 18 +- .../coordinator/duty/MetadataCleanupDuty.java | 61 +- .../loading/CuratorLoadQueuePeon.java | 9 +- .../loading/HttpLoadQueuePeon.java | 17 +- .../loading/LoadQueueTaskMaster.java | 35 +- .../CuratorDruidCoordinatorTest.java | 14 +- .../DruidCoordinatorConfigTest.java | 464 ++++++++++++++-- .../coordinator/DruidCoordinatorTest.java | 39 +- .../TestDruidCoordinatorConfig.java | 523 ------------------ .../coordinator/duty/KillAuditLogTest.java | 66 +-- .../duty/KillCompactionConfigTest.java | 68 +-- .../duty/KillDatasourceMetadataTest.java | 85 +-- .../coordinator/duty/KillRulesTest.java | 65 +-- .../duty/KillSupervisorsCustomDutyTest.java | 11 +- .../coordinator/duty/KillSupervisorsTest.java | 66 +-- .../duty/KillUnusedSegmentsTest.java | 60 +- .../loading/HttpLoadQueuePeonTest.java | 6 +- .../loading/LoadQueuePeonTest.java | 22 +- .../CoordinatorSimulationBuilder.java | 102 ++-- .../org/apache/druid/cli/CliCoordinator.java | 33 +- 35 files changed, 1389 insertions(+), 1356 deletions(-) create mode 100644 server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigBackup.java create mode 100644 server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorKillConfigs.java create mode 100644 server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorPeriodConfig.java create mode 100644 server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorRunConfig.java create mode 100644 server/src/main/java/org/apache/druid/server/coordinator/config/HttpLoadQueuePeonConfig.java create mode 100644 server/src/main/java/org/apache/druid/server/coordinator/config/KillUnusedSegmentsConfig.java create mode 100644 server/src/main/java/org/apache/druid/server/coordinator/config/MetadataCleanupConfig.java delete mode 100644 server/src/test/java/org/apache/druid/server/coordinator/TestDruidCoordinatorConfig.java diff --git a/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java b/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java index 5558d204e81a..2db717044c1c 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java @@ -54,6 +54,8 @@ import org.apache.druid.server.DruidNode; import org.apache.druid.server.coordinator.balancer.BalancerStrategyFactory; import org.apache.druid.server.coordinator.compact.CompactionSegmentSearchPolicy; +import org.apache.druid.server.coordinator.config.CoordinatorKillConfigs; +import org.apache.druid.server.coordinator.config.KillUnusedSegmentsConfig; import org.apache.druid.server.coordinator.duty.BalanceSegments; import org.apache.druid.server.coordinator.duty.CollectSegmentAndServerStats; import org.apache.druid.server.coordinator.duty.CompactSegments; @@ -182,7 +184,6 @@ public DruidCoordinator( ServiceAnnouncer serviceAnnouncer, @Self DruidNode self, CoordinatorCustomDutyGroups customDutyGroups, - BalancerStrategyFactory balancerStrategyFactory, LookupCoordinatorManager lookupCoordinatorManager, @Coordinator DruidLeaderSelector coordLeaderSelector, CompactionSegmentSearchPolicy compactionSegmentSearchPolicy @@ -200,7 +201,7 @@ public DruidCoordinator( this.executorFactory = scheduledExecutorFactory; - this.balancerStrategyFactory = balancerStrategyFactory; + this.balancerStrategyFactory = config.getBalancerStrategyFactory(); this.lookupCoordinatorManager = lookupCoordinatorManager; this.coordLeaderSelector = coordLeaderSelector; this.compactSegments = initializeCompactSegmentsDuty(compactionSegmentSearchPolicy); @@ -526,10 +527,11 @@ private List makeHistoricalManagementDuties() List makeIndexingServiceDuties() { final List duties = new ArrayList<>(); - if (config.isKillUnusedSegmentsEnabled()) { - duties.add(new KillUnusedSegments(metadataManager.segments(), overlordClient, config)); + final KillUnusedSegmentsConfig killUnusedConfig = config.getKillConfigs().unusedSegments(); + if (killUnusedConfig.isCleanupEnabled()) { + duties.add(new KillUnusedSegments(metadataManager.segments(), overlordClient, killUnusedConfig)); } - if (config.isKillPendingSegmentsEnabled()) { + if (config.getKillConfigs().pendingSegments().isCleanupEnabled()) { duties.add(new KillStalePendingSegments(overlordClient)); } @@ -547,12 +549,17 @@ List makeIndexingServiceDuties() private List makeMetadataStoreManagementDuties() { + final CoordinatorKillConfigs killConfigs = config.getKillConfigs(); return Arrays.asList( - new KillSupervisors(config, metadataManager.supervisors()), - new KillAuditLog(config, metadataManager.audit()), - new KillRules(config, metadataManager.rules()), - new KillDatasourceMetadata(config, metadataManager.indexer(), metadataManager.supervisors()), - new KillCompactionConfig(config, metadataManager.segments(), metadataManager.configs()) + new KillSupervisors(killConfigs.supervisors(), metadataManager.supervisors()), + new KillAuditLog(killConfigs.audit(), metadataManager.audit()), + new KillRules(killConfigs.rules(), metadataManager.rules()), + new KillDatasourceMetadata( + killConfigs.datasource(), + metadataManager.indexer(), + metadataManager.supervisors() + ), + new KillCompactionConfig(killConfigs.compaction(), metadataManager.segments(), metadataManager.configs()) ); } diff --git a/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinatorConfig.java b/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinatorConfig.java index 2aeead38d740..93ef3e8e5fdb 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinatorConfig.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinatorConfig.java @@ -19,148 +19,122 @@ package org.apache.druid.server.coordinator; +import com.google.common.base.Preconditions; +import com.google.inject.Inject; +import org.apache.druid.server.coordinator.balancer.BalancerStrategyFactory; +import org.apache.druid.server.coordinator.config.CoordinatorKillConfigs; +import org.apache.druid.server.coordinator.config.CoordinatorPeriodConfig; +import org.apache.druid.server.coordinator.config.CoordinatorRunConfig; +import org.apache.druid.server.coordinator.config.HttpLoadQueuePeonConfig; +import org.apache.druid.server.coordinator.config.KillUnusedSegmentsConfig; +import org.apache.druid.server.coordinator.config.MetadataCleanupConfig; import org.joda.time.Duration; -import org.skife.config.Config; -import org.skife.config.Default; /** + * Contains all static configs for the Coordinator. */ -public abstract class DruidCoordinatorConfig +public class DruidCoordinatorConfig { - @Config("druid.coordinator.startDelay") - @Default("PT300s") - public abstract Duration getCoordinatorStartDelay(); - - @Config("druid.coordinator.period") - @Default("PT60s") - public abstract Duration getCoordinatorPeriod(); - - @Config("druid.coordinator.period.indexingPeriod") - @Default("PT1800s") - public abstract Duration getCoordinatorIndexingPeriod(); - - @Config("druid.coordinator.period.metadataStoreManagementPeriod") - @Default("PT1H") - public abstract Duration getCoordinatorMetadataStoreManagementPeriod(); - - @Config("druid.coordinator.kill.on") - @Default("false") - public abstract boolean isKillUnusedSegmentsEnabled(); - - @Config("druid.coordinator.kill.period") - @Default("P1D") - public abstract Duration getCoordinatorKillPeriod(); - - @Config("druid.coordinator.kill.durationToRetain") - @Default("P90D") - public abstract Duration getCoordinatorKillDurationToRetain(); - - @Config("druid.coordinator.kill.ignoreDurationToRetain") - @Default("false") - public abstract boolean getCoordinatorKillIgnoreDurationToRetain(); - - @Config("druid.coordinator.kill.bufferPeriod") - @Default("P30D") - public abstract Duration getCoordinatorKillBufferPeriod(); - - @Config("druid.coordinator.kill.maxSegments") - @Default("100") - public abstract int getCoordinatorKillMaxSegments(); - - @Config("druid.coordinator.kill.pendingSegments.on") - @Default("true") - public abstract boolean isKillPendingSegmentsEnabled(); - - @Config("druid.coordinator.kill.supervisor.on") - @Default("true") - public abstract boolean isSupervisorKillEnabled(); - - @Config("druid.coordinator.kill.supervisor.period") - @Default("P1D") - public abstract Duration getCoordinatorSupervisorKillPeriod(); - - @Config("druid.coordinator.kill.supervisor.durationToRetain") - @Default("P90D") - public abstract Duration getCoordinatorSupervisorKillDurationToRetain(); - - @Config("druid.coordinator.kill.audit.on") - @Default("true") - public abstract boolean isAuditKillEnabled(); - - @Config("druid.coordinator.kill.audit.period") - @Default("P1D") - public abstract Duration getCoordinatorAuditKillPeriod(); - - @Config("druid.coordinator.kill.audit.durationToRetain") - @Default("P90D") - public abstract Duration getCoordinatorAuditKillDurationToRetain(); - - @Config("druid.coordinator.kill.compaction.on") - @Default("false") - public abstract boolean isCompactionKillEnabled(); - - @Config("druid.coordinator.kill.compaction.period") - @Default("P1D") - public abstract Duration getCoordinatorCompactionKillPeriod(); - - @Config("druid.coordinator.kill.rule.on") - @Default("true") - public abstract boolean isRuleKillEnabled(); - - @Config("druid.coordinator.kill.rule.period") - @Default("P1D") - public abstract Duration getCoordinatorRuleKillPeriod(); + private final CoordinatorRunConfig runConfig; + private final CoordinatorPeriodConfig periodConfig; + private final CoordinatorKillConfigs killConfigs; + private final BalancerStrategyFactory balancerStrategyFactory; + private final HttpLoadQueuePeonConfig httpLoadQueuePeonConfig; + + @Inject + public DruidCoordinatorConfig( + CoordinatorRunConfig runConfig, + CoordinatorPeriodConfig periodConfig, + CoordinatorKillConfigs killConfigs, + BalancerStrategyFactory balancerStrategyFactory, + HttpLoadQueuePeonConfig httpLoadQueuePeonConfig + ) + { + this.killConfigs = killConfigs; + this.runConfig = runConfig; + this.periodConfig = periodConfig; + this.balancerStrategyFactory = balancerStrategyFactory; + this.httpLoadQueuePeonConfig = httpLoadQueuePeonConfig; - @Config("druid.coordinator.kill.rule.durationToRetain") - @Default("P90D") - public abstract Duration getCoordinatorRuleKillDurationToRetain(); + validateKillConfigs(); + } - @Config("druid.coordinator.kill.datasource.on") - @Default("true") - public abstract boolean isDatasourceKillEnabled(); + public Duration getCoordinatorStartDelay() + { + return runConfig.getStartDelay(); + } - @Config("druid.coordinator.kill.datasource.period") - @Default("P1D") - public abstract Duration getCoordinatorDatasourceKillPeriod(); + public Duration getCoordinatorPeriod() + { + return runConfig.getPeriod(); + } - @Config("druid.coordinator.kill.datasource.durationToRetain") - @Default("P90D") - public abstract Duration getCoordinatorDatasourceKillDurationToRetain(); + public Duration getCoordinatorIndexingPeriod() + { + return periodConfig.getIndexingPeriod(); + } - @Config("druid.coordinator.load.timeout") - public Duration getLoadTimeoutDelay() + public Duration getCoordinatorMetadataStoreManagementPeriod() { - return new Duration(15 * 60 * 1000); + return periodConfig.getMetadataStoreManagementPeriod(); } - @Config("druid.coordinator.loadqueuepeon.type") - public String getLoadQueuePeonType() + public CoordinatorKillConfigs getKillConfigs() { - return "http"; + return killConfigs; } - @Config("druid.coordinator.curator.loadqueuepeon.numCallbackThreads") - public int getNumCuratorCallBackThreads() + public BalancerStrategyFactory getBalancerStrategyFactory() { - return 2; + return balancerStrategyFactory; } - @Config("druid.coordinator.loadqueuepeon.http.repeatDelay") - public Duration getHttpLoadQueuePeonRepeatDelay() + public HttpLoadQueuePeonConfig getHttpLoadQueuePeonConfig() { - return Duration.millis(60000); + return httpLoadQueuePeonConfig; } - @Config("druid.coordinator.loadqueuepeon.http.hostTimeout") - public Duration getHttpLoadQueuePeonHostTimeout() + private void validateKillConfigs() { - return Duration.millis(300000); + validateKillConfig(killConfigs.audit(), "audit"); + validateKillConfig(killConfigs.compaction(), "compaction"); + validateKillConfig(killConfigs.datasource(), "datasource"); + validateKillConfig(killConfigs.rules(), "rule"); + validateKillConfig(killConfigs.supervisors(), "supervisor"); + + final KillUnusedSegmentsConfig killUnusedConfig = killConfigs.unusedSegments(); + Preconditions.checkArgument( + killUnusedConfig.getCleanupPeriod().getMillis() >= getCoordinatorIndexingPeriod().getMillis(), + "[druid.coordinator.kill.period] must be greater than or equal to [druid.coordinator.period.indexingPeriod]" + ); } - @Config("druid.coordinator.loadqueuepeon.http.batchSize") - public int getHttpLoadQueuePeonBatchSize() + private void validateKillConfig(MetadataCleanupConfig config, String propertyPrefix) { - return 1; + if (!config.isCleanupEnabled()) { + // Do not perform validation if cleanup is disabled + return; + } + + final Duration metadataManagementPeriod = getCoordinatorMetadataStoreManagementPeriod(); + final Duration period = config.getCleanupPeriod(); + Preconditions.checkArgument( + period != null && period.getMillis() >= metadataManagementPeriod.getMillis(), + "[druid.coordinator.kill.%s.period] must be greater than [druid.coordinator.period.metadataStoreManagementPeriod]", + propertyPrefix + ); + + final Duration retainDuration = config.getDurationToRetain(); + Preconditions.checkArgument( + retainDuration != null && retainDuration.getMillis() >= 0, + "[druid.coordinator.kill.%s.durationToRetain] must be 0 milliseconds or higher", + propertyPrefix + ); + Preconditions.checkArgument( + retainDuration.getMillis() < System.currentTimeMillis(), + "[druid.coordinator.kill.%s.durationToRetain] cannot be greater than current time in milliseconds", + propertyPrefix + ); } } diff --git a/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigBackup.java b/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigBackup.java new file mode 100644 index 000000000000..fb2729d6ca3a --- /dev/null +++ b/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigBackup.java @@ -0,0 +1,166 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.server.coordinator; + +import org.joda.time.Duration; +import org.skife.config.Config; +import org.skife.config.Default; + +/** + */ +public abstract class DruidCoordinatorConfigBackup +{ + @Config("druid.coordinator.startDelay") + @Default("PT300s") + public abstract Duration getCoordinatorStartDelay(); + + @Config("druid.coordinator.period") + @Default("PT60s") + public abstract Duration getCoordinatorPeriod(); + + @Config("druid.coordinator.period.indexingPeriod") + @Default("PT1800s") + public abstract Duration getCoordinatorIndexingPeriod(); + + @Config("druid.coordinator.period.metadataStoreManagementPeriod") + @Default("PT1H") + public abstract Duration getCoordinatorMetadataStoreManagementPeriod(); + + @Config("druid.coordinator.kill.on") + @Default("false") + public abstract boolean isKillUnusedSegmentsEnabled(); + + @Config("druid.coordinator.kill.period") + @Default("P1D") + public abstract Duration getCoordinatorKillPeriod(); + + @Config("druid.coordinator.kill.durationToRetain") + @Default("P90D") + public abstract Duration getCoordinatorKillDurationToRetain(); + + @Config("druid.coordinator.kill.ignoreDurationToRetain") + @Default("false") + public abstract boolean getCoordinatorKillIgnoreDurationToRetain(); + + @Config("druid.coordinator.kill.bufferPeriod") + @Default("P30D") + public abstract Duration getCoordinatorKillBufferPeriod(); + + @Config("druid.coordinator.kill.maxSegments") + @Default("100") + public abstract int getCoordinatorKillMaxSegments(); + + @Config("druid.coordinator.kill.pendingSegments.on") + @Default("true") + public abstract boolean isKillPendingSegmentsEnabled(); + + @Config("druid.coordinator.kill.supervisor.on") + @Default("true") + public abstract boolean isSupervisorKillEnabled(); + + @Config("druid.coordinator.kill.supervisor.period") + @Default("P1D") + public abstract Duration getCoordinatorSupervisorKillPeriod(); + + @Config("druid.coordinator.kill.supervisor.durationToRetain") + @Default("P90D") + public abstract Duration getCoordinatorSupervisorKillDurationToRetain(); + + @Config("druid.coordinator.kill.audit.on") + @Default("true") + public abstract boolean isAuditKillEnabled(); + + @Config("druid.coordinator.kill.audit.period") + @Default("P1D") + public abstract Duration getCoordinatorAuditKillPeriod(); + + @Config("druid.coordinator.kill.audit.durationToRetain") + @Default("P90D") + public abstract Duration getCoordinatorAuditKillDurationToRetain(); + + @Config("druid.coordinator.kill.compaction.on") + @Default("false") + public abstract boolean isCompactionKillEnabled(); + + @Config("druid.coordinator.kill.compaction.period") + @Default("P1D") + public abstract Duration getCoordinatorCompactionKillPeriod(); + + @Config("druid.coordinator.kill.rule.on") + @Default("true") + public abstract boolean isRuleKillEnabled(); + + @Config("druid.coordinator.kill.rule.period") + @Default("P1D") + public abstract Duration getCoordinatorRuleKillPeriod(); + + @Config("druid.coordinator.kill.rule.durationToRetain") + @Default("P90D") + public abstract Duration getCoordinatorRuleKillDurationToRetain(); + + @Config("druid.coordinator.kill.datasource.on") + @Default("true") + public abstract boolean isDatasourceKillEnabled(); + + @Config("druid.coordinator.kill.datasource.period") + @Default("P1D") + public abstract Duration getCoordinatorDatasourceKillPeriod(); + + @Config("druid.coordinator.kill.datasource.durationToRetain") + @Default("P90D") + public abstract Duration getCoordinatorDatasourceKillDurationToRetain(); + + @Config("druid.coordinator.load.timeout") + public Duration getLoadTimeoutDelay() + { + return new Duration(15 * 60 * 1000); + } + + @Config("druid.coordinator.loadqueuepeon.type") + public String getLoadQueuePeonType() + { + return "http"; + } + + @Config("druid.coordinator.curator.loadqueuepeon.numCallbackThreads") + public int getNumCuratorCallBackThreads() + { + return 2; + } + + @Config("druid.coordinator.loadqueuepeon.http.repeatDelay") + public Duration getHttpLoadQueuePeonRepeatDelay() + { + return Duration.millis(60000); + } + + @Config("druid.coordinator.loadqueuepeon.http.hostTimeout") + public Duration getHttpLoadQueuePeonHostTimeout() + { + return Duration.millis(300000); + } + + @Config("druid.coordinator.loadqueuepeon.http.batchSize") + public int getHttpLoadQueuePeonBatchSize() + { + return 1; + } + +} diff --git a/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorKillConfigs.java b/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorKillConfigs.java new file mode 100644 index 000000000000..5713c4de37f1 --- /dev/null +++ b/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorKillConfigs.java @@ -0,0 +1,152 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.server.coordinator.config; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import org.apache.druid.common.config.Configs; +import org.joda.time.Duration; + +public class CoordinatorKillConfigs +{ + public static CoordinatorKillConfigs STANDARD + = new CoordinatorKillConfigs(null, null, null, null, null, null, null, null, null, null, null, null); + + @JsonProperty("supervisor") + private final MetadataCleanupConfig supervisors; + + @JsonProperty("audit") + private final MetadataCleanupConfig audit; + + @JsonProperty("datasource") + private final MetadataCleanupConfig datasource; + + @JsonProperty("rule") + private final MetadataCleanupConfig rules; + + @JsonProperty("compaction") + private final MetadataCleanupConfig compaction; + + @JsonProperty("pendingSegments") + private final MetadataCleanupConfig pendingSegments; + + private final KillUnusedSegmentsConfig unusedSegments; + + // Raw configs for killing unused segments + // These have been added as fields just to keep JsonConfigurator happy + @JsonProperty("on") + private final Boolean killUnusedEnabled; + + @JsonProperty("period") + private final Duration killUnusedPeriod; + + @JsonProperty("durationToRetain") + private final Duration killUnusedDurationToRetain; + + @JsonProperty("ignoredDurationToRetain") + private final Boolean killUnusedIgnoreDurationToRetain; + + @JsonProperty("bufferPeriod") + private final Duration killUnusedBufferPeriod; + + @JsonProperty("maxSegments") + private final Integer killUnusedMaxSegments; + + @JsonCreator + public CoordinatorKillConfigs( + @JsonProperty("pendingSegments") MetadataCleanupConfig pendingSegments, + @JsonProperty("supervisor") MetadataCleanupConfig supervisors, + @JsonProperty("audit") MetadataCleanupConfig audit, + @JsonProperty("datasource") MetadataCleanupConfig datasource, + @JsonProperty("rule") MetadataCleanupConfig rules, + @JsonProperty("compaction") MetadataCleanupConfig compaction, + // Configs for cleanup of unused segments + @JsonProperty("on") Boolean killUnusedEnabled, + @JsonProperty("period") Duration killUnusedPeriod, + @JsonProperty("durationToRetain") Duration killUnusedDurationToRetain, + @JsonProperty("ignoredDurationToRetain") Boolean killUnusedIgnoreDurationToRetain, + @JsonProperty("bufferPeriod") Duration killUnusedBufferPeriod, + @JsonProperty("maxSegments") Integer killUnusedMaxSegments + ) + { + this.pendingSegments = Configs.valueOrDefault(pendingSegments, MetadataCleanupConfig.STANDARD); + this.supervisors = Configs.valueOrDefault(supervisors, MetadataCleanupConfig.STANDARD); + this.audit = Configs.valueOrDefault(audit, MetadataCleanupConfig.STANDARD); + this.datasource = Configs.valueOrDefault(datasource, MetadataCleanupConfig.STANDARD); + this.rules = Configs.valueOrDefault(rules, MetadataCleanupConfig.STANDARD); + this.compaction = Configs.valueOrDefault(compaction, MetadataCleanupConfig.STANDARD); + + this.killUnusedEnabled = killUnusedEnabled; + this.killUnusedPeriod = killUnusedPeriod; + this.killUnusedDurationToRetain = killUnusedDurationToRetain; + this.killUnusedBufferPeriod = killUnusedBufferPeriod; + this.killUnusedIgnoreDurationToRetain = killUnusedIgnoreDurationToRetain; + this.killUnusedMaxSegments = killUnusedMaxSegments; + + this.unusedSegments = createUnusedSegmentsConfig(); + } + + private KillUnusedSegmentsConfig createUnusedSegmentsConfig() + { + return new KillUnusedSegmentsConfig( + killUnusedEnabled, + killUnusedPeriod, + killUnusedDurationToRetain, + killUnusedIgnoreDurationToRetain, + killUnusedBufferPeriod, + killUnusedMaxSegments + ); + } + + public MetadataCleanupConfig audit() + { + return audit; + } + + public MetadataCleanupConfig datasource() + { + return datasource; + } + + public MetadataCleanupConfig rules() + { + return rules; + } + + public MetadataCleanupConfig compaction() + { + return compaction; + } + + public MetadataCleanupConfig pendingSegments() + { + return pendingSegments; + } + + public MetadataCleanupConfig supervisors() + { + return supervisors; + } + + public KillUnusedSegmentsConfig unusedSegments() + { + return unusedSegments; + } +} diff --git a/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorPeriodConfig.java b/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorPeriodConfig.java new file mode 100644 index 000000000000..5deba3da3a0e --- /dev/null +++ b/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorPeriodConfig.java @@ -0,0 +1,57 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.server.coordinator.config; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import org.apache.druid.common.config.Configs; +import org.joda.time.Duration; + +public class CoordinatorPeriodConfig +{ + @JsonProperty + private final Duration metadataStoreManagementPeriod; + + @JsonProperty + private final Duration indexingPeriod; + + @JsonCreator + public CoordinatorPeriodConfig( + @JsonProperty("metadataStoreManagementPeriod") Duration metadataStoreManagementPeriod, + @JsonProperty("indexingPeriod") Duration indexingPeriod + ) + { + this.indexingPeriod = Configs.valueOrDefault(indexingPeriod, Duration.standardMinutes(30)); + this.metadataStoreManagementPeriod = Configs.valueOrDefault( + metadataStoreManagementPeriod, + Duration.standardMinutes(60) + ); + } + + public Duration getMetadataStoreManagementPeriod() + { + return metadataStoreManagementPeriod; + } + + public Duration getIndexingPeriod() + { + return indexingPeriod; + } +} diff --git a/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorRunConfig.java b/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorRunConfig.java new file mode 100644 index 000000000000..b35527e71acc --- /dev/null +++ b/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorRunConfig.java @@ -0,0 +1,54 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.server.coordinator.config; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import org.apache.druid.common.config.Configs; +import org.joda.time.Duration; + +public class CoordinatorRunConfig +{ + @JsonProperty + private final Duration startDelay; + + @JsonProperty + private final Duration period; + + @JsonCreator + public CoordinatorRunConfig( + @JsonProperty("startDelay") Duration startDelay, + @JsonProperty("period") Duration period + ) + { + this.startDelay = Configs.valueOrDefault(startDelay, Duration.standardMinutes(5)); + this.period = Configs.valueOrDefault(period, Duration.standardMinutes(1)); + } + + public Duration getPeriod() + { + return period; + } + + public Duration getStartDelay() + { + return startDelay; + } +} diff --git a/server/src/main/java/org/apache/druid/server/coordinator/config/HttpLoadQueuePeonConfig.java b/server/src/main/java/org/apache/druid/server/coordinator/config/HttpLoadQueuePeonConfig.java new file mode 100644 index 000000000000..06f26d8f44a4 --- /dev/null +++ b/server/src/main/java/org/apache/druid/server/coordinator/config/HttpLoadQueuePeonConfig.java @@ -0,0 +1,71 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.server.coordinator.config; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import org.apache.druid.common.config.Configs; +import org.joda.time.Duration; + +public class HttpLoadQueuePeonConfig +{ + public static final Duration DEFAULT_LOAD_TIMEOUT = Duration.standardMinutes(15); + + @JsonProperty + private final Duration hostTimeout; + + @JsonProperty + private final Duration repeatDelay; + + @JsonProperty + private final int batchSize; + + @JsonCreator + public HttpLoadQueuePeonConfig( + @JsonProperty("hostTimeout") Duration hostTimeout, + @JsonProperty("repeatDelay") Duration repeatDelay, + @JsonProperty("batchSize") Integer batchSize + ) + { + this.hostTimeout = Configs.valueOrDefault(hostTimeout, Duration.standardMinutes(5)); + this.repeatDelay = Configs.valueOrDefault(repeatDelay, Duration.standardMinutes(1)); + this.batchSize = Configs.valueOrDefault(batchSize, 1); + } + + public int getBatchSize() + { + return batchSize; + } + + public Duration getHostTimeout() + { + return hostTimeout; + } + + public Duration getRepeatDelay() + { + return repeatDelay; + } + + public Duration getLoadTimeout() + { + return DEFAULT_LOAD_TIMEOUT; + } +} diff --git a/server/src/main/java/org/apache/druid/server/coordinator/config/KillUnusedSegmentsConfig.java b/server/src/main/java/org/apache/druid/server/coordinator/config/KillUnusedSegmentsConfig.java new file mode 100644 index 000000000000..9a825e669ff4 --- /dev/null +++ b/server/src/main/java/org/apache/druid/server/coordinator/config/KillUnusedSegmentsConfig.java @@ -0,0 +1,69 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.server.coordinator.config; + +import org.apache.druid.common.config.Configs; +import org.joda.time.Duration; + +public class KillUnusedSegmentsConfig extends MetadataCleanupConfig +{ + public static KillUnusedSegmentsConfig STANDARD + = new KillUnusedSegmentsConfig(null, null, null, null, null, null); + + private final int maxSegments; + + private final boolean ignoreDurationToRetain; + + private final Duration bufferPeriod; + + public KillUnusedSegmentsConfig( + Boolean cleanupEnabled, + Duration cleanupPeriod, + Duration durationToRetain, + Boolean ignoreDurationToRetain, + Duration bufferPeriod, + Integer maxSegments + ) + { + super( + Configs.valueOrDefault(cleanupEnabled, false), + cleanupPeriod, + durationToRetain + ); + this.ignoreDurationToRetain = Configs.valueOrDefault(ignoreDurationToRetain, false); + this.bufferPeriod = Configs.valueOrDefault(bufferPeriod, Duration.standardDays(30)); + this.maxSegments = Configs.valueOrDefault(maxSegments, 100); + } + + public int getMaxSegments() + { + return maxSegments; + } + + public Duration getBufferPeriod() + { + return bufferPeriod; + } + + public boolean isIgnoreDurationToRetain() + { + return ignoreDurationToRetain; + } +} diff --git a/server/src/main/java/org/apache/druid/server/coordinator/config/MetadataCleanupConfig.java b/server/src/main/java/org/apache/druid/server/coordinator/config/MetadataCleanupConfig.java new file mode 100644 index 000000000000..0793474b0751 --- /dev/null +++ b/server/src/main/java/org/apache/druid/server/coordinator/config/MetadataCleanupConfig.java @@ -0,0 +1,89 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.server.coordinator.config; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import org.apache.druid.common.config.Configs; +import org.joda.time.Duration; + +import java.util.Objects; + +public class MetadataCleanupConfig +{ + public static MetadataCleanupConfig STANDARD = new MetadataCleanupConfig(null, null, null); + + @JsonProperty("on") + private final boolean cleanupEnabled; + + @JsonProperty("period") + private final Duration cleanupPeriod; + + @JsonProperty("durationToRetain") + private final Duration durationToRetain; + + @JsonCreator + public MetadataCleanupConfig( + @JsonProperty("on") Boolean cleanupEnabled, + @JsonProperty("period") Duration cleanupPeriod, + @JsonProperty("durationToRetain") Duration durationToRetain + ) + { + this.cleanupEnabled = Configs.valueOrDefault(cleanupEnabled, true); + this.cleanupPeriod = Configs.valueOrDefault(cleanupPeriod, Duration.standardDays(1)); + this.durationToRetain = Configs.valueOrDefault(durationToRetain, Duration.standardDays(90)); + } + + public Duration getCleanupPeriod() + { + return cleanupPeriod; + } + + public Duration getDurationToRetain() + { + return durationToRetain; + } + + public boolean isCleanupEnabled() + { + return cleanupEnabled; + } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + MetadataCleanupConfig that = (MetadataCleanupConfig) o; + return cleanupEnabled == that.cleanupEnabled + && Objects.equals(cleanupPeriod, that.cleanupPeriod) + && Objects.equals(durationToRetain, that.durationToRetain); + } + + @Override + public int hashCode() + { + return Objects.hash(cleanupEnabled, cleanupPeriod, durationToRetain); + } +} diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/KillAuditLog.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/KillAuditLog.java index e5248d48d7ed..3d3e58edfa88 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/KillAuditLog.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/KillAuditLog.java @@ -20,7 +20,7 @@ package org.apache.druid.server.coordinator.duty; import org.apache.druid.audit.AuditManager; -import org.apache.druid.server.coordinator.DruidCoordinatorConfig; +import org.apache.druid.server.coordinator.config.MetadataCleanupConfig; import org.apache.druid.server.coordinator.stats.Stats; import org.joda.time.DateTime; @@ -28,17 +28,9 @@ public class KillAuditLog extends MetadataCleanupDuty { private final AuditManager auditManager; - public KillAuditLog(DruidCoordinatorConfig config, AuditManager auditManager) + public KillAuditLog(MetadataCleanupConfig config, AuditManager auditManager) { - super( - "audit logs", - "druid.coordinator.kill.audit", - config.isAuditKillEnabled(), - config.getCoordinatorAuditKillPeriod(), - config.getCoordinatorAuditKillDurationToRetain(), - Stats.Kill.AUDIT_LOGS, - config - ); + super("audit logs", config, Stats.Kill.AUDIT_LOGS); this.auditManager = auditManager; } diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/KillCompactionConfig.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/KillCompactionConfig.java index 304b54e7cf84..c231c31c5436 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/KillCompactionConfig.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/KillCompactionConfig.java @@ -29,10 +29,9 @@ import org.apache.druid.server.coordinator.CoordinatorCompactionConfig; import org.apache.druid.server.coordinator.CoordinatorConfigManager; import org.apache.druid.server.coordinator.DataSourceCompactionConfig; -import org.apache.druid.server.coordinator.DruidCoordinatorConfig; +import org.apache.druid.server.coordinator.config.MetadataCleanupConfig; import org.apache.druid.server.coordinator.stats.Stats; import org.joda.time.DateTime; -import org.joda.time.Duration; import java.util.Map; import java.util.Set; @@ -54,20 +53,12 @@ public class KillCompactionConfig extends MetadataCleanupDuty private final CoordinatorConfigManager configManager; public KillCompactionConfig( - DruidCoordinatorConfig config, + MetadataCleanupConfig config, SegmentsMetadataManager sqlSegmentsMetadataManager, CoordinatorConfigManager configManager ) { - super( - "compaction configs", - "druid.coordinator.kill.compaction", - config.isCompactionKillEnabled(), - config.getCoordinatorCompactionKillPeriod(), - Duration.millis(1), // Retain duration is ignored - Stats.Kill.COMPACTION_CONFIGS, - config - ); + super("compaction configs", config, Stats.Kill.COMPACTION_CONFIGS); this.sqlSegmentsMetadataManager = sqlSegmentsMetadataManager; this.configManager = configManager; } diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/KillDatasourceMetadata.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/KillDatasourceMetadata.java index 9f2d15e1d33b..8e31e503d49f 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/KillDatasourceMetadata.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/KillDatasourceMetadata.java @@ -23,7 +23,7 @@ import org.apache.druid.indexing.overlord.IndexerMetadataStorageCoordinator; import org.apache.druid.indexing.overlord.supervisor.SupervisorSpec; import org.apache.druid.metadata.MetadataSupervisorManager; -import org.apache.druid.server.coordinator.DruidCoordinatorConfig; +import org.apache.druid.server.coordinator.config.MetadataCleanupConfig; import org.apache.druid.server.coordinator.stats.Stats; import org.joda.time.DateTime; @@ -44,20 +44,12 @@ public class KillDatasourceMetadata extends MetadataCleanupDuty private final MetadataSupervisorManager metadataSupervisorManager; public KillDatasourceMetadata( - DruidCoordinatorConfig config, + MetadataCleanupConfig config, IndexerMetadataStorageCoordinator indexerMetadataStorageCoordinator, MetadataSupervisorManager metadataSupervisorManager ) { - super( - "datasources", - "druid.coordinator.kill.datasource", - config.isDatasourceKillEnabled(), - config.getCoordinatorDatasourceKillPeriod(), - config.getCoordinatorDatasourceKillDurationToRetain(), - Stats.Kill.DATASOURCES, - config - ); + super("datasources", config, Stats.Kill.DATASOURCES); this.indexerMetadataStorageCoordinator = indexerMetadataStorageCoordinator; this.metadataSupervisorManager = metadataSupervisorManager; } diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/KillRules.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/KillRules.java index 6b4693bdac9b..a2c56a6e17b2 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/KillRules.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/KillRules.java @@ -20,7 +20,7 @@ package org.apache.druid.server.coordinator.duty; import org.apache.druid.metadata.MetadataRuleManager; -import org.apache.druid.server.coordinator.DruidCoordinatorConfig; +import org.apache.druid.server.coordinator.config.MetadataCleanupConfig; import org.apache.druid.server.coordinator.stats.Stats; import org.joda.time.DateTime; @@ -29,19 +29,11 @@ public class KillRules extends MetadataCleanupDuty private final MetadataRuleManager metadataRuleManager; public KillRules( - DruidCoordinatorConfig config, + MetadataCleanupConfig config, MetadataRuleManager metadataRuleManager ) { - super( - "rules", - "druid.coordinator.kill.rule", - config.isRuleKillEnabled(), - config.getCoordinatorRuleKillPeriod(), - config.getCoordinatorRuleKillDurationToRetain(), - Stats.Kill.RULES, - config - ); + super("rules", config, Stats.Kill.RULES); this.metadataRuleManager = metadataRuleManager; } diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/KillSupervisors.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/KillSupervisors.java index 8a41e4b8f20f..7f9065f7062e 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/KillSupervisors.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/KillSupervisors.java @@ -20,7 +20,7 @@ package org.apache.druid.server.coordinator.duty; import org.apache.druid.metadata.MetadataSupervisorManager; -import org.apache.druid.server.coordinator.DruidCoordinatorConfig; +import org.apache.druid.server.coordinator.config.MetadataCleanupConfig; import org.apache.druid.server.coordinator.stats.Stats; import org.joda.time.DateTime; @@ -32,19 +32,11 @@ public class KillSupervisors extends MetadataCleanupDuty private final MetadataSupervisorManager metadataSupervisorManager; public KillSupervisors( - DruidCoordinatorConfig config, + MetadataCleanupConfig config, MetadataSupervisorManager metadataSupervisorManager ) { - super( - "supervisors", - "druid.coordinator.kill.supervisor", - config.isSupervisorKillEnabled(), - config.getCoordinatorSupervisorKillPeriod(), - config.getCoordinatorSupervisorKillDurationToRetain(), - Stats.Kill.SUPERVISOR_SPECS, - config - ); + super("supervisors", config, Stats.Kill.SUPERVISOR_SPECS); this.metadataSupervisorManager = metadataSupervisorManager; } diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/KillSupervisorsCustomDuty.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/KillSupervisorsCustomDuty.java index 9cdf80107189..9080de517b3a 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/KillSupervisorsCustomDuty.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/KillSupervisorsCustomDuty.java @@ -25,7 +25,7 @@ import org.apache.druid.guice.annotations.UnstableApi; import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.metadata.MetadataSupervisorManager; -import org.apache.druid.server.coordinator.DruidCoordinatorConfig; +import org.apache.druid.server.coordinator.config.MetadataCleanupConfig; import org.apache.druid.server.coordinator.stats.Stats; import org.joda.time.DateTime; import org.joda.time.Duration; @@ -38,9 +38,6 @@ *

* This duty is only an example to demostrate the usage of coordinator custom * duties. All production clusters should continue using {@link KillSupervisors}. - *

- * In the future, we might migrate all metadata management coordinator duties to - * {@link CoordinatorCustomDuty} but until then this class will remain undocumented. */ @UnstableApi public class KillSupervisorsCustomDuty extends MetadataCleanupDuty implements CoordinatorCustomDuty @@ -52,20 +49,14 @@ public class KillSupervisorsCustomDuty extends MetadataCleanupDuty implements Co @JsonCreator public KillSupervisorsCustomDuty( @JsonProperty("durationToRetain") Duration retainDuration, - @JacksonInject MetadataSupervisorManager metadataSupervisorManager, - @JacksonInject DruidCoordinatorConfig coordinatorConfig + @JacksonInject MetadataSupervisorManager metadataSupervisorManager ) { super( "supervisors", - "KillSupervisorsCustomDuty", - true, - // Use the same period as metadata store management so that validation passes - // Actual period of custom duties is configured by the user - coordinatorConfig.getCoordinatorMetadataStoreManagementPeriod(), - retainDuration, - Stats.Kill.SUPERVISOR_SPECS, - coordinatorConfig + // Pass null here, actual period of custom duties is configured separately + new MetadataCleanupConfig(true, null, retainDuration), + Stats.Kill.SUPERVISOR_SPECS ); this.metadataSupervisorManager = metadataSupervisorManager; log.warn("This is only an example implementation of a custom duty and" diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/KillUnusedSegments.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/KillUnusedSegments.java index da7847f45053..a22d68148e72 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/KillUnusedSegments.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/KillUnusedSegments.java @@ -31,8 +31,8 @@ import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.metadata.SegmentsMetadataManager; import org.apache.druid.rpc.indexing.OverlordClient; -import org.apache.druid.server.coordinator.DruidCoordinatorConfig; import org.apache.druid.server.coordinator.DruidCoordinatorRuntimeParams; +import org.apache.druid.server.coordinator.config.KillUnusedSegmentsConfig; import org.apache.druid.server.coordinator.stats.CoordinatorRunStats; import org.apache.druid.server.coordinator.stats.Stats; import org.apache.druid.utils.CollectionUtils; @@ -82,17 +82,13 @@ public class KillUnusedSegments implements CoordinatorDuty public KillUnusedSegments( SegmentsMetadataManager segmentsMetadataManager, OverlordClient overlordClient, - DruidCoordinatorConfig config + KillUnusedSegmentsConfig killConfig ) { - this.period = config.getCoordinatorKillPeriod().getMillis(); - Preconditions.checkArgument( - this.period >= config.getCoordinatorIndexingPeriod().getMillis(), - "coordinator kill period must be greater than or equal to druid.coordinator.period.indexingPeriod" - ); + this.period = killConfig.getCleanupPeriod().getMillis(); - this.ignoreRetainDuration = config.getCoordinatorKillIgnoreDurationToRetain(); - this.retainDuration = config.getCoordinatorKillDurationToRetain().getMillis(); + this.ignoreRetainDuration = killConfig.isIgnoreDurationToRetain(); + this.retainDuration = killConfig.getDurationToRetain().getMillis(); if (this.ignoreRetainDuration) { log.debug( "druid.coordinator.kill.durationToRetain [%s] will be ignored when discovering segments to kill " @@ -100,9 +96,9 @@ public KillUnusedSegments( this.retainDuration ); } - this.bufferPeriod = config.getCoordinatorKillBufferPeriod().getMillis(); + this.bufferPeriod = killConfig.getBufferPeriod().getMillis(); - this.maxSegmentsToKill = config.getCoordinatorKillMaxSegments(); + this.maxSegmentsToKill = killConfig.getMaxSegments(); Preconditions.checkArgument(this.maxSegmentsToKill > 0, "coordinator kill maxSegments must be > 0"); datasourceToLastKillIntervalEnd = new ConcurrentHashMap<>(); diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/MetadataCleanupDuty.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/MetadataCleanupDuty.java index c3509cf21de0..111e1a6f5a7e 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/MetadataCleanupDuty.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/MetadataCleanupDuty.java @@ -19,21 +19,19 @@ package org.apache.druid.server.coordinator.duty; -import com.google.common.base.Preconditions; import org.apache.druid.java.util.common.DateTimes; import org.apache.druid.java.util.common.logger.Logger; -import org.apache.druid.server.coordinator.DruidCoordinatorConfig; import org.apache.druid.server.coordinator.DruidCoordinatorRuntimeParams; +import org.apache.druid.server.coordinator.config.MetadataCleanupConfig; import org.apache.druid.server.coordinator.stats.CoordinatorStat; import org.joda.time.DateTime; -import org.joda.time.Duration; import javax.annotation.Nullable; /** * Performs cleanup of stale metadata entries created before a configured retain duration. *

- * In every invocation of {@link #run}, the duty checks if the {@link #cleanupPeriod} + * In every invocation of {@link #run}, the duty checks if the {@code cleanupPeriod} * has elapsed since the {@link #lastCleanupTime}. If it has, then the method * {@link #cleanupEntriesCreatedBefore(DateTime)} is invoked. Otherwise, the duty * completes immediately without making any changes. @@ -42,40 +40,27 @@ public abstract class MetadataCleanupDuty implements CoordinatorDuty { private static final Logger log = new Logger(MetadataCleanupDuty.class); - private final String propertyPrefix; private final String entryType; private final CoordinatorStat cleanupCountStat; - private final boolean cleanupEnabled; - private final Duration cleanupPeriod; - private final Duration retainDuration; + private final MetadataCleanupConfig cleanupConfig; private DateTime lastCleanupTime = DateTimes.utc(0); protected MetadataCleanupDuty( String entryType, - String propertyPrefix, - boolean cleanupEnabled, - Duration cleanupPeriod, - Duration retainDuration, - CoordinatorStat cleanupCountStat, - DruidCoordinatorConfig coordinatorConfig + MetadataCleanupConfig cleanupConfig, + CoordinatorStat cleanupCountStat ) { - this.propertyPrefix = propertyPrefix; this.entryType = entryType; - this.cleanupEnabled = cleanupEnabled; - this.cleanupPeriod = cleanupPeriod; - this.retainDuration = retainDuration; + this.cleanupConfig = cleanupConfig; this.cleanupCountStat = cleanupCountStat; - if (this.cleanupEnabled) { - validatePeriod(cleanupPeriod, coordinatorConfig.getCoordinatorMetadataStoreManagementPeriod()); - validateRetainDuration(retainDuration); - + if (cleanupConfig.isCleanupEnabled()) { log.debug( "Enabled cleanup of [%s] with period [%s] and durationToRetain [%s].", - entryType, cleanupPeriod, retainDuration + entryType, cleanupConfig.getCleanupPeriod(), cleanupConfig.getDurationToRetain() ); } } @@ -84,18 +69,18 @@ protected MetadataCleanupDuty( @Override public DruidCoordinatorRuntimeParams run(DruidCoordinatorRuntimeParams params) { - if (!cleanupEnabled) { + if (!cleanupConfig.isCleanupEnabled()) { return params; } final DateTime now = DateTimes.nowUtc(); // Perform cleanup only if cleanup period has elapsed - if (lastCleanupTime.plus(cleanupPeriod).isBefore(now)) { + if (lastCleanupTime.plus(cleanupConfig.getCleanupPeriod()).isBefore(now)) { lastCleanupTime = now; try { - DateTime minCreatedTime = now.minus(retainDuration); + DateTime minCreatedTime = now.minus(cleanupConfig.getDurationToRetain()); int deletedEntries = cleanupEntriesCreatedBefore(minCreatedTime); log.info("Removed [%,d] [%s] created before [%s].", deletedEntries, entryType, minCreatedTime); @@ -112,33 +97,11 @@ public DruidCoordinatorRuntimeParams run(DruidCoordinatorRuntimeParams params) /** * Cleans up metadata entries created before the {@code minCreatedTime}. *

- * This method is not invoked if the {@link #cleanupPeriod} has not elapsed + * This method is not invoked if the {@code cleanupPeriod} has not elapsed * since the {@link #lastCleanupTime}. * * @return Number of deleted metadata entries */ protected abstract int cleanupEntriesCreatedBefore(DateTime minCreatedTime); - private void validatePeriod(Duration period, Duration metadataManagementPeriod) - { - Preconditions.checkArgument( - period != null && period.getMillis() >= metadataManagementPeriod.getMillis(), - "[%s.period] must be greater than [druid.coordinator.period.metadataStoreManagementPeriod]", - propertyPrefix - ); - } - - private void validateRetainDuration(Duration retainDuration) - { - Preconditions.checkArgument( - retainDuration != null && retainDuration.getMillis() >= 0, - "[%s.durationToRetain] must be 0 milliseconds or higher", - propertyPrefix - ); - Preconditions.checkArgument( - retainDuration.getMillis() < System.currentTimeMillis(), - "[%s.durationToRetain] cannot be greater than current time in milliseconds", - propertyPrefix - ); - } } diff --git a/server/src/main/java/org/apache/druid/server/coordinator/loading/CuratorLoadQueuePeon.java b/server/src/main/java/org/apache/druid/server/coordinator/loading/CuratorLoadQueuePeon.java index 2303be26d8b5..70996ccaae82 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/loading/CuratorLoadQueuePeon.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/loading/CuratorLoadQueuePeon.java @@ -28,7 +28,7 @@ import org.apache.druid.java.util.emitter.EmittingLogger; import org.apache.druid.server.coordination.SegmentChangeRequestNoop; import org.apache.druid.server.coordinator.DruidCoordinator; -import org.apache.druid.server.coordinator.DruidCoordinatorConfig; +import org.apache.druid.server.coordinator.config.HttpLoadQueuePeonConfig; import org.apache.druid.server.coordinator.stats.CoordinatorRunStats; import org.apache.druid.server.coordinator.stats.Stats; import org.apache.druid.timeline.DataSegment; @@ -72,7 +72,6 @@ public class CuratorLoadQueuePeon implements LoadQueuePeon * with loading or dropping segments. */ private final ExecutorService callBackExecutor; - private final DruidCoordinatorConfig config; private final AtomicLong queuedSize = new AtomicLong(0); private final CoordinatorRunStats stats = new CoordinatorRunStats(); @@ -115,8 +114,7 @@ public CuratorLoadQueuePeon( String basePath, ObjectMapper jsonMapper, ScheduledExecutorService processingExecutor, - ExecutorService callbackExecutor, - DruidCoordinatorConfig config + ExecutorService callbackExecutor ) { this.curator = curator; @@ -124,7 +122,6 @@ public CuratorLoadQueuePeon( this.jsonMapper = jsonMapper; this.callBackExecutor = callbackExecutor; this.processingExecutor = processingExecutor; - this.config = config; } @JsonProperty @@ -309,7 +306,7 @@ private ScheduledFuture scheduleNodeDeletedCheck(String path) failAssign(segmentHolder, false, e); } }, - config.getLoadTimeoutDelay().getMillis(), + HttpLoadQueuePeonConfig.DEFAULT_LOAD_TIMEOUT.getMillis(), TimeUnit.MILLISECONDS ); } diff --git a/server/src/main/java/org/apache/druid/server/coordinator/loading/HttpLoadQueuePeon.java b/server/src/main/java/org/apache/druid/server/coordinator/loading/HttpLoadQueuePeon.java index 92dd4714a454..ac01f0d29d62 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/loading/HttpLoadQueuePeon.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/loading/HttpLoadQueuePeon.java @@ -38,6 +38,7 @@ import org.apache.druid.server.coordination.SegmentLoadDropHandler; import org.apache.druid.server.coordinator.BytesAccumulatingResponseHandler; import org.apache.druid.server.coordinator.DruidCoordinatorConfig; +import org.apache.druid.server.coordinator.config.HttpLoadQueuePeonConfig; import org.apache.druid.server.coordinator.stats.CoordinatorRunStats; import org.apache.druid.server.coordinator.stats.CoordinatorStat; import org.apache.druid.server.coordinator.stats.Dimension; @@ -110,7 +111,7 @@ public class HttpLoadQueuePeon implements LoadQueuePeon private final Object lock = new Object(); - private final DruidCoordinatorConfig config; + private final HttpLoadQueuePeonConfig config; private final ObjectMapper jsonMapper; private final HttpClient httpClient; @@ -126,7 +127,7 @@ public HttpLoadQueuePeon( String baseUrl, ObjectMapper jsonMapper, HttpClient httpClient, - DruidCoordinatorConfig config, + HttpLoadQueuePeonConfig config, ScheduledExecutorService processingExecutor, ExecutorService callBackExecutor ) @@ -144,7 +145,7 @@ public HttpLoadQueuePeon( new URL(baseUrl), StringUtils.nonStrictFormat( "druid-internal/v1/segments/changeRequests?timeout=%d", - config.getHttpLoadQueuePeonHostTimeout().getMillis() + config.getHostTimeout().getMillis() ) ); } @@ -160,7 +161,7 @@ private void doSegmentManagement() return; } - final int batchSize = config.getHttpLoadQueuePeonBatchSize(); + final int batchSize = config.getBatchSize(); final List newRequests = new ArrayList<>(batchSize); @@ -194,7 +195,7 @@ private void doSegmentManagement() serverId, segmentsToLoad.size(), segmentsToDrop.size(), - config.getHttpLoadQueuePeonBatchSize() + config.getBatchSize() ); mainLoopInProgress.set(false); return; @@ -209,7 +210,7 @@ private void doSegmentManagement() .addHeader(HttpHeaders.Names.CONTENT_TYPE, MediaType.APPLICATION_JSON) .setContent(requestBodyWriter.writeValueAsBytes(newRequests)), responseHandler, - new Duration(config.getHttpLoadQueuePeonHostTimeout().getMillis() + 5000) + new Duration(config.getHostTimeout().getMillis() + 5000) ); Futures.addCallback( @@ -345,7 +346,7 @@ public void start() ScheduledExecutors.scheduleAtFixedRate( processingExecutor, - config.getHttpLoadQueuePeonRepeatDelay(), + config.getRepeatDelay(), () -> { if (!stopped) { doSegmentManagement(); @@ -516,7 +517,7 @@ private boolean hasRequestTimedOut(SegmentHolder holder, long currentTimeMillis) { return holder.isRequestSentToServer() && currentTimeMillis - holder.getFirstRequestMillis() - > config.getLoadTimeoutDelay().getMillis(); + > config.getLoadTimeout().getMillis(); } private void onRequestFailed(SegmentHolder holder, String failureCause) diff --git a/server/src/main/java/org/apache/druid/server/coordinator/loading/LoadQueueTaskMaster.java b/server/src/main/java/org/apache/druid/server/coordinator/loading/LoadQueueTaskMaster.java index 7a97a61fa313..647c9e0c4132 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/loading/LoadQueueTaskMaster.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/loading/LoadQueueTaskMaster.java @@ -22,14 +22,10 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.Sets; import com.google.errorprone.annotations.concurrent.GuardedBy; -import com.google.inject.Provider; -import org.apache.curator.framework.CuratorFramework; -import org.apache.curator.utils.ZKPaths; import org.apache.druid.client.ImmutableDruidServer; import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.java.util.http.client.HttpClient; -import org.apache.druid.server.coordinator.DruidCoordinatorConfig; -import org.apache.druid.server.initialization.ZkPathsConfig; +import org.apache.druid.server.coordinator.config.HttpLoadQueuePeonConfig; import java.util.List; import java.util.Map; @@ -46,14 +42,11 @@ public class LoadQueueTaskMaster { private static final Logger log = new Logger(LoadQueueTaskMaster.class); - private final Provider curatorFrameworkProvider; private final ObjectMapper jsonMapper; private final ScheduledExecutorService peonExec; private final ExecutorService callbackExec; - private final DruidCoordinatorConfig config; + private final HttpLoadQueuePeonConfig config; private final HttpClient httpClient; - private final ZkPathsConfig zkPaths; - private final boolean httpLoading; @GuardedBy("this") private final AtomicBoolean isLeader = new AtomicBoolean(false); @@ -61,39 +54,23 @@ public class LoadQueueTaskMaster private final ConcurrentHashMap loadManagementPeons = new ConcurrentHashMap<>(); public LoadQueueTaskMaster( - Provider curatorFrameworkProvider, ObjectMapper jsonMapper, ScheduledExecutorService peonExec, ExecutorService callbackExec, - DruidCoordinatorConfig config, - HttpClient httpClient, - ZkPathsConfig zkPaths + HttpLoadQueuePeonConfig config, + HttpClient httpClient ) { - this.curatorFrameworkProvider = curatorFrameworkProvider; this.jsonMapper = jsonMapper; this.peonExec = peonExec; this.callbackExec = callbackExec; this.config = config; this.httpClient = httpClient; - this.zkPaths = zkPaths; - this.httpLoading = "http".equalsIgnoreCase(config.getLoadQueuePeonType()); } private LoadQueuePeon createPeon(ImmutableDruidServer server) { - if (httpLoading) { - return new HttpLoadQueuePeon(server.getURL(), jsonMapper, httpClient, config, peonExec, callbackExec); - } else { - return new CuratorLoadQueuePeon( - curatorFrameworkProvider.get(), - ZKPaths.makePath(zkPaths.getLoadQueuePath(), server.getName()), - jsonMapper, - peonExec, - callbackExec, - config - ); - } + return new HttpLoadQueuePeon(server.getURL(), jsonMapper, httpClient, config, peonExec, callbackExec); } public Map getAllPeons() @@ -161,6 +138,6 @@ public synchronized void onLeaderStop() public boolean isHttpLoading() { - return httpLoading; + return true; } } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/CuratorDruidCoordinatorTest.java b/server/src/test/java/org/apache/druid/server/coordinator/CuratorDruidCoordinatorTest.java index 2908f7e009f0..26cccff59117 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/CuratorDruidCoordinatorTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/CuratorDruidCoordinatorTest.java @@ -58,7 +58,6 @@ import org.apache.druid.timeline.SegmentId; import org.apache.druid.timeline.partition.NoneShardSpec; import org.easymock.EasyMock; -import org.joda.time.Duration; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -131,13 +130,6 @@ public void setUp() throws Exception curator.create().creatingParentsIfNeeded().forPath(DESTINATION_LOAD_PATH); final ObjectMapper objectMapper = new DefaultObjectMapper(); - DruidCoordinatorConfig druidCoordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withCoordinatorStartDelay(new Duration(COORDINATOR_START_DELAY)) - .withCoordinatorPeriod(new Duration(COORDINATOR_PERIOD)) - .withCoordinatorKillPeriod(new Duration(COORDINATOR_PERIOD)) - .withCoordinatorKillMaxSegments(10) - .withCoordinatorKillIgnoreDurationToRetain(false) - .build(); sourceLoadQueueChildrenCache = new PathChildrenCache( curator, SOURCE_LOAD_PATH, @@ -157,16 +149,14 @@ public void setUp() throws Exception SOURCE_LOAD_PATH, objectMapper, peonExec, - callbackExec, - druidCoordinatorConfig + callbackExec ); destinationLoadQueuePeon = new CuratorLoadQueuePeon( curator, DESTINATION_LOAD_PATH, objectMapper, peonExec, - callbackExec, - druidCoordinatorConfig + callbackExec ); } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java b/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java index 53c2808aa723..7c0d1a0226e5 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java @@ -19,67 +19,429 @@ package org.apache.druid.server.coordinator; -import org.apache.druid.java.util.common.config.Config; +import com.google.common.collect.ImmutableList; +import com.google.inject.Injector; +import org.apache.druid.guice.GuiceInjectors; +import org.apache.druid.guice.JsonConfigProvider; +import org.apache.druid.guice.JsonConfigurator; +import org.apache.druid.server.coordinator.config.CoordinatorKillConfigs; +import org.apache.druid.server.coordinator.config.CoordinatorPeriodConfig; +import org.apache.druid.server.coordinator.config.CoordinatorRunConfig; +import org.apache.druid.server.coordinator.config.HttpLoadQueuePeonConfig; +import org.apache.druid.server.coordinator.config.KillUnusedSegmentsConfig; +import org.apache.druid.server.coordinator.config.MetadataCleanupConfig; import org.joda.time.Duration; +import org.joda.time.Period; import org.junit.Assert; import org.junit.Test; -import org.skife.config.ConfigurationObjectFactory; import java.util.Properties; /** + * */ public class DruidCoordinatorConfigTest { + + @Test + public void testCoordinatorRunConfigDefaultValues() + { + final Properties props = new Properties(); + final CoordinatorRunConfig config = deserializeFrom(props, "druid.coordinator", CoordinatorRunConfig.class); + + Assert.assertEquals(Duration.standardMinutes(1), config.getPeriod()); + Assert.assertEquals(Duration.standardMinutes(5), config.getStartDelay()); + } + + @Test + public void testCoordinatorRunConfigOverrideValues() + { + final Properties props = new Properties(); + props.setProperty("druid.coordinator.startDelay", "PT10M"); + props.setProperty("druid.coordinator.period", "PT30S"); + final CoordinatorRunConfig config = deserializeFrom(props, "druid.coordinator", CoordinatorRunConfig.class); + + Assert.assertEquals(Duration.standardSeconds(30), config.getPeriod()); + Assert.assertEquals(Duration.standardMinutes(10), config.getStartDelay()); + } + + @Test + public void testCoordinatorPeriodConfigDefaultValues() + { + final Properties props = new Properties(); + final CoordinatorPeriodConfig config + = deserializeFrom(props, "druid.coordinator.period", CoordinatorPeriodConfig.class); + + Assert.assertEquals(Duration.standardMinutes(30), config.getIndexingPeriod()); + Assert.assertEquals(Duration.standardMinutes(60), config.getMetadataStoreManagementPeriod()); + } + + @Test + public void testCoordinatorPeriodConfigOverrideValues() + { + final Properties props = new Properties(); + props.setProperty("druid.coordinator.period.indexingPeriod", "PT1M"); + props.setProperty("druid.coordinator.period.metadataStoreManagementPeriod", "PT3M"); + final CoordinatorPeriodConfig config + = deserializeFrom(props, "druid.coordinator.period", CoordinatorPeriodConfig.class); + + Assert.assertEquals(Duration.standardMinutes(1), config.getIndexingPeriod()); + Assert.assertEquals(Duration.standardMinutes(3), config.getMetadataStoreManagementPeriod()); + } + + @Test + public void testLoadQueuePeonConfigDefaultValues() + { + final Properties props = new Properties(); + final HttpLoadQueuePeonConfig config + = deserializeFrom(props, "druid.coordinator.loadqueuepeon.http", HttpLoadQueuePeonConfig.class); + + Assert.assertEquals(Duration.standardMinutes(1), config.getRepeatDelay()); + Assert.assertEquals(Duration.standardMinutes(5), config.getHostTimeout()); + Assert.assertEquals(Duration.standardMinutes(15), config.getLoadTimeout()); + Assert.assertEquals(1, config.getBatchSize()); + } + + @Test + public void testLoadQueuePeonConfigOverrideValues() + { + final Properties props = new Properties(); + props.setProperty("druid.coordinator.loadqueuepeon.http.repeatDelay", "PT20M"); + props.setProperty("druid.coordinator.loadqueuepeon.http.hostTimeout", "PT10M"); + props.setProperty("druid.coordinator.loadqueuepeon.http.batchSize", "100"); + + final HttpLoadQueuePeonConfig config + = deserializeFrom(props, "druid.coordinator.loadqueuepeon.http", HttpLoadQueuePeonConfig.class); + + Assert.assertEquals(Duration.standardMinutes(20), config.getRepeatDelay()); + Assert.assertEquals(Duration.standardMinutes(10), config.getHostTimeout()); + Assert.assertEquals(Duration.standardMinutes(15), config.getLoadTimeout()); + Assert.assertEquals(100, config.getBatchSize()); + } + + @Test + public void testMetadataCleanupConfigDefaultValues() + { + final MetadataCleanupConfig config = new MetadataCleanupConfig(null, null, null); + + Assert.assertTrue(config.isCleanupEnabled()); + Assert.assertEquals(Duration.standardDays(1), config.getCleanupPeriod()); + Assert.assertEquals(Duration.standardDays(90), config.getDurationToRetain()); + } + + @Test + public void testMetadataCleanupConfigOverrideValues() + { + final MetadataCleanupConfig config = new MetadataCleanupConfig( + false, + Period.parse("PT5H").toStandardDuration(), + Period.parse("P1D").toStandardDuration() + ); + + Assert.assertFalse(config.isCleanupEnabled()); + Assert.assertEquals(Duration.standardHours(5), config.getCleanupPeriod()); + Assert.assertEquals(Duration.standardHours(24), config.getDurationToRetain()); + } + + @Test + public void testKillUnusedSegmentsConfigDefaultValues() + { + final KillUnusedSegmentsConfig config = new KillUnusedSegmentsConfig(null, null, null, null, null, null); + + Assert.assertFalse(config.isCleanupEnabled()); + Assert.assertFalse(config.isIgnoreDurationToRetain()); + Assert.assertEquals(Duration.standardDays(1), config.getCleanupPeriod()); + Assert.assertEquals(Duration.standardDays(30), config.getBufferPeriod()); + Assert.assertEquals(Duration.standardDays(90), config.getDurationToRetain()); + Assert.assertEquals(100, config.getMaxSegments()); + } + + @Test + public void testKillUnusedSegmentsConfigOverrideValues() + { + final KillUnusedSegmentsConfig config = new KillUnusedSegmentsConfig( + true, + Period.parse("PT30M").toStandardDuration(), + Period.parse("PT12H").toStandardDuration(), + true, + Period.parse("PT60M").toStandardDuration(), + 500 + ); + + Assert.assertTrue(config.isCleanupEnabled()); + Assert.assertTrue(config.isIgnoreDurationToRetain()); + Assert.assertEquals(Duration.standardMinutes(30), config.getCleanupPeriod()); + Assert.assertEquals(Duration.standardMinutes(60), config.getBufferPeriod()); + Assert.assertEquals(Duration.standardHours(12), config.getDurationToRetain()); + Assert.assertEquals(500, config.getMaxSegments()); + } + + @Test + public void testCoordinatorKillConfigDefaultValues() + { + final CoordinatorKillConfigs killConfigs + = deserializeFrom(new Properties(), "druid.coordinator.kill", CoordinatorKillConfigs.class); + + Assert.assertEquals(MetadataCleanupConfig.STANDARD, killConfigs.audit()); + Assert.assertEquals(MetadataCleanupConfig.STANDARD, killConfigs.supervisors()); + Assert.assertEquals(MetadataCleanupConfig.STANDARD, killConfigs.compaction()); + Assert.assertEquals(MetadataCleanupConfig.STANDARD, killConfigs.datasource()); + Assert.assertEquals(MetadataCleanupConfig.STANDARD, killConfigs.rules()); + Assert.assertEquals(MetadataCleanupConfig.STANDARD, killConfigs.pendingSegments()); + } + + @Test + public void testCoordinatorKillConfigOverrideValues() + { + final Properties props = new Properties(); + props.setProperty("druid.coordinator.kill.audit.on", "false"); + props.setProperty("druid.coordinator.kill.audit.period", "PT10H"); + props.setProperty("druid.coordinator.kill.audit.durationToRetain", "PT20H"); + + props.setProperty("druid.coordinator.kill.compaction.on", "false"); + props.setProperty("druid.coordinator.kill.compaction.period", "PT20H"); + props.setProperty("druid.coordinator.kill.compaction.durationToRetain", "PT30H"); + + props.setProperty("druid.coordinator.kill.datasource.on", "false"); + props.setProperty("druid.coordinator.kill.datasource.period", "PT5H"); + props.setProperty("druid.coordinator.kill.datasource.durationToRetain", "PT10H"); + + props.setProperty("druid.coordinator.kill.rule.on", "false"); + props.setProperty("druid.coordinator.kill.rule.period", "PT11H"); + props.setProperty("druid.coordinator.kill.rule.durationToRetain", "PT12H"); + + props.setProperty("druid.coordinator.kill.supervisor.on", "false"); + props.setProperty("druid.coordinator.kill.supervisor.period", "PT1H"); + props.setProperty("druid.coordinator.kill.supervisor.durationToRetain", "PT2H"); + + props.setProperty("druid.coordinator.kill.pendingSegments.on", "false"); + + final CoordinatorKillConfigs killConfigs + = deserializeFrom(props, "druid.coordinator.kill", CoordinatorKillConfigs.class); + + Assert.assertEquals( + new MetadataCleanupConfig(false, Duration.standardHours(10), Duration.standardHours(20)), + killConfigs.audit() + ); + Assert.assertEquals( + new MetadataCleanupConfig(false, Duration.standardHours(20), Duration.standardHours(30)), + killConfigs.compaction() + ); + Assert.assertEquals( + new MetadataCleanupConfig(false, Duration.standardHours(5), Duration.standardHours(10)), + killConfigs.datasource() + ); + Assert.assertEquals( + new MetadataCleanupConfig(false, Duration.standardHours(11), Duration.standardHours(12)), + killConfigs.rules() + ); + Assert.assertEquals( + new MetadataCleanupConfig(false, Duration.standardHours(1), Duration.standardHours(2)), + killConfigs.supervisors() + ); + Assert.assertFalse(killConfigs.pendingSegments().isCleanupEnabled()); + } + + @Test + public void testCoordinatorConfigFailsWhenCleanupPeriodIsInvalid() + { + // Validation fails when cleanup period is less than metadata store management period + final MetadataCleanupConfig cleanupConfig + = new MetadataCleanupConfig(true, Duration.standardMinutes(30), null); + final Duration metadataStoreManagementPeriod = Duration.standardHours(1); + + verifyCoordinatorConfigFailsWith( + killConfig().audit(cleanupConfig).build(), + metadataStoreManagementPeriod, + "[druid.coordinator.kill.audit.period] must be greater than" + + " [druid.coordinator.period.metadataStoreManagementPeriod]" + ); + verifyCoordinatorConfigFailsWith( + killConfig().compaction(cleanupConfig).build(), + metadataStoreManagementPeriod, + "[druid.coordinator.kill.compaction.period] must be greater than" + + " [druid.coordinator.period.metadataStoreManagementPeriod]" + ); + verifyCoordinatorConfigFailsWith( + killConfig().datasource(cleanupConfig).build(), + metadataStoreManagementPeriod, + "[druid.coordinator.kill.datasource.period] must be greater than" + + " [druid.coordinator.period.metadataStoreManagementPeriod]" + ); + verifyCoordinatorConfigFailsWith( + killConfig().rules(cleanupConfig).build(), + metadataStoreManagementPeriod, + "[druid.coordinator.kill.rule.period] must be greater than" + + " [druid.coordinator.period.metadataStoreManagementPeriod]" + ); + verifyCoordinatorConfigFailsWith( + killConfig().supervisors(cleanupConfig).build(), + metadataStoreManagementPeriod, + "[druid.coordinator.kill.supervisor.period] must be greater than" + + " [druid.coordinator.period.metadataStoreManagementPeriod]" + ); + } + + @Test + public void testCoordinatorConfigFailsWhenRetainDurationIsNegative() + { + final MetadataCleanupConfig cleanupConfig + = new MetadataCleanupConfig(true, null, Duration.standardDays(1).negated()); + verifyCoordinatorConfigFailsWith( + killConfig().audit(cleanupConfig).build(), + null, + "[druid.coordinator.kill.audit.durationToRetain] must be 0 milliseconds or higher" + ); + verifyCoordinatorConfigFailsWith( + killConfig().compaction(cleanupConfig).build(), + null, + "[druid.coordinator.kill.compaction.durationToRetain] must be 0 milliseconds or higher" + ); + verifyCoordinatorConfigFailsWith( + killConfig().datasource(cleanupConfig).build(), + null, + "[druid.coordinator.kill.datasource.durationToRetain] must be 0 milliseconds or higher" + ); + verifyCoordinatorConfigFailsWith( + killConfig().rules(cleanupConfig).build(), + null, + "[druid.coordinator.kill.rule.durationToRetain] must be 0 milliseconds or higher" + ); + verifyCoordinatorConfigFailsWith( + killConfig().supervisors(cleanupConfig).build(), + null, + "[druid.coordinator.kill.supervisor.durationToRetain] must be 0 milliseconds or higher" + ); + } + @Test - public void testDeserialization() - { - ConfigurationObjectFactory factory = Config.createFactory(new Properties()); - - //with defaults - DruidCoordinatorConfig config = factory.build(DruidCoordinatorConfig.class); - - Assert.assertEquals(new Duration("PT300s"), config.getCoordinatorStartDelay()); - Assert.assertEquals(new Duration("PT60s"), config.getCoordinatorPeriod()); - Assert.assertEquals(new Duration("PT1800s"), config.getCoordinatorIndexingPeriod()); - Assert.assertEquals(86400000, config.getCoordinatorKillPeriod().getMillis()); - Assert.assertEquals(7776000000L, config.getCoordinatorKillDurationToRetain().getMillis()); - Assert.assertEquals(100, config.getCoordinatorKillMaxSegments()); - Assert.assertEquals(new Duration(15 * 60 * 1000), config.getLoadTimeoutDelay()); - Assert.assertFalse(config.getCoordinatorKillIgnoreDurationToRetain()); - Assert.assertEquals("http", config.getLoadQueuePeonType()); - - //with non-defaults - Properties props = new Properties(); - props.setProperty("druid.coordinator.startDelay", "PT1s"); - props.setProperty("druid.coordinator.period", "PT1s"); - props.setProperty("druid.coordinator.period.indexingPeriod", "PT1s"); - props.setProperty("druid.coordinator.kill.on", "true"); - props.setProperty("druid.coordinator.kill.period", "PT1s"); - props.setProperty("druid.coordinator.kill.durationToRetain", "PT1s"); - props.setProperty("druid.coordinator.kill.maxSegments", "10000"); - props.setProperty("druid.coordinator.kill.pendingSegments.on", "true"); - props.setProperty("druid.coordinator.load.timeout", "PT1s"); - props.setProperty("druid.coordinator.loadqueuepeon.repeatDelay", "PT0.100s"); - props.setProperty("druid.coordinator.kill.ignoreDurationToRetain", "true"); - - factory = Config.createFactory(props); - config = factory.build(DruidCoordinatorConfig.class); - - Assert.assertEquals(new Duration("PT1s"), config.getCoordinatorStartDelay()); - Assert.assertEquals(new Duration("PT1s"), config.getCoordinatorPeriod()); - Assert.assertEquals(new Duration("PT1s"), config.getCoordinatorIndexingPeriod()); - Assert.assertEquals(new Duration("PT1s"), config.getCoordinatorKillPeriod()); - Assert.assertEquals(new Duration("PT1s"), config.getCoordinatorKillDurationToRetain()); - Assert.assertEquals(10000, config.getCoordinatorKillMaxSegments()); - Assert.assertEquals(new Duration("PT1s"), config.getLoadTimeoutDelay()); - Assert.assertTrue(config.getCoordinatorKillIgnoreDurationToRetain()); - - // Test negative druid.coordinator.kill.durationToRetain now that it is valid. - props = new Properties(); - props.setProperty("druid.coordinator.kill.durationToRetain", "PT-1s"); - factory = Config.createFactory(props); - config = factory.build(DruidCoordinatorConfig.class); - Assert.assertEquals(new Duration("PT-1s"), config.getCoordinatorKillDurationToRetain()); + public void testCoordinatorConfigFailsWhenRetainDurationIsHigherThanCurrentTime() + { + final Duration futureDuration = Duration.millis(System.currentTimeMillis()).plus(10_000); + final MetadataCleanupConfig cleanupConfig = new MetadataCleanupConfig(true, null, futureDuration); + verifyCoordinatorConfigFailsWith( + killConfig().audit(cleanupConfig).build(), + null, + "[druid.coordinator.kill.audit.durationToRetain] cannot be greater than current time in milliseconds" + ); + verifyCoordinatorConfigFailsWith( + killConfig().compaction(cleanupConfig).build(), + null, + "[druid.coordinator.kill.compaction.durationToRetain] cannot be greater than current time in milliseconds" + ); + verifyCoordinatorConfigFailsWith( + killConfig().datasource(cleanupConfig).build(), + null, + "[druid.coordinator.kill.datasource.durationToRetain] cannot be greater than current time in milliseconds" + ); + verifyCoordinatorConfigFailsWith( + killConfig().rules(cleanupConfig).build(), + null, + "[druid.coordinator.kill.rule.durationToRetain] cannot be greater than current time in milliseconds" + ); + verifyCoordinatorConfigFailsWith( + killConfig().supervisors(cleanupConfig).build(), + null, + "[druid.coordinator.kill.supervisor.durationToRetain] cannot be greater than current time in milliseconds" + ); + } + + private KillConfigBuilder killConfig() + { + return new KillConfigBuilder(); + } + + private void verifyCoordinatorConfigFailsWith( + CoordinatorKillConfigs killConfig, + Duration metadataStoreManagementPeriod, + String expectedMessage + ) + { + IllegalArgumentException exception = Assert.assertThrows( + IllegalArgumentException.class, + () -> new DruidCoordinatorConfig( + new CoordinatorRunConfig(null, null), + new CoordinatorPeriodConfig(metadataStoreManagementPeriod, null), + killConfig, + null, + null + ) + ); + Assert.assertEquals(expectedMessage, exception.getMessage()); + } + + private Injector createInjector(String propertyPrefix, Class clazz) + { + return GuiceInjectors.makeStartupInjectorWithModules( + ImmutableList.of( + binder -> JsonConfigProvider.bind(binder, propertyPrefix, clazz) + ) + ); + } + + private T deserializeFrom(Properties props, String propertyPrefix, Class type) + { + final Injector injector = createInjector(propertyPrefix, type); + final JsonConfigProvider provider = JsonConfigProvider.of(propertyPrefix, type); + provider.inject(props, injector.getInstance(JsonConfigurator.class)); + return provider.get(); + } + + private static class KillConfigBuilder + { + MetadataCleanupConfig audit; + MetadataCleanupConfig compaction; + MetadataCleanupConfig datasource; + MetadataCleanupConfig rules; + MetadataCleanupConfig supervisors; + MetadataCleanupConfig pendingSegments; + + KillConfigBuilder audit(MetadataCleanupConfig config) + { + this.audit = config; + return this; + } + + KillConfigBuilder compaction(MetadataCleanupConfig config) + { + this.compaction = config; + return this; + } + + KillConfigBuilder datasource(MetadataCleanupConfig config) + { + this.datasource = config; + return this; + } + + KillConfigBuilder rules(MetadataCleanupConfig config) + { + this.rules = config; + return this; + } + + KillConfigBuilder supervisors(MetadataCleanupConfig config) + { + this.supervisors = config; + return this; + } + + CoordinatorKillConfigs build() + { + return new CoordinatorKillConfigs( + pendingSegments, + supervisors, + audit, + datasource, + rules, + compaction, + null, + null, + null, + null, + null, + null + ); + } } } 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 a9e85ca1b23e..11767be2ae30 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 @@ -52,6 +52,9 @@ import org.apache.druid.server.coordination.ServerType; import org.apache.druid.server.coordinator.balancer.CostBalancerStrategyFactory; import org.apache.druid.server.coordinator.compact.NewestSegmentFirstPolicy; +import org.apache.druid.server.coordinator.config.CoordinatorKillConfigs; +import org.apache.druid.server.coordinator.config.CoordinatorPeriodConfig; +import org.apache.druid.server.coordinator.config.CoordinatorRunConfig; import org.apache.druid.server.coordinator.duty.CompactSegments; import org.apache.druid.server.coordinator.duty.CoordinatorCustomDuty; import org.apache.druid.server.coordinator.duty.CoordinatorCustomDutyGroup; @@ -141,10 +144,13 @@ public void setUp() throws Exception curator.create().creatingParentsIfNeeded().forPath(LOADPATH); objectMapper = new DefaultObjectMapper(); newestSegmentFirstPolicy = new NewestSegmentFirstPolicy(objectMapper); - druidCoordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withCoordinatorStartDelay(new Duration(COORDINATOR_START_DELAY)) - .withCoordinatorPeriod(new Duration(COORDINATOR_PERIOD)) - .build(); + druidCoordinatorConfig = new DruidCoordinatorConfig( + new CoordinatorRunConfig(new Duration(COORDINATOR_START_DELAY), new Duration(COORDINATOR_PERIOD)), + new CoordinatorPeriodConfig(null, null), + CoordinatorKillConfigs.STANDARD, + new CostBalancerStrategyFactory(), + null + ); pathChildrenCache = new PathChildrenCache( curator, LOADPATH, @@ -157,8 +163,7 @@ public void setUp() throws Exception LOADPATH, objectMapper, Execs.scheduledSingleThreaded("coordinator_test_load_queue_peon_scheduled-%d"), - Execs.singleThreaded("coordinator_test_load_queue_peon-%d"), - druidCoordinatorConfig + Execs.singleThreaded("coordinator_test_load_queue_peon-%d") ); loadQueuePeon.start(); druidNode = new DruidNode("hey", "what", false, 1234, null, true, false); @@ -177,7 +182,6 @@ public void setUp() throws Exception new LatchableServiceAnnouncer(leaderAnnouncerLatch, leaderUnannouncerLatch), druidNode, new CoordinatorCustomDutyGroups(ImmutableSet.of()), - new CostBalancerStrategyFactory(), EasyMock.createNiceMock(LookupCoordinatorManager.class), new TestDruidLeaderSelector(), null @@ -349,8 +353,7 @@ public void testCoordinatorTieredRun() throws Exception loadPathCold, objectMapper, Execs.scheduledSingleThreaded("coordinator_test_load_queue_peon_cold_scheduled-%d"), - Execs.singleThreaded("coordinator_test_load_queue_peon_cold-%d"), - druidCoordinatorConfig + Execs.singleThreaded("coordinator_test_load_queue_peon_cold-%d") ); final PathChildrenCache pathChildrenCacheCold = new PathChildrenCache( curator, @@ -448,8 +451,7 @@ public void testComputeUnderReplicationCountsPerDataSourcePerTierForSegmentsWith loadPathCold, objectMapper, Execs.scheduledSingleThreaded("coordinator_test_load_queue_peon_cold_scheduled-%d"), - Execs.singleThreaded("coordinator_test_load_queue_peon_cold-%d"), - druidCoordinatorConfig + Execs.singleThreaded("coordinator_test_load_queue_peon_cold-%d") ); final LoadQueuePeon loadQueuePeonBroker1 = new CuratorLoadQueuePeon( @@ -457,8 +459,7 @@ public void testComputeUnderReplicationCountsPerDataSourcePerTierForSegmentsWith loadPathBroker1, objectMapper, Execs.scheduledSingleThreaded("coordinator_test_load_queue_peon_broker1_scheduled-%d"), - Execs.singleThreaded("coordinator_test_load_queue_peon_broker1-%d"), - druidCoordinatorConfig + Execs.singleThreaded("coordinator_test_load_queue_peon_broker1-%d") ); final LoadQueuePeon loadQueuePeonBroker2 = new CuratorLoadQueuePeon( @@ -466,8 +467,7 @@ public void testComputeUnderReplicationCountsPerDataSourcePerTierForSegmentsWith loadPathBroker2, objectMapper, Execs.scheduledSingleThreaded("coordinator_test_load_queue_peon_broker2_scheduled-%d"), - Execs.singleThreaded("coordinator_test_load_queue_peon_broker2-%d"), - druidCoordinatorConfig + Execs.singleThreaded("coordinator_test_load_queue_peon_broker2-%d") ); final LoadQueuePeon loadQueuePeonPoenServer = new CuratorLoadQueuePeon( @@ -475,8 +475,7 @@ public void testComputeUnderReplicationCountsPerDataSourcePerTierForSegmentsWith loadPathPeon, objectMapper, Execs.scheduledSingleThreaded("coordinator_test_load_queue_peon_peon_scheduled-%d"), - Execs.singleThreaded("coordinator_test_load_queue_peon_peon-%d"), - druidCoordinatorConfig + Execs.singleThreaded("coordinator_test_load_queue_peon_peon-%d") ); final PathChildrenCache pathChildrenCacheCold = new PathChildrenCache( curator, @@ -603,7 +602,6 @@ public void testCompactSegmentsDutyWhenCustomDutyGroupEmpty() new LatchableServiceAnnouncer(leaderAnnouncerLatch, leaderUnannouncerLatch), druidNode, emptyCustomDutyGroups, - new CostBalancerStrategyFactory(), EasyMock.createNiceMock(LookupCoordinatorManager.class), new TestDruidLeaderSelector(), null @@ -627,7 +625,7 @@ public void testInitializeCompactSegmentsDutyWhenCustomDutyGroupDoesNotContainsC CoordinatorCustomDutyGroup group = new CoordinatorCustomDutyGroup( "group1", Duration.standardSeconds(1), - ImmutableList.of(new KillSupervisorsCustomDuty(new Duration("PT1S"), null, druidCoordinatorConfig)) + ImmutableList.of(new KillSupervisorsCustomDuty(new Duration("PT1S"), null)) ); CoordinatorCustomDutyGroups customDutyGroups = new CoordinatorCustomDutyGroups(ImmutableSet.of(group)); coordinator = new DruidCoordinator( @@ -642,7 +640,6 @@ public void testInitializeCompactSegmentsDutyWhenCustomDutyGroupDoesNotContainsC new LatchableServiceAnnouncer(leaderAnnouncerLatch, leaderUnannouncerLatch), druidNode, customDutyGroups, - new CostBalancerStrategyFactory(), EasyMock.createNiceMock(LookupCoordinatorManager.class), new TestDruidLeaderSelector(), null @@ -681,7 +678,6 @@ public void testInitializeCompactSegmentsDutyWhenCustomDutyGroupContainsCompactS new LatchableServiceAnnouncer(leaderAnnouncerLatch, leaderUnannouncerLatch), druidNode, customDutyGroups, - new CostBalancerStrategyFactory(), EasyMock.createNiceMock(LookupCoordinatorManager.class), new TestDruidLeaderSelector(), null @@ -785,7 +781,6 @@ public void testCoordinatorCustomDutyGroupsRunAsExpected() throws Exception new LatchableServiceAnnouncer(leaderAnnouncerLatch, leaderUnannouncerLatch), druidNode, groups, - new CostBalancerStrategyFactory(), EasyMock.createNiceMock(LookupCoordinatorManager.class), new TestDruidLeaderSelector(), null diff --git a/server/src/test/java/org/apache/druid/server/coordinator/TestDruidCoordinatorConfig.java b/server/src/test/java/org/apache/druid/server/coordinator/TestDruidCoordinatorConfig.java deleted file mode 100644 index c5ac565f75bc..000000000000 --- a/server/src/test/java/org/apache/druid/server/coordinator/TestDruidCoordinatorConfig.java +++ /dev/null @@ -1,523 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.apache.druid.server.coordinator; - -import org.joda.time.Duration; - -public class TestDruidCoordinatorConfig extends DruidCoordinatorConfig -{ - private final Duration coordinatorStartDelay; - private final Duration coordinatorPeriod; - private final Duration coordinatorIndexingPeriod; - private final Duration metadataStoreManagementPeriod; - private final Duration loadTimeoutDelay; - private final Duration coordinatorKillPeriod; - private final Duration coordinatorKillDurationToRetain; - private final Duration coordinatorSupervisorKillPeriod; - private final Duration coordinatorSupervisorKillDurationToRetain; - private final Duration coordinatorAuditKillPeriod; - private final Duration coordinatorAuditKillDurationToRetain; - private final Duration coordinatorCompactionKillPeriod; - private final Duration coordinatorRuleKillPeriod; - private final Duration coordinatorRuleKillDurationToRetain; - private final Duration coordinatorDatasourceKillPeriod; - private final Duration coordinatorDatasourceKillDurationToRetain; - private final int coordinatorKillMaxSegments; - private final boolean coordinatorKillIgnoreDurationToRetain; - private final String loadQueuePeonType; - private final Duration httpLoadQueuePeonRepeatDelay; - private final int curatorLoadQueuePeonNumCallbackThreads; - private final Duration httpLoadQueuePeonHostTimeout; - private final int httpLoadQueuePeonBatchSize; - private final Duration coordinatorKillBufferPeriod; - - public TestDruidCoordinatorConfig( - Duration coordinatorStartDelay, - Duration coordinatorPeriod, - Duration coordinatorIndexingPeriod, - Duration metadataStoreManagementPeriod, - Duration loadTimeoutDelay, - Duration coordinatorKillPeriod, - Duration coordinatorKillDurationToRetain, - Duration coordinatorSupervisorKillPeriod, - Duration coordinatorSupervisorKillDurationToRetain, - Duration coordinatorAuditKillPeriod, - Duration coordinatorAuditKillDurationToRetain, - Duration coordinatorCompactionKillPeriod, - Duration coordinatorRuleKillPeriod, - Duration coordinatorRuleKillDurationToRetain, - Duration coordinatorDatasourceKillPeriod, - Duration coordinatorDatasourceKillDurationToRetain, - int coordinatorKillMaxSegments, - boolean coordinatorKillIgnoreDurationToRetain, - String loadQueuePeonType, - Duration httpLoadQueuePeonRepeatDelay, - Duration httpLoadQueuePeonHostTimeout, - int httpLoadQueuePeonBatchSize, - int curatorLoadQueuePeonNumCallbackThreads, - Duration coordinatorKillBufferPeriod - ) - { - this.coordinatorStartDelay = coordinatorStartDelay; - this.coordinatorPeriod = coordinatorPeriod; - this.coordinatorIndexingPeriod = coordinatorIndexingPeriod; - this.metadataStoreManagementPeriod = metadataStoreManagementPeriod; - this.loadTimeoutDelay = loadTimeoutDelay; - this.coordinatorKillPeriod = coordinatorKillPeriod; - this.coordinatorKillDurationToRetain = coordinatorKillDurationToRetain; - this.coordinatorSupervisorKillPeriod = coordinatorSupervisorKillPeriod; - this.coordinatorSupervisorKillDurationToRetain = coordinatorSupervisorKillDurationToRetain; - this.coordinatorAuditKillPeriod = coordinatorAuditKillPeriod; - this.coordinatorAuditKillDurationToRetain = coordinatorAuditKillDurationToRetain; - this.coordinatorCompactionKillPeriod = coordinatorCompactionKillPeriod; - this.coordinatorRuleKillPeriod = coordinatorRuleKillPeriod; - this.coordinatorRuleKillDurationToRetain = coordinatorRuleKillDurationToRetain; - this.coordinatorDatasourceKillPeriod = coordinatorDatasourceKillPeriod; - this.coordinatorDatasourceKillDurationToRetain = coordinatorDatasourceKillDurationToRetain; - this.coordinatorKillMaxSegments = coordinatorKillMaxSegments; - this.coordinatorKillIgnoreDurationToRetain = coordinatorKillIgnoreDurationToRetain; - this.loadQueuePeonType = loadQueuePeonType; - this.httpLoadQueuePeonRepeatDelay = httpLoadQueuePeonRepeatDelay; - this.httpLoadQueuePeonHostTimeout = httpLoadQueuePeonHostTimeout; - this.httpLoadQueuePeonBatchSize = httpLoadQueuePeonBatchSize; - this.curatorLoadQueuePeonNumCallbackThreads = curatorLoadQueuePeonNumCallbackThreads; - this.coordinatorKillBufferPeriod = coordinatorKillBufferPeriod; - } - - @Override - public Duration getCoordinatorStartDelay() - { - return coordinatorStartDelay; - } - - @Override - public Duration getCoordinatorPeriod() - { - return coordinatorPeriod; - } - - @Override - public Duration getCoordinatorIndexingPeriod() - { - return coordinatorIndexingPeriod; - } - - @Override - public Duration getCoordinatorMetadataStoreManagementPeriod() - { - return metadataStoreManagementPeriod; - } - - @Override - public boolean isKillUnusedSegmentsEnabled() - { - return true; - } - - @Override - public boolean isKillPendingSegmentsEnabled() - { - return true; - } - - @Override - public Duration getCoordinatorKillPeriod() - { - return coordinatorKillPeriod; - } - - @Override - public Duration getCoordinatorKillDurationToRetain() - { - return coordinatorKillDurationToRetain; - } - - @Override - public Duration getCoordinatorSupervisorKillPeriod() - { - return coordinatorSupervisorKillPeriod; - } - - @Override - public boolean isSupervisorKillEnabled() - { - return true; - } - - @Override - public Duration getCoordinatorSupervisorKillDurationToRetain() - { - return coordinatorSupervisorKillDurationToRetain; - } - - @Override - public boolean isAuditKillEnabled() - { - return true; - } - - @Override - public Duration getCoordinatorAuditKillPeriod() - { - return coordinatorAuditKillPeriod; - } - - @Override - public Duration getCoordinatorAuditKillDurationToRetain() - { - return coordinatorAuditKillDurationToRetain; - } - - @Override - public boolean isCompactionKillEnabled() - { - return true; - } - - @Override - public Duration getCoordinatorCompactionKillPeriod() - { - return coordinatorCompactionKillPeriod; - } - - @Override - public boolean isRuleKillEnabled() - { - return true; - } - - @Override - public Duration getCoordinatorRuleKillPeriod() - { - return coordinatorRuleKillPeriod; - } - - @Override - public Duration getCoordinatorRuleKillDurationToRetain() - { - return coordinatorRuleKillDurationToRetain; - } - - @Override - public boolean isDatasourceKillEnabled() - { - return true; - } - - @Override - public Duration getCoordinatorDatasourceKillPeriod() - { - return coordinatorDatasourceKillPeriod; - } - - @Override - public Duration getCoordinatorDatasourceKillDurationToRetain() - { - return coordinatorDatasourceKillDurationToRetain; - } - - @Override - public int getCoordinatorKillMaxSegments() - { - return coordinatorKillMaxSegments; - } - - @Override - public Duration getLoadTimeoutDelay() - { - return loadTimeoutDelay == null ? super.getLoadTimeoutDelay() : loadTimeoutDelay; - } - - @Override - public boolean getCoordinatorKillIgnoreDurationToRetain() - { - return coordinatorKillIgnoreDurationToRetain; - } - - @Override - public String getLoadQueuePeonType() - { - return loadQueuePeonType; - } - - @Override - public Duration getHttpLoadQueuePeonRepeatDelay() - { - return httpLoadQueuePeonRepeatDelay; - } - - @Override - public int getNumCuratorCallBackThreads() - { - return curatorLoadQueuePeonNumCallbackThreads; - } - - @Override - public Duration getHttpLoadQueuePeonHostTimeout() - { - return httpLoadQueuePeonHostTimeout; - } - - @Override - public int getHttpLoadQueuePeonBatchSize() - { - return httpLoadQueuePeonBatchSize; - } - - @Override - public Duration getCoordinatorKillBufferPeriod() - { - return coordinatorKillBufferPeriod; - } - - public static class Builder - { - private static final Duration DEFAULT_COORDINATOR_START_DELAY = new Duration("PT300s"); - private static final Duration DEFAULT_COORDINATOR_PERIOD = new Duration("PT60s"); - private static final Duration DEFAULT_COORDINATOR_INDEXING_PERIOD = new Duration("PT1800s"); - private static final Duration DEFAULT_METADATA_STORE_MANAGEMENT_PERIOD = new Duration("PT3600s"); - private static final Duration DEFAULT_COORDINATOR_KILL_PERIOD = new Duration("PT86400s"); - private static final Duration DEFAULT_COORDINATOR_KILL_DURATION_TO_RETAION = new Duration("PT7776000s"); - private static final boolean DEFAULT_COORDINATOR_KILL_IGNORE_DURATION_TO_RETAIN = false; - private static final int DEFAULT_COORDINATOR_KILL_MAX_SEGMENTS = 100; - private static final Duration DEFAULT_COORDINATOR_SUPERVISOR_KILL_PERIOD = new Duration("PT86400s"); - private static final Duration DEFAULT_COORDINATOR_SUPERVISOR_KILL_DURATION_TO_RETAIN = new Duration("PT7776000s"); - private static final Duration DEFAULT_COORDINATOR_COMPACTION_KILL_PERIOD = new Duration("PT86400s"); - private static final Duration DEFAULT_COORDINATOR_RULE_KILL_PERIOD = new Duration("PT86400s"); - private static final Duration DEFAULT_COORDINATOR_RULE_KILL_DURATION_TO_RETAIN = new Duration("PT7776000s"); - private static final Duration DEFAULT_COORDINATOR_DATASOURCE_KILL_PERIOD = new Duration("PT86400s"); - private static final Duration DEFAULT_COORDINATOR_DATASOURCE_KILL_DURATION_TO_RETAIN = new Duration("PT7776000s"); - private static final Duration DEFAULT_LOAD_TIMEOUT_DELAY = new Duration(15 * 60 * 1000); - private static final String DEFAULT_LOAD_QUEUE_PEON_TYPE = "curator"; - private static final int DEFAULT_CURATOR_LOAD_QUEUE_PEON_NUM_CALLBACK_THREADS = 2; - private static final Duration DEFAULT_HTTP_LOAD_QUEUE_PEON_REPEAT_DELAY = Duration.millis(60000); - private static final Duration DEFAULT_HTTP_LOAD_QUEUE_PEON_HOST_TIMEOUT = Duration.millis(300000); - private static final int DEFAULT_HTTP_LOAD_QUEUE_PEON_BATCH_SIZE = 1; - private static final Duration DEFAULT_COORDINATOR_AUDIT_KILL_PERIOD = new Duration("PT86400s"); - private static final Duration DEFAULT_COORDINATOR_AUTIT_KILL_DURATION_TO_RETAIN = new Duration("PT7776000s"); - private static final Duration DEFAULT_COORDINATOR_KILL_BUFFER_PERIOD = new Duration("PT86400s"); - - - private Duration coordinatorStartDelay; - private Duration coordinatorPeriod; - private Duration coordinatorIndexingPeriod; - private Duration metadataStoreManagementPeriod; - private Duration coordinatorKillPeriod; - private Duration coordinatorKillDurationToRetain; - private Boolean coordinatorKillIgnoreDurationToRetain; - private Integer coordinatorKillMaxSegments; - private Duration coordinatorSupervisorKillPeriod; - private Duration coordinatorSupervisorKillDurationToRetain; - private Duration coordinatorCompactionKillPeriod; - private Duration coordinatorRuleKillPeriod; - private Duration coordinatorRuleKillDurationToRetain; - private Duration coordinatorDatasourceKillPeriod; - private Duration coordinatorDatasourceKillDurationToRetain; - private Duration loadTimeoutDelay; - private String loadQueuePeonType; - private Duration httpLoadQueuePeonRepeatDelay; - private Integer curatorLoadQueuePeonNumCallbackThreads; - private Duration httpLoadQueuePeonHostTimeout; - private Integer httpLoadQueuePeonBatchSize; - private Duration coordinatorAuditKillPeriod; - private Duration coordinatorAuditKillDurationToRetain; - private Duration coordinatorKillBufferPeriod; - - public Builder() - { - } - - public Builder withCoordinatorStartDelay(Duration coordinatorStartDelay) - { - this.coordinatorStartDelay = coordinatorStartDelay; - return this; - } - - public Builder withCoordinatorPeriod(Duration coordinatorPeriod) - { - this.coordinatorPeriod = coordinatorPeriod; - return this; - } - - public Builder withCoordinatorIndexingPeriod(Duration coordinatorIndexingPeriod) - { - this.coordinatorIndexingPeriod = coordinatorIndexingPeriod; - return this; - } - - public Builder withMetadataStoreManagementPeriod(Duration metadataStoreManagementPeriod) - { - this.metadataStoreManagementPeriod = metadataStoreManagementPeriod; - return this; - } - - public Builder withCoordinatorKillPeriod(Duration coordinatorKillPeriod) - { - this.coordinatorKillPeriod = coordinatorKillPeriod; - return this; - } - - public Builder withCoordinatorKillDurationToRetain(Duration coordinatorKillDurationToRetain) - { - this.coordinatorKillDurationToRetain = coordinatorKillDurationToRetain; - return this; - } - - public Builder withCoordinatorKillIgnoreDurationToRetain(boolean coordinatorKillIgnoreDurationToRetain) - { - this.coordinatorKillIgnoreDurationToRetain = coordinatorKillIgnoreDurationToRetain; - return this; - } - - public Builder withCoordinatorKillMaxSegments(int coordinatorKillMaxSegments) - { - this.coordinatorKillMaxSegments = coordinatorKillMaxSegments; - return this; - } - - public Builder withCoordinatorSupervisorKillPeriod(Duration coordinatorSupervisorKillPeriod) - { - this.coordinatorSupervisorKillPeriod = coordinatorSupervisorKillPeriod; - return this; - } - - public Builder withCoordinatorSupervisorKillDurationToRetain(Duration coordinatorSupervisorKillDurationToRetain) - { - this.coordinatorSupervisorKillDurationToRetain = coordinatorSupervisorKillDurationToRetain; - return this; - } - - public Builder withCoordinatorCompactionKillPeriod(Duration coordinatorCompactionKillPeriod) - { - this.coordinatorCompactionKillPeriod = coordinatorCompactionKillPeriod; - return this; - } - - public Builder withCoordinatorRuleKillPeriod(Duration coordinatorRuleKillPeriod) - { - this.coordinatorRuleKillPeriod = coordinatorRuleKillPeriod; - return this; - } - - public Builder withCoordinatorRuleKillDurationToRetain(Duration coordinatorRuleKillDurationToRetain) - { - this.coordinatorRuleKillDurationToRetain = coordinatorRuleKillDurationToRetain; - return this; - } - - public Builder withCoordinatorDatasourceKillPeriod(Duration coordinatorDatasourceKillPeriod) - { - this.coordinatorDatasourceKillPeriod = coordinatorDatasourceKillPeriod; - return this; - } - - public Builder withCoordinatorDatasourceKillDurationToRetain(Duration coordinatorDatasourceKillDurationToRetain) - { - this.coordinatorDatasourceKillDurationToRetain = coordinatorDatasourceKillDurationToRetain; - return this; - } - - public Builder withLoadTimeoutDelay(Duration loadTimeoutDelay) - { - this.loadTimeoutDelay = loadTimeoutDelay; - return this; - } - - public Builder withLoadQueuePeonType(String loadQueuePeonType) - { - this.loadQueuePeonType = loadQueuePeonType; - return this; - } - - public Builder withHttpLoadQueuePeonRepeatDelay(Duration httpLoadQueuePeonRepeatDelay) - { - this.httpLoadQueuePeonRepeatDelay = httpLoadQueuePeonRepeatDelay; - return this; - } - - public Builder withCuratorLoadQueuePeonNumCallbackThreads(int curatorLoadQueuePeonNumCallbackThreads) - { - this.curatorLoadQueuePeonNumCallbackThreads = curatorLoadQueuePeonNumCallbackThreads; - return this; - } - - public Builder withHttpLoadQueuePeonHostTimeout(Duration httpLoadQueuePeonHostTimeout) - { - this.httpLoadQueuePeonHostTimeout = httpLoadQueuePeonHostTimeout; - return this; - } - - public Builder withHttpLoadQueuePeonBatchSize(int httpLoadQueuePeonBatchSize) - { - this.httpLoadQueuePeonBatchSize = httpLoadQueuePeonBatchSize; - return this; - } - - public Builder withCoordianatorAuditKillPeriod(Duration coordinatorAuditKillPeriod) - { - this.coordinatorAuditKillPeriod = coordinatorAuditKillPeriod; - return this; - } - - public Builder withCoordinatorAuditKillDurationToRetain(Duration coordinatorAuditKillDurationToRetain) - { - this.coordinatorAuditKillDurationToRetain = coordinatorAuditKillDurationToRetain; - return this; - } - - public Builder withCoordinatorKillBufferPeriod(Duration coordinatorKillBufferPeriod) - { - this.coordinatorKillBufferPeriod = coordinatorKillBufferPeriod; - return this; - } - - public TestDruidCoordinatorConfig build() - { - return new TestDruidCoordinatorConfig( - coordinatorStartDelay == null ? DEFAULT_COORDINATOR_START_DELAY : coordinatorStartDelay, - coordinatorPeriod == null ? DEFAULT_COORDINATOR_PERIOD : coordinatorPeriod, - coordinatorIndexingPeriod == null ? DEFAULT_COORDINATOR_INDEXING_PERIOD : coordinatorIndexingPeriod, - metadataStoreManagementPeriod == null ? DEFAULT_METADATA_STORE_MANAGEMENT_PERIOD : metadataStoreManagementPeriod, - loadTimeoutDelay == null ? DEFAULT_LOAD_TIMEOUT_DELAY : loadTimeoutDelay, - coordinatorKillPeriod == null ? DEFAULT_COORDINATOR_KILL_PERIOD : coordinatorKillPeriod, - coordinatorKillDurationToRetain == null ? DEFAULT_COORDINATOR_KILL_DURATION_TO_RETAION : coordinatorKillDurationToRetain, - coordinatorSupervisorKillPeriod == null ? DEFAULT_COORDINATOR_SUPERVISOR_KILL_PERIOD : coordinatorSupervisorKillPeriod, - coordinatorSupervisorKillDurationToRetain == null ? DEFAULT_COORDINATOR_SUPERVISOR_KILL_DURATION_TO_RETAIN : coordinatorSupervisorKillDurationToRetain, - coordinatorAuditKillPeriod == null ? DEFAULT_COORDINATOR_AUDIT_KILL_PERIOD : coordinatorAuditKillPeriod, - coordinatorAuditKillDurationToRetain == null ? DEFAULT_COORDINATOR_AUTIT_KILL_DURATION_TO_RETAIN : coordinatorAuditKillDurationToRetain, - coordinatorCompactionKillPeriod == null ? DEFAULT_COORDINATOR_COMPACTION_KILL_PERIOD : coordinatorCompactionKillPeriod, - coordinatorRuleKillPeriod == null ? DEFAULT_COORDINATOR_RULE_KILL_PERIOD : coordinatorRuleKillPeriod, - coordinatorRuleKillDurationToRetain == null ? DEFAULT_COORDINATOR_RULE_KILL_DURATION_TO_RETAIN : coordinatorRuleKillDurationToRetain, - coordinatorDatasourceKillPeriod == null ? DEFAULT_COORDINATOR_DATASOURCE_KILL_PERIOD : coordinatorDatasourceKillPeriod, - coordinatorDatasourceKillDurationToRetain == null ? DEFAULT_COORDINATOR_DATASOURCE_KILL_DURATION_TO_RETAIN : coordinatorDatasourceKillDurationToRetain, - coordinatorKillMaxSegments == null ? DEFAULT_COORDINATOR_KILL_MAX_SEGMENTS : coordinatorKillMaxSegments, - coordinatorKillIgnoreDurationToRetain == null ? DEFAULT_COORDINATOR_KILL_IGNORE_DURATION_TO_RETAIN : coordinatorKillIgnoreDurationToRetain, - loadQueuePeonType == null ? DEFAULT_LOAD_QUEUE_PEON_TYPE : loadQueuePeonType, - httpLoadQueuePeonRepeatDelay == null ? DEFAULT_HTTP_LOAD_QUEUE_PEON_REPEAT_DELAY : httpLoadQueuePeonRepeatDelay, - httpLoadQueuePeonHostTimeout == null ? DEFAULT_HTTP_LOAD_QUEUE_PEON_HOST_TIMEOUT : httpLoadQueuePeonHostTimeout, - httpLoadQueuePeonBatchSize == null ? DEFAULT_HTTP_LOAD_QUEUE_PEON_BATCH_SIZE : httpLoadQueuePeonBatchSize, - curatorLoadQueuePeonNumCallbackThreads == null ? DEFAULT_CURATOR_LOAD_QUEUE_PEON_NUM_CALLBACK_THREADS - : curatorLoadQueuePeonNumCallbackThreads, - coordinatorKillBufferPeriod == null ? DEFAULT_COORDINATOR_KILL_BUFFER_PERIOD : coordinatorKillBufferPeriod - ); - } - - } -} diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/KillAuditLogTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/KillAuditLogTest.java index f6475fbe89ab..69ea92ac69a0 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/KillAuditLogTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/KillAuditLogTest.java @@ -21,7 +21,7 @@ import org.apache.druid.audit.AuditManager; import org.apache.druid.server.coordinator.DruidCoordinatorRuntimeParams; -import org.apache.druid.server.coordinator.TestDruidCoordinatorConfig; +import org.apache.druid.server.coordinator.config.MetadataCleanupConfig; import org.apache.druid.server.coordinator.stats.CoordinatorRunStats; import org.apache.druid.server.coordinator.stats.Stats; import org.joda.time.Duration; @@ -56,14 +56,9 @@ public void setup() @Test public void testRunSkipIfLastRunLessThanPeriod() { - TestDruidCoordinatorConfig druidCoordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withMetadataStoreManagementPeriod(new Duration("PT5s")) - .withCoordianatorAuditKillPeriod(new Duration(Long.MAX_VALUE)) - .withCoordinatorAuditKillDurationToRetain(new Duration("PT1S")) - .withCoordinatorKillMaxSegments(10) - .withCoordinatorKillIgnoreDurationToRetain(false) - .build(); - killAuditLog = new KillAuditLog(druidCoordinatorConfig, mockAuditManager); + final MetadataCleanupConfig config + = new MetadataCleanupConfig(true, new Duration(Long.MAX_VALUE), new Duration("PT1S")); + killAuditLog = new KillAuditLog(config, mockAuditManager); killAuditLog.run(mockDruidCoordinatorRuntimeParams); Mockito.verifyNoInteractions(mockAuditManager); } @@ -71,58 +66,11 @@ public void testRunSkipIfLastRunLessThanPeriod() @Test public void testRunNotSkipIfLastRunMoreThanPeriod() { - TestDruidCoordinatorConfig druidCoordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withMetadataStoreManagementPeriod(new Duration("PT5s")) - .withCoordianatorAuditKillPeriod(new Duration("PT6S")) - .withCoordinatorAuditKillDurationToRetain(new Duration("PT1S")) - .withCoordinatorKillMaxSegments(10) - .withCoordinatorKillIgnoreDurationToRetain(false) - .build(); - killAuditLog = new KillAuditLog(druidCoordinatorConfig, mockAuditManager); + final MetadataCleanupConfig config + = new MetadataCleanupConfig(true, new Duration("PT6S"), new Duration("PT1S")); + killAuditLog = new KillAuditLog(config, mockAuditManager); killAuditLog.run(mockDruidCoordinatorRuntimeParams); Mockito.verify(mockAuditManager).removeAuditLogsOlderThan(ArgumentMatchers.anyLong()); Assert.assertTrue(runStats.hasStat(Stats.Kill.AUDIT_LOGS)); } - - @Test - public void testConstructorFailIfInvalidPeriod() - { - TestDruidCoordinatorConfig druidCoordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withMetadataStoreManagementPeriod(new Duration("PT5s")) - .withCoordianatorAuditKillPeriod(new Duration("PT3S")) - .withCoordinatorAuditKillDurationToRetain(new Duration("PT1S")) - .withCoordinatorKillMaxSegments(10) - .withCoordinatorKillIgnoreDurationToRetain(false) - .build(); - - final IllegalArgumentException exception = Assert.assertThrows( - IllegalArgumentException.class, - () -> killAuditLog = new KillAuditLog(druidCoordinatorConfig, mockAuditManager) - ); - Assert.assertEquals( - "[druid.coordinator.kill.audit.period] must be greater than" - + " [druid.coordinator.period.metadataStoreManagementPeriod]", - exception.getMessage() - ); - } - - @Test - public void testConstructorFailIfInvalidRetainDuration() - { - TestDruidCoordinatorConfig druidCoordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withMetadataStoreManagementPeriod(new Duration("PT5S")) - .withCoordianatorAuditKillPeriod(new Duration("PT6S")) - .withCoordinatorAuditKillDurationToRetain(new Duration("PT-1S")) - .withCoordinatorKillMaxSegments(10) - .withCoordinatorKillIgnoreDurationToRetain(false) - .build(); - final IllegalArgumentException exception = Assert.assertThrows( - IllegalArgumentException.class, - () -> killAuditLog = new KillAuditLog(druidCoordinatorConfig, mockAuditManager) - ); - Assert.assertEquals( - "[druid.coordinator.kill.audit.durationToRetain] must be 0 milliseconds or higher", - exception.getMessage() - ); - } } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/KillCompactionConfigTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/KillCompactionConfigTest.java index d331204525a4..ec4547c940c4 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/KillCompactionConfigTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/KillCompactionConfigTest.java @@ -32,8 +32,8 @@ import org.apache.druid.server.coordinator.CoordinatorConfigManager; import org.apache.druid.server.coordinator.DataSourceCompactionConfig; import org.apache.druid.server.coordinator.DruidCoordinatorRuntimeParams; -import org.apache.druid.server.coordinator.TestDruidCoordinatorConfig; import org.apache.druid.server.coordinator.UserCompactionTaskGranularityConfig; +import org.apache.druid.server.coordinator.config.MetadataCleanupConfig; import org.apache.druid.server.coordinator.stats.CoordinatorRunStats; import org.apache.druid.server.coordinator.stats.Stats; import org.joda.time.Duration; @@ -86,14 +86,10 @@ public void setup() @Test public void testRunSkipIfLastRunLessThanPeriod() { - TestDruidCoordinatorConfig druidCoordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withMetadataStoreManagementPeriod(new Duration("PT5S")) - .withCoordinatorCompactionKillPeriod(new Duration(Long.MAX_VALUE)) - .withCoordinatorKillMaxSegments(10) - .withCoordinatorKillIgnoreDurationToRetain(false) - .build(); + final MetadataCleanupConfig config + = new MetadataCleanupConfig(true, new Duration(Long.MAX_VALUE), null); killCompactionConfig = new KillCompactionConfig( - druidCoordinatorConfig, + config, mockSqlSegmentsMetadataManager, coordinatorConfigManager ); @@ -103,32 +99,6 @@ public void testRunSkipIfLastRunLessThanPeriod() Assert.assertEquals(0, runStats.rowCount()); } - @Test - public void testConstructorFailIfInvalidPeriod() - { - TestDruidCoordinatorConfig druidCoordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withMetadataStoreManagementPeriod(new Duration("PT5S")) - .withCoordinatorCompactionKillPeriod(new Duration("PT3S")) - .withCoordinatorKillMaxSegments(10) - .withCoordinatorKillIgnoreDurationToRetain(false) - .build(); - - final IllegalArgumentException exception = Assert.assertThrows( - IllegalArgumentException.class, - () -> killCompactionConfig = new KillCompactionConfig( - druidCoordinatorConfig, - mockSqlSegmentsMetadataManager, - coordinatorConfigManager - ) - ); - Assert.assertEquals( - "[druid.coordinator.kill.compaction.period] must be greater than" - + " [druid.coordinator.period.metadataStoreManagementPeriod]", - exception.getMessage() - ); - } - - @Test public void testRunDoNothingIfCurrentConfigIsEmpty() { @@ -145,14 +115,10 @@ public void testRunDoNothingIfCurrentConfigIsEmpty() ArgumentMatchers.eq(CoordinatorCompactionConfig.empty())) ).thenReturn(CoordinatorCompactionConfig.empty()); - TestDruidCoordinatorConfig druidCoordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withMetadataStoreManagementPeriod(new Duration("PT5S")) - .withCoordinatorCompactionKillPeriod(new Duration("PT6S")) - .withCoordinatorKillMaxSegments(10) - .withCoordinatorKillIgnoreDurationToRetain(false) - .build(); + final MetadataCleanupConfig config + = new MetadataCleanupConfig(true, new Duration("PT6S"), null); killCompactionConfig = new KillCompactionConfig( - druidCoordinatorConfig, + config, mockSqlSegmentsMetadataManager, coordinatorConfigManager ); @@ -232,14 +198,10 @@ public void testRunRemoveInactiveDatasourceCompactionConfig() ArgumentMatchers.any()) ).thenReturn(ConfigManager.SetResult.ok()); - TestDruidCoordinatorConfig druidCoordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withMetadataStoreManagementPeriod(new Duration("PT5S")) - .withCoordinatorCompactionKillPeriod(new Duration("PT6S")) - .withCoordinatorKillMaxSegments(10) - .withCoordinatorKillIgnoreDurationToRetain(false) - .build(); + final MetadataCleanupConfig config + = new MetadataCleanupConfig(true, new Duration("PT6S"), null); killCompactionConfig = new KillCompactionConfig( - druidCoordinatorConfig, + config, mockSqlSegmentsMetadataManager, coordinatorConfigManager ); @@ -324,14 +286,10 @@ public void testRunRetryForRetryableException() ConfigManager.SetResult.ok() ); - TestDruidCoordinatorConfig druidCoordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withMetadataStoreManagementPeriod(new Duration("PT5S")) - .withCoordinatorCompactionKillPeriod(new Duration("PT6S")) - .withCoordinatorKillMaxSegments(10) - .withCoordinatorKillIgnoreDurationToRetain(false) - .build(); + final MetadataCleanupConfig config + = new MetadataCleanupConfig(true, new Duration("PT6S"), null); killCompactionConfig = new KillCompactionConfig( - druidCoordinatorConfig, + config, mockSqlSegmentsMetadataManager, coordinatorConfigManager ); diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/KillDatasourceMetadataTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/KillDatasourceMetadataTest.java index 222d5a715fba..5e848cde0732 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/KillDatasourceMetadataTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/KillDatasourceMetadataTest.java @@ -23,7 +23,7 @@ import org.apache.druid.indexing.overlord.IndexerMetadataStorageCoordinator; import org.apache.druid.metadata.MetadataSupervisorManager; import org.apache.druid.server.coordinator.DruidCoordinatorRuntimeParams; -import org.apache.druid.server.coordinator.TestDruidCoordinatorConfig; +import org.apache.druid.server.coordinator.config.MetadataCleanupConfig; import org.apache.druid.server.coordinator.stats.CoordinatorRunStats; import org.apache.druid.server.coordinator.stats.Stats; import org.joda.time.Duration; @@ -61,14 +61,9 @@ public void setup() @Test public void testRunSkipIfLastRunLessThanPeriod() { - TestDruidCoordinatorConfig druidCoordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withMetadataStoreManagementPeriod(new Duration("PT5S")) - .withCoordinatorDatasourceKillPeriod(new Duration(Long.MAX_VALUE)) - .withCoordinatorDatasourceKillDurationToRetain(new Duration("PT1S")) - .withCoordinatorKillMaxSegments(10) - .withCoordinatorKillIgnoreDurationToRetain(false) - .build(); - killDatasourceMetadata = new KillDatasourceMetadata(druidCoordinatorConfig, mockIndexerMetadataStorageCoordinator, mockMetadataSupervisorManager); + final MetadataCleanupConfig config + = new MetadataCleanupConfig(true, new Duration(Long.MAX_VALUE), new Duration("PT1S")); + killDatasourceMetadata = new KillDatasourceMetadata(config, mockIndexerMetadataStorageCoordinator, mockMetadataSupervisorManager); killDatasourceMetadata.run(mockDruidCoordinatorRuntimeParams); Mockito.verifyNoInteractions(mockIndexerMetadataStorageCoordinator); Mockito.verifyNoInteractions(mockMetadataSupervisorManager); @@ -79,15 +74,10 @@ public void testRunNotSkipIfLastRunMoreThanPeriod() { Mockito.when(mockDruidCoordinatorRuntimeParams.getCoordinatorStats()).thenReturn(runStats); - TestDruidCoordinatorConfig druidCoordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withMetadataStoreManagementPeriod(new Duration("PT5S")) - .withCoordinatorDatasourceKillPeriod(new Duration("PT6S")) - .withCoordinatorDatasourceKillDurationToRetain(new Duration("PT1S")) - .withCoordinatorKillMaxSegments(10) - .withCoordinatorKillIgnoreDurationToRetain(false) - .build(); + final MetadataCleanupConfig config + = new MetadataCleanupConfig(true, new Duration("PT6S"), new Duration("PT1S")); killDatasourceMetadata = new KillDatasourceMetadata( - druidCoordinatorConfig, + config, mockIndexerMetadataStorageCoordinator, mockMetadataSupervisorManager ); @@ -96,67 +86,12 @@ public void testRunNotSkipIfLastRunMoreThanPeriod() Assert.assertTrue(runStats.hasStat(Stats.Kill.DATASOURCES)); } - @Test - public void testConstructorFailIfInvalidPeriod() - { - TestDruidCoordinatorConfig druidCoordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withMetadataStoreManagementPeriod(new Duration("PT5S")) - .withCoordinatorDatasourceKillPeriod(new Duration("PT3S")) - .withCoordinatorDatasourceKillDurationToRetain(new Duration("PT1S")) - .withCoordinatorKillMaxSegments(10) - .withCoordinatorKillIgnoreDurationToRetain(false) - .build(); - - final IllegalArgumentException exception = Assert.assertThrows( - IllegalArgumentException.class, - () -> killDatasourceMetadata = new KillDatasourceMetadata( - druidCoordinatorConfig, - mockIndexerMetadataStorageCoordinator, - mockMetadataSupervisorManager - ) - ); - Assert.assertEquals( - "[druid.coordinator.kill.datasource.period] must be greater than" - + " [druid.coordinator.period.metadataStoreManagementPeriod]", - exception.getMessage() - ); - } - - @Test - public void testConstructorFailIfInvalidRetainDuration() - { - TestDruidCoordinatorConfig druidCoordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withMetadataStoreManagementPeriod(new Duration("PT5S")) - .withCoordinatorDatasourceKillPeriod(new Duration("PT6S")) - .withCoordinatorDatasourceKillDurationToRetain(new Duration("PT-1S")) - .withCoordinatorKillMaxSegments(10) - .withCoordinatorKillIgnoreDurationToRetain(false) - .build(); - final IllegalArgumentException exception = Assert.assertThrows( - IllegalArgumentException.class, - () -> killDatasourceMetadata = new KillDatasourceMetadata( - druidCoordinatorConfig, - mockIndexerMetadataStorageCoordinator, - mockMetadataSupervisorManager - ) - ); - Assert.assertEquals( - "[druid.coordinator.kill.datasource.durationToRetain] must be 0 milliseconds or higher", - exception.getMessage() - ); - } - @Test public void testRunWithEmptyFilterExcludedDatasource() { - TestDruidCoordinatorConfig druidCoordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withMetadataStoreManagementPeriod(new Duration("PT5S")) - .withCoordinatorDatasourceKillPeriod(new Duration("PT6S")) - .withCoordinatorDatasourceKillDurationToRetain(new Duration("PT1S")) - .withCoordinatorKillMaxSegments(10) - .withCoordinatorKillIgnoreDurationToRetain(false) - .build(); - killDatasourceMetadata = new KillDatasourceMetadata(druidCoordinatorConfig, mockIndexerMetadataStorageCoordinator, mockMetadataSupervisorManager); + final MetadataCleanupConfig config + = new MetadataCleanupConfig(true, new Duration("PT6S"), new Duration("PT1S")); + killDatasourceMetadata = new KillDatasourceMetadata(config, mockIndexerMetadataStorageCoordinator, mockMetadataSupervisorManager); killDatasourceMetadata.run(mockDruidCoordinatorRuntimeParams); Mockito.verify(mockIndexerMetadataStorageCoordinator).removeDataSourceMetadataOlderThan(ArgumentMatchers.anyLong(), ArgumentMatchers.eq(ImmutableSet.of())); Assert.assertTrue(runStats.hasStat(Stats.Kill.DATASOURCES)); diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/KillRulesTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/KillRulesTest.java index f1663dffda54..fde2c2a28b98 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/KillRulesTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/KillRulesTest.java @@ -21,7 +21,7 @@ import org.apache.druid.metadata.MetadataRuleManager; import org.apache.druid.server.coordinator.DruidCoordinatorRuntimeParams; -import org.apache.druid.server.coordinator.TestDruidCoordinatorConfig; +import org.apache.druid.server.coordinator.config.MetadataCleanupConfig; import org.apache.druid.server.coordinator.stats.CoordinatorRunStats; import org.apache.druid.server.coordinator.stats.Stats; import org.joda.time.Duration; @@ -56,14 +56,9 @@ public void setup() @Test public void testRunSkipIfLastRunLessThanPeriod() { - TestDruidCoordinatorConfig druidCoordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withMetadataStoreManagementPeriod(new Duration("PT5S")) - .withCoordinatorRuleKillPeriod(new Duration(Long.MAX_VALUE)) - .withCoordinatorRuleKillDurationToRetain(new Duration("PT1S")) - .withCoordinatorKillMaxSegments(10) - .withCoordinatorKillIgnoreDurationToRetain(false) - .build(); - killRules = new KillRules(druidCoordinatorConfig, mockRuleManager); + final MetadataCleanupConfig config + = new MetadataCleanupConfig(true, new Duration(Long.MAX_VALUE), new Duration("PT1S")); + killRules = new KillRules(config, mockRuleManager); killRules.run(mockDruidCoordinatorRuntimeParams); Mockito.verifyNoInteractions(mockRuleManager); } @@ -71,57 +66,11 @@ public void testRunSkipIfLastRunLessThanPeriod() @Test public void testRunNotSkipIfLastRunMoreThanPeriod() { - TestDruidCoordinatorConfig druidCoordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withMetadataStoreManagementPeriod(new Duration("PT5S")) - .withCoordinatorRuleKillPeriod(new Duration("PT6S")) - .withCoordinatorRuleKillDurationToRetain(new Duration("PT1S")) - .withCoordinatorKillMaxSegments(10) - .withCoordinatorKillIgnoreDurationToRetain(false) - .build(); - killRules = new KillRules(druidCoordinatorConfig, mockRuleManager); + final MetadataCleanupConfig config + = new MetadataCleanupConfig(true, new Duration("PT6S"), new Duration("PT1S")); + killRules = new KillRules(config, mockRuleManager); killRules.run(mockDruidCoordinatorRuntimeParams); Mockito.verify(mockRuleManager).removeRulesForEmptyDatasourcesOlderThan(ArgumentMatchers.anyLong()); Assert.assertTrue(runStats.hasStat(Stats.Kill.RULES)); } - - @Test - public void testConstructorFailIfInvalidPeriod() - { - TestDruidCoordinatorConfig druidCoordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withMetadataStoreManagementPeriod(new Duration("PT5S")) - .withCoordinatorRuleKillPeriod(new Duration("PT3S")) - .withCoordinatorRuleKillDurationToRetain(new Duration("PT1S")) - .withCoordinatorKillMaxSegments(10) - .withCoordinatorKillIgnoreDurationToRetain(false) - .build(); - final IllegalArgumentException exception = Assert.assertThrows( - IllegalArgumentException.class, - () -> killRules = new KillRules(druidCoordinatorConfig, mockRuleManager) - ); - Assert.assertEquals( - "[druid.coordinator.kill.rule.period] must be greater than" - + " [druid.coordinator.period.metadataStoreManagementPeriod]", - exception.getMessage() - ); - } - - @Test - public void testConstructorFailIfInvalidRetainDuration() - { - TestDruidCoordinatorConfig druidCoordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withMetadataStoreManagementPeriod(new Duration("PT5S")) - .withCoordinatorRuleKillPeriod(new Duration("PT6S")) - .withCoordinatorRuleKillDurationToRetain(new Duration("PT-1S")) - .withCoordinatorKillMaxSegments(10) - .withCoordinatorKillIgnoreDurationToRetain(false) - .build(); - final IllegalArgumentException exception = Assert.assertThrows( - IllegalArgumentException.class, - () -> killRules = new KillRules(druidCoordinatorConfig, mockRuleManager) - ); - Assert.assertEquals( - "[druid.coordinator.kill.rule.durationToRetain] must be 0 milliseconds or higher", - exception.getMessage() - ); - } } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/KillSupervisorsCustomDutyTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/KillSupervisorsCustomDutyTest.java index e93e04f1b38c..4057b894589b 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/KillSupervisorsCustomDutyTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/KillSupervisorsCustomDutyTest.java @@ -60,7 +60,7 @@ public void testConstructorFailIfRetainDurationNull() { final IllegalArgumentException exception = Assert.assertThrows( IllegalArgumentException.class, - () -> killSupervisors = new KillSupervisorsCustomDuty(null, mockMetadataSupervisorManager, coordinatorConfig) + () -> killSupervisors = new KillSupervisorsCustomDuty(null, mockMetadataSupervisorManager) ); Assert.assertEquals( "[KillSupervisorsCustomDuty.durationToRetain] must be 0 milliseconds or higher", @@ -75,8 +75,7 @@ public void testConstructorFailIfRetainDurationInvalid() IllegalArgumentException.class, () -> killSupervisors = new KillSupervisorsCustomDuty( new Duration("PT-1S"), - mockMetadataSupervisorManager, - coordinatorConfig + mockMetadataSupervisorManager ) ); Assert.assertEquals( @@ -90,8 +89,7 @@ public void testConstructorSuccess() { killSupervisors = new KillSupervisorsCustomDuty( new Duration("PT1S"), - mockMetadataSupervisorManager, - coordinatorConfig + mockMetadataSupervisorManager ); Assert.assertNotNull(killSupervisors); } @@ -103,8 +101,7 @@ public void testRun() Mockito.when(mockDruidCoordinatorRuntimeParams.getCoordinatorStats()).thenReturn(runStats); killSupervisors = new KillSupervisorsCustomDuty( new Duration("PT1S"), - mockMetadataSupervisorManager, - coordinatorConfig + mockMetadataSupervisorManager ); killSupervisors.run(mockDruidCoordinatorRuntimeParams); Mockito.verify(mockMetadataSupervisorManager).removeTerminatedSupervisorsOlderThan(ArgumentMatchers.anyLong()); diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/KillSupervisorsTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/KillSupervisorsTest.java index bbe92a2cd10f..7362c335d08e 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/KillSupervisorsTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/KillSupervisorsTest.java @@ -21,7 +21,7 @@ import org.apache.druid.metadata.MetadataSupervisorManager; import org.apache.druid.server.coordinator.DruidCoordinatorRuntimeParams; -import org.apache.druid.server.coordinator.TestDruidCoordinatorConfig; +import org.apache.druid.server.coordinator.config.MetadataCleanupConfig; import org.apache.druid.server.coordinator.stats.CoordinatorRunStats; import org.apache.druid.server.coordinator.stats.Stats; import org.joda.time.Duration; @@ -56,14 +56,9 @@ public void setup() @Test public void testRunSkipIfLastRunLessThanPeriod() { - TestDruidCoordinatorConfig druidCoordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withMetadataStoreManagementPeriod(new Duration("PT5S")) - .withCoordinatorSupervisorKillPeriod(new Duration(Long.MAX_VALUE)) - .withCoordinatorSupervisorKillDurationToRetain(new Duration("PT1S")) - .withCoordinatorKillMaxSegments(10) - .withCoordinatorKillIgnoreDurationToRetain(false) - .build(); - killSupervisors = new KillSupervisors(druidCoordinatorConfig, mockMetadataSupervisorManager); + final MetadataCleanupConfig config + = new MetadataCleanupConfig(true, new Duration(Long.MAX_VALUE), new Duration("PT1S")); + killSupervisors = new KillSupervisors(config, mockMetadataSupervisorManager); killSupervisors.run(mockDruidCoordinatorRuntimeParams); Mockito.verifyNoInteractions(mockMetadataSupervisorManager); } @@ -71,58 +66,11 @@ public void testRunSkipIfLastRunLessThanPeriod() @Test public void testRunNotSkipIfLastRunMoreThanPeriod() { - TestDruidCoordinatorConfig druidCoordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withMetadataStoreManagementPeriod(new Duration("PT5S")) - .withCoordinatorSupervisorKillPeriod(new Duration("PT6S")) - .withCoordinatorSupervisorKillDurationToRetain(new Duration("PT1S")) - .withCoordinatorKillMaxSegments(10) - .withCoordinatorKillIgnoreDurationToRetain(false) - .build(); - killSupervisors = new KillSupervisors(druidCoordinatorConfig, mockMetadataSupervisorManager); + final MetadataCleanupConfig config + = new MetadataCleanupConfig(true, new Duration("PT6S"), new Duration("PT1S")); + killSupervisors = new KillSupervisors(config, mockMetadataSupervisorManager); killSupervisors.run(mockDruidCoordinatorRuntimeParams); Mockito.verify(mockMetadataSupervisorManager).removeTerminatedSupervisorsOlderThan(ArgumentMatchers.anyLong()); Assert.assertTrue(runStats.hasStat(Stats.Kill.SUPERVISOR_SPECS)); } - - @Test - public void testConstructorFailIfInvalidPeriod() - { - TestDruidCoordinatorConfig druidCoordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withMetadataStoreManagementPeriod(new Duration("PT5S")) - .withCoordinatorSupervisorKillPeriod(new Duration("PT3S")) - .withCoordinatorSupervisorKillDurationToRetain(new Duration("PT1S")) - .withCoordinatorKillMaxSegments(10) - .withCoordinatorKillIgnoreDurationToRetain(false) - .build(); - final IllegalArgumentException exception = Assert.assertThrows( - IllegalArgumentException.class, - () -> killSupervisors = new KillSupervisors(druidCoordinatorConfig, mockMetadataSupervisorManager) - ); - Assert.assertEquals( - "[druid.coordinator.kill.supervisor.period] must be greater than" - + " [druid.coordinator.period.metadataStoreManagementPeriod]", - exception.getMessage() - ); - } - - @Test - public void testConstructorFailIfInvalidRetainDuration() - { - TestDruidCoordinatorConfig druidCoordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withMetadataStoreManagementPeriod(new Duration("PT5S")) - .withCoordinatorSupervisorKillPeriod(new Duration("PT6S")) - .withCoordinatorSupervisorKillDurationToRetain(new Duration("PT-1S")) - .withCoordinatorKillMaxSegments(10) - .withCoordinatorKillIgnoreDurationToRetain(false) - .build(); - - final IllegalArgumentException exception = Assert.assertThrows( - IllegalArgumentException.class, - () -> killSupervisors = new KillSupervisors(druidCoordinatorConfig, mockMetadataSupervisorManager) - ); - Assert.assertEquals( - "[druid.coordinator.kill.supervisor.durationToRetain] must be 0 milliseconds or higher", - exception.getMessage() - ); - } } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/KillUnusedSegmentsTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/KillUnusedSegmentsTest.java index 5adb345c9fa9..510f1cef139e 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/KillUnusedSegmentsTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/KillUnusedSegmentsTest.java @@ -32,8 +32,8 @@ import org.apache.druid.metadata.SegmentsMetadataManager; import org.apache.druid.rpc.indexing.OverlordClient; import org.apache.druid.server.coordinator.CoordinatorDynamicConfig; -import org.apache.druid.server.coordinator.DruidCoordinatorConfig; import org.apache.druid.server.coordinator.DruidCoordinatorRuntimeParams; +import org.apache.druid.server.coordinator.config.KillUnusedSegmentsConfig; import org.apache.druid.server.coordinator.stats.CoordinatorRunStats; import org.apache.druid.server.coordinator.stats.Stats; import org.apache.druid.timeline.DataSegment; @@ -71,7 +71,6 @@ public class KillUnusedSegmentsTest private static final int MAX_SEGMENTS_TO_KILL = 10; private static final Duration COORDINATOR_KILL_PERIOD = Duration.standardMinutes(2); private static final Duration DURATION_TO_RETAIN = Duration.standardDays(1); - private static final Duration INDEXING_PERIOD = Duration.standardMinutes(1); private static final String DATASOURCE = "DS1"; @Mock @@ -79,7 +78,7 @@ public class KillUnusedSegmentsTest @Mock private OverlordClient overlordClient; @Mock(answer = Answers.RETURNS_DEEP_STUBS) - private DruidCoordinatorConfig config; + private KillUnusedSegmentsConfig config; @Mock private CoordinatorRunStats stats; @@ -102,11 +101,10 @@ public void setup() { Mockito.doReturn(coordinatorDynamicConfig).when(params).getCoordinatorDynamicConfig(); Mockito.doReturn(stats).when(params).getCoordinatorStats(); - Mockito.doReturn(COORDINATOR_KILL_PERIOD).when(config).getCoordinatorKillPeriod(); - Mockito.doReturn(DURATION_TO_RETAIN).when(config).getCoordinatorKillDurationToRetain(); - Mockito.doReturn(INDEXING_PERIOD).when(config).getCoordinatorIndexingPeriod(); - Mockito.doReturn(MAX_SEGMENTS_TO_KILL).when(config).getCoordinatorKillMaxSegments(); - Mockito.doReturn(Duration.parse("PT3154000000S")).when(config).getCoordinatorKillBufferPeriod(); + Mockito.doReturn(COORDINATOR_KILL_PERIOD).when(config).getCleanupPeriod(); + Mockito.doReturn(DURATION_TO_RETAIN).when(config).getDurationToRetain(); + Mockito.doReturn(MAX_SEGMENTS_TO_KILL).when(config).getMaxSegments(); + Mockito.doReturn(Duration.parse("PT3154000000S")).when(config).getBufferPeriod(); Mockito.doReturn(Collections.singleton(DATASOURCE)) .when(coordinatorDynamicConfig).getSpecificDataSourcesToKillUnusedSegmentsIn(); @@ -153,11 +151,7 @@ public void setup() return unusedIntervals.size() <= limit ? unusedIntervals : unusedIntervals.subList(0, limit); }); - target = new KillUnusedSegments( - segmentsMetadataManager, - overlordClient, - config - ); + target = new KillUnusedSegments(segmentsMetadataManager, overlordClient, config); } @Test @@ -181,7 +175,7 @@ public void testRunWithNoIntervalShouldNotKillAnySegments() public void testRunWithSpecificDatasourceAndNoIntervalShouldNotKillAnySegments() { Mockito.doReturn(Duration.standardDays(400)) - .when(config).getCoordinatorKillDurationToRetain(); + .when(config).getDurationToRetain(); target = new KillUnusedSegments( segmentsMetadataManager, overlordClient, @@ -214,12 +208,8 @@ public void testNegativeDurationToRetain() { // Duration to retain = -1 day, reinit target for config to take effect Mockito.doReturn(DURATION_TO_RETAIN.negated()) - .when(config).getCoordinatorKillDurationToRetain(); - target = new KillUnusedSegments( - segmentsMetadataManager, - overlordClient, - config - ); + .when(config).getDurationToRetain(); + target = new KillUnusedSegments(segmentsMetadataManager, overlordClient, config); // Segments upto 1 day in the future are killed Interval expectedKillInterval = new Interval( @@ -236,12 +226,8 @@ public void testNegativeDurationToRetain() public void testIgnoreDurationToRetain() { Mockito.doReturn(true) - .when(config).getCoordinatorKillIgnoreDurationToRetain(); - target = new KillUnusedSegments( - segmentsMetadataManager, - overlordClient, - config - ); + .when(config).isIgnoreDurationToRetain(); + target = new KillUnusedSegments(segmentsMetadataManager, overlordClient, config); // All future and past unused segments are killed Interval expectedKillInterval = new Interval( @@ -258,12 +244,8 @@ public void testIgnoreDurationToRetain() public void testMaxSegmentsToKill() { Mockito.doReturn(1) - .when(config).getCoordinatorKillMaxSegments(); - target = new KillUnusedSegments( - segmentsMetadataManager, - overlordClient, - config - ); + .when(config).getMaxSegments(); + target = new KillUnusedSegments(segmentsMetadataManager, overlordClient, config); mockTaskSlotUsage(1.0, Integer.MAX_VALUE, 1, 10); // Only 1 unused segment is killed @@ -276,14 +258,10 @@ public void testMaxSegmentsToKill() public void testMultipleRuns() { Mockito.doReturn(true) - .when(config).getCoordinatorKillIgnoreDurationToRetain(); + .when(config).isIgnoreDurationToRetain(); Mockito.doReturn(2) - .when(config).getCoordinatorKillMaxSegments(); - target = new KillUnusedSegments( - segmentsMetadataManager, - overlordClient, - config - ); + .when(config).getMaxSegments(); + target = new KillUnusedSegments(segmentsMetadataManager, overlordClient, config); mockTaskSlotUsage(1.0, Integer.MAX_VALUE, 1, 10); runAndVerifyKillInterval(new Interval( @@ -362,7 +340,7 @@ public void testGetKillTaskCapacity() private void runAndVerifyKillInterval(Interval expectedKillInterval) { - int limit = config.getCoordinatorKillMaxSegments(); + int limit = config.getMaxSegments(); Mockito.doReturn(Futures.immediateFuture("ok")) .when(overlordClient) .runKillTask( @@ -382,7 +360,7 @@ private void runAndVerifyKillInterval(Interval expectedKillInterval) private void runAndVerifyKillIntervals(List expectedKillIntervals) { - int limit = config.getCoordinatorKillMaxSegments(); + int limit = config.getMaxSegments(); Mockito.doReturn(Futures.immediateFuture("ok")) .when(overlordClient) .runKillTask( diff --git a/server/src/test/java/org/apache/druid/server/coordinator/loading/HttpLoadQueuePeonTest.java b/server/src/test/java/org/apache/druid/server/coordinator/loading/HttpLoadQueuePeonTest.java index c3d3fb0fbe8f..de1801d7ad7a 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/loading/HttpLoadQueuePeonTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/loading/HttpLoadQueuePeonTest.java @@ -33,7 +33,7 @@ import org.apache.druid.server.coordination.DataSegmentChangeRequest; import org.apache.druid.server.coordination.SegmentLoadDropHandler; import org.apache.druid.server.coordinator.CreateDataSegments; -import org.apache.druid.server.coordinator.TestDruidCoordinatorConfig; +import org.apache.druid.server.coordinator.config.HttpLoadQueuePeonConfig; import org.apache.druid.server.coordinator.simulate.BlockingExecutorService; import org.apache.druid.server.coordinator.simulate.WrappingScheduledExecutorService; import org.apache.druid.timeline.DataSegment; @@ -89,9 +89,7 @@ public void setUp() "http://dummy:4000", ServerTestHelper.MAPPER, httpClient, - new TestDruidCoordinatorConfig.Builder() - .withHttpLoadQueuePeonBatchSize(10) - .build(), + new HttpLoadQueuePeonConfig(null, null, 10), new WrappingScheduledExecutorService("HttpLoadQueuePeonTest-%s", processingExecutor, true), callbackExecutor ); diff --git a/server/src/test/java/org/apache/druid/server/coordinator/loading/LoadQueuePeonTest.java b/server/src/test/java/org/apache/druid/server/coordinator/loading/LoadQueuePeonTest.java index aba1c9b6be6c..6717a44bfc3c 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/loading/LoadQueuePeonTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/loading/LoadQueuePeonTest.java @@ -38,11 +38,9 @@ import org.apache.druid.server.coordination.DataSegmentChangeRequest; import org.apache.druid.server.coordination.SegmentChangeRequestDrop; import org.apache.druid.server.coordination.SegmentChangeRequestLoad; -import org.apache.druid.server.coordinator.TestDruidCoordinatorConfig; import org.apache.druid.timeline.DataSegment; import org.apache.druid.timeline.SegmentId; import org.apache.druid.timeline.partition.NoneShardSpec; -import org.joda.time.Duration; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -88,11 +86,7 @@ public void testMultipleLoadDropSegments() throws Exception LOAD_QUEUE_PATH, jsonMapper, Execs.scheduledSingleThreaded("test_load_queue_peon_scheduled-%d"), - Execs.singleThreaded("test_load_queue_peon-%d"), - new TestDruidCoordinatorConfig.Builder() - .withCoordinatorKillMaxSegments(10) - .withCoordinatorKillIgnoreDurationToRetain(false) - .build() + Execs.singleThreaded("test_load_queue_peon-%d") ); loadQueuePeon.start(); @@ -277,13 +271,8 @@ public void testFailAssignForNonTimeoutFailures() throws Exception // This will fail inside SegmentChangeProcessor.run() null, Execs.scheduledSingleThreaded("test_load_queue_peon_scheduled-%d"), - Execs.singleThreaded("test_load_queue_peon-%d"), + Execs.singleThreaded("test_load_queue_peon-%d") // set time-out to 1 ms so that LoadQueuePeon will fail the assignment quickly - new TestDruidCoordinatorConfig.Builder() - .withLoadTimeoutDelay(new Duration(1)) - .withCoordinatorKillMaxSegments(10) - .withCoordinatorKillIgnoreDurationToRetain(false) - .build() ); loadQueuePeon.start(); @@ -318,15 +307,10 @@ public void testFailAssignForLoadDropTimeout() throws Exception LOAD_QUEUE_PATH, jsonMapper, Execs.scheduledSingleThreaded("test_load_queue_peon_scheduled-%d"), - Execs.singleThreaded("test_load_queue_peon-%d"), + Execs.singleThreaded("test_load_queue_peon-%d") // The timeout here was set to 1ms, when this test was acting flakey. A cursory glance makes me wonder if // there's a race where the timeout actually happens before other code can run. 1ms timeout seems aggressive. // 100ms is a great price to pay if it removes the flakeyness - new TestDruidCoordinatorConfig.Builder() - .withLoadTimeoutDelay(new Duration(100)) - .withCoordinatorKillMaxSegments(10) - .withCoordinatorKillIgnoreDurationToRetain(false) - .build() ); loadQueuePeon.start(); diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBuilder.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBuilder.java index f341ebf5df99..ef567d7cfcc8 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBuilder.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBuilder.java @@ -42,7 +42,6 @@ import org.apache.druid.server.coordinator.DruidCoordinator; import org.apache.druid.server.coordinator.DruidCoordinatorConfig; import org.apache.druid.server.coordinator.MetadataManager; -import org.apache.druid.server.coordinator.TestDruidCoordinatorConfig; import org.apache.druid.server.coordinator.balancer.BalancerStrategyFactory; import org.apache.druid.server.coordinator.balancer.CachingCostBalancerStrategyConfig; import org.apache.druid.server.coordinator.balancer.CachingCostBalancerStrategyFactory; @@ -51,6 +50,10 @@ import org.apache.druid.server.coordinator.balancer.RandomBalancerStrategyFactory; import org.apache.druid.server.coordinator.compact.CompactionSegmentSearchPolicy; import org.apache.druid.server.coordinator.compact.NewestSegmentFirstPolicy; +import org.apache.druid.server.coordinator.config.CoordinatorKillConfigs; +import org.apache.druid.server.coordinator.config.CoordinatorPeriodConfig; +import org.apache.druid.server.coordinator.config.CoordinatorRunConfig; +import org.apache.druid.server.coordinator.config.HttpLoadQueuePeonConfig; import org.apache.druid.server.coordinator.duty.CoordinatorCustomDutyGroups; import org.apache.druid.server.coordinator.loading.LoadQueueTaskMaster; import org.apache.druid.server.coordinator.loading.SegmentLoadQueueManager; @@ -181,7 +184,8 @@ public CoordinatorSimulation build() serverInventoryView, dynamicConfig, loadImmediately, - autoSyncInventory + autoSyncInventory, + balancerStrategy ); if (segments != null) { @@ -205,7 +209,6 @@ public CoordinatorSimulation build() new ServiceAnnouncer.Noop(), null, new CoordinatorCustomDutyGroups(Collections.emptySet()), - createBalancerStrategy(env), env.lookupCoordinatorManager, env.leaderSelector, COMPACTION_SEGMENT_SEARCH_POLICY @@ -214,40 +217,6 @@ public CoordinatorSimulation build() return new SimulationImpl(coordinator, env); } - private BalancerStrategyFactory createBalancerStrategy(Environment env) - { - if (balancerStrategy == null) { - return new CostBalancerStrategyFactory(); - } - - switch (balancerStrategy) { - case "cost": - return new CostBalancerStrategyFactory(); - case "cachingCost": - return buildCachingCostBalancerStrategy(env); - case "diskNormalized": - return new DiskNormalizedCostBalancerStrategyFactory(); - case "random": - return new RandomBalancerStrategyFactory(); - default: - throw new IAE("Unknown balancer stratgy: " + balancerStrategy); - } - } - - private BalancerStrategyFactory buildCachingCostBalancerStrategy(Environment env) - { - try { - return new CachingCostBalancerStrategyFactory( - env.coordinatorInventoryView, - env.lifecycle, - new CachingCostBalancerStrategyConfig() - ); - } - catch (Exception e) { - throw new ISE(e, "Error building balancer strategy"); - } - } - /** * Implementation of {@link CoordinatorSimulation}. */ @@ -452,21 +421,13 @@ private Environment( TestServerInventoryView clusterInventory, CoordinatorDynamicConfig dynamicConfig, boolean loadImmediately, - boolean autoSyncInventory + boolean autoSyncInventory, + String balancerStrategy ) { this.inventory = clusterInventory; this.loadImmediately = loadImmediately; this.autoSyncInventory = autoSyncInventory; - - this.coordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withCoordinatorStartDelay(new Duration(1L)) - .withCoordinatorPeriod(Duration.standardMinutes(1)) - .withCoordinatorKillPeriod(Duration.millis(100)) - .withLoadQueuePeonType("http") - .withCoordinatorKillIgnoreDurationToRetain(false) - .build(); - this.executorFactory = new ExecutorFactory(loadImmediately); this.coordinatorInventoryView = autoSyncInventory ? clusterInventory @@ -477,14 +438,19 @@ private Environment( executorFactory.create(1, ExecutorFactory.HISTORICAL_LOADER) ); + this.coordinatorConfig = new DruidCoordinatorConfig( + new CoordinatorRunConfig(new Duration(1L), Duration.standardMinutes(1)), + new CoordinatorPeriodConfig(null, null), + CoordinatorKillConfigs.STANDARD, + createBalancerStrategy(balancerStrategy), + new HttpLoadQueuePeonConfig(null, null, null) + ); this.loadQueueTaskMaster = new LoadQueueTaskMaster( - null, OBJECT_MAPPER, executorFactory.create(1, ExecutorFactory.LOAD_QUEUE_EXECUTOR), executorFactory.create(1, ExecutorFactory.LOAD_CALLBACK_EXECUTOR), - coordinatorConfig, - httpClient, - null + coordinatorConfig.getHttpLoadQueuePeonConfig(), + httpClient ); this.loadQueueManager = new SegmentLoadQueueManager(coordinatorInventoryView, loadQueueTaskMaster); @@ -550,6 +516,40 @@ private JacksonConfigManager mockConfigManager() return jacksonConfigManager; } + + private BalancerStrategyFactory createBalancerStrategy(String strategyName) + { + if (strategyName == null) { + return new CostBalancerStrategyFactory(); + } + + switch (strategyName) { + case "cost": + return new CostBalancerStrategyFactory(); + case "cachingCost": + return buildCachingCostBalancerStrategy(); + case "diskNormalized": + return new DiskNormalizedCostBalancerStrategyFactory(); + case "random": + return new RandomBalancerStrategyFactory(); + default: + throw new IAE("Unknown balancer stratgy: " + strategyName); + } + } + + private BalancerStrategyFactory buildCachingCostBalancerStrategy() + { + try { + return new CachingCostBalancerStrategyFactory( + this.coordinatorInventoryView, + this.lifecycle, + new CachingCostBalancerStrategyConfig() + ); + } + catch (Exception e) { + throw new ISE(e, "Error building balancer strategy"); + } + } } /** diff --git a/services/src/main/java/org/apache/druid/cli/CliCoordinator.java b/services/src/main/java/org/apache/druid/cli/CliCoordinator.java index c6b9e27238c0..c664a584de63 100644 --- a/services/src/main/java/org/apache/druid/cli/CliCoordinator.java +++ b/services/src/main/java/org/apache/druid/cli/CliCoordinator.java @@ -35,7 +35,6 @@ import com.google.inject.multibindings.MapBinder; import com.google.inject.name.Names; import com.google.inject.util.Providers; -import org.apache.curator.framework.CuratorFramework; import org.apache.druid.client.CoordinatorSegmentWatcherConfig; import org.apache.druid.client.CoordinatorServerView; import org.apache.druid.client.DirectDruidClientFactory; @@ -44,7 +43,6 @@ import org.apache.druid.client.coordinator.Coordinator; import org.apache.druid.discovery.NodeRole; import org.apache.druid.error.DruidException; -import org.apache.druid.guice.ConfigProvider; import org.apache.druid.guice.DruidBinders; import org.apache.druid.guice.Jerseys; import org.apache.druid.guice.JsonConfigProvider; @@ -107,6 +105,10 @@ import org.apache.druid.server.coordinator.balancer.CachingCostBalancerStrategyConfig; import org.apache.druid.server.coordinator.compact.CompactionSegmentSearchPolicy; import org.apache.druid.server.coordinator.compact.NewestSegmentFirstPolicy; +import org.apache.druid.server.coordinator.config.CoordinatorKillConfigs; +import org.apache.druid.server.coordinator.config.CoordinatorPeriodConfig; +import org.apache.druid.server.coordinator.config.CoordinatorRunConfig; +import org.apache.druid.server.coordinator.config.HttpLoadQueuePeonConfig; import org.apache.druid.server.coordinator.duty.CoordinatorCustomDuty; import org.apache.druid.server.coordinator.duty.CoordinatorCustomDutyGroup; import org.apache.druid.server.coordinator.duty.CoordinatorCustomDutyGroups; @@ -127,7 +129,6 @@ import org.apache.druid.server.http.SelfDiscoveryResource; import org.apache.druid.server.http.ServersResource; import org.apache.druid.server.http.TiersResource; -import org.apache.druid.server.initialization.ZkPathsConfig; import org.apache.druid.server.initialization.jetty.JettyServerInitializer; import org.apache.druid.server.lookup.cache.LookupCoordinatorManager; import org.apache.druid.server.lookup.cache.LookupCoordinatorManagerConfig; @@ -230,13 +231,15 @@ public void configure(Binder binder) binder.bindConstant().annotatedWith(Names.named("servicePort")).to(8081); binder.bindConstant().annotatedWith(Names.named("tlsServicePort")).to(8281); - ConfigProvider.bind(binder, DruidCoordinatorConfig.class); - binder.bind(MetadataStorage.class).toProvider(MetadataStorageProvider.class); JsonConfigProvider.bind(binder, SegmentsMetadataManagerConfig.CONFIG_PREFIX, SegmentsMetadataManagerConfig.class); JsonConfigProvider.bind(binder, "druid.manager.rules", MetadataRuleManagerConfig.class); JsonConfigProvider.bind(binder, "druid.manager.lookups", LookupCoordinatorManagerConfig.class); + JsonConfigProvider.bind(binder, "druid.coordinator", CoordinatorRunConfig.class); + JsonConfigProvider.bind(binder, "druid.coorinator.kill", CoordinatorKillConfigs.class); + JsonConfigProvider.bind(binder, "druid.coordinator.period", CoordinatorPeriodConfig.class); + JsonConfigProvider.bind(binder, "druid.coordinator.loadqueuepeon.http", HttpLoadQueuePeonConfig.class); JsonConfigProvider.bind(binder, "druid.coordinator.balancer", BalancerStrategyFactory.class); JsonConfigProvider.bind(binder, "druid.coordinator.segment", CoordinatorSegmentWatcherConfig.class); JsonConfigProvider.bind( @@ -245,6 +248,7 @@ public void configure(Binder binder) CachingCostBalancerStrategyConfig.class ); JsonConfigProvider.bind(binder, "druid.coordinator.segmentMetadataCache", SegmentMetadataCacheConfig.class); + binder.bind(DruidCoordinatorConfig.class); binder.bind(RedirectFilter.class).in(LazySingleton.class); if (beOverlord) { @@ -338,34 +342,21 @@ public void configure(Binder binder) @Provides @LazySingleton public LoadQueueTaskMaster getLoadQueueTaskMaster( - Provider curatorFrameworkProvider, ObjectMapper jsonMapper, ScheduledExecutorFactory factory, DruidCoordinatorConfig config, @EscalatedGlobal HttpClient httpClient, - ZkPathsConfig zkPaths, Lifecycle lifecycle ) { - boolean useHttpLoadQueuePeon = "http".equalsIgnoreCase(config.getLoadQueuePeonType()); - ExecutorService callBackExec; - if (useHttpLoadQueuePeon) { - callBackExec = Execs.singleThreaded("LoadQueuePeon-callbackexec--%d"); - } else { - callBackExec = Execs.multiThreaded( - config.getNumCuratorCallBackThreads(), - "LoadQueuePeon-callbackexec--%d" - ); - } + final ExecutorService callBackExec = Execs.singleThreaded("LoadQueuePeon-callbackexec--%d"); ExecutorServices.manageLifecycle(lifecycle, callBackExec); return new LoadQueueTaskMaster( - curatorFrameworkProvider, jsonMapper, factory.create(1, "Master-PeonExec--%d"), callBackExec, - config, - httpClient, - zkPaths + config.getHttpLoadQueuePeonConfig(), + httpClient ); } } From 34e3bd4e5798078baa91a6bc8e4925c6f68bcabd Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Wed, 17 Jan 2024 17:09:40 +0530 Subject: [PATCH 02/17] Remove old config class --- .../DruidCoordinatorConfigBackup.java | 166 ------------------ 1 file changed, 166 deletions(-) delete mode 100644 server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigBackup.java diff --git a/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigBackup.java b/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigBackup.java deleted file mode 100644 index fb2729d6ca3a..000000000000 --- a/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigBackup.java +++ /dev/null @@ -1,166 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.apache.druid.server.coordinator; - -import org.joda.time.Duration; -import org.skife.config.Config; -import org.skife.config.Default; - -/** - */ -public abstract class DruidCoordinatorConfigBackup -{ - @Config("druid.coordinator.startDelay") - @Default("PT300s") - public abstract Duration getCoordinatorStartDelay(); - - @Config("druid.coordinator.period") - @Default("PT60s") - public abstract Duration getCoordinatorPeriod(); - - @Config("druid.coordinator.period.indexingPeriod") - @Default("PT1800s") - public abstract Duration getCoordinatorIndexingPeriod(); - - @Config("druid.coordinator.period.metadataStoreManagementPeriod") - @Default("PT1H") - public abstract Duration getCoordinatorMetadataStoreManagementPeriod(); - - @Config("druid.coordinator.kill.on") - @Default("false") - public abstract boolean isKillUnusedSegmentsEnabled(); - - @Config("druid.coordinator.kill.period") - @Default("P1D") - public abstract Duration getCoordinatorKillPeriod(); - - @Config("druid.coordinator.kill.durationToRetain") - @Default("P90D") - public abstract Duration getCoordinatorKillDurationToRetain(); - - @Config("druid.coordinator.kill.ignoreDurationToRetain") - @Default("false") - public abstract boolean getCoordinatorKillIgnoreDurationToRetain(); - - @Config("druid.coordinator.kill.bufferPeriod") - @Default("P30D") - public abstract Duration getCoordinatorKillBufferPeriod(); - - @Config("druid.coordinator.kill.maxSegments") - @Default("100") - public abstract int getCoordinatorKillMaxSegments(); - - @Config("druid.coordinator.kill.pendingSegments.on") - @Default("true") - public abstract boolean isKillPendingSegmentsEnabled(); - - @Config("druid.coordinator.kill.supervisor.on") - @Default("true") - public abstract boolean isSupervisorKillEnabled(); - - @Config("druid.coordinator.kill.supervisor.period") - @Default("P1D") - public abstract Duration getCoordinatorSupervisorKillPeriod(); - - @Config("druid.coordinator.kill.supervisor.durationToRetain") - @Default("P90D") - public abstract Duration getCoordinatorSupervisorKillDurationToRetain(); - - @Config("druid.coordinator.kill.audit.on") - @Default("true") - public abstract boolean isAuditKillEnabled(); - - @Config("druid.coordinator.kill.audit.period") - @Default("P1D") - public abstract Duration getCoordinatorAuditKillPeriod(); - - @Config("druid.coordinator.kill.audit.durationToRetain") - @Default("P90D") - public abstract Duration getCoordinatorAuditKillDurationToRetain(); - - @Config("druid.coordinator.kill.compaction.on") - @Default("false") - public abstract boolean isCompactionKillEnabled(); - - @Config("druid.coordinator.kill.compaction.period") - @Default("P1D") - public abstract Duration getCoordinatorCompactionKillPeriod(); - - @Config("druid.coordinator.kill.rule.on") - @Default("true") - public abstract boolean isRuleKillEnabled(); - - @Config("druid.coordinator.kill.rule.period") - @Default("P1D") - public abstract Duration getCoordinatorRuleKillPeriod(); - - @Config("druid.coordinator.kill.rule.durationToRetain") - @Default("P90D") - public abstract Duration getCoordinatorRuleKillDurationToRetain(); - - @Config("druid.coordinator.kill.datasource.on") - @Default("true") - public abstract boolean isDatasourceKillEnabled(); - - @Config("druid.coordinator.kill.datasource.period") - @Default("P1D") - public abstract Duration getCoordinatorDatasourceKillPeriod(); - - @Config("druid.coordinator.kill.datasource.durationToRetain") - @Default("P90D") - public abstract Duration getCoordinatorDatasourceKillDurationToRetain(); - - @Config("druid.coordinator.load.timeout") - public Duration getLoadTimeoutDelay() - { - return new Duration(15 * 60 * 1000); - } - - @Config("druid.coordinator.loadqueuepeon.type") - public String getLoadQueuePeonType() - { - return "http"; - } - - @Config("druid.coordinator.curator.loadqueuepeon.numCallbackThreads") - public int getNumCuratorCallBackThreads() - { - return 2; - } - - @Config("druid.coordinator.loadqueuepeon.http.repeatDelay") - public Duration getHttpLoadQueuePeonRepeatDelay() - { - return Duration.millis(60000); - } - - @Config("druid.coordinator.loadqueuepeon.http.hostTimeout") - public Duration getHttpLoadQueuePeonHostTimeout() - { - return Duration.millis(300000); - } - - @Config("druid.coordinator.loadqueuepeon.http.batchSize") - public int getHttpLoadQueuePeonBatchSize() - { - return 1; - } - -} From 7db4bc4f01f3db299ff52ca9f5dddf067ece114b Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Wed, 17 Jan 2024 17:54:38 +0530 Subject: [PATCH 03/17] Address static checks --- .../druid/server/coordinator/loading/HttpLoadQueuePeon.java | 3 +-- services/pom.xml | 4 ---- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/coordinator/loading/HttpLoadQueuePeon.java b/server/src/main/java/org/apache/druid/server/coordinator/loading/HttpLoadQueuePeon.java index ac01f0d29d62..4ec6f2427a33 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/loading/HttpLoadQueuePeon.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/loading/HttpLoadQueuePeon.java @@ -37,7 +37,6 @@ import org.apache.druid.server.coordination.DataSegmentChangeRequest; import org.apache.druid.server.coordination.SegmentLoadDropHandler; import org.apache.druid.server.coordinator.BytesAccumulatingResponseHandler; -import org.apache.druid.server.coordinator.DruidCoordinatorConfig; import org.apache.druid.server.coordinator.config.HttpLoadQueuePeonConfig; import org.apache.druid.server.coordinator.stats.CoordinatorRunStats; import org.apache.druid.server.coordinator.stats.CoordinatorStat; @@ -511,7 +510,7 @@ public Set getSegmentsMarkedToDrop() * A request is considered to have timed out if the time elapsed since it was * first sent to the server is greater than the configured load timeout. * - * @see DruidCoordinatorConfig#getLoadTimeoutDelay() + * @see HttpLoadQueuePeonConfig#getLoadTimeout() */ private boolean hasRequestTimedOut(SegmentHolder holder, long currentTimeMillis) { diff --git a/services/pom.xml b/services/pom.xml index c94fb0714cd7..44a926367471 100644 --- a/services/pom.xml +++ b/services/pom.xml @@ -95,10 +95,6 @@ org.eclipse.jetty jetty-server - - org.apache.curator - curator-framework - org.apache.calcite.avatica avatica-core From 748d37b9ab97751b54b955712c6700eb9b025d64 Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Wed, 17 Jan 2024 19:02:35 +0530 Subject: [PATCH 04/17] Fix tests --- .../config/HttpLoadQueuePeonConfig.java | 2 +- .../config/MetadataCleanupConfig.java | 2 +- .../loading/CuratorLoadQueuePeon.java | 9 ++++-- .../CuratorDruidCoordinatorTest.java | 7 +++-- .../DruidCoordinatorConfigTest.java | 8 +++++ .../coordinator/DruidCoordinatorTest.java | 19 ++++++++---- .../duty/KillSupervisorsCustomDutyTest.java | 29 ------------------- .../loading/LoadQueuePeonTest.java | 12 +++++--- 8 files changed, 42 insertions(+), 46 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/coordinator/config/HttpLoadQueuePeonConfig.java b/server/src/main/java/org/apache/druid/server/coordinator/config/HttpLoadQueuePeonConfig.java index 06f26d8f44a4..f6f402037d1f 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/config/HttpLoadQueuePeonConfig.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/config/HttpLoadQueuePeonConfig.java @@ -26,7 +26,7 @@ public class HttpLoadQueuePeonConfig { - public static final Duration DEFAULT_LOAD_TIMEOUT = Duration.standardMinutes(15); + private static final Duration DEFAULT_LOAD_TIMEOUT = Duration.standardMinutes(15); @JsonProperty private final Duration hostTimeout; diff --git a/server/src/main/java/org/apache/druid/server/coordinator/config/MetadataCleanupConfig.java b/server/src/main/java/org/apache/druid/server/coordinator/config/MetadataCleanupConfig.java index 0793474b0751..a7686e22d15b 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/config/MetadataCleanupConfig.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/config/MetadataCleanupConfig.java @@ -28,7 +28,7 @@ public class MetadataCleanupConfig { - public static MetadataCleanupConfig STANDARD = new MetadataCleanupConfig(null, null, null); + public static final MetadataCleanupConfig STANDARD = new MetadataCleanupConfig(null, null, null); @JsonProperty("on") private final boolean cleanupEnabled; diff --git a/server/src/main/java/org/apache/druid/server/coordinator/loading/CuratorLoadQueuePeon.java b/server/src/main/java/org/apache/druid/server/coordinator/loading/CuratorLoadQueuePeon.java index 70996ccaae82..c79ed146e08f 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/loading/CuratorLoadQueuePeon.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/loading/CuratorLoadQueuePeon.java @@ -28,13 +28,13 @@ import org.apache.druid.java.util.emitter.EmittingLogger; import org.apache.druid.server.coordination.SegmentChangeRequestNoop; import org.apache.druid.server.coordinator.DruidCoordinator; -import org.apache.druid.server.coordinator.config.HttpLoadQueuePeonConfig; import org.apache.druid.server.coordinator.stats.CoordinatorRunStats; import org.apache.druid.server.coordinator.stats.Stats; import org.apache.druid.timeline.DataSegment; import org.apache.zookeeper.CreateMode; import org.apache.zookeeper.KeeperException; import org.apache.zookeeper.data.Stat; +import org.joda.time.Duration; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -72,6 +72,7 @@ public class CuratorLoadQueuePeon implements LoadQueuePeon * with loading or dropping segments. */ private final ExecutorService callBackExecutor; + private final Duration loadTimeout; private final AtomicLong queuedSize = new AtomicLong(0); private final CoordinatorRunStats stats = new CoordinatorRunStats(); @@ -114,7 +115,8 @@ public CuratorLoadQueuePeon( String basePath, ObjectMapper jsonMapper, ScheduledExecutorService processingExecutor, - ExecutorService callbackExecutor + ExecutorService callbackExecutor, + Duration loadTimeout ) { this.curator = curator; @@ -122,6 +124,7 @@ public CuratorLoadQueuePeon( this.jsonMapper = jsonMapper; this.callBackExecutor = callbackExecutor; this.processingExecutor = processingExecutor; + this.loadTimeout = loadTimeout; } @JsonProperty @@ -306,7 +309,7 @@ private ScheduledFuture scheduleNodeDeletedCheck(String path) failAssign(segmentHolder, false, e); } }, - HttpLoadQueuePeonConfig.DEFAULT_LOAD_TIMEOUT.getMillis(), + loadTimeout.getMillis(), TimeUnit.MILLISECONDS ); } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/CuratorDruidCoordinatorTest.java b/server/src/test/java/org/apache/druid/server/coordinator/CuratorDruidCoordinatorTest.java index 26cccff59117..c312cb493b59 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/CuratorDruidCoordinatorTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/CuratorDruidCoordinatorTest.java @@ -58,6 +58,7 @@ import org.apache.druid.timeline.SegmentId; import org.apache.druid.timeline.partition.NoneShardSpec; import org.easymock.EasyMock; +import org.joda.time.Duration; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -149,14 +150,16 @@ public void setUp() throws Exception SOURCE_LOAD_PATH, objectMapper, peonExec, - callbackExec + callbackExec, + Duration.standardMinutes(15) ); destinationLoadQueuePeon = new CuratorLoadQueuePeon( curator, DESTINATION_LOAD_PATH, objectMapper, peonExec, - callbackExec + callbackExec, + Duration.standardMinutes(15) ); } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java b/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java index 7c0d1a0226e5..ae8e7855d19f 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java @@ -346,6 +346,14 @@ public void testCoordinatorConfigFailsWhenRetainDurationIsHigherThanCurrentTime( ); } + @Test + public void testSomeBasicStuff() + { + Duration millis = Duration.millis(1); + Duration days = Duration.standardDays(1); + System.out.println(millis.getMillis() + ", " + days); + } + private KillConfigBuilder killConfig() { return new KillConfigBuilder(); 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 11767be2ae30..b521c41f009e 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 @@ -90,6 +90,7 @@ public class DruidCoordinatorTest extends CuratorTestBase { private static final String LOADPATH = "/druid/loadqueue/localhost:1234"; + private static final Duration LOAD_TIMEOUT = Duration.standardMinutes(15); private static final long COORDINATOR_START_DELAY = 1; private static final long COORDINATOR_PERIOD = 100; @@ -163,7 +164,8 @@ public void setUp() throws Exception LOADPATH, objectMapper, Execs.scheduledSingleThreaded("coordinator_test_load_queue_peon_scheduled-%d"), - Execs.singleThreaded("coordinator_test_load_queue_peon-%d") + Execs.singleThreaded("coordinator_test_load_queue_peon-%d"), + LOAD_TIMEOUT ); loadQueuePeon.start(); druidNode = new DruidNode("hey", "what", false, 1234, null, true, false); @@ -353,7 +355,8 @@ public void testCoordinatorTieredRun() throws Exception loadPathCold, objectMapper, Execs.scheduledSingleThreaded("coordinator_test_load_queue_peon_cold_scheduled-%d"), - Execs.singleThreaded("coordinator_test_load_queue_peon_cold-%d") + Execs.singleThreaded("coordinator_test_load_queue_peon_cold-%d"), + LOAD_TIMEOUT ); final PathChildrenCache pathChildrenCacheCold = new PathChildrenCache( curator, @@ -451,7 +454,8 @@ public void testComputeUnderReplicationCountsPerDataSourcePerTierForSegmentsWith loadPathCold, objectMapper, Execs.scheduledSingleThreaded("coordinator_test_load_queue_peon_cold_scheduled-%d"), - Execs.singleThreaded("coordinator_test_load_queue_peon_cold-%d") + Execs.singleThreaded("coordinator_test_load_queue_peon_cold-%d"), + LOAD_TIMEOUT ); final LoadQueuePeon loadQueuePeonBroker1 = new CuratorLoadQueuePeon( @@ -459,7 +463,8 @@ public void testComputeUnderReplicationCountsPerDataSourcePerTierForSegmentsWith loadPathBroker1, objectMapper, Execs.scheduledSingleThreaded("coordinator_test_load_queue_peon_broker1_scheduled-%d"), - Execs.singleThreaded("coordinator_test_load_queue_peon_broker1-%d") + Execs.singleThreaded("coordinator_test_load_queue_peon_broker1-%d"), + LOAD_TIMEOUT ); final LoadQueuePeon loadQueuePeonBroker2 = new CuratorLoadQueuePeon( @@ -467,7 +472,8 @@ public void testComputeUnderReplicationCountsPerDataSourcePerTierForSegmentsWith loadPathBroker2, objectMapper, Execs.scheduledSingleThreaded("coordinator_test_load_queue_peon_broker2_scheduled-%d"), - Execs.singleThreaded("coordinator_test_load_queue_peon_broker2-%d") + Execs.singleThreaded("coordinator_test_load_queue_peon_broker2-%d"), + LOAD_TIMEOUT ); final LoadQueuePeon loadQueuePeonPoenServer = new CuratorLoadQueuePeon( @@ -475,7 +481,8 @@ public void testComputeUnderReplicationCountsPerDataSourcePerTierForSegmentsWith loadPathPeon, objectMapper, Execs.scheduledSingleThreaded("coordinator_test_load_queue_peon_peon_scheduled-%d"), - Execs.singleThreaded("coordinator_test_load_queue_peon_peon-%d") + Execs.singleThreaded("coordinator_test_load_queue_peon_peon-%d"), + LOAD_TIMEOUT ); final PathChildrenCache pathChildrenCacheCold = new PathChildrenCache( curator, diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/KillSupervisorsCustomDutyTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/KillSupervisorsCustomDutyTest.java index 4057b894589b..9c8bfacb8117 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/KillSupervisorsCustomDutyTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/KillSupervisorsCustomDutyTest.java @@ -55,35 +55,6 @@ public void setup() .thenReturn(new Duration(3600 * 1000)); } - @Test - public void testConstructorFailIfRetainDurationNull() - { - final IllegalArgumentException exception = Assert.assertThrows( - IllegalArgumentException.class, - () -> killSupervisors = new KillSupervisorsCustomDuty(null, mockMetadataSupervisorManager) - ); - Assert.assertEquals( - "[KillSupervisorsCustomDuty.durationToRetain] must be 0 milliseconds or higher", - exception.getMessage() - ); - } - - @Test - public void testConstructorFailIfRetainDurationInvalid() - { - final IllegalArgumentException exception = Assert.assertThrows( - IllegalArgumentException.class, - () -> killSupervisors = new KillSupervisorsCustomDuty( - new Duration("PT-1S"), - mockMetadataSupervisorManager - ) - ); - Assert.assertEquals( - "[KillSupervisorsCustomDuty.durationToRetain] must be 0 milliseconds or higher", - exception.getMessage() - ); - } - @Test public void testConstructorSuccess() { diff --git a/server/src/test/java/org/apache/druid/server/coordinator/loading/LoadQueuePeonTest.java b/server/src/test/java/org/apache/druid/server/coordinator/loading/LoadQueuePeonTest.java index 6717a44bfc3c..3e1c816716c9 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/loading/LoadQueuePeonTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/loading/LoadQueuePeonTest.java @@ -41,6 +41,7 @@ import org.apache.druid.timeline.DataSegment; import org.apache.druid.timeline.SegmentId; import org.apache.druid.timeline.partition.NoneShardSpec; +import org.joda.time.Duration; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -86,7 +87,8 @@ public void testMultipleLoadDropSegments() throws Exception LOAD_QUEUE_PATH, jsonMapper, Execs.scheduledSingleThreaded("test_load_queue_peon_scheduled-%d"), - Execs.singleThreaded("test_load_queue_peon-%d") + Execs.singleThreaded("test_load_queue_peon-%d"), + Duration.standardMinutes(15) ); loadQueuePeon.start(); @@ -271,8 +273,9 @@ public void testFailAssignForNonTimeoutFailures() throws Exception // This will fail inside SegmentChangeProcessor.run() null, Execs.scheduledSingleThreaded("test_load_queue_peon_scheduled-%d"), - Execs.singleThreaded("test_load_queue_peon-%d") + Execs.singleThreaded("test_load_queue_peon-%d"), // set time-out to 1 ms so that LoadQueuePeon will fail the assignment quickly + new Duration(1) ); loadQueuePeon.start(); @@ -307,10 +310,11 @@ public void testFailAssignForLoadDropTimeout() throws Exception LOAD_QUEUE_PATH, jsonMapper, Execs.scheduledSingleThreaded("test_load_queue_peon_scheduled-%d"), - Execs.singleThreaded("test_load_queue_peon-%d") + Execs.singleThreaded("test_load_queue_peon-%d"), // The timeout here was set to 1ms, when this test was acting flakey. A cursory glance makes me wonder if // there's a race where the timeout actually happens before other code can run. 1ms timeout seems aggressive. - // 100ms is a great price to pay if it removes the flakeyness + // 100ms is a great price to pay if it removes the flakeyness, + new Duration(100) ); loadQueuePeon.start(); From 550d0bbae5904583b0303e70248b4c2ff02d156c Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Wed, 17 Jan 2024 21:00:26 +0530 Subject: [PATCH 05/17] Remove unnecessary mocks --- .../duty/KillSupervisorsCustomDutyTest.java | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/KillSupervisorsCustomDutyTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/KillSupervisorsCustomDutyTest.java index 9c8bfacb8117..6dd296d3b517 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/KillSupervisorsCustomDutyTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/KillSupervisorsCustomDutyTest.java @@ -20,13 +20,11 @@ package org.apache.druid.server.coordinator.duty; import org.apache.druid.metadata.MetadataSupervisorManager; -import org.apache.druid.server.coordinator.DruidCoordinatorConfig; import org.apache.druid.server.coordinator.DruidCoordinatorRuntimeParams; import org.apache.druid.server.coordinator.stats.CoordinatorRunStats; import org.apache.druid.server.coordinator.stats.Stats; import org.joda.time.Duration; import org.junit.Assert; -import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentMatchers; @@ -43,18 +41,8 @@ public class KillSupervisorsCustomDutyTest @Mock private DruidCoordinatorRuntimeParams mockDruidCoordinatorRuntimeParams; - @Mock - private DruidCoordinatorConfig coordinatorConfig; - private KillSupervisorsCustomDuty killSupervisors; - @Before - public void setup() - { - Mockito.when(coordinatorConfig.getCoordinatorMetadataStoreManagementPeriod()) - .thenReturn(new Duration(3600 * 1000)); - } - @Test public void testConstructorSuccess() { From a85ec53e1bdbc83806b982323711f8ed7e1a3b30 Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Thu, 18 Jan 2024 10:59:32 +0530 Subject: [PATCH 06/17] Fix config typos --- .../server/coordinator/config/CoordinatorKillConfigs.java | 4 ++-- .../src/main/java/org/apache/druid/cli/CliCoordinator.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorKillConfigs.java b/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorKillConfigs.java index 5713c4de37f1..a558a93d34a7 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorKillConfigs.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorKillConfigs.java @@ -60,7 +60,7 @@ public class CoordinatorKillConfigs @JsonProperty("durationToRetain") private final Duration killUnusedDurationToRetain; - @JsonProperty("ignoredDurationToRetain") + @JsonProperty("ignoreDurationToRetain") private final Boolean killUnusedIgnoreDurationToRetain; @JsonProperty("bufferPeriod") @@ -81,7 +81,7 @@ public CoordinatorKillConfigs( @JsonProperty("on") Boolean killUnusedEnabled, @JsonProperty("period") Duration killUnusedPeriod, @JsonProperty("durationToRetain") Duration killUnusedDurationToRetain, - @JsonProperty("ignoredDurationToRetain") Boolean killUnusedIgnoreDurationToRetain, + @JsonProperty("ignoreDurationToRetain") Boolean killUnusedIgnoreDurationToRetain, @JsonProperty("bufferPeriod") Duration killUnusedBufferPeriod, @JsonProperty("maxSegments") Integer killUnusedMaxSegments ) diff --git a/services/src/main/java/org/apache/druid/cli/CliCoordinator.java b/services/src/main/java/org/apache/druid/cli/CliCoordinator.java index c664a584de63..d5cbf55e93c7 100644 --- a/services/src/main/java/org/apache/druid/cli/CliCoordinator.java +++ b/services/src/main/java/org/apache/druid/cli/CliCoordinator.java @@ -237,7 +237,7 @@ public void configure(Binder binder) JsonConfigProvider.bind(binder, "druid.manager.rules", MetadataRuleManagerConfig.class); JsonConfigProvider.bind(binder, "druid.manager.lookups", LookupCoordinatorManagerConfig.class); JsonConfigProvider.bind(binder, "druid.coordinator", CoordinatorRunConfig.class); - JsonConfigProvider.bind(binder, "druid.coorinator.kill", CoordinatorKillConfigs.class); + JsonConfigProvider.bind(binder, "druid.coordinator.kill", CoordinatorKillConfigs.class); JsonConfigProvider.bind(binder, "druid.coordinator.period", CoordinatorPeriodConfig.class); JsonConfigProvider.bind(binder, "druid.coordinator.loadqueuepeon.http", HttpLoadQueuePeonConfig.class); JsonConfigProvider.bind(binder, "druid.coordinator.balancer", BalancerStrategyFactory.class); From 06b97cb2e9080d6f7ded1ae708b14c1387b3680c Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Tue, 13 Feb 2024 14:18:43 +0530 Subject: [PATCH 07/17] Fix config condition --- .../apache/druid/server/coordinator/DruidCoordinatorConfig.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinatorConfig.java b/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinatorConfig.java index 6be8290c50d0..6467c1471fd9 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinatorConfig.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinatorConfig.java @@ -104,7 +104,7 @@ private void validateKillConfigs() // Validate config for killing unused segments final KillUnusedSegmentsConfig killUnusedConfig = killConfigs.unusedSegments(); - if (killUnusedConfig.getCleanupPeriod().getMillis() > getCoordinatorIndexingPeriod().getMillis()) { + if (killUnusedConfig.getCleanupPeriod().getMillis() <= getCoordinatorIndexingPeriod().getMillis()) { throw InvalidInput.exception( "'druid.coordinator.kill.period'[%s] must be greater than or equal to" + " 'druid.coordinator.period.indexingPeriod'[%s]", From 62f8920bc143d175daa21607b15376c14cea0474 Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Fri, 19 Apr 2024 10:17:22 +0530 Subject: [PATCH 08/17] Fix test, spotbug check --- .../config/KillUnusedSegmentsConfig.java | 3 +- .../duty/KillUnusedSegmentsTest.java | 36 ++----------------- 2 files changed, 3 insertions(+), 36 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/coordinator/config/KillUnusedSegmentsConfig.java b/server/src/main/java/org/apache/druid/server/coordinator/config/KillUnusedSegmentsConfig.java index f2294f45c78a..d8ed6fe17d07 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/config/KillUnusedSegmentsConfig.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/config/KillUnusedSegmentsConfig.java @@ -74,7 +74,6 @@ public static Builder builder() public static class Builder { - private Boolean cleanupEnabled; private Duration durationToRetain; private Duration cleanupPeriod; private Boolean ignoreDurationToRetain; @@ -89,7 +88,7 @@ private Builder() public KillUnusedSegmentsConfig build() { return new KillUnusedSegmentsConfig( - cleanupEnabled, + true, cleanupPeriod, durationToRetain, ignoreDurationToRetain, diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/KillUnusedSegmentsTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/KillUnusedSegmentsTest.java index 55e6f75c8a38..09906454f40d 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/KillUnusedSegmentsTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/KillUnusedSegmentsTest.java @@ -59,7 +59,6 @@ import org.joda.time.Period; import org.junit.Assert; import org.junit.Before; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; @@ -523,8 +522,6 @@ public void testDefaultKillTaskSlotStats() dynamicConfigBuilder = CoordinatorDynamicConfig.builder(); paramsBuilder.withDynamicConfigs(dynamicConfigBuilder.build()); - dynamicConfigBuilder.withKillTaskSlotRatio(1.0) - .withMaxKillTaskSlots(Integer.MAX_VALUE); initDuty(); final CoordinatorRunStats stats = runDutyAndGetStats(); @@ -536,8 +533,8 @@ public void testDefaultKillTaskSlotStats() @Test public void testKillTaskSlotStats1() { - dynamicConfigBuilder.withKillTaskSlotRatio(1.0); - dynamicConfigBuilder.withMaxKillTaskSlots(Integer.MAX_VALUE); + dynamicConfigBuilder.withKillTaskSlotRatio(1.0) + .withMaxKillTaskSlots(Integer.MAX_VALUE); paramsBuilder.withDynamicConfigs(dynamicConfigBuilder.build()); initDuty(); @@ -553,8 +550,6 @@ public void testKillTaskSlotStats2() { dynamicConfigBuilder.withKillTaskSlotRatio(0.0) .withMaxKillTaskSlots(Integer.MAX_VALUE); - dynamicConfigBuilder.withKillTaskSlotRatio(0.0); - dynamicConfigBuilder.withMaxKillTaskSlots(Integer.MAX_VALUE); paramsBuilder.withDynamicConfigs(dynamicConfigBuilder.build()); initDuty(); @@ -570,8 +565,6 @@ public void testKillTaskSlotStats3() { dynamicConfigBuilder.withKillTaskSlotRatio(1.0) .withMaxKillTaskSlots(0); - dynamicConfigBuilder.withKillTaskSlotRatio(1.0); - dynamicConfigBuilder.withMaxKillTaskSlots(0); paramsBuilder.withDynamicConfigs(dynamicConfigBuilder.build()); initDuty(); @@ -587,8 +580,6 @@ public void testKillTaskSlotStats4() { dynamicConfigBuilder.withKillTaskSlotRatio(0.1) .withMaxKillTaskSlots(3); - dynamicConfigBuilder.withKillTaskSlotRatio(0.1); - dynamicConfigBuilder.withMaxKillTaskSlots(3); paramsBuilder.withDynamicConfigs(dynamicConfigBuilder.build()); initDuty(); @@ -633,17 +624,6 @@ public void testKillFirstHalfEternitySegment() validateLastKillStateAndReset(DS1, firstHalfEternity); } - /** - *

- * Regardless of {@link KillUnusedSegmentsConfig#isIgnoreDurationToRetain()} configuration, - * auto-kill doesn't delete unused segments that end at {@link DateTimes#MAX}. - * This is because the kill duty uses {@link DateTimes#COMPARE_DATE_AS_STRING_MAX} as the - * datetime string comparison for the end endpoint when retrieving unused segment intervals. - *

- * For more information, see Issue#15951. - *

- */ - @Ignore @Test public void testKillEternitySegment() { @@ -662,18 +642,6 @@ public void testKillEternitySegment() validateLastKillStateAndReset(DS1, Intervals.ETERNITY); } - /** - * Similar to {@link #testKillEternitySegment()} - *

- * Regardless of {@link KillUnusedSegmentsConfig#isIgnoreDurationToRetain()} configuration, - * auto-kill doesn't delete unused segments that end at {@link DateTimes#MAX}. - * This is because the kill duty uses {@link DateTimes#COMPARE_DATE_AS_STRING_MAX} as the - * datetime string comparison for the end endpoint when retrieving unused segment intervals. - *

- * For more information, see Issue#15951. - *

- */ - @Ignore @Test public void testKillSecondHalfEternitySegment() { From 002044621db6733354684f9ff4aa0471cd77fbf9 Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Fri, 19 Apr 2024 10:27:59 +0530 Subject: [PATCH 09/17] Move validation to DruidCoordinatorConfig --- .../coordinator/DruidCoordinatorConfig.java | 6 +++++ .../coordinator/duty/KillUnusedSegments.java | 10 -------- .../DruidCoordinatorConfigTest.java | 14 +++++++++++ .../duty/KillUnusedSegmentsTest.java | 25 ------------------- 4 files changed, 20 insertions(+), 35 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinatorConfig.java b/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinatorConfig.java index 1e40d2cbcbce..9f7cc6d414ee 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinatorConfig.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinatorConfig.java @@ -112,6 +112,12 @@ private void validateKillConfigs() killUnusedConfig.getCleanupPeriod(), getCoordinatorIndexingPeriod() ); } + if (killUnusedConfig.getMaxSegments() < 0) { + throw newInvalidInputExceptionForOperator( + "'druid.coordinator.kill.maxSegments'[%s] must be a positive integer.", + killUnusedConfig.getMaxSegments() + ); + } } private void validateKillConfig(MetadataCleanupConfig config, String propertyPrefix) diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/KillUnusedSegments.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/KillUnusedSegments.java index 172c21d0fbc3..12a926422fc5 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/KillUnusedSegments.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/KillUnusedSegments.java @@ -21,7 +21,6 @@ import com.google.common.base.Predicate; import org.apache.druid.common.guava.FutureUtils; -import org.apache.druid.error.DruidException; import org.apache.druid.indexer.TaskStatusPlus; import org.apache.druid.java.util.common.DateTimes; import org.apache.druid.java.util.common.JodaUtils; @@ -97,15 +96,6 @@ public KillUnusedSegments( this.period = killConfig.getCleanupPeriod(); this.maxSegmentsToKill = killConfig.getMaxSegments(); - if (maxSegmentsToKill < 0) { - throw DruidException.forPersona(DruidException.Persona.OPERATOR) - .ofCategory(DruidException.Category.INVALID_INPUT) - .build( - "druid.coordinator.kill.maxSegments[%s] must be a positive integer.", - maxSegmentsToKill - ); - } - this.ignoreDurationToRetain = killConfig.isIgnoreDurationToRetain(); this.durationToRetain = killConfig.getDurationToRetain(); if (this.ignoreDurationToRetain) { diff --git a/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java b/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java index e564dc0bc167..86717f783eba 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java @@ -195,6 +195,20 @@ public void testKillUnusedSegmentsPeriodLessThanIndexingPeriod() ); } + @Test + public void testKillUnusedSegmentsMaxSegmentsNegative() + { + KillUnusedSegmentsConfig killUnusedConfig + = KillUnusedSegmentsConfig.builder() + .withMaxSegmentsToKill(-5) + .build(); + verifyCoordinatorConfigFailsWith( + createKillConfig().unusedSegments(killUnusedConfig).build(), + new CoordinatorPeriodConfig(null, Duration.standardSeconds(10)), + "'druid.coordinator.kill.maxSegments'[-5] must be a positive integer." + ); + } + @Test public void testCoordinatorKillConfigDefaultValues() { diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/KillUnusedSegmentsTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/KillUnusedSegmentsTest.java index 09906454f40d..3b34d4246b94 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/KillUnusedSegmentsTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/KillUnusedSegmentsTest.java @@ -28,8 +28,6 @@ import org.apache.druid.client.indexing.IndexingWorker; import org.apache.druid.client.indexing.IndexingWorkerInfo; import org.apache.druid.client.indexing.NoopOverlordClient; -import org.apache.druid.error.DruidException; -import org.apache.druid.error.DruidExceptionMatcher; import org.apache.druid.indexer.RunnerTaskState; import org.apache.druid.indexer.TaskLocation; import org.apache.druid.indexer.TaskState; @@ -52,7 +50,6 @@ import org.apache.druid.server.coordinator.stats.Stats; import org.apache.druid.timeline.DataSegment; import org.apache.druid.timeline.partition.NoneShardSpec; -import org.hamcrest.MatcherAssert; import org.joda.time.DateTime; import org.joda.time.Duration; import org.joda.time.Interval; @@ -701,28 +698,6 @@ public void testKillMultipleSegmentsInSameInterval() validateLastKillStateAndReset(DS1, YEAR_OLD); } - @Test - public void testNegativeMaxKillSegments() - { - MatcherAssert.assertThat( - Assert.assertThrows( - DruidException.class, - () -> new KillUnusedSegments( - sqlSegmentsMetadataManager, - overlordClient, - new KillUnusedSegmentsConfig(true, null, null, null, null, -5) - ) - ), - new DruidExceptionMatcher( - DruidException.Persona.OPERATOR, - DruidException.Category.INVALID_INPUT, - "general" - ).expectMessageIs( - "druid.coordinator.kill.maxSegments[-5] must be a positive integer." - ) - ); - } - private void validateLastKillStateAndReset(final String dataSource, @Nullable final Interval expectedKillInterval) { final Interval observedLastKillInterval = overlordClient.getLastKillInterval(dataSource); From 6ce7c26fda30454f6795e979f7469b03d523b947 Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Fri, 19 Apr 2024 10:44:13 +0530 Subject: [PATCH 10/17] Move DruidCoordinatorConfig to different package --- .../server/coordinator/DruidCoordinator.java | 1 + .../{ => config}/DruidCoordinatorConfig.java | 8 +---- .../DruidCoordinatorConfigTest.java | 1 + .../coordinator/DruidCoordinatorTest.java | 1 + .../coordinator/duty/CompactSegmentsTest.java | 2 +- .../duty/KillUnusedSegmentsTest.java | 32 +++++++++---------- .../CoordinatorSimulationBuilder.java | 2 +- .../org/apache/druid/cli/CliCoordinator.java | 2 +- 8 files changed, 23 insertions(+), 26 deletions(-) rename server/src/main/java/org/apache/druid/server/coordinator/{ => config}/DruidCoordinatorConfig.java (91%) diff --git a/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java b/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java index 1b877b7fdeda..8af104e334c0 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java @@ -55,6 +55,7 @@ import org.apache.druid.server.coordinator.balancer.BalancerStrategyFactory; import org.apache.druid.server.coordinator.compact.CompactionSegmentSearchPolicy; import org.apache.druid.server.coordinator.config.CoordinatorKillConfigs; +import org.apache.druid.server.coordinator.config.DruidCoordinatorConfig; import org.apache.druid.server.coordinator.config.KillUnusedSegmentsConfig; import org.apache.druid.server.coordinator.duty.BalanceSegments; import org.apache.druid.server.coordinator.duty.CollectSegmentAndServerStats; diff --git a/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinatorConfig.java b/server/src/main/java/org/apache/druid/server/coordinator/config/DruidCoordinatorConfig.java similarity index 91% rename from server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinatorConfig.java rename to server/src/main/java/org/apache/druid/server/coordinator/config/DruidCoordinatorConfig.java index 9f7cc6d414ee..64291b1db55b 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinatorConfig.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/config/DruidCoordinatorConfig.java @@ -17,17 +17,11 @@ * under the License. */ -package org.apache.druid.server.coordinator; +package org.apache.druid.server.coordinator.config; import com.google.inject.Inject; import org.apache.druid.error.DruidException; import org.apache.druid.server.coordinator.balancer.BalancerStrategyFactory; -import org.apache.druid.server.coordinator.config.CoordinatorKillConfigs; -import org.apache.druid.server.coordinator.config.CoordinatorPeriodConfig; -import org.apache.druid.server.coordinator.config.CoordinatorRunConfig; -import org.apache.druid.server.coordinator.config.HttpLoadQueuePeonConfig; -import org.apache.druid.server.coordinator.config.KillUnusedSegmentsConfig; -import org.apache.druid.server.coordinator.config.MetadataCleanupConfig; import org.joda.time.Duration; /** diff --git a/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java b/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java index 86717f783eba..527225f3b0c3 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java @@ -30,6 +30,7 @@ import org.apache.druid.server.coordinator.config.CoordinatorKillConfigs; import org.apache.druid.server.coordinator.config.CoordinatorPeriodConfig; import org.apache.druid.server.coordinator.config.CoordinatorRunConfig; +import org.apache.druid.server.coordinator.config.DruidCoordinatorConfig; import org.apache.druid.server.coordinator.config.HttpLoadQueuePeonConfig; import org.apache.druid.server.coordinator.config.KillUnusedSegmentsConfig; import org.apache.druid.server.coordinator.config.MetadataCleanupConfig; 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 25c596e484bc..8855b6dd889d 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 @@ -55,6 +55,7 @@ import org.apache.druid.server.coordinator.config.CoordinatorKillConfigs; import org.apache.druid.server.coordinator.config.CoordinatorPeriodConfig; import org.apache.druid.server.coordinator.config.CoordinatorRunConfig; +import org.apache.druid.server.coordinator.config.DruidCoordinatorConfig; import org.apache.druid.server.coordinator.duty.CompactSegments; import org.apache.druid.server.coordinator.duty.CoordinatorCustomDuty; import org.apache.druid.server.coordinator.duty.CoordinatorCustomDutyGroup; diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/CompactSegmentsTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/CompactSegmentsTest.java index 43b1c50c9698..173e4537ccad 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/CompactSegmentsTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/CompactSegmentsTest.java @@ -69,7 +69,6 @@ import org.apache.druid.server.coordinator.AutoCompactionSnapshot; import org.apache.druid.server.coordinator.CoordinatorCompactionConfig; import org.apache.druid.server.coordinator.DataSourceCompactionConfig; -import org.apache.druid.server.coordinator.DruidCoordinatorConfig; import org.apache.druid.server.coordinator.DruidCoordinatorRuntimeParams; import org.apache.druid.server.coordinator.UserCompactionTaskDimensionsConfig; import org.apache.druid.server.coordinator.UserCompactionTaskGranularityConfig; @@ -78,6 +77,7 @@ import org.apache.druid.server.coordinator.UserCompactionTaskTransformConfig; import org.apache.druid.server.coordinator.compact.CompactionSegmentSearchPolicy; import org.apache.druid.server.coordinator.compact.NewestSegmentFirstPolicy; +import org.apache.druid.server.coordinator.config.DruidCoordinatorConfig; import org.apache.druid.server.coordinator.stats.CoordinatorRunStats; import org.apache.druid.server.coordinator.stats.Stats; import org.apache.druid.timeline.CompactionState; diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/KillUnusedSegmentsTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/KillUnusedSegmentsTest.java index 3b34d4246b94..d9aab048ac6d 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/KillUnusedSegmentsTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/KillUnusedSegmentsTest.java @@ -447,8 +447,8 @@ public void testLargeKillPeriod() @Test public void testKillTaskSlotAtCapacity() { - dynamicConfigBuilder.withKillTaskSlotRatio(0.3) - .withMaxKillTaskSlots(2); + dynamicConfigBuilder.withKillTaskSlotRatio(0.3); + dynamicConfigBuilder.withMaxKillTaskSlots(2); paramsBuilder.withDynamicConfigs(dynamicConfigBuilder.build()); createAndAddUnusedSegment(DS1, YEAR_OLD, VERSION, NOW.minusDays(1)); @@ -478,8 +478,8 @@ public void testKillTaskSlotAtCapacity() @Test public void testKillWithOverlordTaskSlotsFull() { - dynamicConfigBuilder.withKillTaskSlotRatio(0.10) - .withMaxKillTaskSlots(10); + dynamicConfigBuilder.withKillTaskSlotRatio(0.10); + dynamicConfigBuilder.withMaxKillTaskSlots(10); paramsBuilder.withDynamicConfigs(dynamicConfigBuilder.build()); overlordClient = new TestOverlordClient(1, 5); @@ -497,8 +497,8 @@ public void testKillWithOverlordTaskSlotsFull() @Test public void testKillWithOverlordTaskSlotAvailable() { - dynamicConfigBuilder.withKillTaskSlotRatio(1.0) - .withMaxKillTaskSlots(3); + dynamicConfigBuilder.withKillTaskSlotRatio(1.0); + dynamicConfigBuilder.withMaxKillTaskSlots(3); paramsBuilder.withDynamicConfigs(dynamicConfigBuilder.build()); overlordClient = new TestOverlordClient(3, 10); @@ -530,8 +530,8 @@ public void testDefaultKillTaskSlotStats() @Test public void testKillTaskSlotStats1() { - dynamicConfigBuilder.withKillTaskSlotRatio(1.0) - .withMaxKillTaskSlots(Integer.MAX_VALUE); + dynamicConfigBuilder.withKillTaskSlotRatio(1.0); + dynamicConfigBuilder.withMaxKillTaskSlots(Integer.MAX_VALUE); paramsBuilder.withDynamicConfigs(dynamicConfigBuilder.build()); initDuty(); @@ -545,8 +545,8 @@ public void testKillTaskSlotStats1() @Test public void testKillTaskSlotStats2() { - dynamicConfigBuilder.withKillTaskSlotRatio(0.0) - .withMaxKillTaskSlots(Integer.MAX_VALUE); + dynamicConfigBuilder.withKillTaskSlotRatio(0.0); + dynamicConfigBuilder.withMaxKillTaskSlots(Integer.MAX_VALUE); paramsBuilder.withDynamicConfigs(dynamicConfigBuilder.build()); initDuty(); @@ -560,8 +560,8 @@ public void testKillTaskSlotStats2() @Test public void testKillTaskSlotStats3() { - dynamicConfigBuilder.withKillTaskSlotRatio(1.0) - .withMaxKillTaskSlots(0); + dynamicConfigBuilder.withKillTaskSlotRatio(1.0); + dynamicConfigBuilder.withMaxKillTaskSlots(0); paramsBuilder.withDynamicConfigs(dynamicConfigBuilder.build()); initDuty(); @@ -575,8 +575,8 @@ public void testKillTaskSlotStats3() @Test public void testKillTaskSlotStats4() { - dynamicConfigBuilder.withKillTaskSlotRatio(0.1) - .withMaxKillTaskSlots(3); + dynamicConfigBuilder.withKillTaskSlotRatio(0.1); + dynamicConfigBuilder.withMaxKillTaskSlots(3); paramsBuilder.withDynamicConfigs(dynamicConfigBuilder.build()); initDuty(); @@ -590,8 +590,8 @@ public void testKillTaskSlotStats4() @Test public void testKillTaskSlotStats5() { - dynamicConfigBuilder.withKillTaskSlotRatio(0.3) - .withMaxKillTaskSlots(2); + dynamicConfigBuilder.withKillTaskSlotRatio(0.3); + dynamicConfigBuilder.withMaxKillTaskSlots(2); paramsBuilder.withDynamicConfigs(dynamicConfigBuilder.build()); initDuty(); diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBuilder.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBuilder.java index ef567d7cfcc8..d02f8b09e52f 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBuilder.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBuilder.java @@ -40,7 +40,6 @@ import org.apache.druid.server.coordinator.CoordinatorConfigManager; import org.apache.druid.server.coordinator.CoordinatorDynamicConfig; import org.apache.druid.server.coordinator.DruidCoordinator; -import org.apache.druid.server.coordinator.DruidCoordinatorConfig; import org.apache.druid.server.coordinator.MetadataManager; import org.apache.druid.server.coordinator.balancer.BalancerStrategyFactory; import org.apache.druid.server.coordinator.balancer.CachingCostBalancerStrategyConfig; @@ -53,6 +52,7 @@ import org.apache.druid.server.coordinator.config.CoordinatorKillConfigs; import org.apache.druid.server.coordinator.config.CoordinatorPeriodConfig; import org.apache.druid.server.coordinator.config.CoordinatorRunConfig; +import org.apache.druid.server.coordinator.config.DruidCoordinatorConfig; import org.apache.druid.server.coordinator.config.HttpLoadQueuePeonConfig; import org.apache.druid.server.coordinator.duty.CoordinatorCustomDutyGroups; import org.apache.druid.server.coordinator.loading.LoadQueueTaskMaster; diff --git a/services/src/main/java/org/apache/druid/cli/CliCoordinator.java b/services/src/main/java/org/apache/druid/cli/CliCoordinator.java index 4ecda70ef19e..1d40157f5cb1 100644 --- a/services/src/main/java/org/apache/druid/cli/CliCoordinator.java +++ b/services/src/main/java/org/apache/druid/cli/CliCoordinator.java @@ -100,7 +100,6 @@ import org.apache.druid.server.QuerySchedulerProvider; import org.apache.druid.server.coordinator.CoordinatorConfigManager; import org.apache.druid.server.coordinator.DruidCoordinator; -import org.apache.druid.server.coordinator.DruidCoordinatorConfig; import org.apache.druid.server.coordinator.MetadataManager; import org.apache.druid.server.coordinator.balancer.BalancerStrategyFactory; import org.apache.druid.server.coordinator.balancer.CachingCostBalancerStrategyConfig; @@ -109,6 +108,7 @@ import org.apache.druid.server.coordinator.config.CoordinatorKillConfigs; import org.apache.druid.server.coordinator.config.CoordinatorPeriodConfig; import org.apache.druid.server.coordinator.config.CoordinatorRunConfig; +import org.apache.druid.server.coordinator.config.DruidCoordinatorConfig; import org.apache.druid.server.coordinator.config.HttpLoadQueuePeonConfig; import org.apache.druid.server.coordinator.duty.CoordinatorCustomDuty; import org.apache.druid.server.coordinator.duty.CoordinatorCustomDutyGroup; From 18e79a4ca3b015edeb115e1ff7a0679f5a213a19 Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Fri, 19 Apr 2024 12:29:51 +0530 Subject: [PATCH 11/17] Fix validation of killunusedconfig --- .../druid/server/coordinator/config/DruidCoordinatorConfig.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/apache/druid/server/coordinator/config/DruidCoordinatorConfig.java b/server/src/main/java/org/apache/druid/server/coordinator/config/DruidCoordinatorConfig.java index 64291b1db55b..52d0e41dd994 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/config/DruidCoordinatorConfig.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/config/DruidCoordinatorConfig.java @@ -99,7 +99,7 @@ private void validateKillConfigs() // Validate config for killing unused segments final KillUnusedSegmentsConfig killUnusedConfig = killConfigs.unusedSegments(getCoordinatorIndexingPeriod()); - if (killUnusedConfig.getCleanupPeriod().getMillis() <= getCoordinatorIndexingPeriod().getMillis()) { + if (killUnusedConfig.getCleanupPeriod().getMillis() < getCoordinatorIndexingPeriod().getMillis()) { throw newInvalidInputExceptionForOperator( "'druid.coordinator.kill.period'[%s] must be greater than or equal to" + " 'druid.coordinator.period.indexingPeriod'[%s]", From 1c26bc167dad0aaa3235e5593cb4774cf1658a7c Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Fri, 19 Apr 2024 14:52:51 +0530 Subject: [PATCH 12/17] Simplify and fix KillSupervisorsCustomDuty --- .../duty/KillSupervisorsCustomDuty.java | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/KillSupervisorsCustomDuty.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/KillSupervisorsCustomDuty.java index 9080de517b3a..81be233bb5e8 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/KillSupervisorsCustomDuty.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/KillSupervisorsCustomDuty.java @@ -25,9 +25,8 @@ import org.apache.druid.guice.annotations.UnstableApi; import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.metadata.MetadataSupervisorManager; +import org.apache.druid.server.coordinator.DruidCoordinatorRuntimeParams; import org.apache.druid.server.coordinator.config.MetadataCleanupConfig; -import org.apache.druid.server.coordinator.stats.Stats; -import org.joda.time.DateTime; import org.joda.time.Duration; /** @@ -40,11 +39,11 @@ * duties. All production clusters should continue using {@link KillSupervisors}. */ @UnstableApi -public class KillSupervisorsCustomDuty extends MetadataCleanupDuty implements CoordinatorCustomDuty +public class KillSupervisorsCustomDuty implements CoordinatorCustomDuty { private static final Logger log = new Logger(KillSupervisorsCustomDuty.class); - private final MetadataSupervisorManager metadataSupervisorManager; + private final KillSupervisors delegate; @JsonCreator public KillSupervisorsCustomDuty( @@ -52,20 +51,18 @@ public KillSupervisorsCustomDuty( @JacksonInject MetadataSupervisorManager metadataSupervisorManager ) { - super( - "supervisors", - // Pass null here, actual period of custom duties is configured separately - new MetadataCleanupConfig(true, null, retainDuration), - Stats.Kill.SUPERVISOR_SPECS + this.delegate = new KillSupervisors( + // Pass period as zero here, actual period of custom duties is configured at the duty group level + new MetadataCleanupConfig(true, Duration.ZERO, retainDuration), + metadataSupervisorManager ); - this.metadataSupervisorManager = metadataSupervisorManager; log.warn("This is only an example implementation of a custom duty and" + " must not be used in production. Use KillSupervisors duty instead."); } @Override - protected int cleanupEntriesCreatedBefore(DateTime minCreatedTime) + public DruidCoordinatorRuntimeParams run(DruidCoordinatorRuntimeParams params) { - return metadataSupervisorManager.removeTerminatedSupervisorsOlderThan(minCreatedTime.getMillis()); + return delegate.run(params); } } From a2a928c539d5587cc7e5b372600fd765187e1be3 Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Wed, 24 Apr 2024 08:33:39 +0530 Subject: [PATCH 13/17] Address review comments --- .../coordinator/config/CoordinatorKillConfigs.java | 12 ++++++------ .../coordinator/config/KillUnusedSegmentsConfig.java | 2 +- .../coordinator/config/MetadataCleanupConfig.java | 2 +- .../coordinator/duty/KillSupervisorsCustomDuty.java | 2 +- .../server/coordinator/duty/KillUnusedSegments.java | 4 ++-- .../coordinator/DruidCoordinatorConfigTest.java | 12 ++++++------ 6 files changed, 17 insertions(+), 17 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorKillConfigs.java b/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorKillConfigs.java index 3d6b8c8d1547..5b490e77d054 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorKillConfigs.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorKillConfigs.java @@ -84,12 +84,12 @@ public CoordinatorKillConfigs( @JsonProperty("maxSegments") Integer killUnusedMaxSegments ) { - this.pendingSegments = Configs.valueOrDefault(pendingSegments, MetadataCleanupConfig.STANDARD); - this.supervisors = Configs.valueOrDefault(supervisors, MetadataCleanupConfig.STANDARD); - this.audit = Configs.valueOrDefault(audit, MetadataCleanupConfig.STANDARD); - this.datasource = Configs.valueOrDefault(datasource, MetadataCleanupConfig.STANDARD); - this.rules = Configs.valueOrDefault(rules, MetadataCleanupConfig.STANDARD); - this.compaction = Configs.valueOrDefault(compaction, MetadataCleanupConfig.STANDARD); + this.pendingSegments = Configs.valueOrDefault(pendingSegments, MetadataCleanupConfig.DEFAULT); + this.supervisors = Configs.valueOrDefault(supervisors, MetadataCleanupConfig.DEFAULT); + this.audit = Configs.valueOrDefault(audit, MetadataCleanupConfig.DEFAULT); + this.datasource = Configs.valueOrDefault(datasource, MetadataCleanupConfig.DEFAULT); + this.rules = Configs.valueOrDefault(rules, MetadataCleanupConfig.DEFAULT); + this.compaction = Configs.valueOrDefault(compaction, MetadataCleanupConfig.DEFAULT); this.killUnusedEnabled = killUnusedEnabled; this.killUnusedPeriod = killUnusedPeriod; diff --git a/server/src/main/java/org/apache/druid/server/coordinator/config/KillUnusedSegmentsConfig.java b/server/src/main/java/org/apache/druid/server/coordinator/config/KillUnusedSegmentsConfig.java index d8ed6fe17d07..b22513e85eed 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/config/KillUnusedSegmentsConfig.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/config/KillUnusedSegmentsConfig.java @@ -24,7 +24,7 @@ public class KillUnusedSegmentsConfig extends MetadataCleanupConfig { - public static KillUnusedSegmentsConfig STANDARD + public static KillUnusedSegmentsConfig DEFAULT = new KillUnusedSegmentsConfig(null, null, null, null, null, null); private final int maxSegments; diff --git a/server/src/main/java/org/apache/druid/server/coordinator/config/MetadataCleanupConfig.java b/server/src/main/java/org/apache/druid/server/coordinator/config/MetadataCleanupConfig.java index a7686e22d15b..c1e1a956fc2e 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/config/MetadataCleanupConfig.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/config/MetadataCleanupConfig.java @@ -28,7 +28,7 @@ public class MetadataCleanupConfig { - public static final MetadataCleanupConfig STANDARD = new MetadataCleanupConfig(null, null, null); + public static final MetadataCleanupConfig DEFAULT = new MetadataCleanupConfig(null, null, null); @JsonProperty("on") private final boolean cleanupEnabled; diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/KillSupervisorsCustomDuty.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/KillSupervisorsCustomDuty.java index 81be233bb5e8..9069a48bc210 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/KillSupervisorsCustomDuty.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/KillSupervisorsCustomDuty.java @@ -35,7 +35,7 @@ * as {@link KillSupervisors} but uses a different configuration style as * detailed in {@link CoordinatorCustomDuty}. *

- * This duty is only an example to demostrate the usage of coordinator custom + * This duty is only an example to demonstrate the usage of coordinator custom * duties. All production clusters should continue using {@link KillSupervisors}. */ @UnstableApi diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/KillUnusedSegments.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/KillUnusedSegments.java index 12a926422fc5..060be69939bc 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/KillUnusedSegments.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/KillUnusedSegments.java @@ -82,7 +82,7 @@ public class KillUnusedSegments implements CoordinatorDuty */ private final Map datasourceToLastKillIntervalEnd; private DateTime lastKillTime; - private final long bufferPeriod; + private final Duration bufferPeriod; private final SegmentsMetadataManager segmentsMetadataManager; private final OverlordClient overlordClient; @@ -105,7 +105,7 @@ public KillUnusedSegments( durationToRetain ); } - this.bufferPeriod = killConfig.getBufferPeriod().getMillis(); + this.bufferPeriod = killConfig.getBufferPeriod(); datasourceToLastKillIntervalEnd = new ConcurrentHashMap<>(); diff --git a/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java b/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java index 527225f3b0c3..1b2ec8db7880 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java @@ -216,12 +216,12 @@ public void testCoordinatorKillConfigDefaultValues() final CoordinatorKillConfigs killConfigs = deserializeFrom(new Properties(), "druid.coordinator.kill", CoordinatorKillConfigs.class); - Assert.assertEquals(MetadataCleanupConfig.STANDARD, killConfigs.audit()); - Assert.assertEquals(MetadataCleanupConfig.STANDARD, killConfigs.supervisors()); - Assert.assertEquals(MetadataCleanupConfig.STANDARD, killConfigs.compaction()); - Assert.assertEquals(MetadataCleanupConfig.STANDARD, killConfigs.datasource()); - Assert.assertEquals(MetadataCleanupConfig.STANDARD, killConfigs.rules()); - Assert.assertEquals(MetadataCleanupConfig.STANDARD, killConfigs.pendingSegments()); + Assert.assertEquals(MetadataCleanupConfig.DEFAULT, killConfigs.audit()); + Assert.assertEquals(MetadataCleanupConfig.DEFAULT, killConfigs.supervisors()); + Assert.assertEquals(MetadataCleanupConfig.DEFAULT, killConfigs.compaction()); + Assert.assertEquals(MetadataCleanupConfig.DEFAULT, killConfigs.datasource()); + Assert.assertEquals(MetadataCleanupConfig.DEFAULT, killConfigs.rules()); + Assert.assertEquals(MetadataCleanupConfig.DEFAULT, killConfigs.pendingSegments()); } @Test From 47a9e4ecca4697998bba9f2d5847268e76603283 Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Thu, 25 Apr 2024 07:41:03 +0530 Subject: [PATCH 14/17] Fix new tests --- .../KillUnreferencedSegmentSchemaDuty.java | 1 - ...KillUnreferencedSegmentSchemaDutyTest.java | 39 ++++++++++--------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/KillUnreferencedSegmentSchemaDuty.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/KillUnreferencedSegmentSchemaDuty.java index 21020415282f..82ad4cedc779 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/KillUnreferencedSegmentSchemaDuty.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/KillUnreferencedSegmentSchemaDuty.java @@ -21,7 +21,6 @@ import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.segment.metadata.SegmentSchemaManager; -import org.apache.druid.server.coordinator.config.DruidCoordinatorConfig; import org.apache.druid.server.coordinator.config.MetadataCleanupConfig; import org.apache.druid.server.coordinator.stats.Stats; import org.joda.time.DateTime; diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/KillUnreferencedSegmentSchemaDutyTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/KillUnreferencedSegmentSchemaDutyTest.java index 5fe48c23a04e..c9975f94ce5e 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/KillUnreferencedSegmentSchemaDutyTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/KillUnreferencedSegmentSchemaDutyTest.java @@ -37,6 +37,7 @@ import org.apache.druid.segment.metadata.SegmentSchemaManager; import org.apache.druid.segment.metadata.SegmentSchemaTestUtils; import org.apache.druid.server.coordinator.DruidCoordinatorRuntimeParams; +import org.apache.druid.server.coordinator.config.MetadataCleanupConfig; import org.apache.druid.server.coordinator.stats.CoordinatorRunStats; import org.apache.druid.timeline.DataSegment; import org.apache.druid.timeline.SegmentId; @@ -103,14 +104,14 @@ public void testKillUnreferencedSchema() dateTimes.add(now.plusMinutes(61)); dateTimes.add(now.plusMinutes(6 * 60 + 1)); - TestDruidCoordinatorConfig druidCoordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withMetadataStoreManagementPeriod(Period.parse("PT1H").toStandardDuration()) - .withSegmentSchemaKillPeriod(Period.parse("PT1H").toStandardDuration()) - .withSegmentSchemaKillDurationToRetain(Period.parse("PT6H").toStandardDuration()) - .build(); + MetadataCleanupConfig cleanupConfig = new MetadataCleanupConfig( + true, + Period.parse("PT1H").toStandardDuration(), + Period.parse("PT6H").toStandardDuration() + ); KillUnreferencedSegmentSchemaDuty duty = - new TestKillUnreferencedSegmentSchemasDuty(druidCoordinatorConfig, segmentSchemaManager, dateTimes); + new TestKillUnreferencedSegmentSchemasDuty(cleanupConfig, segmentSchemaManager, dateTimes); Set segments = new HashSet<>(); List schemaMetadataPluses = new ArrayList<>(); @@ -209,14 +210,14 @@ public void testKillUnreferencedSchema_repair() dateTimes.add(now); dateTimes.add(now.plusMinutes(61)); - TestDruidCoordinatorConfig druidCoordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withMetadataStoreManagementPeriod(Period.parse("PT1H").toStandardDuration()) - .withSegmentSchemaKillPeriod(Period.parse("PT1H").toStandardDuration()) - .withSegmentSchemaKillDurationToRetain(Period.parse("PT6H").toStandardDuration()) - .build(); + MetadataCleanupConfig cleanupConfig = new MetadataCleanupConfig( + true, + Period.parse("PT1H").toStandardDuration(), + Period.parse("PT6H").toStandardDuration() + ); KillUnreferencedSegmentSchemaDuty duty = - new TestKillUnreferencedSegmentSchemasDuty(druidCoordinatorConfig, segmentSchemaManager, dateTimes); + new TestKillUnreferencedSegmentSchemasDuty(cleanupConfig, segmentSchemaManager, dateTimes); RowSignature rowSignature = RowSignature.builder().add("c1", ColumnType.FLOAT).build(); @@ -281,14 +282,14 @@ public void testKillOlderVersionSchema() dateTimes.add(now.plusMinutes(61)); dateTimes.add(now.plusMinutes(6 * 60 + 1)); - TestDruidCoordinatorConfig druidCoordinatorConfig = new TestDruidCoordinatorConfig.Builder() - .withMetadataStoreManagementPeriod(Period.parse("PT1H").toStandardDuration()) - .withSegmentSchemaKillPeriod(Period.parse("PT1H").toStandardDuration()) - .withSegmentSchemaKillDurationToRetain(Period.parse("PT6H").toStandardDuration()) - .build(); + MetadataCleanupConfig cleanupConfig = new MetadataCleanupConfig( + true, + Period.parse("PT1H").toStandardDuration(), + Period.parse("PT6H").toStandardDuration() + ); KillUnreferencedSegmentSchemaDuty duty = - new TestKillUnreferencedSegmentSchemasDuty(druidCoordinatorConfig, segmentSchemaManager, dateTimes); + new TestKillUnreferencedSegmentSchemasDuty(cleanupConfig, segmentSchemaManager, dateTimes); // create 2 versions of same schema // unreferenced one should get deleted @@ -367,7 +368,7 @@ private static class TestKillUnreferencedSegmentSchemasDuty extends KillUnrefere private int index = -1; public TestKillUnreferencedSegmentSchemasDuty( - DruidCoordinatorConfig config, + MetadataCleanupConfig config, SegmentSchemaManager segmentSchemaManager, List dateTimes ) From 032f2b76a8b9f54a23df5d6737c303d2a8b36d0a Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Thu, 25 Apr 2024 08:52:25 +0530 Subject: [PATCH 15/17] Add KillUnusedSchemasConfig --- .../config/CoordinatorKillConfigs.java | 8 ++- .../config/DruidCoordinatorConfig.java | 1 + .../config/KillUnusedSchemasConfig.java | 49 +++++++++++++++++++ .../coordinator/duty/MetadataCleanupDuty.java | 27 +--------- .../DruidCoordinatorConfigTest.java | 38 +++++++++++++- 5 files changed, 90 insertions(+), 33 deletions(-) create mode 100644 server/src/main/java/org/apache/druid/server/coordinator/config/KillUnusedSchemasConfig.java diff --git a/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorKillConfigs.java b/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorKillConfigs.java index 75c8480be5e4..392415b4a44b 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorKillConfigs.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorKillConfigs.java @@ -48,7 +48,7 @@ public class CoordinatorKillConfigs private final MetadataCleanupConfig pendingSegments; @JsonProperty("segmentSchema") - private final MetadataCleanupConfig segmentSchema; + private final KillUnusedSchemasConfig segmentSchema; // Raw configs for killing unused segments // These have been added as fields because KillUnusedSegmentsConfig is initialized lazily @@ -78,7 +78,7 @@ public CoordinatorKillConfigs( @JsonProperty("datasource") MetadataCleanupConfig datasource, @JsonProperty("rule") MetadataCleanupConfig rules, @JsonProperty("compaction") MetadataCleanupConfig compaction, - @JsonProperty("segmentSchema") MetadataCleanupConfig segmentSchema, + @JsonProperty("segmentSchema") KillUnusedSchemasConfig segmentSchema, // Configs for cleanup of unused segments @JsonProperty("on") Boolean killUnusedEnabled, @JsonProperty("period") Duration killUnusedPeriod, @@ -94,9 +94,7 @@ public CoordinatorKillConfigs( this.datasource = Configs.valueOrDefault(datasource, MetadataCleanupConfig.DEFAULT); this.rules = Configs.valueOrDefault(rules, MetadataCleanupConfig.DEFAULT); this.compaction = Configs.valueOrDefault(compaction, MetadataCleanupConfig.DEFAULT); - - // TODO: Defaults here should be enabled=true, period=1H, retain=6H - this.segmentSchema = Configs.valueOrDefault(segmentSchema, MetadataCleanupConfig.DEFAULT); + this.segmentSchema = Configs.valueOrDefault(segmentSchema, KillUnusedSchemasConfig.DEFAULT); this.killUnusedEnabled = killUnusedEnabled; this.killUnusedPeriod = killUnusedPeriod; diff --git a/server/src/main/java/org/apache/druid/server/coordinator/config/DruidCoordinatorConfig.java b/server/src/main/java/org/apache/druid/server/coordinator/config/DruidCoordinatorConfig.java index 52d0e41dd994..fd61c3075ae0 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/config/DruidCoordinatorConfig.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/config/DruidCoordinatorConfig.java @@ -95,6 +95,7 @@ private void validateKillConfigs() validateKillConfig(killConfigs.datasource(), "datasource"); validateKillConfig(killConfigs.rules(), "rule"); validateKillConfig(killConfigs.supervisors(), "supervisor"); + validateKillConfig(killConfigs.segmentSchema(), "segmentSchema"); // Validate config for killing unused segments final KillUnusedSegmentsConfig killUnusedConfig diff --git a/server/src/main/java/org/apache/druid/server/coordinator/config/KillUnusedSchemasConfig.java b/server/src/main/java/org/apache/druid/server/coordinator/config/KillUnusedSchemasConfig.java new file mode 100644 index 000000000000..1f6ba05b80f7 --- /dev/null +++ b/server/src/main/java/org/apache/druid/server/coordinator/config/KillUnusedSchemasConfig.java @@ -0,0 +1,49 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.server.coordinator.config; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import org.apache.druid.common.config.Configs; +import org.joda.time.Duration; + +/** + * Config to cleanup unused segment schemas. + *

+ * Extended from {@link MetadataCleanupConfig} to allow having different defaults. + */ +public class KillUnusedSchemasConfig extends MetadataCleanupConfig +{ + public static final KillUnusedSchemasConfig DEFAULT = new KillUnusedSchemasConfig(null, null, null); + + @JsonCreator + public KillUnusedSchemasConfig( + @JsonProperty("on") Boolean cleanupEnabled, + @JsonProperty("period") Duration cleanupPeriod, + @JsonProperty("durationToRetain") Duration durationToRetain + ) + { + super( + Configs.valueOrDefault(cleanupEnabled, true), + Configs.valueOrDefault(cleanupPeriod, Duration.standardHours(1)), + Configs.valueOrDefault(durationToRetain, Duration.standardHours(6)) + ); + } +} diff --git a/server/src/main/java/org/apache/druid/server/coordinator/duty/MetadataCleanupDuty.java b/server/src/main/java/org/apache/druid/server/coordinator/duty/MetadataCleanupDuty.java index 5b6265165731..e24f93966895 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/duty/MetadataCleanupDuty.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/duty/MetadataCleanupDuty.java @@ -104,34 +104,9 @@ public DruidCoordinatorRuntimeParams run(DruidCoordinatorRuntimeParams params) */ protected abstract int cleanupEntriesCreatedBefore(DateTime minCreatedTime); -<<<<<<< HEAD -======= - private void validatePeriod(Duration period, Duration metadataManagementPeriod) - { - Preconditions.checkArgument( - period != null && period.getMillis() >= metadataManagementPeriod.getMillis(), - "[%s.period] must be greater than [druid.coordinator.period.metadataStoreManagementPeriod]", - propertyPrefix - ); - } - - private void validateRetainDuration(Duration retainDuration) - { - Preconditions.checkArgument( - retainDuration != null && retainDuration.getMillis() >= 0, - "[%s.durationToRetain] must be 0 milliseconds or higher", - propertyPrefix - ); - Preconditions.checkArgument( - retainDuration.getMillis() < System.currentTimeMillis(), - "[%s.durationToRetain] cannot be greater than current time in milliseconds", - propertyPrefix - ); - } - protected DateTime getCurrentTime() { return DateTimes.nowUtc(); } ->>>>>>> e74da6a6b6d0b614025660023af4644f4e9b54e8 + } diff --git a/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java b/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java index 60ec2a9265a1..cef702359b38 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java @@ -32,6 +32,7 @@ import org.apache.druid.server.coordinator.config.CoordinatorRunConfig; import org.apache.druid.server.coordinator.config.DruidCoordinatorConfig; import org.apache.druid.server.coordinator.config.HttpLoadQueuePeonConfig; +import org.apache.druid.server.coordinator.config.KillUnusedSchemasConfig; import org.apache.druid.server.coordinator.config.KillUnusedSegmentsConfig; import org.apache.druid.server.coordinator.config.MetadataCleanupConfig; import org.hamcrest.MatcherAssert; @@ -222,6 +223,7 @@ public void testCoordinatorKillConfigDefaultValues() Assert.assertEquals(MetadataCleanupConfig.DEFAULT, killConfigs.datasource()); Assert.assertEquals(MetadataCleanupConfig.DEFAULT, killConfigs.rules()); Assert.assertEquals(MetadataCleanupConfig.DEFAULT, killConfigs.pendingSegments()); + Assert.assertEquals(KillUnusedSchemasConfig.DEFAULT, killConfigs.segmentSchema()); } @Test @@ -250,6 +252,10 @@ public void testCoordinatorKillConfigOverrideValues() props.setProperty("druid.coordinator.kill.pendingSegments.on", "false"); + props.setProperty("druid.coordinator.kill.segmentSchema.on", "false"); + props.setProperty("druid.coordinator.kill.segmentSchema.period", "PT2H"); + props.setProperty("druid.coordinator.kill.segmentSchema.durationToRetain", "PT8H"); + final CoordinatorKillConfigs killConfigs = deserializeFrom(props, "druid.coordinator.kill", CoordinatorKillConfigs.class); @@ -273,6 +279,10 @@ public void testCoordinatorKillConfigOverrideValues() new MetadataCleanupConfig(false, Duration.standardHours(1), Duration.standardHours(2)), killConfigs.supervisors() ); + Assert.assertEquals( + new KillUnusedSchemasConfig(false, Duration.standardHours(2), Duration.standardHours(8)), + killConfigs.segmentSchema() + ); Assert.assertFalse(killConfigs.pendingSegments().isCleanupEnabled()); } @@ -314,6 +324,14 @@ public void testCoordinatorConfigFailsWhenCleanupPeriodIsInvalid() "'druid.coordinator.kill.supervisor.period'[PT1800S] must be greater than" + " 'druid.coordinator.period.metadataStoreManagementPeriod'[PT3600S]" ); + verifyCoordinatorConfigFailsWith( + createKillConfig().segmentSchema( + new KillUnusedSchemasConfig(true, cleanupConfig.getCleanupPeriod(), null) + ).build(), + periodConfig, + "'druid.coordinator.kill.segmentSchema.period'[PT1800S] must be greater than" + + " 'druid.coordinator.period.metadataStoreManagementPeriod'[PT3600S]" + ); } @Test @@ -348,6 +366,13 @@ public void testCoordinatorConfigFailsWhenRetainDurationIsNegative() defaultPeriodConfig, "'druid.coordinator.kill.supervisor.durationToRetain'[PT-1S] must be 0 milliseconds or higher" ); + verifyCoordinatorConfigFailsWith( + createKillConfig().segmentSchema( + new KillUnusedSchemasConfig(true, null, cleanupConfig.getDurationToRetain()) + ).build(), + defaultPeriodConfig, + "'druid.coordinator.kill.segmentSchema.durationToRetain'[PT-1S] must be 0 milliseconds or higher" + ); } @Test @@ -386,6 +411,15 @@ public void testCoordinatorConfigFailsWhenRetainDurationIsHigherThanCurrentTime( "'druid.coordinator.kill.supervisor.durationToRetain'[%s] cannot be greater than current time in milliseconds", futureRetainDuration ); + verifyCoordinatorConfigFailsWith( + createKillConfig().segmentSchema( + new KillUnusedSchemasConfig(true, null, futureRetainDuration) + ).build(), + defaultPeriodConfig, + "'druid.coordinator.kill.segmentSchema.durationToRetain'[%s] cannot be" + + " greater than current time in milliseconds", + futureRetainDuration + ); } @Test @@ -455,7 +489,7 @@ private static class KillConfigBuilder MetadataCleanupConfig rules; MetadataCleanupConfig supervisors; MetadataCleanupConfig pendingSegments; - MetadataCleanupConfig segmentSchema; + KillUnusedSchemasConfig segmentSchema; KillUnusedSegmentsConfig unusedSegments; KillConfigBuilder audit(MetadataCleanupConfig config) @@ -488,7 +522,7 @@ KillConfigBuilder supervisors(MetadataCleanupConfig config) return this; } - KillConfigBuilder segmentSchema(MetadataCleanupConfig config) + KillConfigBuilder segmentSchema(KillUnusedSchemasConfig config) { this.segmentSchema = config; return this; From c10a5c9203db31c60a576ff539005f731c4f41af Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Thu, 25 Apr 2024 09:31:46 +0530 Subject: [PATCH 16/17] Remove KillUnusedSchemasConfig --- .../config/CoordinatorKillConfigs.java | 6 +-- .../config/KillUnusedSchemasConfig.java | 49 ------------------- .../DruidCoordinatorConfigTest.java | 21 +++----- ...KillUnreferencedSegmentSchemaDutyTest.java | 3 +- 4 files changed, 11 insertions(+), 68 deletions(-) delete mode 100644 server/src/main/java/org/apache/druid/server/coordinator/config/KillUnusedSchemasConfig.java diff --git a/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorKillConfigs.java b/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorKillConfigs.java index 392415b4a44b..01ce4f1ade57 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorKillConfigs.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorKillConfigs.java @@ -48,7 +48,7 @@ public class CoordinatorKillConfigs private final MetadataCleanupConfig pendingSegments; @JsonProperty("segmentSchema") - private final KillUnusedSchemasConfig segmentSchema; + private final MetadataCleanupConfig segmentSchema; // Raw configs for killing unused segments // These have been added as fields because KillUnusedSegmentsConfig is initialized lazily @@ -78,7 +78,7 @@ public CoordinatorKillConfigs( @JsonProperty("datasource") MetadataCleanupConfig datasource, @JsonProperty("rule") MetadataCleanupConfig rules, @JsonProperty("compaction") MetadataCleanupConfig compaction, - @JsonProperty("segmentSchema") KillUnusedSchemasConfig segmentSchema, + @JsonProperty("segmentSchema") MetadataCleanupConfig segmentSchema, // Configs for cleanup of unused segments @JsonProperty("on") Boolean killUnusedEnabled, @JsonProperty("period") Duration killUnusedPeriod, @@ -94,7 +94,7 @@ public CoordinatorKillConfigs( this.datasource = Configs.valueOrDefault(datasource, MetadataCleanupConfig.DEFAULT); this.rules = Configs.valueOrDefault(rules, MetadataCleanupConfig.DEFAULT); this.compaction = Configs.valueOrDefault(compaction, MetadataCleanupConfig.DEFAULT); - this.segmentSchema = Configs.valueOrDefault(segmentSchema, KillUnusedSchemasConfig.DEFAULT); + this.segmentSchema = Configs.valueOrDefault(segmentSchema, MetadataCleanupConfig.DEFAULT); this.killUnusedEnabled = killUnusedEnabled; this.killUnusedPeriod = killUnusedPeriod; diff --git a/server/src/main/java/org/apache/druid/server/coordinator/config/KillUnusedSchemasConfig.java b/server/src/main/java/org/apache/druid/server/coordinator/config/KillUnusedSchemasConfig.java deleted file mode 100644 index 1f6ba05b80f7..000000000000 --- a/server/src/main/java/org/apache/druid/server/coordinator/config/KillUnusedSchemasConfig.java +++ /dev/null @@ -1,49 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.apache.druid.server.coordinator.config; - -import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonProperty; -import org.apache.druid.common.config.Configs; -import org.joda.time.Duration; - -/** - * Config to cleanup unused segment schemas. - *

- * Extended from {@link MetadataCleanupConfig} to allow having different defaults. - */ -public class KillUnusedSchemasConfig extends MetadataCleanupConfig -{ - public static final KillUnusedSchemasConfig DEFAULT = new KillUnusedSchemasConfig(null, null, null); - - @JsonCreator - public KillUnusedSchemasConfig( - @JsonProperty("on") Boolean cleanupEnabled, - @JsonProperty("period") Duration cleanupPeriod, - @JsonProperty("durationToRetain") Duration durationToRetain - ) - { - super( - Configs.valueOrDefault(cleanupEnabled, true), - Configs.valueOrDefault(cleanupPeriod, Duration.standardHours(1)), - Configs.valueOrDefault(durationToRetain, Duration.standardHours(6)) - ); - } -} diff --git a/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java b/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java index cef702359b38..0ff0660f8181 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java @@ -32,7 +32,6 @@ import org.apache.druid.server.coordinator.config.CoordinatorRunConfig; import org.apache.druid.server.coordinator.config.DruidCoordinatorConfig; import org.apache.druid.server.coordinator.config.HttpLoadQueuePeonConfig; -import org.apache.druid.server.coordinator.config.KillUnusedSchemasConfig; import org.apache.druid.server.coordinator.config.KillUnusedSegmentsConfig; import org.apache.druid.server.coordinator.config.MetadataCleanupConfig; import org.hamcrest.MatcherAssert; @@ -223,7 +222,7 @@ public void testCoordinatorKillConfigDefaultValues() Assert.assertEquals(MetadataCleanupConfig.DEFAULT, killConfigs.datasource()); Assert.assertEquals(MetadataCleanupConfig.DEFAULT, killConfigs.rules()); Assert.assertEquals(MetadataCleanupConfig.DEFAULT, killConfigs.pendingSegments()); - Assert.assertEquals(KillUnusedSchemasConfig.DEFAULT, killConfigs.segmentSchema()); + Assert.assertEquals(MetadataCleanupConfig.DEFAULT, killConfigs.segmentSchema()); } @Test @@ -280,7 +279,7 @@ public void testCoordinatorKillConfigOverrideValues() killConfigs.supervisors() ); Assert.assertEquals( - new KillUnusedSchemasConfig(false, Duration.standardHours(2), Duration.standardHours(8)), + new MetadataCleanupConfig(false, Duration.standardHours(2), Duration.standardHours(8)), killConfigs.segmentSchema() ); Assert.assertFalse(killConfigs.pendingSegments().isCleanupEnabled()); @@ -325,9 +324,7 @@ public void testCoordinatorConfigFailsWhenCleanupPeriodIsInvalid() + " 'druid.coordinator.period.metadataStoreManagementPeriod'[PT3600S]" ); verifyCoordinatorConfigFailsWith( - createKillConfig().segmentSchema( - new KillUnusedSchemasConfig(true, cleanupConfig.getCleanupPeriod(), null) - ).build(), + createKillConfig().segmentSchema(cleanupConfig).build(), periodConfig, "'druid.coordinator.kill.segmentSchema.period'[PT1800S] must be greater than" + " 'druid.coordinator.period.metadataStoreManagementPeriod'[PT3600S]" @@ -367,9 +364,7 @@ public void testCoordinatorConfigFailsWhenRetainDurationIsNegative() "'druid.coordinator.kill.supervisor.durationToRetain'[PT-1S] must be 0 milliseconds or higher" ); verifyCoordinatorConfigFailsWith( - createKillConfig().segmentSchema( - new KillUnusedSchemasConfig(true, null, cleanupConfig.getDurationToRetain()) - ).build(), + createKillConfig().segmentSchema(cleanupConfig).build(), defaultPeriodConfig, "'druid.coordinator.kill.segmentSchema.durationToRetain'[PT-1S] must be 0 milliseconds or higher" ); @@ -412,9 +407,7 @@ public void testCoordinatorConfigFailsWhenRetainDurationIsHigherThanCurrentTime( futureRetainDuration ); verifyCoordinatorConfigFailsWith( - createKillConfig().segmentSchema( - new KillUnusedSchemasConfig(true, null, futureRetainDuration) - ).build(), + createKillConfig().segmentSchema(cleanupConfig).build(), defaultPeriodConfig, "'druid.coordinator.kill.segmentSchema.durationToRetain'[%s] cannot be" + " greater than current time in milliseconds", @@ -489,7 +482,7 @@ private static class KillConfigBuilder MetadataCleanupConfig rules; MetadataCleanupConfig supervisors; MetadataCleanupConfig pendingSegments; - KillUnusedSchemasConfig segmentSchema; + MetadataCleanupConfig segmentSchema; KillUnusedSegmentsConfig unusedSegments; KillConfigBuilder audit(MetadataCleanupConfig config) @@ -522,7 +515,7 @@ KillConfigBuilder supervisors(MetadataCleanupConfig config) return this; } - KillConfigBuilder segmentSchema(KillUnusedSchemasConfig config) + KillConfigBuilder segmentSchema(MetadataCleanupConfig config) { this.segmentSchema = config; return this; diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/KillUnreferencedSegmentSchemaDutyTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/KillUnreferencedSegmentSchemaDutyTest.java index c9975f94ce5e..a7701c4e1042 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/KillUnreferencedSegmentSchemaDutyTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/KillUnreferencedSegmentSchemaDutyTest.java @@ -104,12 +104,11 @@ public void testKillUnreferencedSchema() dateTimes.add(now.plusMinutes(61)); dateTimes.add(now.plusMinutes(6 * 60 + 1)); - MetadataCleanupConfig cleanupConfig = new MetadataCleanupConfig( + final MetadataCleanupConfig cleanupConfig = new MetadataCleanupConfig( true, Period.parse("PT1H").toStandardDuration(), Period.parse("PT6H").toStandardDuration() ); - KillUnreferencedSegmentSchemaDuty duty = new TestKillUnreferencedSegmentSchemasDuty(cleanupConfig, segmentSchemaManager, dateTimes); From 9e79a07c0d2720b864df8f4b6d0a270d9e744a49 Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Mon, 29 Apr 2024 07:58:57 +0530 Subject: [PATCH 17/17] Minor renames --- .../server/coordinator/DruidCoordinator.java | 8 ++-- .../config/CoordinatorKillConfigs.java | 47 ++++++++++--------- .../config/DruidCoordinatorConfig.java | 8 ++-- .../config/KillUnusedSegmentsConfig.java | 5 -- .../DruidCoordinatorConfigTest.java | 16 +++---- .../coordinator/DruidCoordinatorTest.java | 2 +- .../CoordinatorSimulationBuilder.java | 2 +- 7 files changed, 42 insertions(+), 46 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java b/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java index 9dcbd7797ff4..816cf44090b4 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java @@ -590,20 +590,20 @@ private List makeMetadataStoreManagementDuties() final CoordinatorKillConfigs killConfigs = config.getKillConfigs(); final List duties = new ArrayList<>(); duties.add(new KillSupervisors(killConfigs.supervisors(), metadataManager.supervisors())); - duties.add(new KillAuditLog(killConfigs.audit(), metadataManager.audit())); + duties.add(new KillAuditLog(killConfigs.auditLogs(), metadataManager.audit())); duties.add(new KillRules(killConfigs.rules(), metadataManager.rules())); duties.add( new KillDatasourceMetadata( - killConfigs.datasource(), + killConfigs.datasources(), metadataManager.indexer(), metadataManager.supervisors() ) ); duties.add( - new KillCompactionConfig(killConfigs.compaction(), metadataManager.segments(), metadataManager.configs()) + new KillCompactionConfig(killConfigs.compactionConfigs(), metadataManager.segments(), metadataManager.configs()) ); if (centralizedDatasourceSchemaConfig.isEnabled()) { - duties.add(new KillUnreferencedSegmentSchemaDuty(killConfigs.segmentSchema(), metadataManager.schemas())); + duties.add(new KillUnreferencedSegmentSchemaDuty(killConfigs.segmentSchemas(), metadataManager.schemas())); } return duties; } diff --git a/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorKillConfigs.java b/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorKillConfigs.java index 01ce4f1ade57..dfe08a007729 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorKillConfigs.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/config/CoordinatorKillConfigs.java @@ -26,32 +26,32 @@ public class CoordinatorKillConfigs { - public static CoordinatorKillConfigs STANDARD + public static CoordinatorKillConfigs DEFAULT = new CoordinatorKillConfigs(null, null, null, null, null, null, null, null, null, null, null, null, null); @JsonProperty("supervisor") private final MetadataCleanupConfig supervisors; @JsonProperty("audit") - private final MetadataCleanupConfig audit; + private final MetadataCleanupConfig auditLogs; @JsonProperty("datasource") - private final MetadataCleanupConfig datasource; + private final MetadataCleanupConfig datasources; @JsonProperty("rule") private final MetadataCleanupConfig rules; @JsonProperty("compaction") - private final MetadataCleanupConfig compaction; + private final MetadataCleanupConfig compactionConfigs; @JsonProperty("pendingSegments") private final MetadataCleanupConfig pendingSegments; @JsonProperty("segmentSchema") - private final MetadataCleanupConfig segmentSchema; + private final MetadataCleanupConfig segmentSchemas; // Raw configs for killing unused segments - // These have been added as fields because KillUnusedSegmentsConfig is initialized lazily + // These have been added as fields to make JsonConfigurator happy @JsonProperty("on") private final Boolean killUnusedEnabled; @@ -74,11 +74,11 @@ public class CoordinatorKillConfigs public CoordinatorKillConfigs( @JsonProperty("pendingSegments") MetadataCleanupConfig pendingSegments, @JsonProperty("supervisor") MetadataCleanupConfig supervisors, - @JsonProperty("audit") MetadataCleanupConfig audit, - @JsonProperty("datasource") MetadataCleanupConfig datasource, + @JsonProperty("audit") MetadataCleanupConfig auditLogs, + @JsonProperty("datasource") MetadataCleanupConfig datasources, @JsonProperty("rule") MetadataCleanupConfig rules, - @JsonProperty("compaction") MetadataCleanupConfig compaction, - @JsonProperty("segmentSchema") MetadataCleanupConfig segmentSchema, + @JsonProperty("compaction") MetadataCleanupConfig compactionConfigs, + @JsonProperty("segmentSchema") MetadataCleanupConfig segmentSchemas, // Configs for cleanup of unused segments @JsonProperty("on") Boolean killUnusedEnabled, @JsonProperty("period") Duration killUnusedPeriod, @@ -90,11 +90,11 @@ public CoordinatorKillConfigs( { this.pendingSegments = Configs.valueOrDefault(pendingSegments, MetadataCleanupConfig.DEFAULT); this.supervisors = Configs.valueOrDefault(supervisors, MetadataCleanupConfig.DEFAULT); - this.audit = Configs.valueOrDefault(audit, MetadataCleanupConfig.DEFAULT); - this.datasource = Configs.valueOrDefault(datasource, MetadataCleanupConfig.DEFAULT); + this.auditLogs = Configs.valueOrDefault(auditLogs, MetadataCleanupConfig.DEFAULT); + this.datasources = Configs.valueOrDefault(datasources, MetadataCleanupConfig.DEFAULT); this.rules = Configs.valueOrDefault(rules, MetadataCleanupConfig.DEFAULT); - this.compaction = Configs.valueOrDefault(compaction, MetadataCleanupConfig.DEFAULT); - this.segmentSchema = Configs.valueOrDefault(segmentSchema, MetadataCleanupConfig.DEFAULT); + this.compactionConfigs = Configs.valueOrDefault(compactionConfigs, MetadataCleanupConfig.DEFAULT); + this.segmentSchemas = Configs.valueOrDefault(segmentSchemas, MetadataCleanupConfig.DEFAULT); this.killUnusedEnabled = killUnusedEnabled; this.killUnusedPeriod = killUnusedPeriod; @@ -104,14 +104,14 @@ public CoordinatorKillConfigs( this.killUnusedMaxSegments = killUnusedMaxSegments; } - public MetadataCleanupConfig audit() + public MetadataCleanupConfig auditLogs() { - return audit; + return auditLogs; } - public MetadataCleanupConfig datasource() + public MetadataCleanupConfig datasources() { - return datasource; + return datasources; } public MetadataCleanupConfig rules() @@ -119,9 +119,9 @@ public MetadataCleanupConfig rules() return rules; } - public MetadataCleanupConfig compaction() + public MetadataCleanupConfig compactionConfigs() { - return compaction; + return compactionConfigs; } public MetadataCleanupConfig pendingSegments() @@ -134,13 +134,14 @@ public MetadataCleanupConfig supervisors() return supervisors; } - public MetadataCleanupConfig segmentSchema() + public MetadataCleanupConfig segmentSchemas() { - return segmentSchema; + return segmentSchemas; } /** - * Creates a KillUnusedSegmentsConfig. + * Creates a KillUnusedSegmentsConfig. This config is initialized lazily as + * it uses the indexingPeriod as the default cleanup period. */ public KillUnusedSegmentsConfig unusedSegments(Duration indexingPeriod) { diff --git a/server/src/main/java/org/apache/druid/server/coordinator/config/DruidCoordinatorConfig.java b/server/src/main/java/org/apache/druid/server/coordinator/config/DruidCoordinatorConfig.java index fd61c3075ae0..6004c5b1ba47 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/config/DruidCoordinatorConfig.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/config/DruidCoordinatorConfig.java @@ -90,12 +90,12 @@ public HttpLoadQueuePeonConfig getHttpLoadQueuePeonConfig() private void validateKillConfigs() { - validateKillConfig(killConfigs.audit(), "audit"); - validateKillConfig(killConfigs.compaction(), "compaction"); - validateKillConfig(killConfigs.datasource(), "datasource"); + validateKillConfig(killConfigs.auditLogs(), "audit"); + validateKillConfig(killConfigs.compactionConfigs(), "compaction"); + validateKillConfig(killConfigs.datasources(), "datasource"); validateKillConfig(killConfigs.rules(), "rule"); validateKillConfig(killConfigs.supervisors(), "supervisor"); - validateKillConfig(killConfigs.segmentSchema(), "segmentSchema"); + validateKillConfig(killConfigs.segmentSchemas(), "segmentSchema"); // Validate config for killing unused segments final KillUnusedSegmentsConfig killUnusedConfig diff --git a/server/src/main/java/org/apache/druid/server/coordinator/config/KillUnusedSegmentsConfig.java b/server/src/main/java/org/apache/druid/server/coordinator/config/KillUnusedSegmentsConfig.java index b22513e85eed..cbf8cc55bf48 100644 --- a/server/src/main/java/org/apache/druid/server/coordinator/config/KillUnusedSegmentsConfig.java +++ b/server/src/main/java/org/apache/druid/server/coordinator/config/KillUnusedSegmentsConfig.java @@ -24,13 +24,8 @@ public class KillUnusedSegmentsConfig extends MetadataCleanupConfig { - public static KillUnusedSegmentsConfig DEFAULT - = new KillUnusedSegmentsConfig(null, null, null, null, null, null); - private final int maxSegments; - private final boolean ignoreDurationToRetain; - private final Duration bufferPeriod; public KillUnusedSegmentsConfig( diff --git a/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java b/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java index 0ff0660f8181..cb064509c40b 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorConfigTest.java @@ -216,13 +216,13 @@ public void testCoordinatorKillConfigDefaultValues() final CoordinatorKillConfigs killConfigs = deserializeFrom(new Properties(), "druid.coordinator.kill", CoordinatorKillConfigs.class); - Assert.assertEquals(MetadataCleanupConfig.DEFAULT, killConfigs.audit()); + Assert.assertEquals(MetadataCleanupConfig.DEFAULT, killConfigs.auditLogs()); Assert.assertEquals(MetadataCleanupConfig.DEFAULT, killConfigs.supervisors()); - Assert.assertEquals(MetadataCleanupConfig.DEFAULT, killConfigs.compaction()); - Assert.assertEquals(MetadataCleanupConfig.DEFAULT, killConfigs.datasource()); + Assert.assertEquals(MetadataCleanupConfig.DEFAULT, killConfigs.compactionConfigs()); + Assert.assertEquals(MetadataCleanupConfig.DEFAULT, killConfigs.datasources()); Assert.assertEquals(MetadataCleanupConfig.DEFAULT, killConfigs.rules()); Assert.assertEquals(MetadataCleanupConfig.DEFAULT, killConfigs.pendingSegments()); - Assert.assertEquals(MetadataCleanupConfig.DEFAULT, killConfigs.segmentSchema()); + Assert.assertEquals(MetadataCleanupConfig.DEFAULT, killConfigs.segmentSchemas()); } @Test @@ -260,15 +260,15 @@ public void testCoordinatorKillConfigOverrideValues() Assert.assertEquals( new MetadataCleanupConfig(false, Duration.standardHours(10), Duration.standardHours(20)), - killConfigs.audit() + killConfigs.auditLogs() ); Assert.assertEquals( new MetadataCleanupConfig(false, Duration.standardHours(20), Duration.standardHours(30)), - killConfigs.compaction() + killConfigs.compactionConfigs() ); Assert.assertEquals( new MetadataCleanupConfig(false, Duration.standardHours(5), Duration.standardHours(10)), - killConfigs.datasource() + killConfigs.datasources() ); Assert.assertEquals( new MetadataCleanupConfig(false, Duration.standardHours(11), Duration.standardHours(12)), @@ -280,7 +280,7 @@ public void testCoordinatorKillConfigOverrideValues() ); Assert.assertEquals( new MetadataCleanupConfig(false, Duration.standardHours(2), Duration.standardHours(8)), - killConfigs.segmentSchema() + killConfigs.segmentSchemas() ); Assert.assertFalse(killConfigs.pendingSegments().isCleanupEnabled()); } 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 a7e67fc70ff8..a5ed1616a6d0 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 @@ -150,7 +150,7 @@ public void setUp() throws Exception druidCoordinatorConfig = new DruidCoordinatorConfig( new CoordinatorRunConfig(new Duration(COORDINATOR_START_DELAY), new Duration(COORDINATOR_PERIOD)), new CoordinatorPeriodConfig(null, null), - CoordinatorKillConfigs.STANDARD, + CoordinatorKillConfigs.DEFAULT, new CostBalancerStrategyFactory(), null ); diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBuilder.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBuilder.java index 82b32409d863..7b6eb280a398 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBuilder.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/CoordinatorSimulationBuilder.java @@ -444,7 +444,7 @@ private Environment( this.coordinatorConfig = new DruidCoordinatorConfig( new CoordinatorRunConfig(new Duration(1L), Duration.standardMinutes(1)), new CoordinatorPeriodConfig(null, null), - CoordinatorKillConfigs.STANDARD, + CoordinatorKillConfigs.DEFAULT, createBalancerStrategy(balancerStrategy), new HttpLoadQueuePeonConfig(null, null, null) );