Skip to content

Commit 04d6a06

Browse files
Fix debug mode in MaxRetryAllocationDecider (#89973) (#89978)
In #62275 we refactored this code a bit and inadvertently reversed the sense of this conditional when running in debug mode. This commit fixes the mistake. Co-authored-by: Elastic Machine <[email protected]>
1 parent 801ac33 commit 04d6a06

File tree

3 files changed

+23
-22
lines changed

3 files changed

+23
-22
lines changed

docs/changelog/89973.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 89973
2+
summary: Fix debug mode in `MaxRetryAllocationDecider`
3+
area: Allocation
4+
type: bug
5+
issues: []

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ private static Decision decisionWithFailures(
6161
}
6262

6363
private static Decision debugDecision(Decision decision, UnassignedInfo unassignedInfo, int numFailedAllocations, int maxRetry) {
64-
if (decision.type() == Decision.Type.YES) {
64+
if (decision.type() == Decision.Type.NO) {
6565
return Decision.single(
6666
Decision.Type.NO,
6767
NAME,

server/src/test/java/org/elasticsearch/cluster/routing/allocation/MaxRetryAllocationDeciderTests.java

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -183,12 +183,8 @@ public void testFailedAllocation() {
183183
assertThat(unassignedPrimary.unassignedInfo().getMessage(), containsString("boom" + i));
184184
// MaxRetryAllocationDecider#canForceAllocatePrimary should return YES decisions because canAllocate returns YES here
185185
assertEquals(
186-
Decision.YES,
187-
new MaxRetryAllocationDecider().canForceAllocatePrimary(
188-
unassignedPrimary,
189-
null,
190-
new RoutingAllocation(null, clusterState, null, null, 0)
191-
)
186+
Decision.Type.YES,
187+
new MaxRetryAllocationDecider().canForceAllocatePrimary(unassignedPrimary, null, newRoutingAllocation(clusterState)).type()
192188
);
193189
}
194190
// now we go and check that we are actually stick to unassigned on the next failure
@@ -207,12 +203,8 @@ public void testFailedAllocation() {
207203
assertThat(unassignedPrimary.unassignedInfo().getMessage(), containsString("boom"));
208204
// MaxRetryAllocationDecider#canForceAllocatePrimary should return a NO decision because canAllocate returns NO here
209205
assertEquals(
210-
Decision.NO,
211-
new MaxRetryAllocationDecider().canForceAllocatePrimary(
212-
unassignedPrimary,
213-
null,
214-
new RoutingAllocation(null, clusterState, null, null, 0)
215-
)
206+
Decision.Type.NO,
207+
new MaxRetryAllocationDecider().canForceAllocatePrimary(unassignedPrimary, null, newRoutingAllocation(clusterState)).type()
216208
);
217209
}
218210

@@ -247,12 +239,12 @@ public void testFailedAllocation() {
247239
assertThat(unassignedPrimary.unassignedInfo().getMessage(), containsString("boom"));
248240
// bumped up the max retry count, so canForceAllocatePrimary should return a YES decision
249241
assertEquals(
250-
Decision.YES,
242+
Decision.Type.YES,
251243
new MaxRetryAllocationDecider().canForceAllocatePrimary(
252244
routingTable.index("idx").shard(0).shard(0),
253245
null,
254-
new RoutingAllocation(null, clusterState, null, null, 0)
255-
)
246+
newRoutingAllocation(clusterState)
247+
).type()
256248
);
257249

258250
// now we start the shard
@@ -279,13 +271,17 @@ public void testFailedAllocation() {
279271
assertThat(unassignedPrimary.unassignedInfo().getMessage(), containsString("ZOOOMG"));
280272
// Counter reset, so MaxRetryAllocationDecider#canForceAllocatePrimary should return a YES decision
281273
assertEquals(
282-
Decision.YES,
283-
new MaxRetryAllocationDecider().canForceAllocatePrimary(
284-
unassignedPrimary,
285-
null,
286-
new RoutingAllocation(null, clusterState, null, null, 0)
287-
)
274+
Decision.Type.YES,
275+
new MaxRetryAllocationDecider().canForceAllocatePrimary(unassignedPrimary, null, newRoutingAllocation(clusterState)).type()
288276
);
289277
}
290278

279+
private RoutingAllocation newRoutingAllocation(ClusterState clusterState) {
280+
final var routingAllocation = new RoutingAllocation(null, clusterState, null, null, 0);
281+
if (randomBoolean()) {
282+
routingAllocation.setDebugMode(randomFrom(RoutingAllocation.DebugMode.values()));
283+
}
284+
return routingAllocation;
285+
}
286+
291287
}

0 commit comments

Comments
 (0)