Call reroute even if there are no nodes under utilisation/latency threshold

Author: nicktindall

server/src/main/java/org/elasticsearch/cluster/routing/allocation/WriteLoadConstraintMonitor.java

@@ -75,30 +75,20 @@ public void onNewInfo(ClusterInfo clusterInfo) {
 
         final int numberOfNodes = clusterInfo.getNodeUsageStatsForThreadPools().size();
         final Set<String> nodeIdsExceedingLatencyThreshold = Sets.newHashSetWithExpectedSize(numberOfNodes);
-        final Set<String> potentialRelocationTargets = Sets.newHashSetWithExpectedSize(numberOfNodes);
         clusterInfo.getNodeUsageStatsForThreadPools().forEach((nodeId, usageStats) -> {
             final NodeUsageStatsForThreadPools.ThreadPoolUsageStats writeThreadPoolStats = usageStats.threadPoolUsageStatsMap()
                 .get(ThreadPool.Names.WRITE);
             assert writeThreadPoolStats != null : "Write thread pool is not publishing usage stats for node [" + nodeId + "]";
             if (writeThreadPoolStats.maxThreadPoolQueueLatencyMillis() > writeLoadConstraintSettings.getQueueLatencyThreshold().millis()) {
                 nodeIdsExceedingLatencyThreshold.add(nodeId);
-            } else if (writeThreadPoolStats.averageThreadPoolUtilization() <= writeLoadConstraintSettings.getHighUtilizationThreshold()) {
-                potentialRelocationTargets.add(nodeId);
             }
         });
-        assert Sets.intersection(nodeIdsExceedingLatencyThreshold, potentialRelocationTargets).isEmpty()
-            : "We assume any nodes exceeding the latency threshold are not viable targets for relocation";
 
         if (nodeIdsExceedingLatencyThreshold.isEmpty()) {
             logger.debug("No hot-spotting nodes detected");
             return;
         }
 
-        if (potentialRelocationTargets.isEmpty()) {
-            logger.debug("No nodes are suitable as relocation targets");
-            return;
-        }
-
         final long currentTimeMillis = currentTimeMillisSupplier.getAsLong();
         final long timeSinceLastRerouteMillis = currentTimeMillis - lastRerouteTimeMillis;
         final boolean haveCalledRerouteRecently = timeSinceLastRerouteMillis < writeLoadConstraintSettings.getMinimumRerouteInterval()
@@ -108,9 +98,8 @@ public void onNewInfo(ClusterInfo clusterInfo) {
             || Sets.difference(nodeIdsExceedingLatencyThreshold, lastSetOfHotSpottedNodes).isEmpty() == false) {
             if (logger.isDebugEnabled()) {
                 logger.debug(
-                    "Found {} exceeding the write thread pool queue latency threshold ({} with capacity, {} total), triggering reroute",
+                    "Found {} exceeding the write thread pool queue latency threshold ({} total), triggering reroute",
                     nodeSummary(nodeIdsExceedingLatencyThreshold),
-                    nodeSummary(potentialRelocationTargets),
                     state.nodes().size()
                 );
             }

server/src/test/java/org/elasticsearch/cluster/routing/allocation/WriteLoadConstraintMonitorTests.java

@@ -164,89 +164,6 @@ public void testRerouteIsNotCalledWhenNoNodesAreHotSpotting() {
         }
     }
 
-    @TestLogging(
-        value = "org.elasticsearch.cluster.routing.allocation.WriteLoadConstraintMonitor:DEBUG",
-        reason = "ensure we're skipping reroute for the right reason"
-    )
-    public void testRerouteIsNotCalledWhenNoNodeIsUnderUtilizationThreshold() {
-        final TestState testState = createRandomTestStateThatWillTriggerReroute();
-
-        // Transform the node usage stats so that all nodes are at the high-utilization threshold
-        final var nodeUsageStatsWithHighUtilization = Maps.transformValues(
-            testState.clusterInfo.getNodeUsageStatsForThreadPools(),
-            stats -> new NodeUsageStatsForThreadPools(
-                stats.nodeId(),
-                Maps.transformValues(
-                    stats.threadPoolUsageStatsMap(),
-                    tpStats -> new NodeUsageStatsForThreadPools.ThreadPoolUsageStats(
-                        tpStats.totalThreadPoolThreads(),
-                        (testState.highUtilizationThresholdPercent + 1) / 100f,
-                        tpStats.maxThreadPoolQueueLatencyMillis()
-                    )
-                )
-            )
-        );
-
-        final WriteLoadConstraintMonitor writeLoadConstraintMonitor = new WriteLoadConstraintMonitor(
-            testState.clusterSettings,
-            testState.currentTimeSupplier,
-            () -> testState.clusterState,
-            testState.mockRerouteService
-        );
-
-        try (MockLog mockLog = MockLog.capture(WriteLoadConstraintMonitor.class)) {
-            mockLog.addExpectation(
-                new MockLog.SeenEventExpectation(
-                    "don't reroute due to all nodes exceeding utilization threshold",
-                    WriteLoadConstraintMonitor.class.getCanonicalName(),
-                    Level.DEBUG,
-                    "No nodes are suitable as relocation targets"
-                )
-            );
-            writeLoadConstraintMonitor.onNewInfo(
-                ClusterInfo.builder().nodeUsageStatsForThreadPools(nodeUsageStatsWithHighUtilization).build()
-            );
-            mockLog.assertAllExpectationsMatched();
-            verifyNoInteractions(testState.mockRerouteService);
-        }
-    }
-
-    @TestLogging(
-        value = "org.elasticsearch.cluster.routing.allocation.WriteLoadConstraintMonitor:DEBUG",
-        reason = "ensure we're skipping reroute for the right reason"
-    )
-    public void testRerouteIsNotCalledWhenNoNodeIsUnderLatencyThreshold() {
-        final TestState testState = createRandomTestStateThatWillTriggerReroute();
-
-        final ClusterInfo clusterInfoWithAllNodesOverLatencyThreshold = createClusterInfoWithHotSpots(
-            testState.clusterState,
-            testState.numberOfNodes,
-            testState.latencyThresholdMillis,
-            testState.highUtilizationThresholdPercent
-        );
-
-        final WriteLoadConstraintMonitor writeLoadConstraintMonitor = new WriteLoadConstraintMonitor(
-            testState.clusterSettings,
-            testState.currentTimeSupplier,
-            () -> testState.clusterState,
-            testState.mockRerouteService
-        );
-
-        try (MockLog mockLog = MockLog.capture(WriteLoadConstraintMonitor.class)) {
-            mockLog.addExpectation(
-                new MockLog.SeenEventExpectation(
-                    "don't reroute due to all nodes exceeding latency threshold",
-                    WriteLoadConstraintMonitor.class.getCanonicalName(),
-                    Level.DEBUG,
-                    "No nodes are suitable as relocation targets"
-                )
-            );
-            writeLoadConstraintMonitor.onNewInfo(clusterInfoWithAllNodesOverLatencyThreshold);
-            mockLog.assertAllExpectationsMatched();
-            verifyNoInteractions(testState.mockRerouteService);
-        }
-    }
-
     @TestLogging(
         value = "org.elasticsearch.cluster.routing.allocation.WriteLoadConstraintMonitor:DEBUG",
         reason = "ensure we're skipping reroute for the right reason"