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
This commit is contained in:
Henning Andersen 2019-11-18 21:22:36 +01:00 committed by Henning Andersen
parent b0054eecd6
commit 2ac38fd315
6 changed files with 48 additions and 24 deletions

View File

@ -135,6 +135,11 @@ final class RemoteRequestBuilders {
request.addParameter(storedFieldsParamName, fields.toString()); 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 // EMPTY is safe here because we're not calling namedObject
try (XContentBuilder entity = JsonXContent.contentBuilder(); try (XContentBuilder entity = JsonXContent.contentBuilder();
XContentParser queryParser = XContentHelper XContentParser queryParser = XContentHelper

View File

@ -117,7 +117,7 @@ public class ClientScrollableHitSourceTests extends ESTestCase {
client.awaitOperation(); client.awaitOperation();
++expectedSearchRetries; ++expectedSearchRetries;
} }
client.validateRequest(SearchAction.INSTANCE, (SearchRequest r) -> assertTrue(r.allowPartialSearchResults() == Boolean.FALSE));
SearchResponse searchResponse = createSearchResponse(); SearchResponse searchResponse = createSearchResponse();
client.respond(SearchAction.INSTANCE, searchResponse); client.respond(SearchAction.INSTANCE, searchResponse);

View File

@ -41,6 +41,7 @@ import static org.elasticsearch.index.reindex.remote.RemoteRequestBuilders.DEPRE
import static org.elasticsearch.index.reindex.remote.RemoteRequestBuilders.clearScroll; 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.initialSearch;
import static org.elasticsearch.index.reindex.remote.RemoteRequestBuilders.scroll; 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.containsString;
import static org.hamcrest.Matchers.either; import static org.hamcrest.Matchers.either;
import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.empty;
@ -206,6 +207,22 @@ public class RemoteRequestBuildersTests extends ESTestCase {
} }
} }
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<String, String> 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<String, String> params, TimeValue requested) { private void assertScroll(Version remoteVersion, Map<String, String> params, TimeValue requested) {
// V_5_0_0 // V_5_0_0
if (remoteVersion.before(Version.fromId(5000099))) { if (remoteVersion.before(Version.fromId(5000099))) {

View File

@ -28,15 +28,15 @@
source: throw new IllegalArgumentException("Cats!") source: throw new IllegalArgumentException("Cats!")
dest: dest:
index: dest index: dest
- match: {created: 0} - match: {error.type: search_phase_execution_exception}
- match: {updated: 0} - match: {error.reason: "Partial shards failure"}
- match: {version_conflicts: 0} - match: {error.phase: query}
- match: {batches: 0} - match: {error.root_cause.0.type: script_exception}
- match: {failures.0.shard: 0} - match: {error.root_cause.0.reason: runtime error}
- match: {failures.0.index: source} - match: {error.failed_shards.0.shard: 0}
- is_true: failures.0.node - match: {error.failed_shards.0.index: source}
- match: {failures.0.reason.type: script_exception} - is_true: error.failed_shards.0.node
- match: {failures.0.reason.reason: runtime error} - match: {error.failed_shards.0.reason.type: script_exception}
- match: {failures.0.reason.caused_by.type: illegal_argument_exception} - match: {error.failed_shards.0.reason.reason: runtime error}
- match: {failures.0.reason.caused_by.reason: Cats!} - match: {error.failed_shards.0.reason.caused_by.type: illegal_argument_exception}
- gte: { took: 0 } - match: {error.failed_shards.0.reason.caused_by.reason: Cats!}

View File

@ -25,14 +25,15 @@
script: script:
lang: painless lang: painless
source: throw new IllegalArgumentException("Cats!") source: throw new IllegalArgumentException("Cats!")
- match: {updated: 0} - match: {error.type: search_phase_execution_exception}
- match: {version_conflicts: 0} - match: {error.reason: "Partial shards failure"}
- match: {batches: 0} - match: {error.phase: query}
- match: {failures.0.shard: 0} - match: {error.root_cause.0.type: script_exception}
- match: {failures.0.index: source} - match: {error.root_cause.0.reason: runtime error}
- is_true: failures.0.node - match: {error.failed_shards.0.shard: 0}
- match: {failures.0.reason.type: script_exception} - match: {error.failed_shards.0.index: source}
- match: {failures.0.reason.reason: runtime error} - is_true: error.failed_shards.0.node
- match: {failures.0.reason.caused_by.type: illegal_argument_exception} - match: {error.failed_shards.0.reason.type: script_exception}
- match: {failures.0.reason.caused_by.reason: Cats!} - match: {error.failed_shards.0.reason.reason: runtime error}
- gte: { took: 0 } - match: {error.failed_shards.0.reason.caused_by.type: illegal_argument_exception}
- match: {error.failed_shards.0.reason.caused_by.reason: Cats!}

View File

@ -64,6 +64,7 @@ public class ClientScrollableHitSource extends ScrollableHitSource {
super(logger, backoffPolicy, threadPool, countSearchRetry, onResponse, fail); super(logger, backoffPolicy, threadPool, countSearchRetry, onResponse, fail);
this.client = client; this.client = client;
this.firstSearchRequest = firstSearchRequest; this.firstSearchRequest = firstSearchRequest;
firstSearchRequest.allowPartialSearchResults(false);
} }
@Override @Override