-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Configure IndexSearcher.maxClauseCount() based on Node characteristics #81525
Merged
Merged
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
599c297
Configure max clause size based on search pool size and max heap
romseygeek 7b7ae11
Merge remote-tracking branch 'origin/master' into search/max-clauses
romseygeek fa76baa
spotless
romseygeek 47e4949
Merge remote-tracking branch 'origin/master' into search/max-clauses
romseygeek 6f67b60
reworking
romseygeek ee6b47f
spotless
romseygeek 949b32f
test message
romseygeek 2627e4a
Move test that unmapped fields do not get resolved to unit test
romseygeek a053462
Merge remote-tracking branch 'origin/master' into search/max-clauses
romseygeek 691e7a9
Add entry to breaking changes
romseygeek 51976f2
Merge remote-tracking branch 'origin/master' into search/max-clauses
romseygeek 0c56f81
Lower min value to 1024; use fractional GB value
romseygeek 8990bb2
spotless
romseygeek baf1f4a
Merge remote-tracking branch 'origin/master' into search/max-clauses
romseygeek c34cdfe
deef
romseygeek File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,7 +16,7 @@ The default for the `action.destructive_requires_name` setting changes from `fal | |
to `true` in {es} 8.0.0. | ||
|
||
Previously, defaulting to `false` allowed users to use wildcard | ||
patterns to delete, close, or change index blocks on indices. | ||
patterns to delete, close, or change index blocks on indices. | ||
To prevent the accidental deletion of indices that happen to match a | ||
wildcard pattern, we now default to requiring that destructive | ||
operations explicitly name the indices to be modified. | ||
|
@@ -44,19 +44,28 @@ non-frozen node will result in an error on startup. | |
==== | ||
|
||
[[max_clause_count_change]] | ||
.The `indices.query.bool.max_clause_count` setting now limits all query clauses. | ||
.The `indices.query.bool.max_clause_count` setting has been deprecated, and no longer has any effect. | ||
[%collapsible] | ||
==== | ||
*Details* + | ||
Previously, the `indices.query.bool.max_clause_count` would apply to the number | ||
of clauses of a single `bool` query. It now applies to the total number of | ||
clauses of the rewritten query. To reduce chances of breaks, its | ||
default value has been bumped from 1024 to 4096. | ||
Elasticsearch will now dynamically set the maximum number of allowed clauses | ||
in a query, using a heuristic based on the size of the search thread pool and | ||
the size of the heap allocated to the JVM. This limit has a minimum value of | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm tempted to clarify that this number increases with heap size and decreases with the size of the threadpool, wdyt? |
||
1024 and will in most cases be larger (for example, a node with 30Gb RAM and | ||
48 CPUs will have a maximum clause count of around 27,000). Larger heaps lead | ||
to higher values, and larger thread pools result in lower values. | ||
|
||
*Impact* + | ||
Queries with many clauses should be avoided whenever possible. | ||
If you previously bumped this setting to accommodate heavy queries, | ||
you might need to increase it further. | ||
Queries with many clauses should be avoided whenever possible. | ||
If you previously bumped this setting to accommodate heavy queries, | ||
you might need to increase the amount of memory available to Elasticsearch, | ||
or to reduce the size of your search thread pool so that more memory is | ||
available to each concurrent search. | ||
|
||
In previous versions of Lucene you could get around this limit by nesting | ||
boolean queries within each other, but the limit is now based on the total | ||
number of leaf queries within the query as a whole and this workaround will | ||
no longer help. | ||
==== | ||
|
||
[[ilm-poll-interval-limit]] | ||
|
@@ -224,7 +233,7 @@ Remove the `http.content_type.required` setting from `elasticsearch.yml`. Specif | |
The `http.tcp_no_delay` setting was deprecated in 7.x and has been removed in 8.0. Use`http.tcp.no_delay` instead. | ||
|
||
*Impact* + | ||
Replace the `http.tcp_no_delay` setting with `http.tcp.no_delay`. | ||
Replace the `http.tcp_no_delay` setting with `http.tcp.no_delay`. | ||
Specifying `http.tcp_no_delay` in `elasticsearch.yml` will | ||
result in an error on startup. | ||
==== | ||
|
@@ -237,7 +246,7 @@ The `network.tcp.connect_timeout` setting was deprecated in 7.x and has been rem | |
was a fallback setting for `transport.connect_timeout`. | ||
|
||
*Impact* + | ||
Remove the`network.tcp.connect_timeout` setting. | ||
Remove the`network.tcp.connect_timeout` setting. | ||
Use the `transport.connect_timeout` setting to change the default connection | ||
timeout for client connections. Specifying | ||
`network.tcp.connect_timeout` in `elasticsearch.yml` will result in an | ||
|
@@ -282,7 +291,7 @@ since the 5.2 release of {es}. | |
|
||
*Impact* + | ||
Remove the `xpack.security.authz.store.roles.index.cache.max_size` | ||
and `xpack.security.authz.store.roles.index.cache.ttl` settings from `elasticsearch.yml` . | ||
and `xpack.security.authz.store.roles.index.cache.ttl` settings from `elasticsearch.yml` . | ||
Specifying these settings will result in an error on startup. | ||
==== | ||
|
||
|
This file contains 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
This file contains 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
This file contains 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we need a word about the fact that Elasticsearch now checks the number of clauses across the entire query rather than on a per-booleanquery basis? So that anyone who has used the hack that consists of splitting boolean queries knows that they can undo it.