Improve sort-query performance by retaining the default totalHitsThreshold for approximated match_all queries
#18189
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
prudhvigodithi
changed the title
totalHitsThreshold value to defaults with approximation match_allmatch_all retain totalHitsThreshold value to default with approximation
|
{"run-benchmark-test": "id_4"} |
prudhvigodithi
changed the title
match_all retain totalHitsThreshold value to default with approximation match_all with approximation retain totalHitsThreshold value to default
|
❌ Gradle check result for a8a80e3: 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? |
|
The Jenkins job url is https://build.ci.opensearch.org/job/benchmark-pull-request/3039/ . Final results will be published once the job is completed. |
prudhvigodithi
force-pushed
the
approx
branch
2 times, most recently
from
23a29a4 to
fd7d055
Compare
|
❌ Gradle check result for fd7d055: 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? |
|
{"run-benchmark-test": "id_4"} |
|
The Jenkins job url is https://build.ci.opensearch.org/job/benchmark-pull-request/3040/ . Final results will be published once the job is completed. |
|
❌ Gradle check result for 1983cca: 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? |
Benchmark ResultsBenchmark Results for Job: https://build.ci.opensearch.org/job/benchmark-pull-request/3040/
|
Benchmark Baseline Comparison ResultsBenchmark Results for Job: https://build.ci.opensearch.org/job/benchmark-compare/94/
|
prudhvigodithi
changed the title
match_all with approximation retain totalHitsThreshold value to defaulttotalHitsThreshold for approximated match_all queries
Signed-off-by: Prudhvi Godithi <[email protected]>
|
❌ Gradle check result for e85b851: 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? |
|
❌ Gradle check result for e85b851: 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? |
|
❌ Gradle check result for f42e464: 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? |
prudhvigodithi
commented
May 5, 2025
server/src/internalClusterTest/java/org/opensearch/search/sort/FieldSortIT.java
Show resolved
Hide resolved
prudhvigodithi
requested review from
anasalkouz,
andrross and
ashking94
as code owners
harshavamsi
approved these changes
May 5, 2025
bowenlan-amzn
approved these changes
May 5, 2025
server/src/main/java/org/opensearch/search/approximate/ApproximatePointRangeQuery.java
Show resolved
Hide resolved
|
Accidently clicked approval ... |
server/src/main/java/org/opensearch/search/approximate/ApproximateScoreQuery.java
Show resolved
Hide resolved
|
I think you discovered a pretty important part, shortcut total hits, for giving the right Putting down a quick summary of "approximation"
One idea is to have ApproximateQuery itself able to provide a shortcut total hits, and at the time of building TopDocsCollectorContext, provide that. cc: @getsaurabh02 |
Thanks @bowenlan-amzn I see you already approved PR, once the gradle check is green we can get this merged. |
bowenlan-amzn
approved these changes
May 5, 2025
server/src/main/java/org/opensearch/search/approximate/ApproximateScoreQuery.java
Show resolved
Hide resolved
server/src/main/java/org/opensearch/search/approximate/ApproximateScoreQuery.java
Show resolved
Hide resolved
server/src/main/java/org/opensearch/search/query/TopDocsCollectorContext.java
Show resolved
Hide resolved
|
Thanks for the review folks, just a follow up to see if we can get this merged to main. |
This was referenced May 6, 2025
Closed
andrross
approved these changes
May 6, 2025
github-project-automation
bot
moved this from In Progress
to Done
in Performance Roadmap
|
The backport to To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch/backport-3.0 3.0
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch/backport-3.0
# Create a new branch
git switch --create backport/backport-18189-to-3.0
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 560ac1030c941c8cb01c4fcd29a919bd45ccabee
# Push it to GitHub
git push --set-upstream origin backport/backport-18189-to-3.0
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch/backport-3.0Then, create a pull request where the |
This was referenced May 6, 2025
|
Create a manual backport PR #18220. |
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Note: Seen some slight perf gains for desc_sort_timestamp and asc_sort_timestamp with this change.
Related Issues
Resolves #18206
Check List
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.