Skip to content

Revise Default max concurrent search requests #31192

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

Closed
s1monw opened this issue Jun 8, 2018 · 5 comments
Closed

Revise Default max concurrent search requests #31192

s1monw opened this issue Jun 8, 2018 · 5 comments
Assignees
Labels
blocker :Search/Search Search-related issues that do not fall into other categories v7.0.0-beta1

Comments

@s1monw
Copy link
Contributor

s1monw commented Jun 8, 2018

Spinoff from #31171

We today use 5 * numNodes in the cluster as the default number of concurrent search requests issues by a single high level request. This has several issues ie. in the CCS case. There is also valid arguments that this number is arbitrary. Lets discuss this and find a good default that also works for CCS

@s1monw s1monw added blocker :Search/Search Search-related issues that do not fall into other categories v7.0.0 v6.4.0 labels Jun 8, 2018
@s1monw s1monw self-assigned this Jun 8, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

@s1monw
Copy link
Contributor Author

s1monw commented Jun 8, 2018

In #31171 I originally used the avg. number of shards per index in the high level request which had issues as @jpountz stated:

I'm a bit unhappy that it breaks the rule that searching N indices with 1 shard performs the same as searching 1 index with N shards.

I think what we want to achieve here is that we gain a certain level of concurrency while at the same time don't overload the cluster with a search request hitting a large number of shards on a small number of nodes. The smallish clusters with large shard counts are the critical ones here IMO if a query matching all shards causing rejections because the queue of the search threadpool on a single node fills up quickly. So with a queue size or 1000 on the search threadpool I do think we have quite some room here to have a simple default like 5 * numberOfNodes.

I also looked into the CCS issue which is not an issue after all since we take the number of nodes that participate in the request into account already.

In-fact the CCS way is actually better because it only takes into account the nodes that participate in the request. I wonder if we should change the way we use this number and limit the number of concurrent shards requests per node instead. We can then do this dynamically in InitialSearchPhase and queue up shards if we hit a node too hard as a protection mechanism. That would make the default simpler I think and more effective? ie if we set the default to 5 I think we have a good default. WDYT?

@jpountz
Copy link
Contributor

jpountz commented Jun 8, 2018

I think this metric makes more sense and is easier to reason about. +1

@ywelsch
Copy link
Contributor

ywelsch commented Jun 8, 2018

I wonder if we should change the way we use this number and limit the number of concurrent shards requests per node instead. We can then do this dynamically in InitialSearchPhase and queue up shards if we hit a node too hard as a protection mechanism. That would make the default simpler I think and more effective

+1 to making this a per-target node limit instead of a global one.

@jasontedor
Copy link
Member

+1

s1monw added a commit to s1monw/elasticsearch that referenced this issue Jun 8, 2018
With `max_concurrent_shard_requests` we used to throttle / limit
the number of concurrent shard requests a high level search request
can execute per node. This had several problems since it limited the
number on a global level based on the number of nodes. This change
now throttles the number of concurrent requests per node while still
allowing concurrency across multiple nodes.

Closes elastic#31192
s1monw added a commit that referenced this issue Jun 11, 2018
With `max_concurrent_shard_requests` we used to throttle / limit
the number of concurrent shard requests a high level search request
can execute per node. This had several problems since it limited the
number on a global level based on the number of nodes. This change
now throttles the number of concurrent requests per node while still
allowing concurrency across multiple nodes.

Closes #31192
@lcawl lcawl removed the v6.4.0 label Aug 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker :Search/Search Search-related issues that do not fall into other categories v7.0.0-beta1
Projects
None yet
Development

No branches or pull requests

7 participants