Skip to content

Commit 7813daf

Browse files
original-brownbearoriginal-brownbear
committed
CR: add test for short-circuit
1 parent 89bc2c6 commit 7813daf

File tree

2 files changed

+134
-8
lines changed

2 files changed

+134
-8
lines changed

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public Decision canRebalance(ShardRouting shardRouting, RoutingAllocation alloca
5252
// short track if a NO is returned.
5353
if (decision.type() == Decision.Type.NO) {
5454
if (!allocation.debugDecision()) {
55-
return decision;
55+
return Decision.NO;
5656
} else {
5757
ret.add(decision);
5858
}
@@ -79,7 +79,7 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, Routing
7979
}
8080
// short circuit only if debugging is not enabled
8181
if (!allocation.debugDecision()) {
82-
return decision;
82+
return Decision.NO;
8383
} else {
8484
ret.add(decision);
8585
}
@@ -108,7 +108,7 @@ public Decision canRemain(ShardRouting shardRouting, RoutingNode node, RoutingAl
108108
shardRouting, node.nodeId(), allocationDecider.getClass().getSimpleName());
109109
}
110110
if (!allocation.debugDecision()) {
111-
return decision;
111+
return Decision.NO;
112112
} else {
113113
ret.add(decision);
114114
}
@@ -127,7 +127,7 @@ public Decision canAllocate(IndexMetadata indexMetadata, RoutingNode node, Routi
127127
// short track if a NO is returned.
128128
if (decision.type() == Decision.Type.NO) {
129129
if (!allocation.debugDecision()) {
130-
return decision;
130+
return Decision.NO;
131131
} else {
132132
ret.add(decision);
133133
}
@@ -146,7 +146,7 @@ public Decision shouldAutoExpandToNode(IndexMetadata indexMetadata, DiscoveryNod
146146
// short track if a NO is returned.
147147
if (decision.type() == Decision.Type.NO) {
148148
if (!allocation.debugDecision()) {
149-
return decision;
149+
return Decision.NO;
150150
} else {
151151
ret.add(decision);
152152
}
@@ -165,7 +165,7 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingAllocation allocat
165165
// short track if a NO is returned.
166166
if (decision.type() == Decision.Type.NO) {
167167
if (!allocation.debugDecision()) {
168-
return decision;
168+
return Decision.NO;
169169
} else {
170170
ret.add(decision);
171171
}
@@ -184,7 +184,7 @@ public Decision canRebalance(RoutingAllocation allocation) {
184184
// short track if a NO is returned.
185185
if (decision.type() == Decision.Type.NO) {
186186
if (!allocation.debugDecision()) {
187-
return decision;
187+
return Decision.NO;
188188
} else {
189189
ret.add(decision);
190190
}
@@ -212,7 +212,7 @@ public Decision canForceAllocatePrimary(ShardRouting shardRouting, RoutingNode n
212212
shardRouting.shardId(), node.nodeId(), decider.getClass().getSimpleName());
213213
}
214214
if (!allocation.debugDecision()) {
215-
return decision;
215+
return Decision.NO;
216216
} else {
217217
ret.add(decision);
218218
}

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

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,4 +117,130 @@ private void verify(Decision decision, Matcher<Collection<? extends Decision>> m
117117
Decision.Multi multi = (Decision.Multi) decision;
118118
assertThat(multi.getDecisions(), matcher);
119119
}
120+
121+
public void testEarlyTermination() {
122+
final Decision decisionOne = randomFrom(Decision.NO, Decision.single(Decision.Type.NO, "label1", "explanation"));
123+
final Decision decisionTwo = randomFrom(Decision.NO, Decision.single(Decision.Type.NO, "label2", "explanation"));
124+
final AllocationDeciders allocationDeciders = new AllocationDeciders(List.of(
125+
new AllocationDecider() {
126+
@Override
127+
public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, RoutingAllocation allocation) {
128+
return decisionOne;
129+
}
130+
131+
@Override
132+
public Decision canRebalance(ShardRouting shardRouting, RoutingAllocation allocation) {
133+
return decisionOne;
134+
}
135+
136+
@Override
137+
public Decision canRemain(ShardRouting shardRouting, RoutingNode node, RoutingAllocation allocation) {
138+
return decisionOne;
139+
}
140+
141+
@Override
142+
public Decision canAllocate(ShardRouting shardRouting, RoutingAllocation allocation) {
143+
return decisionOne;
144+
}
145+
146+
@Override
147+
public Decision canAllocate(IndexMetadata indexMetadata, RoutingNode node, RoutingAllocation allocation) {
148+
return decisionOne;
149+
}
150+
151+
@Override
152+
public Decision shouldAutoExpandToNode(IndexMetadata indexMetadata, DiscoveryNode node, RoutingAllocation allocation) {
153+
return decisionOne;
154+
}
155+
156+
@Override
157+
public Decision canRebalance(RoutingAllocation allocation) {
158+
return decisionOne;
159+
}
160+
161+
@Override
162+
public Decision canForceAllocatePrimary(ShardRouting shardRouting, RoutingNode node, RoutingAllocation allocation) {
163+
return decisionOne;
164+
}
165+
}, new AllocationDecider() {
166+
167+
@Override
168+
public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, RoutingAllocation allocation) {
169+
return decision(allocation);
170+
}
171+
172+
@Override
173+
public Decision canRebalance(ShardRouting shardRouting, RoutingAllocation allocation) {
174+
return decision(allocation);
175+
}
176+
177+
@Override
178+
public Decision canRemain(ShardRouting shardRouting, RoutingNode node, RoutingAllocation allocation) {
179+
return decision(allocation);
180+
}
181+
182+
@Override
183+
public Decision canAllocate(ShardRouting shardRouting, RoutingAllocation allocation) {
184+
return decision(allocation);
185+
}
186+
187+
@Override
188+
public Decision canAllocate(IndexMetadata indexMetadata, RoutingNode node, RoutingAllocation allocation) {
189+
return decision(allocation);
190+
}
191+
192+
@Override
193+
public Decision shouldAutoExpandToNode(IndexMetadata indexMetadata, DiscoveryNode node, RoutingAllocation allocation) {
194+
return decision(allocation);
195+
}
196+
197+
@Override
198+
public Decision canRebalance(RoutingAllocation allocation) {
199+
return decision(allocation);
200+
}
201+
202+
@Override
203+
public Decision canForceAllocatePrimary(ShardRouting shardRouting, RoutingNode node, RoutingAllocation allocation) {
204+
return decision(allocation);
205+
}
206+
207+
private Decision decision(RoutingAllocation allocation) {
208+
if (allocation.debugDecision() == false) {
209+
throw new AssertionError("Should not be called");
210+
}
211+
return decisionTwo;
212+
}
213+
}));
214+
215+
// no debug should just short-circuit to no, no matter what kind of no type return the first decider returns
216+
final ShardRouting shardRouting = ShardRouting.newUnassigned(new ShardId("test", "testUUID", 0), true,
217+
RecoverySource.ExistingStoreRecoverySource.INSTANCE, new UnassignedInfo(UnassignedInfo.Reason.INDEX_CREATED, "_message"));
218+
final RoutingNode routingNode = new RoutingNode("testNode", null);
219+
final ClusterState clusterState = ClusterState.builder(new ClusterName("test")).build();
220+
final IndexMetadata indexMetadata =
221+
IndexMetadata.builder("idx").settings(settings(Version.CURRENT)).numberOfShards(1).numberOfReplicas(0).build();
222+
223+
final RoutingAllocation allocation = new RoutingAllocation(allocationDeciders,
224+
clusterState.getRoutingNodes(), clusterState, null, null,0L);
225+
assertSame(Decision.NO, allocationDeciders.canAllocate(shardRouting, routingNode, allocation));
226+
assertSame(Decision.NO, allocationDeciders.canRebalance(shardRouting, allocation));
227+
assertSame(Decision.NO, allocationDeciders.canRemain(shardRouting, routingNode, allocation));
228+
assertSame(Decision.NO, allocationDeciders.canAllocate(shardRouting, allocation));
229+
assertSame(Decision.NO, allocationDeciders.canAllocate(indexMetadata, routingNode, allocation));
230+
assertSame(Decision.NO, allocationDeciders.shouldAutoExpandToNode(indexMetadata, null, allocation));
231+
assertSame(Decision.NO, allocationDeciders.canRebalance(allocation));
232+
assertSame(Decision.NO, allocationDeciders.canForceAllocatePrimary(shardRouting, routingNode, allocation));
233+
234+
// debug decision should contain both individual decisions in a multi-decision
235+
allocation.debugDecision(true);
236+
final Decision expectedDebugDecision = new Decision.Multi().add(decisionOne).add(decisionTwo);
237+
assertEquals(expectedDebugDecision, allocationDeciders.canAllocate(shardRouting, routingNode, allocation));
238+
assertEquals(expectedDebugDecision, allocationDeciders.canRebalance(shardRouting, allocation));
239+
assertEquals(expectedDebugDecision, allocationDeciders.canRemain(shardRouting, routingNode, allocation));
240+
assertEquals(expectedDebugDecision, allocationDeciders.canAllocate(shardRouting, allocation));
241+
assertEquals(expectedDebugDecision, allocationDeciders.canAllocate(indexMetadata, routingNode, allocation));
242+
assertEquals(expectedDebugDecision, allocationDeciders.shouldAutoExpandToNode(indexMetadata, null, allocation));
243+
assertEquals(expectedDebugDecision, allocationDeciders.canRebalance(allocation));
244+
assertEquals(expectedDebugDecision, allocationDeciders.canForceAllocatePrimary(shardRouting, routingNode, allocation));
245+
}
120246
}

0 commit comments

Comments
 (0)