Make limit on number of expanded fields configurable#35284
Merged
cbuescher merged 4 commits intoelastic:masterfrom Nov 8, 2018
Merged
Make limit on number of expanded fields configurable#35284cbuescher merged 4 commits intoelastic:masterfrom
cbuescher merged 4 commits intoelastic:masterfrom
Conversation
Currently we introduced a hard limit of 1024 to the number of fields a query can be expanded to in elastic#26541. Instead of using a hard limit, we should make this configurable. This change removes the hard limit check and uses the existing `max_clause_count` setting instead. Closes elastic#34778
Collaborator
|
Pinging @elastic/es-search-aggs |
cbuescher
commented
Nov 6, 2018
Member
Author
cbuescher
left a comment
There was a problem hiding this comment.
I just realize that indices.query.bool.max_clause_count isn't dynamically updateable, I don't know if we touched that point in our discussion that led to reusing this setting for the limit for the expansion of fields as well. Just would like to make whoever reviews this aware of this and ask for their opinion.
jimczi
reviewed
Nov 6, 2018
Contributor
jimczi
left a comment
There was a problem hiding this comment.
Thanks @cbuescher . I left some comments.
I just realize that indices.query.bool.max_clause_count isn't dynamically updateable
I think it's fine. I am not even sure that we should document this extensively. Querying more than 1024 fields is an anti-pattern so new users don't need to be aware of this.
| at once. | ||
| the metadata fields. There is a limit on the number of fields that can be queried | ||
| at once which is defined by the `indices.query.bool.max_clause_count` <<search-settings>> | ||
| which defaults to 1024. |
Contributor
There was a problem hiding this comment.
Can you also add a note in the multi_match query ?
|
|
||
| @BeforeClass | ||
| public static void createRandomClusterSetting() { | ||
| CLUSTER_MAX_CLAUSE_COUNT = randomIntBetween(500, 1500); |
Contributor
There was a problem hiding this comment.
Can we test with small numbers, this seems big for a simple ut.
Member
Author
There was a problem hiding this comment.
Sure, the test was running with the default of 1024 before, so I wanted to stay in that area but there's no need to. Will lower it to the hundreds if that sounds okay.
added 2 commits
November 6, 2018 14:02
Member
Author
|
@jimczi thanks for the first round of reviews, I pushed an update |
cbuescher
pushed a commit
to cbuescher/elasticsearch
that referenced
this pull request
Nov 6, 2018
With elastic#35284 changing the limit on the amount of fields that a query can be expanded to to be determined by the `indices.query.bool.max_clause_count` setting, the changes the deprecation warning and docs in 6.x accordingly. Relates to elastic#35284
cbuescher
pushed a commit
that referenced
this pull request
Nov 8, 2018
ppf2
added a commit
that referenced
this pull request
Aug 13, 2020
>Limiting the number of auto-expanded fieldsedit
Executing queries that use automatic expansion of fields (e.g. query_string, simple_query_string or multi_match) can have performance issues for indices with a large numbers of fields. To safeguard against this, a hard limit of 1024 fields has been introduced for queries using the "all fields" mode ("default_field": "") or other fieldname expansions (e.g. "foo").
Per #35284, it looks like we changed this from a hard limit to a soft limit by leveraging `indices.query.bool.max_clause_count` in 7.0 beta1. We will want to backport this to all 7.x versions.
jrodewig
pushed a commit
that referenced
this pull request
Aug 18, 2020
Per #35284, it looks like we changed this from a max field expansions limit to a soft limit using the `indices.query.bool.max_clause_count` dynamic cluster settting.
jrodewig
pushed a commit
that referenced
this pull request
Aug 18, 2020
Per #35284, it looks like we changed this from a max field expansions limit to a soft limit using the `indices.query.bool.max_clause_count` dynamic cluster settting.
jrodewig
pushed a commit
that referenced
this pull request
Aug 18, 2020
Per #35284, it looks like we changed this from a max field expansions limit to a soft limit using the `indices.query.bool.max_clause_count` dynamic cluster settting.
jrodewig
pushed a commit
that referenced
this pull request
Aug 18, 2020
Per #35284, it looks like we changed this from a max field expansions limit to a soft limit using the `indices.query.bool.max_clause_count` dynamic cluster settting.
jrodewig
pushed a commit
that referenced
this pull request
Aug 18, 2020
Per #35284, it looks like we changed this from a max field expansions limit to a soft limit using the `indices.query.bool.max_clause_count` dynamic cluster settting.
jrodewig
pushed a commit
that referenced
this pull request
Aug 18, 2020
Per #35284, it looks like we changed this from a max field expansions limit to a soft limit using the `indices.query.bool.max_clause_count` dynamic cluster settting.
jrodewig
pushed a commit
that referenced
this pull request
Aug 18, 2020
Per #35284, it looks like we changed this from a max field expansions limit to a soft limit using the `indices.query.bool.max_clause_count` dynamic cluster settting.
jrodewig
pushed a commit
that referenced
this pull request
Aug 18, 2020
Per #35284, it looks like we changed this from a max field expansions limit to a soft limit using the `indices.query.bool.max_clause_count` dynamic cluster settting.
jrodewig
pushed a commit
that referenced
this pull request
Aug 18, 2020
Per #35284, it looks like we changed this from a max field expansions limit to a soft limit using the `indices.query.bool.max_clause_count` dynamic cluster settting.
jrodewig
pushed a commit
that referenced
this pull request
Aug 18, 2020
Per #35284, it looks like we changed this from a max field expansions limit to a soft limit using the `indices.query.bool.max_clause_count` dynamic cluster settting.
jrodewig
pushed a commit
that referenced
this pull request
Aug 18, 2020
Per #35284, it looks like we changed this from a max field expansions limit to a soft limit using the `indices.query.bool.max_clause_count` dynamic cluster settting.
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
4 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.
Currently we introduced a hard limit of 1024 to the number of fields a query can
be expanded to in #26541. Instead of using a hard limit, we should make this
configurable. This change removes the hard limit check and uses the existing
max_clause_countsetting instead.Closes #34778