[Composite Terms Aggregation] Add weight#count check optimization for no match case #18866
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
|
❌ Gradle check result for 4ac38a0: 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? |
Signed-off-by: Sandesh Kumar <[email protected]>
sandeshkr419
changed the title
|
Hello! |
|
{"run-benchmark-test": "id_4"} |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #18866 +/- ##
============================================
+ Coverage 72.67% 72.84% +0.16%
- Complexity 68610 68670 +60
============================================
Files 5577 5577
Lines 315375 315380 +5
Branches 45772 45774 +2
============================================
+ Hits 229209 229745 +536
+ Misses 67613 66965 -648
- Partials 18553 18670 +117 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
The benchmark job https://build.ci.opensearch.org/job/benchmark-pull-request/3919/ failed. |
|
The Jenkins job url is https://build.ci.opensearch.org/job/benchmark-pull-request/3920/ . Final results will be published once the job is completed. |
Benchmark ResultsBenchmark Results for Job: https://build.ci.opensearch.org/job/benchmark-pull-request/3920/
|
Benchmark Baseline Comparison ResultsBenchmark Results for Job: https://build.ci.opensearch.org/job/benchmark-compare/136/
|
|
Great optimization idea for the no-match case. I'm curious about the benchmark results showing slight latency increases in some composite terms scenarios. Could you help clarify:
For performance optimizations like this, it would be valuable to add a specific unit test that validates:
The |
atris
reviewed
Aug 1, 2025
|
@atris Thanks for looking into this. Let me try clarify some of your points:
I am trying to experiment a few changes. 12-15ms (2-3%) is a normal variance. Also, it compares from the baseline where some more changes might be creating noise. I'll try to isolate my change in a manual benchmark next. This optimization is inspired from a previous optimization in GlobalOrdinalsStringTermsAggregator#178. Preferably, this will benefit workloads where a considerable proportion of segments is a match-none case, where we can short-circuit early on.
Weight is independent on cardinality of aggregation, as weight is dependent on query alone and not aggregation, so the number of composite terms is not relevant here.
This is still in draft, will get to adding tests once I finalize on what changes are beneficial. The relevance of these draft PRs are to save manual benchmarking efforts.
So, the default implementation returns a |
|
Closing in favor of #18531 |
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
[Describe what this change achieves]
Related Issues
Resolves #[Issue number to be closed when this PR is merged]
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.