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

Set INDICES_MAX_CLAUSE_COUNT dynamically #13568

Merged
merged 12 commits into from
Jun 18, 2024
Merged

Set INDICES_MAX_CLAUSE_COUNT dynamically #13568

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
dd140e2
Set INDICES_MAX_CLAUSE_COUNT dynamically
harshavamsi May 6, 2024
4ff1d13
Adds new test to verify clause count is updated
harshavamsi May 7, 2024
6321ca8
Changelog
harshavamsi May 7, 2024
05f8d30
Merge main
harshavamsi Jun 5, 2024
a65cecc
Merge main
harshavamsi Jun 5, 2024
15d3f96
Revert static
harshavamsi Jun 5, 2024
75331e8
Update test case to change defaults
harshavamsi Jun 5, 2024
7ede1eb
Merge from main
harshavamsi Jun 6, 2024
dd840fa
Spotless
harshavamsi Jun 6, 2024
e8a8f4d
Revert max_clause count from being volatile
harshavamsi Jun 13, 2024
cd92a9b
Merge branch 'main' into dynamic_max_clause
harshavamsi Jun 13, 2024
e715516
Fix changelog
harshavamsi Jun 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions server/src/internalClusterTest/java/org/opensearch/search/query/QueryStringIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import org.opensearch.search.SearchHit;
import org.opensearch.search.SearchHits;
import org.opensearch.search.SearchModule;
import org.opensearch.search.SearchService;
import org.opensearch.test.ParameterizedStaticSettingsOpenSearchIntegTestCase;
import org.junit.Before;
import org.junit.BeforeClass;
Expand Down Expand Up @@ -87,7 +88,7 @@ public static Collection<Object[]> parameters() {

@BeforeClass
public static void createRandomClusterSetting() {
CLUSTER_MAX_CLAUSE_COUNT = randomIntBetween(50, 100);
CLUSTER_MAX_CLAUSE_COUNT = randomIntBetween(1024, 2048);
}

@Before
Expand All @@ -101,7 +102,7 @@ public void setup() throws Exception {
protected Settings nodeSettings(int nodeOrdinal) {
return Settings.builder()
.put(super.nodeSettings(nodeOrdinal))
.put(SearchModule.INDICES_MAX_CLAUSE_COUNT_SETTING.getKey(), CLUSTER_MAX_CLAUSE_COUNT)
.put(SearchService.INDICES_MAX_CLAUSE_COUNT_SETTING.getKey(), CLUSTER_MAX_CLAUSE_COUNT)
.build();
}

Expand Down
6 changes: 3 additions & 3 deletions server/src/internalClusterTest/java/org/opensearch/search/query/SimpleQueryStringIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
import org.opensearch.plugins.Plugin;
import org.opensearch.search.SearchHit;
import org.opensearch.search.SearchHits;
import org.opensearch.search.SearchModule;
import org.opensearch.search.SearchService;
import org.opensearch.search.builder.SearchSourceBuilder;
import org.opensearch.test.ParameterizedStaticSettingsOpenSearchIntegTestCase;
import org.junit.BeforeClass;
Expand Down Expand Up @@ -115,14 +115,14 @@ public static void createRandomClusterSetting() {
// Lower bound can't be small(such as 60), simpleQueryStringQuery("foo Bar 19 127.0.0.1") in testDocWithAllTypes
// will create many clauses of BooleanClause, In that way, it will throw too_many_nested_clauses exception.
// So we need to set a higher bound(such as 80) to avoid failures.
CLUSTER_MAX_CLAUSE_COUNT = randomIntBetween(80, 100);
CLUSTER_MAX_CLAUSE_COUNT = randomIntBetween(1024, 2048);
}

@Override
protected Settings nodeSettings(int nodeOrdinal) {
return Settings.builder()
.put(super.nodeSettings(nodeOrdinal))
.put(SearchModule.INDICES_MAX_CLAUSE_COUNT_SETTING.getKey(), CLUSTER_MAX_CLAUSE_COUNT)
.put(SearchService.INDICES_MAX_CLAUSE_COUNT_SETTING.getKey(), CLUSTER_MAX_CLAUSE_COUNT)
.build();
}

Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/opensearch/common/settings/ClusterSettings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,7 @@ public void apply(Settings value, Settings current, Settings previous) {
SearchService.MAX_OPEN_PIT_CONTEXT,
SearchService.MAX_PIT_KEEPALIVE_SETTING,
SearchService.MAX_AGGREGATION_REWRITE_FILTERS,
SearchService.INDICES_MAX_CLAUSE_COUNT_SETTING,
CreatePitController.PIT_INIT_KEEP_ALIVE,
Node.WRITE_PORTS_FILE_SETTING,
Node.NODE_NAME_SETTING,
Expand Down Expand Up @@ -581,7 +582,6 @@ public void apply(Settings value, Settings current, Settings previous) {
ResourceWatcherService.RELOAD_INTERVAL_HIGH,
ResourceWatcherService.RELOAD_INTERVAL_MEDIUM,
ResourceWatcherService.RELOAD_INTERVAL_LOW,
SearchModule.INDICES_MAX_CLAUSE_COUNT_SETTING,
ThreadPool.ESTIMATED_TIME_INTERVAL_SETTING,
FastVectorHighlighter.SETTING_TV_HIGHLIGHT_MULTI_VALUE,
Node.BREAKER_TYPE_KEY,
Expand Down
3 changes: 2 additions & 1 deletion server/src/main/java/org/opensearch/index/search/QueryParserHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import org.opensearch.index.query.QueryShardContext;
import org.opensearch.index.query.QueryShardException;
import org.opensearch.search.SearchModule;
import org.opensearch.search.SearchService;

import java.util.Collection;
import java.util.HashMap;
Expand Down Expand Up @@ -177,7 +178,7 @@ static Map<String, Float> resolveMappingField(
}

static void checkForTooManyFields(int numberOfFields, QueryShardContext context, @Nullable String inputPattern) {
Integer limit = SearchModule.INDICES_MAX_CLAUSE_COUNT_SETTING.get(context.getIndexSettings().getSettings());
Integer limit = SearchService.INDICES_MAX_CLAUSE_COUNT_SETTING.get(context.getIndexSettings().getSettings());
msfroh marked this conversation as resolved.
Show resolved Hide resolved
if (numberOfFields > limit) {
StringBuilder errorMsg = new StringBuilder("field expansion ");
if (inputPattern != null) {
Expand Down
8 changes: 0 additions & 8 deletions server/src/main/java/org/opensearch/search/SearchModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,13 +302,6 @@
* @opensearch.internal
*/
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
);

private final Map<String, Highlighter> highlighters;
private final ParseFieldRegistry<MovAvgModel.AbstractModelParser> movingAverageModelParserRegistry = new ParseFieldRegistry<>(
Expand Down Expand Up @@ -1094,7 +1087,6 @@ private void registerQueryParsers(List<SearchPlugin> plugins) {
registerQuery(new QuerySpec<>(MatchAllQueryBuilder.NAME, MatchAllQueryBuilder::new, MatchAllQueryBuilder::fromXContent));
registerQuery(new QuerySpec<>(QueryStringQueryBuilder.NAME, QueryStringQueryBuilder::new, QueryStringQueryBuilder::fromXContent));
registerQuery(new QuerySpec<>(BoostingQueryBuilder.NAME, BoostingQueryBuilder::new, BoostingQueryBuilder::fromXContent));
BooleanQuery.setMaxClauseCount(INDICES_MAX_CLAUSE_COUNT_SETTING.get(settings));
registerQuery(new QuerySpec<>(BoolQueryBuilder.NAME, BoolQueryBuilder::new, BoolQueryBuilder::fromXContent));
registerQuery(new QuerySpec<>(TermQueryBuilder.NAME, TermQueryBuilder::new, TermQueryBuilder::fromXContent));
registerQuery(new QuerySpec<>(TermsQueryBuilder.NAME, TermsQueryBuilder::new, TermsQueryBuilder::fromXContent));
Expand Down
14 changes: 14 additions & 0 deletions server/src/main/java/org/opensearch/search/SearchService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.FieldDoc;
import org.apache.lucene.search.IndexSearcher;
import org.apache.lucene.search.TopDocs;
import org.opensearch.OpenSearchException;
import org.opensearch.action.ActionRunnable;
Expand Down Expand Up @@ -281,6 +283,15 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv
Property.NodeScope
);

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,
Setting.Property.Dynamic
);

public static final int DEFAULT_SIZE = 10;
public static final int DEFAULT_FROM = 0;

Expand Down Expand Up @@ -389,6 +400,9 @@ public SearchService(

lowLevelCancellation = LOW_LEVEL_CANCELLATION_SETTING.get(settings);
clusterService.getClusterSettings().addSettingsUpdateConsumer(LOW_LEVEL_CANCELLATION_SETTING, this::setLowLevelCancellation);

IndexSearcher.setMaxClauseCount(INDICES_MAX_CLAUSE_COUNT_SETTING.get(settings));
clusterService.getClusterSettings().addSettingsUpdateConsumer(INDICES_MAX_CLAUSE_COUNT_SETTING, IndexSearcher::setMaxClauseCount);
msfroh marked this conversation as resolved.
Show resolved Hide resolved
}

private void validateKeepAlives(TimeValue defaultKeepAlive, TimeValue maxKeepAlive) {
Expand Down
Loading