Upstream Fetch

Signed-off-by: Prudhvi Godithi <pgodithi@amazon.com>
opensearch-project · Feb 21, 2025 · ed67958 · ed67958
1 parent 6e2bb1d
commit ed67958
Show file tree

Hide file tree

Showing 6 changed files with 49 additions and 38 deletions.
diff --git a/...sterTest/java/org/opensearch/action/admin/indices/scale/searchonly/SearchOnlyScaleIT.java b/...sterTest/java/org/opensearch/action/admin/indices/scale/searchonly/SearchOnlyScaleIT.java
@@ -6,14 +6,6 @@
  * compatible open source license.
  */
 
-/*
- * SPDX-License-Identifier: Apache-2.0
- *
- * The OpenSearch Contributors require contributions made to
- * this file be licensed under the Apache-2.0 license or a
- * compatible open source license.
- */
-
 package org.opensearch.action.admin.indices.scale.searchonly;
 
 import org.opensearch.action.admin.indices.settings.get.GetSettingsResponse;
@@ -115,8 +107,7 @@ public void testScaleDownToSearchOnly() throws Exception {
         try {
             client().prepareIndex(TEST_INDEX).setId("new-doc").setSource("field1", "new-value").get();
             fail("Expected ClusterBlockException");
-        } catch (ClusterBlockException e) {
-            // Expected exception
+        } catch (ClusterBlockException ignored) {
         }
 
         // Verify routing table structure
@@ -125,7 +116,10 @@ public void testScaleDownToSearchOnly() throws Exception {
             1,
             getClusterState().routingTable().index(TEST_INDEX).shard(0).searchOnlyReplicas().stream().filter(ShardRouting::active).count()
         );
-        assertNull(shardTable.primaryShard());
+        assertBusy(() -> {
+            IndexShardRoutingTable currentShardTable = getClusterState().routingTable().index(TEST_INDEX).shard(0);
+            assertNull("Primary shard should be null after scale-down", currentShardTable.primaryShard());
+        });
 
     }
 
@@ -144,11 +138,12 @@ public void testScaleUpFromSearchOnly() throws Exception {
         ensureGreen(TEST_INDEX);
 
         for (int i = 0; i < 5; i++) {
-            client().prepareIndex(TEST_INDEX)
+            IndexResponse indexResponse = client().prepareIndex(TEST_INDEX)
                 .setId(Integer.toString(i))
                 .setSource("field1", "value" + i)
                 .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
                 .get();
+            assertEquals(RestStatus.CREATED, indexResponse.status());
         }
 
         // Verify initial state has expected replica counts
@@ -203,7 +198,7 @@ public void testScaleUpFromSearchOnly() throws Exception {
 
         // Verify we can search existing data
         SearchResponse searchResponse = client().prepareSearch(TEST_INDEX).get();
-        assertHitCount(searchResponse, 3);
+        assertHitCount(searchResponse, 5);
 
         // Verify we can write to the index again
         IndexResponse indexResponse = client().prepareIndex(TEST_INDEX)

diff --git a/...c/internalClusterTest/java/org/opensearch/indices/replication/SearchReplicaRestoreIT.java b/...c/internalClusterTest/java/org/opensearch/indices/replication/SearchReplicaRestoreIT.java
@@ -8,7 +8,11 @@
 
 package org.opensearch.indices.replication;
 
+import org.opensearch.action.admin.cluster.remotestore.restore.RestoreRemoteStoreRequest;
+import org.opensearch.action.admin.cluster.remotestore.restore.RestoreRemoteStoreResponse;
+import org.opensearch.action.admin.indices.settings.get.GetSettingsResponse;
 import org.opensearch.action.search.SearchResponse;
+import org.opensearch.action.support.PlainActionFuture;
 import org.opensearch.cluster.metadata.IndexMetadata;
 import org.opensearch.cluster.metadata.Metadata;
 import org.opensearch.common.settings.Settings;
@@ -21,7 +25,9 @@
 
 import java.util.List;
 
+import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS;
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SEARCH_REPLICAS;
+import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertHitCount;
 
@@ -102,8 +108,8 @@ private void bootstrapIndexWithOutSearchReplicas(ReplicationType replicationType
 
         Settings settings = Settings.builder()
             .put(super.indexSettings())
-            .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
-            .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1)
+            .put(SETTING_NUMBER_OF_SHARDS, 1)
+            .put(SETTING_NUMBER_OF_REPLICAS, 1)
             .put(SETTING_NUMBER_OF_SEARCH_REPLICAS, 0)
             .put(IndexMetadata.SETTING_REPLICATION_TYPE, replicationType)
             .build();
@@ -114,13 +120,32 @@ private void bootstrapIndexWithOutSearchReplicas(ReplicationType replicationType
         ensureGreen(INDEX_NAME);
     }
 
+    public void testRemoteStoreRestoreFailsForSearchOnlyIndex() throws Exception {
+        bootstrapIndexWithSearchReplicas();
+        assertAcked(client().admin().indices().prepareSearchOnly(INDEX_NAME).setScaleDown(true).get());
+
+        GetSettingsResponse settingsResponse = client().admin().indices().prepareGetSettings(INDEX_NAME).get();
+        assertEquals("true", settingsResponse.getSetting(INDEX_NAME, IndexMetadata.INDEX_BLOCKS_SEARCH_ONLY_SETTING.getKey()));
+
+        IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> {
+            PlainActionFuture<RestoreRemoteStoreResponse> future = PlainActionFuture.newFuture();
+            client().admin().cluster().restoreRemoteStore(new RestoreRemoteStoreRequest().indices(INDEX_NAME), future);
+            future.actionGet();
+        });
+
+        assertTrue(exception.getMessage().contains(
+            "Cannot restore index [" + INDEX_NAME + "] because search-only mode is enabled"
+        ));
+    }
+
+
     private void bootstrapIndexWithSearchReplicas() throws InterruptedException {
         startCluster(3);
 
         Settings settings = Settings.builder()
             .put(super.indexSettings())
-            .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
-            .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1)
+            .put(SETTING_NUMBER_OF_SHARDS, 1)
+            .put(SETTING_NUMBER_OF_REPLICAS, 1)
             .put(SETTING_NUMBER_OF_SEARCH_REPLICAS, 1)
             .put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT)
             .build();

diff --git a/...e/searchonly/ScaleOperationValidator.java → ...rchonly/SearchOnlyOperationValidator.java b/...e/searchonly/ScaleOperationValidator.java → ...rchonly/SearchOnlyOperationValidator.java
@@ -21,7 +21,7 @@
  * index state compatibility, and configuration prerequisites such as remote store
  * and segment replication settings.
  */
-class ScaleOperationValidator {
+class SearchOnlyOperationValidator {
 
     /**
      * Validates that the given index meets the prerequisites for the scale operation.

diff --git a/.../java/org/opensearch/action/admin/indices/scale/searchonly/TransportSearchOnlyAction.java b/.../java/org/opensearch/action/admin/indices/scale/searchonly/TransportSearchOnlyAction.java
@@ -76,7 +76,7 @@ public class TransportSearchOnlyAction extends TransportClusterManagerNodeAction
     private final IndicesService indicesService;
     private final TransportService transportService;
 
-    private final ScaleOperationValidator validator;
+    private final SearchOnlyOperationValidator validator;
     private final SearchOnlyClusterStateBuilder searchOnlyClusterStateBuilder;
     private final SearchOnlyShardSyncManager searchOnlyShardSyncManager;
 
@@ -131,7 +131,7 @@ public TransportSearchOnlyAction(
         this.allocationService = allocationService;
         this.indicesService = indicesService;
         this.transportService = transportService;
-        this.validator = new ScaleOperationValidator();
+        this.validator = new SearchOnlyOperationValidator();
         this.searchOnlyClusterStateBuilder = new SearchOnlyClusterStateBuilder();
         this.searchOnlyShardSyncManager = new SearchOnlyShardSyncManager(clusterService, transportService, NAME);
 

diff --git a/server/src/main/java/org/opensearch/index/recovery/RemoteStoreRestoreService.java b/server/src/main/java/org/opensearch/index/recovery/RemoteStoreRestoreService.java
@@ -167,40 +167,31 @@ public RemoteRestoreResult restore(
                 IndicesOptions.fromOptions(true, true, true, true)
             );
 
-            boolean allSearchOnly = true;
             for (String indexName : filteredIndices) {
                 IndexMetadata indexMetadata = currentState.metadata().index(indexName);
                 if (indexMetadata == null) {
-                    logger.warn("Skipping restore: index [{}] does not exist.", indexName);
+                    logger.warn("Index restore is not supported for non-existent index. Skipping: {}", indexName);
                     continue;
                 }
-
                 boolean isSearchOnly = indexMetadata.getSettings()
                     .getAsBoolean(IndexMetadata.INDEX_BLOCKS_SEARCH_ONLY_SETTING.getKey(), false);
-
                 if (isSearchOnly) {
-                    logger.warn("Skipping _remotestore/_restore for index [{}] as search-only mode is enabled.", indexName);
-                } else if (restoreAllShards && IndexMetadata.State.CLOSE.equals(indexMetadata.getState()) == false) {
+                    throw new IllegalArgumentException(
+                        String.format(Locale.ROOT, "Cannot restore index [%s] because search-only mode is enabled", indexName)
+                    );
+                }
+                if (restoreAllShards && IndexMetadata.State.CLOSE.equals(indexMetadata.getState()) == false) {
                     throw new IllegalStateException(
                         String.format(
                             Locale.ROOT,
                             "cannot restore index [%s] because an open index with same name/uuid already exists in the cluster.",
                             indexName
                         ) + " Close the existing index."
                     );
-                } else {
-                    allSearchOnly = false;
-                    indexMetadataMap.put(indexName, new Tuple<>(false, indexMetadata));
                 }
-            }
-
-            if (allSearchOnly) {
-                throw new IllegalArgumentException(
-                    "Skipping _remotestore/_restore for all selected indices as search-only mode is enabled."
-                );
+                indexMetadataMap.put(indexName, new Tuple<>(false, indexMetadata));
             }
         }
-
         return executeRestore(currentState, indexMetadataMap, restoreAllShards, remoteState);
     }
 

diff --git a/server/src/main/java/org/opensearch/indices/IndicesService.java b/server/src/main/java/org/opensearch/indices/IndicesService.java
@@ -1185,9 +1185,9 @@ public void removeIndex(final Index index, final IndexRemovalReason reason, fina
             }
 
             listener.beforeIndexRemoved(indexService, reason);
-            logger.debug("{} closing index service (reason [{}][{}])", index, reason, extraInfo);
+            logger.info("{} closing index service (reason [{}][{}])", index, reason, extraInfo);
             indexService.close(extraInfo, reason == IndexRemovalReason.DELETED);
-            logger.debug("{} closed... (reason [{}][{}])", index, reason, extraInfo);
+            logger.info("{} closed... (reason [{}][{}])", index, reason, extraInfo);
             final IndexSettings indexSettings = indexService.getIndexSettings();
             listener.afterIndexRemoved(indexService.index(), indexSettings, reason);
             if (reason == IndexRemovalReason.DELETED) {