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

IndexSearcher max clause count #46433

Closed
jimczi opened this issue Sep 6, 2019 · 3 comments · Fixed by #81525
Closed

IndexSearcher max clause count #46433

jimczi opened this issue Sep 6, 2019 · 3 comments · Fixed by #81525
Labels
blocker :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team v8.0.0-rc1

Comments

@jimczi
Copy link
Contributor

jimczi commented Sep 6, 2019

In Lucene 9.0 index searchers will start to check the overall number of clauses in a Query and will throw an error if it is above a certain threshold. The default value is the same than the max boolean clause (1024) but is configurable per index searcher. This is breaking change so only Elasticsearch 8.0 could be affected by this but I am opening this issue to raise awareness and discuss the best options to introduce this change in the next major version. Today we have a setting in Elasticsearch that controls the maximum number of clauses that a single boolean query can handle (indices.query.bool.max_clause_count) and we use it in different layer as an hard limit (filter aggregations, fuzzy query expansion, ...). Since the checking of the number of clauses is now global to a query one first actionable item would be to change the name of the setting to reflect the new behavior. So instead of indices.query.bool.max_clause_count it would be indices.query.max_clause_count. Another point for the discussion is the handling of stored queries (percolator, ...) in Elasticsearch that breaks the new limit, should we accept them if they were created in a version where the limit was per boolean query ?

@jimczi jimczi added blocker :Search/Search Search-related issues that do not fall into other categories v8.0.0 labels Sep 6, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

@rharige
Copy link

rharige commented Jun 5, 2020

@jimczi @rjernst while the max-clause threshold value is known and set in configuration, users may also like to know what is the range of clauses they are hitting so they can tune/increase the threshold of maxclause setting appropriately. So can this information (clause-count) be added as part of maxclause exceeded error message?

@jpountz
Copy link
Contributor

jpountz commented Oct 12, 2021

@colings86 @qhoxie and I had a discussion about this problem and we'd like to propose the following plan:

  • The maximum number of clauses of boolean queries is no longer configurable as of 8.0. This aligns the experience with ESS where this setting is not allow-listed.
  • The maximum number of clauses is computed ergonomically depending on the heap size and the number of processors in a way that it would be possible to create slow queries, but almost impossible to create out-of-memory errors.

Finding out the right formula to determine the maximum number of clauses ergonomically is going to require some testing. I did some quick math for the worst-case scenario, which would give something like that in terms of overhead per boolean clause:

  • In-memory buffer on the case of niofs: BufferedIndexInput's 1kB buffer.
  • In memory buffer for doc IDs, freqs, positions, offsets, etc. 128 longs (8 bytes) each, so ~4kB total.
  • In-memory skip data and impacts.
  • Lucene objects: PostingsEnum, Scorer, etc.
    This gives maybe 8kB, so if we assume 16kB to have some safety margin, this suggests that we could go away with limiting the number of clauses to (heap_size_in_gb / search_pool_size) * 65,536?

If you have 1GB of heap and 8 processors, this gives a limit of ~5k clauses. If you have 30GB of heap and 48 processors, this gives a limit of ~27k clauses. (I'm assuming the default search threadpool size of num_processors * 3 / 2 + 1.)

There are other things that can consume heap in the JVM, so we couldn't use the entire heap only for query clauses. One assumption is that other memory consumers would hopefully check the real-memory circuit breaker so that they would figure out that they shouldn't allocate more memory because it's already used by query clauses. And maybe we could also find a way to check the real-memory circuit breaker while Lucene is building the tree of scorers to further improve protection against OOMEs.

@jtibshirani jtibshirani mentioned this issue Oct 12, 2021
16 tasks
romseygeek added a commit that referenced this issue Dec 17, 2021
#81525)

This commit deprecates the indices.query.bool.max_clause_count node setting,
and instead configures the maximum clause count for lucene based on the available
heap and the size of the thread pool.

Closes #46433
romseygeek added a commit to romseygeek/elasticsearch that referenced this issue Dec 17, 2021
elastic#81525)

This commit deprecates the indices.query.bool.max_clause_count node setting,
and instead configures the maximum clause count for lucene based on the available
heap and the size of the thread pool.

Closes elastic#46433
elasticsearchmachine pushed a commit that referenced this issue Dec 17, 2021
#81525) (#81850)

This commit deprecates the indices.query.bool.max_clause_count node setting,
and instead configures the maximum clause count for lucene based on the available
heap and the size of the thread pool.

Closes #46433
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 Team:Search Meta label for search team v8.0.0-rc1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants