Allow null metadata file only is shard is inactive

Signed-off-by: Ankit Kala <ankikala@amazon.com>
opensearch-project · Jul 7, 2023 · 8fcd17d · 8fcd17d
1 parent 69596ea
commit 8fcd17d
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 20 deletions.
diff --git a/server/src/main/java/org/opensearch/indices/replication/RemoteStoreReplicationSource.java b/server/src/main/java/org/opensearch/indices/replication/RemoteStoreReplicationSource.java
@@ -14,6 +14,7 @@
 import org.apache.lucene.util.Version;
 import org.opensearch.action.ActionListener;
 import org.opensearch.index.shard.IndexShard;
+import org.opensearch.index.shard.IndexShardState;
 import org.opensearch.index.store.RemoteSegmentStoreDirectory;
 import org.opensearch.index.store.Store;
 import org.opensearch.index.store.StoreFileMetadata;
@@ -55,27 +56,29 @@ public void getCheckpointMetadata(
         final Version version = indexShard.getSegmentInfosSnapshot().get().getCommitLuceneVersion();
         try {
             RemoteSegmentMetadata mdFile = remoteDirectory.readLatestMetadataFile();
-            if (mdFile == null) {
+            // During initial recovery flow, the remote store might not have metadata as primary hasn't uploaded anything yet.
+            if (mdFile == null && indexShard.state().equals(IndexShardState.STARTED) == false) {
                 listener.onResponse(new CheckpointInfoResponse(checkpoint, Collections.emptyMap(), null));
-            } else {
-                metadataMap = mdFile.getMetadata()
-                    .entrySet()
-                    .stream()
-                    .collect(
-                        Collectors.toMap(
-                            e -> e.getKey(),
-                            e -> new StoreFileMetadata(
-                                e.getValue().getOriginalFilename(),
-                                e.getValue().getLength(),
-                                Store.digestToString(Long.valueOf(e.getValue().getChecksum())),
-                                version,
-                                null
-                            )
-                        )
-                    );
-                // TODO: GET current checkpoint from remote store.
-                listener.onResponse(new CheckpointInfoResponse(checkpoint, metadataMap, null));
+                return;
             }
+            assert mdFile != null : "Remote metadata file can't be null if shard is active " + indexShard.state();
+            metadataMap = mdFile.getMetadata()
+                .entrySet()
+                .stream()
+                .collect(
+                    Collectors.toMap(
+                        e -> e.getKey(),
+                        e -> new StoreFileMetadata(
+                            e.getValue().getOriginalFilename(),
+                            e.getValue().getLength(),
+                            Store.digestToString(Long.valueOf(e.getValue().getChecksum())),
+                            version,
+                            null
+                        )
+                    )
+                );
+            // TODO: GET current checkpoint from remote store.
+            listener.onResponse(new CheckpointInfoResponse(checkpoint, metadataMap, null));
         } catch (Exception e) {
             listener.onFailure(e);
         }

diff --git a/...r/src/test/java/org/opensearch/indices/replication/RemoteStoreReplicationSourceTests.java b/...r/src/test/java/org/opensearch/indices/replication/RemoteStoreReplicationSourceTests.java
@@ -17,6 +17,7 @@
 import org.opensearch.index.engine.InternalEngineFactory;
 import org.opensearch.index.replication.OpenSearchIndexLevelReplicationTestCase;
 import org.opensearch.index.shard.IndexShard;
+import org.opensearch.index.shard.IndexShardState;
 import org.opensearch.index.shard.RemoteStoreRefreshListenerTests;
 import org.opensearch.index.store.RemoteSegmentStoreDirectory;
 import org.opensearch.index.store.Store;
@@ -70,7 +71,6 @@ public void setUp() throws Exception {
             new RemoteStoreRefreshListenerTests.TestFilterDirectory(remoteSegmentStoreDirectory)
         );
         when(remoteStore.directory()).thenReturn(remoteStoreFilterDirectory);
-
         replicationSource = new RemoteStoreReplicationSource(mockShard);
     }
 
@@ -140,10 +140,19 @@ public void testGetCheckpointMetadataEmpty() throws ExecutionException, Interrup
             when(remoteStore.directory()).thenReturn(remoteStoreFilterDirectory);
 
             final PlainActionFuture<CheckpointInfoResponse> res = PlainActionFuture.newFuture();
+            when(mockShard.state()).thenReturn(IndexShardState.RECOVERING);
+            // Recovering shard should just do a noop and return empty metadata map.
             replicationSource.getCheckpointMetadata(REPLICATION_ID, checkpoint, res);
             CheckpointInfoResponse response = res.get();
             assert (response.getCheckpoint().equals(checkpoint));
             assert (response.getMetadataMap().isEmpty());
+
+            when(mockShard.state()).thenReturn(IndexShardState.STARTED);
+            // Started shard should fail with assertion error.
+            expectThrows(AssertionError.class, () -> {
+                final PlainActionFuture<CheckpointInfoResponse> res2 = PlainActionFuture.newFuture();
+                replicationSource.getCheckpointMetadata(REPLICATION_ID, checkpoint, res2);
+            });
         } finally {
             closeShards(emptyIndexShard);
         }