diff --git a/core/src/main/java/org/elasticsearch/index/reindex/AbstractBulkByScrollRequest.java b/core/src/main/java/org/elasticsearch/index/reindex/AbstractBulkByScrollRequest.java index 5bfae5fde924a..9f10304622ba4 100644 --- a/core/src/main/java/org/elasticsearch/index/reindex/AbstractBulkByScrollRequest.java +++ b/core/src/main/java/org/elasticsearch/index/reindex/AbstractBulkByScrollRequest.java @@ -171,6 +171,9 @@ public int getSize() { * documents. */ public Self setSize(int size) { + if (size < 0) { + throw new IllegalArgumentException("[size] parameter cannot be negative, found [" + size + "]"); + } this.size = size; return self(); } @@ -367,10 +370,13 @@ protected Self doForSlice(Self request, TaskId slicingTask) { .setShouldStoreResult(false) // Split requests per second between all slices .setRequestsPerSecond(requestsPerSecond / slices) - // Size is split between workers. This means the size might round down! - .setSize(size == SIZE_ALL_MATCHES ? SIZE_ALL_MATCHES : size / slices) // Sub requests don't have workers .setSlices(1); + if (size != -1) { + // Size is split between workers. This means the size might round + // down! + request.setSize(size == SIZE_ALL_MATCHES ? SIZE_ALL_MATCHES : size / slices); + } // Set the parent task so this task is cancelled if we cancel the parent request.setParentTask(slicingTask); // TODO It'd be nice not to refresh on every slice. Instead we should refresh after the sub requests finish. diff --git a/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java b/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java index 99f74d1f8e0da..0ab6e5ebcc498 100644 --- a/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java @@ -346,6 +346,9 @@ public int from() { * The number of search hits to return. Defaults to 10. */ public SearchSourceBuilder size(int size) { + if (size < 0) { + throw new IllegalArgumentException("[size] parameter cannot be negative, found [" + size + "]"); + } this.size = size; return this; } diff --git a/core/src/test/java/org/elasticsearch/index/reindex/AbstractBulkByScrollRequestTestCase.java b/core/src/test/java/org/elasticsearch/index/reindex/AbstractBulkByScrollRequestTestCase.java index debeab52d979d..8a255d376aff5 100644 --- a/core/src/test/java/org/elasticsearch/index/reindex/AbstractBulkByScrollRequestTestCase.java +++ b/core/src/test/java/org/elasticsearch/index/reindex/AbstractBulkByScrollRequestTestCase.java @@ -42,7 +42,9 @@ public void testForSlice() { original.setSlices(between(2, 1000)); original.setRequestsPerSecond( randomBoolean() ? Float.POSITIVE_INFINITY : randomValueOtherThanMany(r -> r < 0, ESTestCase::randomFloat)); - original.setSize(randomBoolean() ? AbstractBulkByScrollRequest.SIZE_ALL_MATCHES : between(0, Integer.MAX_VALUE)); + if (randomBoolean()) { + original.setSize(between(0, Integer.MAX_VALUE)); + } TaskId slicingTask = new TaskId(randomAlphaOfLength(5), randomLong()); SearchRequest sliceRequest = new SearchRequest(); diff --git a/core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java b/core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java index f8bd7b80d0e9f..4b2599adda5c0 100644 --- a/core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java @@ -365,6 +365,15 @@ public void testNegativeFromErrors() { assertEquals("[from] parameter cannot be negative", expected.getMessage()); } + public void testNegativeSizeErrors() { + int randomSize = randomIntBetween(-100000, -2); + IllegalArgumentException expected = expectThrows(IllegalArgumentException.class, + () -> new SearchSourceBuilder().size(randomSize)); + assertEquals("[size] parameter cannot be negative, found [" + randomSize + "]", expected.getMessage()); + expected = expectThrows(IllegalArgumentException.class, () -> new SearchSourceBuilder().size(-1)); + assertEquals("[size] parameter cannot be negative, found [-1]", expected.getMessage()); + } + private void assertIndicesBoostParseErrorMessage(String restContent, String expectedErrorMessage) throws IOException { try (XContentParser parser = createParser(JsonXContent.jsonXContent, restContent)) { ParsingException e = expectThrows(ParsingException.class, () -> SearchSourceBuilder.fromXContent(createParseContext(parser))); diff --git a/docs/reference/migration/migrate_6_0.asciidoc b/docs/reference/migration/migrate_6_0.asciidoc index 22a698bd482d8..811439ab8ba62 100644 --- a/docs/reference/migration/migrate_6_0.asciidoc +++ b/docs/reference/migration/migrate_6_0.asciidoc @@ -31,6 +31,7 @@ way to reindex old indices is to use the `reindex` API. * <> * <> * <> +* <> * <> * <> * <> @@ -55,6 +56,8 @@ include::migrate_6_0/mappings.asciidoc[] include::migrate_6_0/docs.asciidoc[] +include::migrate_6_0/reindex.asciidoc[] + include::migrate_6_0/cluster.asciidoc[] include::migrate_6_0/settings.asciidoc[] diff --git a/docs/reference/migration/migrate_6_0/reindex.asciidoc b/docs/reference/migration/migrate_6_0/reindex.asciidoc new file mode 100644 index 0000000000000..d74a5102d91b7 --- /dev/null +++ b/docs/reference/migration/migrate_6_0/reindex.asciidoc @@ -0,0 +1,6 @@ +[[breaking_60_reindex_changes]] +=== Reindex changes + +==== `size` parameter + +The `size` parameter can no longer be explicitly set to `-1`. If all documents are required then the `size` parameter should not be set. \ No newline at end of file diff --git a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/AbstractBulkByQueryRestHandler.java b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/AbstractBulkByQueryRestHandler.java index 64209653c6f4f..e9973c9950053 100644 --- a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/AbstractBulkByQueryRestHandler.java +++ b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/AbstractBulkByQueryRestHandler.java @@ -32,8 +32,6 @@ import java.util.Map; import java.util.function.Consumer; -import static org.elasticsearch.index.reindex.AbstractBulkByScrollRequest.SIZE_ALL_MATCHES; - /** * Rest handler for reindex actions that accepts a search request like Update-By-Query or Delete-By-Query */ @@ -52,7 +50,6 @@ protected void parseInternalRequest(Request internal, RestRequest restRequest, SearchRequest searchRequest = internal.getSearchRequest(); int scrollSize = searchRequest.source().size(); - searchRequest.source().size(SIZE_ALL_MATCHES); try (XContentParser parser = extractRequestSpecificFields(restRequest, bodyConsumers)) { RestSearchAction.parseSearchRequest(searchRequest, restRequest, parser); diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RoundTripTests.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RoundTripTests.java index a65463c5ed5c6..aadab54803c8c 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RoundTripTests.java +++ b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RoundTripTests.java @@ -131,7 +131,9 @@ public void testDeleteByQueryRequest() throws IOException { private void randomRequest(AbstractBulkByScrollRequest request) { request.getSearchRequest().indices("test"); request.getSearchRequest().source().size(between(1, 1000)); - request.setSize(random().nextBoolean() ? between(1, Integer.MAX_VALUE) : -1); + if (randomBoolean()) { + request.setSize(between(1, Integer.MAX_VALUE)); + } request.setAbortOnVersionConflict(random().nextBoolean()); request.setRefresh(rarely()); request.setTimeout(TimeValue.parseTimeValue(randomTimeValue(), null, "test")); diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/20_validation.yml b/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/20_validation.yml index 7527db948422c..9daf1502a362f 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/20_validation.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/20_validation.yml @@ -44,7 +44,7 @@ id: 1 body: { "text": "test" } - do: - catch: /size should be greater than 0 if the request is limited to some number of documents or -1 if it isn't but it was \[-4\]/ + catch: /\[size\] parameter cannot be negative, found \[-4\]/ delete_by_query: index: test size: -4 diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/reindex/20_validation.yml b/modules/reindex/src/test/resources/rest-api-spec/test/reindex/20_validation.yml index c9f441c9cd31d..b64eaac7dec22 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/reindex/20_validation.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/reindex/20_validation.yml @@ -104,7 +104,7 @@ id: 1 body: { "text": "test" } - do: - catch: /size should be greater than 0 if the request is limited to some number of documents or -1 if it isn't but it was \[-4\]/ + catch: /\[size\] parameter cannot be negative, found \[-4\]/ reindex: body: source: diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/20_validation.yml b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/20_validation.yml index be83c0cb9f24f..8f8d492df3aa0 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/20_validation.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/20_validation.yml @@ -21,7 +21,7 @@ id: 1 body: { "text": "test" } - do: - catch: /size should be greater than 0 if the request is limited to some number of documents or -1 if it isn't but it was \[-4\]/ + catch: /\[size\] parameter cannot be negative, found \[-4\]/ update_by_query: index: test size: -4