Skip to content

Commit 83c1b52

Browse files
mhl-bmhl-b
authored
Add not-preferred allocation decision (elastic#133177)
1 parent 1c9aa8d commit 83c1b52

File tree

9 files changed

+93
-74
lines changed

9 files changed

+93
-74
lines changed

‎server/src/main/java/org/elasticsearch/TransportVersions.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,7 @@ static TransportVersion def(int id) {
368368
public static final TransportVersion DATA_STREAM_WRITE_INDEX_ONLY_SETTINGS = def(9_142_0_00);
369369
public static final TransportVersion SCRIPT_RESCORER = def(9_143_0_00);
370370
public static final TransportVersion ESQL_LOOKUP_OPERATOR_EMITTED_ROWS = def(9_144_0_00);
371+
public static final TransportVersion ALLOCATION_DECISION_NOT_PREFERRED = def(9_145_0_00);
371372

372373
/*
373374
* STOP! READ THIS FIRST! No, really,

‎server/src/main/java/org/elasticsearch/cluster/routing/allocation/AllocationDecision.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,8 @@ public static AllocationDecision fromAllocationStatus(AllocationStatus allocatio
111111
*/
112112
public static AllocationDecision fromDecisionType(Decision.Type type) {
113113
return switch (type) {
114-
case YES -> YES;
114+
// TODO: should not_preferred have own variant? ES-12729
115+
case YES, NOT_PREFERRED -> YES;
115116
case THROTTLE -> THROTTLED;
116117
case NO -> NO;
117118
};

‎server/src/main/java/org/elasticsearch/cluster/routing/allocation/MoveDecision.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ public String getExplanation() {
232232
? Explanations.Rebalance.CANNOT_REBALANCE_CAN_ALLOCATE
233233
: Explanations.Rebalance.CANNOT_REBALANCE_CANNOT_ALLOCATE;
234234
case THROTTLE -> Explanations.Rebalance.CLUSTER_THROTTLE;
235-
case YES -> {
235+
case YES, NOT_PREFERRED -> {
236236
if (getTargetNode() != null) {
237237
yield canMoveDecision == AllocationDecision.THROTTLED
238238
? Explanations.Rebalance.NODE_THROTTLE

‎server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/AllocationDeciders.java‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -205,31 +205,32 @@ private Decision withDeciders(
205205
BiFunction<String, Decision, String> logMessageCreator
206206
) {
207207
if (debugMode == RoutingAllocation.DebugMode.OFF) {
208-
Decision result = Decision.YES;
208+
Decision mostNegativeDecision = Decision.YES;
209209
for (AllocationDecider decider : deciders) {
210210
var decision = deciderAction.apply(decider);
211-
if (decision.type() == Decision.Type.NO) {
212-
if (logger.isTraceEnabled()) {
213-
logger.trace(() -> logMessageCreator.apply(decider.getClass().getSimpleName(), decision));
211+
if (mostNegativeDecision.type().higherThan(decision.type())) {
212+
mostNegativeDecision = decision;
213+
if (mostNegativeDecision.type() == Decision.Type.NO) {
214+
if (logger.isTraceEnabled()) {
215+
logger.trace(() -> logMessageCreator.apply(decider.getClass().getSimpleName(), decision));
216+
}
217+
break;
214218
}
215-
return decision;
216-
} else if (result.type() == Decision.Type.YES && decision.type() == Decision.Type.THROTTLE) {
217-
result = decision;
218219
}
219220
}
220-
return result;
221+
return mostNegativeDecision;
221222
} else {
222-
var result = new Decision.Multi();
223+
final var multiDecision = new Decision.Multi();
223224
for (AllocationDecider decider : deciders) {
224225
var decision = deciderAction.apply(decider);
225226
if (logger.isTraceEnabled() && decision.type() == Decision.Type.NO) {
226227
logger.trace(() -> logMessageCreator.apply(decider.getClass().getSimpleName(), decision));
227228
}
228229
if (decision != Decision.ALWAYS && (debugMode == RoutingAllocation.DebugMode.ON || decision.type() != Decision.Type.YES)) {
229-
result.add(decision);
230+
multiDecision.add(decision);
230231
}
231232
}
232-
return result;
233+
return multiDecision;
233234
}
234235
}
235236

‎server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/Decision.java‎

Lines changed: 41 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
package org.elasticsearch.cluster.routing.allocation.decider;
1111

12+
import org.elasticsearch.TransportVersions;
1213
import org.elasticsearch.common.io.stream.StreamInput;
1314
import org.elasticsearch.common.io.stream.StreamOutput;
1415
import org.elasticsearch.common.io.stream.Writeable;
@@ -33,6 +34,7 @@ public sealed interface Decision extends ToXContent, Writeable permits Decision.
3334

3435
Single ALWAYS = new Single(Type.YES);
3536
Single YES = new Single(Type.YES);
37+
Single NOT_PREFERRED = new Single(Type.NOT_PREFERRED);
3638
Single NO = new Single(Type.NO);
3739
Single THROTTLE = new Single(Type.THROTTLE);
3840

@@ -72,6 +74,7 @@ private static Single readSingleFrom(StreamInput in) throws IOException {
7274
if (label == null && explanation == null) {
7375
return switch (type) {
7476
case YES -> YES;
77+
case NOT_PREFERRED -> NOT_PREFERRED;
7578
case THROTTLE -> THROTTLE;
7679
case NO -> NO;
7780
};
@@ -103,48 +106,58 @@ private static Single readSingleFrom(StreamInput in) throws IOException {
103106
List<Decision> getDecisions();
104107

105108
/**
106-
* This enumeration defines the
107-
* possible types of decisions
109+
* This enumeration defines the possible types of decisions
108110
*/
109111
enum Type implements Writeable {
110-
YES(1),
111-
THROTTLE(2),
112-
NO(0);
112+
// ordered by positiveness; order matters for serialization and comparison
113+
NO,
114+
THROTTLE,
115+
NOT_PREFERRED,
116+
YES;
113117

114-
private final int id;
115-
116-
Type(int id) {
117-
this.id = id;
118+
public static Type readFrom(StreamInput in) throws IOException {
119+
if (in.getTransportVersion().onOrAfter(TransportVersions.ALLOCATION_DECISION_NOT_PREFERRED)) {
120+
return in.readEnum(Type.class);
121+
} else {
122+
int i = in.readVInt();
123+
return switch (i) {
124+
case 0 -> NO;
125+
case 1 -> YES;
126+
case 2 -> THROTTLE;
127+
default -> throw new IllegalArgumentException("No Type for integer [" + i + "]");
128+
};
129+
}
118130
}
119131

120-
public static Type readFrom(StreamInput in) throws IOException {
121-
int i = in.readVInt();
122-
return switch (i) {
123-
case 0 -> NO;
124-
case 1 -> YES;
125-
case 2 -> THROTTLE;
126-
default -> throw new IllegalArgumentException("No Type for integer [" + i + "]");
127-
};
132+
/**
133+
* @return lowest decision by natural order
134+
*/
135+
public static Type min(Type a, Type b) {
136+
return a.compareTo(b) < 0 ? a : b;
128137
}
129138

130139
@Override
131140
public void writeTo(StreamOutput out) throws IOException {
132-
out.writeVInt(id);
141+
if (out.getTransportVersion().onOrAfter(TransportVersions.ALLOCATION_DECISION_NOT_PREFERRED)) {
142+
out.writeEnum(this);
143+
} else {
144+
out.writeVInt(switch (this) {
145+
case NO -> 0;
146+
case NOT_PREFERRED, YES -> 1;
147+
case THROTTLE -> 2;
148+
});
149+
}
133150
}
134151

135152
public boolean higherThan(Type other) {
136-
if (this == NO) {
137-
return false;
138-
} else if (other == NO) {
139-
return true;
140-
} else return other == THROTTLE && this == YES;
153+
return this.compareTo(other) > 0;
141154
}
142155

143156
/**
144-
* @return lowest decision by precedence NO->THROTTLE->YES
157+
* @return true if Type is one of {NOT_PREFERRED, YES}
145158
*/
146-
public static Type min(Type a, Type b) {
147-
return a.higherThan(b) ? b : a;
159+
public boolean allowed() {
160+
return this.compareTo(NOT_PREFERRED) >= 0;
148161
}
149162

150163
}
@@ -243,16 +256,8 @@ public Multi add(Decision decision) {
243256

244257
@Override
245258
public Type type() {
246-
Type ret = Type.YES;
247-
for (int i = 0; i < decisions.size(); i++) {
248-
Type type = decisions.get(i).type();
249-
if (type == Type.NO) {
250-
return type;
251-
} else if (type == Type.THROTTLE) {
252-
ret = type;
253-
}
254-
}
255-
return ret;
259+
// returns most negative decision
260+
return decisions.stream().map(Single::type).reduce(Type.YES, Type::min);
256261
}
257262

258263
@Override

‎server/src/main/java/org/elasticsearch/gateway/PrimaryShardAllocator.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ private static NodesToAllocate buildNodesToAllocate(
402402
: allocation.deciders().canAllocate(shardRouting, node, allocation);
403403
DecidedNode decidedNode = new DecidedNode(nodeShardState, decision);
404404
(switch (decision.type()) {
405-
case YES -> yesNodeShards;
405+
case YES, NOT_PREFERRED -> yesNodeShards;
406406
case THROTTLE -> throttledNodeShards;
407407
case NO -> noNodeShards;
408408
}).add(decidedNode);

‎server/src/test/java/org/elasticsearch/cluster/routing/allocation/MoveDecisionTests.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,12 +96,12 @@ public void testSerialization() throws IOException {
9696
DiscoveryNode node1 = DiscoveryNodeUtils.builder("node1").roles(emptySet()).build();
9797
DiscoveryNode node2 = DiscoveryNodeUtils.builder("node2").roles(emptySet()).build();
9898
Type finalDecision = randomFrom(Type.values());
99-
DiscoveryNode assignedNode = finalDecision == Type.YES ? node1 : null;
99+
DiscoveryNode assignedNode = finalDecision.allowed() ? node1 : null;
100100
nodeDecisions.add(new NodeAllocationResult(node1, Decision.NO, 2));
101101
nodeDecisions.add(
102102
new NodeAllocationResult(
103103
node2,
104-
finalDecision == Type.YES ? Decision.YES : randomFrom(Decision.NO, Decision.THROTTLE, Decision.YES),
104+
finalDecision.allowed() ? Decision.YES : randomFrom(Decision.NO, Decision.THROTTLE, Decision.YES),
105105
1
106106
)
107107
);

‎server/src/test/java/org/elasticsearch/cluster/routing/allocation/decider/AllocationDecidersTests.java‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,23 @@ public void testCheckAllDecidersBeforeReturningYes() {
5656
}
5757

5858
public void testCheckAllDecidersBeforeReturningThrottle() {
59-
var allDecisions = generateDecisions(Decision.THROTTLE, () -> Decision.YES);
59+
var allDecisions = generateDecisions(Decision.THROTTLE, () -> randomFrom(Decision.YES, Decision.NOT_PREFERRED));
6060
var debugMode = randomFrom(RoutingAllocation.DebugMode.values());
6161
var expectedDecision = switch (debugMode) {
6262
case OFF -> Decision.THROTTLE;
63-
case EXCLUDE_YES_DECISIONS -> new Decision.Multi().add(Decision.THROTTLE);
63+
case EXCLUDE_YES_DECISIONS -> collectToMultiDecision(allDecisions, d -> d.type() != Decision.Type.YES);
64+
case ON -> collectToMultiDecision(allDecisions);
65+
};
66+
67+
verifyDecidersCall(debugMode, allDecisions, allDecisions.size(), expectedDecision);
68+
}
69+
70+
public void testCheckAllDecidersBeforeReturningNotPreferred() {
71+
var allDecisions = generateDecisions(Decision.NOT_PREFERRED, () -> Decision.YES);
72+
var debugMode = randomFrom(RoutingAllocation.DebugMode.values());
73+
var expectedDecision = switch (debugMode) {
74+
case OFF -> Decision.NOT_PREFERRED;
75+
case EXCLUDE_YES_DECISIONS -> new Decision.Multi().add(Decision.NOT_PREFERRED);
6476
case ON -> collectToMultiDecision(allDecisions);
6577
};
6678

@@ -69,7 +81,7 @@ public void testCheckAllDecidersBeforeReturningThrottle() {
6981

7082
public void testExitsAfterFirstNoDecision() {
7183
var expectedDecision = randomFrom(Decision.NO, Decision.single(Decision.Type.NO, "no with label", "explanation"));
72-
var allDecisions = generateDecisions(expectedDecision, () -> randomFrom(Decision.YES, Decision.THROTTLE));
84+
var allDecisions = generateDecisions(expectedDecision, () -> randomFrom(Decision.YES, Decision.NOT_PREFERRED, Decision.THROTTLE));
7385
var expectedCalls = allDecisions.indexOf(expectedDecision) + 1;
7486

7587
verifyDecidersCall(RoutingAllocation.DebugMode.OFF, allDecisions, expectedCalls, expectedDecision);
@@ -79,6 +91,7 @@ public void testCollectsAllDecisionsForDebugModeOn() {
7991
var allDecisions = generateDecisions(
8092
() -> randomFrom(
8193
Decision.YES,
94+
Decision.NOT_PREFERRED,
8295
Decision.THROTTLE,
8396
Decision.single(Decision.Type.THROTTLE, "throttle with label", "explanation"),
8497
Decision.NO,
@@ -94,6 +107,7 @@ public void testCollectsNoAndThrottleDecisionsForDebugModeExcludeYesDecisions()
94107
var allDecisions = generateDecisions(
95108
() -> randomFrom(
96109
Decision.YES,
110+
Decision.NOT_PREFERRED,
97111
Decision.THROTTLE,
98112
Decision.single(Decision.Type.THROTTLE, "throttle with label", "explanation"),
99113
Decision.NO,

‎server/src/test/java/org/elasticsearch/cluster/routing/allocation/decider/DecisionTests.java‎

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,13 @@
99

1010
package org.elasticsearch.cluster.routing.allocation.decider;
1111

12-
import org.elasticsearch.cluster.routing.allocation.decider.Decision.Type;
1312
import org.elasticsearch.test.ESTestCase;
13+
import org.elasticsearch.test.EnumSerializationTestUtils;
14+
15+
import java.util.List;
1416

1517
import static org.elasticsearch.cluster.routing.allocation.decider.Decision.Type.NO;
18+
import static org.elasticsearch.cluster.routing.allocation.decider.Decision.Type.NOT_PREFERRED;
1619
import static org.elasticsearch.cluster.routing.allocation.decider.Decision.Type.THROTTLE;
1720
import static org.elasticsearch.cluster.routing.allocation.decider.Decision.Type.YES;
1821

@@ -21,23 +24,17 @@
2124
*/
2225
public class DecisionTests extends ESTestCase {
2326

24-
/**
25-
* Tests {@link Type#higherThan(Type)}
26-
*/
27-
public void testHigherThan() {
28-
// test YES type
29-
assertTrue(YES.higherThan(NO));
30-
assertTrue(YES.higherThan(THROTTLE));
31-
assertFalse(YES.higherThan(YES));
32-
33-
// test THROTTLE type
34-
assertTrue(THROTTLE.higherThan(NO));
35-
assertFalse(THROTTLE.higherThan(THROTTLE));
36-
assertFalse(THROTTLE.higherThan(YES));
37-
38-
// test NO type
39-
assertFalse(NO.higherThan(NO));
40-
assertFalse(NO.higherThan(THROTTLE));
41-
assertFalse(NO.higherThan(YES));
27+
public void testTypeEnumOrder() {
28+
EnumSerializationTestUtils.assertEnumSerialization(Decision.Type.class, NO, THROTTLE, NOT_PREFERRED, YES);
29+
}
30+
31+
public void testTypeHigherThan() {
32+
assertTrue(YES.higherThan(NOT_PREFERRED) && NOT_PREFERRED.higherThan(THROTTLE) && THROTTLE.higherThan(NO));
4233
}
34+
35+
public void testTypeAllowed() {
36+
List.of(NOT_PREFERRED, YES).forEach(d -> assertTrue(d.allowed()));
37+
List.of(NO, THROTTLE).forEach(d -> assertFalse(d.allowed()));
38+
}
39+
4340
}

0 commit comments

Comments
 (0)