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 index 5139dbaac38d7..5d5f317348001 100644 --- 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 @@ -10,10 +10,8 @@ package org.elasticsearch.cluster.routing.allocation.decider; import org.apache.logging.log4j.Level; -import org.elasticsearch.action.admin.cluster.allocation.ClusterAllocationExplainRequest; import org.elasticsearch.action.admin.cluster.allocation.DesiredBalanceRequest; import org.elasticsearch.action.admin.cluster.allocation.DesiredBalanceResponse; -import org.elasticsearch.action.admin.cluster.allocation.TransportClusterAllocationExplainAction; import org.elasticsearch.action.admin.cluster.allocation.TransportGetDesiredBalanceAction; import org.elasticsearch.action.admin.cluster.node.usage.NodeUsageStatsForThreadPoolsAction; import org.elasticsearch.action.admin.cluster.node.usage.TransportNodeUsageStatsForThreadPoolsAction; @@ -29,14 +27,11 @@ import org.elasticsearch.cluster.routing.RoutingNodes; import org.elasticsearch.cluster.routing.ShardRouting; import org.elasticsearch.cluster.routing.UnassignedInfo; -import org.elasticsearch.cluster.routing.allocation.AllocationDecision; -import org.elasticsearch.cluster.routing.allocation.Explanations; import org.elasticsearch.cluster.routing.allocation.WriteLoadConstraintSettings; import org.elasticsearch.cluster.routing.allocation.allocator.BalancedShardsAllocator; import org.elasticsearch.cluster.routing.allocation.allocator.DesiredBalanceMetrics; import org.elasticsearch.cluster.routing.allocation.allocator.DesiredBalanceShardsAllocator; import org.elasticsearch.cluster.service.ClusterService; -import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.CollectionUtils; import org.elasticsearch.core.TimeValue; @@ -269,55 +264,13 @@ public void testShardsAreAssignedToNotPreferredWhenAlternativeIsNo() { } } - @TestLogging( - reason = "track when reconciliation has completed", - value = "org.elasticsearch.cluster.routing.allocation.allocator.DesiredBalanceShardsAllocator:DEBUG" - ) - public void testAllocationExplainMoveShardNotPreferred() { - TestHarness harness = setUpThreeTestNodesAndAllIndexShardsOnFirstNode(); - - // Running the {@link #runCanRemainNotPreferredIsIgnoredWhenAllOtherNodesReturnNotPreferred} logic should set up a cluster where - // shards are all allocated to a {@link AllocationDeciders#canRemain} {@link Decision#NOT_PREFERRED} node, while the other nodes - // return {@link AllocationDeciders#canAllocate} {@link Decision#NOT_PREFERRED} responses. This should exercise NOT_PREFERRED in - // the allocation/explain paths for remaining on a node AND assignment to other nodes. - runCanRemainNotPreferredIsIgnoredWhenAllOtherNodesReturnNotPreferred(harness); - int numDataNodes = internalCluster().numDataNodes(); - assertThat( - "test requires at least two nodes, one node for canRemain explanation, one for canAllocation explanation", - numDataNodes, - greaterThanOrEqualTo(2) - ); - - ClusterAllocationExplainRequest allocationExplainRequest = new ClusterAllocationExplainRequest(TEST_REQUEST_TIMEOUT).setIndex( - harness.indexName - ).setShard(0).setPrimary(true); - var allocationExplainResponse = safeGet(client().execute(TransportClusterAllocationExplainAction.TYPE, allocationExplainRequest)); - logger.info("---> Allocation explain response: " + Strings.toString(allocationExplainResponse.getExplanation(), true, true)); - - var decision = allocationExplainResponse.getExplanation().getShardAllocationDecision().getMoveDecision(); - assertThat("Rebalancing should be disabled", decision.canRebalanceCluster(), equalTo(false)); - assertThat(decision.getCanRemainDecision().type(), equalTo(Decision.NOT_PREFERRED.type())); - assertNull(decision.getTargetNode()); - assertThat(decision.getAllocationDecision(), equalTo(AllocationDecision.NOT_PREFERRED)); - - var canAllocateDecisions = allocationExplainResponse.getExplanation() - .getShardAllocationDecision() - .getMoveDecision() - .getNodeDecisions(); - assertThat(canAllocateDecisions.size(), equalTo(/* number of nodes to which the shard can be relocated = */ numDataNodes - 1)); - canAllocateDecisions.forEach(nodeDecision -> assertThat(nodeDecision.getNodeDecision(), equalTo(AllocationDecision.NOT_PREFERRED))); - } - @TestLogging( reason = "track when reconciliation has completed", value = "org.elasticsearch.cluster.routing.allocation.allocator.DesiredBalanceShardsAllocator:DEBUG" ) public void testCanRemainNotPreferredIsIgnoredWhenAllOtherNodesReturnNotPreferred() { TestHarness harness = setUpThreeTestNodesAndAllIndexShardsOnFirstNode(); - runCanRemainNotPreferredIsIgnoredWhenAllOtherNodesReturnNotPreferred(harness); - } - private void runCanRemainNotPreferredIsIgnoredWhenAllOtherNodesReturnNotPreferred(TestHarness harness) { /** * Override the {@link TransportNodeUsageStatsForThreadPoolsAction} action on the data nodes to supply artificial thread pool write * load stats. The stats will show all the nodes above the high utilization threshold, so they do not accept new shards, while the @@ -502,80 +455,6 @@ public void testCanRemainRelocatesOneShardWhenAHotSpotOccurs() { } } - public void testAllocationExplainRebalancingNotPreferred() { - var harness = setUpThreeTestNodesAndAllIndexShardsOnFirstNode(); - - // Set up all data nodes to report write thread pool usage above the utilization threshold, to stop canAllocate, but no write thread - // pool queuing to force a shard relocation. This will leave all the data nodes unable to accept new shards when rebalancing - // attempts to redistribute shards more evenly than all shards on a single node. - - final NodeUsageStatsForThreadPools firstNodeAboveUtilizationThresholdNodeStats = createNodeUsageStatsForThreadPools( - harness.firstDiscoveryNode, - harness.randomNumberOfWritePoolThreads, - randomIntBetween(harness.randomUtilizationThresholdPercent, 100) / 100f, - 0 - ); - final NodeUsageStatsForThreadPools secondNodeAboveUtilizationThresholdNodeStats = createNodeUsageStatsForThreadPools( - harness.secondDiscoveryNode, - harness.randomNumberOfWritePoolThreads, - randomIntBetween(harness.randomUtilizationThresholdPercent, 100) / 100f, - 0 - ); - final NodeUsageStatsForThreadPools thirdNodeAboveUtilizationThresholdNodeStats = createNodeUsageStatsForThreadPools( - harness.thirdDiscoveryNode, - harness.randomNumberOfWritePoolThreads, - randomIntBetween(harness.randomUtilizationThresholdPercent, 100) / 100f, - 0 - ); - setUpMockTransportNodeUsageStatsResponse(harness.firstDiscoveryNode, firstNodeAboveUtilizationThresholdNodeStats); - setUpMockTransportNodeUsageStatsResponse(harness.secondDiscoveryNode, secondNodeAboveUtilizationThresholdNodeStats); - setUpMockTransportNodeUsageStatsResponse(harness.thirdDiscoveryNode, thirdNodeAboveUtilizationThresholdNodeStats); - - // Override the {@link TransportIndicesStatsAction} action on the data nodes to supply artificial shard write load stats. The stats - // will show that all shards have non-empty write load stats (so that the WriteLoadDecider will evaluate assigning them to a node). - final ClusterState originalClusterState = internalCluster().getCurrentMasterNodeInstance(ClusterService.class).state(); - final IndexMetadata indexMetadata = originalClusterState.getMetadata().getProject().index(harness.indexName); - setUpMockTransportIndicesStatsResponse( - harness.firstDiscoveryNode, - indexMetadata.getNumberOfShards(), - createShardStatsResponseForIndex(indexMetadata, harness.maxShardWriteLoad, harness.firstDataNodeId) - ); - setUpMockTransportIndicesStatsResponse(harness.secondDiscoveryNode, 0, List.of()); - setUpMockTransportIndicesStatsResponse(harness.thirdDiscoveryNode, 0, List.of()); - - logger.info("---> Refreshing the cluster info to pull in the dummy thread pool stats from the data nodes"); - refreshClusterInfo(); - - // Allow rebalancing and clear the exclusion setting that holds the shards on a single node. - updateClusterSettings( - Settings.builder() - .put(EnableAllocationDecider.CLUSTER_ROUTING_REBALANCE_ENABLE_SETTING.getKey(), EnableAllocationDecider.Rebalance.ALL) - .putNull("cluster.routing.allocation.exclude._name") - ); - - logger.info("---> Rebalancing is now allowed"); - - ClusterAllocationExplainRequest allocationExplainRequest = new ClusterAllocationExplainRequest(TEST_REQUEST_TIMEOUT).setIndex( - harness.indexName - ).setShard(0).setPrimary(true); - var allocationExplainResponse = safeGet(client().execute(TransportClusterAllocationExplainAction.TYPE, allocationExplainRequest)); - logger.info("---> Allocation explain response: " + Strings.toString(allocationExplainResponse.getExplanation(), true, true)); - - var decision = allocationExplainResponse.getExplanation().getShardAllocationDecision().getMoveDecision(); - assertThat("Rebalancing should be enabled", decision.canRebalanceCluster(), equalTo(true)); - assertThat(decision.getCanRemainDecision().type(), equalTo(Decision.YES.type())); - assertNull(decision.getTargetNode()); - assertThat(decision.getAllocationDecision(), equalTo(AllocationDecision.NOT_PREFERRED)); - assertThat(decision.getExplanation(), equalTo(Explanations.Rebalance.NOT_PREFERRED)); - - var canAllocateDecisions = allocationExplainResponse.getExplanation() - .getShardAllocationDecision() - .getMoveDecision() - .getNodeDecisions(); - assertThat(canAllocateDecisions.size(), equalTo(2)); - canAllocateDecisions.forEach(nodeDecision -> assertThat(nodeDecision.getNodeDecision(), equalTo(AllocationDecision.NOT_PREFERRED))); - } - /** * Determine which shard was moved and check that it's the "best" according to * {@link org.elasticsearch.cluster.routing.allocation.allocator.BalancedShardsAllocator.Balancer.PrioritiseByShardWriteLoadComparator} diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/AllocationDecision.java b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/AllocationDecision.java index 1e6123ea6a9e2..bf6c60de47a49 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/AllocationDecision.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/AllocationDecision.java @@ -154,8 +154,8 @@ public static AllocationDecision fromAllocationStatus(AllocationStatus allocatio */ public static AllocationDecision fromDecisionType(Decision.Type type) { return switch (type) { - case YES -> YES; - case NOT_PREFERRED -> NOT_PREFERRED; + // TODO: this + case YES, NOT_PREFERRED -> YES; case THROTTLE -> THROTTLED; case NO -> NO; }; diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/MoveDecision.java b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/MoveDecision.java index fa0a282576982..d84df3fad8f57 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/MoveDecision.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/MoveDecision.java @@ -118,34 +118,15 @@ public static MoveDecision move( ) { assert canRemainDecision != null; assert canRemainDecision.type() != Type.YES : "create decision with MoveDecision#createRemainYesDecision instead"; - assert decisionAndTargetAreConsistent(canRemainDecision, moveDecision, targetNode) - : "targetNode: " + targetNode + ", move decision: " + moveDecision; if (nodeDecisions == null && moveDecision == AllocationDecision.NO) { // the final decision is NO (no node to move the shard to) and we are not in explain mode, return a cached version return CACHED_CANNOT_MOVE_DECISION; } else { + assert ((targetNode == null) == (moveDecision != AllocationDecision.YES)); return new MoveDecision(targetNode, nodeDecisions, moveDecision, canRemainDecision, null, 0); } } - private static boolean decisionAndTargetAreConsistent( - Decision canRemainDecision, - AllocationDecision moveDecision, - DiscoveryNode targetNode - ) { - return switch (moveDecision) { - // YES must always have a target - case AllocationDecision.YES -> (targetNode != null); - case AllocationDecision.NOT_PREFERRED -> { - // If canRemain is not-preferred, then there should be no shard move, and thus no target node. - assert (canRemainDecision.type() == Type.NOT_PREFERRED) == (targetNode == null) - : "remain decision: " + canRemainDecision + ", target node: " + targetNode; - yield true; - } - default -> targetNode == null; - }; - } - /** * Creates a decision for whether to move the shard to a different node to form a better cluster balance. */ @@ -282,29 +263,23 @@ public String getExplanation() { ? Explanations.Rebalance.CANNOT_REBALANCE_CAN_ALLOCATE : Explanations.Rebalance.CANNOT_REBALANCE_CANNOT_ALLOCATE; case THROTTLE -> Explanations.Rebalance.CLUSTER_THROTTLE; - case YES -> { + case YES, NOT_PREFERRED -> { if (getTargetNode() != null) { yield canMoveDecision == AllocationDecision.THROTTLED ? Explanations.Rebalance.NODE_THROTTLE : Explanations.Rebalance.YES; } else { - yield canMoveDecision == AllocationDecision.NOT_PREFERRED - ? Explanations.Rebalance.NOT_PREFERRED - : Explanations.Rebalance.ALREADY_BALANCED; + yield Explanations.Rebalance.ALREADY_BALANCED; } } - case NOT_PREFERRED -> Explanations.Rebalance.NOT_PREFERRED; }; } else { // it was a decision by an allocation decider to move the shard assert cannotRemain(); return switch (canMoveDecision) { - case YES -> canRemainNotPreferred() ? Explanations.Move.NOT_PREFERRED_TO_YES : Explanations.Move.YES; - case NOT_PREFERRED -> canRemainNotPreferred() - ? Explanations.Move.NOT_PREFERRED_TO_NOT_PREFERRED - : Explanations.Move.NOT_PREFERRED; - case THROTTLED -> canRemainNotPreferred() ? Explanations.Move.NOT_PREFERRED_TO_THROTTLED : Explanations.Move.THROTTLED; - case NO -> canRemainNotPreferred() ? Explanations.Move.NOT_PREFERRED_TO_NO : Explanations.Move.NO; + case YES, NOT_PREFERRED -> Explanations.Move.YES; + case THROTTLED -> Explanations.Move.THROTTLED; + case NO -> Explanations.Move.NO; case WORSE_BALANCE, AWAITING_INFO, ALLOCATION_DELAYED, NO_VALID_SHARD_COPY, NO_ATTEMPT -> { assert false : canMoveDecision; yield canMoveDecision.toString(); @@ -341,13 +316,7 @@ public Iterator toXContentChunked(ToXContent.Params params builder.field("can_rebalance_to_other_node", canMoveDecision); builder.field("rebalance_explanation", getExplanation()); } else { - if (cannotRemainAndCanMove()) { - builder.field("can_move_to_other_node", "yes"); - } else if (cannotRemainAndNotPreferredMove()) { - builder.field("can_move_to_other_node", "not-preferred"); - } else { - builder.field("can_move_to_other_node", "no"); - } + builder.field("can_move_to_other_node", cannotRemainAndCanMove() ? "yes" : "no"); builder.field("move_explanation", getExplanation()); } return builder; diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java index 75846f6bbe849..efe0ea616d69b 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java @@ -531,15 +531,10 @@ private MoveDecision explainRebalanceDecision(final ProjectIndex index, final Sh // with the shard remaining on the current node, and we are allowed to allocate to the // node in question, then allow the rebalance if (rebalanceConditionsMet && canAllocate.type().higherThan(bestRebalanceCanAllocateDecisionType)) { - // Overwrite the best decision since it is better than the last. This means that YES/THROTTLE decisions will replace - // NOT_PREFERRED/NO decisions, and a YES decision will replace a THROTTLE decision. NOT_PREFERRED will also replace - // NO, even if neither are acted upon for rebalancing, for allocation explain purposes. + // rebalance to the node, only will get overwritten if the decision here is to + // THROTTLE and we get a decision with YES on another node bestRebalanceCanAllocateDecisionType = canAllocate.type(); - if (canAllocate.type().higherThan(Type.NOT_PREFERRED)) { - // Movement is only allowed to THROTTLE/YES nodes. NOT_PREFERRED is the same as no for rebalancing, since - // rebalancing aims to distribute resource usage and NOT_PREFERRED means the move could cause hot-spots. - targetNode = node; - } + targetNode = node; } } Tuple nodeResult = Tuple.tuple(node, canAllocate); @@ -1047,12 +1042,10 @@ private MoveDecision decideMove( nodeResults.add(new NodeAllocationResult(currentNode.getRoutingNode().node(), allocationDecision, ++weightRanking)); } if (allocationDecision.type() == Type.NOT_PREFERRED && remainDecision.type() == Type.NOT_PREFERRED) { - // Whether or not a relocation target node can be found, it's important to explain the canAllocate response as - // NOT_PREFERRED, as opposed to NO. - bestDecision = Type.NOT_PREFERRED; - // Relocating a shard from one NOT_PREFERRED node to another NOT_PREFERRED node would not improve the situation. + // Relocating a shard from one NOT_PREFERRED node to another would not improve the situation. continue; } + if (allocationDecision.type().higherThan(bestDecision)) { bestDecision = allocationDecision.type(); if (bestDecision == Type.YES) { @@ -1064,12 +1057,8 @@ private MoveDecision decideMove( } } else if (bestDecision == Type.NOT_PREFERRED) { assert remainDecision.type() != Type.NOT_PREFERRED; - // If we don't ever find a YES/THROTTLE decision, we'll settle for NOT_PREFERRED as preferable to NO. + // If we don't ever find a YES decision, we'll settle for NOT_PREFERRED as preferable to NO. targetNode = target; - } else if (bestDecision == Type.THROTTLE) { - assert allocation.isSimulating() == false; - // THROTTLE is better than NOT_PREFERRED, we just need to wait for a YES. - targetNode = null; } } } diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/Decision.java b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/Decision.java index e0682b507cbcb..abc26da5dd6fd 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/Decision.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/Decision.java @@ -118,10 +118,9 @@ private static Single readSingleFrom(StreamInput in) throws IOException { enum Type implements Writeable { // ordered by positiveness; order matters for serialization and comparison NO, - NOT_PREFERRED, - // Temporarily throttled is a better choice than choosing a not-preferred node, - // but NOT_PREFERRED and THROTTLED are generally not comparable. + // NOT_PREFERRED and THROTTLED should not be compared. THROTTLE, + NOT_PREFERRED, YES; private static final TransportVersion ALLOCATION_DECISION_NOT_PREFERRED = TransportVersion.fromName( @@ -162,6 +161,7 @@ public void writeTo(StreamOutput out) throws IOException { } } + // TODO: Remove this method, causes too much headache. THROTTLE and NOT_PREFERRED should not be compared. public boolean higherThan(Type other) { return this.compareTo(other) > 0; } diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/allocation/ClusterAllocationExplainActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/allocation/ClusterAllocationExplainActionTests.java index e392cd0de525a..c644a886dac9c 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/allocation/ClusterAllocationExplainActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/allocation/ClusterAllocationExplainActionTests.java @@ -10,11 +10,8 @@ package org.elasticsearch.action.admin.cluster.allocation; import org.elasticsearch.action.support.replication.ClusterStateCreationUtils; -import org.elasticsearch.cluster.ClusterInfo; import org.elasticsearch.cluster.ClusterState; -import org.elasticsearch.cluster.EmptyClusterInfoService; import org.elasticsearch.cluster.TestShardRoutingRoleStrategies; -import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.metadata.ProjectId; import org.elasticsearch.cluster.routing.IndexRoutingTable; import org.elasticsearch.cluster.routing.IndexShardRoutingTable; @@ -25,19 +22,13 @@ import org.elasticsearch.cluster.routing.UnassignedInfo; import org.elasticsearch.cluster.routing.allocation.AllocationDecision; import org.elasticsearch.cluster.routing.allocation.AllocationService; -import org.elasticsearch.cluster.routing.allocation.Explanations; import org.elasticsearch.cluster.routing.allocation.RoutingAllocation; import org.elasticsearch.cluster.routing.allocation.ShardAllocationDecision; -import org.elasticsearch.cluster.routing.allocation.allocator.BalancedShardsAllocator; import org.elasticsearch.cluster.routing.allocation.allocator.ShardsAllocator; -import org.elasticsearch.cluster.routing.allocation.decider.AllocationDecider; import org.elasticsearch.cluster.routing.allocation.decider.AllocationDeciders; -import org.elasticsearch.cluster.routing.allocation.decider.Decision; import org.elasticsearch.common.Strings; -import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.ChunkedToXContent; import org.elasticsearch.common.xcontent.XContentHelper; -import org.elasticsearch.snapshots.EmptySnapshotsInfoService; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.gateway.TestGatewayAllocator; import org.elasticsearch.xcontent.ToXContent; @@ -46,7 +37,6 @@ import java.time.Instant; import java.util.Collections; -import java.util.List; import java.util.Locale; import java.util.Set; import java.util.stream.Collectors; @@ -55,7 +45,6 @@ import static org.hamcrest.Matchers.aMapWithSize; import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.equalTo; /** * Tests for the {@link TransportClusterAllocationExplainAction} class. @@ -64,284 +53,6 @@ public class ClusterAllocationExplainActionTests extends ESTestCase { private static final AllocationDeciders NOOP_DECIDERS = new AllocationDeciders(Collections.emptyList()); - private class CanAllocateNotPreferredAllocationDecider extends AllocationDecider { - @Override - public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, RoutingAllocation allocation) { - return Decision.single(Decision.Type.NOT_PREFERRED, "CanAllocateNotPreferredAllocationDecider", "Test not-preferred"); - } - - @Override - public Decision canRebalance(RoutingAllocation allocation) { - return Decision.NO; - } - } - - private class CanAllocateThrottledAllocationDecider extends AllocationDecider { - @Override - public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, RoutingAllocation allocation) { - return Decision.single(Decision.Type.THROTTLE, "CanAllocateThrottledAllocationDecider", "Test throttle"); - } - - @Override - public Decision canRebalance(RoutingAllocation allocation) { - return Decision.NO; - } - } - - private class CanRemainNotPreferredAllocationDecider extends AllocationDecider { - @Override - public Decision canRemain(IndexMetadata indexMetadata, ShardRouting shardRouting, RoutingNode node, RoutingAllocation allocation) { - return Decision.single(Decision.Type.NOT_PREFERRED, "CanRemainNotPreferredAllocationDecider", "Test not-preferred"); - } - - @Override - public Decision canRebalance(RoutingAllocation allocation) { - return Decision.NO; - } - } - - private class CanRemainNoAllocationDecider extends AllocationDecider { - @Override - public Decision canRemain(IndexMetadata indexMetadata, ShardRouting shardRouting, RoutingNode node, RoutingAllocation allocation) { - return Decision.single(Decision.Type.NO, "CanRemainNoAllocationDecider", "Test no"); - } - - @Override - public Decision canRebalance(RoutingAllocation allocation) { - return Decision.NO; - } - } - - public void testNoToNotPreferredShardAllocationExplanation() { - ClusterState clusterState = ClusterStateCreationUtils.state("idx", randomBoolean(), ShardRoutingState.STARTED); - logger.info("---> Cluster state: " + clusterState); - final ProjectId projectId = clusterState.metadata().projects().keySet().iterator().next(); - ShardRouting shardRouting = clusterState.globalRoutingTable().routingTable(projectId).index("idx").shard(0).primaryShard(); - ClusterInfo clusterInfo = ClusterInfo.builder().build(); - // Set up allocation deciders that will say: 1) shard cannot remain where it is and 2) shard allocation elsewhere is not-preferred. - // Relocation should proceed, with a target node for the MoveDecision. - AllocationDeciders allocationDeciders = new AllocationDeciders( - List.of(new CanRemainNoAllocationDecider(), new CanAllocateNotPreferredAllocationDecider()) - ); - RoutingAllocation allocation = new RoutingAllocation(allocationDeciders, clusterState, clusterInfo, null, System.nanoTime()); - - ClusterAllocationExplanation allocationExplanation = TransportClusterAllocationExplainAction.explainShard( - shardRouting, - allocation, - clusterInfo, - randomBoolean(), - true, - new AllocationService( - allocationDeciders, - new TestGatewayAllocator(), - new BalancedShardsAllocator(Settings.EMPTY), - EmptyClusterInfoService.INSTANCE, - EmptySnapshotsInfoService.INSTANCE, - TestShardRoutingRoleStrategies.DEFAULT_ROLE_ONLY - ) - ); - - logger.info("---> Allocation explain response: " + Strings.toString(allocationExplanation, true, true)); - // canRemain on the current node should be NO. - assertThat(allocationExplanation.getShardAllocationDecision().getMoveDecision().canRemain(), equalTo(false)); - assertThat( - allocationExplanation.getShardAllocationDecision().getMoveDecision().getCanRemainDecision().type(), - equalTo(Decision.Type.NO) - ); - assertThat( - "All other potential nodes should be not-preferred, resulting in an overall not-preferred relocation", - allocationExplanation.getShardAllocationDecision().getMoveDecision().getAllocationDecision(), - equalTo(AllocationDecision.NOT_PREFERRED) - ); - for (var nodeDecision : allocationExplanation.getShardAllocationDecision().getMoveDecision().getNodeDecisions()) { - assertThat( - "Each individual node should report not-preferred", - nodeDecision.getNodeDecision(), - equalTo(AllocationDecision.NOT_PREFERRED) - ); - } - assertThat( - "This is the explanation expected forcing a move from NO to NOT_PREFERRED, even though not ideal", - allocationExplanation.getShardAllocationDecision().getMoveDecision().getExplanation(), - equalTo(Explanations.Move.NOT_PREFERRED) - ); - assertNotNull( - "A relocation target node should have been selected: canRemain no overrides canAllocate not-preferred", - allocationExplanation.getShardAllocationDecision().getMoveDecision().getTargetNode() - ); - } - - public void testNotPreferredToNotPreferredShardAllocationExplanation() { - ClusterState clusterState = ClusterStateCreationUtils.state("idx", randomBoolean(), ShardRoutingState.STARTED); - logger.info("---> Cluster state: " + clusterState); - final ProjectId projectId = clusterState.metadata().projects().keySet().iterator().next(); - ShardRouting shardRouting = clusterState.globalRoutingTable().routingTable(projectId).index("idx").shard(0).primaryShard(); - ClusterInfo clusterInfo = ClusterInfo.builder().build(); - // Set up allocation deciders that will say: 1) shard not-preferred to remain where it is and 2) shard allocation elsewhere is - // not-preferred. Relocation should _not_ proceed, no target node for the MoveDecision. - AllocationDeciders allocationDeciders = new AllocationDeciders( - List.of(new CanRemainNotPreferredAllocationDecider(), new CanAllocateNotPreferredAllocationDecider()) - ); - RoutingAllocation allocation = new RoutingAllocation(allocationDeciders, clusterState, clusterInfo, null, System.nanoTime()); - - ClusterAllocationExplanation allocationExplanation = TransportClusterAllocationExplainAction.explainShard( - shardRouting, - allocation, - clusterInfo, - randomBoolean(), - true, - new AllocationService( - allocationDeciders, - new TestGatewayAllocator(), - new BalancedShardsAllocator(Settings.EMPTY), - EmptyClusterInfoService.INSTANCE, - EmptySnapshotsInfoService.INSTANCE, - TestShardRoutingRoleStrategies.DEFAULT_ROLE_ONLY - ) - ); - - logger.info("---> Allocation explain response: " + Strings.toString(allocationExplanation, true, true)); - // canRemain on the current node should be NOT_PREFERRED. - assertThat(allocationExplanation.getShardAllocationDecision().getMoveDecision().canRemainNotPreferred(), equalTo(true)); - assertThat( - allocationExplanation.getShardAllocationDecision().getMoveDecision().getCanRemainDecision().type(), - equalTo(Decision.Type.NOT_PREFERRED) - ); - assertThat( - "All other potential nodes should be not-preferred, resulting in an overall not-preferred relocation", - allocationExplanation.getShardAllocationDecision().getMoveDecision().getAllocationDecision(), - equalTo(AllocationDecision.NOT_PREFERRED) - ); - for (var nodeDecision : allocationExplanation.getShardAllocationDecision().getMoveDecision().getNodeDecisions()) { - assertThat( - "Each individual node should report not-preferred", - nodeDecision.getNodeDecision(), - equalTo(AllocationDecision.NOT_PREFERRED) - ); - } - assertThat( - "Expected the explanation for a move from NOT_PREFERRED to NOT_PREFERRED to say the shard won't move", - allocationExplanation.getShardAllocationDecision().getMoveDecision().getExplanation(), - equalTo(Explanations.Move.NOT_PREFERRED_TO_NOT_PREFERRED) - ); - assertNull( - "A relocation target node should not have been selected: canRemain not-preferred does not override canAllocate not-preferred", - allocationExplanation.getShardAllocationDecision().getMoveDecision().getTargetNode() - ); - } - - public void testNotPreferredToYesShardAllocationExplanation() { - ClusterState clusterState = ClusterStateCreationUtils.state("idx", randomBoolean(), ShardRoutingState.STARTED); - logger.info("---> Cluster state: " + clusterState); - final ProjectId projectId = clusterState.metadata().projects().keySet().iterator().next(); - ShardRouting shardRouting = clusterState.globalRoutingTable().routingTable(projectId).index("idx").shard(0).primaryShard(); - ClusterInfo clusterInfo = ClusterInfo.builder().build(); - // Set up allocation deciders that will say: 1) shard not-preferred to remain where it is and 2) shard can be allocated elsewhere - // (the default without a decider to say no). Relocation should proceed, there should be a target node for the MoveDecision. - AllocationDeciders allocationDeciders = new AllocationDeciders(List.of(new CanRemainNotPreferredAllocationDecider())); - RoutingAllocation allocation = new RoutingAllocation(allocationDeciders, clusterState, clusterInfo, null, System.nanoTime()); - - ClusterAllocationExplanation allocationExplanation = TransportClusterAllocationExplainAction.explainShard( - shardRouting, - allocation, - clusterInfo, - randomBoolean(), - true, - new AllocationService( - allocationDeciders, - new TestGatewayAllocator(), - new BalancedShardsAllocator(Settings.EMPTY), - EmptyClusterInfoService.INSTANCE, - EmptySnapshotsInfoService.INSTANCE, - TestShardRoutingRoleStrategies.DEFAULT_ROLE_ONLY - ) - ); - - logger.info("---> Allocation explain response: " + Strings.toString(allocationExplanation, true, true)); - // canRemain on the current node should be NOT_PREFERRED. - assertThat(allocationExplanation.getShardAllocationDecision().getMoveDecision().canRemainNotPreferred(), equalTo(true)); - assertThat( - allocationExplanation.getShardAllocationDecision().getMoveDecision().getCanRemainDecision().type(), - equalTo(Decision.Type.NOT_PREFERRED) - ); - assertThat( - "All other potential nodes should be YES, resulting in an overall YES move decision", - allocationExplanation.getShardAllocationDecision().getMoveDecision().getAllocationDecision(), - equalTo(AllocationDecision.YES) - ); - for (var nodeDecision : allocationExplanation.getShardAllocationDecision().getMoveDecision().getNodeDecisions()) { - assertThat("Each individual node should report YES", nodeDecision.getNodeDecision(), equalTo(AllocationDecision.YES)); - } - assertThat( - "Expected the explanation for a move from NOT_PREFERRED to YES to say the shard will move", - allocationExplanation.getShardAllocationDecision().getMoveDecision().getExplanation(), - equalTo(Explanations.Move.NOT_PREFERRED_TO_YES) - ); - assertNotNull( - "A relocation target node should have been selected going from a not-preferred remain node to a yes canAllocate node", - allocationExplanation.getShardAllocationDecision().getMoveDecision().getTargetNode() - ); - } - - public void testNotPreferredToThrottleShardAllocationExplanation() { - ClusterState clusterState = ClusterStateCreationUtils.state("idx", randomBoolean(), ShardRoutingState.STARTED); - logger.info("---> Cluster state: " + clusterState); - final ProjectId projectId = clusterState.metadata().projects().keySet().iterator().next(); - ShardRouting shardRouting = clusterState.globalRoutingTable().routingTable(projectId).index("idx").shard(0).primaryShard(); - ClusterInfo clusterInfo = ClusterInfo.builder().build(); - // Set up allocation deciders that will say: 1) shard not-preferred to remain where it is and 2) shard allocation elsewhere is - // throttled. Relocation should _not_ proceed, no target node for the MoveDecision. /// TODO DIANNA NOT MERGE - AllocationDeciders allocationDeciders = new AllocationDeciders( - List.of(new CanRemainNotPreferredAllocationDecider(), new CanAllocateThrottledAllocationDecider()) - ); - RoutingAllocation allocation = new RoutingAllocation(allocationDeciders, clusterState, clusterInfo, null, System.nanoTime()); - - ClusterAllocationExplanation allocationExplanation = TransportClusterAllocationExplainAction.explainShard( - shardRouting, - allocation, - clusterInfo, - randomBoolean(), - true, - new AllocationService( - allocationDeciders, - new TestGatewayAllocator(), - new BalancedShardsAllocator(Settings.EMPTY), - EmptyClusterInfoService.INSTANCE, - EmptySnapshotsInfoService.INSTANCE, - TestShardRoutingRoleStrategies.DEFAULT_ROLE_ONLY - ) - ); - - logger.info("---> Allocation explain response: " + Strings.toString(allocationExplanation, true, true)); - // canRemain on the current node should be NOT_PREFERRED. - assertThat(allocationExplanation.getShardAllocationDecision().getMoveDecision().canRemainNotPreferred(), equalTo(true)); - assertThat( - allocationExplanation.getShardAllocationDecision().getMoveDecision().getCanRemainDecision().type(), - equalTo(Decision.Type.NOT_PREFERRED) - ); - assertThat( - "All other potential nodes should be throttled, resulting in an overall throttled relocation", - allocationExplanation.getShardAllocationDecision().getMoveDecision().getAllocationDecision(), - equalTo(AllocationDecision.THROTTLED) - ); - for (var nodeDecision : allocationExplanation.getShardAllocationDecision().getMoveDecision().getNodeDecisions()) { - assertThat( - "Each individual node should report throttled", - nodeDecision.getNodeDecision(), - equalTo(AllocationDecision.THROTTLED) - ); - } - assertThat( - "Expected the explanation for a move from NOT_PREFERRED to THROTTLED to say the shard won't move right now", - allocationExplanation.getShardAllocationDecision().getMoveDecision().getExplanation(), - equalTo(Explanations.Move.NOT_PREFERRED_TO_THROTTLED) - ); - assertNull( - "A relocation target node should have been selected, even though canRemain not-preferred must wait for throttling to cease", - allocationExplanation.getShardAllocationDecision().getMoveDecision().getTargetNode() - ); - } - public void testInitializingOrRelocatingShardExplanation() throws Exception { ShardRoutingState shardRoutingState = randomFrom(ShardRoutingState.INITIALIZING, ShardRoutingState.RELOCATING); ClusterState clusterState = ClusterStateCreationUtils.state("idx", randomBoolean(), shardRoutingState); diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationDecisionTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationDecisionTests.java index feb1bf67da52e..0f26f7675bd36 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationDecisionTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationDecisionTests.java @@ -109,12 +109,9 @@ public void testValuesOrder() { public void testFromDecisionType() { Type type = randomFrom(Type.values()); AllocationDecision allocationDecision = AllocationDecision.fromDecisionType(type); - AllocationDecision expected = switch (type) { - case NO -> AllocationDecision.NO; - case NOT_PREFERRED -> AllocationDecision.NOT_PREFERRED; - case THROTTLE -> AllocationDecision.THROTTLED; - case YES -> AllocationDecision.YES; - }; + AllocationDecision expected = type == Type.NO ? AllocationDecision.NO + : type == Type.THROTTLE ? AllocationDecision.THROTTLED + : AllocationDecision.YES; assertEquals(expected, allocationDecision); } diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/MoveDecisionTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/MoveDecisionTests.java index f60c459a2a9b0..a06e3d6019739 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/MoveDecisionTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/MoveDecisionTests.java @@ -123,7 +123,6 @@ public void testSerialization() throws IOException { assertEquals(moveDecision.getTargetNode(), readDecision.getTargetNode()); assertEquals(moveDecision.getAllocationDecision(), readDecision.getAllocationDecision()); // node2 should have the highest sort order - assertEquals("node2", moveDecision.getNodeDecisions().iterator().next().getNode().getId()); assertEquals("node2", readDecision.getNodeDecisions().iterator().next().getNode().getId()); } diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/decider/AllocationDecidersTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/decider/AllocationDecidersTests.java index 16c3e26f70061..0ea6dc20cc80a 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/decider/AllocationDecidersTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/decider/AllocationDecidersTests.java @@ -56,11 +56,11 @@ public void testCheckAllDecidersBeforeReturningYes() { } public void testCheckAllDecidersBeforeReturningThrottle() { - var allDecisions = generateDecisions(Decision.THROTTLE, () -> Decision.YES); + var allDecisions = generateDecisions(Decision.THROTTLE, () -> randomFrom(Decision.YES, Decision.NOT_PREFERRED)); var debugMode = randomFrom(RoutingAllocation.DebugMode.values()); var expectedDecision = switch (debugMode) { case OFF -> Decision.THROTTLE; - case EXCLUDE_YES_DECISIONS -> new Decision.Multi().add(Decision.THROTTLE); + case EXCLUDE_YES_DECISIONS -> filterAndCollectToMultiDecision(allDecisions, d -> d.type() != Decision.Type.YES); case ON -> collectToMultiDecision(allDecisions); }; @@ -68,7 +68,7 @@ public void testCheckAllDecidersBeforeReturningThrottle() { } public void testCheckAllDecidersBeforeReturningNotPreferred() { - var allDecisions = generateDecisions(Decision.NOT_PREFERRED, () -> randomFrom(Decision.YES, Decision.THROTTLE)); + var allDecisions = generateDecisions(Decision.NOT_PREFERRED, () -> Decision.YES); var debugMode = randomFrom(RoutingAllocation.DebugMode.values()); var expectedDecision = switch (debugMode) { case OFF -> Decision.NOT_PREFERRED; diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/decider/DecisionTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/decider/DecisionTests.java index 95db962b3b289..034b2e39b721f 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/decider/DecisionTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/decider/DecisionTests.java @@ -25,11 +25,11 @@ public class DecisionTests extends ESTestCase { public void testTypeEnumOrder() { - EnumSerializationTestUtils.assertEnumSerialization(Decision.Type.class, NO, NOT_PREFERRED, THROTTLE, YES); + EnumSerializationTestUtils.assertEnumSerialization(Decision.Type.class, NO, THROTTLE, NOT_PREFERRED, YES); } public void testTypeHigherThan() { - assertTrue(YES.higherThan(THROTTLE) && THROTTLE.higherThan(NOT_PREFERRED) && NOT_PREFERRED.higherThan(NO)); + assertTrue(YES.higherThan(NOT_PREFERRED) && NOT_PREFERRED.higherThan(THROTTLE) && THROTTLE.higherThan(NO)); } public void testTypeAllowed() {