Skip to content

Commit c5c6024

Browse files
harshavamsiharshavamsi
authored
Set INDICES_MAX_CLAUSE_COUNT dynamically (opensearch-project#13568)
 --------- Signed-off-by: Harsha Vamsi Kalluri <harshavamsi096@gmail.com>
1 parent 802f2e6 commit c5c6024

File tree

7 files changed

+68
-17
lines changed

7 files changed

+68
-17
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
2121
- Bump `com.gradle.develocity` from 3.17.4 to 3.17.5 ([#14397](https://github.com/opensearch-project/OpenSearch/pull/14397))
2222

2323
### Changed
24+
- Updated the `indices.query.bool.max_clause_count` setting from being static to dynamically updateable ([#13568](https://github.com/opensearch-project/OpenSearch/pull/13568))
2425

2526
### Deprecated
2627

‎server/src/internalClusterTest/java/org/opensearch/search/query/QueryStringIT.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545
import org.opensearch.index.query.QueryStringQueryBuilder;
4646
import org.opensearch.search.SearchHit;
4747
import org.opensearch.search.SearchHits;
48-
import org.opensearch.search.SearchModule;
48+
import org.opensearch.search.SearchService;
4949
import org.opensearch.test.ParameterizedStaticSettingsOpenSearchIntegTestCase;
5050
import org.junit.Before;
5151
import org.junit.BeforeClass;
@@ -101,7 +101,7 @@ public void setup() throws Exception {
101101
protected Settings nodeSettings(int nodeOrdinal) {
102102
return Settings.builder()
103103
.put(super.nodeSettings(nodeOrdinal))
104-
.put(SearchModule.INDICES_MAX_CLAUSE_COUNT_SETTING.getKey(), CLUSTER_MAX_CLAUSE_COUNT)
104+
.put(SearchService.INDICES_MAX_CLAUSE_COUNT_SETTING.getKey(), CLUSTER_MAX_CLAUSE_COUNT)
105105
.build();
106106
}
107107

‎server/src/internalClusterTest/java/org/opensearch/search/query/SimpleQueryStringIT.java

+49-2
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@
5757
import org.opensearch.plugins.Plugin;
5858
import org.opensearch.search.SearchHit;
5959
import org.opensearch.search.SearchHits;
60-
import org.opensearch.search.SearchModule;
60+
import org.opensearch.search.SearchService;
6161
import org.opensearch.search.builder.SearchSourceBuilder;
6262
import org.opensearch.test.ParameterizedStaticSettingsOpenSearchIntegTestCase;
6363
import org.junit.BeforeClass;
@@ -79,6 +79,7 @@
7979
import static org.opensearch.index.query.QueryBuilders.simpleQueryStringQuery;
8080
import static org.opensearch.index.query.QueryBuilders.termQuery;
8181
import static org.opensearch.search.SearchService.CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING;
82+
import static org.opensearch.search.SearchService.INDICES_MAX_CLAUSE_COUNT_SETTING;
8283
import static org.opensearch.test.StreamsUtils.copyToStringFromClasspath;
8384
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
8485
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertFailures;
@@ -122,7 +123,7 @@ public static void createRandomClusterSetting() {
122123
protected Settings nodeSettings(int nodeOrdinal) {
123124
return Settings.builder()
124125
.put(super.nodeSettings(nodeOrdinal))
125-
.put(SearchModule.INDICES_MAX_CLAUSE_COUNT_SETTING.getKey(), CLUSTER_MAX_CLAUSE_COUNT)
126+
.put(SearchService.INDICES_MAX_CLAUSE_COUNT_SETTING.getKey(), CLUSTER_MAX_CLAUSE_COUNT)
126127
.build();
127128
}
128129

@@ -720,6 +721,52 @@ public void testFieldAliasOnDisallowedFieldType() throws Exception {
720721
assertHits(response.getHits(), "1");
721722
}
722723

724+
public void testDynamicClauseCountUpdate() throws Exception {
725+
client().prepareIndex("testdynamic").setId("1").setSource("field", "foo bar baz").get();
726+
assertAcked(
727+
client().admin()
728+
.cluster()
729+
.prepareUpdateSettings()
730+
.setTransientSettings(Settings.builder().put(INDICES_MAX_CLAUSE_COUNT_SETTING.getKey(), CLUSTER_MAX_CLAUSE_COUNT - 1))
731+
);
732+
refresh();
733+
StringBuilder sb = new StringBuilder("foo");
734+
735+
// create clause_count + 1 clauses to hit error
736+
for (int i = 0; i <= CLUSTER_MAX_CLAUSE_COUNT; i++) {
737+
sb.append(" OR foo" + i);
738+
}
739+
740+
QueryStringQueryBuilder qb = queryStringQuery(sb.toString()).field("field");
741+
742+
SearchPhaseExecutionException e = expectThrows(SearchPhaseExecutionException.class, () -> {
743+
client().prepareSearch("testdynamic").setQuery(qb).get();
744+
});
745+
746+
assert (e.getDetailedMessage().contains("maxClauseCount is set to " + (CLUSTER_MAX_CLAUSE_COUNT - 1)));
747+
748+
// increase clause count by 2
749+
assertAcked(
750+
client().admin()
751+
.cluster()
752+
.prepareUpdateSettings()
753+
.setTransientSettings(Settings.builder().put(INDICES_MAX_CLAUSE_COUNT_SETTING.getKey(), CLUSTER_MAX_CLAUSE_COUNT + 2))
754+
);
755+
756+
Thread.sleep(1);
757+
758+
SearchResponse response = client().prepareSearch("testdynamic").setQuery(qb).get();
759+
assertHitCount(response, 1);
760+
assertHits(response.getHits(), "1");
761+
762+
assertAcked(
763+
client().admin()
764+
.cluster()
765+
.prepareUpdateSettings()
766+
.setTransientSettings(Settings.builder().putNull(INDICES_MAX_CLAUSE_COUNT_SETTING.getKey()))
767+
);
768+
}
769+
723770
private void assertHits(SearchHits hits, String... ids) {
724771
assertThat(hits.getTotalHits().value, equalTo((long) ids.length));
725772
Set<String> hitIds = new HashSet<>();

‎server/src/main/java/org/opensearch/common/settings/ClusterSettings.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,6 @@
150150
import org.opensearch.repositories.fs.FsRepository;
151151
import org.opensearch.rest.BaseRestHandler;
152152
import org.opensearch.script.ScriptService;
153-
import org.opensearch.search.SearchModule;
154153
import org.opensearch.search.SearchService;
155154
import org.opensearch.search.aggregations.MultiBucketConsumerService;
156155
import org.opensearch.search.backpressure.settings.NodeDuressSettings;
@@ -540,6 +539,7 @@ public void apply(Settings value, Settings current, Settings previous) {
540539
SearchService.MAX_OPEN_PIT_CONTEXT,
541540
SearchService.MAX_PIT_KEEPALIVE_SETTING,
542541
SearchService.MAX_AGGREGATION_REWRITE_FILTERS,
542+
SearchService.INDICES_MAX_CLAUSE_COUNT_SETTING,
543543
SearchService.CARDINALITY_AGGREGATION_PRUNING_THRESHOLD,
544544
CreatePitController.PIT_INIT_KEEP_ALIVE,
545545
Node.WRITE_PORTS_FILE_SETTING,
@@ -590,7 +590,6 @@ public void apply(Settings value, Settings current, Settings previous) {
590590
ResourceWatcherService.RELOAD_INTERVAL_HIGH,
591591
ResourceWatcherService.RELOAD_INTERVAL_MEDIUM,
592592
ResourceWatcherService.RELOAD_INTERVAL_LOW,
593-
SearchModule.INDICES_MAX_CLAUSE_COUNT_SETTING,
594593
ThreadPool.ESTIMATED_TIME_INTERVAL_SETTING,
595594
FastVectorHighlighter.SETTING_TV_HIGHLIGHT_MULTI_VALUE,
596595
Node.BREAKER_TYPE_KEY,

‎server/src/main/java/org/opensearch/index/search/QueryParserHelper.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
import org.opensearch.index.mapper.MappedFieldType;
3939
import org.opensearch.index.query.QueryShardContext;
4040
import org.opensearch.index.query.QueryShardException;
41-
import org.opensearch.search.SearchModule;
41+
import org.opensearch.search.SearchService;
4242

4343
import java.util.Collection;
4444
import java.util.HashMap;
@@ -180,7 +180,7 @@ static Map<String, Float> resolveMappingField(
180180
}
181181

182182
static void checkForTooManyFields(int numberOfFields, QueryShardContext context, @Nullable String inputPattern) {
183-
Integer limit = SearchModule.INDICES_MAX_CLAUSE_COUNT_SETTING.get(context.getIndexSettings().getSettings());
183+
int limit = SearchService.INDICES_MAX_CLAUSE_COUNT_SETTING.get(context.getIndexSettings().getSettings());
184184
if (numberOfFields > limit) {
185185
StringBuilder errorMsg = new StringBuilder("field expansion ");
186186
if (inputPattern != null) {

‎server/src/main/java/org/opensearch/search/SearchModule.java

-9
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@
3737
import org.opensearch.common.Nullable;
3838
import org.opensearch.common.geo.GeoShapeType;
3939
import org.opensearch.common.geo.ShapesAvailability;
40-
import org.opensearch.common.settings.Setting;
4140
import org.opensearch.common.settings.Settings;
4241
import org.opensearch.common.xcontent.ParseFieldRegistry;
4342
import org.opensearch.core.ParseField;
@@ -302,13 +301,6 @@
302301
* @opensearch.internal
303302
*/
304303
public class SearchModule {
305-
public static final Setting<Integer> INDICES_MAX_CLAUSE_COUNT_SETTING = Setting.intSetting(
306-
"indices.query.bool.max_clause_count",
307-
1024,
308-
1,
309-
Integer.MAX_VALUE,
310-
Setting.Property.NodeScope
311-
);
312304

313305
private final Map<String, Highlighter> highlighters;
314306
private final ParseFieldRegistry<MovAvgModel.AbstractModelParser> movingAverageModelParserRegistry = new ParseFieldRegistry<>(
@@ -1094,7 +1086,6 @@ private void registerQueryParsers(List<SearchPlugin> plugins) {
10941086
registerQuery(new QuerySpec<>(MatchAllQueryBuilder.NAME, MatchAllQueryBuilder::new, MatchAllQueryBuilder::fromXContent));
10951087
registerQuery(new QuerySpec<>(QueryStringQueryBuilder.NAME, QueryStringQueryBuilder::new, QueryStringQueryBuilder::fromXContent));
10961088
registerQuery(new QuerySpec<>(BoostingQueryBuilder.NAME, BoostingQueryBuilder::new, BoostingQueryBuilder::fromXContent));
1097-
BooleanQuery.setMaxClauseCount(INDICES_MAX_CLAUSE_COUNT_SETTING.get(settings));
10981089
registerQuery(new QuerySpec<>(BoolQueryBuilder.NAME, BoolQueryBuilder::new, BoolQueryBuilder::fromXContent));
10991090
registerQuery(new QuerySpec<>(TermQueryBuilder.NAME, TermQueryBuilder::new, TermQueryBuilder::fromXContent));
11001091
registerQuery(new QuerySpec<>(TermsQueryBuilder.NAME, TermsQueryBuilder::new, TermsQueryBuilder::fromXContent));

‎server/src/main/java/org/opensearch/search/SearchService.java

+13
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import org.apache.logging.log4j.LogManager;
3636
import org.apache.logging.log4j.Logger;
3737
import org.apache.lucene.search.FieldDoc;
38+
import org.apache.lucene.search.IndexSearcher;
3839
import org.apache.lucene.search.TopDocs;
3940
import org.opensearch.OpenSearchException;
4041
import org.opensearch.action.ActionRunnable;
@@ -281,6 +282,15 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv
281282
Property.NodeScope
282283
);
283284

285+
public static final Setting<Integer> INDICES_MAX_CLAUSE_COUNT_SETTING = Setting.intSetting(
286+
"indices.query.bool.max_clause_count",
287+
1024,
288+
1,
289+
Integer.MAX_VALUE,
290+
Setting.Property.NodeScope,
291+
Setting.Property.Dynamic
292+
);
293+
284294
public static final Setting<Boolean> CLUSTER_ALLOW_DERIVED_FIELD_SETTING = Setting.boolSetting(
285295
"search.derived_field.enabled",
286296
true,
@@ -411,6 +421,9 @@ public SearchService(
411421
lowLevelCancellation = LOW_LEVEL_CANCELLATION_SETTING.get(settings);
412422
clusterService.getClusterSettings().addSettingsUpdateConsumer(LOW_LEVEL_CANCELLATION_SETTING, this::setLowLevelCancellation);
413423

424+
IndexSearcher.setMaxClauseCount(INDICES_MAX_CLAUSE_COUNT_SETTING.get(settings));
425+
clusterService.getClusterSettings().addSettingsUpdateConsumer(INDICES_MAX_CLAUSE_COUNT_SETTING, IndexSearcher::setMaxClauseCount);
426+
414427
allowDerivedField = CLUSTER_ALLOW_DERIVED_FIELD_SETTING.get(settings);
415428
clusterService.getClusterSettings().addSettingsUpdateConsumer(CLUSTER_ALLOW_DERIVED_FIELD_SETTING, this::setAllowDerivedField);
416429
}

0 commit comments

Comments
 (0)