Skip to content

Conversation

gaobinlong
Copy link
Contributor

@gaobinlong gaobinlong commented Oct 9, 2025

Description

Similar to #19181, in SimpleTopDocsCollectorContext, when concurrent segment search is enabled, we can also omit the maxScoreCollector if score is the primary sort.

This change is small, and the existing tests can cover the change. Below are the benchmark result:

GET big5/_search?track_scores=true
{
  "query": {
    "bool": {
      "must": [
        {
          "bool": {
            "should": [
              {
                "match": {
                  "message": "sshd"
                }
              },
              {
                "match": {
                  "message": "ip-50-136-239-27"
                }
              }
            ]
          }
        },
        {
          "term": {
            "cloud.region": {
              "value": "ap-south-1"
            }
          }
        }
      ]
    }
  },
  "sort": [
    {
      "_score": {
        "order": "desc"
      }
    },
   {
      "_doc": {
        "order": "asc"
      }
    }
  ]
}

Before:
|                                   50th percentile service time | sortingByScore |     286.633 |     ms |
|                                   90th percentile service time | sortingByScore |     344.757 |     ms |
|                                   99th percentile service time | sortingByScore |     421.239 |     ms |
|                                 99.9th percentile service time | sortingByScore |     486.528 |     ms |
|                                  100th percentile service time | sortingByScore |     504.309 |     ms |

|                                   50th percentile service time | sortingByScore |     285.596 |     ms |
|                                   90th percentile service time | sortingByScore |     344.858 |     ms |
|                                   99th percentile service time | sortingByScore |     407.116 |     ms |
|                                 99.9th percentile service time | sortingByScore |     422.874 |     ms |
|                                  100th percentile service time | sortingByScore |     426.929 |     ms |

|                                   50th percentile service time | sortingByScore |     284.964 |     ms |
|                                   90th percentile service time | sortingByScore |     326.003 |     ms |
|                                   99th percentile service time | sortingByScore |      412.18 |     ms |
|                                 99.9th percentile service time | sortingByScore |     453.819 |     ms |
|                                  100th percentile service time | sortingByScore |     480.079 |     ms |

After:

|                                   50th percentile service time | sortingByScore |     256.814 |     ms |
|                                   90th percentile service time | sortingByScore |     282.388 |     ms |
|                                   99th percentile service time | sortingByScore |     330.551 |     ms |
|                                 99.9th percentile service time | sortingByScore |      358.14 |     ms |
|                                  100th percentile service time | sortingByScore |     364.175 |     ms |

|                                   50th percentile service time | sortingByScore |     260.364 |     ms |
|                                   90th percentile service time | sortingByScore |     309.238 |     ms |
|                                   99th percentile service time | sortingByScore |     358.525 |     ms |
|                                 99.9th percentile service time | sortingByScore |     382.012 |     ms |
|                                  100th percentile service time | sortingByScore |     383.192 |     ms |

Related Issues

No issue.

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.

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

github-actions bot commented Oct 9, 2025

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

Copy link
Contributor

❌ Gradle check result for 5b0a90f: null

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

❕ Gradle check result for 5b0a90f: 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 Oct 10, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.05%. Comparing base (006a53f) to head (5b0a90f).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main   #19584   +/-   ##
=========================================
  Coverage     73.05%   73.05%           
+ Complexity    70627    70618    -9     
=========================================
  Files          5723     5723           
  Lines        323489   323490    +1     
  Branches      46851    46852    +1     
=========================================
+ Hits         236311   236326   +15     
+ Misses        68174    68098   -76     
- Partials      19004    19066   +62     

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

@gaobinlong gaobinlong merged commit df704b7 into opensearch-project:main Oct 10, 2025
35 of 39 checks passed
rgsriram pushed a commit to rgsriram/OpenSearch that referenced this pull request Oct 11, 2025
…nt segment search enabled (opensearch-project#19584)

* Omit maxScoreCollector in SimpleTopDocsCollectorContext when concurrent segment search enabled

Signed-off-by: Binlong Gao <[email protected]>

* Modify changelog

Signed-off-by: Binlong Gao <[email protected]>

---------

Signed-off-by: Binlong Gao <[email protected]>
peteralfonsi pushed a commit to peteralfonsi/OpenSearch that referenced this pull request Oct 15, 2025
…nt segment search enabled (opensearch-project#19584)

* Omit maxScoreCollector in SimpleTopDocsCollectorContext when concurrent segment search enabled

Signed-off-by: Binlong Gao <[email protected]>

* Modify changelog

Signed-off-by: Binlong Gao <[email protected]>

---------

Signed-off-by: Binlong Gao <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants