From 3a5853e0ba78316741fc0f5e44cddfcc8f4ef0e1 Mon Sep 17 00:00:00 2001 From: Henning Andersen Date: Thu, 22 Aug 2019 12:00:11 +0200 Subject: [PATCH 1/3] Reindex and friends fail on RED shards Reindex, update by query and delete by query would silently disregard RED/unavailable shards, thus not copying, updating or deleting matching data in those shards. Now use `allow_partial_search_results=false` to ensure these operations fail if the search crosses an unavailable chard. Relates #45739 and #42612 --- .../reindex/remote/RemoteRequestBuilders.java | 5 ++++ .../ClientScrollableHitSourceTests.java | 2 +- .../remote/RemoteRequestBuildersTests.java | 17 +++++++++++++ .../test/reindex/35_search_failures.yml | 24 +++++++++---------- .../update_by_query/35_search_failure.yml | 23 +++++++++--------- .../reindex/ClientScrollableHitSource.java | 1 + 6 files changed, 48 insertions(+), 24 deletions(-) diff --git a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuilders.java b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuilders.java index 40c4ba757d176..61204c1a0c2a7 100644 --- a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuilders.java +++ b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuilders.java @@ -125,6 +125,11 @@ static Request initialSearch(SearchRequest searchRequest, BytesReference query, request.addParameter(storedFieldsParamName, fields.toString()); } + if (remoteVersion.onOrAfter(Version.fromId(6030099))) { + // allow_partial_results introduced in 6.3, running remote reindex against earlier versions still silently discards RED shards. + request.addParameter("allow_partial_search_results", "false"); + } + // EMPTY is safe here because we're not calling namedObject try (XContentBuilder entity = JsonXContent.contentBuilder(); XContentParser queryParser = XContentHelper diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ClientScrollableHitSourceTests.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ClientScrollableHitSourceTests.java index 37425a7c600ef..6526f608109b3 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ClientScrollableHitSourceTests.java +++ b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ClientScrollableHitSourceTests.java @@ -117,7 +117,7 @@ private void dotestBasicsWithRetry(int retries, int minFailures, int maxFailures client.awaitOperation(); ++expectedSearchRetries; } - + client.validateRequest(SearchAction.INSTANCE, (SearchRequest r) -> assertTrue(r.allowPartialSearchResults() == Boolean.FALSE)); SearchResponse searchResponse = createSearchResponse(); client.respond(SearchAction.INSTANCE, searchResponse); diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuildersTests.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuildersTests.java index 0005fa921b33b..cc31c9bd2081d 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuildersTests.java +++ b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuildersTests.java @@ -40,6 +40,7 @@ import static org.elasticsearch.index.reindex.remote.RemoteRequestBuilders.clearScroll; import static org.elasticsearch.index.reindex.remote.RemoteRequestBuilders.initialSearch; import static org.elasticsearch.index.reindex.remote.RemoteRequestBuilders.scroll; +import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.either; import static org.hamcrest.Matchers.empty; @@ -178,6 +179,22 @@ public void testInitialSearchParamsMisc() { } } + public void testInitialSearchDisallowPartialResults() { + final String allowPartialParamName = "allow_partial_search_results"; + final int v6_3 = 6030099; + + BytesReference query = new BytesArray("{}"); + SearchRequest searchRequest = new SearchRequest().source(new SearchSourceBuilder()); + + Version disallowVersion = Version.fromId(between(v6_3, Version.CURRENT.id)); + Map params = initialSearch(searchRequest, query, disallowVersion).getParameters(); + assertEquals("false", params.get(allowPartialParamName)); + + Version allowVersion = Version.fromId(between(0, v6_3-1)); + params = initialSearch(searchRequest, query, allowVersion).getParameters(); + assertThat(params.keySet(), not(contains(allowPartialParamName))); + } + private void assertScroll(Version remoteVersion, Map params, TimeValue requested) { // V_5_0_0 if (remoteVersion.before(Version.fromId(5000099))) { diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/reindex/35_search_failures.yml b/modules/reindex/src/test/resources/rest-api-spec/test/reindex/35_search_failures.yml index 5fd888f77a119..44b36119fbc76 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/reindex/35_search_failures.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/reindex/35_search_failures.yml @@ -28,15 +28,15 @@ source: throw new IllegalArgumentException("Cats!") dest: index: dest - - match: {created: 0} - - match: {updated: 0} - - match: {version_conflicts: 0} - - match: {batches: 0} - - match: {failures.0.shard: 0} - - match: {failures.0.index: source} - - is_true: failures.0.node - - match: {failures.0.reason.type: script_exception} - - match: {failures.0.reason.reason: runtime error} - - match: {failures.0.reason.caused_by.type: illegal_argument_exception} - - match: {failures.0.reason.caused_by.reason: Cats!} - - gte: { took: 0 } + - match: {error.type: search_phase_execution_exception} + - match: {error.reason: "Partial shards failure"} + - match: {error.phase: query} + - match: {error.root_cause.0.type: script_exception} + - match: {error.root_cause.0.reason: runtime error} + - match: {error.failed_shards.0.shard: 0} + - match: {error.failed_shards.0.index: source} + - is_true: error.failed_shards.0.node + - match: {error.failed_shards.0.reason.type: script_exception} + - match: {error.failed_shards.0.reason.reason: runtime error} + - match: {error.failed_shards.0.reason.caused_by.type: illegal_argument_exception} + - match: {error.failed_shards.0.reason.caused_by.reason: Cats!} diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/35_search_failure.yml b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/35_search_failure.yml index da94187f50afa..5a22eec88c0f7 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/35_search_failure.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/35_search_failure.yml @@ -25,14 +25,15 @@ script: lang: painless source: throw new IllegalArgumentException("Cats!") - - match: {updated: 0} - - match: {version_conflicts: 0} - - match: {batches: 0} - - match: {failures.0.shard: 0} - - match: {failures.0.index: source} - - is_true: failures.0.node - - match: {failures.0.reason.type: script_exception} - - match: {failures.0.reason.reason: runtime error} - - match: {failures.0.reason.caused_by.type: illegal_argument_exception} - - match: {failures.0.reason.caused_by.reason: Cats!} - - gte: { took: 0 } + - match: {error.type: search_phase_execution_exception} + - match: {error.reason: "Partial shards failure"} + - match: {error.phase: query} + - match: {error.root_cause.0.type: script_exception} + - match: {error.root_cause.0.reason: runtime error} + - match: {error.failed_shards.0.shard: 0} + - match: {error.failed_shards.0.index: source} + - is_true: error.failed_shards.0.node + - match: {error.failed_shards.0.reason.type: script_exception} + - match: {error.failed_shards.0.reason.reason: runtime error} + - match: {error.failed_shards.0.reason.caused_by.type: illegal_argument_exception} + - match: {error.failed_shards.0.reason.caused_by.reason: Cats!} diff --git a/server/src/main/java/org/elasticsearch/index/reindex/ClientScrollableHitSource.java b/server/src/main/java/org/elasticsearch/index/reindex/ClientScrollableHitSource.java index 04bfb1c36adfc..de65023a37bf7 100644 --- a/server/src/main/java/org/elasticsearch/index/reindex/ClientScrollableHitSource.java +++ b/server/src/main/java/org/elasticsearch/index/reindex/ClientScrollableHitSource.java @@ -64,6 +64,7 @@ public ClientScrollableHitSource(Logger logger, BackoffPolicy backoffPolicy, Thr super(logger, backoffPolicy, threadPool, countSearchRetry, onResponse, fail); this.client = client; this.firstSearchRequest = firstSearchRequest; + firstSearchRequest.allowPartialSearchResults(false); } @Override From 9cb38aec1c2af800b4ac04f051c9531400264918 Mon Sep 17 00:00:00 2001 From: Henning Andersen Date: Mon, 9 Sep 2019 13:26:01 +0200 Subject: [PATCH 2/3] Add ability to reindex RED shards --- docs/reference/docs/reindex.asciidoc | 21 +++++++++ .../index/reindex/RestReindexAction.java | 5 ++ .../reindex/remote/RemoteRequestBuilders.java | 8 +++- .../ClientScrollableHitSourceTests.java | 12 ++++- .../remote/RemoteRequestBuildersTests.java | 16 +++++++ .../test/reindex/35_search_failures.yml | 46 ++++++++++++++++++- .../resources/rest-api-spec/api/reindex.json | 5 ++ .../reindex/ClientScrollableHitSource.java | 2 +- 8 files changed, 109 insertions(+), 6 deletions(-) diff --git a/docs/reference/docs/reindex.asciidoc b/docs/reference/docs/reindex.asciidoc index f9f33beb21029..28676ecaec644 100644 --- a/docs/reference/docs/reindex.asciidoc +++ b/docs/reference/docs/reindex.asciidoc @@ -1255,3 +1255,24 @@ POST _reindex <1> `_reindex` defaults to sorting by `_doc` so `random_score` will not have any effect unless you override the sort to `_score`. + +[float] +==== Reindexing RED shards + +`_reindex` by default fails if a shard is unavailable (red). It is possible to +reindex only the available shards using: + +[source,js] +---------------------------------------------------------------- +POST _reindex?allow_partial_search_results=true +{ + "source": { + "index": "twitter" + }, + "dest": { + "index": "red_twitter" + } +} +---------------------------------------------------------------- +// CONSOLE +// TEST[setup:big_twitter] diff --git a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/RestReindexAction.java b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/RestReindexAction.java index 516d47b835803..79fcfc8338420 100644 --- a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/RestReindexAction.java +++ b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/RestReindexAction.java @@ -64,6 +64,11 @@ protected ReindexRequest buildRequest(RestRequest request) throws IOException { if (request.hasParam("scroll")) { internal.setScroll(parseTimeValue(request.param("scroll"), "scroll")); } + + if (request.hasParam("allow_partial_search_results")) { + internal.getSearchRequest().allowPartialSearchResults(request.paramAsBoolean("allow_partial_search_results",false)); + } + return internal; } } diff --git a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuilders.java b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuilders.java index 61204c1a0c2a7..95e200572a016 100644 --- a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuilders.java +++ b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuilders.java @@ -125,9 +125,13 @@ static Request initialSearch(SearchRequest searchRequest, BytesReference query, request.addParameter(storedFieldsParamName, fields.toString()); } + // allow_partial_results introduced in 6.3, running remote reindex against earlier versions still silently discards RED shards. if (remoteVersion.onOrAfter(Version.fromId(6030099))) { - // allow_partial_results introduced in 6.3, running remote reindex against earlier versions still silently discards RED shards. - request.addParameter("allow_partial_search_results", "false"); + boolean allowPartialSearchResults = searchRequest.allowPartialSearchResults() == Boolean.TRUE; + // be explicit always from 7.5 to stop relying on the default. + if (allowPartialSearchResults == false || remoteVersion.onOrAfter(Version.V_7_5_0)) { + request.addParameter("allow_partial_search_results", Boolean.toString(allowPartialSearchResults)); + } } // EMPTY is safe here because we're not calling namedObject diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ClientScrollableHitSourceTests.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ClientScrollableHitSourceTests.java index 6526f608109b3..418c2ef7c5a2f 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ClientScrollableHitSourceTests.java +++ b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ClientScrollableHitSourceTests.java @@ -106,10 +106,15 @@ private void dotestBasicsWithRetry(int retries, int minFailures, int maxFailures TaskId parentTask = new TaskId("thenode", randomInt()); AtomicInteger actualSearchRetries = new AtomicInteger(); int expectedSearchRetries = 0; + SearchRequest searchRequest = new SearchRequest().scroll("1m"); + boolean allowPartialSearchResults = randomBoolean(); + if (allowPartialSearchResults || randomBoolean()) { + searchRequest.allowPartialSearchResults(allowPartialSearchResults); + } ClientScrollableHitSource hitSource = new ClientScrollableHitSource(logger, BackoffPolicy.constantBackoff(TimeValue.ZERO, retries), threadPool, actualSearchRetries::incrementAndGet, responses::add, failureHandler, new ParentTaskAssigningClient(client, parentTask), - new SearchRequest().scroll("1m")); + searchRequest); hitSource.start(); for (int retry = 0; retry < randomIntBetween(minFailures, maxFailures); ++retry) { @@ -117,7 +122,10 @@ private void dotestBasicsWithRetry(int retries, int minFailures, int maxFailures client.awaitOperation(); ++expectedSearchRetries; } - client.validateRequest(SearchAction.INSTANCE, (SearchRequest r) -> assertTrue(r.allowPartialSearchResults() == Boolean.FALSE)); + + // we explicitly set it on 7.5+, to not rely on the default anymore. + client.validateRequest(SearchAction.INSTANCE, + (SearchRequest r) -> assertTrue(r.allowPartialSearchResults() == allowPartialSearchResults)); SearchResponse searchResponse = createSearchResponse(); client.respond(SearchAction.INSTANCE, searchResponse); diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuildersTests.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuildersTests.java index cc31c9bd2081d..3e53f52d44eff 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuildersTests.java +++ b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuildersTests.java @@ -195,6 +195,22 @@ public void testInitialSearchDisallowPartialResults() { assertThat(params.keySet(), not(contains(allowPartialParamName))); } + public void testInitialSearchExplicitlyAllowPartialResults() { + final String allowPartialParamName = "allow_partial_search_results"; + + BytesReference query = new BytesArray("{}"); + SearchRequest searchRequest = new SearchRequest().source(new SearchSourceBuilder()).allowPartialSearchResults(true); + + // we explicitly set it against 7.5+, to not rely on the default anymore. + Version allowVersion = Version.fromId(between(Version.V_7_5_0.id, Version.CURRENT.id)); + Map params = initialSearch(searchRequest, query, allowVersion).getParameters(); + assertEquals("true", params.get(allowPartialParamName)); + + Version notSetVersion = Version.fromId(between(0, Version.V_7_5_0.id-1)); + params = initialSearch(searchRequest, query, notSetVersion).getParameters(); + assertThat(params.keySet(), not(contains(allowPartialParamName))); + } + private void assertScroll(Version remoteVersion, Map params, TimeValue requested) { // V_5_0_0 if (remoteVersion.before(Version.fromId(5000099))) { diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/reindex/35_search_failures.yml b/modules/reindex/src/test/resources/rest-api-spec/test/reindex/35_search_failures.yml index 44b36119fbc76..c9084624cc317 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/reindex/35_search_failures.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/reindex/35_search_failures.yml @@ -1,5 +1,49 @@ --- -"Response format for search failures": +"Response format for search failures with allow_partial_search_results=true": + - do: + indices.create: + index: source + body: + settings: + index.number_of_shards: 2 + + - do: + index: + index: source + id: 1 + body: { "text": "test" } + - do: + indices.refresh: {} + + - do: + catch: bad_request + reindex: + allow_partial_search_results: true + body: + source: + index: source + query: + script: + script: + lang: painless + source: throw new IllegalArgumentException("Cats!") + dest: + index: dest + - match: {created: 0} + - match: {updated: 0} + - match: {version_conflicts: 0} + - match: {batches: 0} + - match: {failures.0.shard: 0} + - match: {failures.0.index: source} + - is_true: failures.0.node + - match: {failures.0.reason.type: script_exception} + - match: {failures.0.reason.reason: runtime error} + - match: {failures.0.reason.caused_by.type: illegal_argument_exception} + - match: {failures.0.reason.caused_by.reason: Cats!} + - gte: { took: 0 } + +--- +"Response format for search failures normal": - do: indices.create: index: source diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/reindex.json b/rest-api-spec/src/main/resources/rest-api-spec/api/reindex.json index 72f7b0670bc51..272f4341944f7 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/reindex.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/reindex.json @@ -52,6 +52,11 @@ "max_docs":{ "type":"number", "description":"Maximum number of documents to process (default: all documents)" + }, + "allow_partial_search_results":{ + "type":"boolean", + "default":false, + "description":"Indicate if an error should be returned if there is a partial search failure or timeout" } }, "body":{ diff --git a/server/src/main/java/org/elasticsearch/index/reindex/ClientScrollableHitSource.java b/server/src/main/java/org/elasticsearch/index/reindex/ClientScrollableHitSource.java index de65023a37bf7..f858d858c0777 100644 --- a/server/src/main/java/org/elasticsearch/index/reindex/ClientScrollableHitSource.java +++ b/server/src/main/java/org/elasticsearch/index/reindex/ClientScrollableHitSource.java @@ -64,7 +64,7 @@ public ClientScrollableHitSource(Logger logger, BackoffPolicy backoffPolicy, Thr super(logger, backoffPolicy, threadPool, countSearchRetry, onResponse, fail); this.client = client; this.firstSearchRequest = firstSearchRequest; - firstSearchRequest.allowPartialSearchResults(false); + firstSearchRequest.allowPartialSearchResults(firstSearchRequest.allowPartialSearchResults() == Boolean.TRUE); } @Override From 633eafede32f8f1bec3330f1d09a4d0adfefba7c Mon Sep 17 00:00:00 2001 From: Henning Andersen Date: Wed, 11 Sep 2019 21:04:11 +0200 Subject: [PATCH 3/3] Revert "Add ability to reindex RED shards" This reverts commit 9cb38aec1c2af800b4ac04f051c9531400264918. --- docs/reference/docs/reindex.asciidoc | 21 --------- .../index/reindex/RestReindexAction.java | 5 -- .../reindex/remote/RemoteRequestBuilders.java | 8 +--- .../ClientScrollableHitSourceTests.java | 12 +---- .../remote/RemoteRequestBuildersTests.java | 16 ------- .../test/reindex/35_search_failures.yml | 46 +------------------ .../resources/rest-api-spec/api/reindex.json | 5 -- .../reindex/ClientScrollableHitSource.java | 2 +- 8 files changed, 6 insertions(+), 109 deletions(-) diff --git a/docs/reference/docs/reindex.asciidoc b/docs/reference/docs/reindex.asciidoc index 28676ecaec644..f9f33beb21029 100644 --- a/docs/reference/docs/reindex.asciidoc +++ b/docs/reference/docs/reindex.asciidoc @@ -1255,24 +1255,3 @@ POST _reindex <1> `_reindex` defaults to sorting by `_doc` so `random_score` will not have any effect unless you override the sort to `_score`. - -[float] -==== Reindexing RED shards - -`_reindex` by default fails if a shard is unavailable (red). It is possible to -reindex only the available shards using: - -[source,js] ----------------------------------------------------------------- -POST _reindex?allow_partial_search_results=true -{ - "source": { - "index": "twitter" - }, - "dest": { - "index": "red_twitter" - } -} ----------------------------------------------------------------- -// CONSOLE -// TEST[setup:big_twitter] diff --git a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/RestReindexAction.java b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/RestReindexAction.java index 79fcfc8338420..516d47b835803 100644 --- a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/RestReindexAction.java +++ b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/RestReindexAction.java @@ -64,11 +64,6 @@ protected ReindexRequest buildRequest(RestRequest request) throws IOException { if (request.hasParam("scroll")) { internal.setScroll(parseTimeValue(request.param("scroll"), "scroll")); } - - if (request.hasParam("allow_partial_search_results")) { - internal.getSearchRequest().allowPartialSearchResults(request.paramAsBoolean("allow_partial_search_results",false)); - } - return internal; } } diff --git a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuilders.java b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuilders.java index 95e200572a016..61204c1a0c2a7 100644 --- a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuilders.java +++ b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuilders.java @@ -125,13 +125,9 @@ static Request initialSearch(SearchRequest searchRequest, BytesReference query, request.addParameter(storedFieldsParamName, fields.toString()); } - // allow_partial_results introduced in 6.3, running remote reindex against earlier versions still silently discards RED shards. if (remoteVersion.onOrAfter(Version.fromId(6030099))) { - boolean allowPartialSearchResults = searchRequest.allowPartialSearchResults() == Boolean.TRUE; - // be explicit always from 7.5 to stop relying on the default. - if (allowPartialSearchResults == false || remoteVersion.onOrAfter(Version.V_7_5_0)) { - request.addParameter("allow_partial_search_results", Boolean.toString(allowPartialSearchResults)); - } + // allow_partial_results introduced in 6.3, running remote reindex against earlier versions still silently discards RED shards. + request.addParameter("allow_partial_search_results", "false"); } // EMPTY is safe here because we're not calling namedObject diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ClientScrollableHitSourceTests.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ClientScrollableHitSourceTests.java index 418c2ef7c5a2f..6526f608109b3 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ClientScrollableHitSourceTests.java +++ b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ClientScrollableHitSourceTests.java @@ -106,15 +106,10 @@ private void dotestBasicsWithRetry(int retries, int minFailures, int maxFailures TaskId parentTask = new TaskId("thenode", randomInt()); AtomicInteger actualSearchRetries = new AtomicInteger(); int expectedSearchRetries = 0; - SearchRequest searchRequest = new SearchRequest().scroll("1m"); - boolean allowPartialSearchResults = randomBoolean(); - if (allowPartialSearchResults || randomBoolean()) { - searchRequest.allowPartialSearchResults(allowPartialSearchResults); - } ClientScrollableHitSource hitSource = new ClientScrollableHitSource(logger, BackoffPolicy.constantBackoff(TimeValue.ZERO, retries), threadPool, actualSearchRetries::incrementAndGet, responses::add, failureHandler, new ParentTaskAssigningClient(client, parentTask), - searchRequest); + new SearchRequest().scroll("1m")); hitSource.start(); for (int retry = 0; retry < randomIntBetween(minFailures, maxFailures); ++retry) { @@ -122,10 +117,7 @@ private void dotestBasicsWithRetry(int retries, int minFailures, int maxFailures client.awaitOperation(); ++expectedSearchRetries; } - - // we explicitly set it on 7.5+, to not rely on the default anymore. - client.validateRequest(SearchAction.INSTANCE, - (SearchRequest r) -> assertTrue(r.allowPartialSearchResults() == allowPartialSearchResults)); + client.validateRequest(SearchAction.INSTANCE, (SearchRequest r) -> assertTrue(r.allowPartialSearchResults() == Boolean.FALSE)); SearchResponse searchResponse = createSearchResponse(); client.respond(SearchAction.INSTANCE, searchResponse); diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuildersTests.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuildersTests.java index 3e53f52d44eff..cc31c9bd2081d 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuildersTests.java +++ b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuildersTests.java @@ -195,22 +195,6 @@ public void testInitialSearchDisallowPartialResults() { assertThat(params.keySet(), not(contains(allowPartialParamName))); } - public void testInitialSearchExplicitlyAllowPartialResults() { - final String allowPartialParamName = "allow_partial_search_results"; - - BytesReference query = new BytesArray("{}"); - SearchRequest searchRequest = new SearchRequest().source(new SearchSourceBuilder()).allowPartialSearchResults(true); - - // we explicitly set it against 7.5+, to not rely on the default anymore. - Version allowVersion = Version.fromId(between(Version.V_7_5_0.id, Version.CURRENT.id)); - Map params = initialSearch(searchRequest, query, allowVersion).getParameters(); - assertEquals("true", params.get(allowPartialParamName)); - - Version notSetVersion = Version.fromId(between(0, Version.V_7_5_0.id-1)); - params = initialSearch(searchRequest, query, notSetVersion).getParameters(); - assertThat(params.keySet(), not(contains(allowPartialParamName))); - } - private void assertScroll(Version remoteVersion, Map params, TimeValue requested) { // V_5_0_0 if (remoteVersion.before(Version.fromId(5000099))) { diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/reindex/35_search_failures.yml b/modules/reindex/src/test/resources/rest-api-spec/test/reindex/35_search_failures.yml index c9084624cc317..44b36119fbc76 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/reindex/35_search_failures.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/reindex/35_search_failures.yml @@ -1,49 +1,5 @@ --- -"Response format for search failures with allow_partial_search_results=true": - - do: - indices.create: - index: source - body: - settings: - index.number_of_shards: 2 - - - do: - index: - index: source - id: 1 - body: { "text": "test" } - - do: - indices.refresh: {} - - - do: - catch: bad_request - reindex: - allow_partial_search_results: true - body: - source: - index: source - query: - script: - script: - lang: painless - source: throw new IllegalArgumentException("Cats!") - dest: - index: dest - - match: {created: 0} - - match: {updated: 0} - - match: {version_conflicts: 0} - - match: {batches: 0} - - match: {failures.0.shard: 0} - - match: {failures.0.index: source} - - is_true: failures.0.node - - match: {failures.0.reason.type: script_exception} - - match: {failures.0.reason.reason: runtime error} - - match: {failures.0.reason.caused_by.type: illegal_argument_exception} - - match: {failures.0.reason.caused_by.reason: Cats!} - - gte: { took: 0 } - ---- -"Response format for search failures normal": +"Response format for search failures": - do: indices.create: index: source diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/reindex.json b/rest-api-spec/src/main/resources/rest-api-spec/api/reindex.json index 272f4341944f7..72f7b0670bc51 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/reindex.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/reindex.json @@ -52,11 +52,6 @@ "max_docs":{ "type":"number", "description":"Maximum number of documents to process (default: all documents)" - }, - "allow_partial_search_results":{ - "type":"boolean", - "default":false, - "description":"Indicate if an error should be returned if there is a partial search failure or timeout" } }, "body":{ diff --git a/server/src/main/java/org/elasticsearch/index/reindex/ClientScrollableHitSource.java b/server/src/main/java/org/elasticsearch/index/reindex/ClientScrollableHitSource.java index f858d858c0777..de65023a37bf7 100644 --- a/server/src/main/java/org/elasticsearch/index/reindex/ClientScrollableHitSource.java +++ b/server/src/main/java/org/elasticsearch/index/reindex/ClientScrollableHitSource.java @@ -64,7 +64,7 @@ public ClientScrollableHitSource(Logger logger, BackoffPolicy backoffPolicy, Thr super(logger, backoffPolicy, threadPool, countSearchRetry, onResponse, fail); this.client = client; this.firstSearchRequest = firstSearchRequest; - firstSearchRequest.allowPartialSearchResults(firstSearchRequest.allowPartialSearchResults() == Boolean.TRUE); + firstSearchRequest.allowPartialSearchResults(false); } @Override