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

Should we have a default timeout? #26308

Closed
jpountz opened this issue Aug 21, 2017 · 10 comments
Closed

Should we have a default timeout? #26308

jpountz opened this issue Aug 21, 2017 · 10 comments
Labels
>feature :Search/Search Search-related issues that do not fall into other categories

Comments

@jpountz
Copy link
Contributor

jpountz commented Aug 21, 2017

While discussing #26258 @jimczi asked whether we should set a default timeout. This proved a bit controversial: on the one hand it would reduce the impact that insane queries could have on clusters but on the other hand this might be a bit trappy as users might not be checking the timed_out field of search responses, which is the way to know that the response is partial due to a timeout. I'm opening this issue to get opinions from a wider audience.

@jpountz jpountz added :Search/Search Search-related issues that do not fall into other categories discuss labels Aug 21, 2017
@dakrone
Copy link
Member

dakrone commented Aug 22, 2017

+1 to have a default timeout, return a 206 Partial Content for partial content (timed out responses) and set the timeout to something reasonable, like 5 minutes

@jasontedor
Copy link
Member

jasontedor commented Aug 22, 2017

206 status code is inappropriate. It's appropriate only if the client sends a range request (as indicated by a Range header field).

@dakrone
Copy link
Member

dakrone commented Aug 22, 2017 via email

@nik9000
Copy link
Member

nik9000 commented Aug 23, 2017

The thing that bothers me about the default timeout is the timeout reporting. Right now we return a 200 OK and you have to inspect the response to make sure it is 100% ok. Adding a default timeout adds another way that the response can be not ok without the user asking for it. I'd certainly prefer that we have a default timeout, but I'd prefer we tie it to some change in behavior around error reporting so it is obvious when you get partial results.

@jimczi jimczi added >feature and removed discuss labels Aug 25, 2017
@jimczi
Copy link
Contributor

jimczi commented Aug 25, 2017

Discussed it in FixItFriday, the partial result is a big concern so we decided to explore adding a new setting at the cluster level (or at the index level if needed) that would add an hard timeout to search request. It could be activated by default with a very high value like 5 minutes. The benefit would be to automatize the garbage collection of long running queries.
Though we still need to benchmark the cost of adding a timeout checks to every query:
#26258
@danielmitterdorfer @polyfractal can you rerun the tests you've setup for #26258 but with timeouts this time ? This should help decide if we activate timeouts by default.

@polyfractal
Copy link
Contributor

/cc @dakrone since he ran the tests, I just played with some spreadsheets to generate the p-values :)

@talevy talevy added :Search/Search Search-related issues that do not fall into other categories and removed :Search/Search Search-related issues that do not fall into other categories labels Mar 16, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

@javanna
Copy link
Member

javanna commented Apr 9, 2019

Should we close this in favour of #26258 ?

@jimczi jimczi removed the help wanted adoptme label Apr 9, 2019
@jimczi
Copy link
Contributor

jimczi commented Apr 9, 2019

If we close I think it should be in favor of #30897 and we can discuss the need for a default value there ?

@cbuescher
Copy link
Member

cbuescher commented May 24, 2019

Since #26258 is already closed now, closing this in favour of #30897 where we can continue the discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>feature :Search/Search Search-related issues that do not fall into other categories
Projects
None yet
Development

No branches or pull requests

10 participants