This is an automated email from the ASF dual-hosted git repository.
kfaraz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new d9221e46e4 Completely disable cachingCost balancer strategy (#14798)
d9221e46e4 is described below
commit d9221e46e4b30dde80724dc49355a3ba5abcb312
Author: Kashif Faraz <[email protected]>
AuthorDate: Wed Aug 16 11:43:52 2023 +0530
Completely disable cachingCost balancer strategy (#14798)
`cachingCost` has been deprecated in #14484 and is not advised to be used in
production clusters as it may cause usage skew across historicals which the
coordinator is unable to rectify. This PR completely disables `cachingCost`
strategy
as it has now been rendered redundant due to recent performance improvements
made to `cost` strategy.
Changes
- Disable `cachingCost` strategy
- Add `DisabledCachingCostBalancerStrategyFactory` for the time being so
that we
can give a proper error message before falling back to
`CostBalancerStrategy`. This
will be removed in subsequent releases.
- Retain `CachingCostBalancerStrategy` for testing/benchmarking purposes.
- Add javadocs to `DiskNormalizedCostBalancerStrategy`
---
.../balancer/BalancerStrategyFactory.java | 2 +-
.../balancer/CachingCostBalancerStrategy.java | 5 ++
.../CachingCostBalancerStrategyFactory.java | 5 ++
.../balancer/CostBalancerStrategyFactory.java | 2 +-
...isabledCachingCostBalancerStrategyFactory.java} | 8 ++-
.../DiskNormalizedCostBalancerStrategy.java | 21 ++++++--
.../balancer/BalancerStrategyFactoryTest.java | 63 ++++++++++++++++++++++
7 files changed, 98 insertions(+), 8 deletions(-)
diff --git
a/server/src/main/java/org/apache/druid/server/coordinator/balancer/BalancerStrategyFactory.java
b/server/src/main/java/org/apache/druid/server/coordinator/balancer/BalancerStrategyFactory.java
index f27341fd3e..d0d426f7b5 100644
---
a/server/src/main/java/org/apache/druid/server/coordinator/balancer/BalancerStrategyFactory.java
+++
b/server/src/main/java/org/apache/druid/server/coordinator/balancer/BalancerStrategyFactory.java
@@ -26,7 +26,7 @@ import
com.google.common.util.concurrent.ListeningExecutorService;
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "strategy", defaultImpl =
CostBalancerStrategyFactory.class)
@JsonSubTypes(value = {
@JsonSubTypes.Type(name = "cost", value =
CostBalancerStrategyFactory.class),
- @JsonSubTypes.Type(name = "cachingCost", value =
CachingCostBalancerStrategyFactory.class),
+ @JsonSubTypes.Type(name = "cachingCost", value =
DisabledCachingCostBalancerStrategyFactory.class),
@JsonSubTypes.Type(name = "diskNormalized", value =
DiskNormalizedCostBalancerStrategyFactory.class),
@JsonSubTypes.Type(name = "random", value =
RandomBalancerStrategyFactory.class)
})
diff --git
a/server/src/main/java/org/apache/druid/server/coordinator/balancer/CachingCostBalancerStrategy.java
b/server/src/main/java/org/apache/druid/server/coordinator/balancer/CachingCostBalancerStrategy.java
index eda9928915..64d77842e3 100644
---
a/server/src/main/java/org/apache/druid/server/coordinator/balancer/CachingCostBalancerStrategy.java
+++
b/server/src/main/java/org/apache/druid/server/coordinator/balancer/CachingCostBalancerStrategy.java
@@ -28,6 +28,11 @@ import org.apache.druid.timeline.DataSegment;
import java.util.Collections;
import java.util.Set;
+/**
+ * @deprecated This is currently being used only in tests for benchmarking
purposes
+ * and will be removed in future releases.
+ */
+@Deprecated
public class CachingCostBalancerStrategy extends CostBalancerStrategy
{
private final ClusterCostCache clusterCostCache;
diff --git
a/server/src/main/java/org/apache/druid/server/coordinator/balancer/CachingCostBalancerStrategyFactory.java
b/server/src/main/java/org/apache/druid/server/coordinator/balancer/CachingCostBalancerStrategyFactory.java
index 0ddacaead7..89b7cf7546 100644
---
a/server/src/main/java/org/apache/druid/server/coordinator/balancer/CachingCostBalancerStrategyFactory.java
+++
b/server/src/main/java/org/apache/druid/server/coordinator/balancer/CachingCostBalancerStrategyFactory.java
@@ -39,6 +39,11 @@ import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.RejectedExecutionException;
+/**
+ * @deprecated This is currently being used only in tests for benchmarking
purposes
+ * and will be removed in future releases.
+ */
+@Deprecated
public class CachingCostBalancerStrategyFactory implements
BalancerStrategyFactory
{
private static final EmittingLogger LOG = new
EmittingLogger(CachingCostBalancerStrategyFactory.class);
diff --git
a/server/src/main/java/org/apache/druid/server/coordinator/balancer/CostBalancerStrategyFactory.java
b/server/src/main/java/org/apache/druid/server/coordinator/balancer/CostBalancerStrategyFactory.java
index 3085f35b6b..10d5952390 100644
---
a/server/src/main/java/org/apache/druid/server/coordinator/balancer/CostBalancerStrategyFactory.java
+++
b/server/src/main/java/org/apache/druid/server/coordinator/balancer/CostBalancerStrategyFactory.java
@@ -24,7 +24,7 @@ import
com.google.common.util.concurrent.ListeningExecutorService;
public class CostBalancerStrategyFactory implements BalancerStrategyFactory
{
@Override
- public CostBalancerStrategy createBalancerStrategy(ListeningExecutorService
exec)
+ public BalancerStrategy createBalancerStrategy(ListeningExecutorService exec)
{
return new CostBalancerStrategy(exec);
}
diff --git
a/server/src/main/java/org/apache/druid/server/coordinator/balancer/CostBalancerStrategyFactory.java
b/server/src/main/java/org/apache/druid/server/coordinator/balancer/DisabledCachingCostBalancerStrategyFactory.java
similarity index 71%
copy from
server/src/main/java/org/apache/druid/server/coordinator/balancer/CostBalancerStrategyFactory.java
copy to
server/src/main/java/org/apache/druid/server/coordinator/balancer/DisabledCachingCostBalancerStrategyFactory.java
index 3085f35b6b..7d2f0d96bc 100644
---
a/server/src/main/java/org/apache/druid/server/coordinator/balancer/CostBalancerStrategyFactory.java
+++
b/server/src/main/java/org/apache/druid/server/coordinator/balancer/DisabledCachingCostBalancerStrategyFactory.java
@@ -20,12 +20,16 @@
package org.apache.druid.server.coordinator.balancer;
import com.google.common.util.concurrent.ListeningExecutorService;
+import org.apache.druid.java.util.common.logger.Logger;
-public class CostBalancerStrategyFactory implements BalancerStrategyFactory
+public class DisabledCachingCostBalancerStrategyFactory implements
BalancerStrategyFactory
{
+ private static final Logger log = new Logger(BalancerStrategyFactory.class);
+
@Override
- public CostBalancerStrategy createBalancerStrategy(ListeningExecutorService
exec)
+ public BalancerStrategy createBalancerStrategy(ListeningExecutorService exec)
{
+ log.warn("Balancer strategy 'cachingCost' is disabled. Using 'cost'
strategy instead.");
return new CostBalancerStrategy(exec);
}
}
diff --git
a/server/src/main/java/org/apache/druid/server/coordinator/balancer/DiskNormalizedCostBalancerStrategy.java
b/server/src/main/java/org/apache/druid/server/coordinator/balancer/DiskNormalizedCostBalancerStrategy.java
index 601e5b042e..1d16c4785b 100644
---
a/server/src/main/java/org/apache/druid/server/coordinator/balancer/DiskNormalizedCostBalancerStrategy.java
+++
b/server/src/main/java/org/apache/druid/server/coordinator/balancer/DiskNormalizedCostBalancerStrategy.java
@@ -23,6 +23,23 @@ import
com.google.common.util.concurrent.ListeningExecutorService;
import org.apache.druid.server.coordinator.ServerHolder;
import org.apache.druid.timeline.DataSegment;
+/**
+ * A {@link BalancerStrategy} which can be used when historicals in a tier have
+ * varying disk capacities. This strategy normalizes the cost of placing a
segment on
+ * a server as calculated by {@link CostBalancerStrategy} by doing the
following:
+ * <ul>
+ * <li>Divide the cost by the number of segments on the server. This ensures
that
+ * cost does not increase just because the number of segments on a server is
higher.</li>
+ * <li>Multiply the resulting value by disk usage ratio. This ensures that all
+ * hosts have equivalent levels of percentage disk utilization.</li>
+ * </ul>
+ * i.e. to place a segment on a given server
+ * <pre>
+ * cost = as computed by CostBalancerStrategy
+ * normalizedCost = (cost / numSegments) * usageRatio
+ * = (cost / numSegments) * (diskUsed / totalDiskSpace)
+ * </pre>
+ */
public class DiskNormalizedCostBalancerStrategy extends CostBalancerStrategy
{
public DiskNormalizedCostBalancerStrategy(ListeningExecutorService exec)
@@ -30,10 +47,6 @@ public class DiskNormalizedCostBalancerStrategy extends
CostBalancerStrategy
super(exec);
}
- /**
- * Averages the cost obtained from CostBalancerStrategy. Also the costs are
weighted according to their usage ratios.
- * This ensures that all the hosts will have the same % disk utilization.
- */
@Override
protected double computePlacementCost(
final DataSegment proposalSegment,
diff --git
a/server/src/test/java/org/apache/druid/server/coordinator/balancer/BalancerStrategyFactoryTest.java
b/server/src/test/java/org/apache/druid/server/coordinator/balancer/BalancerStrategyFactoryTest.java
new file mode 100644
index 0000000000..d08b8ff104
--- /dev/null
+++
b/server/src/test/java/org/apache/druid/server/coordinator/balancer/BalancerStrategyFactoryTest.java
@@ -0,0 +1,63 @@
+/*
+ * 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.balancer;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.util.concurrent.ListeningExecutorService;
+import com.google.common.util.concurrent.MoreExecutors;
+import org.apache.druid.segment.TestHelper;
+import org.apache.druid.server.coordinator.simulate.BlockingExecutorService;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+public class BalancerStrategyFactoryTest
+{
+ private final ObjectMapper MAPPER = TestHelper.makeJsonMapper();
+
+ private ListeningExecutorService executorService;
+
+ @Before
+ public void setup()
+ {
+ executorService = MoreExecutors.listeningDecorator(
+ new BlockingExecutorService("StrategyFactoryTest-%s")
+ );
+ }
+
+ @After
+ public void tearDown()
+ {
+ executorService.shutdownNow();
+ }
+
+ @Test
+ public void testCachingCostStrategyFallsBackToCost() throws
JsonProcessingException
+ {
+ final String json = "{\"strategy\":\"cachingCost\"}";
+ BalancerStrategyFactory factory = MAPPER.readValue(json,
BalancerStrategyFactory.class);
+ BalancerStrategy strategy =
factory.createBalancerStrategy(executorService);
+
+ Assert.assertTrue(strategy instanceof CostBalancerStrategy);
+ Assert.assertFalse(strategy instanceof CachingCostBalancerStrategy);
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
