[DOCS] add docs for async search

[javanna]

Relates to #49091

[elasticmachine]

Pinging @elastic/es-docs (>docs)

[elasticmachine]

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

[javanna]

I was hoping this would work but it does cause problems. I need to see if this escaping is needed, to my mind it isn't but I may be wrong. In that case I need to only escape unless curly brackets are part of an expression like ${expression}, which I don't look forward to.

[javanna]

turns out this is easy to address, there were only a couple of tests where we were using unescaped | which we can manually escape instead.

[javanna]

@jimczi I was wondering if you left this out on purpose. It is settable hence I added it, let me know what you think

[jimczi]

We should probably prevent users to set this and assume that the can_match phase always runs on async_search. That means that we should throw an error if the value is set in the rest API but that's out of scope for this pr. +1 to remove it from the spec here though

[javanna]

I will have a look at disallowing to set this parameter in a followup

[jrodewig]

I left some comments to help fix wording, capitalization, and a few other things. Aside from those, this looks pretty good.

I do wonder if the API docs might be easier to parse if they were formatted similar to the API reference here:
https://github.com/elastic/docs/blob/master/shared/api-ref-ex.asciidoc

Some of the sections regarding parameters could probably benefit from that structure. However, that would likely involve duplicating a lot of content from the current search API docs, which need work themselves. I'm okay with this for now.

[jimczi]

Thanks @javanna , I left some comments regarding the documentation of the different options.

[jimczi]

Maybe move this note near to the submit query in order to explain why the example uses a sort ?

[jimczi]

The keep_alive for the get api is a bit different since it allows to extend/change the expiration time for an existing id. Maybe good to make the distinction here since the default for get is to keep the current expiration time.

[jimczi]

I think we can list the ones that can be changed and omit the other. batched_reduce_size, request_cache and max_concurrent_shard_requests + the specific options (wait_for_completion, keep_alive) should be enough for this API. I think we said that we want to remove pre_filter_shard_size so I'd remove it from here, same for ccs_minimize_roundtrips which sounds like it could be changed but cannot.
So to be clear, a concrete list that presents all these options and then we can add a link to the search source documentation that should be unchanged from normal search request ?

[jimczi]

batched_reduce_size is also important to document since that's the granularity at which partial results will be made available.

[javanna]

agreed, I was planning on looking back to this once we disallow setting pre_filter_shard_size and maybe others, but I can address the docs in the meantime.

[javanna]

we can add a link to the search source documentation

there is already a link. Why document max_concurrent_shard_request here? The default is the same as for search?

[jimczi]

because I thought we would have the exhaustive list of options here rather than linking to the entire search request option. We only accept a subset of it so better to be explicit ?

[javanna]

the thing is that there's a ton of options that can be set to the search request. See https://www.elastic.co/guide/en/elasticsearch/reference/current/search-search.html#search-search-api-query-params . I thought it's better to just link to search rather than duplicate its docs, and mention the special cases or important aspects only in the async search docs. Would you rather list all of the search options also here?

[jimczi]

Ok for a simple link but we should at least document batched_reduce_size explicitly here for the reason exposed above. We can also add the list of options that are not accepted here (scroll, ccs_minimize_roundtrips and pre_filter_shard_size) ?

[javanna]

yep that's what I had in mind. batched_reduce_size makes a lot of sense to explain, I already did, I will push shortly

[javanna]

thanks a lot for all the help @jrodewig ! I suspect you will have suggestions on my latest update, see bbebf3c specifically. thanks again!

[javanna]

@jimczi I pushed some updates, would you like to have another look?

[jimczi]

I left two minor comments but the change looks good to me.

[jimczi]

That's only when security is enabled that we restrict the access. We should make the distinction here.

[jimczi]

Can you add an explanation for expiration_time_in_millis, is_partial and is_running ? The last two are important since they determine how to interpret the search response (partial or not) and if the search is still running.