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

Search request timeout option is confusing #47716

Closed
jimczi opened this issue Oct 8, 2019 · 6 comments · Fixed by #71713
Closed

Search request timeout option is confusing #47716

jimczi opened this issue Oct 8, 2019 · 6 comments · Fixed by #71713
Labels
>docs General docs changes :Search/Search Search-related issues that do not fall into other categories Team:Docs Meta label for docs team Team:Search Meta label for search team

Comments

@jimczi
Copy link
Contributor

jimczi commented Oct 8, 2019

The documentation around the timeout option of the search request is confusing for users. Most of the time it is understood as a way to cancel any search request that takes longer than the configured threshold and users don't expect to get partial results in the response. In fact they don't even expect to get a response. While the documentation could be improved I also wonder if we should not renamed the option or simply remove it. We have a new way to cancel search requests efficiently from the client in 7.4 (by closing the underlying http channel) and we expose this in our official Java client so we don't need this option on the server if the goal is to trash any requests that take more than a configured threshold. One option I was thinking regarding the renaming was to handle time unit in the terminate_after option, today users can set the terminate_after option to control the max number of documents that should be collected per shard but we could also accept a time as in terminate_after=10ms. IMO this would be less confusing and would leave any real timeout option to the client side where it belongs. I am also curious to hear if this feature is really used to get partial results since that's the only reason why it should be preferred over a timeout on the client side.

@jimczi jimczi added >docs General docs changes discuss :Search/Search Search-related issues that do not fall into other categories labels Oct 8, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-docs (>docs)

@elasticmachine
Copy link
Collaborator

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

@markharwood
Copy link
Contributor

I am also curious to hear if this feature is really used to get partial results

Wasn't this targeting e-commerce scenarios? Shoppers have a limited attention span and there's a desire to serve something within a given time window.

@debadair
Copy link
Contributor

[docs issue triage]

@jimczi
Copy link
Contributor Author

jimczi commented Oct 18, 2019

We discussed this in FixIt Friday and agreed that we should try to address the gap between the perception of the feature and what it really does. Since users expect this option to be a global timeout, one option that we discussed would be to change the feature as it is today and make it a global timeout. That would be a breaking change so we'd need to wait 8.0 (the next major) to expos the change. One concern we had was that the cancellation of search request can now be done directly in the client by closing the http connection. In such case the server (Elasticsearch) will automatically cancels the request. So adding an option on the server side would mean that we have two ways to do the same thing but since we have this timeout option on other APIs as well it would be more consistent.

@jimczi jimczi removed the discuss label Oct 18, 2019
@matriv
Copy link
Contributor

matriv commented Jan 28, 2020

There seems to be also some confusion regarding the global search timeout, based on sdh issue https://github.com/elastic/sdh-elasticsearch/issues/2159:

So global search timeout discusses canceling request but Request Body Search does not and that is another confusion to me that canceling is only required for global search timeout. If this is not the case, I think we should improve Request Body Search as well.

jimczi added a commit to jimczi/elasticsearch that referenced this issue Feb 4, 2020
This change deprecates the usage of the `timeout` option for a more explicit name: `shard_timeout`.
This commit implements the deprecation in order to be able to backport to 7x but the old name
should be removed from master (8.0) in a follow up.
This change slightly rewrites the documentation around this option to make it clear that the timeout
is per shard and not a global one.

Closes elastic#47716
@rjernst rjernst added Team:Docs Meta label for docs team Team:Search Meta label for search team labels May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>docs General docs changes :Search/Search Search-related issues that do not fall into other categories Team:Docs Meta label for docs team Team:Search Meta label for search team
Projects
None yet
6 participants