diff --git a/server/src/internalClusterTest/java/org/elasticsearch/action/admin/cluster/allocation/ClusterAllocationExplainIT.java b/server/src/internalClusterTest/java/org/elasticsearch/action/admin/cluster/allocation/ClusterAllocationExplainIT.java index e745bd75be510..a1a799ebfcf4c 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/action/admin/cluster/allocation/ClusterAllocationExplainIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/action/admin/cluster/allocation/ClusterAllocationExplainIT.java @@ -539,7 +539,7 @@ public void testAllocationFilteringPreventsShardMove() throws Exception { assertEquals(AllocationDecision.NO, moveDecision.getAllocationDecision()); assertEquals(Explanations.Move.NO, moveDecision.getExplanation()); assertFalse(moveDecision.canRemain()); - assertFalse(moveDecision.forceMove()); + assertFalse(moveDecision.cannotRemainAndCanMove()); assertFalse(moveDecision.canRebalanceCluster()); assertNull(moveDecision.getClusterRebalanceDecision()); assertNull(moveDecision.getTargetNode()); @@ -652,7 +652,7 @@ public void testRebalancingNotAllowed() throws Exception { assertEquals(AllocationDecision.NO, moveDecision.getAllocationDecision()); assertEquals(Explanations.Rebalance.CANNOT_REBALANCE_CAN_ALLOCATE, moveDecision.getExplanation()); assertTrue(moveDecision.canRemain()); - assertFalse(moveDecision.forceMove()); + assertFalse(moveDecision.cannotRemainAndCanMove()); assertFalse(moveDecision.canRebalanceCluster()); assertNotNull(moveDecision.getCanRemainDecision()); assertNull(moveDecision.getTargetNode()); @@ -758,7 +758,7 @@ public void testWorseBalance() throws Exception { assertEquals(AllocationDecision.NO, moveDecision.getAllocationDecision()); assertEquals(Explanations.Rebalance.ALREADY_BALANCED, moveDecision.getExplanation()); assertTrue(moveDecision.canRemain()); - assertFalse(moveDecision.forceMove()); + assertFalse(moveDecision.cannotRemainAndCanMove()); assertTrue(moveDecision.canRebalanceCluster()); assertNotNull(moveDecision.getCanRemainDecision()); assertNull(moveDecision.getTargetNode()); @@ -856,7 +856,7 @@ public void testBetterBalanceButCannotAllocate() throws Exception { assertEquals(AllocationDecision.NO, moveDecision.getAllocationDecision()); assertEquals(Explanations.Rebalance.ALREADY_BALANCED, moveDecision.getExplanation()); assertTrue(moveDecision.canRemain()); - assertFalse(moveDecision.forceMove()); + assertFalse(moveDecision.cannotRemainAndCanMove()); assertTrue(moveDecision.canRebalanceCluster()); assertNotNull(moveDecision.getCanRemainDecision()); assertNull(moveDecision.getTargetNode()); @@ -964,7 +964,7 @@ public void testAssignedReplicaOnSpecificNode() throws Exception { assertEquals(AllocationDecision.NO, moveDecision.getAllocationDecision()); assertEquals(Explanations.Rebalance.CANNOT_REBALANCE_CANNOT_ALLOCATE, moveDecision.getExplanation()); assertTrue(moveDecision.canRemain()); - assertFalse(moveDecision.forceMove()); + assertFalse(moveDecision.cannotRemainAndCanMove()); assertFalse(moveDecision.canRebalanceCluster()); assertNotNull(moveDecision.getCanRemainDecision()); assertNull(moveDecision.getTargetNode()); 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 368451661fba0..f90f0fb8609d3 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 @@ -92,11 +92,11 @@ public void writeTo(StreamOutput out) throws IOException { * Creates a move decision for the shard being able to remain on its current node, so the shard won't * be forced to move to another node. */ - public static MoveDecision remain(Decision canRemainDecision) { + public static MoveDecision createMoveDecisionWithRemainYesDecision(Decision canRemainDecision) { + assert canRemainDecision.type() != Type.NO; if (canRemainDecision == Decision.YES) { return CACHED_STAY_DECISION; } - assert canRemainDecision.type() != Type.NO; return new MoveDecision(null, null, AllocationDecision.NO_ATTEMPT, canRemainDecision, null, 0); } @@ -150,11 +150,21 @@ public boolean isDecisionTaken() { * returns {@code false} otherwise. If {@link #isDecisionTaken()} returns {@code false}, * then invoking this method will throw an {@code IllegalStateException}. */ - public boolean forceMove() { + public boolean cannotRemainAndCanMove() { checkDecisionState(); return canRemain() == false && canMoveDecision == AllocationDecision.YES; } + /** + * Returns {@code true} if the shard cannot remain on its current node and _cannot_ be moved. + * returns {@code false} otherwise. If {@link #isDecisionTaken()} returns {@code false}, + * then invoking this method will throw an {@code IllegalStateException}. + */ + public boolean cannotRemainAndCannotMove() { + checkDecisionState(); + return canRemainDecision.type() != Type.YES && canMoveDecision != AllocationDecision.YES; + } + /** * Returns {@code true} if the shard can remain on its current node, returns {@code false} otherwise. * If {@link #isDecisionTaken()} returns {@code false}, then invoking this method will throw an {@code IllegalStateException}. @@ -287,7 +297,7 @@ public Iterator toXContentChunked(ToXContent.Params params builder.field("can_rebalance_to_other_node", canMoveDecision); builder.field("rebalance_explanation", getExplanation()); } else { - builder.field("can_move_to_other_node", forceMove() ? "yes" : "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 5e8297fd393c5..fe1458ad349a3 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 @@ -818,7 +818,7 @@ public boolean moveShards() { shardRouting, bestNonPreferredShardMovementsTracker::shardIsBetterThanCurrent ); - if (moveDecision.isDecisionTaken() && moveDecision.forceMove()) { + if (moveDecision.isDecisionTaken() && moveDecision.cannotRemainAndCanMove()) { // Defer moving of not-preferred until we've moved the NOs if (moveDecision.getCanRemainDecision().type() == Type.NOT_PREFERRED) { bestNonPreferredShardMovementsTracker.putBestMoveDecision(shardRouting, moveDecision); @@ -842,7 +842,7 @@ public boolean moveShards() { // invalid, so we must call `decideMove` again. If not, we know we haven't made any moves, and we // can use the cached decision. final var moveDecision = shardMoved ? decideMove(index, shardRouting) : storedShardMovement.moveDecision(); - if (moveDecision.isDecisionTaken() && moveDecision.forceMove()) { + if (moveDecision.isDecisionTaken() && moveDecision.cannotRemainAndCanMove()) { executeMove(shardRouting, index, moveDecision, "move-non-preferred"); // Return after a single move so that the change can be simulated before further moves are made. return true; @@ -895,8 +895,9 @@ public MoveDecision decideMove(final ProjectIndex index, final ShardRouting shar * 2. If the shard is allowed to remain on its current node, no attempt will be made to move the shard and * {@link MoveDecision#getCanRemainDecision} will have a decision type of YES. All other fields in the object will be null. * 3. If the shard is not allowed to remain on its current node, then {@link MoveDecision#getAllocationDecision()} will be - * populated with the decision of moving to another node. If {@link MoveDecision#forceMove()} returns {@code true}, then - * {@link MoveDecision#getTargetNode} will return a non-null value, otherwise the assignedNodeId will be null. + * populated with the decision of moving to another node. If {@link MoveDecision#cannotRemainAndCanMove()} returns + * {@code true}, then {@link MoveDecision#getTargetNode} will return a non-null value, otherwise the assignedNodeId will be + * null. * 4. If the method is invoked in explain mode (e.g. from the cluster allocation explain APIs), then * {@link MoveDecision#getNodeDecisions} will have a non-null value. * @@ -923,7 +924,7 @@ private MoveDecision decideMove(ProjectIndex index, ShardRouting shardRouting, P RoutingNode routingNode = sourceNode.getRoutingNode(); Decision canRemain = allocation.deciders().canRemain(shardRouting, routingNode, allocation); if (canRemain.type() != Decision.Type.NO && canRemain.type() != Decision.Type.NOT_PREFERRED) { - return MoveDecision.remain(canRemain); + return MoveDecision.createMoveDecisionWithRemainYesDecision(canRemain); } // Check predicate to decide whether to assess movement options @@ -939,7 +940,7 @@ private MoveDecision decideMove(ProjectIndex index, ShardRouting shardRouting, P * allocate on the minimal eligible node. */ final MoveDecision moveDecision = decideMove(sorter, shardRouting, sourceNode, canRemain, this::decideCanAllocate); - if (moveDecision.canRemain() == false && moveDecision.forceMove() == false) { + if (moveDecision.cannotRemainAndCannotMove()) { final boolean shardsOnReplacedNode = allocation.metadata().nodeShutdowns().contains(shardRouting.currentNodeId(), REPLACE); if (shardsOnReplacedNode) { return decideMove(sorter, shardRouting, sourceNode, canRemain, this::decideCanForceAllocateForVacate); 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 4dbd2eace1167..7787ac137bac4 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 @@ -29,12 +29,12 @@ public class MoveDecisionTests extends ESTestCase { public void testCachedDecisions() { // cached stay decision - MoveDecision stay1 = MoveDecision.remain(Decision.YES); - MoveDecision stay2 = MoveDecision.remain(Decision.YES); + MoveDecision stay1 = MoveDecision.createMoveDecisionWithRemainYesDecision(Decision.YES); + MoveDecision stay2 = MoveDecision.createMoveDecisionWithRemainYesDecision(Decision.YES); assertSame(stay1, stay2); // not in explain mode, so should use cached decision - stay1 = MoveDecision.remain(new Decision.Single(Type.YES, null, null, (Object[]) null)); - stay2 = MoveDecision.remain(new Decision.Single(Type.YES, null, null, (Object[]) null)); + stay1 = MoveDecision.createMoveDecisionWithRemainYesDecision(new Decision.Single(Type.YES, null, null, (Object[]) null)); + stay2 = MoveDecision.createMoveDecisionWithRemainYesDecision(new Decision.Single(Type.YES, null, null, (Object[]) null)); assertNotSame(stay1, stay2); // cached cannot move decision @@ -57,16 +57,16 @@ public void testCachedDecisions() { } public void testStayDecision() { - MoveDecision stay = MoveDecision.remain(Decision.YES); + MoveDecision stay = MoveDecision.createMoveDecisionWithRemainYesDecision(Decision.YES); assertTrue(stay.canRemain()); - assertFalse(stay.forceMove()); + assertFalse(stay.cannotRemainAndCanMove()); assertTrue(stay.isDecisionTaken()); assertNull(stay.getNodeDecisions()); assertEquals(AllocationDecision.NO_ATTEMPT, stay.getAllocationDecision()); - stay = MoveDecision.remain(Decision.YES); + stay = MoveDecision.createMoveDecisionWithRemainYesDecision(Decision.YES); assertTrue(stay.canRemain()); - assertFalse(stay.forceMove()); + assertFalse(stay.cannotRemainAndCanMove()); assertTrue(stay.isDecisionTaken()); assertNull(stay.getNodeDecisions()); assertEquals(AllocationDecision.NO_ATTEMPT, stay.getAllocationDecision()); @@ -116,7 +116,7 @@ public void testSerialization() throws IOException { MoveDecision readDecision = new MoveDecision(output.bytes().streamInput()); assertEquals(moveDecision.canRemain(), readDecision.canRemain()); assertEquals(moveDecision.getExplanation(), readDecision.getExplanation()); - assertEquals(moveDecision.forceMove(), readDecision.forceMove()); + assertEquals(moveDecision.cannotRemainAndCanMove(), readDecision.cannotRemainAndCanMove()); assertEquals(moveDecision.getNodeDecisions().size(), readDecision.getNodeDecisions().size()); assertEquals(moveDecision.getTargetNode(), readDecision.getTargetNode()); assertEquals(moveDecision.getAllocationDecision(), readDecision.getAllocationDecision());