Allow setting max_concurrent_shard_requests for _msearch

[s1monw]

Elasticsearch will expose max_concurrent_shard_requests in _msearch in 6.5. In order to resolve issues like elastic/elasticsearch#31877 kibana should expose this setting to make sure environments with a low number of concurrent requests can tune dashboard performance by executing in parallel or at least make use of the available concurrency

The PR exposing max_concurrent_shard_requests in _msearch is here: elastic/elasticsearch#33016

[s1monw]

Quoting @epixa here:

It seems to me that that setting could be set Kibana-wide. I don’t see any reason to do it on a dashboard-by-dashboard basis, for example.Then courier would just add it to all msearches.

ping @elastic/kibana-discovery

[epixa]

I think we'll need to update elasticsearch-js to support an additional search parameter for msearch: https://www.elastic.co/guide/en/elasticsearch/client/javascript-api/current/api-reference.html#api-msearch cc @spalger

[Bargs]

I looked into this and came to the same conclusion, elasticsearch.js will probably need an update. Once that's in place the update to Kibana should be simple. I'm not sure what the process is for generating the API modules in es.js so I'd need @spalger's help.

[spalger]

elasticsearch.js will be updated once the API spec is updated (and the script is rerun by @delvedor) but any unknown keys are passed through as query string parameters so changes to the client aren't required to start using this new parameter.

[s1monw]

elasticsearch.js will be updated once the API spec is updated (and the script is rerun by @delvedor) but any unknown keys are passed through as query string parameters so changes to the client aren't required to start using this new parameter.

@spalger @Bargs the spec has been changed for 6.x and master. 2 days ago so I think you are just left with @delvedor running the script

[s1monw]

@spalger @Bargs I learned today that there are multiple places in kibana that execute searches and not just msearch. I think this needs to be reopened and also passed to all different places where we search on elasticsearch?

[spalger]

Hmm, max_concurrent_shard_requests needs to be set for every search request we do? That's a reasonable amount of work, we do searches in a lot of places.

[rayafratkina]

cc @stacey-gammon @timroes
I think we should create separate issues for other places and prioritize them according to how important the change is in the relevant app

[elasticmachine]

Pinging @elastic/kibana-app

[s1monw]

Hmm, max_concurrent_shard_requests needs to be set for every search request we do? That's a reasonable amount of work, we do searches in a lot of places.

my assumption when I opened the issue was that we only do msearch which caused the confusion. It think if we offer this we should to it across the board? Are the other places in kibana also hitting multiple indices?

[timroes]

Looking at the ES PR, that Simon linked (elastic/elasticsearch#33016), it seems this parameter is only relevant for _msearch. We're only using _msearch in Kibana in two places: Courier (where @Bargs already implemented that) and in TSVB.

So we still need to fix that in TSVB. The TSVB behavior might anyway change soon to using _search due to rollup support, so not sure if we should touch that now.

Looking through the code it seems that @elastic/infrastructure-ui is also using msearch so they should check if they are using that anywhere.

[spalger]

There are places like vega, timelion, tsvb, where the user can configure which indices to query that don't use our default search abstraction (courier). Other apps like monitoring, ML, maybe APM, Infraops, might need to incorporate a setting like this because they can query over arbitrary time ranges and I believe store their data in time based indices, while most of the time querying ES mostly direct.

Looking at the PR it seems this parameter is only relevant for _msearch.

@timroes I think that's what @s1monw is saying was a miscommunication in #22206 (comment)

[spalger]

@s1monw if we need to send this for every search request it makes me wonder why it's not a default configured in Elasticsearch?

[s1monw]

@s1monw if we need to send this for every search request it makes me wonder why it's not a default configured in Elasticsearch?

this is a special case if you have for instance only a single node cluster you wanna max it out. The way how ES gains concurrency is on a request level this instead give you more concurrency per request. also you apparently only set it if the user sets it. so you are on-par with the default in ES. I think there are certain request parameters folks want access to so we should pass them on consistently.

[lukasolson]

Closing this out. We are encouraging all applications and solutions to migrate to the data plugin search service, which properly sets this parameter.