From 462365a12d00275f0d4a58ebb519527148bd038b Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Fri, 7 Aug 2020 14:52:34 -0500 Subject: [PATCH 1/2] Default server.maxsize --- docs/configuration/index.md | 2 +- .../druid/client/DruidServerConfig.java | 20 +++ .../segment/loading/SegmentLoaderConfig.java | 11 ++ .../druid/client/DruidServerConfigTest.java | 130 ++++++++++++++++++ .../druid/guice/StorageNodeModuleTest.java | 35 ++--- 5 files changed, 177 insertions(+), 21 deletions(-) create mode 100644 server/src/test/java/org/apache/druid/client/DruidServerConfigTest.java diff --git a/docs/configuration/index.md b/docs/configuration/index.md index 8e363fdc2666..b02c467141a1 100644 --- a/docs/configuration/index.md +++ b/docs/configuration/index.md @@ -1370,7 +1370,7 @@ These Historical configurations can be defined in the `historical/runtime.proper |Property|Description|Default| |--------|-----------|-------| -|`druid.server.maxSize`|The maximum number of bytes-worth of segments that the process wants assigned to it. The Coordinator process will attempt to assign segments to a Historical process only if this property is greater than the total size of segments served by it. Since this property defines the upper limit on the total segment size that can be assigned to a Historical, it can be set to the sum of all `maxSize` values specified within `druid.segmentCache.locations` property. Human-readable format is supported, see [here](human-readable-byte.md). |0| +|`druid.server.maxSize`|The maximum number of bytes-worth of segments that the process wants assigned to it. The Coordinator process will attempt to assign segments to a Historical process only if this property is greater than the total size of segments served by it. Since this property defines the upper limit on the total segment size that can be assigned to a Historical, it is defaulted to the sum of all `maxSize` values specified within `druid.segmentCache.locations` property. Human-readable format is supported, see [here](human-readable-byte.md). |Sum of `maxSize` values defined within `druid.segmentCache.locations`| |`druid.server.tier`| A string to name the distribution tier that the storage process belongs to. Many of the [rules Coordinator processes use](../operations/rule-configuration.md) to manage segments can be keyed on tiers. | `_default_tier` | |`druid.server.priority`|In a tiered architecture, the priority of the tier, thus allowing control over which processes are queried. Higher numbers mean higher priority. The default (no priority) works for architecture with no cross replication (tiers that have no data-storage overlap). Data centers typically have equal priority. | 0 | diff --git a/server/src/main/java/org/apache/druid/client/DruidServerConfig.java b/server/src/main/java/org/apache/druid/client/DruidServerConfig.java index 3b7e8f228e23..7c15789e5e8b 100644 --- a/server/src/main/java/org/apache/druid/client/DruidServerConfig.java +++ b/server/src/main/java/org/apache/druid/client/DruidServerConfig.java @@ -19,10 +19,14 @@ package org.apache.druid.client; +import com.fasterxml.jackson.annotation.JacksonInject; +import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.collect.Sets; +import com.google.inject.Inject; import org.apache.druid.java.util.common.HumanReadableBytes; import org.apache.druid.java.util.common.HumanReadableBytesRange; +import org.apache.druid.segment.loading.SegmentLoaderConfig; import javax.validation.constraints.NotNull; import java.util.Set; @@ -45,8 +49,23 @@ public class DruidServerConfig @NotNull private Set hiddenProperties = Sets.newHashSet("druid.s3.accessKey", "druid.s3.secretKey", "druid.metadata.storage.connector.password"); + private SegmentLoaderConfig segmentLoaderConfig; + + // Guice inject added here to properly bind this dependency into its dependents such as StatusResource + @Inject + @JsonCreator + public DruidServerConfig( + @JacksonInject SegmentLoaderConfig segmentLoaderConfig + ) + { + this.segmentLoaderConfig = segmentLoaderConfig; + } + public long getMaxSize() { + if (maxSize.equals(HumanReadableBytes.ZERO)) { + return segmentLoaderConfig.getCombinedMaxSize(); + } return maxSize.getBytes(); } @@ -64,4 +83,5 @@ public Set getHiddenProperties() { return hiddenProperties; } + } diff --git a/server/src/main/java/org/apache/druid/segment/loading/SegmentLoaderConfig.java b/server/src/main/java/org/apache/druid/segment/loading/SegmentLoaderConfig.java index 883ac5b0211e..edd2c0ddb6aa 100644 --- a/server/src/main/java/org/apache/druid/segment/loading/SegmentLoaderConfig.java +++ b/server/src/main/java/org/apache/druid/segment/loading/SegmentLoaderConfig.java @@ -30,6 +30,7 @@ import java.util.concurrent.TimeUnit; /** + * */ public class SegmentLoaderConfig { @@ -63,6 +64,8 @@ public class SegmentLoaderConfig @JsonProperty private int statusQueueMaxSize = 100; + private long combinedMaxSize = 0; + public List getLocations() { return locations; @@ -120,6 +123,14 @@ public int getStatusQueueMaxSize() return statusQueueMaxSize; } + public long getCombinedMaxSize() + { + if (combinedMaxSize == 0) { + combinedMaxSize = getLocations().stream().mapToLong(StorageLocationConfig::getMaxSize).sum(); + } + return combinedMaxSize; + } + public SegmentLoaderConfig withLocations(List locations) { SegmentLoaderConfig retVal = new SegmentLoaderConfig(); diff --git a/server/src/test/java/org/apache/druid/client/DruidServerConfigTest.java b/server/src/test/java/org/apache/druid/client/DruidServerConfigTest.java new file mode 100644 index 000000000000..b3d962b4812e --- /dev/null +++ b/server/src/test/java/org/apache/druid/client/DruidServerConfigTest.java @@ -0,0 +1,130 @@ +/* + * 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.client; + +import com.fasterxml.jackson.databind.InjectableValues; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.collect.ImmutableList; +import com.google.inject.Injector; +import com.google.inject.Module; +import com.google.inject.name.Names; +import org.apache.druid.guice.GuiceInjectors; +import org.apache.druid.initialization.Initialization; +import org.apache.druid.jackson.DefaultObjectMapper; +import org.apache.druid.segment.loading.SegmentLoaderConfig; +import org.apache.druid.segment.loading.StorageLocationConfig; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +import java.io.File; +import java.util.ArrayList; +import java.util.List; + +public class DruidServerConfigTest +{ + private File testSegmentCacheDir1; + private File testSegmentCacheDir2; + + @Rule + public final TemporaryFolder tmpFolder = new TemporaryFolder(); + + public ObjectMapper mapper = new DefaultObjectMapper(); + + private static final Module SERVER_CONFIG_MODULE = (binder) -> { + binder.bindConstant().annotatedWith(Names.named("serviceName")).to("druid/test"); + binder.bindConstant().annotatedWith(Names.named("servicePort")).to(0); + binder.bindConstant().annotatedWith(Names.named("tlsServicePort")).to(-1); + }; + + @Before + public void setUp() throws Exception + { + testSegmentCacheDir1 = tmpFolder.newFolder("segment_cache_folder1"); + testSegmentCacheDir2 = tmpFolder.newFolder("segment_cache_folder2"); + + } + + @Test + public void testBasicInjection() + { + final Injector injector = Initialization.makeInjectorWithModules( + GuiceInjectors.makeStartupInjector(), ImmutableList.of(SERVER_CONFIG_MODULE) + ); + final DruidServerConfig druidServerConfig = injector.getInstance(DruidServerConfig.class); + + Assert.assertNotNull(druidServerConfig); + Assert.assertEquals(DruidServerConfig.class, druidServerConfig.getClass()); + + } + + @Test + public void testCombinedSize() + { + final List locations = new ArrayList<>(); + final StorageLocationConfig locationConfig1 = new StorageLocationConfig(testSegmentCacheDir1, 10000000000L, null); + final StorageLocationConfig locationConfig2 = new StorageLocationConfig(testSegmentCacheDir2, 20000000000L, null); + locations.add(locationConfig1); + locations.add(locationConfig2); + DruidServerConfig druidServerConfig = new DruidServerConfig(new SegmentLoaderConfig().withLocations(locations)); + Assert.assertEquals(30000000000L, druidServerConfig.getMaxSize()); + } + + @Test + public void testServerMaxSizePrecedence() throws Exception + { + String serverConfigWithDefaultSizeStr = "{\"maxSize\":0,\"tier\":\"_default_tier\",\"priority\":0," + + "\"hiddenProperties\":[\"druid.metadata.storage.connector.password\"," + + "\"druid.s3.accessKey\",\"druid.s3.secretKey\"]}\n"; + + String serverConfigWithNonDefaultSizeStr = "{\"maxSize\":123456,\"tier\":\"_default_tier\",\"priority\":0," + + "\"hiddenProperties\":[\"druid.metadata.storage.connector.password\"," + + "\"druid.s3.accessKey\",\"druid.s3.secretKey\"]}\n"; + + final List locations = new ArrayList<>(); + final StorageLocationConfig locationConfig1 = new StorageLocationConfig(testSegmentCacheDir1, 10000000000L, null); + locations.add(locationConfig1); + mapper.setInjectableValues(new InjectableValues.Std().addValue(ObjectMapper.class, new DefaultObjectMapper()) + .addValue( + SegmentLoaderConfig.class, + new SegmentLoaderConfig().withLocations(locations) + )); + + DruidServerConfig serverConfigWithDefaultSize = mapper.readValue( + mapper.writeValueAsString( + mapper.readValue(serverConfigWithDefaultSizeStr, DruidServerConfig.class) + ), + DruidServerConfig.class + ); + + DruidServerConfig serverConfigWithNonDefaultSize = mapper.readValue( + mapper.writeValueAsString( + mapper.readValue(serverConfigWithNonDefaultSizeStr, DruidServerConfig.class) + ), + DruidServerConfig.class + ); + + Assert.assertEquals(serverConfigWithDefaultSize.getMaxSize(), 10000000000L); + Assert.assertEquals(serverConfigWithNonDefaultSize.getMaxSize(), 123456L); + } +} + diff --git a/server/src/test/java/org/apache/druid/guice/StorageNodeModuleTest.java b/server/src/test/java/org/apache/druid/guice/StorageNodeModuleTest.java index c844cdd1e80c..e849bb6910e8 100644 --- a/server/src/test/java/org/apache/druid/guice/StorageNodeModuleTest.java +++ b/server/src/test/java/org/apache/druid/guice/StorageNodeModuleTest.java @@ -19,16 +19,15 @@ package org.apache.druid.guice; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.google.inject.Guice; +import com.google.common.collect.ImmutableList; import com.google.inject.Injector; import com.google.inject.Key; import com.google.inject.ProvisionException; -import com.google.inject.Scopes; import com.google.inject.name.Names; import com.google.inject.util.Modules; import org.apache.druid.discovery.DataNodeService; import org.apache.druid.guice.annotations.Self; +import org.apache.druid.initialization.Initialization; import org.apache.druid.query.DruidProcessingConfig; import org.apache.druid.segment.loading.SegmentLoaderConfig; import org.apache.druid.segment.loading.StorageLocationConfig; @@ -46,8 +45,6 @@ import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; -import javax.validation.Validation; -import javax.validation.Validator; import java.util.Collections; @RunWith(MockitoJUnitRunner.class) @@ -58,8 +55,6 @@ public class StorageNodeModuleTest @Rule public ExpectedException exceptionRule = ExpectedException.none(); - @Mock(answer = Answers.RETURNS_MOCKS) - private ObjectMapper mapper; @Mock(answer = Answers.RETURNS_MOCKS) private DruidNode self; @Mock(answer = Answers.RETURNS_MOCKS) @@ -169,24 +164,24 @@ public void testDruidServerMetadataWithNoServerTypeConfigShouldThrowProvisionExc private Injector makeInjector(boolean withServerTypeConfig) { - return Guice.createInjector( - Modules.override( + return Initialization.makeInjectorWithModules( + GuiceInjectors.makeStartupInjector(), (ImmutableList.of(Modules.override( (binder) -> { binder.bind(DruidNode.class).annotatedWith(Self.class).toInstance(self); - binder.bind(Validator.class).toInstance(Validation.buildDefaultValidatorFactory().getValidator()); + binder.bindConstant().annotatedWith(Names.named("serviceName")).to("test"); + binder.bindConstant().annotatedWith(Names.named("servicePort")).to(0); + binder.bindConstant().annotatedWith(Names.named("tlsServicePort")).to(-1); binder.bind(DruidProcessingConfig.class).toInstance(druidProcessingConfig); - binder.bindScope(LazySingleton.class, Scopes.SINGLETON); }, - target - ).with( - (binder) -> { - binder.bind(SegmentLoaderConfig.class).toInstance(segmentLoaderConfig); - if (withServerTypeConfig) { - binder.bind(ServerTypeConfig.class).toInstance(serverTypeConfig); + target).with( + (binder) -> { + binder.bind(SegmentLoaderConfig.class).toInstance(segmentLoaderConfig); + if (withServerTypeConfig) { + binder.bind(ServerTypeConfig.class).toInstance(serverTypeConfig); + } } - } - ) - ); + ) + ))); } private void mockSegmentCacheNotConfigured() From 37d8b583a6854d60589b30fde2c9e14bea221ce5 Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Sun, 9 Aug 2020 14:48:13 -0500 Subject: [PATCH 2/2] Remove maxsize refs from config --- dev/intellij-setup.md | 4 ++-- docs/ingestion/faq.md | 1 - docs/operations/basic-cluster-tuning.md | 10 ++++------ .../druid/cluster/data/historical/runtime.properties | 1 - .../single-server/large/historical/runtime.properties | 1 - .../single-server/medium/historical/runtime.properties | 1 - .../micro-quickstart/historical/runtime.properties | 1 - .../nano-quickstart/historical/runtime.properties | 1 - .../single-server/small/historical/runtime.properties | 1 - .../single-server/xlarge/historical/runtime.properties | 1 - .../docker/environment-configs/historical | 1 - .../historical-for-query-retry-test | 1 - 12 files changed, 6 insertions(+), 18 deletions(-) diff --git a/dev/intellij-setup.md b/dev/intellij-setup.md index 977f92fceb6c..74b09a0739f1 100644 --- a/dev/intellij-setup.md +++ b/dev/intellij-setup.md @@ -78,7 +78,7 @@ You can configure application definitions in XML for import into IntelliJ. Below