Skip to content

[Aggregations] Optimize singleton handling in GlobalOrdinalValuesSource #17740

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

shreyah963
Copy link
Contributor

@shreyah963 shreyah963 commented Mar 31, 2025

Description

This PR optimizes the handling of single-valued fields in composite aggregations by modifying how we process ordinal values in GlobalOrdinalValuesSource. The key change is eliminating the need for iterative ordinal processing when we can use Lucene's singleton optimization.

Key Changes:

In GlobalOrdinalValuesSource.getLeafCollector(), we replace the iterative ordinal processing:

// Before - Iterative processing for all fields:
if (dvs.advanceExact(doc)) {
    long ord;
    int count = dvs.docValueCount();
    while ((count-- > 0) && (ord = dvs.nextOrd()) != NO_MORE_DOCS) {  // Iterative ordinal processing
        currentValue = ord;
        next.collect(doc, bucket);
    }
}

// After - Direct ordinal access for single-valued fields:
final SortedDocValues singleton = DocValues.unwrapSingleton(dvs);
if (singleton != null) {
    if (singleton.advanceExact(doc)) {
        currentValue = singleton.ordValue();  // Direct access without iteration
        next.collect(doc, bucket);
    }
}

The key improvement is removing the while loop for single-valued fields. Instead of iterating through ordinals (which is unnecessary for single values), we now:

  1. Use DocValues.unwrapSingleton() to identify true single-valued fields
  2. Access the ordinal directly via ordValue() without iteration
  3. Fall back to the original iterative approach only for multi-valued fields

Performance Impact:

  • 9.52% improvement in 90th percentile service time for composite aggregations on the big5 workload
  • Benchmark command:
sudo docker run -v $HOME/benchmarks:/opensearch-benchmark/.benchmark \
opensearchproject/opensearch-benchmark:latest execute-test \
--pipeline=benchmark-only \
--workload=big5 \
--include-tasks="composite_terms-keyword" \
--target-hosts=<OpenSearch Cluster Endpoint>:80
  • Number of test runs: 5
Run Before Change (ms) After Change (ms) % Difference
1 396.2090165907284 365.46215792477597 7.76%
2 412.5848856027005 388.9842171949567 5.72%
3 428.8026589085348 388.66532070969697 9.36%
4 421.8712995760143 381.5606589982053 9.56%
5 430.41677701694425 366.2208680965705 14.91%
Testing Performance Improvement

Infrastructure Details

  • Instance Type: AWS r6g.xlarge (ARM-based)
  • Operating System: Amazon Linux 2
  • AMI: amzn2-ami-kernel-5.10-hvm-2.0.20250305.0-arm64-gp2
  • OpenSearch Version: 3.0.0

Run 1

Before change

Metric Task Value Unit
Segment count 16
Min Throughput composite_terms-keyword 2.00 ops/s
Mean Throughput composite_terms-keyword 2.00 ops/s
Median Throughput composite_terms-keyword 2.00 ops/s
Max Throughput composite_terms-keyword 2.00 ops/s
50th percentile latency composite_terms-keyword 396.16381948872004 ms
90th percentile latency composite_terms-keyword 397.7446157980012 ms
99th percentile latency composite_terms-keyword 399.87472952168906 ms
100th percentile latency composite_terms-keyword 405.53292899858207 ms
50th percentile service time composite_terms-keyword 394.6733914926881 ms
90th percentile service time composite_terms-keyword 396.2090165907284 ms
99th percentile service time composite_terms-keyword 398.61815138196107 ms
100th percentile service time composite_terms-keyword 403.5990770207718 ms
error rate composite_terms-keyword 0.00 %

After change

Metric Task Value Unit
Segment count 16
Min Throughput composite_terms-keyword 2.00 ops/s
Mean Throughput composite_terms-keyword 2.00 ops/s
Median Throughput composite_terms-keyword 2.00 ops/s
Max Throughput composite_terms-keyword 2.00 ops/s
50th percentile latency composite_terms-keyword 365.35704000561964 ms
90th percentile latency composite_terms-keyword 366.74923670943826 ms
99th percentile latency composite_terms-keyword 367.7020134878694 ms
100th percentile latency composite_terms-keyword 368.6830539954826 ms
50th percentile service time composite_terms-keyword 364.48558449046686 ms
90th percentile service time composite_terms-keyword 365.46215792477597 ms
99th percentile service time composite_terms-keyword 366.726313487743 ms
100th percentile service time composite_terms-keyword 367.2434400068596 ms
error rate composite_terms-keyword 0.00 %

Run 2

Before change

Metric Task Value Unit
Segment count 19
Min Throughput composite_terms-keyword 2.00 ops/s
Mean Throughput composite_terms-keyword 2.00 ops/s
Median Throughput composite_terms-keyword 2.00 ops/s
Max Throughput composite_terms-keyword 2.00 ops/s
50th percentile latency composite_terms-keyword 412.67960649565794 ms
90th percentile latency composite_terms-keyword 413.732826692285 ms
99th percentile latency composite_terms-keyword 418.20061942533357 ms
100th percentile latency composite_terms-keyword 420.7352599769365 ms
50th percentile service time composite_terms-keyword 411.5690805047052 ms
90th percentile service time composite_terms-keyword 412.5848856027005 ms
99th percentile service time composite_terms-keyword 417.0573683051043 ms
100th percentile service time composite_terms-keyword 419.71228100010194 ms
error rate composite_terms-keyword 0.00 %

After change

Metric Task Value Unit
Segment count 19
Min Throughput composite_terms-keyword 2.00 ops/s
Mean Throughput composite_terms-keyword 2.00 ops/s
Median Throughput composite_terms-keyword 2.00 ops/s
Max Throughput composite_terms-keyword 2.00 ops/s
50th percentile latency composite_terms-keyword 388.71410601132084 ms
90th percentile latency composite_terms-keyword 389.9167404131731 ms
99th percentile latency composite_terms-keyword 393.258988384041 ms
100th percentile latency composite_terms-keyword 396.21100801741704 ms
50th percentile service time composite_terms-keyword 387.870787497377 ms
90th percentile service time composite_terms-keyword 388.9842171949567 ms
99th percentile service time composite_terms-keyword 392.67881602747366 ms
100th percentile service time composite_terms-keyword 395.0395730207674 ms
error rate composite_terms-keyword 0.00 %

Run 3

Before change

Metric Task Value Unit
Segment count 13
Min Throughput composite_terms-keyword 2.00 ops/s
Mean Throughput composite_terms-keyword 2.00 ops/s
Median Throughput composite_terms-keyword 2.00 ops/s
Max Throughput composite_terms-keyword 2.00 ops/s
50th percentile latency composite_terms-keyword 429.2484114994295 ms
90th percentile latency composite_terms-keyword 430.4278267023619 ms
99th percentile latency composite_terms-keyword 440.79763345245743 ms
100th percentile latency composite_terms-keyword 451.0352670040447 ms
50th percentile service time composite_terms-keyword 427.78990099031944 ms
90th percentile service time composite_terms-keyword 428.8026589085348 ms
99th percentile service time composite_terms-keyword 440.06675194104906 ms
100th percentile service time composite_terms-keyword 449.1370279865805 ms
error rate composite_terms-keyword 0.00 %

After change

Metric Task Value Unit
Segment count 13
Min Throughput composite_terms-keyword 2.00 ops/s
Mean Throughput composite_terms-keyword 2.00 ops/s
Median Throughput composite_terms-keyword 2.00 ops/s
Max Throughput composite_terms-keyword 2.00 ops/s
50th percentile latency composite_terms-keyword 388.1630899850279 ms
90th percentile latency composite_terms-keyword 389.4363162893569 ms
99th percentile latency composite_terms-keyword 391.94549819687387 ms
100th percentile latency composite_terms-keyword 403.01975698093884 ms
50th percentile service time composite_terms-keyword 387.41195149486884 ms
90th percentile service time composite_terms-keyword 388.66532070969697 ms
99th percentile service time composite_terms-keyword 390.8706203309703 ms
100th percentile service time composite_terms-keyword 401.90717397490516 ms
error rate composite_terms-keyword 0.00 %

Run 4

Before change

Metric Task Value Unit
Segment count 18
Min Throughput composite_terms-keyword 2.00 ops/s
Mean Throughput composite_terms-keyword 2.00 ops/s
Median Throughput composite_terms-keyword 2.00 ops/s
Max Throughput composite_terms-keyword 2.00 ops/s
50th percentile latency composite_terms-keyword 420.10582849616185 ms
90th percentile latency composite_terms-keyword 422.53990989993326 ms
99th percentile latency composite_terms-keyword 444.7503272519681 ms
100th percentile latency composite_terms-keyword 469.4900579925161 ms
50th percentile service time composite_terms-keyword 419.3513369827997 ms
90th percentile service time composite_terms-keyword 421.8712995760143 ms
99th percentile service time composite_terms-keyword 444.1732821214829 ms
100th percentile service time composite_terms-keyword 468.97705300943926 ms
error rate composite_terms-keyword 0.00 %

After change

Metric Task Value Unit
Segment count 18
Min Throughput composite_terms-keyword 2.00 ops/s
Mean Throughput composite_terms-keyword 2.00 ops/s
Median Throughput composite_terms-keyword 2.00 ops/s
Max Throughput composite_terms-keyword 2.00 ops/s
50th percentile latency composite_terms-keyword 372.43178200151306 ms
90th percentile latency composite_terms-keyword 382.45731669594534 ms
99th percentile latency composite_terms-keyword 395.8043100434589 ms
100th percentile latency composite_terms-keyword 412.1486200019717 ms
50th percentile service time composite_terms-keyword 371.5587774931919 ms
90th percentile service time composite_terms-keyword 381.5606589982053 ms
99th percentile service time composite_terms-keyword 394.78188366250845 ms
100th percentile service time composite_terms-keyword 411.43889899831265 ms
error rate composite_terms-keyword 0.00 %

Run 5

Before change

Metric Task Value Unit
Segment count 21
Min Throughput composite_terms-keyword 2.00 ops/s
Mean Throughput composite_terms-keyword 2.00 ops/s
Median Throughput composite_terms-keyword 2.00 ops/s
Max Throughput composite_terms-keyword 2.00 ops/s
50th percentile latency composite_terms-keyword 429.72924100467935 ms
90th percentile latency composite_terms-keyword 431.76490269834176 ms
99th percentile latency composite_terms-keyword 443.3606589207193 ms
100th percentile latency composite_terms-keyword 446.37609101482667 ms
50th percentile service time composite_terms-keyword 428.2711215055315 ms
90th percentile service time composite_terms-keyword 430.41677701694425 ms
99th percentile service time composite_terms-keyword 441.7303356458433 ms
100th percentile service time composite_terms-keyword 444.5912009978201 ms
error rate composite_terms-keyword 0.00 %

After change

Metric Task Value Unit
Segment count 21
Min Throughput composite_terms-keyword 2.00 ops/s
Mean Throughput composite_terms-keyword 2.00 ops/s
Median Throughput composite_terms-keyword 2.00 ops/s
Max Throughput composite_terms-keyword 2.00 ops/s
50th percentile latency composite_terms-keyword 365.75915951107163 ms
90th percentile latency composite_terms-keyword 367.46013380470686 ms
99th percentile latency composite_terms-keyword 375.8739322569454 ms
100th percentile latency composite_terms-keyword 377.1341280080378 ms
50th percentile service time composite_terms-keyword 364.4930154987378 ms
90th percentile service time composite_terms-keyword 366.2208680965705 ms
99th percentile service time composite_terms-keyword 374.8813359538326 ms
100th percentile service time composite_terms-keyword 375.28792599914595 ms
error rate composite_terms-keyword 0.00 %

Check List

  • New Functionality has been documented
  • Commits are signed per the DCO using --signoff
  • Failing checks are inspected and point to the corresponding known issue(s) (See: Troubleshooting Failing Builds)
  •  Commit changes are listed out in CHANGELOG.md file

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.

Copy link
Contributor

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

@shreyah963
Copy link
Contributor Author

{"run-benchmark-test": "id_4"}

Copy link
Contributor

github-actions bot commented Apr 1, 2025

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

@shreyah963 shreyah963 force-pushed the GlobalOrdinalValuesSource branch from f715191 to 8ec48cd Compare April 2, 2025 21:18
Copy link
Contributor

github-actions bot commented Apr 2, 2025

❌ Gradle check result for 8ec48cd: 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: shreyah963 <[email protected]>
…esSource and simplify the optimization logic in GlobalOrdinalValuesSource. The singleton optimization is now only applied when DocValues.unwrapSingleton() succeeds, preventing array index out of bounds errors with high cardinality fields.

Signed-off-by: shreyah963 <[email protected]>
@shreyah963 shreyah963 force-pushed the GlobalOrdinalValuesSource branch from 8ec48cd to 4a13f79 Compare April 3, 2025 23:54
Copy link
Contributor

github-actions bot commented Apr 4, 2025

❌ Gradle check result for 4a13f79:

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?

Copy link
Contributor

github-actions bot commented Apr 7, 2025

❕ Gradle check result for 4a13f79: 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.

Copy link

codecov bot commented Apr 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.48%. Comparing base (c91cdcb) to head (a6f9f4f).
Report is 15 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main   #17740      +/-   ##
============================================
+ Coverage     72.46%   72.48%   +0.02%     
- Complexity    66502    66519      +17     
============================================
  Files          5408     5408              
  Lines        308080   308192     +112     
  Branches      44720    44749      +29     
============================================
+ Hits         223239   223396     +157     
- Misses        66536    66537       +1     
+ Partials      18305    18259      -46     

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

@shreyah963 shreyah963 marked this pull request as ready for review April 8, 2025 04:04
@shreyah963 shreyah963 requested a review from VachaShah as a code owner April 8, 2025 04:04
@jainankitk
Copy link
Contributor

@shreyah963 - Can you add CHANGELOG entry as well?

Signed-off-by: shreyah963 <[email protected]>
Copy link
Contributor

github-actions bot commented Apr 8, 2025

✅ Gradle check result for 6ee7cfc: SUCCESS

shreyah963 and others added 2 commits April 8, 2025 09:43
Co-authored-by: bowenlan-amzn <[email protected]>
Signed-off-by: shreyah963 <[email protected]>
Copy link
Contributor

github-actions bot commented Apr 8, 2025

❕ Gradle check result for d93f589: 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.

Copy link
Contributor

github-actions bot commented Apr 8, 2025

❕ Gradle check result for a6f9f4f: 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.

@jainankitk jainankitk added the backport 2.x Backport to 2.x branch label Apr 8, 2025
@jainankitk jainankitk merged commit 3823169 into opensearch-project:main Apr 8, 2025
35 checks passed
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

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-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch/backport-2.x
# Create a new branch
git switch --create backport/backport-17740-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 38231693572bd00363db1b0221a0bc632c4ceef2
# Push it to GitHub
git push --set-upstream origin backport/backport-17740-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-17740-to-2.x.

Harsh-87 pushed a commit to Harsh-87/OpenSearch that referenced this pull request May 7, 2025
…ce (opensearch-project#17740)

* added singleton optimization path to globalordinalvaluesource

Signed-off-by: shreyah963 <[email protected]>

* enabled remote debugging

Signed-off-by: shreyah963 <[email protected]>

* Removed the minimum cap to handle larger ordinal values

Signed-off-by: shreyah963 <[email protected]>

* emove redundant singleton optimization state from SingleDimensionValuesSource and simplify the optimization logic in GlobalOrdinalValuesSource. The singleton optimization is now only applied when DocValues.unwrapSingleton() succeeds, preventing array index out of bounds errors with high cardinality fields.

Signed-off-by: shreyah963 <[email protected]>

* removed redundant initialization

Signed-off-by: shreyah963 <[email protected]>

* reverted the array allocation in the constructer to its original form

Signed-off-by: shreyah963 <[email protected]>

* [Docs] Add detailed comments to GlobalOrdinalValuesSource collector

Signed-off-by: shreyah963 <[email protected]>

* Remote redundant imports and disable remote debugging

Signed-off-by: shreyah963 <[email protected]>

* replaced wildcard import with only necessary imports

Signed-off-by: shreyah963 <[email protected]>

* Update CHANGELOG.md

Signed-off-by: shreyah963 <[email protected]>

* Update CHANGELOG.md

Co-authored-by: bowenlan-amzn <[email protected]>
Signed-off-by: shreyah963 <[email protected]>

* Remove redundant comments from GlobalOrdinalValuesSource

Signed-off-by: shreyah963 <[email protected]>

---------

Signed-off-by: shreyah963 <[email protected]>
Signed-off-by: Ankit Jain <[email protected]>
Co-authored-by: bowenlan-amzn <[email protected]>
Co-authored-by: Ankit Jain <[email protected]>
Signed-off-by: Harsh Kothari <[email protected]>
Harsh-87 pushed a commit to Harsh-87/OpenSearch that referenced this pull request May 7, 2025
…ce (opensearch-project#17740)

* added singleton optimization path to globalordinalvaluesource

Signed-off-by: shreyah963 <[email protected]>

* enabled remote debugging

Signed-off-by: shreyah963 <[email protected]>

* Removed the minimum cap to handle larger ordinal values

Signed-off-by: shreyah963 <[email protected]>

* emove redundant singleton optimization state from SingleDimensionValuesSource and simplify the optimization logic in GlobalOrdinalValuesSource. The singleton optimization is now only applied when DocValues.unwrapSingleton() succeeds, preventing array index out of bounds errors with high cardinality fields.

Signed-off-by: shreyah963 <[email protected]>

* removed redundant initialization

Signed-off-by: shreyah963 <[email protected]>

* reverted the array allocation in the constructer to its original form

Signed-off-by: shreyah963 <[email protected]>

* [Docs] Add detailed comments to GlobalOrdinalValuesSource collector

Signed-off-by: shreyah963 <[email protected]>

* Remote redundant imports and disable remote debugging

Signed-off-by: shreyah963 <[email protected]>

* replaced wildcard import with only necessary imports

Signed-off-by: shreyah963 <[email protected]>

* Update CHANGELOG.md

Signed-off-by: shreyah963 <[email protected]>

* Update CHANGELOG.md

Co-authored-by: bowenlan-amzn <[email protected]>
Signed-off-by: shreyah963 <[email protected]>

* Remove redundant comments from GlobalOrdinalValuesSource

Signed-off-by: shreyah963 <[email protected]>

---------

Signed-off-by: shreyah963 <[email protected]>
Signed-off-by: Ankit Jain <[email protected]>
Co-authored-by: bowenlan-amzn <[email protected]>
Co-authored-by: Ankit Jain <[email protected]>
Signed-off-by: Harsh Kothari <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch backport-failed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants