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
This commit is contained in:
Jason Tedor 2017-06-30 17:50:20 -04:00 committed by GitHub
parent 6deb18c0de
commit c70c440050
3 changed files with 17 additions and 11 deletions

View File

@ -139,7 +139,7 @@ public class TransportClusterAllocationExplainAction
foundShard = ui.next();
}
if (foundShard == null) {
throw new IllegalStateException("unable to find any unassigned shards to explain [" + request + "]");
throw new IllegalArgumentException("unable to find any unassigned shards to explain [" + request + "]");
}
} else {
String index = request.getIndex();
@ -151,7 +151,8 @@ public class TransportClusterAllocationExplainAction
DiscoveryNode primaryNode = allocation.nodes().resolveNode(request.getCurrentNode());
// the primary is assigned to a node other than the node specified in the request
if (primaryNode.getId().equals(foundShard.currentNodeId()) == false) {
throw new IllegalStateException("unable to find primary shard assigned to node [" + request.getCurrentNode() + "]");
throw new IllegalArgumentException(
"unable to find primary shard assigned to node [" + request.getCurrentNode() + "]");
}
}
} else {
@ -168,7 +169,7 @@ public class TransportClusterAllocationExplainAction
}
}
if (foundShard == null) {
throw new IllegalStateException("unable to find a replica shard assigned to node [" +
throw new IllegalArgumentException("unable to find a replica shard assigned to node [" +
request.getCurrentNode() + "]");
}
} else {
@ -193,7 +194,7 @@ public class TransportClusterAllocationExplainAction
}
if (foundShard == null) {
throw new IllegalStateException("unable to find any shards to explain [" + request + "] in the routing table");
throw new IllegalArgumentException("unable to find any shards to explain [" + request + "] in the routing table");
}
return foundShard;
}

View File

@ -108,7 +108,7 @@ public class ClusterAllocationExplainActionTests extends ESTestCase {
final ClusterState allStartedClusterState = ClusterStateCreationUtils.state("idx", randomBoolean(),
ShardRoutingState.STARTED, ShardRoutingState.STARTED);
final ClusterAllocationExplainRequest anyUnassignedShardsRequest = new ClusterAllocationExplainRequest();
expectThrows(IllegalStateException.class, () ->
expectThrows(IllegalArgumentException.class, () ->
findShardToExplain(anyUnassignedShardsRequest, routingAllocation(allStartedClusterState)));
}
@ -161,7 +161,7 @@ public class ClusterAllocationExplainActionTests extends ESTestCase {
}
}
final ClusterAllocationExplainRequest failingRequest = new ClusterAllocationExplainRequest("idx", 0, primary, explainNode);
expectThrows(IllegalStateException.class, () -> findShardToExplain(failingRequest, allocation));
expectThrows(IllegalArgumentException.class, () -> findShardToExplain(failingRequest, allocation));
}
private static RoutingAllocation routingAllocation(ClusterState clusterState) {

View File

@ -1,14 +1,19 @@
"bad cluster shard allocation explanation request":
- skip:
version: " - 5.99.99"
reason: response status on bad request was changed starting in 5.6.0
- do:
# there aren't any unassigned shards to explain
catch: /illegal_argument_exception/
cluster.allocation_explain: {}
---
"cluster shard allocation explanation test":
- skip:
version: " - 5.1.99"
reason: explain API response output was changed starting in 5.2.0
- do:
# there aren't any unassigned shards to explain
catch: /illegal_state_exception/
cluster.allocation_explain: {}
- do:
indices.create:
index: test