Skip to content

Conversation

@rgsriram
Copy link
Contributor

Description

Timestamp sort optimisations (leafSorter) were not working consistently across all engine types. While InternalEngine (primary shards) correctly applied leafSorter, other engine types like ReadOnlyEngine (searchable snapshots) and NRTReplicationEngine (segment replication) were not passing leafSorter when opening DirectoryReader instances.
This caused inconsistent performance for time-series workloads:

  • Primary shards: Fast time-series queries (segments ordered by timestamp)
  • Searchable snapshots: Slower time-series queries (segments in arbitrary order)
  • Segment replication replicas: Slower time-series queries (segments in arbitrary order)

Solution
Apply leafSorter optimisation consistently across all engine types by ensuring DirectoryReader.open() calls include the leafSorter parameter.

Related Issues

Resolves #[Issue number to be closed when this PR is merged]
#17579

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.

@github-actions
Copy link
Contributor

❌ Gradle check result for 589d285: 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
Contributor

✅ Gradle check result for 70b752d: SUCCESS

@codecov
Copy link

codecov bot commented Jun 28, 2025

Codecov Report

Attention: Patch coverage is 76.92308% with 3 lines in your changes missing coverage. Please review.

Project coverage is 72.77%. Comparing base (2906600) to head (c5aff16).
Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
...va/org/opensearch/index/engine/ReadOnlyEngine.java 40.00% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #18639      +/-   ##
============================================
- Coverage     72.78%   72.77%   -0.01%     
+ Complexity    68544    68502      -42     
============================================
  Files          5567     5569       +2     
  Lines        314911   314937      +26     
  Branches      45684    45685       +1     
============================================
- Hits         229201   229189      -12     
- Misses        67092    67143      +51     
+ Partials      18618    18605      -13     

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

@rgsriram
Copy link
Contributor Author

@msfroh - Could you please review it?.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 2, 2025

✅ Gradle check result for 115bd30: SUCCESS

@github-actions
Copy link
Contributor

❌ Gradle check result for 7b394f8: 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
Contributor

❌ Gradle check result for 25e79de: 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
Contributor

❌ Gradle check result for 07de023: 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
Contributor

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

@github-actions
Copy link
Contributor

❌ Gradle check result for 20f7527: 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
Contributor

✅ Gradle check result for 892401e: SUCCESS

@github-actions
Copy link
Contributor

❌ Gradle check result for 7a03880: 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
Contributor

❌ Gradle check result for 5fdca4d: 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
Contributor

❌ Gradle check result for 16c2978: 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
Contributor

❌ Gradle check result for 3d87ec3: 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
Contributor

✅ Gradle check result for c5aff16: SUCCESS

Copy link
Contributor

@msfroh msfroh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thanks for sticking it out with this one @rgsriram!

@msfroh msfroh merged commit 5486c63 into opensearch-project:main Jul 23, 2025
31 checks passed
tandonks pushed a commit to tandonks/OpenSearch that referenced this pull request Aug 5, 2025
vinaykpud pushed a commit to vinaykpud/OpenSearch that referenced this pull request Sep 26, 2025
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