Skip to content

Commit 63e3ce8

Browse files
committed
Simulate shards moved by explicit commands
In DesiredBalancerComputer, shard movements should be simulated by starting any shards that are initializatng. Previously the explicitly moved shards are not covered. This PR fixes it. Resolves: ES-12943
1 parent 414972d commit 63e3ce8

File tree

3 files changed

+51
-23
lines changed

3 files changed

+51
-23
lines changed

server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -156,9 +156,8 @@ public void allocate(RoutingAllocation allocation) {
156156
assert allocation.ignoreDisable() == false;
157157
assert allocation.isSimulating() == false || allocation.routingNodes().hasInactiveShards() == false
158158
: "expect no initializing shard, but got " + allocation.routingNodes();
159-
// TODO: ES-12943 cannot assert the following because shards moved by commands are not simulated promptly in DesiredBalanceComputer
160-
// assert allocation.isSimulating() == false || allocation.routingNodes().getRelocatingShardCount() == 0
161-
// : "expect no relocating shard, but got " + allocation.routingNodes();
159+
assert allocation.isSimulating() == false || allocation.routingNodes().getRelocatingShardCount() == 0
160+
: "expect no relocating shard, but got " + allocation.routingNodes();
162161

163162
if (allocation.routingNodes().size() == 0) {
164163
failAllocationOfNewPrimaries(allocation);

server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputer.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.elasticsearch.index.shard.ShardId;
3737

3838
import java.util.ArrayList;
39+
import java.util.Arrays;
3940
import java.util.HashMap;
4041
import java.util.HashSet;
4142
import java.util.LinkedList;
@@ -291,7 +292,16 @@ public DesiredBalance compute(
291292
while ((commands = pendingDesiredBalanceMoves.poll()) != null) {
292293
for (MoveAllocationCommand command : commands) {
293294
try {
294-
command.execute(routingAllocation, false);
295+
final var rerouteExplanation = command.execute(routingAllocation, false);
296+
if (rerouteExplanation.decisions().type() != Decision.Type.NO) {
297+
final ShardRouting[] initializingShards = routingNodes.node(command.toNode()).initializing();
298+
assert initializingShards.length == 1 && command.fromNode().equals(initializingShards[0].relocatingNodeId())
299+
: "expect one relocating shard, but got : " + List.of(initializingShards);
300+
Arrays.stream(initializingShards).forEach(shard -> {
301+
clusterInfoSimulator.simulateShardStarted(shard);
302+
routingNodes.startShard(shard, changes, 0L);
303+
});
304+
}
295305
} catch (RuntimeException e) {
296306
logger.debug(
297307
() -> "move shard ["

server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputerTests.java

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
import org.elasticsearch.common.time.TimeProviderUtils;
5555
import org.elasticsearch.common.unit.ByteSizeValue;
5656
import org.elasticsearch.common.util.Maps;
57+
import org.elasticsearch.common.util.set.Sets;
5758
import org.elasticsearch.core.Strings;
5859
import org.elasticsearch.core.TimeValue;
5960
import org.elasticsearch.core.Tuple;
@@ -566,7 +567,21 @@ public void testNoDataNodes() {
566567
}
567568

568569
public void testAppliesMoveCommands() {
569-
var desiredBalanceComputer = createDesiredBalanceComputer();
570+
var desiredBalanceComputer = createDesiredBalanceComputer(new ShardsAllocator() {
571+
@Override
572+
public void allocate(RoutingAllocation allocation) {
573+
assertThat(
574+
"unexpected relocating shards: " + allocation.routingNodes(),
575+
allocation.routingNodes().getRelocatingShardCount(),
576+
equalTo(0)
577+
);
578+
}
579+
580+
@Override
581+
public ShardAllocationDecision decideShardAllocation(ShardRouting shard, RoutingAllocation allocation) {
582+
throw new AssertionError("only used for allocation explain");
583+
}
584+
});
570585
var clusterState = createInitialClusterState(3);
571586
var index = clusterState.metadata().getProject().index(TEST_INDEX).getIndex();
572587

@@ -578,25 +593,29 @@ public void testAppliesMoveCommands() {
578593
}
579594
clusterState = rebuildRoutingTable(clusterState, routingNodes);
580595

581-
var desiredBalance = desiredBalanceComputer.compute(
582-
DesiredBalance.BECOME_MASTER_INITIAL,
583-
createInput(clusterState),
584-
queue(
585-
new MoveAllocationCommand(index.getName(), 0, "node-1", "node-2"),
586-
new MoveAllocationCommand(index.getName(), 1, "node-1", "node-2")
587-
),
588-
input -> true
589-
);
596+
final var dataNodeIds = clusterState.nodes().getDataNodes().keySet();
597+
for (var nodeId : List.of("node-0", "node-1")) {
598+
var desiredBalance = desiredBalanceComputer.compute(
599+
DesiredBalance.BECOME_MASTER_INITIAL,
600+
createInput(clusterState),
601+
queue(
602+
new MoveAllocationCommand(index.getName(), 0, nodeId, "node-2"),
603+
new MoveAllocationCommand(index.getName(), 1, nodeId, "node-2")
604+
),
605+
input -> true
606+
);
590607

591-
assertDesiredAssignments(
592-
desiredBalance,
593-
Map.of(
594-
new ShardId(index, 0),
595-
new ShardAssignment(Set.of("node-0", "node-2"), 2, 0, 0),
596-
new ShardId(index, 1),
597-
new ShardAssignment(Set.of("node-0", "node-2"), 2, 0, 0)
598-
)
599-
);
608+
final Set<String> expectedNodeIds = Sets.difference(dataNodeIds, Set.of(nodeId));
609+
assertDesiredAssignments(
610+
desiredBalance,
611+
Map.of(
612+
new ShardId(index, 0),
613+
new ShardAssignment(expectedNodeIds, 2, 0, 0),
614+
new ShardId(index, 1),
615+
new ShardAssignment(expectedNodeIds, 2, 0, 0)
616+
)
617+
);
618+
}
600619
}
601620

602621
public void testDesiredBalanceShouldConvergeInABigCluster() {

0 commit comments

Comments
 (0)