diff --git a/docs/configuration/index.md b/docs/configuration/index.md index cd1b8dfc1f9e..0ed82017c3dd 100644 --- a/docs/configuration/index.md +++ b/docs/configuration/index.md @@ -1908,12 +1908,12 @@ The broker uses processing configs for nested groupBy queries. |`druid.processing.fifo`|If the processing queue should treat tasks of equal priority in a FIFO manner|`true`| |`druid.processing.tmpDir`|Path where temporary files created while processing a query should be stored. If specified, this configuration takes priority over the default `java.io.tmpdir` path.|path represented by `java.io.tmpdir`| |`druid.processing.merge.useParallelMergePool`|Enable automatic parallel merging for Brokers on a dedicated async ForkJoinPool. If `false`, instead merges will be done serially on the `HTTP` thread pool.|`true`| -|`druid.processing.merge.pool.parallelism`|Size of ForkJoinPool. Note that the default configuration assumes that the value returned by `Runtime.getRuntime().availableProcessors()` represents 2 hyper-threads per physical core, and multiplies this value by `0.75` in attempt to size `1.5` times the number of _physical_ cores.|`Runtime.getRuntime().availableProcessors() * 0.75` (rounded up)| -|`druid.processing.merge.pool.defaultMaxQueryParallelism`|Default maximum number of parallel merge tasks per query. Note that the default configuration assumes that the value returned by `Runtime.getRuntime().availableProcessors()` represents 2 hyper-threads per physical core, and multiplies this value by `0.5` in attempt to size to the number of _physical_ cores.|`Runtime.getRuntime().availableProcessors() * 0.5` (rounded up)| -|`druid.processing.merge.pool.awaitShutdownMillis`|Time to wait for merge ForkJoinPool tasks to complete before ungracefully stopping on process shutdown in milliseconds.|`60_000`| -|`druid.processing.merge.task.targetRunTimeMillis`|Ideal run-time of each ForkJoinPool merge task, before forking off a new task to continue merging sequences.|100| -|`druid.processing.merge.task.initialYieldNumRows`|Number of rows to yield per ForkJoinPool merge task, before forking off a new task to continue merging sequences.|16384| -|`druid.processing.merge.task.smallBatchNumRows`|Size of result batches to operate on in ForkJoinPool merge tasks.|4096| +|`druid.processing.merge.parallelism`|Size of ForkJoinPool. Note that the default configuration assumes that the value returned by `Runtime.getRuntime().availableProcessors()` represents 2 hyper-threads per physical core, and multiplies this value by `0.75` in attempt to size `1.5` times the number of _physical_ cores.|`Runtime.getRuntime().availableProcessors() * 0.75` (rounded up)| +|`druid.processing.merge.defaultMaxQueryParallelism`|Default maximum number of parallel merge tasks per query. Note that the default configuration assumes that the value returned by `Runtime.getRuntime().availableProcessors()` represents 2 hyper-threads per physical core, and multiplies this value by `0.5` in attempt to size to the number of _physical_ cores.|`Runtime.getRuntime().availableProcessors() * 0.5` (rounded up)| +|`druid.processing.merge.awaitShutdownMillis`|Time to wait for merge ForkJoinPool tasks to complete before ungracefully stopping on process shutdown in milliseconds.|`60_000`| +|`druid.processing.merge.targetRunTimeMillis`|Ideal run-time of each ForkJoinPool merge task, before forking off a new task to continue merging sequences.|100| +|`druid.processing.merge.initialYieldNumRows`|Number of rows to yield per ForkJoinPool merge task, before forking off a new task to continue merging sequences.|16384| +|`druid.processing.merge.smallBatchNumRows`|Size of result batches to operate on in ForkJoinPool merge tasks.|4096| The amount of direct memory needed by Druid is at least `druid.processing.buffer.sizeBytes * (druid.processing.numMergeBuffers + 1)`. You can diff --git a/docs/configuration/logging.md b/docs/configuration/logging.md index 0764ffcf862e..36b3492bb9e0 100644 --- a/docs/configuration/logging.md +++ b/docs/configuration/logging.md @@ -95,9 +95,6 @@ The following example log4j2.xml is based upon the micro quickstart: - - - diff --git a/docs/querying/query-context.md b/docs/querying/query-context.md index d5ddea04f27d..f62c7f8334f6 100644 --- a/docs/querying/query-context.md +++ b/docs/querying/query-context.md @@ -58,9 +58,9 @@ See [SQL query context](sql-query-context.md) for other query context parameters |`serializeDateTimeAsLong`| `false` | If true, DateTime is serialized as long in the result returned by Broker and the data transportation between Broker and compute process| |`serializeDateTimeAsLongInner`| `false` | If true, DateTime is serialized as long in the data transportation between Broker and compute process| |`enableParallelMerge`|`true`|Enable parallel result merging on the Broker. Note that `druid.processing.merge.useParallelMergePool` must be enabled for this setting to be set to `true`. See [Broker configuration](../configuration/index.md#broker) for more details.| -|`parallelMergeParallelism`|`druid.processing.merge.pool.parallelism`|Maximum number of parallel threads to use for parallel result merging on the Broker. See [Broker configuration](../configuration/index.md#broker) for more details.| -|`parallelMergeInitialYieldRows`|`druid.processing.merge.task.initialYieldNumRows`|Number of rows to yield per ForkJoinPool merge task for parallel result merging on the Broker, before forking off a new task to continue merging sequences. See [Broker configuration](../configuration/index.md#broker) for more details.| -|`parallelMergeSmallBatchRows`|`druid.processing.merge.task.smallBatchNumRows`|Size of result batches to operate on in ForkJoinPool merge tasks for parallel result merging on the Broker. See [Broker configuration](../configuration/index.md#broker) for more details.| +|`parallelMergeParallelism`|`druid.processing.merge.parallelism`|Maximum number of parallel threads to use for parallel result merging on the Broker. See [Broker configuration](../configuration/index.md#broker) for more details.| +|`parallelMergeInitialYieldRows`|`druid.processing.merge.initialYieldNumRows`|Number of rows to yield per ForkJoinPool merge task for parallel result merging on the Broker, before forking off a new task to continue merging sequences. See [Broker configuration](../configuration/index.md#broker) for more details.| +|`parallelMergeSmallBatchRows`|`druid.processing.merge.smallBatchNumRows`|Size of result batches to operate on in ForkJoinPool merge tasks for parallel result merging on the Broker. See [Broker configuration](../configuration/index.md#broker) for more details.| |`useFilterCNF`|`false`| If true, Druid will attempt to convert the query filter to Conjunctive Normal Form (CNF). During query processing, columns can be pre-filtered by intersecting the bitmap indexes of all values that match the eligible filters, often greatly reducing the raw number of rows which need to be scanned. But this effect only happens for the top level filter, or individual clauses of a top level 'and' filter. As such, filters in CNF potentially have a higher chance to utilize a large amount of bitmap indexes on string columns during pre-filtering. However, this setting should be used with great caution, as it can sometimes have a negative effect on performance, and in some cases, the act of computing CNF of a filter can be expensive. We recommend hand tuning your filters to produce an optimal form if possible, or at least verifying through experimentation that using this parameter actually improves your query performance with no ill-effects.| |`secondaryPartitionPruning`|`true`|Enable secondary partition pruning on the Broker. The Broker will always prune unnecessary segments from the input scan based on a filter on time intervals, but if the data is further partitioned with hash or range partitioning, this option will enable additional pruning based on a filter on secondary partition dimensions.| |`debug`| `false` | Flag indicating whether to enable debugging outputs for the query. When set to false, no additional logs will be produced (logs produced will be entirely dependent on your logging level). When set to true, the following addition logs will be produced:
- Log the stack trace of the exception (if any) produced by the query | diff --git a/examples/conf/druid/auto/_common/log4j2.xml b/examples/conf/druid/auto/_common/log4j2.xml index eb24a2a89406..b9f456c43eef 100644 --- a/examples/conf/druid/auto/_common/log4j2.xml +++ b/examples/conf/druid/auto/_common/log4j2.xml @@ -76,9 +76,6 @@ - - - diff --git a/examples/conf/druid/cluster/_common/log4j2.xml b/examples/conf/druid/cluster/_common/log4j2.xml index eb24a2a89406..b9f456c43eef 100644 --- a/examples/conf/druid/cluster/_common/log4j2.xml +++ b/examples/conf/druid/cluster/_common/log4j2.xml @@ -76,9 +76,6 @@ - - - diff --git a/examples/conf/druid/single-server/large/_common/log4j2.xml b/examples/conf/druid/single-server/large/_common/log4j2.xml index eb24a2a89406..b9f456c43eef 100644 --- a/examples/conf/druid/single-server/large/_common/log4j2.xml +++ b/examples/conf/druid/single-server/large/_common/log4j2.xml @@ -76,9 +76,6 @@ - - - diff --git a/examples/conf/druid/single-server/medium/_common/log4j2.xml b/examples/conf/druid/single-server/medium/_common/log4j2.xml index eb24a2a89406..b9f456c43eef 100644 --- a/examples/conf/druid/single-server/medium/_common/log4j2.xml +++ b/examples/conf/druid/single-server/medium/_common/log4j2.xml @@ -76,9 +76,6 @@ - - - diff --git a/examples/conf/druid/single-server/micro-quickstart/_common/log4j2.xml b/examples/conf/druid/single-server/micro-quickstart/_common/log4j2.xml index eb24a2a89406..b9f456c43eef 100644 --- a/examples/conf/druid/single-server/micro-quickstart/_common/log4j2.xml +++ b/examples/conf/druid/single-server/micro-quickstart/_common/log4j2.xml @@ -76,9 +76,6 @@ - - - diff --git a/examples/conf/druid/single-server/nano-quickstart/_common/log4j2.xml b/examples/conf/druid/single-server/nano-quickstart/_common/log4j2.xml index eb24a2a89406..b9f456c43eef 100644 --- a/examples/conf/druid/single-server/nano-quickstart/_common/log4j2.xml +++ b/examples/conf/druid/single-server/nano-quickstart/_common/log4j2.xml @@ -76,9 +76,6 @@ - - - diff --git a/examples/conf/druid/single-server/small/_common/log4j2.xml b/examples/conf/druid/single-server/small/_common/log4j2.xml index eb24a2a89406..b9f456c43eef 100644 --- a/examples/conf/druid/single-server/small/_common/log4j2.xml +++ b/examples/conf/druid/single-server/small/_common/log4j2.xml @@ -76,9 +76,6 @@ - - - diff --git a/examples/conf/druid/single-server/xlarge/_common/log4j2.xml b/examples/conf/druid/single-server/xlarge/_common/log4j2.xml index eb24a2a89406..b9f456c43eef 100644 --- a/examples/conf/druid/single-server/xlarge/_common/log4j2.xml +++ b/examples/conf/druid/single-server/xlarge/_common/log4j2.xml @@ -76,9 +76,6 @@ - - - diff --git a/integration-tests-ex/cases/src/test/java/org/apache/druid/testsEx/config/Initializer.java b/integration-tests-ex/cases/src/test/java/org/apache/druid/testsEx/config/Initializer.java index d420b65dd8df..200cfad17c70 100644 --- a/integration-tests-ex/cases/src/test/java/org/apache/druid/testsEx/config/Initializer.java +++ b/integration-tests-ex/cases/src/test/java/org/apache/druid/testsEx/config/Initializer.java @@ -35,9 +35,9 @@ import org.apache.druid.discovery.DruidNodeDiscoveryProvider; import org.apache.druid.discovery.NodeRole; import org.apache.druid.guice.AnnouncerModule; +import org.apache.druid.guice.BrokerProcessingModule; import org.apache.druid.guice.JsonConfigProvider; import org.apache.druid.guice.LazySingleton; -import org.apache.druid.guice.LegacyBrokerParallelMergeConfigModule; import org.apache.druid.guice.ManageLifecycle; import org.apache.druid.guice.PolyBind; import org.apache.druid.guice.SQLMetadataStorageDruidModule; @@ -508,8 +508,7 @@ private static Injector makeInjector( new AnnouncerModule(), new DiscoveryModule(), // Dependencies from other modules - new LegacyBrokerParallelMergeConfigModule(), - // Dependencies from other modules + new BrokerProcessingModule(), new StorageNodeModule(), new MSQExternalDataSourceModule(), diff --git a/licenses.yaml b/licenses.yaml index b72b819d79e4..8f46132a18fe 100644 --- a/licenses.yaml +++ b/licenses.yaml @@ -2493,16 +2493,6 @@ libraries: --- -name: Config Magic -license_category: binary -module: java-core -license_name: Apache License version 2.0 -version: 0.9 -libraries: - - org.skife.config: config-magic - ---- - name: Apache Hadoop license_category: binary module: hadoop-client diff --git a/pom.xml b/pom.xml index fc7d78ccecb0..00d7492ceecf 100644 --- a/pom.xml +++ b/pom.xml @@ -388,17 +388,6 @@ airline 2.8.4 - - org.skife.config - config-magic - 0.9 - - - org.slf4j - slf4j-api - - - net.minidev json-smart diff --git a/processing/pom.xml b/processing/pom.xml index a320ad89c12a..2c985df48c49 100644 --- a/processing/pom.xml +++ b/processing/pom.xml @@ -65,10 +65,6 @@ com.fasterxml.jackson.dataformat jackson-dataformat-smile - - org.skife.config - config-magic - commons-io commons-io diff --git a/processing/src/main/java/org/apache/druid/guice/ConfigModule.java b/processing/src/main/java/org/apache/druid/guice/ConfigModule.java index 522ee63337e4..835999e634b2 100644 --- a/processing/src/main/java/org/apache/druid/guice/ConfigModule.java +++ b/processing/src/main/java/org/apache/druid/guice/ConfigModule.java @@ -21,13 +21,9 @@ import com.google.inject.Binder; import com.google.inject.Module; -import com.google.inject.Provides; -import org.apache.druid.java.util.common.config.Config; -import org.skife.config.ConfigurationObjectFactory; import javax.validation.Validation; import javax.validation.Validator; -import java.util.Properties; /** */ @@ -39,10 +35,4 @@ public void configure(Binder binder) binder.bind(Validator.class).toInstance(Validation.buildDefaultValidatorFactory().getValidator()); binder.bind(JsonConfigurator.class).in(LazySingleton.class); } - - @Provides @LazySingleton - public ConfigurationObjectFactory makeFactory(Properties props) - { - return Config.createFactory(props); - } } diff --git a/processing/src/main/java/org/apache/druid/guice/ConfigProvider.java b/processing/src/main/java/org/apache/druid/guice/ConfigProvider.java deleted file mode 100644 index 682dce0f7cb4..000000000000 --- a/processing/src/main/java/org/apache/druid/guice/ConfigProvider.java +++ /dev/null @@ -1,87 +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.guice; - -import com.google.common.base.Preconditions; -import com.google.inject.Binder; -import com.google.inject.Inject; -import com.google.inject.Provider; -import org.apache.druid.java.util.common.logger.Logger; -import org.skife.config.ConfigurationObjectFactory; - -import java.util.Map; - -/** - * - */ -@Deprecated -public class ConfigProvider implements Provider -{ - private static final Logger log = new Logger(ConfigProvider.class); - - public static void bind(Binder binder, Class clazz) - { - binder.bind(clazz).toProvider(of(clazz)).in(LazySingleton.class); - } - - public static Provider of(Class clazz) - { - return of(clazz, null); - } - - public static Provider of(Class clazz, Map replacements) - { - return new ConfigProvider<>(clazz, replacements); - } - - private final Class clazz; - private final Map replacements; - - private ConfigurationObjectFactory factory = null; - - public ConfigProvider( - Class clazz, - Map replacements - ) - { - this.clazz = clazz; - this.replacements = replacements; - } - - @Inject - public void inject(ConfigurationObjectFactory factory) - { - this.factory = factory; - } - - @Override - public T get() - { - try { - // ConfigMagic handles a null replacements - Preconditions.checkNotNull(factory, "Code misconfigured, inject() didn't get called."); - return factory.buildWithReplacements(clazz, replacements); - } - catch (IllegalArgumentException e) { - log.info("Unable to build instance of class[%s]", clazz); - throw e; - } - } -} diff --git a/processing/src/main/java/org/apache/druid/guice/DruidSecondaryModule.java b/processing/src/main/java/org/apache/druid/guice/DruidSecondaryModule.java index a22ef051457b..be0ea786f58e 100644 --- a/processing/src/main/java/org/apache/druid/guice/DruidSecondaryModule.java +++ b/processing/src/main/java/org/apache/druid/guice/DruidSecondaryModule.java @@ -31,7 +31,6 @@ import org.apache.druid.guice.annotations.Json; import org.apache.druid.guice.annotations.JsonNonNull; import org.apache.druid.guice.annotations.Smile; -import org.skife.config.ConfigurationObjectFactory; import javax.validation.Validator; import java.util.Properties; @@ -40,7 +39,6 @@ public class DruidSecondaryModule implements Module { private final Properties properties; - private final ConfigurationObjectFactory factory; private final ObjectMapper jsonMapper; private final ObjectMapper jsonMapperOnlyNonNullValueSerialization; private final ObjectMapper smileMapper; @@ -49,7 +47,6 @@ public class DruidSecondaryModule implements Module @Inject public DruidSecondaryModule( Properties properties, - ConfigurationObjectFactory factory, @Json ObjectMapper jsonMapper, @JsonNonNull ObjectMapper jsonMapperOnlyNonNullValueSerialization, @Smile ObjectMapper smileMapper, @@ -57,7 +54,6 @@ public DruidSecondaryModule( ) { this.properties = properties; - this.factory = factory; this.jsonMapper = jsonMapper; this.jsonMapperOnlyNonNullValueSerialization = jsonMapperOnlyNonNullValueSerialization; this.smileMapper = smileMapper; @@ -69,7 +65,6 @@ public void configure(Binder binder) { binder.install(new DruidGuiceExtensions()); binder.bind(Properties.class).toInstance(properties); - binder.bind(ConfigurationObjectFactory.class).toInstance(factory); binder.bind(ObjectMapper.class).to(Key.get(ObjectMapper.class, Json.class)); binder.bind(Validator.class).toInstance(validator); binder.bind(JsonConfigurator.class); diff --git a/processing/src/main/java/org/apache/druid/guice/StartupInjectorBuilder.java b/processing/src/main/java/org/apache/druid/guice/StartupInjectorBuilder.java index bdf73993efac..1d524a9a1e79 100644 --- a/processing/src/main/java/org/apache/druid/guice/StartupInjectorBuilder.java +++ b/processing/src/main/java/org/apache/druid/guice/StartupInjectorBuilder.java @@ -165,6 +165,51 @@ public void validate() docsLink ); } + + validateRemovedProcessingConfigs(); + } + + private void validateRemovedProcessingConfigs() + { + checkDeletedConfigAndThrow( + "druid.processing.merge.task.initialYieldNumRows", + "druid.processing.merge.initialYieldNumRows" + ); + checkDeletedConfigAndThrow( + "druid.processing.merge.task.targetRunTimeMillis", + "druid.processing.merge.targetRunTimeMillis" + ); + checkDeletedConfigAndThrow( + "druid.processing.merge.task.smallBatchNumRows", + "druid.processing.merge.smallBatchNumRows" + ); + + checkDeletedConfigAndThrow( + "druid.processing.merge.pool.awaitShutdownMillis", + "druid.processing.merge.awaitShutdownMillis" + ); + checkDeletedConfigAndThrow( + "druid.processing.merge.pool.parallelism", + "druid.processing.merge.parallelism" + ); + checkDeletedConfigAndThrow( + "druid.processing.merge.pool.defaultMaxQueryParallelism", + "druid.processing.merge.defaultMaxQueryParallelism" + ); + } + + /** + * Checks if a deleted config is present in the properties and throws an ISE. + */ + private void checkDeletedConfigAndThrow(String deletedConfigName, String replaceConfigName) + { + if (properties.getProperty(deletedConfigName) != null) { + throw new ISE( + "Config[%s] has been removed. Please use config[%s] instead.", + deletedConfigName, + replaceConfigName + ); + } } } diff --git a/processing/src/main/java/org/apache/druid/java/util/common/config/Config.java b/processing/src/main/java/org/apache/druid/java/util/common/config/Config.java deleted file mode 100644 index e5a7c96275f0..000000000000 --- a/processing/src/main/java/org/apache/druid/java/util/common/config/Config.java +++ /dev/null @@ -1,36 +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.java.util.common.config; - -import org.skife.config.ConfigurationObjectFactory; - -import java.util.Properties; - -/** -*/ -public class Config -{ - public static ConfigurationObjectFactory createFactory(Properties props) - { - ConfigurationObjectFactory configFactory = new ConfigurationObjectFactory(props); - configFactory.addCoercible(new DurationCoercible()); - return configFactory; - } -} diff --git a/processing/src/main/java/org/apache/druid/java/util/common/config/DurationCoercible.java b/processing/src/main/java/org/apache/druid/java/util/common/config/DurationCoercible.java deleted file mode 100644 index 5ba543bbfc08..000000000000 --- a/processing/src/main/java/org/apache/druid/java/util/common/config/DurationCoercible.java +++ /dev/null @@ -1,47 +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.java.util.common.config; - -import org.joda.time.Duration; -import org.joda.time.Period; -import org.skife.config.Coercer; -import org.skife.config.Coercible; - -/** -*/ -public class DurationCoercible implements Coercible -{ - @Override - public Coercer accept(Class clazz) - { - if (Duration.class != clazz) { - return null; - } - - return new Coercer<>() - { - @Override - public Duration coerce(String value) - { - return new Period(value).toStandardDuration(); - } - }; - } -} diff --git a/processing/src/test/java/org/apache/druid/guice/StartupInjectorBuilderTest.java b/processing/src/test/java/org/apache/druid/guice/StartupInjectorBuilderTest.java index 892434defcee..bcf85742c2ac 100644 --- a/processing/src/test/java/org/apache/druid/guice/StartupInjectorBuilderTest.java +++ b/processing/src/test/java/org/apache/druid/guice/StartupInjectorBuilderTest.java @@ -21,10 +21,12 @@ import com.google.inject.Injector; import org.apache.druid.common.config.NullValueHandlingConfig; +import org.apache.druid.error.ExceptionMatcher; import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.math.expr.ExpressionProcessingConfig; import org.apache.druid.utils.RuntimeInfo; +import org.hamcrest.MatcherAssert; import org.junit.Assert; import org.junit.Test; @@ -194,4 +196,62 @@ public void testValidator() t.getMessage() ); } + + @Test + public void verifyInjectorBuild_withDeletedConfig_throwsException() + { + verifyInjectorBuild_withDeletedConfig_throwsException( + "druid.processing.merge.pool.parallelism", + "10", + "Config[druid.processing.merge.pool.parallelism] has been removed." + + " Please use config[druid.processing.merge.parallelism] instead." + ); + verifyInjectorBuild_withDeletedConfig_throwsException( + "druid.processing.merge.pool.awaitShutdownMillis", + "1000", + "Config[druid.processing.merge.pool.awaitShutdownMillis] has been removed." + + " Please use config[druid.processing.merge.awaitShutdownMillis] instead." + ); + verifyInjectorBuild_withDeletedConfig_throwsException( + "druid.processing.merge.pool.defaultMaxQueryParallelism", + "100", + "Config[druid.processing.merge.pool.defaultMaxQueryParallelism] has been removed." + + " Please use config[druid.processing.merge.defaultMaxQueryParallelism] instead." + ); + + verifyInjectorBuild_withDeletedConfig_throwsException( + "druid.processing.merge.task.targetRunTimeMillis", + "10", + "Config[druid.processing.merge.task.targetRunTimeMillis] has been removed." + + " Please use config[druid.processing.merge.targetRunTimeMillis] instead." + ); + verifyInjectorBuild_withDeletedConfig_throwsException( + "druid.processing.merge.task.initialYieldNumRows", + "1000", + "Config[druid.processing.merge.task.initialYieldNumRows] has been removed." + + " Please use config[druid.processing.merge.initialYieldNumRows] instead." + ); + verifyInjectorBuild_withDeletedConfig_throwsException( + "druid.processing.merge.task.smallBatchNumRows", + "100", + "Config[druid.processing.merge.task.smallBatchNumRows] has been removed." + + " Please use config[druid.processing.merge.smallBatchNumRows] instead." + ); + } + + private static void verifyInjectorBuild_withDeletedConfig_throwsException( + String removedProperty, + String dummyValue, + String expectedMessage + ) + { + final Properties props = new Properties(); + props.setProperty(removedProperty, dummyValue); + + final StartupInjectorBuilder builder = new StartupInjectorBuilder().withExtensions().withProperties(props); + MatcherAssert.assertThat( + Assert.assertThrows(ISE.class, builder::build), + ExceptionMatcher.of(ISE.class).expectMessageIs(expectedMessage) + ); + } } diff --git a/server/pom.xml b/server/pom.xml index eb664b51a9fc..1dd27993e313 100644 --- a/server/pom.xml +++ b/server/pom.xml @@ -254,10 +254,6 @@ com.google.guava guava - - org.skife.config - config-magic - org.apache.curator curator-recipes diff --git a/server/src/main/java/org/apache/druid/guice/LegacyBrokerParallelMergeConfigModule.java b/server/src/main/java/org/apache/druid/guice/LegacyBrokerParallelMergeConfigModule.java deleted file mode 100644 index 3d44cf23f72b..000000000000 --- a/server/src/main/java/org/apache/druid/guice/LegacyBrokerParallelMergeConfigModule.java +++ /dev/null @@ -1,42 +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.guice; - -import com.google.inject.Binder; -import com.google.inject.Module; -import org.apache.druid.query.LegacyBrokerParallelMergeConfig; - -/** - * Backwards compatibility for runtime.properties for Druid 27 and older to make deprecated config paths of - * {@link LegacyBrokerParallelMergeConfig} still work for Druid 28. - * {@link org.apache.druid.query.BrokerParallelMergeConfig} has replaced these configs, and will warn when these - * deprecated paths are configured. This module should be removed in Druid 29, along with - * {@link LegacyBrokerParallelMergeConfig} as well as the config-magic library that makes it work. - */ -@Deprecated -public class LegacyBrokerParallelMergeConfigModule implements Module -{ - - @Override - public void configure(Binder binder) - { - ConfigProvider.bind(binder, LegacyBrokerParallelMergeConfig.class); - } -} diff --git a/server/src/main/java/org/apache/druid/query/BrokerParallelMergeConfig.java b/server/src/main/java/org/apache/druid/query/BrokerParallelMergeConfig.java index 1f8d25a10c2d..a7f0123c155a 100644 --- a/server/src/main/java/org/apache/druid/query/BrokerParallelMergeConfig.java +++ b/server/src/main/java/org/apache/druid/query/BrokerParallelMergeConfig.java @@ -19,10 +19,10 @@ package org.apache.druid.query; -import com.fasterxml.jackson.annotation.JacksonInject; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.annotations.VisibleForTesting; +import org.apache.druid.common.config.Configs; import org.apache.druid.java.util.common.guava.ParallelMergeCombiningSequence; import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.utils.JvmUtils; @@ -57,25 +57,15 @@ public BrokerParallelMergeConfig( @JsonProperty("defaultMaxQueryParallelism") @Nullable Integer defaultMaxQueryParallelism, @JsonProperty("targetRunTimeMillis") @Nullable Integer targetRunTimeMillis, @JsonProperty("initialYieldNumRows") @Nullable Integer initialYieldNumRows, - @JsonProperty("smallBatchNumRows") @Nullable Integer smallBatchNumRows, - @JacksonInject LegacyBrokerParallelMergeConfig oldConfig + @JsonProperty("smallBatchNumRows") @Nullable Integer smallBatchNumRows ) { - if (parallelism == null) { - if (oldConfig == null || oldConfig.getMergePoolParallelism() == null) { + this.parallelism = Configs.valueOrDefault( + parallelism, // assume 2 hyper-threads per core, so that this value is probably by default the number // of physical cores * 1.5 - this.parallelism = (int) Math.ceil(JvmUtils.getRuntimeInfo().getAvailableProcessors() * 0.75); - } else { - warnDeprecated( - "druid.processing.merge.pool.parallelism", - "druid.processing.merge.parallelism" - ); - this.parallelism = oldConfig.getMergePoolParallelism(); - } - } else { - this.parallelism = parallelism; - } + (int) Math.ceil(JvmUtils.getRuntimeInfo().getAvailableProcessors() * 0.75) + ); // need at least 3 to do 2 layer merge if (this.parallelism > 2) { @@ -90,81 +80,36 @@ public BrokerParallelMergeConfig( this.useParallelMergePool = false; } - if (awaitShutdownMillis == null) { - if (oldConfig == null || oldConfig.getMergePoolAwaitShutdownMillis() == null) { - this.awaitShutdownMillis = DEFAULT_MERGE_POOL_AWAIT_SHUTDOWN_MILLIS; - } else { - warnDeprecated( - "druid.processing.merge.pool.awaitShutdownMillis", - "druid.processing.merge.awaitShutdownMillis" - ); - this.awaitShutdownMillis = oldConfig.getMergePoolAwaitShutdownMillis(); - } - } else { - this.awaitShutdownMillis = awaitShutdownMillis; - } + this.awaitShutdownMillis = Configs.valueOrDefault( + awaitShutdownMillis, + DEFAULT_MERGE_POOL_AWAIT_SHUTDOWN_MILLIS + ); - if (defaultMaxQueryParallelism == null) { - if (oldConfig == null || oldConfig.getMergePoolDefaultMaxQueryParallelism() == null) { - this.defaultMaxQueryParallelism = (int) Math.max(JvmUtils.getRuntimeInfo().getAvailableProcessors() * 0.5, 1); - } else { - warnDeprecated( - "druid.processing.merge.pool.defaultMaxQueryParallelism", - "druid.processing.merge.defaultMaxQueryParallelism" - ); - this.defaultMaxQueryParallelism = oldConfig.getMergePoolDefaultMaxQueryParallelism(); - } - } else { - this.defaultMaxQueryParallelism = defaultMaxQueryParallelism; - } + this.defaultMaxQueryParallelism = Configs.valueOrDefault( + defaultMaxQueryParallelism, + (int) Math.max(JvmUtils.getRuntimeInfo().getAvailableProcessors() * 0.5, 1) + ); - if (targetRunTimeMillis == null) { - if (oldConfig == null || oldConfig.getMergePoolTargetTaskRunTimeMillis() == null) { - this.targetRunTimeMillis = ParallelMergeCombiningSequence.DEFAULT_TASK_TARGET_RUN_TIME_MILLIS; - } else { - warnDeprecated( - "druid.processing.merge.task.targetRunTimeMillis", - "druid.processing.merge.targetRunTimeMillis" - ); - this.targetRunTimeMillis = oldConfig.getMergePoolTargetTaskRunTimeMillis(); - } - } else { - this.targetRunTimeMillis = targetRunTimeMillis; - } + this.targetRunTimeMillis = Configs.valueOrDefault( + targetRunTimeMillis, + ParallelMergeCombiningSequence.DEFAULT_TASK_TARGET_RUN_TIME_MILLIS + ); - if (initialYieldNumRows == null) { - if (oldConfig == null || oldConfig.getMergePoolTaskInitialYieldRows() == null) { - this.initialYieldNumRows = ParallelMergeCombiningSequence.DEFAULT_TASK_INITIAL_YIELD_NUM_ROWS; - } else { - warnDeprecated( - "druid.processing.merge.task.initialYieldNumRows", - "druid.processing.merge.initialYieldNumRows" - ); - this.initialYieldNumRows = oldConfig.getMergePoolTaskInitialYieldRows(); - } - } else { - this.initialYieldNumRows = initialYieldNumRows; - } + this.initialYieldNumRows = Configs.valueOrDefault( + initialYieldNumRows, + ParallelMergeCombiningSequence.DEFAULT_TASK_INITIAL_YIELD_NUM_ROWS + ); - if (smallBatchNumRows == null) { - if (oldConfig == null || oldConfig.getMergePoolSmallBatchRows() == null) { - this.smallBatchNumRows = ParallelMergeCombiningSequence.DEFAULT_TASK_SMALL_BATCH_NUM_ROWS; - } else { - warnDeprecated( - "druid.processing.merge.task.smallBatchNumRows", - "druid.processing.merge.smallBatchNumRows" - ); - this.smallBatchNumRows = oldConfig.getMergePoolSmallBatchRows(); - } - } else { - this.smallBatchNumRows = smallBatchNumRows; - } + this.smallBatchNumRows = Configs.valueOrDefault( + smallBatchNumRows, + ParallelMergeCombiningSequence.DEFAULT_TASK_SMALL_BATCH_NUM_ROWS + ); } @VisibleForTesting public BrokerParallelMergeConfig() { - this(null, null, null, null, null, null, null, null); + this(null, null, null, null, null, null, null); } public boolean useParallelMergePool() @@ -201,15 +146,4 @@ public int getSmallBatchNumRows() { return smallBatchNumRows; } - - private static void warnDeprecated(String oldPath, String newPath) - { - LOG.warn( - "Using deprecated config [%s] which has been replace by [%s]. This path is deprecated and will be " - + "removed in a future release, please transition to using [%s]", - oldPath, - newPath, - newPath - ); - } } diff --git a/server/src/main/java/org/apache/druid/query/LegacyBrokerParallelMergeConfig.java b/server/src/main/java/org/apache/druid/query/LegacyBrokerParallelMergeConfig.java deleted file mode 100644 index 25b11ab63d17..000000000000 --- a/server/src/main/java/org/apache/druid/query/LegacyBrokerParallelMergeConfig.java +++ /dev/null @@ -1,74 +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.query; - -import org.skife.config.Config; - -import javax.annotation.Nullable; - -/** - * Backwards compatibility for Druid 27 and older runtime.properties configs, replaced by - * {@link BrokerParallelMergeConfig} in Druid 28. This config should be removed in Druid 29. - */ -@Deprecated -public abstract class LegacyBrokerParallelMergeConfig -{ - @Nullable - @Config(value = "druid.processing.merge.pool.parallelism") - public Integer getMergePoolParallelism() - { - return null; - } - - @Nullable - @Config(value = "druid.processing.merge.pool.awaitShutdownMillis") - public Long getMergePoolAwaitShutdownMillis() - { - return null; - } - - @Nullable - @Config(value = "druid.processing.merge.pool.defaultMaxQueryParallelism") - public Integer getMergePoolDefaultMaxQueryParallelism() - { - return null; - } - - @Nullable - @Config(value = "druid.processing.merge.task.targetRunTimeMillis") - public Integer getMergePoolTargetTaskRunTimeMillis() - { - return null; - } - - @Nullable - @Config(value = "druid.processing.merge.task.initialYieldNumRows") - public Integer getMergePoolTaskInitialYieldRows() - { - return null; - } - - @Nullable - @Config(value = "druid.processing.merge.task.smallBatchNumRows") - public Integer getMergePoolSmallBatchRows() - { - return null; - } -} diff --git a/server/src/test/java/org/apache/druid/guice/BrokerProcessingModuleTest.java b/server/src/test/java/org/apache/druid/guice/BrokerProcessingModuleTest.java index 7fca81aaea30..a4bf2e282114 100644 --- a/server/src/test/java/org/apache/druid/guice/BrokerProcessingModuleTest.java +++ b/server/src/test/java/org/apache/druid/guice/BrokerProcessingModuleTest.java @@ -28,10 +28,8 @@ import org.apache.druid.client.cache.CachePopulator; import org.apache.druid.client.cache.CachePopulatorStats; import org.apache.druid.initialization.Initialization; -import org.apache.druid.java.util.common.config.Config; import org.apache.druid.query.BrokerParallelMergeConfig; import org.apache.druid.query.DruidProcessingConfig; -import org.apache.druid.query.LegacyBrokerParallelMergeConfig; import org.apache.druid.utils.JvmUtils; import org.junit.Assert; import org.junit.Assume; @@ -40,7 +38,6 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; -import org.skife.config.ConfigurationObjectFactory; import java.util.Properties; @@ -85,20 +82,6 @@ public void testMergeProcessingPool() module.getMergeProcessingPoolProvider(config); } - @Test - public void testMergeProcessingPoolLegacyConfigs() - { - Properties props = new Properties(); - props.put("druid.processing.merge.pool.parallelism", "10"); - props.put("druid.processing.merge.pool.defaultMaxQueryParallelism", "10"); - props.put("druid.processing.merge.task.targetRunTimeMillis", "1000"); - Injector gadget = makeInjector(props); - BrokerParallelMergeConfig config = gadget.getInstance(BrokerParallelMergeConfig.class); - Assert.assertEquals(10, config.getParallelism()); - Assert.assertEquals(10, config.getDefaultMaxQueryParallelism()); - Assert.assertEquals(1000, config.getTargetRunTimeMillis()); - } - @Test public void testCachePopulatorAsSingleton() { @@ -138,10 +121,6 @@ private Injector makeInjector(Properties props) binder.bindConstant().annotatedWith(Names.named("servicePort")).to(0); binder.bindConstant().annotatedWith(Names.named("tlsServicePort")).to(-1); binder.bind(Properties.class).toInstance(props); - ConfigurationObjectFactory factory = Config.createFactory(props); - LegacyBrokerParallelMergeConfig legacyConfig = factory.build(LegacyBrokerParallelMergeConfig.class); - binder.bind(ConfigurationObjectFactory.class).toInstance(factory); - binder.bind(LegacyBrokerParallelMergeConfig.class).toInstance(legacyConfig); }, target ).with((binder) -> { diff --git a/server/src/test/java/org/apache/druid/guice/QueryableModuleTest.java b/server/src/test/java/org/apache/druid/guice/QueryableModuleTest.java index 8fad8924bf88..bc74551d60a3 100644 --- a/server/src/test/java/org/apache/druid/guice/QueryableModuleTest.java +++ b/server/src/test/java/org/apache/druid/guice/QueryableModuleTest.java @@ -77,7 +77,6 @@ private Injector makeInjector(Properties properties) new JacksonModule(), new ConfigModule(), new QueryRunnerFactoryModule(), - new LegacyBrokerParallelMergeConfigModule(), new BrokerProcessingModule(), new LifecycleModule(), binder -> binder.bind(ServiceEmitter.class).to(NoopServiceEmitter.class), diff --git a/services/src/main/java/org/apache/druid/cli/CliBroker.java b/services/src/main/java/org/apache/druid/cli/CliBroker.java index 09212ec75972..42bb7b51421c 100644 --- a/services/src/main/java/org/apache/druid/cli/CliBroker.java +++ b/services/src/main/java/org/apache/druid/cli/CliBroker.java @@ -47,7 +47,6 @@ import org.apache.druid.guice.JoinableFactoryModule; import org.apache.druid.guice.JsonConfigProvider; import org.apache.druid.guice.LazySingleton; -import org.apache.druid.guice.LegacyBrokerParallelMergeConfigModule; import org.apache.druid.guice.LifecycleModule; import org.apache.druid.guice.ManageLifecycle; import org.apache.druid.guice.QueryRunnerFactoryModule; @@ -117,7 +116,6 @@ protected Set getNodeRoles(Properties properties) protected List getModules() { return ImmutableList.of( - new LegacyBrokerParallelMergeConfigModule(), new BrokerProcessingModule(), new QueryableModule(), new QueryRunnerFactoryModule(),