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] Update timeout configurations #75321

Closed
lizozom opened this issue Aug 18, 2020 · 4 comments · Fixed by #75728
Closed

[Search] Update timeout configurations #75321

lizozom opened this issue Aug 18, 2020 · 4 comments · Fixed by #75728
Assignees
Labels
Feature:Search Querying infrastructure in Kibana v7.10.0 v8.0.0

Comments

@lizozom
Copy link
Contributor

lizozom commented Aug 18, 2020

At the moment, we have two search related timeout configurations:

  • elasticsearch.shardTimeout - Used to indicate the shard timeout (i.e. how long it can run on a single shard). It is taken from kibana.yml setting, which is 30s by default.
  • elasticsearch.requestTimeout - Used by the Elastic Search client, to put a maximum time limit on each Elasticsearch request (it might be running requests on multiple shards). It is 30s by default.

Moving forward with the Background Sessions and Async Search initiatives, I would like to propose the following changes:

Introduce search.searchTimeout and remove elasticsearch.requestTimeout in async search

The elasticsearch.requestTimeout option is currently being used to limit the amount of time a series of async_search requests is allowed to run.

This use is incorrect and limiting, and hence we want to separate the timeout controlling this behavior into a separate setting: search.searchTimeout. This setting will default to a higher value (2\5 minutes TBD @giladgal) in 7.10, with the goal of making it unlimited by default in 7.11 (hence allowing a user running longer queries, as long as they stay on screen).

Doing so, will leave elasticsearch.requestTimeout to be a configuration used only by ESClient.

Use elasticsearch.shardTimeout on the server

Since the user can't (and doesn't need to) override the value of shardTimeout, we can use it on the server only. It should be sent with every search request. It can be used on the server side only, and removed from the client.

@lizozom lizozom added Feature:Search Querying infrastructure in Kibana v8.0.0 Team:AppArch v7.10.0 labels Aug 18, 2020
@lizozom lizozom self-assigned this Aug 18, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

@lukeelmers
Copy link
Member

Use elasticsearch.shardTimeout on the server

++ This makes sense to me. All of the usages of shardTimeout that I'm aware of are either already on the server (low-level search, timelion, tsvb), or can be configured on the server in the background (SearchSource uses an internal search route which could simply apply the settings behind the scenes).

Also worth noting that technically Vega is using shardTimeout on the client, but only to pass the value in to SearchSource so that usage could be removed too.

Introduce search.asyncTimeout and remove elasticsearch.requestTimeout

Introducing search.asyncTimeout makes sense to me. However I'm less sure about removing elasticsearch.requestTimeout... it feels like this could still be a useful setting for folks who aren't using async search, yeah?

If we were to remove it... is anybody outside of the data plugin using the setting? AFAICT nobody is, but I'm not 100% certain.

And if we were to keep it, I wonder if it should move from elasticsearch.requestTimeout to search.requestTimeout, so it all falls under the umbrella of the data plugin config.

Either way we probably have time to decide on that since we wouldn't be able to make this breaking change until 8.0, so there would have to be some interim period of time where we check for search.asyncTimeout and fallback to elasticsearch.requestTimeout.

@lukasolson
Copy link
Member

lukasolson commented Aug 18, 2020

Introducing search.asyncTimeout makes sense to me. However I'm less sure about removing elasticsearch.requestTimeout... it feels like this could still be a useful setting for folks who aren't using async search, yeah?

@lukeelmers I think there is just a misunderstanding here. We do not want to remove elasticsearch.requestTimeout entirely, we just want to replace its usage with a new setting in async search. So it will still limit the duration of any single request to Elasticsearch... We just won't use it to limit the entire duration of a series of async search requests.

@lukeelmers
Copy link
Member

Ahh thanks for clarifying... that makes much more sense then 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Search Querying infrastructure in Kibana v7.10.0 v8.0.0
Projects
None yet
4 participants