diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index 1338339b01173..81e4cca69769c 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -368,6 +368,7 @@ static TransportVersion def(int id) { public static final TransportVersion DATA_STREAM_WRITE_INDEX_ONLY_SETTINGS = def(9_142_0_00); public static final TransportVersion SCRIPT_RESCORER = def(9_143_0_00); public static final TransportVersion ESQL_LOOKUP_OPERATOR_EMITTED_ROWS = def(9_144_0_00); + public static final TransportVersion ALLOCATION_DECISION_NOT_PREFERRED = def(9_145_0_00); /* * STOP! READ THIS FIRST! No, really, 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 348ad727c3b63..344cd1e4ce710 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 @@ -111,7 +111,8 @@ public static AllocationDecision fromAllocationStatus(AllocationStatus allocatio */ public static AllocationDecision fromDecisionType(Decision.Type type) { return switch (type) { - case YES -> YES; + // TODO: should not_preferred have own variant? ES-12729 + 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 891818b8e68f7..17c28ffe78b47 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 @@ -232,7 +232,7 @@ 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 diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/AllocationDeciders.java b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/AllocationDeciders.java index 29d8e8051b421..6a09c894dbc7d 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/AllocationDeciders.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/AllocationDeciders.java @@ -205,31 +205,32 @@ private Decision withDeciders( BiFunction logMessageCreator ) { if (debugMode == RoutingAllocation.DebugMode.OFF) { - Decision result = Decision.YES; + Decision mostNegativeDecision = Decision.YES; for (AllocationDecider decider : deciders) { var decision = deciderAction.apply(decider); - if (decision.type() == Decision.Type.NO) { - if (logger.isTraceEnabled()) { - logger.trace(() -> logMessageCreator.apply(decider.getClass().getSimpleName(), decision)); + if (mostNegativeDecision.type().higherThan(decision.type())) { + mostNegativeDecision = decision; + if (mostNegativeDecision.type() == Decision.Type.NO) { + if (logger.isTraceEnabled()) { + logger.trace(() -> logMessageCreator.apply(decider.getClass().getSimpleName(), decision)); + } + break; } - return decision; - } else if (result.type() == Decision.Type.YES && decision.type() == Decision.Type.THROTTLE) { - result = decision; } } - return result; + return mostNegativeDecision; } else { - var result = new Decision.Multi(); + final var multiDecision = new Decision.Multi(); for (AllocationDecider decider : deciders) { var decision = deciderAction.apply(decider); if (logger.isTraceEnabled() && decision.type() == Decision.Type.NO) { logger.trace(() -> logMessageCreator.apply(decider.getClass().getSimpleName(), decision)); } if (decision != Decision.ALWAYS && (debugMode == RoutingAllocation.DebugMode.ON || decision.type() != Decision.Type.YES)) { - result.add(decision); + multiDecision.add(decision); } } - return result; + return multiDecision; } } 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 b2cfd9cc986aa..441909e08a73c 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 @@ -9,6 +9,7 @@ package org.elasticsearch.cluster.routing.allocation.decider; +import org.elasticsearch.TransportVersions; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; @@ -33,6 +34,7 @@ public sealed interface Decision extends ToXContent, Writeable permits Decision. Single ALWAYS = new Single(Type.YES); Single YES = new Single(Type.YES); + Single NOT_PREFERRED = new Single(Type.NOT_PREFERRED); Single NO = new Single(Type.NO); Single THROTTLE = new Single(Type.THROTTLE); @@ -72,6 +74,7 @@ private static Single readSingleFrom(StreamInput in) throws IOException { if (label == null && explanation == null) { return switch (type) { case YES -> YES; + case NOT_PREFERRED -> NOT_PREFERRED; case THROTTLE -> THROTTLE; case NO -> NO; }; @@ -103,48 +106,58 @@ private static Single readSingleFrom(StreamInput in) throws IOException { List getDecisions(); /** - * This enumeration defines the - * possible types of decisions + * This enumeration defines the possible types of decisions */ enum Type implements Writeable { - YES(1), - THROTTLE(2), - NO(0); + // ordered by positiveness; order matters for serialization and comparison + NO, + THROTTLE, + NOT_PREFERRED, + YES; - private final int id; - - Type(int id) { - this.id = id; + public static Type readFrom(StreamInput in) throws IOException { + if (in.getTransportVersion().onOrAfter(TransportVersions.ALLOCATION_DECISION_NOT_PREFERRED)) { + return in.readEnum(Type.class); + } else { + int i = in.readVInt(); + return switch (i) { + case 0 -> NO; + case 1 -> YES; + case 2 -> THROTTLE; + default -> throw new IllegalArgumentException("No Type for integer [" + i + "]"); + }; + } } - public static Type readFrom(StreamInput in) throws IOException { - int i = in.readVInt(); - return switch (i) { - case 0 -> NO; - case 1 -> YES; - case 2 -> THROTTLE; - default -> throw new IllegalArgumentException("No Type for integer [" + i + "]"); - }; + /** + * @return lowest decision by natural order + */ + public static Type min(Type a, Type b) { + return a.compareTo(b) < 0 ? a : b; } @Override public void writeTo(StreamOutput out) throws IOException { - out.writeVInt(id); + if (out.getTransportVersion().onOrAfter(TransportVersions.ALLOCATION_DECISION_NOT_PREFERRED)) { + out.writeEnum(this); + } else { + out.writeVInt(switch (this) { + case NO -> 0; + case NOT_PREFERRED, YES -> 1; + case THROTTLE -> 2; + }); + } } public boolean higherThan(Type other) { - if (this == NO) { - return false; - } else if (other == NO) { - return true; - } else return other == THROTTLE && this == YES; + return this.compareTo(other) > 0; } /** - * @return lowest decision by precedence NO->THROTTLE->YES + * @return true if Type is one of {NOT_PREFERRED, YES} */ - public static Type min(Type a, Type b) { - return a.higherThan(b) ? b : a; + public boolean allowed() { + return this.compareTo(NOT_PREFERRED) >= 0; } } @@ -243,16 +256,8 @@ public Multi add(Decision decision) { @Override public Type type() { - Type ret = Type.YES; - for (int i = 0; i < decisions.size(); i++) { - Type type = decisions.get(i).type(); - if (type == Type.NO) { - return type; - } else if (type == Type.THROTTLE) { - ret = type; - } - } - return ret; + // returns most negative decision + return decisions.stream().map(Single::type).reduce(Type.YES, Type::min); } @Override diff --git a/server/src/main/java/org/elasticsearch/gateway/PrimaryShardAllocator.java b/server/src/main/java/org/elasticsearch/gateway/PrimaryShardAllocator.java index 4754a53314b83..03e975883e3e9 100644 --- a/server/src/main/java/org/elasticsearch/gateway/PrimaryShardAllocator.java +++ b/server/src/main/java/org/elasticsearch/gateway/PrimaryShardAllocator.java @@ -402,7 +402,7 @@ private static NodesToAllocate buildNodesToAllocate( : allocation.deciders().canAllocate(shardRouting, node, allocation); DecidedNode decidedNode = new DecidedNode(nodeShardState, decision); (switch (decision.type()) { - case YES -> yesNodeShards; + case YES, NOT_PREFERRED -> yesNodeShards; case THROTTLE -> throttledNodeShards; case NO -> noNodeShards; }).add(decidedNode); 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 6744d6a31bbb3..4dbd2eace1167 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 @@ -96,12 +96,12 @@ public void testSerialization() throws IOException { DiscoveryNode node1 = DiscoveryNodeUtils.builder("node1").roles(emptySet()).build(); DiscoveryNode node2 = DiscoveryNodeUtils.builder("node2").roles(emptySet()).build(); Type finalDecision = randomFrom(Type.values()); - DiscoveryNode assignedNode = finalDecision == Type.YES ? node1 : null; + DiscoveryNode assignedNode = finalDecision.allowed() ? node1 : null; nodeDecisions.add(new NodeAllocationResult(node1, Decision.NO, 2)); nodeDecisions.add( new NodeAllocationResult( node2, - finalDecision == Type.YES ? Decision.YES : randomFrom(Decision.NO, Decision.THROTTLE, Decision.YES), + finalDecision.allowed() ? Decision.YES : randomFrom(Decision.NO, Decision.THROTTLE, Decision.YES), 1 ) ); 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 20ad4a751e3df..75e97be17ac11 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,23 @@ 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 -> collectToMultiDecision(allDecisions, d -> d.type() != Decision.Type.YES); + case ON -> collectToMultiDecision(allDecisions); + }; + + verifyDecidersCall(debugMode, allDecisions, allDecisions.size(), expectedDecision); + } + + public void testCheckAllDecidersBeforeReturningNotPreferred() { + var allDecisions = generateDecisions(Decision.NOT_PREFERRED, () -> Decision.YES); + var debugMode = randomFrom(RoutingAllocation.DebugMode.values()); + var expectedDecision = switch (debugMode) { + case OFF -> Decision.NOT_PREFERRED; + case EXCLUDE_YES_DECISIONS -> new Decision.Multi().add(Decision.NOT_PREFERRED); case ON -> collectToMultiDecision(allDecisions); }; @@ -69,7 +81,7 @@ public void testCheckAllDecidersBeforeReturningThrottle() { public void testExitsAfterFirstNoDecision() { var expectedDecision = randomFrom(Decision.NO, Decision.single(Decision.Type.NO, "no with label", "explanation")); - var allDecisions = generateDecisions(expectedDecision, () -> randomFrom(Decision.YES, Decision.THROTTLE)); + var allDecisions = generateDecisions(expectedDecision, () -> randomFrom(Decision.YES, Decision.NOT_PREFERRED, Decision.THROTTLE)); var expectedCalls = allDecisions.indexOf(expectedDecision) + 1; verifyDecidersCall(RoutingAllocation.DebugMode.OFF, allDecisions, expectedCalls, expectedDecision); @@ -79,6 +91,7 @@ public void testCollectsAllDecisionsForDebugModeOn() { var allDecisions = generateDecisions( () -> randomFrom( Decision.YES, + Decision.NOT_PREFERRED, Decision.THROTTLE, Decision.single(Decision.Type.THROTTLE, "throttle with label", "explanation"), Decision.NO, @@ -94,6 +107,7 @@ public void testCollectsNoAndThrottleDecisionsForDebugModeExcludeYesDecisions() var allDecisions = generateDecisions( () -> randomFrom( Decision.YES, + Decision.NOT_PREFERRED, Decision.THROTTLE, Decision.single(Decision.Type.THROTTLE, "throttle with label", "explanation"), Decision.NO, 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 63444b6f7aa4d..f4f318a89f1cb 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 @@ -9,10 +9,13 @@ package org.elasticsearch.cluster.routing.allocation.decider; -import org.elasticsearch.cluster.routing.allocation.decider.Decision.Type; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.EnumSerializationTestUtils; + +import java.util.List; import static org.elasticsearch.cluster.routing.allocation.decider.Decision.Type.NO; +import static org.elasticsearch.cluster.routing.allocation.decider.Decision.Type.NOT_PREFERRED; import static org.elasticsearch.cluster.routing.allocation.decider.Decision.Type.THROTTLE; import static org.elasticsearch.cluster.routing.allocation.decider.Decision.Type.YES; @@ -21,23 +24,17 @@ */ public class DecisionTests extends ESTestCase { - /** - * Tests {@link Type#higherThan(Type)} - */ - public void testHigherThan() { - // test YES type - assertTrue(YES.higherThan(NO)); - assertTrue(YES.higherThan(THROTTLE)); - assertFalse(YES.higherThan(YES)); - - // test THROTTLE type - assertTrue(THROTTLE.higherThan(NO)); - assertFalse(THROTTLE.higherThan(THROTTLE)); - assertFalse(THROTTLE.higherThan(YES)); - - // test NO type - assertFalse(NO.higherThan(NO)); - assertFalse(NO.higherThan(THROTTLE)); - assertFalse(NO.higherThan(YES)); + public void testTypeEnumOrder() { + EnumSerializationTestUtils.assertEnumSerialization(Decision.Type.class, NO, THROTTLE, NOT_PREFERRED, YES); + } + + public void testTypeHigherThan() { + assertTrue(YES.higherThan(NOT_PREFERRED) && NOT_PREFERRED.higherThan(THROTTLE) && THROTTLE.higherThan(NO)); } + + public void testTypeAllowed() { + List.of(NOT_PREFERRED, YES).forEach(d -> assertTrue(d.allowed())); + List.of(NO, THROTTLE).forEach(d -> assertFalse(d.allowed())); + } + }