Skip to content

Commit c70c440

Browse files
authored
Adjust status on bad allocation explain requests
When a user requests a cluster allocation explain in a situation where it does not make sense (for example, there are no unassigned shards), we should consider this a bad request instead of a server error. Yet, today by throwing an illegal state exception, these are treated as server errors. This commit adjusts these so that they throw illegal argument exceptions and are treated as bad requests. Relates #25503
1 parent 6deb18c commit c70c440

File tree

3 files changed

+17
-11
lines changed

3 files changed

+17
-11
lines changed

core/src/main/java/org/elasticsearch/action/admin/cluster/allocation/TransportClusterAllocationExplainAction.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ public static ShardRouting findShardToExplain(ClusterAllocationExplainRequest re
139139
foundShard = ui.next();
140140
}
141141
if (foundShard == null) {
142-
throw new IllegalStateException("unable to find any unassigned shards to explain [" + request + "]");
142+
throw new IllegalArgumentException("unable to find any unassigned shards to explain [" + request + "]");
143143
}
144144
} else {
145145
String index = request.getIndex();
@@ -151,7 +151,8 @@ public static ShardRouting findShardToExplain(ClusterAllocationExplainRequest re
151151
DiscoveryNode primaryNode = allocation.nodes().resolveNode(request.getCurrentNode());
152152
// the primary is assigned to a node other than the node specified in the request
153153
if (primaryNode.getId().equals(foundShard.currentNodeId()) == false) {
154-
throw new IllegalStateException("unable to find primary shard assigned to node [" + request.getCurrentNode() + "]");
154+
throw new IllegalArgumentException(
155+
"unable to find primary shard assigned to node [" + request.getCurrentNode() + "]");
155156
}
156157
}
157158
} else {
@@ -168,7 +169,7 @@ public static ShardRouting findShardToExplain(ClusterAllocationExplainRequest re
168169
}
169170
}
170171
if (foundShard == null) {
171-
throw new IllegalStateException("unable to find a replica shard assigned to node [" +
172+
throw new IllegalArgumentException("unable to find a replica shard assigned to node [" +
172173
request.getCurrentNode() + "]");
173174
}
174175
} else {
@@ -193,7 +194,7 @@ public static ShardRouting findShardToExplain(ClusterAllocationExplainRequest re
193194
}
194195

195196
if (foundShard == null) {
196-
throw new IllegalStateException("unable to find any shards to explain [" + request + "] in the routing table");
197+
throw new IllegalArgumentException("unable to find any shards to explain [" + request + "] in the routing table");
197198
}
198199
return foundShard;
199200
}

core/src/test/java/org/elasticsearch/action/admin/cluster/allocation/ClusterAllocationExplainActionTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ public void testFindAnyUnassignedShardToExplain() {
108108
final ClusterState allStartedClusterState = ClusterStateCreationUtils.state("idx", randomBoolean(),
109109
ShardRoutingState.STARTED, ShardRoutingState.STARTED);
110110
final ClusterAllocationExplainRequest anyUnassignedShardsRequest = new ClusterAllocationExplainRequest();
111-
expectThrows(IllegalStateException.class, () ->
111+
expectThrows(IllegalArgumentException.class, () ->
112112
findShardToExplain(anyUnassignedShardsRequest, routingAllocation(allStartedClusterState)));
113113
}
114114

@@ -161,7 +161,7 @@ public void testFindShardAssignedToNode() {
161161
}
162162
}
163163
final ClusterAllocationExplainRequest failingRequest = new ClusterAllocationExplainRequest("idx", 0, primary, explainNode);
164-
expectThrows(IllegalStateException.class, () -> findShardToExplain(failingRequest, allocation));
164+
expectThrows(IllegalArgumentException.class, () -> findShardToExplain(failingRequest, allocation));
165165
}
166166

167167
private static RoutingAllocation routingAllocation(ClusterState clusterState) {

rest-api-spec/src/main/resources/rest-api-spec/test/cluster.allocation_explain/10_basic.yml

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,19 @@
1-
---
2-
"cluster shard allocation explanation test":
1+
"bad cluster shard allocation explanation request":
32
- skip:
4-
version: " - 5.1.99"
5-
reason: explain API response output was changed starting in 5.2.0
3+
version: " - 5.99.99"
4+
reason: response status on bad request was changed starting in 5.6.0
65

76
- do:
87
# there aren't any unassigned shards to explain
9-
catch: /illegal_state_exception/
8+
catch: /illegal_argument_exception/
109
cluster.allocation_explain: {}
1110

11+
---
12+
"cluster shard allocation explanation test":
13+
- skip:
14+
version: " - 5.1.99"
15+
reason: explain API response output was changed starting in 5.2.0
16+
1217
- do:
1318
indices.create:
1419
index: test

0 commit comments

Comments
 (0)