[Test] Add more logs for WriteLoadConstraintDeciderIT (elastic#137171)

ywangd · web-flow · commit 5ffa07cdf83b · 2025-10-28T09:09:42.000+01:00
Add cluster state dumps on setup failure. Relates: elastic#137162
diff --git a/muted-tests.yml b/muted-tests.yml
@@ -501,9 +501,6 @@ tests:
 - class: org.elasticsearch.xpack.esql.qa.single_node.GenerativeMetricsIT
   method: test
   issue: https://github.com/elastic/elasticsearch/issues/137071
-- class: org.elasticsearch.cluster.routing.allocation.decider.WriteLoadConstraintDeciderIT
-  method: testCanRemainNotPreferredIsIgnoredWhenAllOtherNodesReturnNotPreferred
-  issue: https://github.com/elastic/elasticsearch/issues/137162
 - class: org.elasticsearch.xpack.ml.integration.DatafeedJobsIT
   method: testDatafeedTimingStats_DatafeedRecreated
   issue: https://github.com/elastic/elasticsearch/issues/137207
diff --git a/server/src/internalClusterTest/java/org/elasticsearch/cluster/routing/allocation/decider/WriteLoadConstraintDeciderIT.java b/server/src/internalClusterTest/java/org/elasticsearch/cluster/routing/allocation/decider/WriteLoadConstraintDeciderIT.java
@@ -64,8 +64,6 @@
 import java.util.stream.StreamSupport;
 
 import static java.util.stream.IntStream.range;
-import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS;
-import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.everyItem;
 import static org.hamcrest.Matchers.greaterThanOrEqualTo;
@@ -146,7 +144,7 @@ public void testHighNodeWriteLoadPreventsNewShardAllocation() {
          * second node, since the third is reporting as hot-spotted and should not accept any shards.
          */
 
-        logger.info("---> Refreshing the cluster info to pull in the dummy thread pool stats with a hot-spotting node");
+        logger.info("--> Refreshing the cluster info to pull in the dummy thread pool stats with a hot-spotting node");
         refreshClusterInfo();
 
         var temporaryClusterStateListener = ClusterServiceUtils.addMasterTemporaryStateListener(clusterState -> {
@@ -165,24 +163,14 @@ public void testHighNodeWriteLoadPreventsNewShardAllocation() {
 
         try {
             logger.info(
-                "---> Update the filter to exclude " + harness.firstDataNodeName + " so shards will be reassigned away to the other nodes"
+                "--> Update the filter to exclude " + harness.firstDataNodeName + " so shards will be reassigned away to the other nodes"
             );
             // Updating the cluster settings will trigger a reroute request, no need to explicitly request one in the test.
             updateClusterSettings(Settings.builder().put("cluster.routing.allocation.exclude._name", harness.firstDataNodeName));
 
             safeAwait(temporaryClusterStateListener);
         } catch (AssertionError error) {
-            ClusterState state = internalCluster().client()
-                .admin()
-                .cluster()
-                .prepareState(TEST_REQUEST_TIMEOUT)
-                .clear()
-                .setMetadata(true)
-                .setNodes(true)
-                .setRoutingTable(true)
-                .get()
-                .getState();
-            logger.info("---> Failed to reach expected allocation state. Dumping assignments: " + state.getRoutingNodes());
+            dumpClusterState();
             throw error;
         }
     }
@@ -251,7 +239,7 @@ public void testShardsAreAssignedToNotPreferredWhenAlternativeIsNo() {
          * the second and third node reporting hot-spotting: a canRemain::NO response should override a canAllocate::NOT_PREFERRED answer.
          */
 
-        logger.info("---> Refreshing the cluster info to pull in the dummy thread pool stats with a hot-spotting node");
+        logger.info("--> Refreshing the cluster info to pull in the dummy thread pool stats with a hot-spotting node");
         refreshClusterInfo();
 
         var temporaryClusterStateListener = ClusterServiceUtils.addMasterTemporaryStateListener(clusterState -> {
@@ -265,25 +253,13 @@ public void testShardsAreAssignedToNotPreferredWhenAlternativeIsNo() {
         });
 
         try {
-            logger.info(
-                "---> Update the filter to remove exclusions so that shards can be reassigned based on the write load decider only"
-            );
+            logger.info("--> Update the filter to remove exclusions so that shards can be reassigned based on the write load decider only");
             // Updating the cluster settings will trigger a reroute request, no need to explicitly request one in the test.
-            updateClusterSettings(Settings.builder().put("cluster.routing.allocation.exclude._name", ""));
+            updateClusterSettings(Settings.builder().putNull("cluster.routing.allocation.exclude._name"));
 
             safeAwait(temporaryClusterStateListener);
         } catch (AssertionError error) {
-            ClusterState state = internalCluster().client()
-                .admin()
-                .cluster()
-                .prepareState(TEST_REQUEST_TIMEOUT)
-                .clear()
-                .setMetadata(true)
-                .setNodes(true)
-                .setRoutingTable(true)
-                .get()
-                .getState();
-            logger.info("---> Failed to reach expected allocation state. Dumping assignments: " + state.getRoutingNodes());
+            dumpClusterState();
             throw error;
         }
     }
@@ -351,17 +327,15 @@ public void testCanRemainNotPreferredIsIgnoredWhenAllOtherNodesReturnNotPreferre
 
         // Wait for the DesiredBalance to be recomputed as a result of the ClusterInfo refresh. Ensures no async computation.
         MockLog.awaitLogger(() -> {
-            logger.info("---> Refreshing the cluster info to pull in the dummy thread pool stats with hot-spot stats");
+            logger.info("--> Refreshing the cluster info to pull in the dummy thread pool stats with hot-spot stats");
             refreshClusterInfo();
         }, DesiredBalanceShardsAllocator.class, createBalancerConvergedSeenEvent());
 
         // Wait for the DesiredBalance to be recomputed as a result of the settings change.
         MockLog.awaitLogger(() -> {
-            logger.info(
-                "---> Update the filter to remove exclusions so that shards can be reassigned based on the write load decider only"
-            );
+            logger.info("--> Update the filter to remove exclusions so that shards can be reassigned based on the write load decider only");
             // Updating the cluster settings will trigger a reroute request.
-            updateClusterSettings(Settings.builder().put("cluster.routing.allocation.exclude._name", ""));
+            updateClusterSettings(Settings.builder().putNull("cluster.routing.allocation.exclude._name"));
         }, DesiredBalanceShardsAllocator.class, createBalancerConvergedSeenEvent());
 
         try {
@@ -371,23 +345,14 @@ public void testCanRemainNotPreferredIsIgnoredWhenAllOtherNodesReturnNotPreferre
                 client().execute(TransportGetDesiredBalanceAction.TYPE, new DesiredBalanceRequest(TEST_REQUEST_TIMEOUT))
             );
             Map<Integer, DesiredBalanceResponse.DesiredShards> shardsMap = desiredBalanceResponse.getRoutingTable().get(harness.indexName);
-            logger.info("---> Checking desired shard assignments are still on the first data node. Desired assignments: " + shardsMap);
+            logger.info("--> Checking desired shard assignments are still on the first data node. Desired assignments: " + shardsMap);
             for (var desiredShard : shardsMap.values()) {
                 for (var desiredNodeId : desiredShard.desired().nodeIds()) {
                     assertEquals("Found a shard assigned to an unexpected node: " + shardsMap, desiredNodeId, harness.firstDataNodeId);
                 }
             }
         } catch (AssertionError error) {
-            ClusterState state = client().admin()
-                .cluster()
-                .prepareState(TEST_REQUEST_TIMEOUT)
-                .clear()
-                .setMetadata(true)
-                .setNodes(true)
-                .setRoutingTable(true)
-                .get()
-                .getState();
-            logger.info("---> Failed to reach expected allocation state. Dumping assignments: " + state.getRoutingNodes());
+            dumpClusterState();
             throw error;
         }
     }
@@ -452,17 +417,15 @@ public void testCanRemainRelocatesOneShardWhenAHotSpotOccurs() {
 
         // Wait for the DesiredBalance to be recomputed as a result of the ClusterInfo refresh. This way nothing async is running.
         MockLog.awaitLogger(() -> {
-            logger.info("---> Refreshing the cluster info to pull in the dummy thread pool stats with hot-spot stats");
+            logger.info("--> Refreshing the cluster info to pull in the dummy thread pool stats with hot-spot stats");
             refreshClusterInfo();
         }, DesiredBalanceShardsAllocator.class, createBalancerConvergedSeenEvent());
 
         // Wait for the DesiredBalance to be recomputed as a result of the settings change.
         MockLog.awaitLogger(() -> {
-            logger.info(
-                "---> Update the filter to remove exclusions so that shards can be reassigned based on the write load decider only"
-            );
+            logger.info("--> Update the filter to remove exclusions so that shards can be reassigned based on the write load decider only");
             // Updating the cluster settings will trigger a reroute request.
-            updateClusterSettings(Settings.builder().put("cluster.routing.allocation.exclude._name", ""));
+            updateClusterSettings(Settings.builder().putNull("cluster.routing.allocation.exclude._name"));
         }, DesiredBalanceShardsAllocator.class, createBalancerConvergedSeenEvent());
 
         try {
@@ -471,7 +434,7 @@ public void testCanRemainRelocatesOneShardWhenAHotSpotOccurs() {
                 client().execute(TransportGetDesiredBalanceAction.TYPE, new DesiredBalanceRequest(TEST_REQUEST_TIMEOUT))
             );
             Map<Integer, DesiredBalanceResponse.DesiredShards> shardsMap = desiredBalanceResponse.getRoutingTable().get(harness.indexName);
-            logger.info("---> Checking desired shard assignments. Desired assignments: " + shardsMap);
+            logger.info("--> Checking desired shard assignments. Desired assignments: " + shardsMap);
             int countShardsStillAssignedToFirstNode = 0;
             for (var desiredShard : shardsMap.values()) {
                 for (var desiredNodeId : desiredShard.desired().nodeIds()) {
@@ -487,16 +450,7 @@ public void testCanRemainRelocatesOneShardWhenAHotSpotOccurs() {
             );
             assertThatTheBestShardWasMoved(harness, originalClusterState, desiredBalanceResponse);
         } catch (AssertionError error) {
-            ClusterState state = client().admin()
-                .cluster()
-                .prepareState(TEST_REQUEST_TIMEOUT)
-                .clear()
-                .setMetadata(true)
-                .setNodes(true)
-                .setRoutingTable(true)
-                .get()
-                .getState();
-            logger.info("---> Failed to reach expected allocation state. Dumping assignments: " + state.getRoutingNodes());
+            dumpClusterState();
             throw error;
         }
     }
@@ -789,7 +743,7 @@ private TestHarness setUpThreeTestNodesAndAllIndexShardsOnFirstNode() {
         final DiscoveryNode thirdDiscoveryNode = getDiscoveryNode(thirdDataNodeName);
 
         logger.info(
-            "---> first node name "
+            "--> first node name "
                 + firstDataNodeName
                 + " and ID "
                 + firstDataNodeId
@@ -804,7 +758,7 @@ private TestHarness setUpThreeTestNodesAndAllIndexShardsOnFirstNode() {
         );
 
         logger.info(
-            "---> utilization threshold: "
+            "--> utilization threshold: "
                 + randomUtilizationThresholdPercent
                 + ",  write threads: "
                 + randomNumberOfWritePoolThreads
@@ -817,7 +771,7 @@ private TestHarness setUpThreeTestNodesAndAllIndexShardsOnFirstNode() {
          * Then create an index with many shards, which will all be assigned to the first data node.
          */
 
-        logger.info("---> Limit shard assignment to node " + firstDataNodeName + " by excluding the other nodes");
+        logger.info("--> Limit shard assignment to node " + firstDataNodeName + " by excluding the other nodes");
         updateClusterSettings(
             Settings.builder().put("cluster.routing.allocation.exclude._name", secondDataNodeName + "," + thirdDataNodeName)
         );
@@ -849,14 +803,16 @@ private TestHarness setUpThreeTestNodesAndAllIndexShardsOnFirstNode() {
             );
         });
 
-        createIndex(
-            indexName,
-            Settings.builder().put(SETTING_NUMBER_OF_SHARDS, randomNumberOfShards).put(SETTING_NUMBER_OF_REPLICAS, 0).build()
-        );
+        createIndex(indexName, randomNumberOfShards, 0);
         ensureGreen(indexName);
 
-        logger.info("---> Waiting for all [" + randomNumberOfShards + "] shards to be assigned to node " + firstDataNodeName);
-        safeAwait(verifyAssignmentToFirstNodeListener);
+        logger.info("--> Waiting for all [" + randomNumberOfShards + "] shards to be assigned to node " + firstDataNodeName);
+        try {
+            safeAwait(verifyAssignmentToFirstNodeListener);
+        } catch (AssertionError error) {
+            dumpClusterState();
+            throw error;
+        }
 
         return new TestHarness(
             firstDataNodeName,
@@ -878,6 +834,11 @@ private TestHarness setUpThreeTestNodesAndAllIndexShardsOnFirstNode() {
         );
     }
 
+    private void dumpClusterState() {
+        logger.info("--> Failed to reach expected allocation state. Dumping cluster state");
+        logClusterState();
+    }
+
     /**
      * Carries set-up state from {@link #setUpThreeTestNodesAndAllIndexShardsOnFirstNode()} to the testing logic.
      */
