From d14dd771ff9404e3cfd8f06a1e3538f40e113860 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Tue, 2 Sep 2025 14:08:57 +1000 Subject: [PATCH 1/5] Log allocation explain for unassigned in desiredBalance computaion This PR adds allocaiton explain to logs when there are unassigned shards after a converged DesiredBalance computation. The allocation explain prefers unassigned primary over replica. The logging is frequency capped at one minute by default. Relates: ES-12797 --- .../elasticsearch/cluster/ClusterModule.java | 17 ++- .../allocator/DesiredBalanceComputer.java | 58 +++++++- .../DesiredBalanceShardsAllocator.java | 19 ++- .../common/settings/ClusterSettings.java | 2 + ...nsportDeleteDesiredBalanceActionTests.java | 2 +- .../AllocationStatsServiceTests.java | 3 +- .../ClusterAllocationSimulationTests.java | 3 +- .../DesiredBalanceComputerTests.java | 129 +++++++++++++++++- .../DesiredBalanceShardsAllocatorTests.java | 30 ++-- .../cluster/ESAllocationTestCase.java | 8 +- 10 files changed, 249 insertions(+), 22 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/ClusterModule.java b/server/src/main/java/org/elasticsearch/cluster/ClusterModule.java index 48c67d8520e59..0b1fc54bb619f 100644 --- a/server/src/main/java/org/elasticsearch/cluster/ClusterModule.java +++ b/server/src/main/java/org/elasticsearch/cluster/ClusterModule.java @@ -37,6 +37,8 @@ import org.elasticsearch.cluster.routing.allocation.AllocationStatsService; import org.elasticsearch.cluster.routing.allocation.ExistingShardsAllocator; import org.elasticsearch.cluster.routing.allocation.NodeAllocationStatsAndWeightsCalculator; +import org.elasticsearch.cluster.routing.allocation.RoutingAllocation; +import org.elasticsearch.cluster.routing.allocation.ShardAllocationDecision; import org.elasticsearch.cluster.routing.allocation.WriteLoadForecaster; import org.elasticsearch.cluster.routing.allocation.allocator.BalancedShardsAllocator; import org.elasticsearch.cluster.routing.allocation.allocator.BalancerSettings; @@ -107,6 +109,8 @@ import java.util.Objects; import java.util.function.Supplier; +import static org.elasticsearch.cluster.routing.allocation.allocator.DesiredBalanceShardsAllocator.*; + /** * Configures classes and services that affect the entire cluster. */ @@ -171,7 +175,8 @@ public ClusterModule( this::reconcile, writeLoadForecaster, telemetryProvider, - nodeAllocationStatsAndWeightsCalculator + nodeAllocationStatsAndWeightsCalculator, + this::explainShardAllocation ); this.clusterService = clusterService; this.indexNameExpressionResolver = new IndexNameExpressionResolver(threadPool.getThreadContext(), systemIndices, projectResolver); @@ -237,6 +242,10 @@ private ClusterState reconcile(ClusterState clusterState, RerouteStrategy rerout return allocationService.executeWithRoutingAllocation(clusterState, "reconcile-desired-balance", rerouteStrategy); } + private ShardAllocationDecision explainShardAllocation(ShardRouting shardRouting, RoutingAllocation allocation) { + return allocationService.explainShardAllocation(shardRouting, allocation); + } + public static List getNamedWriteables() { List entries = new ArrayList<>(); // Cluster State @@ -489,7 +498,8 @@ private static ShardsAllocator createShardsAllocator( DesiredBalanceReconcilerAction reconciler, WriteLoadForecaster writeLoadForecaster, TelemetryProvider telemetryProvider, - NodeAllocationStatsAndWeightsCalculator nodeAllocationStatsAndWeightsCalculator + NodeAllocationStatsAndWeightsCalculator nodeAllocationStatsAndWeightsCalculator, + ShardAllocationExplainer shardAllocationExplainer ) { Map> allocators = new HashMap<>(); allocators.put( @@ -505,7 +515,8 @@ private static ShardsAllocator createShardsAllocator( clusterService, reconciler, telemetryProvider, - nodeAllocationStatsAndWeightsCalculator + nodeAllocationStatsAndWeightsCalculator, + shardAllocationExplainer ) ); diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputer.java b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputer.java index a0e103edf97c7..fe03b4a5a3d6b 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputer.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputer.java @@ -17,13 +17,17 @@ import org.elasticsearch.cluster.routing.ShardRouting; import org.elasticsearch.cluster.routing.UnassignedInfo; import org.elasticsearch.cluster.routing.allocation.RoutingAllocation; +import org.elasticsearch.cluster.routing.allocation.ShardAllocationDecision; +import org.elasticsearch.cluster.routing.allocation.allocator.DesiredBalanceShardsAllocator.ShardAllocationExplainer; import org.elasticsearch.cluster.routing.allocation.command.MoveAllocationCommand; import org.elasticsearch.cluster.routing.allocation.decider.Decision; +import org.elasticsearch.common.FrequencyCappedAction; import org.elasticsearch.common.metrics.MeanMetric; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.time.TimeProvider; import org.elasticsearch.common.util.Maps; +import org.elasticsearch.common.xcontent.ChunkedToXContentHelper; import org.elasticsearch.core.Strings; import org.elasticsearch.core.TimeValue; import org.elasticsearch.index.shard.ShardId; @@ -39,6 +43,7 @@ import java.util.TreeMap; import java.util.TreeSet; import java.util.function.Predicate; +import java.util.stream.Stream; import static java.util.stream.Collectors.toUnmodifiableSet; @@ -48,9 +53,13 @@ public class DesiredBalanceComputer { private static final Logger logger = LogManager.getLogger(DesiredBalanceComputer.class); + private static final Logger allocationExplainLogger = LogManager.getLogger( + DesiredBalanceComputer.class.getCanonicalName() + ".allocation_explain" + ); private final ShardsAllocator delegateAllocator; private final TimeProvider timeProvider; + private final ShardAllocationExplainer shardAllocationExplainer; // stats protected final MeanMetric iterations = new MeanMetric(); @@ -77,15 +86,27 @@ public class DesiredBalanceComputer { private long lastConvergedTimeMillis; private long lastNotConvergedLogMessageTimeMillis; private Level convergenceLogMsgLevel; + private final FrequencyCappedAction logAllocationExplainForUnassigned; - public DesiredBalanceComputer(ClusterSettings clusterSettings, TimeProvider timeProvider, ShardsAllocator delegateAllocator) { + public DesiredBalanceComputer( + ClusterSettings clusterSettings, + TimeProvider timeProvider, + ShardsAllocator delegateAllocator, + ShardAllocationExplainer shardAllocationExplainer + ) { this.delegateAllocator = delegateAllocator; this.timeProvider = timeProvider; + this.shardAllocationExplainer = shardAllocationExplainer; this.numComputeCallsSinceLastConverged = 0; this.numIterationsSinceLastConverged = 0; + this.logAllocationExplainForUnassigned = new FrequencyCappedAction(timeProvider::relativeTimeInMillis, TimeValue.ZERO); this.lastConvergedTimeMillis = timeProvider.relativeTimeInMillis(); this.lastNotConvergedLogMessageTimeMillis = lastConvergedTimeMillis; this.convergenceLogMsgLevel = Level.DEBUG; + clusterSettings.initializeAndWatch( + DesiredBalanceShardsAllocator.ALLOCATION_EXPLAIN_LOGGING_INTERVAL, + logAllocationExplainForUnassigned::setMinInterval + ); clusterSettings.initializeAndWatch(PROGRESS_LOG_INTERVAL_SETTING, value -> this.progressLogInterval = value); clusterSettings.initializeAndWatch( MAX_BALANCE_COMPUTATION_TIME_DURING_INDEX_CREATION_SETTING, @@ -462,10 +483,45 @@ public DesiredBalance compute( ); } + maybeLogAllocationExplainForUnassigned(finishReason, routingNodes, routingAllocation); + long lastConvergedIndex = hasChanges ? previousDesiredBalance.lastConvergedIndex() : desiredBalanceInput.index(); return new DesiredBalance(lastConvergedIndex, assignments, routingNodes.getBalanceWeightStatsPerNode(), finishReason); } + private void maybeLogAllocationExplainForUnassigned( + DesiredBalance.ComputationFinishReason finishReason, + RoutingNodes routingNodes, + RoutingAllocation routingAllocation + ) { + if (allocationExplainLogger.isDebugEnabled() + && finishReason == DesiredBalance.ComputationFinishReason.CONVERGED + && routingNodes.hasUnassignedShards()) { + logAllocationExplainForUnassigned.maybeExecute(() -> { + final var unassigned = routingNodes.unassigned(); + final var shardRouting = Stream.concat(unassigned.stream(), unassigned.ignored().stream()) + .filter(ShardRouting::primary) + .findFirst() + .orElse(Stream.concat(unassigned.stream(), unassigned.ignored().stream()).iterator().next()); + final var originalDebugMode = routingAllocation.getDebugMode(); + routingAllocation.setDebugMode(RoutingAllocation.DebugMode.EXCLUDE_YES_DECISIONS); + final ShardAllocationDecision shardAllocationDecision; + try { + shardAllocationDecision = shardAllocationExplainer.explain(shardRouting, routingAllocation); + } finally { + routingAllocation.setDebugMode(originalDebugMode); + } + allocationExplainLogger.debug( + "unassigned shard [{}] with allocation decision {}", + shardRouting, + org.elasticsearch.common.Strings.toString( + p -> ChunkedToXContentHelper.object("node_allocation_decision", shardAllocationDecision.toXContentChunked(p)) + ) + ); + }); + } + } + // visible for testing boolean hasEnoughIterations(int currentIteration) { return true; diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceShardsAllocator.java b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceShardsAllocator.java index e0b927a84519c..6f833f1b89664 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceShardsAllocator.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceShardsAllocator.java @@ -35,6 +35,8 @@ import org.elasticsearch.common.metrics.CounterMetric; import org.elasticsearch.common.metrics.MeanMetric; import org.elasticsearch.common.settings.ClusterSettings; +import org.elasticsearch.common.settings.Setting; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.telemetry.TelemetryProvider; import org.elasticsearch.threadpool.ThreadPool; @@ -57,6 +59,13 @@ public class DesiredBalanceShardsAllocator implements ShardsAllocator { private static final Logger logger = LogManager.getLogger(DesiredBalanceShardsAllocator.class); + public static final Setting ALLOCATION_EXPLAIN_LOGGING_INTERVAL = Setting.timeSetting( + "cluster.routing.allocation.desired_balance.allocation_explain_log_interval", + TimeValue.timeValueMinutes(1), + Setting.Property.Dynamic, + Setting.Property.NodeScope + ); + private final ShardsAllocator delegateAllocator; private final ThreadPool threadPool; /** @@ -109,6 +118,11 @@ public interface DesiredBalanceReconcilerAction { ClusterState apply(ClusterState clusterState, RerouteStrategy rerouteStrategy); } + @FunctionalInterface + public interface ShardAllocationExplainer { + ShardAllocationDecision explain(ShardRouting shard, RoutingAllocation allocation); + } + public DesiredBalanceShardsAllocator( ClusterSettings clusterSettings, ShardsAllocator delegateAllocator, @@ -116,13 +130,14 @@ public DesiredBalanceShardsAllocator( ClusterService clusterService, DesiredBalanceReconcilerAction reconciler, TelemetryProvider telemetryProvider, - NodeAllocationStatsAndWeightsCalculator nodeAllocationStatsAndWeightsCalculator + NodeAllocationStatsAndWeightsCalculator nodeAllocationStatsAndWeightsCalculator, + ShardAllocationExplainer shardAllocationExplainer ) { this( delegateAllocator, threadPool, clusterService, - new DesiredBalanceComputer(clusterSettings, threadPool, delegateAllocator), + new DesiredBalanceComputer(clusterSettings, threadPool, delegateAllocator, shardAllocationExplainer), reconciler, telemetryProvider, nodeAllocationStatsAndWeightsCalculator diff --git a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java index 9c2d6fab10368..95e26f623d1a8 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java @@ -51,6 +51,7 @@ import org.elasticsearch.cluster.routing.allocation.allocator.BalancedShardsAllocator; import org.elasticsearch.cluster.routing.allocation.allocator.DesiredBalanceComputer; import org.elasticsearch.cluster.routing.allocation.allocator.DesiredBalanceReconciler; +import org.elasticsearch.cluster.routing.allocation.allocator.DesiredBalanceShardsAllocator; import org.elasticsearch.cluster.routing.allocation.decider.AwarenessAllocationDecider; import org.elasticsearch.cluster.routing.allocation.decider.ClusterRebalanceAllocationDecider; import org.elasticsearch.cluster.routing.allocation.decider.ConcurrentRebalanceAllocationDecider; @@ -229,6 +230,7 @@ public void apply(Settings value, Settings current, Settings previous) { DataStreamAutoShardingService.CLUSTER_AUTO_SHARDING_MIN_WRITE_THREADS, DataStreamAutoShardingService.DATA_STREAMS_AUTO_SHARDING_INCREASE_SHARDS_LOAD_METRIC, DataStreamAutoShardingService.DATA_STREAMS_AUTO_SHARDING_DECREASE_SHARDS_LOAD_METRIC, + DesiredBalanceShardsAllocator.ALLOCATION_EXPLAIN_LOGGING_INTERVAL, DesiredBalanceComputer.PROGRESS_LOG_INTERVAL_SETTING, DesiredBalanceComputer.MAX_BALANCE_COMPUTATION_TIME_DURING_INDEX_CREATION_SETTING, DesiredBalanceReconciler.UNDESIRED_ALLOCATIONS_LOG_INTERVAL_SETTING, diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/allocation/TransportDeleteDesiredBalanceActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/allocation/TransportDeleteDesiredBalanceActionTests.java index 999845fa73610..271f8864b1aa4 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/allocation/TransportDeleteDesiredBalanceActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/allocation/TransportDeleteDesiredBalanceActionTests.java @@ -99,7 +99,7 @@ public void testDeleteDesiredBalance() throws Exception { var clusterSettings = ClusterSettings.createBuiltInClusterSettings(settings); var delegate = new BalancedShardsAllocator(); - var computer = new DesiredBalanceComputer(clusterSettings, threadPool, delegate) { + var computer = new DesiredBalanceComputer(clusterSettings, threadPool, delegate, DUMMY_EXPLAINER) { final AtomicReference lastComputationInput = new AtomicReference<>(); diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationStatsServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationStatsServiceTests.java index 4ce195721b228..d36a0b66ce742 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationStatsServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationStatsServiceTests.java @@ -176,7 +176,8 @@ public void testUndesiredShardCount() { clusterService, (innerState, strategy) -> innerState, TelemetryProvider.NOOP, - EMPTY_NODE_ALLOCATION_STATS + EMPTY_NODE_ALLOCATION_STATS, + DUMMY_EXPLAINER ) { @Override public DesiredBalance getDesiredBalance() { diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/ClusterAllocationSimulationTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/ClusterAllocationSimulationTests.java index 6ef622948f5c5..7ae8467ecac4c 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/ClusterAllocationSimulationTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/ClusterAllocationSimulationTests.java @@ -490,7 +490,8 @@ private Map.Entry createNewAllocationSer (clusterState, routingAllocationAction) -> strategyRef.get() .executeWithRoutingAllocation(clusterState, "reconcile-desired-balance", routingAllocationAction), TelemetryProvider.NOOP, - EMPTY_NODE_ALLOCATION_STATS + EMPTY_NODE_ALLOCATION_STATS, + DUMMY_EXPLAINER ) { @Override public void allocate(RoutingAllocation allocation, ActionListener listener) { diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputerTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputerTests.java index d204c1c925d40..ecae57bdd60fa 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputerTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputerTests.java @@ -22,6 +22,9 @@ import org.elasticsearch.cluster.TestShardRoutingRoleStrategies; import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.metadata.Metadata; +import org.elasticsearch.cluster.metadata.NodesShutdownMetadata; +import org.elasticsearch.cluster.metadata.SingleNodeShutdownMetadata; +import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNodeRole; import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.cluster.routing.AllocationId; @@ -60,6 +63,7 @@ import org.elasticsearch.snapshots.SnapshotId; import org.elasticsearch.snapshots.SnapshotShardSizeInfo; import org.elasticsearch.test.MockLog; +import org.elasticsearch.test.junit.annotations.TestLogging; import java.util.ArrayList; import java.util.HashMap; @@ -1288,7 +1292,7 @@ public void allocate(RoutingAllocation allocation) { public ShardAllocationDecision decideShardAllocation(ShardRouting shard, RoutingAllocation allocation) { throw new AssertionError("only used for allocation explain"); } - }); + }, DUMMY_EXPLAINER); assertLoggerExpectationsFor(() -> { var iteration = new AtomicInteger(0); @@ -1320,7 +1324,8 @@ public void testLoggingOfComputeCallsAndIterationsSinceConvergence() { final var computer = new DesiredBalanceComputer( clusterSettings, TimeProviderUtils.create(timeInMillis::incrementAndGet), - new BalancedShardsAllocator(Settings.EMPTY) + new BalancedShardsAllocator(Settings.EMPTY), + DUMMY_EXPLAINER ) { @Override boolean hasEnoughIterations(int currentIteration) { @@ -1441,6 +1446,124 @@ record LogExpectationData( assertDesiredAssignments(desiredBalance.get(), expectedAssignmentsMap); } + @TestLogging( + value = "org.elasticsearch.cluster.routing.allocation.allocator.DesiredBalanceComputer.allocation_explain:DEBUG", + reason = "test logging for allocation explain" + ) + public void testLogAllocationExplainForUnassigned() { + final var timeInMillis = new AtomicLong(0L); + final long logIntervalInSeconds = between(60, 120); + final ClusterSettings clusterSettings = createBuiltInClusterSettings( + Settings.builder() + .put( + DesiredBalanceShardsAllocator.ALLOCATION_EXPLAIN_LOGGING_INTERVAL.getKey(), + TimeValue.timeValueSeconds(logIntervalInSeconds) + ) + .build() + ); + final var computer = new DesiredBalanceComputer( + clusterSettings, + TimeProviderUtils.create(timeInMillis::incrementAndGet), + new BalancedShardsAllocator(Settings.EMPTY), + createAllocationService()::explainShardAllocation + ); + final String loggerName = DesiredBalanceComputer.class.getCanonicalName() + ".allocation_explain"; + + // No logging since no unassigned shards + { + final var allocation = new RoutingAllocation( + randomAllocationDeciders(Settings.EMPTY, clusterSettings), + createInitialClusterState(1, 1, 0), + ClusterInfo.EMPTY, + SnapshotShardSizeInfo.EMPTY, + 0L + ); + try (var mockLog = MockLog.capture(loggerName)) { + mockLog.addExpectation( + new MockLog.UnseenEventExpectation( + "Should log allocation explain for unassigned primary shard", + loggerName, + Level.DEBUG, + "unassigned shard * with allocation decision *" + ) + ); + computer.compute(DesiredBalance.BECOME_MASTER_INITIAL, DesiredBalanceInput.create(1, allocation), queue(), ignore -> true); + mockLog.assertAllExpectationsMatched(); + } + } + + var initialState = createInitialClusterState(1, 1, 1); + // Logging for unassigned primary shard (preferred over unassigned replica) + { + final DiscoveryNode dataNode = initialState.nodes().getDataNodes().values().iterator().next(); + final var clusterState = initialState.copyAndUpdateMetadata( + b -> b.putCustom( + NodesShutdownMetadata.TYPE, + new NodesShutdownMetadata( + Map.of( + dataNode.getId(), + SingleNodeShutdownMetadata.builder() + .setNodeId(dataNode.getId()) + .setType(SingleNodeShutdownMetadata.Type.REMOVE) + .setReason("test") + .setStartedAtMillis(timeInMillis.get()) + .build() + ) + ) + ) + ); + final var allocation = new RoutingAllocation( + randomAllocationDeciders(Settings.EMPTY, clusterSettings), + clusterState, + ClusterInfo.EMPTY, + SnapshotShardSizeInfo.EMPTY, + 0L + ); + try (var mockLog = MockLog.capture(loggerName)) { + mockLog.addExpectation( + new MockLog.SeenEventExpectation( + "Should log allocation explain for unassigned primary shard", + loggerName, + Level.DEBUG, + "unassigned shard [[test-index][0], node[null], [P], * with allocation decision *" + + "\"decider\":\"node_shutdown\",\"decision\":\"NO\"*" + ) + ); + computer.compute(DesiredBalance.BECOME_MASTER_INITIAL, DesiredBalanceInput.create(1, allocation), queue(), ignore -> true); + mockLog.assertAllExpectationsMatched(); + } + } + + // Logging for unassigned replica shard (since primary is now assigned) + { + final var allocation = new RoutingAllocation( + randomAllocationDeciders(Settings.EMPTY, clusterSettings), + initialState, + ClusterInfo.EMPTY, + SnapshotShardSizeInfo.EMPTY, + 0L + ); + try (var mockLog = MockLog.capture(loggerName)) { + final var expectation = new MockLog.EventuallySeenEventExpectation( + "Should log allocation explain for unassigned replica shard", + loggerName, + Level.DEBUG, + "unassigned shard [[test-index][0], node[null], [R], * with allocation decision *" + + "\"decider\":\"same_shard\",\"decision\":\"NO\"*" + ); + mockLog.addExpectation(expectation); + computer.compute(DesiredBalance.BECOME_MASTER_INITIAL, DesiredBalanceInput.create(1, allocation), queue(), ignore -> true); + mockLog.assertAllExpectationsMatched(); + + // Simulate time passing + timeInMillis.addAndGet(logIntervalInSeconds * 1000); + expectation.setExpectSeen(); + computer.compute(DesiredBalance.BECOME_MASTER_INITIAL, DesiredBalanceInput.create(1, allocation), queue(), ignore -> true); + mockLog.assertAllExpectationsMatched(); + } + } + } + private static ShardId findShardId(ClusterState clusterState, String name) { return clusterState.getRoutingTable().index(name).shard(0).shardId(); } @@ -1542,7 +1665,7 @@ public ShardAllocationDecision decideShardAllocation(ShardRouting shard, Routing } private static DesiredBalanceComputer createDesiredBalanceComputer(ShardsAllocator allocator) { - return new DesiredBalanceComputer(createBuiltInClusterSettings(), TimeProviderUtils.create(() -> 0L), allocator); + return new DesiredBalanceComputer(createBuiltInClusterSettings(), TimeProviderUtils.create(() -> 0L), allocator, DUMMY_EXPLAINER); } private static void assertDesiredAssignments(DesiredBalance desiredBalance, Map expected) { diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceShardsAllocatorTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceShardsAllocatorTests.java index 21d547c1593b8..23f95f4218c2f 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceShardsAllocatorTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceShardsAllocatorTests.java @@ -174,7 +174,8 @@ public ClusterState apply(ClusterState clusterState, RerouteStrategy routingAllo clusterService, reconcileAction, TelemetryProvider.NOOP, - EMPTY_NODE_ALLOCATION_STATS + EMPTY_NODE_ALLOCATION_STATS, + DUMMY_EXPLAINER ); assertValidStats(desiredBalanceShardsAllocator.getStats()); var allocationService = createAllocationService(desiredBalanceShardsAllocator, createGatewayAllocator(allocateUnassigned)); @@ -302,7 +303,8 @@ public ClusterState apply(ClusterState clusterState, RerouteStrategy routingAllo clusterService, reconcileAction, TelemetryProvider.NOOP, - EMPTY_NODE_ALLOCATION_STATS + EMPTY_NODE_ALLOCATION_STATS, + DUMMY_EXPLAINER ); var allocationService = new AllocationService( new AllocationDeciders(List.of()), @@ -403,7 +405,7 @@ public ShardAllocationDecision decideShardAllocation(ShardRouting shard, Routing shardsAllocator, threadPool, clusterService, - new DesiredBalanceComputer(clusterSettings, TimeProviderUtils.create(time::get), shardsAllocator) { + new DesiredBalanceComputer(clusterSettings, TimeProviderUtils.create(time::get), shardsAllocator, DUMMY_EXPLAINER) { @Override public DesiredBalance compute( DesiredBalance previousDesiredBalance, @@ -530,7 +532,7 @@ public ClusterState apply(ClusterState clusterState, RerouteStrategy routingAllo shardsAllocator, threadPool, clusterService, - new DesiredBalanceComputer(clusterSettings, threadPool, shardsAllocator) { + new DesiredBalanceComputer(clusterSettings, threadPool, shardsAllocator, DUMMY_EXPLAINER) { @Override public DesiredBalance compute( DesiredBalance previousDesiredBalance, @@ -634,7 +636,7 @@ public ClusterState apply(ClusterState clusterState, RerouteStrategy routingAllo shardsAllocator, threadPool, clusterService, - new DesiredBalanceComputer(clusterSettings, threadPool, shardsAllocator) { + new DesiredBalanceComputer(clusterSettings, threadPool, shardsAllocator, DUMMY_EXPLAINER) { @Override public DesiredBalance compute( DesiredBalance previousDesiredBalance, @@ -723,7 +725,7 @@ public void testResetDesiredBalance() { var delegateAllocator = createShardsAllocator(); var clusterSettings = createBuiltInClusterSettings(); - var desiredBalanceComputer = new DesiredBalanceComputer(clusterSettings, threadPool, delegateAllocator) { + var desiredBalanceComputer = new DesiredBalanceComputer(clusterSettings, threadPool, delegateAllocator, DUMMY_EXPLAINER) { final AtomicReference lastComputationInput = new AtomicReference<>(); @@ -792,7 +794,12 @@ public void testResetDesiredBalanceOnNoLongerMaster() { var clusterService = ClusterServiceUtils.createClusterService(clusterState, threadPool); var delegateAllocator = createShardsAllocator(); - var desiredBalanceComputer = new DesiredBalanceComputer(createBuiltInClusterSettings(), threadPool, delegateAllocator); + var desiredBalanceComputer = new DesiredBalanceComputer( + createBuiltInClusterSettings(), + threadPool, + delegateAllocator, + DUMMY_EXPLAINER + ); var desiredBalanceShardsAllocator = new DesiredBalanceShardsAllocator( delegateAllocator, threadPool, @@ -842,7 +849,12 @@ public void testResetDesiredBalanceOnNodeShutdown() { final var resetCalled = new AtomicBoolean(); var delegateAllocator = createShardsAllocator(); - var desiredBalanceComputer = new DesiredBalanceComputer(createBuiltInClusterSettings(), threadPool, delegateAllocator); + var desiredBalanceComputer = new DesiredBalanceComputer( + createBuiltInClusterSettings(), + threadPool, + delegateAllocator, + DUMMY_EXPLAINER + ); var desiredBalanceAllocator = new DesiredBalanceShardsAllocator( delegateAllocator, threadPool, @@ -932,7 +944,7 @@ public void testNotReconcileEagerlyForEmptyRoutingTable() { shardsAllocator, threadPool, clusterService, - new DesiredBalanceComputer(clusterSettings, TimeProviderUtils.create(() -> 1L), shardsAllocator) { + new DesiredBalanceComputer(clusterSettings, TimeProviderUtils.create(() -> 1L), shardsAllocator, DUMMY_EXPLAINER) { @Override public DesiredBalance compute( DesiredBalance previousDesiredBalance, diff --git a/test/framework/src/main/java/org/elasticsearch/cluster/ESAllocationTestCase.java b/test/framework/src/main/java/org/elasticsearch/cluster/ESAllocationTestCase.java index a2d81e0203f15..664b4484221be 100644 --- a/test/framework/src/main/java/org/elasticsearch/cluster/ESAllocationTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/cluster/ESAllocationTestCase.java @@ -29,6 +29,7 @@ import org.elasticsearch.cluster.routing.allocation.FailedShard; import org.elasticsearch.cluster.routing.allocation.NodeAllocationStatsAndWeightsCalculator; import org.elasticsearch.cluster.routing.allocation.RoutingAllocation; +import org.elasticsearch.cluster.routing.allocation.ShardAllocationDecision; import org.elasticsearch.cluster.routing.allocation.WriteLoadForecaster; import org.elasticsearch.cluster.routing.allocation.allocator.BalancedShardsAllocator; import org.elasticsearch.cluster.routing.allocation.allocator.BalancerSettings; @@ -98,6 +99,10 @@ public OptionalDouble getForecastedWriteLoad(IndexMetadata indexMetadata) { public void refreshLicense() {} }; + public static final DesiredBalanceShardsAllocator.ShardAllocationExplainer DUMMY_EXPLAINER = ( + shard, + allocation) -> ShardAllocationDecision.NOT_TAKEN; + public static MockAllocationService createAllocationService() { return createAllocationService(Settings.EMPTY); } @@ -176,7 +181,8 @@ private static DesiredBalanceShardsAllocator createDesiredBalanceShardsAllocator clusterService, null, TelemetryProvider.NOOP, - EMPTY_NODE_ALLOCATION_STATS + EMPTY_NODE_ALLOCATION_STATS, + DUMMY_EXPLAINER ) { private RoutingAllocation lastAllocation; From d70df7a377f7c8c5fe31ccc17af13a649dbebc0a Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Tue, 2 Sep 2025 16:18:17 +1000 Subject: [PATCH 2/5] import --- .../src/main/java/org/elasticsearch/cluster/ClusterModule.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/ClusterModule.java b/server/src/main/java/org/elasticsearch/cluster/ClusterModule.java index 0b1fc54bb619f..361b98bcc86b4 100644 --- a/server/src/main/java/org/elasticsearch/cluster/ClusterModule.java +++ b/server/src/main/java/org/elasticsearch/cluster/ClusterModule.java @@ -109,7 +109,7 @@ import java.util.Objects; import java.util.function.Supplier; -import static org.elasticsearch.cluster.routing.allocation.allocator.DesiredBalanceShardsAllocator.*; +import static org.elasticsearch.cluster.routing.allocation.allocator.DesiredBalanceShardsAllocator.ShardAllocationExplainer; /** * Configures classes and services that affect the entire cluster. From e1b620743810f5dbfadda09424c50b9babd88cf1 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Wed, 3 Sep 2025 11:34:22 +1000 Subject: [PATCH 3/5] rename --- ...ansportDeleteDesiredBalanceActionTests.java | 2 +- .../AllocationStatsServiceTests.java | 2 +- .../ClusterAllocationSimulationTests.java | 2 +- .../allocator/DesiredBalanceComputerTests.java | 11 ++++++++--- .../DesiredBalanceShardsAllocatorTests.java | 18 +++++++++--------- .../cluster/ESAllocationTestCase.java | 4 ++-- 6 files changed, 22 insertions(+), 17 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/allocation/TransportDeleteDesiredBalanceActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/allocation/TransportDeleteDesiredBalanceActionTests.java index 271f8864b1aa4..3955842010503 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/allocation/TransportDeleteDesiredBalanceActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/allocation/TransportDeleteDesiredBalanceActionTests.java @@ -99,7 +99,7 @@ public void testDeleteDesiredBalance() throws Exception { var clusterSettings = ClusterSettings.createBuiltInClusterSettings(settings); var delegate = new BalancedShardsAllocator(); - var computer = new DesiredBalanceComputer(clusterSettings, threadPool, delegate, DUMMY_EXPLAINER) { + var computer = new DesiredBalanceComputer(clusterSettings, threadPool, delegate, TEST_ONLY_EXPLAINER) { final AtomicReference lastComputationInput = new AtomicReference<>(); diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationStatsServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationStatsServiceTests.java index d36a0b66ce742..9794e22df3db7 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationStatsServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationStatsServiceTests.java @@ -177,7 +177,7 @@ public void testUndesiredShardCount() { (innerState, strategy) -> innerState, TelemetryProvider.NOOP, EMPTY_NODE_ALLOCATION_STATS, - DUMMY_EXPLAINER + TEST_ONLY_EXPLAINER ) { @Override public DesiredBalance getDesiredBalance() { diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/ClusterAllocationSimulationTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/ClusterAllocationSimulationTests.java index 7ae8467ecac4c..4cb9e2b10a97e 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/ClusterAllocationSimulationTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/ClusterAllocationSimulationTests.java @@ -491,7 +491,7 @@ private Map.Entry createNewAllocationSer .executeWithRoutingAllocation(clusterState, "reconcile-desired-balance", routingAllocationAction), TelemetryProvider.NOOP, EMPTY_NODE_ALLOCATION_STATS, - DUMMY_EXPLAINER + TEST_ONLY_EXPLAINER ) { @Override public void allocate(RoutingAllocation allocation, ActionListener listener) { diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputerTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputerTests.java index ecae57bdd60fa..966d5207cd4b8 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputerTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputerTests.java @@ -1292,7 +1292,7 @@ public void allocate(RoutingAllocation allocation) { public ShardAllocationDecision decideShardAllocation(ShardRouting shard, RoutingAllocation allocation) { throw new AssertionError("only used for allocation explain"); } - }, DUMMY_EXPLAINER); + }, TEST_ONLY_EXPLAINER); assertLoggerExpectationsFor(() -> { var iteration = new AtomicInteger(0); @@ -1325,7 +1325,7 @@ public void testLoggingOfComputeCallsAndIterationsSinceConvergence() { clusterSettings, TimeProviderUtils.create(timeInMillis::incrementAndGet), new BalancedShardsAllocator(Settings.EMPTY), - DUMMY_EXPLAINER + TEST_ONLY_EXPLAINER ) { @Override boolean hasEnoughIterations(int currentIteration) { @@ -1665,7 +1665,12 @@ public ShardAllocationDecision decideShardAllocation(ShardRouting shard, Routing } private static DesiredBalanceComputer createDesiredBalanceComputer(ShardsAllocator allocator) { - return new DesiredBalanceComputer(createBuiltInClusterSettings(), TimeProviderUtils.create(() -> 0L), allocator, DUMMY_EXPLAINER); + return new DesiredBalanceComputer( + createBuiltInClusterSettings(), + TimeProviderUtils.create(() -> 0L), + allocator, + TEST_ONLY_EXPLAINER + ); } private static void assertDesiredAssignments(DesiredBalance desiredBalance, Map expected) { diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceShardsAllocatorTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceShardsAllocatorTests.java index 23f95f4218c2f..bedbbc7b1f639 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceShardsAllocatorTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceShardsAllocatorTests.java @@ -175,7 +175,7 @@ public ClusterState apply(ClusterState clusterState, RerouteStrategy routingAllo reconcileAction, TelemetryProvider.NOOP, EMPTY_NODE_ALLOCATION_STATS, - DUMMY_EXPLAINER + TEST_ONLY_EXPLAINER ); assertValidStats(desiredBalanceShardsAllocator.getStats()); var allocationService = createAllocationService(desiredBalanceShardsAllocator, createGatewayAllocator(allocateUnassigned)); @@ -304,7 +304,7 @@ public ClusterState apply(ClusterState clusterState, RerouteStrategy routingAllo reconcileAction, TelemetryProvider.NOOP, EMPTY_NODE_ALLOCATION_STATS, - DUMMY_EXPLAINER + TEST_ONLY_EXPLAINER ); var allocationService = new AllocationService( new AllocationDeciders(List.of()), @@ -405,7 +405,7 @@ public ShardAllocationDecision decideShardAllocation(ShardRouting shard, Routing shardsAllocator, threadPool, clusterService, - new DesiredBalanceComputer(clusterSettings, TimeProviderUtils.create(time::get), shardsAllocator, DUMMY_EXPLAINER) { + new DesiredBalanceComputer(clusterSettings, TimeProviderUtils.create(time::get), shardsAllocator, TEST_ONLY_EXPLAINER) { @Override public DesiredBalance compute( DesiredBalance previousDesiredBalance, @@ -532,7 +532,7 @@ public ClusterState apply(ClusterState clusterState, RerouteStrategy routingAllo shardsAllocator, threadPool, clusterService, - new DesiredBalanceComputer(clusterSettings, threadPool, shardsAllocator, DUMMY_EXPLAINER) { + new DesiredBalanceComputer(clusterSettings, threadPool, shardsAllocator, TEST_ONLY_EXPLAINER) { @Override public DesiredBalance compute( DesiredBalance previousDesiredBalance, @@ -636,7 +636,7 @@ public ClusterState apply(ClusterState clusterState, RerouteStrategy routingAllo shardsAllocator, threadPool, clusterService, - new DesiredBalanceComputer(clusterSettings, threadPool, shardsAllocator, DUMMY_EXPLAINER) { + new DesiredBalanceComputer(clusterSettings, threadPool, shardsAllocator, TEST_ONLY_EXPLAINER) { @Override public DesiredBalance compute( DesiredBalance previousDesiredBalance, @@ -725,7 +725,7 @@ public void testResetDesiredBalance() { var delegateAllocator = createShardsAllocator(); var clusterSettings = createBuiltInClusterSettings(); - var desiredBalanceComputer = new DesiredBalanceComputer(clusterSettings, threadPool, delegateAllocator, DUMMY_EXPLAINER) { + var desiredBalanceComputer = new DesiredBalanceComputer(clusterSettings, threadPool, delegateAllocator, TEST_ONLY_EXPLAINER) { final AtomicReference lastComputationInput = new AtomicReference<>(); @@ -798,7 +798,7 @@ public void testResetDesiredBalanceOnNoLongerMaster() { createBuiltInClusterSettings(), threadPool, delegateAllocator, - DUMMY_EXPLAINER + TEST_ONLY_EXPLAINER ); var desiredBalanceShardsAllocator = new DesiredBalanceShardsAllocator( delegateAllocator, @@ -853,7 +853,7 @@ public void testResetDesiredBalanceOnNodeShutdown() { createBuiltInClusterSettings(), threadPool, delegateAllocator, - DUMMY_EXPLAINER + TEST_ONLY_EXPLAINER ); var desiredBalanceAllocator = new DesiredBalanceShardsAllocator( delegateAllocator, @@ -944,7 +944,7 @@ public void testNotReconcileEagerlyForEmptyRoutingTable() { shardsAllocator, threadPool, clusterService, - new DesiredBalanceComputer(clusterSettings, TimeProviderUtils.create(() -> 1L), shardsAllocator, DUMMY_EXPLAINER) { + new DesiredBalanceComputer(clusterSettings, TimeProviderUtils.create(() -> 1L), shardsAllocator, TEST_ONLY_EXPLAINER) { @Override public DesiredBalance compute( DesiredBalance previousDesiredBalance, diff --git a/test/framework/src/main/java/org/elasticsearch/cluster/ESAllocationTestCase.java b/test/framework/src/main/java/org/elasticsearch/cluster/ESAllocationTestCase.java index 664b4484221be..9ebc25fef9a8f 100644 --- a/test/framework/src/main/java/org/elasticsearch/cluster/ESAllocationTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/cluster/ESAllocationTestCase.java @@ -99,7 +99,7 @@ public OptionalDouble getForecastedWriteLoad(IndexMetadata indexMetadata) { public void refreshLicense() {} }; - public static final DesiredBalanceShardsAllocator.ShardAllocationExplainer DUMMY_EXPLAINER = ( + public static final DesiredBalanceShardsAllocator.ShardAllocationExplainer TEST_ONLY_EXPLAINER = ( shard, allocation) -> ShardAllocationDecision.NOT_TAKEN; @@ -182,7 +182,7 @@ private static DesiredBalanceShardsAllocator createDesiredBalanceShardsAllocator null, TelemetryProvider.NOOP, EMPTY_NODE_ALLOCATION_STATS, - DUMMY_EXPLAINER + TEST_ONLY_EXPLAINER ) { private RoutingAllocation lastAllocation; From 9a4e07fd9bc3610b5057022be49456384be8f66a Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Wed, 3 Sep 2025 13:09:04 +1000 Subject: [PATCH 4/5] Track one unassigned shard for logging --- .../allocator/DesiredBalanceComputer.java | 46 +++++++----- .../DesiredBalanceShardsAllocator.java | 9 --- .../common/settings/ClusterSettings.java | 2 - .../DesiredBalanceComputerTests.java | 71 +++++++++++-------- 4 files changed, 70 insertions(+), 58 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputer.java b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputer.java index fe03b4a5a3d6b..5f2e0e6f5fa0c 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputer.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputer.java @@ -21,7 +21,6 @@ import org.elasticsearch.cluster.routing.allocation.allocator.DesiredBalanceShardsAllocator.ShardAllocationExplainer; import org.elasticsearch.cluster.routing.allocation.command.MoveAllocationCommand; import org.elasticsearch.cluster.routing.allocation.decider.Decision; -import org.elasticsearch.common.FrequencyCappedAction; import org.elasticsearch.common.metrics.MeanMetric; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Setting; @@ -86,7 +85,7 @@ public class DesiredBalanceComputer { private long lastConvergedTimeMillis; private long lastNotConvergedLogMessageTimeMillis; private Level convergenceLogMsgLevel; - private final FrequencyCappedAction logAllocationExplainForUnassigned; + private ShardRouting lastTrackedUnassignedShard; public DesiredBalanceComputer( ClusterSettings clusterSettings, @@ -99,14 +98,9 @@ public DesiredBalanceComputer( this.shardAllocationExplainer = shardAllocationExplainer; this.numComputeCallsSinceLastConverged = 0; this.numIterationsSinceLastConverged = 0; - this.logAllocationExplainForUnassigned = new FrequencyCappedAction(timeProvider::relativeTimeInMillis, TimeValue.ZERO); this.lastConvergedTimeMillis = timeProvider.relativeTimeInMillis(); this.lastNotConvergedLogMessageTimeMillis = lastConvergedTimeMillis; this.convergenceLogMsgLevel = Level.DEBUG; - clusterSettings.initializeAndWatch( - DesiredBalanceShardsAllocator.ALLOCATION_EXPLAIN_LOGGING_INTERVAL, - logAllocationExplainForUnassigned::setMinInterval - ); clusterSettings.initializeAndWatch(PROGRESS_LOG_INTERVAL_SETTING, value -> this.progressLogInterval = value); clusterSettings.initializeAndWatch( MAX_BALANCE_COMPUTATION_TIME_DURING_INDEX_CREATION_SETTING, @@ -494,31 +488,45 @@ private void maybeLogAllocationExplainForUnassigned( RoutingNodes routingNodes, RoutingAllocation routingAllocation ) { - if (allocationExplainLogger.isDebugEnabled() - && finishReason == DesiredBalance.ComputationFinishReason.CONVERGED - && routingNodes.hasUnassignedShards()) { - logAllocationExplainForUnassigned.maybeExecute(() -> { - final var unassigned = routingNodes.unassigned(); - final var shardRouting = Stream.concat(unassigned.stream(), unassigned.ignored().stream()) - .filter(ShardRouting::primary) - .findFirst() - .orElse(Stream.concat(unassigned.stream(), unassigned.ignored().stream()).iterator().next()); + if (allocationExplainLogger.isDebugEnabled()) { + if (lastTrackedUnassignedShard != null) { + if (Stream.concat(routingNodes.unassigned().stream(), routingNodes.unassigned().ignored().stream()) + .noneMatch(shardRouting -> shardRouting.equals(lastTrackedUnassignedShard))) { + allocationExplainLogger.debug("previously tracked unassigned shard [{}] is now assigned", lastTrackedUnassignedShard); + lastTrackedUnassignedShard = null; + } else { + return; // The last tracked unassigned shard is still unassigned, keep tracking it + } + } + + assert lastTrackedUnassignedShard == null : "unexpected non-null lastTrackedUnassignedShard " + lastTrackedUnassignedShard; + if (routingNodes.hasUnassignedShards() && finishReason == DesiredBalance.ComputationFinishReason.CONVERGED) { + final Predicate predicate = routingNodes.hasUnassignedPrimaries() ? ShardRouting::primary : shard -> true; + lastTrackedUnassignedShard = Stream.concat( + routingNodes.unassigned().stream(), + routingNodes.unassigned().ignored().stream().filter(predicate) + ).findFirst().orElseThrow(); + final var originalDebugMode = routingAllocation.getDebugMode(); routingAllocation.setDebugMode(RoutingAllocation.DebugMode.EXCLUDE_YES_DECISIONS); final ShardAllocationDecision shardAllocationDecision; try { - shardAllocationDecision = shardAllocationExplainer.explain(shardRouting, routingAllocation); + shardAllocationDecision = shardAllocationExplainer.explain(lastTrackedUnassignedShard, routingAllocation); } finally { routingAllocation.setDebugMode(originalDebugMode); } allocationExplainLogger.debug( "unassigned shard [{}] with allocation decision {}", - shardRouting, + lastTrackedUnassignedShard, org.elasticsearch.common.Strings.toString( p -> ChunkedToXContentHelper.object("node_allocation_decision", shardAllocationDecision.toXContentChunked(p)) ) ); - }); + } + } else { + if (lastTrackedUnassignedShard != null) { + lastTrackedUnassignedShard = null; + } } } diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceShardsAllocator.java b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceShardsAllocator.java index 6f833f1b89664..bcc9b3c58eabf 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceShardsAllocator.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceShardsAllocator.java @@ -35,8 +35,6 @@ import org.elasticsearch.common.metrics.CounterMetric; import org.elasticsearch.common.metrics.MeanMetric; import org.elasticsearch.common.settings.ClusterSettings; -import org.elasticsearch.common.settings.Setting; -import org.elasticsearch.core.TimeValue; import org.elasticsearch.telemetry.TelemetryProvider; import org.elasticsearch.threadpool.ThreadPool; @@ -59,13 +57,6 @@ public class DesiredBalanceShardsAllocator implements ShardsAllocator { private static final Logger logger = LogManager.getLogger(DesiredBalanceShardsAllocator.class); - public static final Setting ALLOCATION_EXPLAIN_LOGGING_INTERVAL = Setting.timeSetting( - "cluster.routing.allocation.desired_balance.allocation_explain_log_interval", - TimeValue.timeValueMinutes(1), - Setting.Property.Dynamic, - Setting.Property.NodeScope - ); - private final ShardsAllocator delegateAllocator; private final ThreadPool threadPool; /** diff --git a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java index 95e26f623d1a8..9c2d6fab10368 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java @@ -51,7 +51,6 @@ import org.elasticsearch.cluster.routing.allocation.allocator.BalancedShardsAllocator; import org.elasticsearch.cluster.routing.allocation.allocator.DesiredBalanceComputer; import org.elasticsearch.cluster.routing.allocation.allocator.DesiredBalanceReconciler; -import org.elasticsearch.cluster.routing.allocation.allocator.DesiredBalanceShardsAllocator; import org.elasticsearch.cluster.routing.allocation.decider.AwarenessAllocationDecider; import org.elasticsearch.cluster.routing.allocation.decider.ClusterRebalanceAllocationDecider; import org.elasticsearch.cluster.routing.allocation.decider.ConcurrentRebalanceAllocationDecider; @@ -230,7 +229,6 @@ public void apply(Settings value, Settings current, Settings previous) { DataStreamAutoShardingService.CLUSTER_AUTO_SHARDING_MIN_WRITE_THREADS, DataStreamAutoShardingService.DATA_STREAMS_AUTO_SHARDING_INCREASE_SHARDS_LOAD_METRIC, DataStreamAutoShardingService.DATA_STREAMS_AUTO_SHARDING_DECREASE_SHARDS_LOAD_METRIC, - DesiredBalanceShardsAllocator.ALLOCATION_EXPLAIN_LOGGING_INTERVAL, DesiredBalanceComputer.PROGRESS_LOG_INTERVAL_SETTING, DesiredBalanceComputer.MAX_BALANCE_COMPUTATION_TIME_DURING_INDEX_CREATION_SETTING, DesiredBalanceReconciler.UNDESIRED_ALLOCATIONS_LOG_INTERVAL_SETTING, diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputerTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputerTests.java index 966d5207cd4b8..99a419a66f3be 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputerTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputerTests.java @@ -1451,25 +1451,16 @@ record LogExpectationData( reason = "test logging for allocation explain" ) public void testLogAllocationExplainForUnassigned() { - final var timeInMillis = new AtomicLong(0L); - final long logIntervalInSeconds = between(60, 120); - final ClusterSettings clusterSettings = createBuiltInClusterSettings( - Settings.builder() - .put( - DesiredBalanceShardsAllocator.ALLOCATION_EXPLAIN_LOGGING_INTERVAL.getKey(), - TimeValue.timeValueSeconds(logIntervalInSeconds) - ) - .build() - ); + final ClusterSettings clusterSettings = createBuiltInClusterSettings(); final var computer = new DesiredBalanceComputer( clusterSettings, - TimeProviderUtils.create(timeInMillis::incrementAndGet), + TimeProviderUtils.create(() -> 0L), new BalancedShardsAllocator(Settings.EMPTY), createAllocationService()::explainShardAllocation ); final String loggerName = DesiredBalanceComputer.class.getCanonicalName() + ".allocation_explain"; - // No logging since no unassigned shards + // No logging since no unassigned shard { final var allocation = new RoutingAllocation( randomAllocationDeciders(Settings.EMPTY, clusterSettings), @@ -1481,7 +1472,7 @@ public void testLogAllocationExplainForUnassigned() { try (var mockLog = MockLog.capture(loggerName)) { mockLog.addExpectation( new MockLog.UnseenEventExpectation( - "Should log allocation explain for unassigned primary shard", + "Should NOT log allocation explain since all shards are assigned", loggerName, Level.DEBUG, "unassigned shard * with allocation decision *" @@ -1506,7 +1497,7 @@ public void testLogAllocationExplainForUnassigned() { .setNodeId(dataNode.getId()) .setType(SingleNodeShutdownMetadata.Type.REMOVE) .setReason("test") - .setStartedAtMillis(timeInMillis.get()) + .setStartedAtMillis(0L) .build() ) ) @@ -1519,6 +1510,7 @@ public void testLogAllocationExplainForUnassigned() { SnapshotShardSizeInfo.EMPTY, 0L ); + final DesiredBalance newDesiredBalance; try (var mockLog = MockLog.capture(loggerName)) { mockLog.addExpectation( new MockLog.SeenEventExpectation( @@ -1529,7 +1521,27 @@ public void testLogAllocationExplainForUnassigned() { + "\"decider\":\"node_shutdown\",\"decision\":\"NO\"*" ) ); - computer.compute(DesiredBalance.BECOME_MASTER_INITIAL, DesiredBalanceInput.create(1, allocation), queue(), ignore -> true); + newDesiredBalance = computer.compute( + DesiredBalance.BECOME_MASTER_INITIAL, + DesiredBalanceInput.create(1, allocation), + queue(), + ignore -> true + ); + mockLog.assertAllExpectationsMatched(); + } + + // No logging since the same tracked primary is still unassigned + try (var mockLog = MockLog.capture(loggerName)) { + mockLog.addExpectation( + new MockLog.UnseenEventExpectation( + "Should NOT log allocation explain again for existing tracked unassigned shard", + loggerName, + Level.DEBUG, + "unassigned shard [[test-index][0], node[null], [P], * with allocation decision *" + + "\"decider\":\"node_shutdown\",\"decision\":\"NO\"*" + ) + ); + computer.compute(newDesiredBalance, DesiredBalanceInput.create(2, allocation), queue(), ignore -> true); mockLog.assertAllExpectationsMatched(); } } @@ -1544,20 +1556,23 @@ public void testLogAllocationExplainForUnassigned() { 0L ); try (var mockLog = MockLog.capture(loggerName)) { - final var expectation = new MockLog.EventuallySeenEventExpectation( - "Should log allocation explain for unassigned replica shard", - loggerName, - Level.DEBUG, - "unassigned shard [[test-index][0], node[null], [R], * with allocation decision *" - + "\"decider\":\"same_shard\",\"decision\":\"NO\"*" + mockLog.addExpectation( + new MockLog.SeenEventExpectation( + "Should log for previously unassigned shard becomes assigned", + loggerName, + Level.DEBUG, + "previously tracked unassigned shard [[test-index][0], node[null], [P],* is now assigned" + ) + ); + mockLog.addExpectation( + new MockLog.SeenEventExpectation( + "Should log allocation explain for unassigned replica shard", + loggerName, + Level.DEBUG, + "unassigned shard [[test-index][0], node[null], [R], * with allocation decision *" + + "\"decider\":\"same_shard\",\"decision\":\"NO\"*" + ) ); - mockLog.addExpectation(expectation); - computer.compute(DesiredBalance.BECOME_MASTER_INITIAL, DesiredBalanceInput.create(1, allocation), queue(), ignore -> true); - mockLog.assertAllExpectationsMatched(); - - // Simulate time passing - timeInMillis.addAndGet(logIntervalInSeconds * 1000); - expectation.setExpectSeen(); computer.compute(DesiredBalance.BECOME_MASTER_INITIAL, DesiredBalanceInput.create(1, allocation), queue(), ignore -> true); mockLog.assertAllExpectationsMatched(); } From 56dfacba4d973bd825df6f7b24a516b307b8fff7 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Thu, 4 Sep 2025 09:53:42 +1000 Subject: [PATCH 5/5] tweak --- .../allocation/allocator/DesiredBalanceComputer.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputer.java b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputer.java index 5f2e0e6f5fa0c..33a592ef05524 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputer.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputer.java @@ -502,10 +502,10 @@ private void maybeLogAllocationExplainForUnassigned( assert lastTrackedUnassignedShard == null : "unexpected non-null lastTrackedUnassignedShard " + lastTrackedUnassignedShard; if (routingNodes.hasUnassignedShards() && finishReason == DesiredBalance.ComputationFinishReason.CONVERGED) { final Predicate predicate = routingNodes.hasUnassignedPrimaries() ? ShardRouting::primary : shard -> true; - lastTrackedUnassignedShard = Stream.concat( - routingNodes.unassigned().stream(), - routingNodes.unassigned().ignored().stream().filter(predicate) - ).findFirst().orElseThrow(); + lastTrackedUnassignedShard = Stream.concat(routingNodes.unassigned().stream(), routingNodes.unassigned().ignored().stream()) + .filter(predicate) + .findFirst() + .orElseThrow(); final var originalDebugMode = routingAllocation.getDebugMode(); routingAllocation.setDebugMode(RoutingAllocation.DebugMode.EXCLUDE_YES_DECISIONS);