Disallow resize for Warm Index, add Parameterized ITs for close in remote store (opensearch-project#18686)

Signed-off-by: Gagan Singh Saini <gagasa@amazon.com>
Co-authored-by: Gaurav Bafna <gbbafna@amazon.com>

Author: 2 people

CHANGELOG.md

@@ -7,6 +7,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 ### Added
 - Add support for Warm Indices Write Block on Flood Watermark breach ([#18375](https://github.com/opensearch-project/OpenSearch/pull/18375))
 - Add support for custom index name resolver from cluster plugin ([#18593](https://github.com/opensearch-project/OpenSearch/pull/18593))
+- Disallow resize for Warm Index, add Parameterized ITs for close in remote store ([#18686](https://github.com/opensearch-project/OpenSearch/pull/18686))
 - Ability to run Code Coverage with Gradle and produce the jacoco reports locally ([#18509](https://github.com/opensearch-project/OpenSearch/issues/18509))
 - Add NodeResourceUsageStats to ClusterInfo ([#18480](https://github.com/opensearch-project/OpenSearch/issues/18472))
 - Introduce SecureHttpTransportParameters experimental API (to complement SecureTransportParameters counterpart) ([#18572](https://github.com/opensearch-project/OpenSearch/issues/18572))

server/src/internalClusterTest/java/org/opensearch/indices/state/CloseIndexIT.java

@@ -32,6 +32,8 @@
 
 package org.opensearch.indices.state;
 
+import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
+
 import org.opensearch.ExceptionsHelper;
 import org.opensearch.action.ActionRequestValidationException;
 import org.opensearch.action.admin.indices.close.CloseIndexRequestBuilder;
@@ -56,11 +58,13 @@
 import org.opensearch.indices.recovery.RecoveryState;
 import org.opensearch.test.BackgroundIndexer;
 import org.opensearch.test.InternalTestCluster;
-import org.opensearch.test.OpenSearchIntegTestCase;
+import org.opensearch.test.ParameterizedStaticSettingsOpenSearchIntegTestCase;
 import org.opensearch.transport.client.Client;
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 import java.util.Locale;
 import java.util.concurrent.CountDownLatch;
@@ -70,6 +74,7 @@
 import static java.util.Collections.emptySet;
 import static java.util.stream.Collectors.toList;
 import static org.opensearch.action.support.IndicesOptions.lenientExpandOpen;
+import static org.opensearch.search.SearchService.CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING;
 import static org.opensearch.search.internal.SearchContext.TRACK_TOTAL_HITS_ACCURATE;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertHitCount;
@@ -82,10 +87,23 @@
 import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.nullValue;
 
-public class CloseIndexIT extends OpenSearchIntegTestCase {
+public class CloseIndexIT extends ParameterizedStaticSettingsOpenSearchIntegTestCase {
 
     private static final int MAX_DOCS = 25_000;
 
+    public CloseIndexIT(Settings nodeSettings) {
+        super(nodeSettings);
+    }
+
+    // This is to reuse CloseIndexIT in RemoteCloseIndexIT .
+    // Concurrent search deosn't make a difference in these tests.
+    @ParametersFactory
+    public static Collection<Object[]> parameters() {
+        return Collections.singletonList(
+            new Object[] { Settings.builder().put(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(), false).build() }
+        );
+    }
+
     @Override
     public Settings indexSettings() {
         return Settings.builder()

server/src/internalClusterTest/java/org/opensearch/indices/state/RemoteCloseIndexIT.java

@@ -0,0 +1,74 @@
+/*
+ * 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.indices.state;
+
+import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
+
+import org.opensearch.cluster.metadata.IndexMetadata;
+import org.opensearch.common.settings.Settings;
+import org.opensearch.core.common.unit.ByteSizeUnit;
+import org.opensearch.core.common.unit.ByteSizeValue;
+import org.opensearch.index.IndexModule;
+import org.opensearch.index.IndexSettings;
+import org.opensearch.node.Node;
+
+import java.nio.file.Path;
+import java.util.Arrays;
+import java.util.Collection;
+
+import static org.opensearch.common.util.FeatureFlags.WRITABLE_WARM_INDEX_SETTING;
+
+public class RemoteCloseIndexIT extends CloseIndexIT {
+
+    public RemoteCloseIndexIT(Settings nodeSettings) {
+        super(nodeSettings);
+    }
+
+    protected Path remoteRepoPath;
+
+    protected final static String TEST_REMOTE_STORE_REPO_SUFFIX = "__rs";
+    protected static final String BASE_REMOTE_REPO = "test-rs-repo" + TEST_REMOTE_STORE_REPO_SUFFIX;
+
+    @Override
+    protected Settings nodeSettings(int nodeOrdinal) {
+        if (remoteRepoPath == null) {
+            remoteRepoPath = randomRepoPath().toAbsolutePath();
+        }
+        ByteSizeValue cacheSize = new ByteSizeValue(16, ByteSizeUnit.GB);
+        return Settings.builder()
+            .put(super.nodeSettings(nodeOrdinal))
+            .put(remoteStoreClusterSettings(BASE_REMOTE_REPO, remoteRepoPath))
+            .put(Node.NODE_SEARCH_CACHE_SIZE_SETTING.getKey(), cacheSize.toString())
+            .build();
+    }
+
+    protected Settings.Builder getIndexSettings(int numOfShards, int numOfReplicas) {
+        Settings.Builder settingsBuilder = Settings.builder()
+            .put(super.indexSettings())
+            .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, numOfShards)
+            .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numOfReplicas)
+            .put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), "300s");
+        if (WRITABLE_WARM_INDEX_SETTING.get(settings)) {
+            settingsBuilder.put(IndexModule.IS_WARM_INDEX_SETTING.getKey(), true);
+        }
+        return settingsBuilder;
+    }
+
+    @ParametersFactory
+    public static Collection<Object[]> parameters() {
+        return Arrays.asList(
+            new Object[] { Settings.builder().put(WRITABLE_WARM_INDEX_SETTING.getKey(), true).build() },
+            new Object[] { Settings.builder().put(WRITABLE_WARM_INDEX_SETTING.getKey(), false).build() }
+        );
+    }
+
+    void assertNoFileBasedRecovery(String indexName) {
+        // skipping for remote store
+    }
+}

server/src/internalClusterTest/java/org/opensearch/remotestore/WritableWarmIT.java

@@ -18,6 +18,7 @@
 import org.opensearch.action.admin.indices.delete.DeleteIndexRequest;
 import org.opensearch.action.admin.indices.get.GetIndexRequest;
 import org.opensearch.action.admin.indices.get.GetIndexResponse;
+import org.opensearch.action.admin.indices.shrink.ResizeType;
 import org.opensearch.action.search.SearchResponse;
 import org.opensearch.cluster.metadata.IndexMetadata;
 import org.opensearch.common.settings.Settings;
@@ -34,9 +35,11 @@
 import org.opensearch.index.store.remote.filecache.FileCache;
 import org.opensearch.index.store.remote.utils.FileTypeUtils;
 import org.opensearch.indices.IndicesService;
+import org.opensearch.indices.replication.common.ReplicationType;
 import org.opensearch.node.Node;
 import org.opensearch.test.InternalTestCluster;
 import org.opensearch.test.OpenSearchIntegTestCase;
+import org.junit.Assert;
 
 import java.util.Arrays;
 import java.util.HashSet;
@@ -45,8 +48,11 @@
 import java.util.concurrent.ExecutionException;
 import java.util.stream.Collectors;
 
+import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_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;
+import static org.hamcrest.core.StringContains.containsString;
 
 @ThreadLeakFilters(filters = CleanerDaemonThreadLeakFilter.class)
 @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0, supportsDedicatedMasters = false)
@@ -247,4 +253,41 @@ public void testFullFileAndFileCacheStats() throws ExecutionException, Interrupt
     private boolean isFileCacheEmpty(AggregateFileCacheStats stats) {
         return stats.getUsed().getBytes() == 0L && stats.getActive().getBytes() == 0L;
     }
+
+    public void testNoResizeOnWarm() {
+        InternalTestCluster internalTestCluster = internalCluster();
+        internalTestCluster.startClusterManagerOnlyNode();
+        internalCluster().startDataAndWarmNodes(1).get(0);
+        Settings idxSettings = Settings.builder()
+            .put(super.indexSettings())
+            .put(IndexModule.INDEX_QUERY_CACHE_ENABLED_SETTING.getKey(), false)
+            .put(SETTING_NUMBER_OF_REPLICAS, 0)
+            .put(SETTING_NUMBER_OF_SHARDS, 2)
+            .put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT)
+            .put(IndexModule.IS_WARM_INDEX_SETTING.getKey(), true)
+            .build();
+
+        createIndex(INDEX_NAME, idxSettings);
+        ensureYellowAndNoInitializingShards(INDEX_NAME);
+        ensureGreen(INDEX_NAME);
+        client().admin().indices().prepareUpdateSettings(INDEX_NAME).setSettings(Settings.builder().put("index.blocks.write", true)).get();
+        ResizeType type = randomFrom(ResizeType.CLONE, ResizeType.SHRINK, ResizeType.SPLIT);
+        try {
+            client().admin()
+                .indices()
+                .prepareResizeIndex(INDEX_NAME, "target")
+                .setResizeType(type)
+                .setSettings(
+                    Settings.builder()
+                        .put("index.number_of_replicas", 0)
+                        .put("index.number_of_shards", 2)
+                        .putNull("index.blocks.write")
+                        .build()
+                )
+                .get();
+            fail();
+        } catch (Exception e) {
+            Assert.assertThat(e.getMessage(), containsString("cannot resize warm index"));
+        }
+    }
 }

server/src/main/java/org/opensearch/action/admin/indices/shrink/TransportResizeAction.java

@@ -53,6 +53,7 @@
 import org.opensearch.core.common.io.stream.StreamInput;
 import org.opensearch.core.common.unit.ByteSizeValue;
 import org.opensearch.core.index.shard.ShardId;
+import org.opensearch.index.IndexModule;
 import org.opensearch.index.IndexNotFoundException;
 import org.opensearch.index.IndexSettings;
 import org.opensearch.index.shard.DocsStats;
@@ -146,6 +147,10 @@ protected void clusterManagerOperation(
         final String sourceIndex = indexNameExpressionResolver.resolveDateMathExpression(resizeRequest.getSourceIndex());
         final String targetIndex = indexNameExpressionResolver.resolveDateMathExpression(resizeRequest.getTargetIndexRequest().index());
         IndexMetadata indexMetadata = state.metadata().index(sourceIndex);
+        if (indexMetadata.getSettings().getAsBoolean(IndexModule.IS_WARM_INDEX_SETTING.getKey(), false) == true) {
+            throw new IllegalStateException("cannot resize warm index");
+        }
+
         ClusterSettings clusterSettings = clusterService.getClusterSettings();
         if (resizeRequest.getResizeType().equals(ResizeType.SHRINK)
             && state.metadata().isSegmentReplicationEnabled(sourceIndex)