Skip to content

Commit

Permalink
Prevent throttled indices to be searched though wildcards by default
Browse files Browse the repository at this point in the history
Today if a wildcard, date-math expression or alias expands/resolves
to an index that is search-throttled we still search it. This is likely
not the desired behavior since it can unexpectedly slow down searches
significantly.

This change adds a new indices option that allows `search`, `count`
and `msearch` to ignore throttled indices by default. Users can
force expansion to throttled indices by using `ignore_throttled=true`
on the rest request to expand also to throttled indices.

Relates to elastic#34352
  • Loading branch information
s1monw committed Oct 8, 2018
1 parent 6f32f71 commit 6400d7f
Show file tree
Hide file tree
Showing 23 changed files with 233 additions and 79 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public void testRankEvalRequest() throws IOException {

// now try this when test2 is closed
client().performRequest(new Request("POST", "index2/_close"));
rankEvalRequest.indicesOptions(IndicesOptions.fromParameters(null, "true", null, SearchRequest.DEFAULT_INDICES_OPTIONS));
rankEvalRequest.indicesOptions(IndicesOptions.fromParameters(null, "true", null, "false", SearchRequest.DEFAULT_INDICES_OPTIONS));
response = execute(rankEvalRequest, highLevelClient()::rankEval, highLevelClient()::rankEvalAsync);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -971,7 +971,8 @@ public void testMultiSearch() throws IOException {
IndicesOptions msearchDefault = new MultiSearchRequest().indicesOptions();
searchRequest.indicesOptions(IndicesOptions.fromOptions(randomlyGenerated.ignoreUnavailable(),
randomlyGenerated.allowNoIndices(), randomlyGenerated.expandWildcardsOpen(), randomlyGenerated.expandWildcardsClosed(),
msearchDefault.allowAliasesToMultipleIndices(), msearchDefault.forbidClosedIndices(), msearchDefault.ignoreAliases()));
msearchDefault.allowAliasesToMultipleIndices(), msearchDefault.forbidClosedIndices(), msearchDefault.ignoreAliases(),
msearchDefault.ignoreThrottled()));
multiSearchRequest.add(searchRequest);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public class MultiSearchTemplateRequest extends ActionRequest implements Composi
private int maxConcurrentSearchRequests = 0;
private List<SearchTemplateRequest> requests = new ArrayList<>();

private IndicesOptions indicesOptions = IndicesOptions.strictExpandOpenAndForbidClosed();
private IndicesOptions indicesOptions = IndicesOptions.strictExpandOpenAndForbidClosedIgnoreThrottled();

/**
* Add a search template request to execute. Note, the order is important, the search response will be returned in the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,45 +286,45 @@ public void testIndicesOptions() {
// test that ignore_unavailable=true works but returns one result less
assertTrue(client().admin().indices().prepareClose("test2").get().isAcknowledged());

request.indicesOptions(IndicesOptions.fromParameters(null, "true", null, SearchRequest.DEFAULT_INDICES_OPTIONS));
request.indicesOptions(IndicesOptions.fromParameters(null, "true", null, "false", SearchRequest.DEFAULT_INDICES_OPTIONS));
response = client().execute(RankEvalAction.INSTANCE, request).actionGet();
details = (PrecisionAtK.Detail) response.getPartialResults().get("amsterdam_query").getMetricDetails();
assertEquals(6, details.getRetrieved());
assertEquals(5, details.getRelevantRetrieved());

// test that ignore_unavailable=false or default settings throw an IndexClosedException
assertTrue(client().admin().indices().prepareClose("test2").get().isAcknowledged());
request.indicesOptions(IndicesOptions.fromParameters(null, "false", null, SearchRequest.DEFAULT_INDICES_OPTIONS));
request.indicesOptions(IndicesOptions.fromParameters(null, "false", null, "false", SearchRequest.DEFAULT_INDICES_OPTIONS));
response = client().execute(RankEvalAction.INSTANCE, request).actionGet();
assertEquals(1, response.getFailures().size());
assertThat(response.getFailures().get("amsterdam_query"), instanceOf(IndexClosedException.class));

// test expand_wildcards
request = new RankEvalRequest(task, new String[] { "tes*" });
request.indicesOptions(IndicesOptions.fromParameters("none", null, null, SearchRequest.DEFAULT_INDICES_OPTIONS));
request.indicesOptions(IndicesOptions.fromParameters("none", null, null, "false", SearchRequest.DEFAULT_INDICES_OPTIONS));
response = client().execute(RankEvalAction.INSTANCE, request).actionGet();
details = (PrecisionAtK.Detail) response.getPartialResults().get("amsterdam_query").getMetricDetails();
assertEquals(0, details.getRetrieved());

request.indicesOptions(IndicesOptions.fromParameters("open", null, null, SearchRequest.DEFAULT_INDICES_OPTIONS));
request.indicesOptions(IndicesOptions.fromParameters("open", null, null, "false", SearchRequest.DEFAULT_INDICES_OPTIONS));
response = client().execute(RankEvalAction.INSTANCE, request).actionGet();
details = (PrecisionAtK.Detail) response.getPartialResults().get("amsterdam_query").getMetricDetails();
assertEquals(6, details.getRetrieved());
assertEquals(5, details.getRelevantRetrieved());

request.indicesOptions(IndicesOptions.fromParameters("closed", null, null, SearchRequest.DEFAULT_INDICES_OPTIONS));
request.indicesOptions(IndicesOptions.fromParameters("closed", null, null, "false", SearchRequest.DEFAULT_INDICES_OPTIONS));
response = client().execute(RankEvalAction.INSTANCE, request).actionGet();
assertEquals(1, response.getFailures().size());
assertThat(response.getFailures().get("amsterdam_query"), instanceOf(IndexClosedException.class));

// test allow_no_indices
request = new RankEvalRequest(task, new String[] { "bad*" });
request.indicesOptions(IndicesOptions.fromParameters(null, null, "true", SearchRequest.DEFAULT_INDICES_OPTIONS));
request.indicesOptions(IndicesOptions.fromParameters(null, null, "true", "false", SearchRequest.DEFAULT_INDICES_OPTIONS));
response = client().execute(RankEvalAction.INSTANCE, request).actionGet();
details = (PrecisionAtK.Detail) response.getPartialResults().get("amsterdam_query").getMetricDetails();
assertEquals(0, details.getRetrieved());

request.indicesOptions(IndicesOptions.fromParameters(null, null, "false", SearchRequest.DEFAULT_INDICES_OPTIONS));
request.indicesOptions(IndicesOptions.fromParameters(null, null, "false", "false", SearchRequest.DEFAULT_INDICES_OPTIONS));
response = client().execute(RankEvalAction.INSTANCE, request).actionGet();
assertEquals(1, response.getFailures().size());
assertThat(response.getFailures().get("amsterdam_query"), instanceOf(IndexNotFoundException.class));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ protected RankEvalRequest createTestInstance() {
}
RankEvalRequest rankEvalRequest = new RankEvalRequest(RankEvalSpecTests.createTestItem(), indices);
IndicesOptions indicesOptions = IndicesOptions.fromOptions(
randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean());
randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean(),
randomBoolean());
rankEvalRequest.indicesOptions(indicesOptions);
return rankEvalRequest;
}
Expand Down
4 changes: 4 additions & 0 deletions rest-api-spec/src/main/resources/rest-api-spec/api/count.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@
"type" : "boolean",
"description" : "Whether specified concrete indices should be ignored when unavailable (missing or closed)"
},
"ignore_throttled": {
"type" : "boolean",
"description" : "Whether specified concrete, expanded or aliased indices should be ignored when throttled"
},
"allow_no_indices": {
"type" : "boolean",
"description" : "Whether to ignore if a wildcard indices expression resolves into no concrete indices. (This includes `_all` string or when no indices have been specified)"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@
"type" : "boolean",
"description" : "Whether specified concrete indices should be ignored when unavailable (missing or closed)"
},
"ignore_throttled": {
"type" : "boolean",
"description" : "Whether specified concrete, expanded or aliased indices should be ignored when throttled"
},
"allow_no_indices": {
"type" : "boolean",
"description" : "Whether to ignore if a wildcard indices expression resolves into no concrete indices. (This includes `_all` string or when no indices have been specified)"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@
"type" : "boolean",
"description" : "Whether specified concrete indices should be ignored when unavailable (missing or closed)"
},
"ignore_throttled": {
"type" : "boolean",
"description" : "Whether specified concrete, expanded or aliased indices should be ignored when throttled"
},
"allow_no_indices": {
"type" : "boolean",
"description" : "Whether to ignore if a wildcard indices expression resolves into no concrete indices. (This includes `_all` string or when no indices have been specified)"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public class IndicesAliasesRequest extends AcknowledgedRequest<IndicesAliasesReq
// indices options that require every specified index to exist, expand wildcards only to open
// indices, don't allow that no indices are resolved from wildcard expressions and resolve the
// expressions only against indices
private static final IndicesOptions INDICES_OPTIONS = IndicesOptions.fromOptions(false, false, true, false, true, false, true);
private static final IndicesOptions INDICES_OPTIONS = IndicesOptions.fromOptions(false, false, true, false, true, false, true, false);

public IndicesAliasesRequest() {
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public class DeleteIndexRequest extends AcknowledgedRequest<DeleteIndexRequest>

private String[] indices;
// Delete index should work by default on both open and closed indices.
private IndicesOptions indicesOptions = IndicesOptions.fromOptions(false, true, true, true, false, false, true);
private IndicesOptions indicesOptions = IndicesOptions.fromOptions(false, true, true, true, false, false, true, false);

public DeleteIndexRequest() {
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public class MultiSearchRequest extends ActionRequest implements CompositeIndice
private int maxConcurrentSearchRequests = 0;
private List<SearchRequest> requests = new ArrayList<>();

private IndicesOptions indicesOptions = IndicesOptions.strictExpandOpenAndForbidClosed();
private IndicesOptions indicesOptions = IndicesOptions.strictExpandOpenAndForbidClosedIgnoreThrottled();

/**
* Add a search request to execute. Note, the order is important, the search response will be returned in the
Expand Down Expand Up @@ -287,7 +287,7 @@ public static byte[] writeMultiLineFormat(MultiSearchRequest multiSearchRequest,
}
return output.toByteArray();
}

public static void writeSearchRequestParams(SearchRequest request, XContentBuilder xContentBuilder) throws IOException {
xContentBuilder.startObject();
if (request.indices() != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
*/
public class MultiSearchRequestBuilder extends ActionRequestBuilder<MultiSearchRequest, MultiSearchResponse> {


public MultiSearchRequestBuilder(ElasticsearchClient client, MultiSearchAction action) {
super(client, action, new MultiSearchRequest());
}
Expand All @@ -40,7 +41,8 @@ public MultiSearchRequestBuilder(ElasticsearchClient client, MultiSearchAction a
* will not be used (if set).
*/
public MultiSearchRequestBuilder add(SearchRequest request) {
if (request.indicesOptions() == IndicesOptions.strictExpandOpenAndForbidClosed() && request().indicesOptions() != IndicesOptions.strictExpandOpenAndForbidClosed()) {
if (request.indicesOptions() == IndicesOptions.strictExpandOpenAndForbidClosed()
&& request().indicesOptions() != IndicesOptions.strictExpandOpenAndForbidClosed()) {
request.indicesOptions(request().indicesOptions());
}

Expand All @@ -53,7 +55,8 @@ public MultiSearchRequestBuilder add(SearchRequest request) {
* same order as the search requests.
*/
public MultiSearchRequestBuilder add(SearchRequestBuilder request) {
if (request.request().indicesOptions() == IndicesOptions.strictExpandOpenAndForbidClosed() && request().indicesOptions() != IndicesOptions.strictExpandOpenAndForbidClosed()) {
if (request.request().indicesOptions() == SearchRequest.DEFAULT_INDICES_OPTIONS
&& request().indicesOptions() != SearchRequest.DEFAULT_INDICES_OPTIONS) {
request.request().indicesOptions(request().indicesOptions());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public final class SearchRequest extends ActionRequest implements IndicesRequest

private String[] types = Strings.EMPTY_ARRAY;

public static final IndicesOptions DEFAULT_INDICES_OPTIONS = IndicesOptions.strictExpandOpenAndForbidClosed();
public static final IndicesOptions DEFAULT_INDICES_OPTIONS = IndicesOptions.strictExpandOpenAndForbidClosedIgnoreThrottled();

private IndicesOptions indicesOptions = DEFAULT_INDICES_OPTIONS;

Expand Down
Loading

0 comments on commit 6400d7f

Please sign in to comment.