[8.x] Skip eager reconciliation for empty routing table (#116903) (#117103)

* Skip eager reconciliation for empty routing table (#116903)

No need to start the eager reconciliation when the routing table is
empty. An empty routing table means either the cluster has no shards or
the state has not recovered. The eager reconciliation is not necessary
in both cases.

Resolves: #115885

* fix compilation

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

@@ -124,6 +124,10 @@ public class DesiredBalanceShardsAllocator implements ShardsAllocator {
 
                 recordTime(
                     cumulativeComputationTime,
+                    // We set currentDesiredBalance back to INITIAL when the node stands down as master in onNoLongerMaster.
+                    // However, it is possible that we revert the effect here by setting it again since the computation is async
+                    // and does not check whether the node is master. This should have little to no practical impact. But it may
+                    // lead to unexpected behaviours for tests. See also https://github.com/elastic/elasticsearch/pull/116904
                     () -> setCurrentDesiredBalance(
                         desiredBalanceComputer.compute(
                             getInitialDesiredBalance(),
@@ -202,6 +206,10 @@ public class DesiredBalanceShardsAllocator implements ShardsAllocator {
         queue.add(index, listener);
         desiredBalanceComputation.onNewInput(DesiredBalanceInput.create(index, allocation));
 
+        if (allocation.routingTable().indicesRouting().isEmpty()) {
+            logger.debug("No eager reconciliation needed for empty routing table");
+            return;
+        }
         // Starts reconciliation towards desired balance that might have not been updated with a recent calculation yet.
         // This is fine as balance should have incremental rather than radical changes.
         // This should speed up achieving the desired balance in cases current state is still different from it (due to THROTTLING).

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

@@ -13,6 +13,7 @@ import org.apache.logging.log4j.Level;
 import org.apache.lucene.util.SetOnce;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.support.ActionTestUtils;
+import org.elasticsearch.action.support.PlainActionFuture;
 import org.elasticsearch.cluster.ClusterInfo;
 import org.elasticsearch.cluster.ClusterName;
 import org.elasticsearch.cluster.ClusterState;
@@ -56,6 +57,7 @@ import org.elasticsearch.test.ClusterServiceUtils;
 import org.elasticsearch.test.MockLog;
 import org.elasticsearch.threadpool.TestThreadPool;
 
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Queue;
@@ -78,7 +80,9 @@ import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.greaterThanOrEqualTo;
 import static org.hamcrest.Matchers.hasItem;
+import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.not;
+import static org.hamcrest.Matchers.sameInstance;
 
 public class DesiredBalanceShardsAllocatorTests extends ESAllocationTestCase {
 
@@ -906,6 +910,76 @@ public class DesiredBalanceShardsAllocatorTests extends ESAllocationTestCase {
         }
     }
 
+    public void testNotReconcileEagerlyForEmptyRoutingTable() {
+        final var threadPool = new TestThreadPool(getTestName());
+        final var clusterService = ClusterServiceUtils.createClusterService(ClusterState.EMPTY_STATE, threadPool);
+        final var clusterSettings = createBuiltInClusterSettings();
+        final var shardsAllocator = createShardsAllocator();
+        final var reconciliationTaskSubmitted = new AtomicBoolean();
+        final var desiredBalanceShardsAllocator = new DesiredBalanceShardsAllocator(
+            shardsAllocator,
+            threadPool,
+            clusterService,
+            new DesiredBalanceComputer(clusterSettings, threadPool, shardsAllocator) {
+                @Override
+                public DesiredBalance compute(
+                    DesiredBalance previousDesiredBalance,
+                    DesiredBalanceInput desiredBalanceInput,
+                    Queue<List<MoveAllocationCommand>> pendingDesiredBalanceMoves,
+                    Predicate<DesiredBalanceInput> isFresh
+                ) {
+                    assertThat(previousDesiredBalance, sameInstance(DesiredBalance.INITIAL));
+                    return new DesiredBalance(desiredBalanceInput.index(), Map.of());
+                }
+            },
+            (clusterState, rerouteStrategy) -> null,
+            TelemetryProvider.NOOP
+        ) {
+
+            private ActionListener<Void> lastListener;
+
+            @Override
+            public void allocate(RoutingAllocation allocation, ActionListener<Void> listener) {
+                lastListener = listener;
+                super.allocate(allocation, listener);
+            }
+
+            @Override
+            protected void reconcile(DesiredBalance desiredBalance, RoutingAllocation allocation) {
+                fail("should not call reconcile");
+            }
+
+            @Override
+            protected void submitReconcileTask(DesiredBalance desiredBalance) {
+                assertThat(desiredBalance.lastConvergedIndex(), equalTo(0L));
+                reconciliationTaskSubmitted.set(true);
+                lastListener.onResponse(null);
+            }
+        };
+        assertThat(desiredBalanceShardsAllocator.getDesiredBalance(), sameInstance(DesiredBalance.INITIAL));
+        try {
+            final PlainActionFuture<Void> future = new PlainActionFuture<>();
+            desiredBalanceShardsAllocator.allocate(
+                new RoutingAllocation(
+                    new AllocationDeciders(Collections.emptyList()),
+                    clusterService.state(),
+                    null,
+                    null,
+                    randomNonNegativeLong()
+                ),
+                future
+            );
+            safeGet(future);
+            assertThat(desiredBalanceShardsAllocator.getStats().computationSubmitted(), equalTo(1L));
+            assertThat(desiredBalanceShardsAllocator.getStats().computationExecuted(), equalTo(1L));
+            assertThat(reconciliationTaskSubmitted.get(), is(true));
+            assertThat(desiredBalanceShardsAllocator.getDesiredBalance().lastConvergedIndex(), equalTo(0L));
+        } finally {
+            clusterService.close();
+            terminate(threadPool);
+        }
+    }
+
     private static IndexMetadata createIndex(String name) {
         return IndexMetadata.builder(name).settings(indexSettings(IndexVersion.current(), 1, 0)).build();
     }