Remove node-level canAllocate override (#59389)

Today there is a node-level `canAllocate` override which the balancer
uses to ignore certain nodes to which it is certain no more shards can
be allocated. In fact this override only ignores nodes which have hit
the rarely-used `cluster.routing.allocation.total_shards_per_node`
limit, so this optimization doesn't have a meaningful impact on real
clusters.

This commit removes this unnecessary fast path from the balancer, and
also removes all the machinery needed to support it.
This commit is contained in:
David Turner 2020-07-23 08:48:23 +01:00
parent 43a6ff5eb1
commit bf7e53a91e
8 changed files with 5 additions and 103 deletions

View File

@ -47,7 +47,6 @@ import org.elasticsearch.common.settings.ClusterSettings;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Setting.Property;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.set.Sets;
import org.elasticsearch.gateway.PriorityComparator;
import java.util.ArrayList;
@ -55,7 +54,6 @@ import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.IdentityHashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
@ -132,7 +130,7 @@ public class BalancedShardsAllocator implements ShardsAllocator {
AllocateUnassignedDecision allocateUnassignedDecision = AllocateUnassignedDecision.NOT_TAKEN;
MoveDecision moveDecision = MoveDecision.NOT_TAKEN;
if (shard.unassigned()) {
allocateUnassignedDecision = balancer.decideAllocateUnassigned(shard, Sets.newHashSet());
allocateUnassignedDecision = balancer.decideAllocateUnassigned(shard);
} else {
moveDecision = balancer.decideMove(shard);
if (moveDecision.isDecisionTaken() && moveDecision.canRemain()) {
@ -798,11 +796,10 @@ public class BalancedShardsAllocator implements ShardsAllocator {
int secondaryLength = 0;
int primaryLength = primary.length;
ArrayUtil.timSort(primary, comparator);
final Set<ModelNode> throttledNodes = Collections.newSetFromMap(new IdentityHashMap<>());
do {
for (int i = 0; i < primaryLength; i++) {
ShardRouting shard = primary[i];
AllocateUnassignedDecision allocationDecision = decideAllocateUnassigned(shard, throttledNodes);
final AllocateUnassignedDecision allocationDecision = decideAllocateUnassigned(shard);
final String assignedNodeId = allocationDecision.getTargetNode() != null ?
allocationDecision.getTargetNode().getId() : null;
final ModelNode minNode = assignedNodeId != null ? nodes.get(assignedNodeId) : null;
@ -838,16 +835,6 @@ public class BalancedShardsAllocator implements ShardsAllocator {
ShardRouting.UNAVAILABLE_EXPECTED_SHARD_SIZE,
allocation.clusterInfo(), allocation.metadata(), allocation.routingTable());
minNode.addShard(shard.initialize(minNode.getNodeId(), null, shardSize));
final RoutingNode node = minNode.getRoutingNode();
final Decision.Type nodeLevelDecision = deciders.canAllocate(node, allocation).type();
if (nodeLevelDecision != Type.YES) {
if (logger.isTraceEnabled()) {
logger.trace("Can not allocate on node [{}] remove from round decision [{}]", node,
allocationDecision.getAllocationStatus());
}
assert nodeLevelDecision == Type.NO;
throttledNodes.add(minNode);
}
} else {
if (logger.isTraceEnabled()) {
logger.trace("No Node found to assign shard [{}]", shard);
@ -877,7 +864,7 @@ public class BalancedShardsAllocator implements ShardsAllocator {
* {@link ModelNode} representing the node that the shard should be assigned to. If the decision returned
* is of type {@link Type#NO}, then the assigned node will be null.
*/
private AllocateUnassignedDecision decideAllocateUnassigned(final ShardRouting shard, final Set<ModelNode> throttledNodes) {
private AllocateUnassignedDecision decideAllocateUnassigned(final ShardRouting shard) {
if (shard.assignedToNode()) {
// we only make decisions for unassigned shards here
return AllocateUnassignedDecision.NOT_TAKEN;
@ -894,17 +881,12 @@ public class BalancedShardsAllocator implements ShardsAllocator {
float minWeight = Float.POSITIVE_INFINITY;
ModelNode minNode = null;
Decision decision = null;
if (throttledNodes.size() >= nodes.size() && explain == false) {
// all nodes are throttled, so we know we won't be able to allocate this round,
// so if we are not in explain mode, short circuit
return AllocateUnassignedDecision.no(AllocationStatus.DECIDERS_NO, null);
}
/* Don't iterate over an identity hashset here the
* iteration order is different for each run and makes testing hard */
Map<String, NodeAllocationResult> nodeExplanationMap = explain ? new HashMap<>() : null;
List<Tuple<String, Float>> nodeWeights = explain ? new ArrayList<>() : null;
for (ModelNode node : nodes.values()) {
if ((throttledNodes.contains(node) || node.containsShard(shard)) && explain == false) {
if (node.containsShard(shard) && explain == false) {
// decision is NO without needing to check anything further, so short circuit
continue;
}

View File

@ -73,14 +73,6 @@ public abstract class AllocationDecider {
return Decision.ALWAYS;
}
/**
* Returns a {@link Decision} whether the given node can allow any allocation at all at this state of the
* {@link RoutingAllocation}. The default is {@link Decision#ALWAYS}.
*/
public Decision canAllocate(RoutingNode node, RoutingAllocation allocation) {
return Decision.ALWAYS;
}
/**
* Returns a {@link Decision} whether shards of the given index should be auto-expanded to this node at this state of the
* {@link RoutingAllocation}. The default is {@link Decision#ALWAYS}.

View File

@ -176,25 +176,6 @@ public class AllocationDeciders extends AllocationDecider {
return ret;
}
@Override
public Decision canAllocate(RoutingNode node, RoutingAllocation allocation) {
Decision.Multi ret = new Decision.Multi();
for (AllocationDecider allocationDecider : allocations) {
Decision decision = allocationDecider.canAllocate(node, allocation);
// short track if a NO is returned.
if (decision == Decision.NO) {
if (!allocation.debugDecision()) {
return decision;
} else {
ret.add(decision);
}
} else {
addDecision(ret, decision, allocation);
}
}
return ret;
}
@Override
public Decision canRebalance(RoutingAllocation allocation) {
Decision.Multi ret = new Decision.Multi();

View File

@ -129,33 +129,4 @@ public class ShardsLimitAllocationDecider extends AllocationDecider {
nodeShardCount, indexShardLimit, clusterShardLimit);
}
@Override
public Decision canAllocate(RoutingNode node, RoutingAllocation allocation) {
// Only checks the node-level limit, not the index-level
// Capture the limit here in case it changes during this method's
// execution
final int clusterShardLimit = this.clusterShardLimit;
if (clusterShardLimit <= 0) {
return allocation.decision(Decision.YES, NAME, "total shard limits are disabled: [cluster: %d] <= 0",
clusterShardLimit);
}
int nodeShardCount = 0;
for (ShardRouting nodeShard : node) {
// don't count relocating shards...
if (nodeShard.relocating()) {
continue;
}
nodeShardCount++;
}
if (clusterShardLimit >= 0 && nodeShardCount >= clusterShardLimit) {
return allocation.decision(Decision.NO, NAME,
"too many shards [%d] allocated to this node, cluster setting [%s=%d]",
nodeShardCount, CLUSTER_TOTAL_SHARDS_PER_NODE_SETTING.getKey(), clusterShardLimit);
}
return allocation.decision(Decision.YES, NAME,
"the shard count [%d] for this node is under the cluster level node limit [%d]",
nodeShardCount, clusterShardLimit);
}
}

View File

@ -71,14 +71,7 @@ public class DecisionsImpactOnClusterHealthTests extends ESAllocationTestCase {
Settings settings = Settings.builder()
.put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toAbsolutePath().toString())
.build();
AllocationDecider decider = new TestAllocateDecision(Decision.THROTTLE) {
// the only allocation decider that implements this is ShardsLimitAllocationDecider and it always
// returns only YES or NO, never THROTTLE
@Override
public Decision canAllocate(RoutingNode node, RoutingAllocation allocation) {
return randomBoolean() ? Decision.YES : Decision.NO;
}
};
AllocationDecider decider = new TestAllocateDecision(Decision.THROTTLE);
// if deciders THROTTLE allocating a primary shard, stay in YELLOW state
runAllocationTest(
settings, indexName, Collections.singleton(decider), ClusterHealthStatus.YELLOW

View File

@ -78,11 +78,6 @@ public class AllocationDecidersTests extends ESTestCase {
return Decision.YES;
}
@Override
public Decision canAllocate(RoutingNode node, RoutingAllocation allocation) {
return Decision.YES;
}
@Override
public Decision shouldAutoExpandToNode(IndexMetadata indexMetadata, DiscoveryNode node, RoutingAllocation allocation) {
return Decision.YES;
@ -109,7 +104,6 @@ public class AllocationDecidersTests extends ESTestCase {
verify(deciders.canAllocate(shardRouting, routingNode, allocation), matcher);
verify(deciders.canAllocate(idx, routingNode, allocation), matcher);
verify(deciders.canAllocate(shardRouting, allocation), matcher);
verify(deciders.canAllocate(routingNode, allocation), matcher);
verify(deciders.canRebalance(shardRouting, allocation), matcher);
verify(deciders.canRebalance(allocation), matcher);
verify(deciders.canRemain(shardRouting, routingNode, allocation), matcher);

View File

@ -220,12 +220,6 @@ public class EnableAllocationShortCircuitTests extends ESAllocationTestCase {
canAllocateAttempts++;
return super.canAllocate(indexMetadata, node, allocation);
}
@Override
public Decision canAllocate(RoutingNode node, RoutingAllocation allocation) {
canAllocateAttempts++;
return super.canAllocate(node, allocation);
}
}
}
}

View File

@ -223,11 +223,6 @@ public abstract class ESAllocationTestCase extends ESTestCase {
public Decision canAllocate(ShardRouting shardRouting, RoutingAllocation allocation) {
return decision;
}
@Override
public Decision canAllocate(RoutingNode node, RoutingAllocation allocation) {
return decision;
}
}
/** A lock {@link AllocationService} allowing tests to override time */