Skip to content

[Backport 3.4] Add Hybrid Cardinality collector to prioritize Ordinals Collector#20208

Merged
sandeshkr419 merged 1 commit into
3.4from
backport/backport-19524-to-3.4
Dec 10, 2025
Merged

[Backport 3.4] Add Hybrid Cardinality collector to prioritize Ordinals Collector#20208
sandeshkr419 merged 1 commit into
3.4from
backport/backport-19524-to-3.4

Conversation

@opensearch-trigger-bot
Copy link
Copy Markdown
Contributor

@opensearch-trigger-bot opensearch-trigger-bot Bot commented Dec 10, 2025

Backport 710d02f from #19524.

@github-actions
Add Hybrid Cardinality collector to prioritize Ordinals Collector (#1…
24a90f6 
…9524)

* Add Hybrid Cardinality collector to prioritize Ordinals Collector

Current cardinality aggregator logic selects DirectCollector over OrdinalsCollector when relative memory overhead due to OrdinalsCollector (compared to DirectCollector) is higher. Because of this relative memory consumption logic, DirectCollector is selected for high cardinality aggregation queries. DirectCollector is slower compared to OrdinalsCollector. This default selection leads to higher search latency even when Opensearch process have available memory to use ordinals collector for faster query performance.

There is no way to figure out memory requirement for nested aggregation because number of buckets are dynamically created as we traverse through all the matching document ids. To overcome this limitation, this change create a hybrid collector which will first use Ordinals Collector and will switch to DirectCollector if memory usage for Ordinals Collector Increase beyond certain threshold. When Hybrid collector switch from Ordinals Collector to Direct Collector, it will utilize already computed aggregation data from Ordinals Collector so that we do not have to rebuild aggregation result using Direct Collector.

Signed-off-by: Anand Pravinbhai Patel <anapat@amazon.com>

* Address PR comments

Signed-off-by: Anand Pravinbhai Patel <anapat@amazon.com>

* Address PR comments

Signed-off-by: Anand Pravinbhai Patel <anapat@amazon.com>

* Fix UTs

Signed-off-by: Anand Pravinbhai Patel <anapat@amazon.com>

---------

Signed-off-by: Anand Pravinbhai Patel <anapat@amazon.com>
(cherry picked from commit 710d02f)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 10, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch backport/backport-19524-to-3.4

Comment @coderabbitai help to get the list of available commands and usage tips.

@sandeshkr419 sandeshkr419 added the v3.4.0 Issues and PRs related to version 3.4.0 label Dec 10, 2025
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 10, 2025

❌ Gradle check result for 24a90f6: 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?

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 10, 2025

❕ Gradle check result for 24a90f6: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 10, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 78.75000% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.27%. Comparing base (f791594) to head (24a90f6).
⚠️ Report is 3 commits behind head on 3.4.

Files with missing lines Patch % Lines
...ch/aggregations/metrics/CardinalityAggregator.java 76.19% 14 Missing and 1 partial ⚠️
...va/org/opensearch/search/DefaultSearchContext.java 88.88% 1 Missing ⚠️
.../org/opensearch/search/internal/SearchContext.java 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##                3.4   #20208      +/-   ##
============================================
+ Coverage     73.23%   73.27%   +0.03%     
+ Complexity    71787    71760      -27     
============================================
  Files          5793     5794       +1     
  Lines        328098   328218     +120     
  Branches      47250    47261      +11     
============================================
+ Hits         240295   240486     +191     
+ Misses        68556    68408     -148     
- Partials      19247    19324      +77

☔ 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.

@sandeshkr419 sandeshkr419 merged commit 7d87ca2 into 3.4 Dec 10, 2025
57 of 59 checks passed
@sandeshkr419 sandeshkr419 deleted the backport/backport-19524-to-3.4 branch December 10, 2025 05:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sandeshkr419 sandeshkr419 sandeshkr419 approved these changes

@anasalkouz anasalkouz Awaiting requested review from anasalkouz

@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

@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

@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

Assignees

No one assigned

Labels

v3.4.0 Issues and PRs related to version 3.4.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@sandeshkr419