Skip to content

Introduced new setting search.query.max_query_string_heap_size#19493

Open
nibix wants to merge 3 commits intoopensearch-project:mainfrom
nibix:query-strin-heap-limit
Open

Introduced new setting search.query.max_query_string_heap_size#19493
nibix wants to merge 3 commits intoopensearch-project:mainfrom
nibix:query-strin-heap-limit

Conversation

@nibix
Copy link
Copy Markdown
Contributor

@nibix nibix commented Oct 1, 2025

Description

This introduces a new cluster wide setting search.query.max_query_string_heap_size which can be used to limit the max heap utilization of instantiated Lucene-style query strings. This complements the search.query.max_query_string_length setting.

A couple of considerations:

  • The default value is 50 MB, which should be a generous limit. When considering this value one has to keep in mind that a single search operation instantiates the query strings per searched shard. Thus, several instances might be created in parallel. Thus, this value should be chosen not too close to the total heap size of the cluster.
  • The code that estimates the heap size uses a heuristic value for the size of a single member field query object of 180 bytes. This seemed to be a good fit for our tests. The class RamUsageEstimator would also offer the possibility to use reflective methods to gain more precise figures for the object sizes. However, I am not sure if this might have performance impacts. Thus, I have not used this for now.

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Oct 1, 2025

❌ Gradle check result for a39b97b: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@kkhatua kkhatua requested a review from kaushalmahi12 October 1, 2025 19:21
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Oct 1, 2025

❌ Gradle check result for 626b486: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@nibix nibix force-pushed the query-strin-heap-limit branch from 626b486 to 027860e Compare October 1, 2025 20:44
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Oct 1, 2025

❌ Gradle check result for 027860e: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

nibix added 3 commits October 2, 2025 06:28
@nibix
Introduced new setting search.query.max_query_string_heap_size
b2a8313 
Signed-off-by: Nils Bandener <nils.bandener@eliatra.com>
@nibix
Added changelog
0aecb78 
Signed-off-by: Nils Bandener <nils.bandener@eliatra.com>
@nibix
Handle null query param
7818306 
Signed-off-by: Nils Bandener <nils.bandener@eliatra.com>
@nibix nibix force-pushed the query-strin-heap-limit branch from 027860e to 7818306 Compare October 2, 2025 04:28
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Oct 2, 2025

✅ Gradle check result for 7818306: SUCCESS

@codecov
Copy link
Copy Markdown

codecov bot commented Oct 2, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 65.95745% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.06%. Comparing base (48b08fb) to head (7818306).
⚠️ Report is 539 commits behind head on main.

Files with missing lines Patch % Lines
...pensearch/index/search/QueryStringQueryParser.java 62.79% 14 Missing and 2 partials ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##               main   #19493      +/-   ##
============================================
- Coverage     73.08%   73.06%   -0.02%     
- Complexity    70470    70484      +14     
============================================
  Files          5712     5712              
  Lines        322762   322788      +26     
  Branches      46743    46746       +3     
============================================
- Hits         235877   235856      -21     
- Misses        67912    67931      +19     
- Partials      18973    19001      +28

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This was referenced Oct 9, 2025
Open
Open
Open
@cwperks
Copy link
Copy Markdown
Member

cwperks commented Oct 31, 2025

I wonder if we can apply a similar approach to the GET _mapping API because this API is very expensive and can cause cluster instability:

See:

@opensearch-trigger-bot
Copy link
Copy Markdown
Contributor

opensearch-trigger-bot bot commented Dec 1, 2025

This PR is stalled because it has been open for 30 days with no activity.

@opensearch-trigger-bot opensearch-trigger-bot bot added stalled Issues that have stalled and removed stalled Issues that have stalled labels Dec 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anasalkouz anasalkouz Awaiting requested review from anasalkouz anasalkouz is a code owner

@andrross andrross Awaiting requested review from andrross andrross is a code owner

@ashking94 ashking94 Awaiting requested review from ashking94 ashking94 is a code owner

@Bukhtawar Bukhtawar Awaiting requested review from Bukhtawar Bukhtawar is a code owner

@CEHENKLE CEHENKLE Awaiting requested review from CEHENKLE CEHENKLE is a code owner

@cwperks cwperks Awaiting requested review from cwperks cwperks is a code owner

@dbwiddis dbwiddis Awaiting requested review from dbwiddis dbwiddis is a code owner

@gbbafna gbbafna Awaiting requested review from gbbafna gbbafna is a code owner

@jed326 jed326 Awaiting requested review from jed326 jed326 is a code owner

@kotwanikunal kotwanikunal Awaiting requested review from kotwanikunal kotwanikunal is a code owner

@mch2 mch2 Awaiting requested review from mch2 mch2 is a code owner

@msfroh msfroh Awaiting requested review from msfroh msfroh is a code owner

@owaiskazi19 owaiskazi19 Awaiting requested review from owaiskazi19 owaiskazi19 is a code owner

@reta reta Awaiting requested review from reta reta is a code owner

@Rishikesh1159 Rishikesh1159 Awaiting requested review from Rishikesh1159 Rishikesh1159 is a code owner

@sachinpkale sachinpkale Awaiting requested review from sachinpkale sachinpkale is a code owner

@saratvemulapalli saratvemulapalli Awaiting requested review from saratvemulapalli saratvemulapalli is a code owner

@shwetathareja shwetathareja Awaiting requested review from shwetathareja shwetathareja is a code owner

@sohami sohami Awaiting requested review from sohami sohami is a code owner

@kaushalmahi12 kaushalmahi12 Awaiting requested review from kaushalmahi12

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nibix @cwperks