Skip to content

Commit

Permalink
Completely disable cachingCost balancer strategy (#14798)
Browse files Browse the repository at this point in the history
`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`
  • Loading branch information
kfaraz authored Aug 16, 2023
1 parent 9be0f64 commit d9221e4
Show file tree
Hide file tree
Showing 7 changed files with 127 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
@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)
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@
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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
public class CostBalancerStrategyFactory implements BalancerStrategyFactory
{
@Override
public CostBalancerStrategy createBalancerStrategy(ListeningExecutorService exec)
public BalancerStrategy createBalancerStrategy(ListeningExecutorService exec)
{
return new CostBalancerStrategy(exec);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* 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.google.common.util.concurrent.ListeningExecutorService;
import org.apache.druid.java.util.common.logger.Logger;

public class DisabledCachingCostBalancerStrategyFactory implements BalancerStrategyFactory
{
private static final Logger log = new Logger(BalancerStrategyFactory.class);

@Override
public BalancerStrategy createBalancerStrategy(ListeningExecutorService exec)
{
log.warn("Balancer strategy 'cachingCost' is disabled. Using 'cost' strategy instead.");
return new CostBalancerStrategy(exec);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,30 @@
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)
{
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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}

0 comments on commit d9221e4

Please sign in to comment.