Fixes for remote store integration by mgodwan · Pull Request #20325 · opensearch-project/OpenSearch

mgodwan · 2025-12-26T09:30:12Z

Fixes in upload flow

Close acquired snapshot
Clone should not acquire
Translog interaction should be on indexer, rather than engine.
Seq No management for remote store verification

Pending: Tests

coderabbitai · 2025-12-26T09:30:17Z

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

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

github-actions · 2025-12-26T09:36:33Z

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

Bukhtawar · 2025-12-27T13:45:09Z

...er/src/main/java/org/opensearch/index/engine/exec/coord/LastRefreshedCheckpointListener.java

+
+final class LastRefreshedCheckpointListener implements ReferenceManager.RefreshListener {
+    final AtomicLong refreshedCheckpoint;
+    volatile AtomicLong pendingCheckpoint;


AtomicLong is implicitly volatile in terms of memory visibility

Bukhtawar · 2025-12-27T13:47:48Z

...er/src/main/java/org/opensearch/index/engine/exec/coord/LastRefreshedCheckpointListener.java

+
+import java.util.concurrent.atomic.AtomicLong;
+
+final class LastRefreshedCheckpointListener implements ReferenceManager.RefreshListener {


Maybe move this as an inner class for CompositeEngine?

Kept this outside so that is can shared between InternalEngine and CompositeEngine. Made changes in InternalEngine as well.

Bukhtawar · 2025-12-27T14:00:28Z

server/src/main/java/org/opensearch/index/engine/exec/coord/CatalogSnapshot.java

+    public CatalogSnapshot cloneNoAcquire() {
+        // Still using the clone call since Lucene call requires clone. This will allow a SegmentsInfos backed CatalogSnapshot to use the same method in calls.
+        return this;
    }


Technically this isn't clone right?

Yes.
We need a clone for SegmentsInfo but it is not required for default CatalogSnaphot. Once we have a SegmentInfos backed CatalogSnapshot for Lucene changes to work with same code, this would make more sense.
Hence, kept it like this for now.

Bukhtawar · 2025-12-27T14:01:39Z

server/src/main/java/org/opensearch/index/shard/ReleasableRetryableRefreshListener.java

+        try {
+            catalogSnapshot.close();
+        } catch (Exception ex) {
+
+        }


Assuming this is the older version of the snapshot that would be dec referenced?

This is the latest one, but the RefreshListener relies on fetching the latest catalog snapshot from engine later. I've modified this to be passed a supplier so that it can acquire and release both in the same context.

Signed-off-by: Mohit Godwani <mgodwan@amazon.com>

github-actions · 2025-12-29T07:22:07Z

❌ Gradle check result for 1854fe5: 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: Mohit Godwani <mgodwan@amazon.com>

* Changes to get remote upload and replication to work for lucene indices Signed-off-by: Raghuvansh Raj <raghraaj@amazon.com> * Replication changes Signed-off-by: Raghuvansh Raj <raghraaj@amazon.com> * Fixes for remote store integration (#20325) Signed-off-by: Mohit Godwani <mgodwan@amazon.com> * Updated and Refactored the code to have only required changes for recovery * Removed extra logging lines * Resolving PR comments * Updated the VSRManagerTests to use ParquetFileMetadata as flushResult * Fixing RemoteSegmentStoreDirectoryTests and RemoteSegmentStoreDirectoryWithPinnedTimestampTests * Fix for merged files not getting uploaded/deleted Signed-off-by: Raghuvansh Raj <raghraaj@amazon.com> * Added support for SSE KMS testing via run.gradle Signed-off-by: Raghuvansh Raj <raghraaj@amazon.com> * [RemoteStore] Add support for repository with server side encryption enabled to avoid client side encryption (#19630) Signed-off-by: Pranit Kumar <pranikum@amazon.com> * Adding support for SSE KMS in RemoteSegmentStoreDirectoryFactory Signed-off-by: Raghuvansh Raj <raghraaj@amazon.com> * Fixing lucene remote recovery and files should not get written in replicas * Removing extra logging statements * Revert "Fixing lucene remote recovery and files should not get written in replicas" This reverts commit 1f3aa80. * Using NIOFSDirectory in GenericStoreDirectory Signed-off-by: Raghuvansh Raj <raghraaj@amazon.com> * Adding NRTReplicationCompositeEngine for checkpoint tracking during recovery Signed-off-by: Raghuvansh Raj <raghraaj@amazon.com> * Using index input for checksum calculation instead of input stream Signed-off-by: Raghuvansh Raj <raghraaj@amazon.com> * Fixing lucene remote recovery and files should not get written in replicas * Fixed RemoteIndexRecoveryIT::testRerouteRecovery * Removed the isReadonly check from compositeEngine as we have the NRTEngine now * Removing override for getHistoryUUID in NRTReplicationCompositeEngine Signed-off-by: Raghuvansh Raj <raghraaj@amazon.com> * Updated the IndexFileDeleterTests to use absolute file path * Refactoring syncSegmentsFromGivenRemoteSegmentStore and syncSegmentsFromRemoteSegmentStore APIs * Made deleteUnrefrencedFiles api more generic * Modifying UUID to be added before file name extension for catalog support on parquet files Signed-off-by: Raghuvansh Raj <raghraaj@amazon.com> --------- Signed-off-by: Raghuvansh Raj <raghraaj@amazon.com> Signed-off-by: Mohit Godwani <mgodwan@amazon.com> Signed-off-by: Pranit Kumar <pranikum@amazon.com> Co-authored-by: Mohit Godwani <81609427+mgodwan@users.noreply.github.com> Co-authored-by: Kamal Nayan <askkamal@amazon.com> Co-authored-by: pranikum <kpranit81@gmail.com>

Signed-off-by: Mohit Godwani <mgodwan@amazon.com>

* Changes to get remote upload and replication to work for lucene indices Signed-off-by: Raghuvansh Raj <raghraaj@amazon.com> * Replication changes Signed-off-by: Raghuvansh Raj <raghraaj@amazon.com> * Fixes for remote store integration (opensearch-project#20325) Signed-off-by: Mohit Godwani <mgodwan@amazon.com> * Updated and Refactored the code to have only required changes for recovery * Removed extra logging lines * Resolving PR comments * Updated the VSRManagerTests to use ParquetFileMetadata as flushResult * Fixing RemoteSegmentStoreDirectoryTests and RemoteSegmentStoreDirectoryWithPinnedTimestampTests * Fix for merged files not getting uploaded/deleted Signed-off-by: Raghuvansh Raj <raghraaj@amazon.com> * Added support for SSE KMS testing via run.gradle Signed-off-by: Raghuvansh Raj <raghraaj@amazon.com> * [RemoteStore] Add support for repository with server side encryption enabled to avoid client side encryption (opensearch-project#19630) Signed-off-by: Pranit Kumar <pranikum@amazon.com> * Adding support for SSE KMS in RemoteSegmentStoreDirectoryFactory Signed-off-by: Raghuvansh Raj <raghraaj@amazon.com> * Fixing lucene remote recovery and files should not get written in replicas * Removing extra logging statements * Revert "Fixing lucene remote recovery and files should not get written in replicas" This reverts commit 1f3aa80. * Using NIOFSDirectory in GenericStoreDirectory Signed-off-by: Raghuvansh Raj <raghraaj@amazon.com> * Adding NRTReplicationCompositeEngine for checkpoint tracking during recovery Signed-off-by: Raghuvansh Raj <raghraaj@amazon.com> * Using index input for checksum calculation instead of input stream Signed-off-by: Raghuvansh Raj <raghraaj@amazon.com> * Fixing lucene remote recovery and files should not get written in replicas * Fixed RemoteIndexRecoveryIT::testRerouteRecovery * Removed the isReadonly check from compositeEngine as we have the NRTEngine now * Removing override for getHistoryUUID in NRTReplicationCompositeEngine Signed-off-by: Raghuvansh Raj <raghraaj@amazon.com> * Updated the IndexFileDeleterTests to use absolute file path * Refactoring syncSegmentsFromGivenRemoteSegmentStore and syncSegmentsFromRemoteSegmentStore APIs * Made deleteUnrefrencedFiles api more generic * Modifying UUID to be added before file name extension for catalog support on parquet files Signed-off-by: Raghuvansh Raj <raghraaj@amazon.com> --------- Signed-off-by: Raghuvansh Raj <raghraaj@amazon.com> Signed-off-by: Mohit Godwani <mgodwan@amazon.com> Signed-off-by: Pranit Kumar <pranikum@amazon.com> Co-authored-by: Mohit Godwani <81609427+mgodwan@users.noreply.github.com> Co-authored-by: Kamal Nayan <askkamal@amazon.com> Co-authored-by: pranikum <kpranit81@gmail.com>

mgodwan requested review from a team, Bukhtawar, CEHENKLE, Rishikesh1159, anasalkouz, andrross, ashking94, cwperks, dbwiddis, gbbafna, jed326, kotwanikunal, mch2, msfroh, owaiskazi19, reta, sachinpkale, saratvemulapalli, shwetathareja and sohami as code owners December 26, 2025 09:30

Bukhtawar reviewed Dec 27, 2025

View reviewed changes

mgodwan force-pushed the remote-store-validation branch from 3294fec to 0c32359 Compare December 29, 2025 07:08

Fixes for remote store integration

1854fe5

Signed-off-by: Mohit Godwani <mgodwan@amazon.com>

mgodwan force-pushed the remote-store-validation branch from 0c32359 to 1854fe5 Compare December 29, 2025 07:11

Bukhtawar merged commit 283b9f1 into opensearch-project:feature/datafusion Dec 29, 2025
8 of 31 checks passed

bharath-techie pushed a commit to bharath-techie/OpenSearch that referenced this pull request Dec 30, 2025

Fixes for remote store integration (opensearch-project#20325)

7d9ecad

Signed-off-by: Mohit Godwani <mgodwan@amazon.com>

raghuvanshraj pushed a commit to raghuvanshraj/OpenSearch-Fork that referenced this pull request Jan 13, 2026

Fixes for remote store integration (opensearch-project#20325)

0799bb1

Signed-off-by: Mohit Godwani <mgodwan@amazon.com>

raghuvanshraj pushed a commit to raghuvanshraj/OpenSearch-Fork that referenced this pull request Jan 19, 2026

Fixes for remote store integration (opensearch-project#20325)

32a4171

Signed-off-by: Mohit Godwani <mgodwan@amazon.com>

abhita pushed a commit to abhita/OpenSearch that referenced this pull request Jan 26, 2026

Fixes for remote store integration (opensearch-project#20325)

2c1c86a

Signed-off-by: Mohit Godwani <mgodwan@amazon.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Fixes for remote store integration#20325

Fixes for remote store integration#20325
Bukhtawar merged 1 commit intoopensearch-project:feature/datafusionfrom
mgodwan:remote-store-validation

mgodwan commented Dec 26, 2025

Uh oh!

coderabbitai bot commented Dec 26, 2025

Review skipped

Uh oh!

github-actions bot commented Dec 26, 2025

Uh oh!

Bukhtawar Dec 27, 2025

Uh oh!

mgodwan Dec 29, 2025

Uh oh!

Bukhtawar Dec 27, 2025

Uh oh!

mgodwan Dec 29, 2025

Uh oh!

Bukhtawar Dec 27, 2025

Uh oh!

mgodwan Dec 29, 2025 •

edited

Loading

Uh oh!

Bukhtawar Dec 27, 2025

Uh oh!

mgodwan Dec 29, 2025

Uh oh!

github-actions bot commented Dec 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		import java.util.concurrent.atomic.AtomicLong;

		final class LastRefreshedCheckpointListener implements ReferenceManager.RefreshListener {

Comments

Conversation

mgodwan commented Dec 26, 2025

Uh oh!

coderabbitai bot commented Dec 26, 2025

Review skipped

Uh oh!

github-actions bot commented Dec 26, 2025

Uh oh!

Bukhtawar Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

mgodwan Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Bukhtawar Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

mgodwan Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Bukhtawar Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

mgodwan Dec 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Bukhtawar Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

mgodwan Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Dec 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mgodwan Dec 29, 2025 •

edited

Loading