Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose group_shard_failures query_string parameter #32598

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,12 @@
import org.elasticsearch.action.admin.cluster.settings.ClusterGetSettingsRequest;
import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest;
import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotRequest;
import org.elasticsearch.action.admin.cluster.snapshots.delete.DeleteSnapshotRequest;
import org.elasticsearch.action.admin.cluster.snapshots.get.GetSnapshotsRequest;
import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotRequest;
import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotsStatusRequest;
import org.elasticsearch.action.admin.cluster.storedscripts.DeleteStoredScriptRequest;
import org.elasticsearch.action.admin.cluster.storedscripts.GetStoredScriptRequest;
import org.elasticsearch.action.admin.cluster.snapshots.delete.DeleteSnapshotRequest;
import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotsStatusRequest;
import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest;
import org.elasticsearch.action.admin.indices.alias.get.GetAliasesRequest;
import org.elasticsearch.action.admin.indices.analyze.AnalyzeRequest;
Expand Down Expand Up @@ -78,8 +78,8 @@
import org.elasticsearch.action.index.IndexRequest;
import org.elasticsearch.action.ingest.DeletePipelineRequest;
import org.elasticsearch.action.ingest.GetPipelineRequest;
import org.elasticsearch.action.ingest.SimulatePipelineRequest;
import org.elasticsearch.action.ingest.PutPipelineRequest;
import org.elasticsearch.action.ingest.SimulatePipelineRequest;
import org.elasticsearch.action.search.ClearScrollRequest;
import org.elasticsearch.action.search.MultiSearchRequest;
import org.elasticsearch.action.search.SearchRequest;
Expand Down Expand Up @@ -107,10 +107,10 @@
import org.elasticsearch.index.VersionType;
import org.elasticsearch.index.rankeval.RankEvalRequest;
import org.elasticsearch.protocol.xpack.XPackInfoRequest;
import org.elasticsearch.protocol.xpack.watcher.DeleteWatchRequest;
import org.elasticsearch.protocol.xpack.watcher.PutWatchRequest;
import org.elasticsearch.protocol.xpack.XPackUsageRequest;
import org.elasticsearch.protocol.xpack.license.PutLicenseRequest;
import org.elasticsearch.protocol.xpack.watcher.DeleteWatchRequest;
import org.elasticsearch.protocol.xpack.watcher.PutWatchRequest;
import org.elasticsearch.rest.action.search.RestSearchAction;
import org.elasticsearch.script.mustache.MultiSearchTemplateRequest;
import org.elasticsearch.script.mustache.SearchTemplateRequest;
Expand Down Expand Up @@ -556,6 +556,7 @@ static Request search(SearchRequest searchRequest) throws IOException {

private static void addSearchRequestParams(Params params, SearchRequest searchRequest) {
params.putParam(RestSearchAction.TYPED_KEYS_PARAM, "true");
params.withGroupShardFailures(searchRequest.groupShardFailures());
params.withRouting(searchRequest.routing());
params.withPreference(searchRequest.preference());
params.withIndicesOptions(searchRequest.indicesOptions());
Expand Down Expand Up @@ -589,6 +590,7 @@ static Request multiSearch(MultiSearchRequest multiSearchRequest) throws IOExcep

Params params = new Params(request);
params.putParam(RestSearchAction.TYPED_KEYS_PARAM, "true");
params.withGroupShardFailures(multiSearchRequest.groupShardFailures());
if (multiSearchRequest.maxConcurrentSearchRequests() != MultiSearchRequest.MAX_CONCURRENT_SEARCH_REQUESTS_DEFAULT) {
params.putParam("max_concurrent_searches", Integer.toString(multiSearchRequest.maxConcurrentSearchRequests()));
}
Expand Down Expand Up @@ -622,6 +624,7 @@ static Request multiSearchTemplate(MultiSearchTemplateRequest multiSearchTemplat

Params params = new Params(request);
params.putParam(RestSearchAction.TYPED_KEYS_PARAM, "true");
params.withGroupShardFailures(multiSearchTemplateRequest.groupShardFailures());
if (multiSearchTemplateRequest.maxConcurrentSearchRequests() != MultiSearchRequest.MAX_CONCURRENT_SEARCH_REQUESTS_DEFAULT) {
params.putParam("max_concurrent_searches", Integer.toString(multiSearchTemplateRequest.maxConcurrentSearchRequests()));
}
Expand Down Expand Up @@ -1309,6 +1312,10 @@ Params withRouting(String routing) {
return putParam("routing", routing);
}

Params withGroupShardFailures(boolean groupShardFailures) {
return putParam(RestSearchAction.GROUP_SHARD_FAILURES_PARAM, Boolean.toString(groupShardFailures));
}

Params withStoredFields(String[] storedFields) {
if (storedFields != null && storedFields.length > 0) {
return putParam("stored_fields", String.join(",", storedFields));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1307,6 +1307,7 @@ public void testMultiSearch() throws IOException {

Map<String, String> expectedParams = new HashMap<>();
expectedParams.put(RestSearchAction.TYPED_KEYS_PARAM, "true");
setRandomGroupShardFailures(multiSearchRequest::groupShardFailures, expectedParams);
if (randomBoolean()) {
multiSearchRequest.maxConcurrentSearchRequests(randomIntBetween(1, 8));
expectedParams.put("max_concurrent_searches", Integer.toString(multiSearchRequest.maxConcurrentSearchRequests()));
Expand Down Expand Up @@ -1360,7 +1361,6 @@ public void testClearScroll() throws IOException {
}

public void testSearchTemplate() throws Exception {
// Create a random request.
String[] indices = randomIndicesNames(0, 5);
SearchRequest searchRequest = new SearchRequest(indices);

Expand All @@ -1369,7 +1369,6 @@ public void testSearchTemplate() throws Exception {
setRandomIndicesOptions(searchRequest::indicesOptions, searchRequest::indicesOptions, expectedParams);

SearchTemplateRequest searchTemplateRequest = new SearchTemplateRequest(searchRequest);

searchTemplateRequest.setScript("{\"query\": { \"match\" : { \"{{field}}\" : \"{{value}}\" }}}");
searchTemplateRequest.setScriptType(ScriptType.INLINE);
searchTemplateRequest.setProfile(randomBoolean());
Expand Down Expand Up @@ -1421,21 +1420,28 @@ public void testRenderSearchTemplate() throws Exception {
public void testMultiSearchTemplate() throws Exception {
final int numSearchRequests = randomIntBetween(1, 10);
MultiSearchTemplateRequest multiSearchTemplateRequest = new MultiSearchTemplateRequest();
Map<String, String> expectedParams = new HashMap<>();
expectedParams.put(RestSearchAction.TYPED_KEYS_PARAM, "true");
setRandomGroupShardFailures(multiSearchTemplateRequest::groupShardFailures, expectedParams);
if (randomBoolean()) {
multiSearchTemplateRequest.maxConcurrentSearchRequests(randomIntBetween(1, 8));
expectedParams.put("max_concurrent_searches", Integer.toString(multiSearchTemplateRequest.maxConcurrentSearchRequests()));
}

for (int i = 0; i < numSearchRequests; i++) {
// Create a random request.
String[] indices = randomIndicesNames(0, 5);
SearchRequest searchRequest = new SearchRequest(indices);

Map<String, String> expectedParams = new HashMap<>();
setRandomSearchParams(searchRequest, expectedParams);
Map<String, String> ignored = new HashMap<>();
setRandomSearchParams(searchRequest, ignored);

// scroll is not supported in the current msearch or msearchtemplate api, so unset it:
searchRequest.scroll((Scroll) null);
// batched reduce size is currently not set-able on a per-request basis as it is a query string parameter only
searchRequest.setBatchedReduceSize(SearchRequest.DEFAULT_BATCHED_REDUCE_SIZE);

setRandomIndicesOptions(searchRequest::indicesOptions, searchRequest::indicesOptions, expectedParams);
setRandomIndicesOptions(searchRequest::indicesOptions, searchRequest::indicesOptions, ignored);

SearchTemplateRequest searchTemplateRequest = new SearchTemplateRequest(searchRequest);

Expand All @@ -1455,6 +1461,7 @@ public void testMultiSearchTemplate() throws Exception {

assertEquals(HttpPost.METHOD_NAME, multiRequest.getMethod());
assertEquals("/_msearch/template", multiRequest.getEndpoint());
assertEquals(expectedParams, multiRequest.getParameters());
List<SearchTemplateRequest> searchRequests = multiSearchTemplateRequest.requests();
assertEquals(numSearchRequests, searchRequests.size());

Expand Down Expand Up @@ -2624,6 +2631,7 @@ private static void randomizeFetchSourceContextParams(Consumer<FetchSourceContex
private static void setRandomSearchParams(SearchRequest searchRequest,
Map<String, String> expectedParams) {
expectedParams.put(RestSearchAction.TYPED_KEYS_PARAM, "true");
setRandomGroupShardFailures(searchRequest::groupShardFailures, expectedParams);
if (randomBoolean()) {
searchRequest.routing(randomAlphaOfLengthBetween(3, 10));
expectedParams.put("routing", searchRequest.routing());
Expand Down Expand Up @@ -2683,6 +2691,15 @@ private static void setRandomIncludeDefaults(GetIndexRequest request, Map<String
}
}

private static void setRandomGroupShardFailures(Consumer<Boolean> consumer, Map<String, String> expectedParams) {
boolean groupShardFailures = true;
if (randomBoolean()) {
groupShardFailures = randomBoolean();
consumer.accept(groupShardFailures);
}
expectedParams.put(RestSearchAction.GROUP_SHARD_FAILURES_PARAM, Boolean.toString(groupShardFailures));
}

private static void setRandomHumanReadable(GetIndexRequest request, Map<String, String> expectedParams) {
if (randomBoolean()) {
boolean humanReadable = randomBoolean();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@ public class MultiSearchTemplateRequest extends ActionRequest implements Composi
private int maxConcurrentSearchRequests = 0;
private List<SearchTemplateRequest> requests = new ArrayList<>();

/**
* Flag that controls whether shard failures are grouped by reason when outputting them
* using {@link org.elasticsearch.action.search.SearchResponse#toXContent(XContentBuilder, ToXContent.Params)}. This flag is used
* in the high-level REST client while it has no effect in the transport client.
*/
private boolean groupShardFailures = true;

private IndicesOptions indicesOptions = IndicesOptions.strictExpandOpenAndForbidClosed();

/**
Expand All @@ -66,7 +73,6 @@ public MultiSearchTemplateRequest add(SearchTemplateRequest request) {
return this;
}


/**
* Returns the amount of search requests specified in this multi search requests are allowed to be ran concurrently.
*/
Expand All @@ -86,6 +92,25 @@ public MultiSearchTemplateRequest maxConcurrentSearchRequests(int maxConcurrentS
return this;
}

/**
* Controls whether shard failures should be grouped by reason when outputting them
* using {@link org.elasticsearch.action.search.SearchResponse#toXContent(XContentBuilder, ToXContent.Params)}.
* This flag is used in the high-level REST client while it has no effect in the transport client.
*/
public MultiSearchTemplateRequest groupShardFailures(boolean groupShardFailures) {
this.groupShardFailures = groupShardFailures;
return this;
}

/**
* Returns whether shard failures will be grouped by reason when outputting them
* using {@link org.elasticsearch.action.search.SearchResponse#toXContent(XContentBuilder, ToXContent.Params)}.
* This flag is used in the high-level REST client while it has no effect in the transport client.
*/
public boolean groupShardFailures() {
return this.groupShardFailures;
}

public List<SearchTemplateRequest> requests() {
return this.requests;
}
Expand Down Expand Up @@ -134,7 +159,7 @@ public void writeTo(StreamOutput out) throws IOException {
}
out.writeStreamableList(requests);
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
Expand All @@ -148,9 +173,9 @@ public boolean equals(Object o) {
@Override
public int hashCode() {
return Objects.hash(maxConcurrentSearchRequests, requests, indicesOptions);
}
public static byte[] writeMultiLineFormat(MultiSearchTemplateRequest multiSearchTemplateRequest,
}

public static byte[] writeMultiLineFormat(MultiSearchTemplateRequest multiSearchTemplateRequest,
XContent xContent) throws IOException {
ByteArrayOutputStream output = new ByteArrayOutputStream();
for (SearchTemplateRequest templateRequest : multiSearchTemplateRequest.requests()) {
Expand All @@ -168,5 +193,5 @@ public static byte[] writeMultiLineFormat(MultiSearchTemplateRequest multiSearch
}
return output.toByteArray();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,17 @@
import org.elasticsearch.rest.action.search.RestSearchAction;

import java.io.IOException;
import java.util.Collections;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;

import static org.elasticsearch.rest.RestRequest.Method.GET;
import static org.elasticsearch.rest.RestRequest.Method.POST;

public class RestMultiSearchTemplateAction extends BaseRestHandler {

private static final Set<String> RESPONSE_PARAMS = Collections.singleton(RestSearchAction.TYPED_KEYS_PARAM);
private static final Set<String> RESPONSE_PARAMS = new HashSet<>(Arrays.asList(RestSearchAction.TYPED_KEYS_PARAM,
RestSearchAction.GROUP_SHARD_FAILURES_PARAM));

private final boolean allowExplicitIndex;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,17 @@
import org.elasticsearch.rest.action.search.RestSearchAction;

import java.io.IOException;
import java.util.Collections;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;

import static org.elasticsearch.rest.RestRequest.Method.GET;
import static org.elasticsearch.rest.RestRequest.Method.POST;

public class RestSearchTemplateAction extends BaseRestHandler {

private static final Set<String> RESPONSE_PARAMS = Collections.singleton(RestSearchAction.TYPED_KEYS_PARAM);
private static final Set<String> RESPONSE_PARAMS = new HashSet<>(Arrays.asList(RestSearchAction.TYPED_KEYS_PARAM,
RestSearchAction.GROUP_SHARD_FAILURES_PARAM));

public RestSearchTemplateAction(Settings settings, RestController controller) {
super(settings);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
---
"shard failures":
- do:
indices.create:
index: text
body:
settings:
number_of_replicas: 0
number_of_shards: 1
mappings:
doc:
properties:
creation_date:
type: keyword

- do:
indices.create:
index: date
body:
settings:
number_of_replicas: 0
number_of_shards: 3
mappings:
doc:
properties:
creation_date:
type: date

- do:
search_template:
body:
source:
query:
match:
creation_date: "{{text_value}}"
params:
text_value: "text"

- length: { _shards.failures: 1 }
- match: { _shards.failed: 3}
- match: { _shards.total: 4}
- match: { _shards.successful: 1}

- do:
search_template:
group_shard_failures: false
body:
source:
query:
match:
creation_date: "{{text_value}}"
params:
text_value: "text"

- length: { _shards.failures: 3 }
- match: { _shards.failed: 3}
- match: { _shards.total: 4}
- match: { _shards.successful: 1}

- do:
msearch_template:
body:
- index: text,date
- source:
query:
match:
creation_date: "{{text_value}}"
params:
text_value: "text"

- length: { responses.0._shards.failures: 1 }
- match: { responses.0._shards.failed: 3}
- match: { responses.0._shards.total: 4}
- match: { responses.0._shards.successful: 1}

- do:
msearch_template:
group_shard_failures: false
body:
- index: text,date
- source:
query:
match:
creation_date: "{{text_value}}"
params:
text_value: "text"

- length: { responses.0._shards.failures: 3 }
- match: { responses.0._shards.failed: 3}
- match: { responses.0._shards.total: 4}
- match: { responses.0._shards.successful: 1}
4 changes: 4 additions & 0 deletions rest-api-spec/src/main/resources/rest-api-spec/api/count.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@
"terminate_after" : {
"type" : "number",
"description" : "The maximum count for each shard, upon reaching which the query execution will terminate early"
},
"group_shard_failures" : {
"type" : "boolean",
"description" : "Specify whether shard failures should be grouped by reason, default is true"
}
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@
"type" : "number",
"description" : "A threshold that enforces a pre-filter roundtrip to prefilter search shards based on query rewriting if the number of shards the search request expands to exceeds the threshold. This filter roundtrip can limit the number of shards significantly if for instance a shard can not match any documents based on it's rewrite method ie. if date filters are mandatory to match but the shard bounds and the query are disjoint.",
"default" : 128
},
"group_shard_failures" : {
Copy link
Member Author

@javanna javanna Aug 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with msearch and msearch_template, I had to go with exposing the param only globally. Given how it's read (through ToXContent.Params when outputting SearchResponse output, it becomes very complicated to support the option per item, and have each response item with its own value read from its own request item.

I also wonder if, in general, it is a good idea to expose this new parameter rather than removing support for it, given that it is not documented, we did not realize for a long time that it was not exposed, and we did not get any report about this either.

"type" : "boolean",
"description" : "Specify whether shard failures should be grouped by reason, default is true"
}
}
},
Expand Down
Loading