From 6b457abbd3c607a5959bcd2f72950caf856c5098 Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Tue, 24 Mar 2020 17:04:38 +0100 Subject: [PATCH] Async search: prevent users from overriding pre_filter_shard_size (#54088) Submit async search forces pre_filter_shard_size for the underlying search that it creates. With this commit we also prevent users from overriding such default as part of request validation. --- docs/reference/search/async-search.asciidoc | 7 ++++--- .../xpack/search/RestSubmitAsyncSearchAction.java | 3 +++ .../xpack/search/SubmitAsyncSearchRequestTests.java | 9 +++++++++ .../core/search/action/SubmitAsyncSearchRequest.java | 6 +++++- 4 files changed, 21 insertions(+), 4 deletions(-) diff --git a/docs/reference/search/async-search.asciidoc b/docs/reference/search/async-search.asciidoc index 7f5957a94e4..25abf09aafd 100644 --- a/docs/reference/search/async-search.asciidoc +++ b/docs/reference/search/async-search.asciidoc @@ -109,9 +109,10 @@ become available, which happens whenever shard results are reduced. A partial reduction is performed every time the coordinating node has received a certain number of new shard responses (`5` by default). * `request_cache` defaults to `true` -* `pre_filter_shard_size` defaults to `1`: this is to enforce the execution of -a pre-filter roundtrip to retrieve statistics from each shard so that the ones -that surely don't hold any document matching the query get skipped. +* `pre_filter_shard_size` defaults to `1` and cannot be changed: this is to +enforce the execution of a pre-filter roundtrip to retrieve statistics from +each shard so that the ones that surely don't hold any document matching the +query get skipped. * `ccs_minimize_roundtrips` defaults to `false`, which is also the only supported value diff --git a/x-pack/plugin/async-search/src/main/java/org/elasticsearch/xpack/search/RestSubmitAsyncSearchAction.java b/x-pack/plugin/async-search/src/main/java/org/elasticsearch/xpack/search/RestSubmitAsyncSearchAction.java index e1020f8e5a2..9e1d5fe9121 100644 --- a/x-pack/plugin/async-search/src/main/java/org/elasticsearch/xpack/search/RestSubmitAsyncSearchAction.java +++ b/x-pack/plugin/async-search/src/main/java/org/elasticsearch/xpack/search/RestSubmitAsyncSearchAction.java @@ -45,6 +45,9 @@ public final class RestSubmitAsyncSearchAction extends BaseRestHandler { protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { SubmitAsyncSearchRequest submit = new SubmitAsyncSearchRequest(); IntConsumer setSize = size -> submit.getSearchRequest().source().size(size); + //for simplicity, we share parsing with ordinary search. That means a couple of unsupported parameters, like scroll, + // pre_filter_shard_size and ccs_minimize_roundtrips get set to the search request although the REST spec don't list + //them as supported. We rely on SubmitAsyncSearchRequest#validate to fail in case they are set. request.withContentOrSourceParamParserOrNull(parser -> parseSearchRequest(submit.getSearchRequest(), request, parser, setSize)); diff --git a/x-pack/plugin/async-search/src/test/java/org/elasticsearch/xpack/search/SubmitAsyncSearchRequestTests.java b/x-pack/plugin/async-search/src/test/java/org/elasticsearch/xpack/search/SubmitAsyncSearchRequestTests.java index 2aae3817205..7f53f24e8d9 100644 --- a/x-pack/plugin/async-search/src/test/java/org/elasticsearch/xpack/search/SubmitAsyncSearchRequestTests.java +++ b/x-pack/plugin/async-search/src/test/java/org/elasticsearch/xpack/search/SubmitAsyncSearchRequestTests.java @@ -109,4 +109,13 @@ public class SubmitAsyncSearchRequestTests extends AbstractWireSerializingTransf assertThat(exc.validationErrors().size(), equalTo(1)); assertThat(exc.validationErrors().get(0), containsString("suggest")); } + + public void testValidatePreFilterShardSize() { + SubmitAsyncSearchRequest req = new SubmitAsyncSearchRequest(); + req.getSearchRequest().setPreFilterShardSize(randomIntBetween(2, Integer.MAX_VALUE)); + ActionRequestValidationException exc = req.validate(); + assertNotNull(exc); + assertThat(exc.validationErrors().size(), equalTo(1)); + assertThat(exc.validationErrors().get(0), containsString("[pre_filter_shard_size]")); + } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/search/action/SubmitAsyncSearchRequest.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/search/action/SubmitAsyncSearchRequest.java index 6717c766f03..c96be38d642 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/search/action/SubmitAsyncSearchRequest.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/search/action/SubmitAsyncSearchRequest.java @@ -128,7 +128,7 @@ public class SubmitAsyncSearchRequest extends ActionRequest { public ActionRequestValidationException validate() { ActionRequestValidationException validationException = request.validate(); if (request.scroll() != null) { - addValidationError("[scroll] queries are not supported", validationException); + validationException = addValidationError("[scroll] queries are not supported", validationException); } if (request.isSuggestOnly()) { validationException = addValidationError("suggest-only queries are not supported", validationException); @@ -141,6 +141,10 @@ public class SubmitAsyncSearchRequest extends ActionRequest { validationException = addValidationError("[ccs_minimize_roundtrips] is not supported on async search queries", validationException); } + if (request.getPreFilterShardSize() == null || request.getPreFilterShardSize() != 1) { + validationException = + addValidationError("[pre_filter_shard_size] cannot be changed for async search queries", validationException); + } return validationException; }