Skip to content

Commit 2f6ac68

Browse files
Deciders should not by default collect yes'es (#52438)
AllocationDeciders would collect Yes decisions when not asking for debug info. Changed to only include Yes decisions when debug is requested (explain).
1 parent 30316d6 commit 2f6ac68

File tree

2 files changed

+152
-31
lines changed

2 files changed

+152
-31
lines changed

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

Lines changed: 26 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,6 @@
3030
import java.util.Collection;
3131
import java.util.Collections;
3232

33-
import static org.elasticsearch.cluster.routing.allocation.RoutingAllocation.DebugMode.EXCLUDE_YES_DECISIONS;
34-
3533
/**
3634
* A composite {@link AllocationDecider} combining the "decision" of multiple
3735
* {@link AllocationDecider} implementations into a single allocation decision.
@@ -58,9 +56,8 @@ public Decision canRebalance(ShardRouting shardRouting, RoutingAllocation alloca
5856
} else {
5957
ret.add(decision);
6058
}
61-
} else if (decision != Decision.ALWAYS
62-
&& (allocation.getDebugMode() != EXCLUDE_YES_DECISIONS || decision.type() != Decision.Type.YES)) {
63-
ret.add(decision);
59+
} else {
60+
addDecision(ret, decision, allocation);
6461
}
6562
}
6663
return ret;
@@ -86,11 +83,8 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, Routing
8683
} else {
8784
ret.add(decision);
8885
}
89-
} else if (decision != Decision.ALWAYS
90-
&& (allocation.getDebugMode() != EXCLUDE_YES_DECISIONS || decision.type() != Decision.Type.YES)) {
91-
// the assumption is that a decider that returns the static instance Decision#ALWAYS
92-
// does not really implements canAllocate
93-
ret.add(decision);
86+
} else {
87+
addDecision(ret, decision, allocation);
9488
}
9589
}
9690
return ret;
@@ -118,9 +112,8 @@ public Decision canRemain(ShardRouting shardRouting, RoutingNode node, RoutingAl
118112
} else {
119113
ret.add(decision);
120114
}
121-
} else if (decision != Decision.ALWAYS
122-
&& (allocation.getDebugMode() != EXCLUDE_YES_DECISIONS || decision.type() != Decision.Type.YES)) {
123-
ret.add(decision);
115+
} else {
116+
addDecision(ret, decision, allocation);
124117
}
125118
}
126119
return ret;
@@ -138,9 +131,8 @@ public Decision canAllocate(IndexMetaData indexMetaData, RoutingNode node, Routi
138131
} else {
139132
ret.add(decision);
140133
}
141-
} else if (decision != Decision.ALWAYS
142-
&& (allocation.getDebugMode() != EXCLUDE_YES_DECISIONS || decision.type() != Decision.Type.YES)) {
143-
ret.add(decision);
134+
} else {
135+
addDecision(ret, decision, allocation);
144136
}
145137
}
146138
return ret;
@@ -158,9 +150,8 @@ public Decision shouldAutoExpandToNode(IndexMetaData indexMetaData, DiscoveryNod
158150
} else {
159151
ret.add(decision);
160152
}
161-
} else if (decision != Decision.ALWAYS
162-
&& (allocation.getDebugMode() != EXCLUDE_YES_DECISIONS || decision.type() != Decision.Type.YES)) {
163-
ret.add(decision);
153+
} else {
154+
addDecision(ret, decision, allocation);
164155
}
165156
}
166157
return ret;
@@ -178,9 +169,8 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingAllocation allocat
178169
} else {
179170
ret.add(decision);
180171
}
181-
} else if (decision != Decision.ALWAYS
182-
&& (allocation.getDebugMode() != EXCLUDE_YES_DECISIONS || decision.type() != Decision.Type.YES)) {
183-
ret.add(decision);
172+
} else {
173+
addDecision(ret, decision, allocation);
184174
}
185175
}
186176
return ret;
@@ -198,9 +188,8 @@ public Decision canAllocate(RoutingNode node, RoutingAllocation allocation) {
198188
} else {
199189
ret.add(decision);
200190
}
201-
} else if (decision != Decision.ALWAYS
202-
&& (allocation.getDebugMode() != EXCLUDE_YES_DECISIONS || decision.type() != Decision.Type.YES)) {
203-
ret.add(decision);
191+
} else {
192+
addDecision(ret, decision, allocation);
204193
}
205194
}
206195
return ret;
@@ -218,9 +207,8 @@ public Decision canRebalance(RoutingAllocation allocation) {
218207
} else {
219208
ret.add(decision);
220209
}
221-
} else if (decision != Decision.ALWAYS
222-
&& (allocation.getDebugMode() != EXCLUDE_YES_DECISIONS || decision.type() != Decision.Type.YES)) {
223-
ret.add(decision);
210+
} else {
211+
addDecision(ret, decision, allocation);
224212
}
225213
}
226214
return ret;
@@ -247,11 +235,18 @@ public Decision canForceAllocatePrimary(ShardRouting shardRouting, RoutingNode n
247235
} else {
248236
ret.add(decision);
249237
}
250-
} else if (decision != Decision.ALWAYS
251-
&& (allocation.getDebugMode() != EXCLUDE_YES_DECISIONS || decision.type() != Decision.Type.YES)) {
252-
ret.add(decision);
238+
} else {
239+
addDecision(ret, decision, allocation);
253240
}
254241
}
255242
return ret;
256243
}
244+
245+
private void addDecision(Decision.Multi ret, Decision decision, RoutingAllocation allocation) {
246+
// We never add ALWAYS decisions and only add YES decisions when requested by debug mode (since Multi default is YES).
247+
if (decision != Decision.ALWAYS
248+
&& (allocation.getDebugMode() == RoutingAllocation.DebugMode.ON || decision.type() != Decision.Type.YES)) {
249+
ret.add(decision);
250+
}
251+
}
257252
}
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.elasticsearch.cluster.routing.allocation.decider;
21+
22+
import org.elasticsearch.Version;
23+
import org.elasticsearch.cluster.ClusterName;
24+
import org.elasticsearch.cluster.ClusterState;
25+
import org.elasticsearch.cluster.metadata.IndexMetaData;
26+
import org.elasticsearch.cluster.node.DiscoveryNode;
27+
import org.elasticsearch.cluster.routing.RecoverySource;
28+
import org.elasticsearch.cluster.routing.RoutingNode;
29+
import org.elasticsearch.cluster.routing.ShardRouting;
30+
import org.elasticsearch.cluster.routing.UnassignedInfo;
31+
import org.elasticsearch.cluster.routing.allocation.RoutingAllocation;
32+
import org.elasticsearch.index.shard.ShardId;
33+
import org.elasticsearch.test.ESTestCase;
34+
import org.hamcrest.Matcher;
35+
import org.hamcrest.Matchers;
36+
37+
import java.util.Collection;
38+
import java.util.List;
39+
40+
public class AllocationDecidersTests extends ESTestCase {
41+
42+
public void testDebugMode() {
43+
verifyDebugMode(RoutingAllocation.DebugMode.ON, Matchers.hasSize(1));
44+
}
45+
46+
public void testNoDebugMode() {
47+
verifyDebugMode(RoutingAllocation.DebugMode.OFF, Matchers.empty());
48+
}
49+
50+
public void testDebugExcludeYesMode() {
51+
verifyDebugMode(RoutingAllocation.DebugMode.EXCLUDE_YES_DECISIONS, Matchers.empty());
52+
}
53+
54+
private void verifyDebugMode(RoutingAllocation.DebugMode mode, Matcher<Collection<? extends Decision>> matcher) {
55+
AllocationDeciders deciders = new AllocationDeciders(List.of(new AllocationDecider() {
56+
@Override
57+
public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, RoutingAllocation allocation) {
58+
return Decision.YES;
59+
}
60+
61+
@Override
62+
public Decision canRebalance(ShardRouting shardRouting, RoutingAllocation allocation) {
63+
return Decision.YES;
64+
}
65+
66+
@Override
67+
public Decision canRemain(ShardRouting shardRouting, RoutingNode node, RoutingAllocation allocation) {
68+
return Decision.YES;
69+
}
70+
71+
@Override
72+
public Decision canAllocate(ShardRouting shardRouting, RoutingAllocation allocation) {
73+
return Decision.YES;
74+
}
75+
76+
@Override
77+
public Decision canAllocate(IndexMetaData indexMetaData, RoutingNode node, RoutingAllocation allocation) {
78+
return Decision.YES;
79+
}
80+
81+
@Override
82+
public Decision canAllocate(RoutingNode node, RoutingAllocation allocation) {
83+
return Decision.YES;
84+
}
85+
86+
@Override
87+
public Decision shouldAutoExpandToNode(IndexMetaData indexMetaData, DiscoveryNode node, RoutingAllocation allocation) {
88+
return Decision.YES;
89+
}
90+
91+
@Override
92+
public Decision canRebalance(RoutingAllocation allocation) {
93+
return Decision.YES;
94+
}
95+
}));
96+
97+
ClusterState clusterState = ClusterState.builder(new ClusterName("test")).build();
98+
final RoutingAllocation allocation = new RoutingAllocation(deciders,
99+
clusterState.getRoutingNodes(), clusterState, null, 0L);
100+
101+
allocation.setDebugMode(mode);
102+
final UnassignedInfo unassignedInfo = new UnassignedInfo(UnassignedInfo.Reason.INDEX_CREATED, "_message");
103+
final ShardRouting shardRouting = ShardRouting.newUnassigned(new ShardId("test", "testUUID", 0), true,
104+
RecoverySource.ExistingStoreRecoverySource.INSTANCE, unassignedInfo);
105+
IndexMetaData idx =
106+
IndexMetaData.builder("idx").settings(settings(Version.CURRENT)).numberOfShards(1).numberOfReplicas(0).build();
107+
108+
RoutingNode routingNode = new RoutingNode("testNode", null);
109+
verify(deciders.canAllocate(shardRouting, routingNode, allocation), matcher);
110+
verify(deciders.canAllocate(idx, routingNode, allocation), matcher);
111+
verify(deciders.canAllocate(shardRouting, allocation), matcher);
112+
verify(deciders.canAllocate(routingNode, allocation), matcher);
113+
verify(deciders.canRebalance(shardRouting, allocation), matcher);
114+
verify(deciders.canRebalance(allocation), matcher);
115+
verify(deciders.canRemain(shardRouting, routingNode, allocation), matcher);
116+
verify(deciders.canForceAllocatePrimary(shardRouting, routingNode, allocation), matcher);
117+
verify(deciders.shouldAutoExpandToNode(idx, null, allocation), matcher);
118+
}
119+
120+
private void verify(Decision decision, Matcher<Collection<? extends Decision>> matcher) {
121+
assertThat(decision.type(), Matchers.equalTo(Decision.Type.YES));
122+
assertThat(decision, Matchers.instanceOf(Decision.Multi.class));
123+
Decision.Multi multi = (Decision.Multi) decision;
124+
assertThat(multi.getDecisions(), matcher);
125+
}
126+
}

0 commit comments

Comments
 (0)