From 52b92b68ba4de4d06e1f8925d67dd3caacc70a8f Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Tue, 18 Oct 2022 16:05:46 +0200 Subject: [PATCH 1/6] [TEST] Limit number of objects created by testMaxNestedDepth Stop testing that the limit gets hit when the value is the default one, as that causes a lot of objects being created which can cause OOM when executing the test. Instead, added a specific test to verify that the default value is being set appropriately based on its corresponding setting. Closes #90984 Closes #90985 Closes #90986 --- .../index/query/AbstractQueryBuilder.java | 6 +++++- .../elasticsearch/search/SearchModuleTests.java | 15 +++++++++++++++ .../elasticsearch/test/AbstractQueryTestCase.java | 10 ++-------- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java index 3a5e2b3b7e989..0e3f2bac857d4 100644 --- a/server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java @@ -50,7 +50,7 @@ public abstract class AbstractQueryBuilder> public static final ParseField NAME_FIELD = new ParseField("_name"); public static final ParseField BOOST_FIELD = new ParseField("boost"); - private static int maxNestedDepth = 20; + private static int maxNestedDepth; protected String queryName; protected float boost = DEFAULT_BOOST; @@ -427,6 +427,10 @@ public static void setMaxNestedDepth(int maxNestedDepth) { AbstractQueryBuilder.maxNestedDepth = maxNestedDepth; } + public static int getMaxNestedDepth() { + return maxNestedDepth; + } + @Override public final String toString() { return Strings.toString(this, true, true); diff --git a/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java b/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java index 9fe6829861ba1..8cacfb3f98552 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java @@ -16,6 +16,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; import org.elasticsearch.core.RestApiVersion; +import org.elasticsearch.index.query.AbstractQueryBuilder; import org.elasticsearch.index.query.CommonTermsQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryRewriteContext; @@ -779,4 +780,18 @@ public List> getQueries() { assertTrue(RestApiVersion.minimumSupported().matches(compatEntry.get(0).restApiCompatibility)); assertFalse(RestApiVersion.current().matches(compatEntry.get(0).restApiCompatibility)); } + + public void testDefaultMaxNestedDepth() { + new SearchModule(Settings.EMPTY, emptyList()); + assertEquals(SearchModule.INDICES_MAX_NESTED_DEPTH_SETTING.getDefault(Settings.EMPTY).intValue(), + AbstractQueryBuilder.getMaxNestedDepth()); + } + + public void testCustomMaxNestedDepth() { + int customValue = randomIntBetween(1, 100); + Settings settings = Settings.builder().put( + SearchModule.INDICES_MAX_NESTED_DEPTH_SETTING.getKey(), customValue).build(); + new SearchModule(settings, emptyList()); + assertEquals(customValue, AbstractQueryBuilder.getMaxNestedDepth()); + } } diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java index 666cfc457763c..9172857dd3e71 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java @@ -99,7 +99,6 @@ protected QB createQueryWithInnerQuery(QueryBuilder queryBuilder) { throw new UnsupportedOperationException(); } - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/90984") public void testMaxNestedDepth() throws IOException { QB query = null; try { @@ -107,13 +106,8 @@ public void testMaxNestedDepth() throws IOException { } catch (UnsupportedOperationException e) { assumeNoException("Runs only for queries that support nesting", e); } - int maxDepth; - if (frequently()) { - maxDepth = randomIntBetween(3, 5); - AbstractQueryBuilder.setMaxNestedDepth(maxDepth); - } else { - maxDepth = INDICES_MAX_NESTED_DEPTH_SETTING.getDefault(Settings.EMPTY); - } + int maxDepth = randomIntBetween(3, 5); + AbstractQueryBuilder.setMaxNestedDepth(maxDepth); try { for (int i = 1; i < maxDepth - 1; i++) { query = createQueryWithInnerQuery(query); From 77da94d7a522508fa3aa14576757346e48d47a43 Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Tue, 18 Oct 2022 16:10:52 +0200 Subject: [PATCH 2/6] spotless --- .../java/org/elasticsearch/search/SearchModuleTests.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java b/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java index 8cacfb3f98552..fa3bd155a001e 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java @@ -783,14 +783,15 @@ public List> getQueries() { public void testDefaultMaxNestedDepth() { new SearchModule(Settings.EMPTY, emptyList()); - assertEquals(SearchModule.INDICES_MAX_NESTED_DEPTH_SETTING.getDefault(Settings.EMPTY).intValue(), - AbstractQueryBuilder.getMaxNestedDepth()); + assertEquals( + SearchModule.INDICES_MAX_NESTED_DEPTH_SETTING.getDefault(Settings.EMPTY).intValue(), + AbstractQueryBuilder.getMaxNestedDepth() + ); } public void testCustomMaxNestedDepth() { int customValue = randomIntBetween(1, 100); - Settings settings = Settings.builder().put( - SearchModule.INDICES_MAX_NESTED_DEPTH_SETTING.getKey(), customValue).build(); + Settings settings = Settings.builder().put(SearchModule.INDICES_MAX_NESTED_DEPTH_SETTING.getKey(), customValue).build(); new SearchModule(settings, emptyList()); assertEquals(customValue, AbstractQueryBuilder.getMaxNestedDepth()); } From 860cd5a73ce415811a963ae9bb3f1a14e13b6a58 Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Tue, 18 Oct 2022 16:34:51 +0200 Subject: [PATCH 3/6] iter --- .../org/elasticsearch/index/query/AbstractQueryBuilder.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java index 0e3f2bac857d4..68a824a26b896 100644 --- a/server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java @@ -17,7 +17,9 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.lucene.BytesRefs; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.SuggestingErrorOnUnknown; +import org.elasticsearch.search.SearchModule; import org.elasticsearch.xcontent.AbstractObjectParser; import org.elasticsearch.xcontent.FilterXContentParser; import org.elasticsearch.xcontent.FilterXContentParserWrapper; @@ -49,8 +51,8 @@ public abstract class AbstractQueryBuilder> public static final float DEFAULT_BOOST = 1.0f; public static final ParseField NAME_FIELD = new ParseField("_name"); public static final ParseField BOOST_FIELD = new ParseField("boost"); - - private static int maxNestedDepth; + //We set the default value for tests that don't go through SearchModule + private static int maxNestedDepth = SearchModule.INDICES_MAX_CLAUSE_COUNT_SETTING.getDefault(Settings.EMPTY); protected String queryName; protected float boost = DEFAULT_BOOST; From 231aefc0820117fd4635e59286ff6bbe3a7403c8 Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Tue, 18 Oct 2022 17:02:43 +0200 Subject: [PATCH 4/6] spotless --- .../org/elasticsearch/index/query/AbstractQueryBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java index 68a824a26b896..850d0142d8d18 100644 --- a/server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java @@ -51,7 +51,7 @@ public abstract class AbstractQueryBuilder> public static final float DEFAULT_BOOST = 1.0f; public static final ParseField NAME_FIELD = new ParseField("_name"); public static final ParseField BOOST_FIELD = new ParseField("boost"); - //We set the default value for tests that don't go through SearchModule + // We set the default value for tests that don't go through SearchModule private static int maxNestedDepth = SearchModule.INDICES_MAX_CLAUSE_COUNT_SETTING.getDefault(Settings.EMPTY); protected String queryName; From fc7d24456d9b5393372a1aa9a5e15c5fda6f6e2b Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Tue, 18 Oct 2022 17:03:23 +0200 Subject: [PATCH 5/6] iter --- .../org/elasticsearch/index/query/AbstractQueryBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java index 850d0142d8d18..fa44c84beae3c 100644 --- a/server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java @@ -52,7 +52,7 @@ public abstract class AbstractQueryBuilder> public static final ParseField NAME_FIELD = new ParseField("_name"); public static final ParseField BOOST_FIELD = new ParseField("boost"); // We set the default value for tests that don't go through SearchModule - private static int maxNestedDepth = SearchModule.INDICES_MAX_CLAUSE_COUNT_SETTING.getDefault(Settings.EMPTY); + private static int maxNestedDepth = INDICES_MAX_NESTED_DEPTH_SETTING.getDefault(Settings.EMPTY); protected String queryName; protected float boost = DEFAULT_BOOST; From 5b5d4bf0434bb4e2910fafd7ae89c7607f3452e4 Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Tue, 18 Oct 2022 17:19:55 +0200 Subject: [PATCH 6/6] spotless again --- .../java/org/elasticsearch/index/query/AbstractQueryBuilder.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java index fa44c84beae3c..1ef974850fede 100644 --- a/server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java @@ -19,7 +19,6 @@ import org.elasticsearch.common.lucene.BytesRefs; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.SuggestingErrorOnUnknown; -import org.elasticsearch.search.SearchModule; import org.elasticsearch.xcontent.AbstractObjectParser; import org.elasticsearch.xcontent.FilterXContentParser; import org.elasticsearch.xcontent.FilterXContentParserWrapper;