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.

Sign up for GitHub

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

Jump to bottom

Limit the depth of nested queries #55303

Closed
jimczi opened this issue Apr 16, 2020 · 2 comments · Fixed by #66204
Closed

Limit the depth of nested queries #55303

jimczi opened this issue Apr 16, 2020 · 2 comments · Fixed by #66204
Labels
>bug :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team

Comments

@jimczi
Copy link
Contributor

jimczi commented Apr 16, 2020

Today we rely on recursion to parse, rewrite and create query builders that contains nested queries. This recursion can lead to stack overflow errors, for instance when dealing with a bool query that contains a chain of nested bool queries:

{
	"bool": {
		"should": {
			"boold": {
				"should": {
					"bool": {
						"should": {
							"bool": {
								"should": {
									...

These queries, usually created by mistake by a script, can have a negative impact on the cluster if they are not detected early.
We should have a mechanism to detect them and throw a comprehensive error when we see one in search or when trying to save a query.
@jimczi jimczi added >bug :Search/Search Search-related issues that do not fall into other categories labels Apr 16, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (:Search/Search)

@zacharymorn
Copy link
Contributor

Not sure if this solution would work:

  1. Define a depth limit system property for admin of cluster to set
  2. Change query processing method signatures to include depth accumulator
  3. At recursive call, caller add 1 to accumulator
  4. Upon entry to callee method, check accumulator against depth limit, and throw exception if exceeded

@rjernst rjernst added the Team:Search Meta label for search team label May 4, 2020
@jimczi jimczi mentioned this issue Nov 20, 2020
Closed
@chengyang14 chengyang14 mentioned this issue Dec 11, 2020
Merged
mayya-sharipova pushed a commit that referenced this issue Jan 12, 2021
@chengyang14
limit the depth of nested bool queries (#66204)
168d98b 
limit the depth of nested bool queries 

Introduce a new node level setting `indices.query.bool.max_nested_depth`
that controls the depth of nested bool queries.
Throw an error if a nested depth of a bool query exceeds the maximum
allowed nested depth.

Closes #55303
mayya-sharipova pushed a commit that referenced this issue Jan 13, 2021
@chengyang14
limit the depth of nested bool queries
9169e5a 
Limit the depth of nested bool queries

Introduce a new node level setting indices.query.bool.max_nested_depth
that controls the depth of nested bool queries.
Throw an error if a nested depth of a bool query exceeds the maximum
allowed nested depth.

Backport #66204
closes #55303
@gmmorris gmmorris mentioned this issue Jan 14, 2021
Closed
@pmuellr pmuellr mentioned this issue Jan 26, 2021
Merged
9 tasks
pmuellr added a commit to pmuellr/kibana that referenced this issue Jan 28, 2021
@pmuellr
[data] change KQL node builder to not generate recursive and/or clauses
fc7a242 
resolves elastic#88367

Prior to this PR, the KQL node_builder code was using recursion to generate
"and" & "or" expressions.  Eg, `and(foo1=bar1, foo2=bar2, foo3=bar3)` would
be generated as if was specified as `and(foo1=bar1, and(foo2=bar2, foo3=bar3))`.

Calls to the builder with long lists of expressions would generate nested JSON
as deep as the lists are long.  This is problematic, as Elasticsearch is
changing the default limit on nested bools to 20 levels, and alerting already
generates nested bools greater than that limit.
See: elastic/elasticsearch#55303

This PR changes the generated shape of above, so that all the nodes are at the
same level, instead of the previous "recursive" treatment.
pmuellr added a commit to pmuellr/kibana that referenced this issue Jan 28, 2021
@pmuellr
[data] change KQL node builder to not generate recursive and/or clauses
a48ba76 
resolves elastic#88367

Prior to this PR, the KQL node_builder code was using recursion to generate
"and" & "or" expressions.  Eg, `and(foo1=bar1, foo2=bar2, foo3=bar3)` would
be generated as if was specified as `and(foo1=bar1, and(foo2=bar2, foo3=bar3))`.

Calls to the builder with long lists of expressions would generate nested JSON
as deep as the lists are long.  This is problematic, as Elasticsearch is
changing the default limit on nested bools to 20 levels, and alerting already
generates nested bools greater than that limit.
See: elastic/elasticsearch#55303

This PR changes the generated shape of above, so that all the nodes are at the
same level, instead of the previous "recursive" treatment.
pmuellr added a commit to pmuellr/kibana that referenced this issue Jan 28, 2021
@pmuellr
[data] change KQL node builder to not generate recursive and/or clauses
fb0945c 
resolves elastic#88367

Prior to this PR, the KQL node_builder code was using recursion to generate
"and" & "or" expressions.  Eg, `and(foo1=bar1, foo2=bar2, foo3=bar3)` would
be generated as if was specified as `and(foo1=bar1, and(foo2=bar2, foo3=bar3))`.

Calls to the builder with long lists of expressions would generate nested JSON
as deep as the lists are long.  This is problematic, as Elasticsearch is
changing the default limit on nested bools to 20 levels, and alerting already
generates nested bools greater than that limit.
See: elastic/elasticsearch#55303

This PR changes the generated shape of above, so that all the nodes are at the
same level, instead of the previous "recursive" treatment.
pmuellr added a commit to pmuellr/kibana that referenced this issue Jan 28, 2021
@pmuellr
[data] change KQL node builder to not generate recursive and/or clauses
c3361b2 
resolves elastic#88367

Prior to this PR, the KQL node_builder code was using recursion to generate
"and" & "or" expressions.  Eg, `and(foo1=bar1, foo2=bar2, foo3=bar3)` would
be generated as if was specified as `and(foo1=bar1, and(foo2=bar2, foo3=bar3))`.

Calls to the builder with long lists of expressions would generate nested JSON
as deep as the lists are long.  This is problematic, as Elasticsearch is
changing the default limit on nested bools to 20 levels, and alerting already
generates nested bools greater than that limit.
See: elastic/elasticsearch#55303

This PR changes the generated shape of above, so that all the nodes are at the
same level, instead of the previous "recursive" treatment.
pmuellr added a commit to pmuellr/kibana that referenced this issue Jan 28, 2021
@pmuellr
[data] change KQL node builder to not generate recursive and/or clauses
598b23d 
resolves elastic#88367

Prior to this PR, the KQL node_builder code was using recursion to generate
"and" & "or" expressions.  Eg, `and(foo1=bar1, foo2=bar2, foo3=bar3)` would
be generated as if was specified as `and(foo1=bar1, and(foo2=bar2, foo3=bar3))`.

Calls to the builder with long lists of expressions would generate nested JSON
as deep as the lists are long.  This is problematic, as Elasticsearch is
changing the default limit on nested bools to 20 levels, and alerting already
generates nested bools greater than that limit.
See: elastic/elasticsearch#55303

This PR changes the generated shape of above, so that all the nodes are at the
same level, instead of the previous "recursive" treatment.
pmuellr added a commit to pmuellr/kibana that referenced this issue Jan 29, 2021
@pmuellr
[data] change KQL node builder to not generate recursive and/or clauses
49e715c 
resolves elastic#88367

Prior to this PR, the KQL node_builder code was using recursion to generate
"and" & "or" expressions.  Eg, `and(foo1=bar1, foo2=bar2, foo3=bar3)` would
be generated as if was specified as `and(foo1=bar1, and(foo2=bar2, foo3=bar3))`.

Calls to the builder with long lists of expressions would generate nested JSON
as deep as the lists are long.  This is problematic, as Elasticsearch is
changing the default limit on nested bools to 20 levels, and alerting already
generates nested bools greater than that limit.
See: elastic/elasticsearch#55303

This PR changes the generated shape of above, so that all the nodes are at the
same level, instead of the previous "recursive" treatment.
pmuellr added a commit to elastic/kibana that referenced this issue Feb 1, 2021
@pmuellr
[data] change KQL node builder to not generate recursive and/or claus…
e4c344a 
…es (#89345)

resolves #88367

Prior to this PR, the KQL node_builder code was using recursion to generate
"and" & "or" expressions.  Eg, `and(foo1=bar1, foo2=bar2, foo3=bar3)` would
be generated as if was specified as `and(foo1=bar1, and(foo2=bar2, foo3=bar3))`.

Calls to the builder with long lists of expressions would generate nested JSON
as deep as the lists are long.  This is problematic, as Elasticsearch is
changing the default limit on nested bools to 20 levels, and alerting already
generates nested bools greater than that limit.
See: elastic/elasticsearch#55303

This PR changes the generated shape of above, so that all the nodes are at the
same level, instead of the previous "recursive" treatment.
pmuellr added a commit to pmuellr/kibana that referenced this issue Feb 1, 2021
@pmuellr
[data] change KQL node builder to not generate recursive and/or claus…
56c3e96 
…es (elastic#89345)

resolves elastic#88367

Prior to this PR, the KQL node_builder code was using recursion to generate
"and" & "or" expressions.  Eg, `and(foo1=bar1, foo2=bar2, foo3=bar3)` would
be generated as if was specified as `and(foo1=bar1, and(foo2=bar2, foo3=bar3))`.

Calls to the builder with long lists of expressions would generate nested JSON
as deep as the lists are long.  This is problematic, as Elasticsearch is
changing the default limit on nested bools to 20 levels, and alerting already
generates nested bools greater than that limit.
See: elastic/elasticsearch#55303

This PR changes the generated shape of above, so that all the nodes are at the
same level, instead of the previous "recursive" treatment.
pmuellr added a commit to elastic/kibana that referenced this issue Feb 1, 2021
@pmuellr
[data] change KQL node builder to not generate recursive and/or claus…
7c9028a 
…es (#89345) (#89890)

resolves #88367

Prior to this PR, the KQL node_builder code was using recursion to generate
"and" & "or" expressions.  Eg, `and(foo1=bar1, foo2=bar2, foo3=bar3)` would
be generated as if was specified as `and(foo1=bar1, and(foo2=bar2, foo3=bar3))`.

Calls to the builder with long lists of expressions would generate nested JSON
as deep as the lists are long.  This is problematic, as Elasticsearch is
changing the default limit on nested bools to 20 levels, and alerting already
generates nested bools greater than that limit.
See: elastic/elasticsearch#55303

This PR changes the generated shape of above, so that all the nodes are at the
same level, instead of the previous "recursive" treatment.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
>bug :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

limit the depth of nested bool queries chengyang14/elasticsearch
4 participants
@rjernst @zacharymorn @elasticmachine @jimczi