Skip to content

Conversation

@kh3ra
Copy link
Contributor

@kh3ra kh3ra commented Jul 4, 2025

Description

This PR is based on PR18255's follow-up work. It implements the core process of warming merged segments in remote-store-enabled domains.

During the IndexReaderWarmer#warm,

  1. The primary shard (low priority) uploads the newly created merged segments to the remote store.
  2. The primary shard then sends a RemoteStorePublishMergedSegmentRequest request containing information about the newly created merged segment.
  3. The replica shards (low priority) download the merged segments from the remote store, and confirm the download to the primary shard.
  4. Once the primary shard receives confirmation from all shards, IndexReaderWarmer#warm completes and the merge resumes.

Notes -

  • Any failures in the IndexReaderWarmer#warm flow are ignored and the merged segments are then expected to get replicated in the regular replication flow.

Related Issues

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

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

github-actions bot commented Jul 4, 2025

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

@kh3ra kh3ra force-pushed the dev/remote-merged-segment-warmer branch from d7f0e5b to dae97b6 Compare July 4, 2025 08:34
@github-actions
Copy link
Contributor

github-actions bot commented Jul 4, 2025

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

@kh3ra kh3ra force-pushed the dev/remote-merged-segment-warmer branch from dae97b6 to 628d4c8 Compare July 4, 2025 09:05
@kh3ra kh3ra marked this pull request as draft July 4, 2025 09:09
@github-actions
Copy link
Contributor

github-actions bot commented Jul 4, 2025

❌ Gradle check result for 628d4c8: 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

github-actions bot commented Jul 4, 2025

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

@kh3ra kh3ra force-pushed the dev/remote-merged-segment-warmer branch from e4aae33 to 48d3c50 Compare July 4, 2025 10:58
@github-actions
Copy link
Contributor

github-actions bot commented Jul 4, 2025

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

@kh3ra kh3ra force-pushed the dev/remote-merged-segment-warmer branch from 0457da0 to 21d8c13 Compare August 4, 2025 20:17
@github-actions
Copy link
Contributor

github-actions bot commented Aug 4, 2025

❕ Gradle check result for 21d8c13: 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

github-actions bot commented Aug 5, 2025

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

@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2025

✅ Gradle check result for 162b3eb: SUCCESS

@kh3ra
Copy link
Contributor Author

kh3ra commented Aug 5, 2025

Regarding codecov: Achieved 72.43% vs expected 72.77%

The slight coverage gap is due to challenges in writing automated tests mergedSegmentWarmer, as it interacts with several final/private classes in Lucene's codebase which limits our testing capabilities at this point.

@ashking94
Copy link
Member

@kh3ra Lets add a cluster setting to control enablement of remote merged segment warmer along with the experimental setting. If we want to disable it later, it would come in handy.

@ashking94 ashking94 merged commit 163c870 into opensearch-project:main Aug 5, 2025
31 of 34 checks passed
vinaykpud pushed a commit to vinaykpud/OpenSearch that referenced this pull request Sep 26, 2025
…rch-project#18683)

* Changes to support upload and download of merge segments using the IndexWriter.IndexWarmer.warm() flow

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

* Unit tests

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

* Fixing build issues

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

* Fixing build issues - forbiddenAPIs/spotlessApply

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

* Upload merge segments in low priority, minor fixes

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

* Addressing review comments + rebase main

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

* Test fixes + javadocs

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

* 1. Bug fix to RemoteDirectory.DownloadRateLimiterProvider
2. Fixes to tests

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

* Bug fix to replica updates to ActiveMergesRegistry

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

* Addressing review comments - round 2

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

* new tests + test fixes + minor bug fixes

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

* Bug fix

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

* Fixes to RemoteStorePublishMergedSegmentActionTests

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

* Adding integration tests

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

* Tracking stats for merged segment warmer

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

* Revert "Tracking stats for merged segment warmer"

This reverts commit 1476e22.

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

* Addressing review comments for tests

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

* Addressing review comments

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

* Rebasing

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

* spotlessApply

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

* test fix

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

* Empty commit

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

* Adding tests, enhancing logs

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

* Adding MergedSegmentWarmerFactory tests + enhancing existing tests

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

* Applying spotless

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

* Added test for Timeout case for  RemoteStoreReplicationSource

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

* Restored RemoteSegmentStoreDirectory PublicAPI + added changelog

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

---------

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants