indices.query.bool.max_clause_count now limits all query clauses#75297
Merged
jpountz merged 7 commits intoelastic:masterfrom Jul 21, 2021
Merged
indices.query.bool.max_clause_count now limits all query clauses#75297jpountz merged 7 commits intoelastic:masterfrom
indices.query.bool.max_clause_count now limits all query clauses#75297jpountz merged 7 commits intoelastic:masterfrom
Conversation
In the upcoming Lucene 9 release, `indices.query.bool.max_clause_count` is going to apply to the entire query tree rather than per `bool` query. In order to avoid breaks, the limit has been bumped from 1024 to 4096. The semantics will effectively change when we upgrade to Lucene 9, this PR is only about agreeing on a migration strategy and documenting this change. To avoid further breaks, I am leaning towards keeping the current setting name even though it contains `bool`. I believe that it still makes sense given that `bool` queries are typically the main contributors to high numbers of clauses.
Collaborator
|
Pinging @elastic/es-search (Team:Search) |
romseygeek
reviewed
Jul 13, 2021
Contributor
romseygeek
left a comment
There was a problem hiding this comment.
Looks good! I have a couple of questions.
| public class SearchModule { | ||
| public static final Setting<Integer> INDICES_MAX_CLAUSE_COUNT_SETTING = Setting.intSetting("indices.query.bool.max_clause_count", | ||
| 1024, 1, Integer.MAX_VALUE, Setting.Property.NodeScope); | ||
| 2048, 1, Integer.MAX_VALUE, Setting.Property.NodeScope); |
Contributor
There was a problem hiding this comment.
I think this should be 4096?
Contributor
Author
There was a problem hiding this comment.
I went back and forth between 2048 and 4096, thanks for catching!
| from becoming too large, and taking up too much CPU and memory. In case you're considering | ||
| increasing this setting, make sure you've exhausted all other options to avoid having to do this. | ||
| Higher values can lead to performance degradations and memory issues, especially in clusters with | ||
| a high load or few resources. |
Contributor
There was a problem hiding this comment.
I wonder if it's worth mentioning terms queries and index_prefix settings here as well?
Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com>
indices.query.bool.max_clause_count.indices.query.bool.max_clause_count now limits all query clauses
jpountz
added a commit
to jpountz/elasticsearch
that referenced
this pull request
Jul 22, 2021
They are due to changes to `indices.query.bool.max_clause_count` that got pushed in elastic#75297. Closes elastic#75591 Closes elastic#75592
jpountz
added a commit
that referenced
this pull request
Jul 22, 2021
ywangd
pushed a commit
to ywangd/elasticsearch
that referenced
this pull request
Jul 30, 2021
…lastic#75297) In the upcoming Lucene 9 release, `indices.query.bool.max_clause_count` is going to apply to the entire query tree rather than per `bool` query. In order to avoid breaks, the limit has been bumped from 1024 to 4096. The semantics will effectively change when we upgrade to Lucene 9, this PR is only about agreeing on a migration strategy and documenting this change. To avoid further breaks, I am leaning towards keeping the current setting name even though it contains `bool`. I believe that it still makes sense given that `bool` queries are typically the main contributors to high numbers of clauses. Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com>
ywangd
pushed a commit
to ywangd/elasticsearch
that referenced
this pull request
Jul 30, 2021
They are due to changes to `indices.query.bool.max_clause_count` that got pushed in elastic#75297. Closes elastic#75591 Closes elastic#75592
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In the upcoming Lucene 9 release,
indices.query.bool.max_clause_countisgoing to apply to the entire query tree rather than per
boolquery. In orderto avoid breaks, the limit has been bumped from 1024 to 4096.
The semantics will effectively change when we upgrade to Lucene 9, this PR
is only about agreeing on a migration strategy and documenting this change.
To avoid further breaks, I am leaning towards keeping the current setting name
even though it contains
bool. I believe that it still makes sense given thatboolqueries are typically the main contributors to high numbers of clauses.