Consistency between _search and _msearch

[ofavre]

The _msearch API doesn't accept the same header parameters than _search's query parameters.
For instance version is both valid in the search body and query parameters for _search, but is ignored in the query parameters and header in _msearch.

MultiSearchRequest.add() should rely on RestSearchAction.parseSearchSource() or even RestSearchAction.parseSearchRequest() to parse the subrequests, for maximum compatibility.

IMHO, a clean bulk API should use the header parameters as query parameters on the target endpoint, which would imply that they keep in sync no matter the code changes in the target endpoints.

[kimchy]

yea, agreed, that would be nice to have

[clintongormley]

Should _search be implemented internally via _msearch, in which case the two would never go out of sync? @Christoph and @martijnvg what do you think?

[mayya-sharipova]

cc @elastic/es-search-aggs

[pedromatt]

Is this issue still active? I can't completely understand what is trying to be fixed and I'd like to work on it

[javanna]

hi @pedromatt there hasn't been any activity on this issue for a long while. You are welcome to have a look at it. I had a quick look myself and I think that the first step was to double check if the issue still exists. The issue was initially opened because there were parameters that the search API supports, which were not supported as part of the msearch API. For instance version is mentioned in the description of this issue, and timeout in another linked issue. First thing would be to check if these two fields are supported by both API (search and msearch). Is this enough info for you to start working on this?

[zacharymorn]

Looks like this issue has been around for a while and there's no activity on pull request from @vieirajlt for about 10 months either. I would like to see if I can still reproduce the issue in the latest version as @javanna suggested and submit a different PR if that's ok. I'm new to contributing to open source so this may take some time.

[zacharymorn]

So it does look like as of ES snapshot version 8, at least timeout and version are still not supported for _msearch

curl -XGET 'localhost:9200'  -H 'Content-Type: application/json' -u elastic-admin:elastic-password


{
  "name" : "node-0",
  "cluster_name" : "distribution_run",
  "cluster_uuid" : "PegNKHByS0ePaB8Qqw9LQg",
  "version" : {
    "number" : "8.0.0-SNAPSHOT",
    "build_flavor" : "default",
    "build_type" : "tar",
    "build_hash" : "33ed38eccd8fac04031410241b8bc1876dfc5265",
    "build_date" : "2019-09-19T18:13:44.674660Z",
    "build_snapshot" : true,
    "lucene_version" : "8.2.0",
    "minimum_wire_compatibility_version" : "7.5.0",
    "minimum_index_compatibility_version" : "7.0.0"
  },
  "tagline" : "You Know, for Search"
}

curl -XPOST  'http://localhost:9200/_msearch?timeout=10s&version=true' -H 'Content-Type: application/json' -u elastic-admin:elastic-password -d '
{}
 
{"query" : {"match_all" : {}}}
'


{"error":{"root_cause":[{"type":"illegal_argument_exception","reason":"request [/_msearch] contains unrecognized parameters: [timeout], [version]"}],"type":"illegal_argument_exception","reason":"request [/_msearch] contains unrecognized parameters: [timeout], [version]"},"status":400}

Does it look like a successful reproduction of the issue, or is there something more I need to check?

[javanna]

heya @zacharymorn thanks for looking into this. Sounds like this still needs fixing. It may be worth to dig a bit deeper and see if there are other parameters that search supports which msearch does not. Looking at our rest spec for search and msearch may help here, see https://github.com/elastic/elasticsearch/tree/master/rest-api-spec/src/main/resources/rest-api-spec/api.

[zacharymorn]

@javanna Sounds good & that's a good API reference link I can use in the future! I'm assuming you would like me to compare API support for both search.json and msearch.json ? I do see from the json files along that search.json contains a much longer param list than that of msearch.json.

I also feel a good future-prove guard for this is to create a test case that consumes both specs and spit out any params inconsistency in the list, and actually execute all the APIs listed. This would require certain assumption such as params available for _msearch will always be the same for params available to _search, or a diff list will need to be maintained. What do you think?

[javanna]

all makes sense, but not as trivial as it sounds :) especially the testing part as some params work only in some specific context. Also, it may be expected that some params are supported by search but not by msearch. Can you make a list and post it here so we can discuss it?

[zacharymorn]

Just in time! By comparing the search.json and msearch.json alone, I get the following diff list

only available in search.json:

    params:
      "analyzer"
      "analyze_wildcard"
      "default_operator"
      "df"
      "explain"
      "stored_fields"
      "docvalue_fields"
      "from"
      "ignore_unavailable"
      "ignore_throttled"
      "allow_no_indices"
      "expand_wildcards"
      "lenient"
      "preference"
      "q" 
      "routing"
      "scroll"
      "size"
      "sort"
      "_source"
      "_source_excludes"
      "_source_includes"
      "terminate_after"
      "stats"
      "suggest_field"
      "suggest_mode"
      "suggest_size"
      "suggest_text"
      "timeout"
      "track_scores"
      "track_total_hits"
      "allow_partial_search_results"
      "version"
      "seq_no_primary_term"
      "request_cache"
      "batched_reduce_size"

only available in msearch.json:

    search type:
        query_and_fetch
        dfs_query_and_fetch
    params:
        max_concurrent_searches

[javanna]

heya @zacharymorn I am not sure that all these parameters make sense for msearch as global parameters. What I would focus on to start with is to check whether all of the search parameters are accepted and work properly as part of the msearch request body, as part of each search item. Unfortunately there is no spec for request/response bodies, hence it is really a matter of digging and testing. Once we have a full picture of what is missing we can think of what we want to address and how.

[zacharymorn]

Sounds good @javanna . I'm currently in the process of building up query body data to test those params. Will post update once I have some results.

[zacharymorn]

Hey @javanna I got back some initial result.

I originally started with these commands that basically include all missing param in msearch request body

curl -XPOST  'http://localhost:9200/_msearch' -H 'Content-Type: application/json' -u elastic-admin:elastic-password -d '
{}

{"query": {"term": {"body": {"value": "easy", "boost": 1.0 }}}, "analyzer": "english", "analyze_wildcard": true, "default_operator": "AND", "df": "body", "explain": true, "stored_fields": ["user", "body", "title"], "docvalue_fields": ["user", "body", "title"], "from": 1, "ignore_unavailable": true, "ignore_throttled": true, "allow_no_indices": true, "expand_wildcards": "all", "lenient": true, "preference": "_local", "q": "*:*", "routing": "user", "scroll": "1m", "size": 11, "sort": "user:asc", "_source": true, "_source_excludes": ["postDate"], "_source_includes": ["user", "body", "title"], "terminate_after": 2, "stats": ["merge", "refresh"], "suggest_field": "body", "suggest_mode": "popular", "suggest_size": 2, "suggest_text": "hello", "timeout": "10s", "track_scores": true, "track_total_hits": true, "allow_partial_search_results": true, "version": true, "seq_no_primary_term": true, "request_cache": true, "batched_reduce_size": 128

By try-and-eliminate one by one, a lot of them generated errors of the form:

{"error":{"root_cause":[{"type":"parsing_exception","reason":"Unknown key for a VALUE_STRING in [analyzer].","line":1,"col":77}],"type":"parsing_exception","reason":"Unknown key for a VALUE_STRING in [analyzer].","line":1,"col":77},"status":400}

{"error":{"root_cause":[{"type":"parsing_exception","reason":"Unknown key for a START_ARRAY in [_source_excludes].","line":1,"col":255}],"type":"parsing_exception","reason":"Unknown key for a START_ARRAY in [_source_excludes].","line":1,"col":255},"status":400}

which basically all come from SearchSourceBuilder.parseXContent

After eliminating unsupported params, the minimum-viable-request-body and its response are something like this

curl -XPOST  'http://localhost:9200/_msearch' -H 'Content-Type: application/json' -u elastic-admin:elastic-password -d '
{}

{"query": {"term": {"body": {"value": "easy", "boost": 1.0 }}}, "explain": true, "stored_fields": ["user", "body", "title"], "docvalue_fields": ["user", "body", "title"], "from": 1,    "size": 11, "sort": "user:asc", "_source": true, "terminate_after": 2, "stats": ["merge", "refresh"],  "timeout": "10s", "track_scores": true, "track_total_hits": true,  "version": true, "seq_no_primary_term": true  } 
'

{"took":15,"responses":[{"took":7,"timed_out":false,"_shards":{"total":0,"successful":0,"skipped":0,"failed":0},"hits":{"total":{"value":0,"relation":"eq"},"max_score":0.0,"hits":[]},"status":200},{"took":0,"timed_out":false,"_shards":{"total":0,"successful":0,"skipped":0,"failed":0},"hits":{"total":{"value":0,"relation":"eq"},"max_score":0.0,"hits":[]},"status":200}]}

I restarted the ES server so no document is left to match, but I think the response basically proved that those params are accepted in request body.

The full list of params I eliminated is the following

analyzer
analyze_wildcard
default_operator
df
ignore_unavailable
ignore_throttled
allow_no_indices
expand_wildcards
lenient
preference
routing
scroll
_source_excludes
_source_includes
suggest_field
suggest_text
allow_partial_search_results
request_cache
batched_reduce_size

I guess next step will be to actually index some more documents and see if the supported params actually work as expected?

[javanna]

ok that would suggest that timeout and version are supported than as part of the individual search items in msearch? are these params listed in your last comment the ones that seem supported or not supported by msearch?

[zacharymorn]

@javanna I've taken a stab at it and came up with this draft commit zacharymorn@3a24265

Basically I was trying to move the shared rest request params parsing logic below from RestSearchAction to a common class, which can later be used in RestMultiSearchAction. This is the most local changes I can think of for now, and haven't throughly tested it yet. Please let me know what do you think about this approach.

elasticsearch/server/src/main/java/org/elasticsearch/rest/action/search/RestSearchAction.java

Lines 180 to 269 in 8195d68

	int from = request.paramAsInt("from", -1);
	if (from != -1) {
	searchSourceBuilder.from(from);
	}
	int size = request.paramAsInt("size", -1);
	if (size != -1) {
	setSize.accept(size);
	}
	if (request.hasParam("explain")) {
	searchSourceBuilder.explain(request.paramAsBoolean("explain", null));
	}
	if (request.hasParam("version")) {
	searchSourceBuilder.version(request.paramAsBoolean("version", null));
	}
	if (request.hasParam("seq_no_primary_term")) {
	searchSourceBuilder.seqNoAndPrimaryTerm(request.paramAsBoolean("seq_no_primary_term", null));
	}
	if (request.hasParam("timeout")) {
	searchSourceBuilder.timeout(request.paramAsTime("timeout", null));
	}
	if (request.hasParam("terminate_after")) {
	int terminateAfter = request.paramAsInt("terminate_after",
	SearchContext.DEFAULT_TERMINATE_AFTER);
	if (terminateAfter < 0) {
	throw new IllegalArgumentException("terminateAfter must be > 0");
	} else if (terminateAfter > 0) {
	searchSourceBuilder.terminateAfter(terminateAfter);
	}
	}
	StoredFieldsContext storedFieldsContext =
	StoredFieldsContext.fromRestRequest(SearchSourceBuilder.STORED_FIELDS_FIELD.getPreferredName(), request);
	if (storedFieldsContext != null) {
	searchSourceBuilder.storedFields(storedFieldsContext);
	}
	String sDocValueFields = request.param("docvalue_fields");
	if (sDocValueFields != null) {
	if (Strings.hasText(sDocValueFields)) {
	String[] sFields = Strings.splitStringByCommaToArray(sDocValueFields);
	for (String field : sFields) {
	searchSourceBuilder.docValueField(field, null);
	}
	}
	}
	FetchSourceContext fetchSourceContext = FetchSourceContext.parseFromRestRequest(request);
	if (fetchSourceContext != null) {
	searchSourceBuilder.fetchSource(fetchSourceContext);
	}
	if (request.hasParam("track_scores")) {
	searchSourceBuilder.trackScores(request.paramAsBoolean("track_scores", false));
	}
	if (request.hasParam("track_total_hits")) {
	if (Booleans.isBoolean(request.param("track_total_hits"))) {
	searchSourceBuilder.trackTotalHits(
	request.paramAsBoolean("track_total_hits", true)
	);
	} else {
	searchSourceBuilder.trackTotalHitsUpTo(
	request.paramAsInt("track_total_hits", SearchContext.DEFAULT_TRACK_TOTAL_HITS_UP_TO)
	);
	}
	}
	String sSorts = request.param("sort");
	if (sSorts != null) {
	String[] sorts = Strings.splitStringByCommaToArray(sSorts);
	for (String sort : sorts) {
	int delimiter = sort.lastIndexOf(":");
	if (delimiter != -1) {
	String sortField = sort.substring(0, delimiter);
	String reverse = sort.substring(delimiter + 1);
	if ("asc".equals(reverse)) {
	searchSourceBuilder.sort(sortField, SortOrder.ASC);
	} else if ("desc".equals(reverse)) {
	searchSourceBuilder.sort(sortField, SortOrder.DESC);
	}
	} else {
	searchSourceBuilder.sort(sort);
	}
	}
	}
	String sStats = request.param("stats");
	if (sStats != null) {
	searchSourceBuilder.stats(Arrays.asList(Strings.splitStringByCommaToArray(sStats)));
	}

[zacharymorn]

I did some basic testings for the draft commit, looks like the version and timeout request query params will be consumed properly after the changes:

With version param

Request with version=true

curl -XPOST  'http://localhost:9200/_msearch?version=true' -H 'Content-Type: application/json' -u elastic-admin:elastic-password -d '
{"query": {"term": {"title": {"value": "history"}}}}
'

Expected result with _version field

{"took":159,"responses":[{"took":156,"timed_out":false,"_shards":{"total":1,"successful":1,"skipped":0,"failed":0},"hits":{"total":{"value":2,"relation":"eq"},"max_score":3.8111825,"hits":[{"_index":"book-index","_type":"book-type","_id":"Zkol720BO4z9gS7QCk0b","_version":1,"_score":3.8111825,"_source":{ "title": "She: A History of Adventure", "author": "H. Rider Haggard" }},{"_index":"book-index","_type":"book-type","_id":"5Eol720BO4z9gS7QCk0c","_version":1,"_score":3.8111825,"_source":{ "title": "A Brief History of Time", "author": "Stephen Hawking" }}]},"status":200}]}

Request with version=false

curl -XPOST  'http://localhost:9200/_msearch?version=false' -H 'Content-Type: application/json' -u elastic-admin:elastic-password -d '
{"query": {"term": {"title": {"value": "history"}}}}
‘

Expected result without _version field

{"took":3,"responses":[{"took":3,"timed_out":false,"_shards":{"total":1,"successful":1,"skipped":0,"failed":0},"hits":{"total":{"value":2,"relation":"eq"},"max_score":3.8111825,"hits":[{"_index":"book-index","_type":"book-type","_id":"Zkol720BO4z9gS7QCk0b","_score":3.8111825,"_source":{ "title": "She: A History of Adventure", "author": "H. Rider Haggard" }},{"_index":"book-index","_type":"book-type","_id":"5Eol720BO4z9gS7QCk0c","_score":3.8111825,"_source":{ "title": "A Brief History of Time", "author": "Stephen Hawking" }}]},"status":200}]}

With timeout param

Request & with debug breakpoint pause forcing timeout

curl -XPOST  'http://localhost:9200/_msearch?timeout=1s' -H 'Content-Type: application/json' -u elastic-admin:elastic-password -d '
{"query": {"term": {"title": {"value": "history"}}}}
'

Expected result with timeout

{"took":76139,"responses":[{"took":76137,"timed_out":true,"_shards":{"total":1,"successful":1,"skipped":0,"failed":0},"hits":{"total":{"value":2,"relation":"eq"},"max_score":null,"hits":[]},"status":200}]}

[javanna]

the idea looks good to me @zacharymorn . Can you open a PR so we can discuss the changes?

[zacharymorn]

I opened the PR above. I do have a few more questions though, primarily around whether certain query params are also applicable to _msearch. I’ve marked them with TODO comments in the changes.

[zacharymorn]

I looked into the unsupported params for _msearch further. As it turns out, many that I previously found not supported, were actually supported in the index metadata section of the msearch request, rather than in query body. Those are parsed in

elasticsearch/server/src/main/java/org/elasticsearch/action/search/MultiSearchRequest.java

Lines 215 to 248 in b4bc6ac

	if ("index".equals(entry.getKey()) || "indices".equals(entry.getKey())) {
	if (!allowExplicitIndex) {
	throw new IllegalArgumentException("explicit index in multi search is not allowed");
	}
	searchRequest.indices(nodeStringArrayValue(value));
	} else if ("search_type".equals(entry.getKey()) || "searchType".equals(entry.getKey())) {
	searchRequest.searchType(nodeStringValue(value, null));
	} else if ("ccs_minimize_roundtrips".equals(entry.getKey()) || "ccsMinimizeRoundtrips".equals(entry.getKey())) {
	searchRequest.setCcsMinimizeRoundtrips(nodeBooleanValue(value));
	} else if ("request_cache".equals(entry.getKey()) || "requestCache".equals(entry.getKey())) {
	searchRequest.requestCache(nodeBooleanValue(value, entry.getKey()));
	} else if ("preference".equals(entry.getKey())) {
	searchRequest.preference(nodeStringValue(value, null));
	} else if ("routing".equals(entry.getKey())) {
	searchRequest.routing(nodeStringValue(value, null));
	} else if ("allow_partial_search_results".equals(entry.getKey())) {
	searchRequest.allowPartialSearchResults(nodeBooleanValue(value, null));
	} else if ("expand_wildcards".equals(entry.getKey()) || "expandWildcards".equals(entry.getKey())) {
	expandWildcards = value;
	} else if ("ignore_unavailable".equals(entry.getKey()) || "ignoreUnavailable".equals(entry.getKey())) {
	ignoreUnavailable = value;
	} else if ("allow_no_indices".equals(entry.getKey()) || "allowNoIndices".equals(entry.getKey())) {
	allowNoIndices = value;
	} else if ("ignore_throttled".equals(entry.getKey()) || "ignoreThrottled".equals(entry.getKey())) {
	ignoreThrottled = value;
	} else {
	throw new IllegalArgumentException("key [" + entry.getKey() + "] is not supported in the metadata section");
	}
	}
	defaultOptions = IndicesOptions.fromParameters(expandWildcards, ignoreUnavailable, allowNoIndices, ignoreThrottled,
	defaultOptions);
	}
	}
	searchRequest.indicesOptions(defaultOptions);

There are also three params that have different field names when used in query body

Query Param Names

_source_excludes 
_source_includes
suggest_text

Query Body Field Names

... "_source": {"includes": [ "author"], "excludes": [ "title" ]} ...
..."suggest": {"text" : "the"}...

[zacharymorn]

Essentially there are 3 locations that params can be specified for _msearch

1. URL query params
2. Header section for each search request in payload
3. Body section for each search request in payload

The header & body specifically refer to https://www.elastic.co/guide/en/elasticsearch/reference/current/search-multi-search.html

Specifically, here are the params supported in each of these 3 locations

URL query params

// This is set globally for msearch request
max_concurrent_searches

// These two affect each individual search request
pre_filter_shard_size
max_concurrent_shard_requests

// These will be passed down as default for each individual search request
index
search_type
ccs_minimize_roundtrips
routing

Header section for each search request in payload

index | indices
search_type | searchType
ccs_minimize_roundtrips | ccsMinimizeRoundtrips
request_cache | requestCache
expand_wildcards | expandWildcards
ignore_unavailable | ignoreUnavailable
allow_no_indices | allowNoIndices
ignore_throttled | ignoreThrottled
preference
routing
allow_partial_search_results

Body section for each search request in payload

As both _msearch and _search eventually use SearchSourceBuilder.parseXContent for this part, both support the same set of fields / parameters. This set includes the version and timeout fields that are missing in URL query param and header section.

Hence, I guess for _msearch we may need to keep 3 locations in sync (this corresponds to @ofavre 's initial comment in this thread), whereas for _search we only need to keep 2. However, I could also see the reasoning that for _msearch, it might be better to just keep different supported fields in header and body sections, instead of having them overlapped. What's your thought on this @javanna ?

---

On the other hand, so far I was only able to find the followings that are supported for _search, but not for _msearch

Seen in RestActions.urlParamsToQueryBuilder, used by RestSearchAction.parseSearchSource

df
analyzer
analyze_wildcard
lenient
default_operator

Seen in RestSearchAction.parseSearchSource

suggest_mode
suggest_size

scroll  
batched_reduce_size

In addition, suggest_mode and suggest_size seems to be easily supportable for _msearch, as it currently can handle suggest and its associated field text in query body section. But for the rest I'm not sure if they are not available for _msearch by design.

[zacharymorn]

@javanna Any update on this ?

[xtermi2]

We are migrating from TransportClient to RestHighLevelClient and I stumbled across this issue because TransportClient supports at least some parameters which RestHighLevelClient (or REST API in general) does not. In our case this is scroll. I created a test project to reproduce: https://github.com/xtermi2/elasticsearch-resthighlevelclient-msearch-scrollId

This is realy unpredictibal, because you get no error just other results when dooing the same request with RestHighLevelClient instead TransportClient.

So please fix this issue or at least throw an exception when trying to do a request with RestHighLevelClient with unsupported parameters.

[javanna]

heya @xtermi2 I don't follow what you mean. I am wondering if you mean that something different between high level client and transport client, or between msearch and search, and where the scroll, or maybe you meant scroll_id, comes into the picture.

[javanna]

@zacharymorn sorry I was out for a long while and I missed your ping, I will try to get back to this soon.

[xtermi2]

@javanna Yes, there is a difference in transport client and rest client (or in general in REST API). Just have a look at https://github.com/xtermi2/elasticsearch-resthighlevelclient-msearch-scrollId/blob/master/src/test/java/com/github/xtermi2/elasticsearch/RHLCMsearchScrollIdTest.java

• restHighLevelClient_msearch_should_return_scrollId failes, because the request does not provide the scroll parameter, so the response does not contain a scroll_id
• transportClient_msearch_should_return_scrollId passes, because transport client somehow provides the scroll parameter in its request and the the response of course contains a scroll_id.

It's the same request (MultiSearchRequest) for both clients.

So from the perspective of the transport-client/RestHighLevelClient I have the exact same API but get different behavior which is totally unexpected.

As I said, the minimum should be to provide a Exception when you want to do a _msearch request which provides unsupported parameters via RestHighLevelClient - until this issue is fixed and _msearch supports all parameters from _search

[javanna]

ok @xtermi2 thanks for clarifying, could you please open a new issue and post the exact error you are getting as well as how to reproduce it? Thanks!

[JonasMatos0]

Hello,

What's the current state of this issue?
I'm taking a look on these really old GFI tickets.
I can try to help, just need some details about how to proceed.

Thanks in advance.

[xtermi2]

The part I mention in #4227 (comment) is still not fixed! The referenced github project is still failing with the latest elasticsearch version. There is a integrationtest comparing transport client with rest client/API
@JonasMatos0 What additional information do you need?

[JonasMatos0]

Hi, @xtermi2 .
I understand your point is suggesting to have at least a simple throw with details whenever scroll is required
My concern is that this issue is broader than that.

@zacharymorn PR #48673
is still opened with no discussion to it.

Is there any plans to that PR @javanna ? How should this ticket be handled? Is it preferred I open a PR solving only @xtermi2 concern?

[akshaybhadange]

can i work on this issue , plz guide me its my first contribution

[elasticsearchmachine]

Pinging @elastic/es-search (Team:Search)

[carlosdelest]

Created a draft PR here - it would be great to have feedback on the overall approach

[elasticsearchmachine]

Pinging @elastic/es-search-foundations (Team:Search Foundations)