skip overriding routing table when it already contains entries with r…

…emote recovery source (opensearch-project#9962) * skip overriding routing table when it already contains entries with remote recovery source Signed-off-by: bansvaru <bansvaru@amazon.com>
Poojita-Raj · Sep 16, 2023 · 5fdd418 · 5fdd418
1 parent 70a582f
commit 5fdd418
Show file tree

Hide file tree

Showing 3 changed files with 337 additions and 2 deletions.
diff --git a/server/src/main/java/org/opensearch/gateway/ClusterStateUpdaters.java b/server/src/main/java/org/opensearch/gateway/ClusterStateUpdaters.java
@@ -41,6 +41,7 @@
 import org.opensearch.cluster.metadata.Metadata;
 import org.opensearch.cluster.node.DiscoveryNode;
 import org.opensearch.cluster.node.DiscoveryNodes;
+import org.opensearch.cluster.routing.RecoverySource;
 import org.opensearch.cluster.routing.RoutingTable;
 import org.opensearch.common.settings.ClusterSettings;
 
@@ -120,7 +121,21 @@ static ClusterState updateRoutingTable(final ClusterState state) {
         // initialize all index routing tables as empty
         final RoutingTable.Builder routingTableBuilder = RoutingTable.builder(state.routingTable());
         for (final IndexMetadata cursor : state.metadata().indices().values()) {
-            routingTableBuilder.addAsRecovery(cursor);
+            // Whether IndexMetadata is recovered from local disk or remote it doesn't matter to us at this point.
+            // We are only concerned about index data recovery here. Which is why we only check for remote store enabled and not for remote
+            // cluster state enabled.
+            if (cursor.getSettings().getAsBoolean(IndexMetadata.SETTING_REMOTE_STORE_ENABLED, false) == false
+                || state.routingTable().hasIndex(cursor.getIndex()) == false
+                || state.routingTable()
+                    .index(cursor.getIndex())
+                    .shardsMatchingPredicateCount(
+                        shardRouting -> shardRouting.primary()
+                            // We need to ensure atleast one of the primaries is being recovered from remote.
+                            // This ensures we have gone through the RemoteStoreRestoreService and routing table is updated
+                            && shardRouting.recoverySource() instanceof RecoverySource.RemoteStoreRecoverySource
+                    ) == 0) {
+                routingTableBuilder.addAsRecovery(cursor);
+            }
         }
         // start with 0 based versions for routing table
         routingTableBuilder.version(0);

diff --git a/server/src/main/java/org/opensearch/index/recovery/RemoteStoreRestoreService.java b/server/src/main/java/org/opensearch/index/recovery/RemoteStoreRestoreService.java
@@ -187,7 +187,7 @@ private RemoteRestoreResult executeRestore(
             IndexMetadata indexMetadata = indexMetadataEntry.getValue().v2();
             boolean metadataFromRemoteStore = indexMetadataEntry.getValue().v1();
             IndexMetadata updatedIndexMetadata = indexMetadata;
-            if (restoreAllShards || metadataFromRemoteStore) {
+            if (metadataFromRemoteStore == false && restoreAllShards) {
                 updatedIndexMetadata = IndexMetadata.builder(indexMetadata)
                     .state(IndexMetadata.State.OPEN)
                     .version(1 + indexMetadata.getVersion())