Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/changelog/128063.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 128063
summary: More efficient sort in `tryRelocateShard`
area: Allocation
type: enhancement
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@
import java.util.Map;
import java.util.Set;
import java.util.function.BiFunction;
import java.util.stream.StreamSupport;

import static org.elasticsearch.cluster.metadata.SingleNodeShutdownMetadata.Type.REPLACE;
import static org.elasticsearch.cluster.routing.ExpectedShardSizeEstimator.getExpectedShardSize;
Expand Down Expand Up @@ -1091,7 +1090,13 @@ private AllocateUnassignedDecision decideAllocateUnassigned(final ProjectIndex i
return AllocateUnassignedDecision.fromDecision(decision, minNode != null ? minNode.routingNode.node() : null, nodeDecisions);
}

private static final Comparator<ShardRouting> BY_DESCENDING_SHARD_ID = Comparator.comparing(ShardRouting::shardId).reversed();
private static final Comparator<ShardRouting> BY_DESCENDING_SHARD_ID = (s1, s2) -> Integer.compare(s2.id(), s1.id());

/**
* Scratch space for accumulating/sorting the {@link ShardRouting} instances when contemplating moving the shards away from a node
* in {@link #tryRelocateShard} - re-used to avoid extraneous allocations etc.
*/
private ShardRouting[] shardRoutingsOnMaxWeightNode;

/**
* Tries to find a relocation from the max node to the minimal node for an arbitrary shard of the given index on the
Expand All @@ -1102,13 +1107,24 @@ private boolean tryRelocateShard(ModelNode minNode, ModelNode maxNode, ProjectIn
final ModelIndex index = maxNode.getIndex(idx);
if (index != null) {
logger.trace("Try relocating shard of [{}] from [{}] to [{}]", idx, maxNode.getNodeId(), minNode.getNodeId());
final Iterable<ShardRouting> shardRoutings = StreamSupport.stream(index.spliterator(), false)
.filter(ShardRouting::started) // cannot rebalance unassigned, initializing or relocating shards anyway
.sorted(BY_DESCENDING_SHARD_ID) // check in descending order of shard id so that the decision is deterministic
::iterator;
if (shardRoutingsOnMaxWeightNode == null || shardRoutingsOnMaxWeightNode.length < index.numShards()) {
shardRoutingsOnMaxWeightNode = new ShardRouting[index.numShards() * 2]; // oversized so reuse is more likely
}

int startedShards = 0;
for (final var shardRouting : index) {
if (shardRouting.started()) { // cannot rebalance unassigned, initializing or relocating shards anyway
shardRoutingsOnMaxWeightNode[startedShards] = shardRouting;
startedShards += 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opt: ++startedShards or shardRoutingsOnMaxWeightNode[startedShards++] = shardRouting;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO unary increments are harder on the reader than spelling out a += 1 so I'd rather leave it like this.

}
}
// check in descending order of shard id so that the decision is deterministic
ArrayUtil.timSort(shardRoutingsOnMaxWeightNode, 0, startedShards, BY_DESCENDING_SHARD_ID);

final AllocationDeciders deciders = allocation.deciders();
for (ShardRouting shard : shardRoutings) {
for (int shardIndex = 0; shardIndex < startedShards; shardIndex++) {
final ShardRouting shard = shardRoutingsOnMaxWeightNode[shardIndex];

final Decision rebalanceDecision = deciders.canRebalance(shard, allocation);
if (rebalanceDecision.type() == Type.NO) {
continue;
Expand Down
Loading