Revert "Only simulate legal desired moves"

This reverts commit 1f86eb8.

Author: DaveCTurner

docs/changelog/93635.yaml

@@ -40,7 +40,7 @@
 import static org.hamcrest.Matchers.notNullValue;
 
 public class SnapshotBasedRecoveryIT extends AbstractRollingTestCase {
-
+    @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/91383")
     public void testSnapshotBasedRecovery() throws Exception {
         final String indexName = "snapshot_based_recovery";
         final String repositoryName = "snapshot_based_recovery_repo";

qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/SnapshotBasedRecoveryIT.java

@@ -17,7 +17,6 @@
 import org.elasticsearch.cluster.routing.ShardRouting;
 import org.elasticsearch.cluster.routing.UnassignedInfo;
 import org.elasticsearch.cluster.routing.allocation.command.MoveAllocationCommand;
-import org.elasticsearch.cluster.routing.allocation.decider.Decision;
 import org.elasticsearch.common.metrics.MeanMetric;
 import org.elasticsearch.core.Strings;
 import org.elasticsearch.index.shard.ShardId;
@@ -144,21 +143,14 @@ public DesiredBalance compute(
             // Here existing shards are moved to desired locations before initializing unassigned shards because we prefer not to leave
             // immovable shards allocated to undesirable locations (e.g. a node that is shutting down or an allocation filter which was
             // only recently applied). In contrast, reconciliation prefers to initialize the unassigned shards first.
-            relocateToDesiredLocation: for (final var shardRouting : shardsToRelocate.values()) {
+            for (final var shardRouting : shardsToRelocate.values()) {
                 assert shardRouting.started();
-
-                while (targetNodesIterator.hasNext()) {
-                    final var targetNodeId = targetNodesIterator.next();
-                    final var targetNode = routingNodes.node(targetNodeId);
-                    if (targetNode != null
-                        && routingAllocation.deciders()
-                            .canAllocate(shardRouting, targetNode, routingAllocation)
-                            .type() != Decision.Type.NO) {
-                        final var shardToRelocate = routingNodes.relocateShard(shardRouting, targetNodeId, 0L, changes).v2();
-                        clusterInfoSimulator.simulateShardStarted(shardToRelocate);
-                        routingNodes.startShard(logger, shardToRelocate, changes, 0L);
-                        continue relocateToDesiredLocation;
-                    }
+                if (targetNodesIterator.hasNext()) {
+                    ShardRouting shardToRelocate = routingNodes.relocateShard(shardRouting, targetNodesIterator.next(), 0L, changes).v2();
+                    clusterInfoSimulator.simulateShardStarted(shardToRelocate);
+                    routingNodes.startShard(logger, shardToRelocate, changes, 0L);
+                } else {
+                    break;
                 }
             }
 
@@ -180,15 +172,9 @@ public DesiredBalance compute(
                 final var nodeIds = unassignedShardsToInitialize.get(shardRouting);
                 if (nodeIds != null && nodeIds.isEmpty() == false) {
                     final var nodeId = nodeIds.removeFirst();
-                    final var routingNode = routingNodes.node(nodeId);
-                    if (routingNode != null
-                        && routingAllocation.deciders()
-                            .canAllocate(shardRouting, routingNode, routingAllocation)
-                            .type() != Decision.Type.NO) {
-                        final var shardToInitialize = unassignedPrimaryIterator.initialize(nodeId, null, 0L, changes);
-                        clusterInfoSimulator.simulateShardStarted(shardToInitialize);
-                        routingNodes.startShard(logger, shardToInitialize, changes, 0L);
-                    }
+                    final var shardToInitialize = unassignedPrimaryIterator.initialize(nodeId, null, 0L, changes);
+                    clusterInfoSimulator.simulateShardStarted(shardToInitialize);
+                    routingNodes.startShard(logger, shardToInitialize, changes, 0L);
                 }
             }
         }
@@ -199,16 +185,10 @@ public DesiredBalance compute(
             if (unassignedPrimaries.contains(shardRouting.shardId()) == false) {
                 final var nodeIds = unassignedShardsToInitialize.get(shardRouting);
                 if (nodeIds != null && nodeIds.isEmpty() == false) {
-                    final var nodeId = nodeIds.removeFirst();
-                    final var routingNode = routingNodes.node(nodeId);
-                    if (routingNode != null
-                        && routingAllocation.deciders()
-                            .canAllocate(shardRouting, routingNode, routingAllocation)
-                            .type() != Decision.Type.NO) {
-                        final var shardToInitialize = unassignedReplicaIterator.initialize(nodeId, null, 0L, changes);
-                        clusterInfoSimulator.simulateShardStarted(shardToInitialize);
-                        routingNodes.startShard(logger, shardToInitialize, changes, 0L);
-                    }
+                    final String nodeId = nodeIds.removeFirst();
+                    ShardRouting shardToInitialize = unassignedReplicaIterator.initialize(nodeId, null, 0L, changes);
+                    clusterInfoSimulator.simulateShardStarted(shardToInitialize);
+                    routingNodes.startShard(logger, shardToInitialize, changes, 0L);
                 }
             }
         }

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

@@ -318,13 +318,13 @@ private static String diff(DesiredBalance old, DesiredBalance updated) {
             var oldAssignment = old.getAssignment(shardId);
             var updatedAssignment = updated.getAssignment(shardId);
             if (Objects.equals(oldAssignment, updatedAssignment) == false) {
-                builder.append(newLine).append(shardId).append(": ").append(oldAssignment).append(" -> ").append(updatedAssignment);
+                builder.append(newLine).append(shardId).append(": ").append(oldAssignment).append(" --> ").append(updatedAssignment);
             }
         }
         for (ShardId shardId : diff) {
             var oldAssignment = old.getAssignment(shardId);
             var updatedAssignment = updated.getAssignment(shardId);
-            builder.append(newLine).append(shardId).append(": ").append(oldAssignment).append(" -> ").append(updatedAssignment);
+            builder.append(newLine).append(shardId).append(": ").append(oldAssignment).append(" --> ").append(updatedAssignment);
         }
         return builder.append(newLine).toString();
     }

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

@@ -17,7 +17,6 @@
 import org.elasticsearch.cluster.ESAllocationTestCase;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
 import org.elasticsearch.cluster.metadata.Metadata;
-import org.elasticsearch.cluster.node.DiscoveryNode;
 import org.elasticsearch.cluster.node.DiscoveryNodes;
 import org.elasticsearch.cluster.routing.RoutingNode;
 import org.elasticsearch.cluster.routing.RoutingTable;
@@ -36,7 +35,6 @@
 import java.util.HashMap;
 import java.util.Map;
 import java.util.function.UnaryOperator;
-import java.util.stream.Collectors;
 
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.singletonList;
@@ -934,24 +932,15 @@ public void testUnassignedShardsWithUnbalancedZones() {
 
         // Cancel all initializing shards and move started primary to another node.
         AllocationCommands commands = new AllocationCommands();
-        final var unusedNodes = clusterState.nodes().stream().map(DiscoveryNode::getId).collect(Collectors.toSet());
-        // Cancel all initializing shards
-        for (ShardRouting routing : clusterState.routingTable().allShards()) {
-            unusedNodes.remove(routing.currentNodeId());
-            if (routing.initializing()) {
-                commands.add(new CancelAllocationCommand(routing.shardId().getIndexName(), routing.id(), routing.currentNodeId(), false));
-            }
-        }
-        // Move started primary to another node.
+        String primaryNode = null;
         for (ShardRouting routing : clusterState.routingTable().allShards()) {
             if (routing.primary()) {
-                var currentNodeId = routing.currentNodeId();
-                unusedNodes.remove(currentNodeId);
-                var otherNodeId = randomFrom(unusedNodes);
-                commands.add(new MoveAllocationCommand("test", 0, currentNodeId, otherNodeId));
-                break;
+                primaryNode = routing.currentNodeId();
+            } else if (routing.initializing()) {
+                commands.add(new CancelAllocationCommand(routing.shardId().getIndexName(), routing.id(), routing.currentNodeId(), false));
             }
         }
+        commands.add(new MoveAllocationCommand("test", 0, primaryNode, "A-4"));
 
         clusterState = strategy.reroute(clusterState, commands, false, false, false, ActionListener.noop()).clusterState();