Skip to content

Commit 2ac38fd

Browse files
Reindex and friends fail on RED shards (#45830)
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. Added the option to explicitly specify `allow_partial_search_results=true` for reindex only (seemed too strange for update/delete by query). Relates #45739 and #42612
1 parent b0054ee commit 2ac38fd

File tree

6 files changed

+48
-24
lines changed

6 files changed

+48
-24
lines changed

modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuilders.java

+5
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,11 @@ static Request initialSearch(SearchRequest searchRequest, BytesReference query,
135135
request.addParameter(storedFieldsParamName, fields.toString());
136136
}
137137

138+
if (remoteVersion.onOrAfter(Version.fromId(6030099))) {
139+
// allow_partial_results introduced in 6.3, running remote reindex against earlier versions still silently discards RED shards.
140+
request.addParameter("allow_partial_search_results", "false");
141+
}
142+
138143
// EMPTY is safe here because we're not calling namedObject
139144
try (XContentBuilder entity = JsonXContent.contentBuilder();
140145
XContentParser queryParser = XContentHelper

modules/reindex/src/test/java/org/elasticsearch/index/reindex/ClientScrollableHitSourceTests.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ private void dotestBasicsWithRetry(int retries, int minFailures, int maxFailures
117117
client.awaitOperation();
118118
++expectedSearchRetries;
119119
}
120-
120+
client.validateRequest(SearchAction.INSTANCE, (SearchRequest r) -> assertTrue(r.allowPartialSearchResults() == Boolean.FALSE));
121121
SearchResponse searchResponse = createSearchResponse();
122122
client.respond(SearchAction.INSTANCE, searchResponse);
123123

modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/RemoteRequestBuildersTests.java

+17
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import static org.elasticsearch.index.reindex.remote.RemoteRequestBuilders.clearScroll;
4242
import static org.elasticsearch.index.reindex.remote.RemoteRequestBuilders.initialSearch;
4343
import static org.elasticsearch.index.reindex.remote.RemoteRequestBuilders.scroll;
44+
import static org.hamcrest.Matchers.contains;
4445
import static org.hamcrest.Matchers.containsString;
4546
import static org.hamcrest.Matchers.either;
4647
import static org.hamcrest.Matchers.empty;
@@ -206,6 +207,22 @@ public void testInitialSearchParamsMisc() {
206207
}
207208
}
208209

210+
public void testInitialSearchDisallowPartialResults() {
211+
final String allowPartialParamName = "allow_partial_search_results";
212+
final int v6_3 = 6030099;
213+
214+
BytesReference query = new BytesArray("{}");
215+
SearchRequest searchRequest = new SearchRequest().source(new SearchSourceBuilder());
216+
217+
Version disallowVersion = Version.fromId(between(v6_3, Version.CURRENT.id));
218+
Map<String, String> params = initialSearch(searchRequest, query, disallowVersion).getParameters();
219+
assertEquals("false", params.get(allowPartialParamName));
220+
221+
Version allowVersion = Version.fromId(between(0, v6_3-1));
222+
params = initialSearch(searchRequest, query, allowVersion).getParameters();
223+
assertThat(params.keySet(), not(contains(allowPartialParamName)));
224+
}
225+
209226
private void assertScroll(Version remoteVersion, Map<String, String> params, TimeValue requested) {
210227
// V_5_0_0
211228
if (remoteVersion.before(Version.fromId(5000099))) {

modules/reindex/src/test/resources/rest-api-spec/test/reindex/35_search_failures.yml

+12-12
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,15 @@
2828
source: throw new IllegalArgumentException("Cats!")
2929
dest:
3030
index: dest
31-
- match: {created: 0}
32-
- match: {updated: 0}
33-
- match: {version_conflicts: 0}
34-
- match: {batches: 0}
35-
- match: {failures.0.shard: 0}
36-
- match: {failures.0.index: source}
37-
- is_true: failures.0.node
38-
- match: {failures.0.reason.type: script_exception}
39-
- match: {failures.0.reason.reason: runtime error}
40-
- match: {failures.0.reason.caused_by.type: illegal_argument_exception}
41-
- match: {failures.0.reason.caused_by.reason: Cats!}
42-
- gte: { took: 0 }
31+
- match: {error.type: search_phase_execution_exception}
32+
- match: {error.reason: "Partial shards failure"}
33+
- match: {error.phase: query}
34+
- match: {error.root_cause.0.type: script_exception}
35+
- match: {error.root_cause.0.reason: runtime error}
36+
- match: {error.failed_shards.0.shard: 0}
37+
- match: {error.failed_shards.0.index: source}
38+
- is_true: error.failed_shards.0.node
39+
- match: {error.failed_shards.0.reason.type: script_exception}
40+
- match: {error.failed_shards.0.reason.reason: runtime error}
41+
- match: {error.failed_shards.0.reason.caused_by.type: illegal_argument_exception}
42+
- match: {error.failed_shards.0.reason.caused_by.reason: Cats!}

modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/35_search_failure.yml

+12-11
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,15 @@
2525
script:
2626
lang: painless
2727
source: throw new IllegalArgumentException("Cats!")
28-
- match: {updated: 0}
29-
- match: {version_conflicts: 0}
30-
- match: {batches: 0}
31-
- match: {failures.0.shard: 0}
32-
- match: {failures.0.index: source}
33-
- is_true: failures.0.node
34-
- match: {failures.0.reason.type: script_exception}
35-
- match: {failures.0.reason.reason: runtime error}
36-
- match: {failures.0.reason.caused_by.type: illegal_argument_exception}
37-
- match: {failures.0.reason.caused_by.reason: Cats!}
38-
- gte: { took: 0 }
28+
- match: {error.type: search_phase_execution_exception}
29+
- match: {error.reason: "Partial shards failure"}
30+
- match: {error.phase: query}
31+
- match: {error.root_cause.0.type: script_exception}
32+
- match: {error.root_cause.0.reason: runtime error}
33+
- match: {error.failed_shards.0.shard: 0}
34+
- match: {error.failed_shards.0.index: source}
35+
- is_true: error.failed_shards.0.node
36+
- match: {error.failed_shards.0.reason.type: script_exception}
37+
- match: {error.failed_shards.0.reason.reason: runtime error}
38+
- match: {error.failed_shards.0.reason.caused_by.type: illegal_argument_exception}
39+
- match: {error.failed_shards.0.reason.caused_by.reason: Cats!}

server/src/main/java/org/elasticsearch/index/reindex/ClientScrollableHitSource.java

+1
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ public ClientScrollableHitSource(Logger logger, BackoffPolicy backoffPolicy, Thr
6464
super(logger, backoffPolicy, threadPool, countSearchRetry, onResponse, fail);
6565
this.client = client;
6666
this.firstSearchRequest = firstSearchRequest;
67+
firstSearchRequest.allowPartialSearchResults(false);
6768
}
6869

6970
@Override

0 commit comments

Comments
 (0)