Skip to content

Commit 377eca4

Browse files
Introduced monitoring mode for query string query max length.
Signed-off-by: Lukasz Soszynski <lukasz.soszynski@eliatra.com>
1 parent d151cfa commit 377eca4

File tree

5 files changed

+85
-8
lines changed

5 files changed

+85
-8
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
5555
- Harden the circuit breaker and failure handle logic in query result consumer ([#19396](https://github.com/opensearch-project/OpenSearch/pull/19396))
5656
- Add streaming cardinality aggregator ([#19484](https://github.com/opensearch-project/OpenSearch/pull/19484))
5757
- Disable request cache for streaming aggregation queries ([#19520](https://github.com/opensearch-project/OpenSearch/pull/19520))
58+
- New cluster setting search.query.max_query_string_length_monitor_only ([#19539](https://github.com/opensearch-project/OpenSearch/pull/19539))
5859

5960
### Changed
6061
- Refactor `if-else` chains to use `Java 17 pattern matching switch expressions`(([#18965](https://github.com/opensearch-project/OpenSearch/pull/18965))

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@
8181
import static org.opensearch.search.SearchService.CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING;
8282
import static org.opensearch.search.SearchService.INDICES_MAX_CLAUSE_COUNT_SETTING;
8383
import static org.opensearch.search.SearchService.SEARCH_MAX_QUERY_STRING_LENGTH;
84+
import static org.opensearch.search.SearchService.SEARCH_MAX_QUERY_STRING_LENGTH_MONITOR_ONLY;
8485
import static org.opensearch.test.StreamsUtils.copyToStringFromClasspath;
8586
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
8687
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertFailures;
@@ -796,6 +797,45 @@ public void testMaxQueryStringLength() throws Exception {
796797
}
797798
}
798799

800+
public void testMaxQueryStringLengthMonitorMode() throws Exception {
801+
try {
802+
String indexBody = copyToStringFromClasspath("/org/opensearch/search/query/all-query-index.json");
803+
assertAcked(prepareCreate("test").setSource(indexBody, MediaTypeRegistry.JSON));
804+
ensureGreen("test");
805+
806+
assertAcked(
807+
client().admin()
808+
.cluster()
809+
.prepareUpdateSettings()
810+
.setTransientSettings(Settings.builder().put(SEARCH_MAX_QUERY_STRING_LENGTH.getKey(), 10))
811+
);
812+
assertAcked(
813+
client().admin()
814+
.cluster()
815+
.prepareUpdateSettings()
816+
.setTransientSettings(Settings.builder().put(SEARCH_MAX_QUERY_STRING_LENGTH_MONITOR_ONLY.getKey(), true))
817+
);
818+
819+
// query run with the default value of search.query.max_query_string_length which is true
820+
SearchResponse response = client().prepareSearch("test").setQuery(queryStringQuery("foo OR foo OR foo OR foo")).get();
821+
822+
assertNoFailures(response);
823+
} finally {
824+
assertAcked(
825+
client().admin()
826+
.cluster()
827+
.prepareUpdateSettings()
828+
.setTransientSettings(Settings.builder().putNull(SEARCH_MAX_QUERY_STRING_LENGTH.getKey()))
829+
);
830+
assertAcked(
831+
client().admin()
832+
.cluster()
833+
.prepareUpdateSettings()
834+
.setTransientSettings(Settings.builder().putNull(SEARCH_MAX_QUERY_STRING_LENGTH_MONITOR_ONLY.getKey()))
835+
);
836+
}
837+
}
838+
799839
private void assertHits(SearchHits hits, String... ids) {
800840
assertThat(hits.getTotalHits().value(), equalTo((long) ids.length));
801841
Set<String> hitIds = new HashSet<>();

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -577,6 +577,7 @@ public void apply(Settings value, Settings current, Settings previous) {
577577
SearchService.AGGREGATION_REWRITE_FILTER_SEGMENT_THRESHOLD,
578578
SearchService.INDICES_MAX_CLAUSE_COUNT_SETTING,
579579
SearchService.SEARCH_MAX_QUERY_STRING_LENGTH,
580+
SearchService.SEARCH_MAX_QUERY_STRING_LENGTH_MONITOR_ONLY,
580581
SearchService.CARDINALITY_AGGREGATION_PRUNING_THRESHOLD,
581582
SearchService.KEYWORD_INDEX_OR_DOC_VALUES_ENABLED,
582583
CreatePitController.PIT_INIT_KEEP_ALIVE,

server/src/main/java/org/opensearch/index/search/QueryStringQueryParser.java

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232

3333
package org.opensearch.index.search;
3434

35+
import org.apache.logging.log4j.LogManager;
36+
import org.apache.logging.log4j.Logger;
3537
import org.apache.lucene.analysis.Analyzer;
3638
import org.apache.lucene.analysis.TokenStream;
3739
import org.apache.lucene.analysis.tokenattributes.CharTermAttribute;
@@ -97,9 +99,11 @@
9799
* @opensearch.internal
98100
*/
99101
public class QueryStringQueryParser extends XQueryParser {
102+
private static final Logger logger = LogManager.getLogger(QueryStringQueryParser.class);
100103
private static final String EXISTS_FIELD = "_exists_";
101104
@SuppressWarnings("NonFinalStaticField")
102105
private static int maxQueryStringLength = SearchService.SEARCH_MAX_QUERY_STRING_LENGTH.get(Settings.EMPTY);
106+
private static boolean maxQueryStringLengthMonitorMode = SearchService.SEARCH_MAX_QUERY_STRING_LENGTH_MONITOR_ONLY.get(Settings.EMPTY);
103107

104108
private final QueryShardContext context;
105109
private final Map<String, Float> fieldsAndWeights;
@@ -872,14 +876,24 @@ public Query parse(String query) throws ParseException {
872876
return Queries.newMatchNoDocsQuery("Matching no documents because no terms present");
873877
}
874878
if (query.length() > maxQueryStringLength) {
875-
throw new ParseException(
876-
"Query string length exceeds max allowed length "
877-
+ maxQueryStringLength
878-
+ " ("
879-
+ SearchService.SEARCH_MAX_QUERY_STRING_LENGTH.getKey()
880-
+ "); actual length: "
881-
+ query.length()
882-
);
879+
if (maxQueryStringLengthMonitorMode) {
880+
// Log a warning and continue
881+
logger.warn(
882+
"Query string length exceeds max allowed length {} ({}); actual length: {}",
883+
maxQueryStringLength,
884+
SearchService.SEARCH_MAX_QUERY_STRING_LENGTH.getKey(),
885+
query.length()
886+
);
887+
} else {
888+
throw new ParseException(
889+
"Query string length exceeds max allowed length "
890+
+ maxQueryStringLength
891+
+ " ("
892+
+ SearchService.SEARCH_MAX_QUERY_STRING_LENGTH.getKey()
893+
+ "); actual length: "
894+
+ query.length()
895+
);
896+
}
883897
}
884898
return super.parse(query);
885899
}
@@ -890,4 +904,12 @@ public Query parse(String query) throws ParseException {
890904
public static void setMaxQueryStringLength(int maxQueryStringLength) {
891905
QueryStringQueryParser.maxQueryStringLength = maxQueryStringLength;
892906
}
907+
908+
/**
909+
* Sets whether the max query string length should be enforced in or not
910+
* @param monitorMode if true, the max query string length will not be enforced
911+
*/
912+
public static void setMaxQueryStringLengthMonitorMode(boolean monitorMode) {
913+
QueryStringQueryParser.maxQueryStringLengthMonitorMode = monitorMode;
914+
}
893915
}

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,13 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv
379379
Setting.Property.Dynamic
380380
);
381381

382+
public static final Setting<Boolean> SEARCH_MAX_QUERY_STRING_LENGTH_MONITOR_ONLY = Setting.boolSetting(
383+
"search.query.max_query_string_length_monitor_only",
384+
false,
385+
Setting.Property.NodeScope,
386+
Setting.Property.Dynamic
387+
);
388+
382389
public static final Setting<Boolean> CLUSTER_ALLOW_DERIVED_FIELD_SETTING = Setting.boolSetting(
383390
"search.derived_field.enabled",
384391
true,
@@ -537,6 +544,12 @@ public SearchService(
537544
QueryStringQueryParser.setMaxQueryStringLength(SEARCH_MAX_QUERY_STRING_LENGTH.get(settings));
538545
clusterService.getClusterSettings()
539546
.addSettingsUpdateConsumer(SEARCH_MAX_QUERY_STRING_LENGTH, QueryStringQueryParser::setMaxQueryStringLength);
547+
QueryStringQueryParser.setMaxQueryStringLengthMonitorMode(SEARCH_MAX_QUERY_STRING_LENGTH_MONITOR_ONLY.get(settings));
548+
clusterService.getClusterSettings()
549+
.addSettingsUpdateConsumer(
550+
SEARCH_MAX_QUERY_STRING_LENGTH_MONITOR_ONLY,
551+
QueryStringQueryParser::setMaxQueryStringLengthMonitorMode
552+
);
540553

541554
allowDerivedField = CLUSTER_ALLOW_DERIVED_FIELD_SETTING.get(settings);
542555
clusterService.getClusterSettings().addSettingsUpdateConsumer(CLUSTER_ALLOW_DERIVED_FIELD_SETTING, this::setAllowDerivedField);

0 commit comments

Comments
 (0)