Fix IndexShardTests: Add explicit null-check for Engine reference and fix FileMetadata serialization handling #20444

ask-kamal-nayan · 2026-01-20T11:40:58Z

Description

This PR fixes failing IndexShardTests by addressing two main issues:

Adding explicit null-checking when accessing the underlying Engine reference
Fixing FileMetadata serialization/deserialization for proper handling of Lucene file names in non-optimized indices

Problem 1: Engine Null Reference

Several IndexShardTests were failing with NullPointerException when tests accessed shard methods before the engine was fully initialized or after it was closed.

Problem 2: FileMetadata Serialization

For non-optimized (standard Lucene) indices, file names were incorrectly being used in serialized format (e.g., segment_1.si:::lucene) instead of plain file names (e.g., segment_1.si), causing file lookup failures and test issues.

Problem 3: RemoteStoreRefreshListenerTests Failing:

Because of the changes made to RemoteStoreRefreshListener class to support mulit-format because of which all tests in RemoteStoreRefreshListenerTests were failing.

Tests Now Passing (4 tests fixed)

testCloseShardWhileEngineIsWarming
testEstimateTotalDocSize
testIndexingOperationsListeners
testSyncSegmentsFromGivenRemoteSegmentStore

All RemoteStoreRefreshListenerTests are fixed now:

Before this fix 81 tests were passing:

After this fix 85 tests are passing. Most of the remaining failed tests are related to translogOps compression during shardClose call:

Tests:

:modules:parquet-data-format:internalClusterTest are passing.
ParquetRemoteStoreRecoveryIt tests are passing except testReplicaPromotionWithTranslogReplay, also failing in datafusion branch.
Manual tests going on.

coderabbitai · 2026-01-20T11:41:04Z

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.

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

github-actions · 2026-01-20T11:46:49Z

❌ Gradle check result for 0ae6d0b: 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 · 2026-01-20T17:16:46Z

❌ Gradle check result for 6154c82: 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 · 2026-01-21T07:30:53Z

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

raghuvanshraj · 2026-01-21T16:05:26Z

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

+                    Collection<FileMetadata> localFileMetadataList = catalogSnapshotRef.getRef().getFileMetadataList();
+                    Set<String> localFiles = localFileMetadataList.stream()
+                        .filter(fm -> !RemoteStoreRefreshListener.EXCLUDE_FILES.contains(fm.file()))
+                        .map(FileMetadata::serialize)


If these are local file references, why are we using serialize? We should be using FileMetadata::file here.

There is a check below which checks if the uploadedFiles contains these local files and uploadedFile names are serialized. I have updated the name to serializedLocalFiles for better readability of the code.

raghuvanshraj · 2026-01-21T16:07:40Z

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

+                    if(isOptimizedIndex())
+                    {
+                        segmentNFile = file;
+                    }
+                    else
+                    {
+                        segmentNFile = fileMetadata.file();
+                    }


If we are doing a new FileMetadata(file) call, file should already be delimited with data format name, why is the segmentsNFile reference okay to be updated as file here?

These segmentNFiles are used by caller syncSegmentsFromGivenRemoteSegmentStore. syncSegmentsFromGivenRemoteSegmentStore API calls OpenInput on this segmentNFile so for normal storeDirectory we need the name in plain file format but for compositeStoreDirectory we need the serialized name so that it can be routed to correct directory.

github-actions · 2026-01-22T09:33:55Z

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

raghuvanshraj · 2026-01-22T10:23:09Z

server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java

    @Override
    public void deleteFile(String name) throws IOException {
-        String remoteFilename = getExistingRemoteFilename(name);
+        String fileName = new FileMetadata(name).toString();


nit: Can we use serialize directly here?

raghuvanshraj · 2026-01-22T10:27:37Z

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

                    assert segmentNFile == null : "There should be only one SegmentInfosSnapshot file";
-                    if(isOptimizedIndex())
-                    {
+                    if(isOptimizedIndex()){
                        segmentNFile = file;
                    }
-                    else
-                    {
+                    else{
                        segmentNFile = fileMetadata.file();
                    }
                }


nit: formatting

github-actions · 2026-01-22T11:08:28Z

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

… fix FileMetadata serialization handling (opensearch-project#20444) * Fixing IndexShardTests recovery tests * Removed unnecessary comment * updated getNativeBytesused api * Fixed RemoteStoreListenerTests and resolved comments * nit fix --------- Co-authored-by: Kamal Nayan <[email protected]>

Fixing IndexShardTests recovery tests

0ae6d0b

ask-kamal-nayan marked this pull request as ready for review January 20, 2026 11:41

ask-kamal-nayan 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 January 20, 2026 11:41

Removed unnecessary comment

6154c82

updated getNativeBytesused api

e1948d5

raghuvanshraj suggested changes Jan 21, 2026

View reviewed changes

Fixed RemoteStoreListenerTests and resolved comments

13fbfed

opensearch-infra bot added the lucene label Jan 22, 2026

raghuvanshraj reviewed Jan 22, 2026

View reviewed changes

nit fix

007efe5

bharath-techie merged commit d2f1abe into opensearch-project:feature/datafusion Jan 22, 2026
7 of 31 checks passed

Fix IndexShardTests: Add explicit null-check for Engine reference and fix FileMetadata serialization handling #20444

Fix IndexShardTests: Add explicit null-check for Engine reference and fix FileMetadata serialization handling #20444

Conversation

ask-kamal-nayan commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Problem 1: Engine Null Reference

Problem 2: FileMetadata Serialization

Problem 3: RemoteStoreRefreshListenerTests Failing:

Tests Now Passing (4 tests fixed)

All RemoteStoreRefreshListenerTests are fixed now:

Tests:

Uh oh!

coderabbitai bot commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

github-actions bot commented Jan 20, 2026

Uh oh!

github-actions bot commented Jan 20, 2026

Uh oh!

github-actions bot commented Jan 21, 2026

Uh oh!

raghuvanshraj Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

ask-kamal-nayan Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

raghuvanshraj Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

ask-kamal-nayan Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 22, 2026

Uh oh!

raghuvanshraj Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

raghuvanshraj Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ask-kamal-nayan commented Jan 20, 2026 •

edited

Loading

coderabbitai bot commented Jan 20, 2026 •

edited

Loading