[Remote Store] Removing feature flag to mark feature GA (#9761)

Signed-off-by: Shourya Dutta Biswas <114977491+shourya035@users.noreply.github.com>
opensearch-project · Sep 7, 2023 · 6a0c50e · 6a0c50e
1 parent bdf6f1d
commit 6a0c50e
Show file tree

Hide file tree

Showing 37 changed files with 60 additions and 282 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -53,6 +53,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - [Remote Store] Add Segment download stats to remotestore stats API ([#8718](https://github.com/opensearch-project/OpenSearch/pull/8718))
 - [Remote Store] Add remote segment transfer stats on NodesStats API ([#9168](https://github.com/opensearch-project/OpenSearch/pull/9168) [#9393](https://github.com/opensearch-project/OpenSearch/pull/9393) [#9454](https://github.com/opensearch-project/OpenSearch/pull/9454))
 - Return 409 Conflict HTTP status instead of 503 on failure to concurrently execute snapshots ([#8986](https://github.com/opensearch-project/OpenSearch/pull/5855))
+- [Remote Store] Removing feature flag to mark feature GA ([#9761](https://github.com/opensearch-project/OpenSearch/pull/9761))
 
 ### Deprecated
 

diff --git a/distribution/src/config/opensearch.yml b/distribution/src/config/opensearch.yml
@@ -98,18 +98,10 @@ ${path.logs}
 # node.attr.remote_store.translog.repository: my-repo-1
 #
 # ---------------------------------- Experimental Features -----------------------------------
-#
 # Gates the visibility of the experimental segment replication features until they are production ready.
 #
 #opensearch.experimental.feature.segment_replication_experimental.enabled: false
 #
-#
-# Gates the visibility of the index setting that allows persisting data to remote store along with local disk.
-# Once the feature is ready for production release, this feature flag can be removed.
-#
-#opensearch.experimental.feature.remote_store.enabled: false
-#
-#
 # Gates the functionality of a new parameter to the snapshot restore API
 # that allows for creation of a new index type that searches a snapshot
 # directly in a remote repository without restoring all index data to disk

diff --git a/server/src/internalClusterTest/java/org/opensearch/index/SegmentReplicationPressureIT.java b/server/src/internalClusterTest/java/org/opensearch/index/SegmentReplicationPressureIT.java
@@ -15,7 +15,6 @@
 import org.opensearch.common.lease.Releasable;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.unit.TimeValue;
-import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.core.concurrency.OpenSearchRejectedExecutionException;
 import org.opensearch.core.rest.RestStatus;
 import org.opensearch.index.shard.IndexShard;
@@ -31,7 +30,6 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
-import java.util.Objects;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -264,9 +262,9 @@ public void testFailStaleReplica() throws Exception {
     }
 
     public void testWithDocumentReplicationEnabledIndex() throws Exception {
-        assumeTrue(
-            "Can't create DocRep index with remote store enabled. Skipping.",
-            Objects.equals(featureFlagSettings().get(FeatureFlags.REMOTE_STORE, "false"), "false")
+        assumeFalse(
+            "Skipping the test as its not compatible with segment replication with remote store. Cannot create DocRep indices with Remote store enabled",
+            segmentReplicationWithRemoteEnabled()
         );
         Settings settings = Settings.builder()
             .put(MAX_REPLICATION_TIME_BACKPRESSURE_SETTING.getKey(), TimeValue.timeValueMillis(500))

diff --git a/...internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationBaseIT.java b/...internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationBaseIT.java
@@ -18,7 +18,6 @@
 import org.opensearch.common.Nullable;
 import org.opensearch.common.lease.Releasable;
 import org.opensearch.common.settings.Settings;
-import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.core.index.Index;
 import org.opensearch.core.index.shard.ShardId;
 import org.opensearch.index.IndexModule;
@@ -204,8 +203,7 @@ protected IndexShard getIndexShard(String node, String indexName) {
     }
 
     protected boolean segmentReplicationWithRemoteEnabled() {
-        return IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING.get(indexSettings()).booleanValue()
-            && "true".equalsIgnoreCase(featureFlagSettings().get(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL));
+        return IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING.get(indexSettings()).booleanValue();
     }
 
     protected Releasable blockReplication(List<String> nodes, CountDownLatch latch) {

diff --git a/...rTest/java/org/opensearch/remotestore/AbstractRemoteStoreMockRepositoryIntegTestCase.java b/...rTest/java/org/opensearch/remotestore/AbstractRemoteStoreMockRepositoryIntegTestCase.java
@@ -12,15 +12,12 @@
 import org.opensearch.cluster.metadata.IndexMetadata;
 import org.opensearch.common.UUIDs;
 import org.opensearch.common.settings.Settings;
-import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.core.util.FileSystemUtils;
 import org.opensearch.index.IndexModule;
 import org.opensearch.index.IndexSettings;
 import org.opensearch.index.store.RemoteSegmentStoreDirectory;
 import org.opensearch.indices.replication.common.ReplicationType;
 import org.opensearch.snapshots.AbstractSnapshotIntegTestCase;
-import org.opensearch.test.FeatureFlagSetter;
-import org.junit.Before;
 
 import java.io.IOException;
 import java.nio.file.Path;
@@ -44,17 +41,6 @@ public abstract class AbstractRemoteStoreMockRepositoryIntegTestCase extends Abs
     protected static final String TRANSLOG_REPOSITORY_NAME = "my-translog-repo-1";
     protected static final String INDEX_NAME = "remote-store-test-idx-1";
 
-    @Override
-    protected Settings featureFlagSettings() {
-        return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.REMOTE_STORE, "true").build();
-    }
-
-    @Before
-    public void setup() {
-        FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE);
-        FeatureFlagSetter.set(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL);
-    }
-
     @Override
     public Settings indexSettings() {
         return remoteStoreIndexSettings(0);

diff --git a/...c/internalClusterTest/java/org/opensearch/remotestore/RemoteIndexPrimaryRelocationIT.java b/...c/internalClusterTest/java/org/opensearch/remotestore/RemoteIndexPrimaryRelocationIT.java
@@ -10,7 +10,6 @@
 
 import org.opensearch.cluster.metadata.IndexMetadata;
 import org.opensearch.common.settings.Settings;
-import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.indices.recovery.IndexPrimaryRelocationIT;
 import org.opensearch.indices.replication.common.ReplicationType;
 import org.opensearch.test.OpenSearchIntegTestCase;
@@ -50,15 +49,6 @@ public Settings indexSettings() {
             .build();
     }
 
-    @Override
-    protected Settings featureFlagSettings() {
-        return Settings.builder()
-            .put(super.featureFlagSettings())
-            .put(FeatureFlags.REMOTE_STORE, "true")
-            .put(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL, "true")
-            .build();
-    }
-
     public void testPrimaryRelocationWhileIndexing() throws Exception {
         internalCluster().startClusterManagerOnlyNode();
         super.testPrimaryRelocationWhileIndexing();

diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteIndexRecoveryIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteIndexRecoveryIT.java
@@ -10,7 +10,6 @@
 
 import org.opensearch.cluster.metadata.IndexMetadata;
 import org.opensearch.common.settings.Settings;
-import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.index.IndexModule;
 import org.opensearch.index.IndexSettings;
 import org.opensearch.indices.recovery.IndexRecoveryIT;
@@ -46,15 +45,6 @@ protected Settings nodeSettings(int nodeOrdinal) {
             .build();
     }
 
-    @Override
-    protected Settings featureFlagSettings() {
-        return Settings.builder()
-            .put(super.featureFlagSettings())
-            .put(FeatureFlags.REMOTE_STORE, "true")
-            .put(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL, "true")
-            .build();
-    }
-
     @Override
     public Settings indexSettings() {
         return Settings.builder()

diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java
@@ -20,7 +20,6 @@
 import org.opensearch.cluster.metadata.IndexMetadata;
 import org.opensearch.common.io.PathUtils;
 import org.opensearch.common.settings.Settings;
-import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.core.rest.RestStatus;
 import org.opensearch.index.IndexSettings;
 import org.opensearch.indices.replication.common.ReplicationType;
@@ -65,7 +64,6 @@ public void teardown() {
     protected Settings nodeSettings(int nodeOrdinal) {
         return Settings.builder()
             .put(super.nodeSettings(nodeOrdinal))
-            .put(FeatureFlags.REMOTE_STORE, "true")
             .put(remoteStoreClusterSettings(BASE_REMOTE_REPO, remoteRepoPath))
             .build();
     }

diff --git a/...src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreBaseIntegTestCase.java b/...src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreBaseIntegTestCase.java
@@ -22,7 +22,6 @@
 import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.common.UUIDs;
 import org.opensearch.common.settings.Settings;
-import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.core.common.unit.ByteSizeUnit;
 import org.opensearch.index.IndexModule;
 import org.opensearch.index.IndexSettings;
@@ -48,7 +47,6 @@
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.stream.Collectors;
 
-import static org.opensearch.indices.IndicesService.CLUSTER_REPLICATION_TYPE_SETTING;
 import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY;
 import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_REPOSITORY_SETTINGS_ATTRIBUTE_KEY_PREFIX;
 import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_REPOSITORY_TYPE_ATTRIBUTE_KEY_FORMAT;
@@ -136,15 +134,6 @@ protected Settings nodeSettings(int nodeOrdinal) {
         }
     }
 
-    @Override
-    protected Settings featureFlagSettings() {
-        return Settings.builder()
-            .put(super.featureFlagSettings())
-            .put(FeatureFlags.REMOTE_STORE, "true")
-            .put(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL, "true")
-            .build();
-    }
-
     public Settings indexSettings() {
         return defaultIndexSettings();
     }
@@ -186,13 +175,7 @@ public static Settings remoteStoreClusterSettings(
         Path translogRepoPath
     ) {
         Settings.Builder settingsBuilder = Settings.builder();
-
-        if (randomBoolean()) {
-            settingsBuilder.put(CLUSTER_REPLICATION_TYPE_SETTING.getKey(), ReplicationType.SEGMENT);
-        }
-
         settingsBuilder.put(buildRemoteStoreNodeAttributes(segmentRepoName, segmentRepoPath, translogRepoName, translogRepoPath, false));
-
         return settingsBuilder.build();
     }
 

diff --git a/...rnalClusterTest/java/org/opensearch/remotestore/SegmentReplicationUsingRemoteStoreIT.java b/...rnalClusterTest/java/org/opensearch/remotestore/SegmentReplicationUsingRemoteStoreIT.java
@@ -9,7 +9,6 @@
 package org.opensearch.remotestore;
 
 import org.opensearch.common.settings.Settings;
-import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.indices.replication.SegmentReplicationIT;
 import org.opensearch.test.OpenSearchIntegTestCase;
 import org.junit.After;
@@ -22,9 +21,6 @@
 
 /**
  * This class runs Segment Replication Integ test suite with remote store enabled.
- * Setup is similar to SegmentReplicationRemoteStoreIT but this also enables the segment replication using remote store which
- * is behind SEGMENT_REPLICATION_EXPERIMENTAL flag. After this is moved out of experimental, we can combine and keep only one
- * test suite for Segment and Remote store integration tests.
  */
 @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0)
 public class SegmentReplicationUsingRemoteStoreIT extends SegmentReplicationIT {
@@ -47,15 +43,6 @@ protected boolean segmentReplicationWithRemoteEnabled() {
         return true;
     }
 
-    @Override
-    protected Settings featureFlagSettings() {
-        return Settings.builder()
-            .put(super.featureFlagSettings())
-            .put(FeatureFlags.REMOTE_STORE, "true")
-            .put(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL, "true")
-            .build();
-    }
-
     @Before
     public void setup() {
         internalCluster().startClusterManagerOnlyNode();

diff --git a/...sterTest/java/org/opensearch/remotestore/SegmentReplicationWithRemoteStorePressureIT.java b/...sterTest/java/org/opensearch/remotestore/SegmentReplicationWithRemoteStorePressureIT.java
@@ -9,7 +9,6 @@
 package org.opensearch.remotestore;
 
 import org.opensearch.common.settings.Settings;
-import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.index.SegmentReplicationPressureIT;
 import org.opensearch.test.OpenSearchIntegTestCase;
 import org.junit.After;
@@ -22,8 +21,6 @@
 
 /**
  * This class executes the SegmentReplicationPressureIT suite with remote store integration enabled.
- * Setup is similar to SegmentReplicationPressureIT but this also enables the segment replication using remote store which
- * is behind SEGMENT_REPLICATION_EXPERIMENTAL flag.
  */
 @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0)
 public class SegmentReplicationWithRemoteStorePressureIT extends SegmentReplicationPressureIT {
@@ -36,15 +33,6 @@ protected boolean segmentReplicationWithRemoteEnabled() {
         return true;
     }
 
-    @Override
-    protected Settings featureFlagSettings() {
-        return Settings.builder()
-            .put(super.featureFlagSettings())
-            .put(FeatureFlags.REMOTE_STORE, "true")
-            .put(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL, "true")
-            .build();
-    }
-
     @Override
     protected Settings nodeSettings(int nodeOrdinal) {
         return Settings.builder()

diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/CloneSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/CloneSnapshotIT.java
@@ -44,7 +44,6 @@
 import org.opensearch.common.action.ActionFuture;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.unit.TimeValue;
-import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.core.xcontent.NamedXContentRegistry;
 import org.opensearch.index.IndexNotFoundException;
 import org.opensearch.index.snapshots.blobstore.BlobStoreIndexShardSnapshot;
@@ -57,7 +56,6 @@
 import org.opensearch.repositories.RepositoryShardId;
 import org.opensearch.repositories.blobstore.BlobStoreRepository;
 import org.opensearch.snapshots.mockstore.MockRepository;
-import org.opensearch.test.FeatureFlagSetter;
 import org.opensearch.test.OpenSearchIntegTestCase;
 
 import java.nio.file.Path;
@@ -159,7 +157,6 @@ public void testCloneSnapshotIndex() throws Exception {
 
     public void testCloneShallowSnapshotIndex() throws Exception {
         disableRepoConsistencyCheck("This test uses remote store repository");
-        FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE);
         final String remoteStoreRepoName = "remote-store-repo-name";
         final Path remoteStoreRepoPath = randomRepoPath();
         internalCluster().startClusterManagerOnlyNode(remoteStoreClusterSettings(remoteStoreRepoName, remoteStoreRepoPath));
@@ -204,7 +201,6 @@ public void testCloneShallowSnapshotIndex() throws Exception {
 
     public void testShallowCloneNameAvailability() throws Exception {
         disableRepoConsistencyCheck("This test uses remote store repository");
-        FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE);
         final String remoteStoreRepoName = "remote-store-repo-name";
         final Path remoteStorePath = randomRepoPath().toAbsolutePath();
         internalCluster().startClusterManagerOnlyNode(
@@ -245,7 +241,6 @@ public void testShallowCloneNameAvailability() throws Exception {
 
     public void testCloneAfterRepoShallowSettingEnabled() throws Exception {
         disableRepoConsistencyCheck("This test uses remote store repository");
-        FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE);
         final String remoteStoreRepoName = "remote-store-repo-name";
         final Path remoteStoreRepoPath = randomRepoPath();
         internalCluster().startClusterManagerOnlyNode(remoteStoreClusterSettings(remoteStoreRepoName, remoteStoreRepoPath));
@@ -280,7 +275,6 @@ public void testCloneAfterRepoShallowSettingEnabled() throws Exception {
 
     public void testCloneAfterRepoShallowSettingDisabled() throws Exception {
         disableRepoConsistencyCheck("This test uses remote store repository");
-        FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE);
         final String remoteStoreRepoName = "remote-store-repo-name";
         final Path remoteStoreRepoPath = randomRepoPath();
         internalCluster().startClusterManagerOnlyNode(remoteStoreClusterSettings(remoteStoreRepoName, remoteStoreRepoPath));

diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/DeleteSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/DeleteSnapshotIT.java
@@ -16,9 +16,7 @@
 import org.opensearch.common.action.ActionFuture;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.unit.TimeValue;
-import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.remotestore.RemoteStoreBaseIntegTestCase;
-import org.opensearch.test.FeatureFlagSetter;
 import org.opensearch.test.OpenSearchIntegTestCase;
 
 import java.nio.file.Path;
@@ -41,7 +39,6 @@ public class DeleteSnapshotIT extends AbstractSnapshotIntegTestCase {
 
     public void testDeleteSnapshot() throws Exception {
         disableRepoConsistencyCheck("Remote store repository is being used in the test");
-        FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE);
         final Path remoteStoreRepoPath = randomRepoPath();
         internalCluster().startClusterManagerOnlyNode(remoteStoreClusterSettings(REMOTE_REPO_NAME, remoteStoreRepoPath));
         internalCluster().startDataOnlyNode(remoteStoreClusterSettings(REMOTE_REPO_NAME, remoteStoreRepoPath));
@@ -69,7 +66,6 @@ public void testDeleteSnapshot() throws Exception {
 
     public void testDeleteShallowCopySnapshot() throws Exception {
         disableRepoConsistencyCheck("Remote store repository is being used in the test");
-        FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE);
         final Path remoteStoreRepoPath = randomRepoPath();
         internalCluster().startClusterManagerOnlyNode(remoteStoreClusterSettings(REMOTE_REPO_NAME, remoteStoreRepoPath));
         internalCluster().startDataOnlyNode(remoteStoreClusterSettings(REMOTE_REPO_NAME, remoteStoreRepoPath));
@@ -98,8 +94,6 @@ public void testDeleteShallowCopySnapshot() throws Exception {
     // Deleting multiple shallow copy snapshots as part of single delete call with repo having only shallow copy snapshots.
     public void testDeleteMultipleShallowCopySnapshotsCase1() throws Exception {
         disableRepoConsistencyCheck("Remote store repository is being used in the test");
-        FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE);
-
         final Path remoteStoreRepoPath = randomRepoPath();
         internalCluster().startClusterManagerOnlyNode(remoteStoreClusterSettings(REMOTE_REPO_NAME, remoteStoreRepoPath));
         internalCluster().startDataOnlyNode(remoteStoreClusterSettings(REMOTE_REPO_NAME, remoteStoreRepoPath));
@@ -142,8 +136,6 @@ public void testDeleteMultipleShallowCopySnapshotsCase1() throws Exception {
     @AwaitsFix(bugUrl = "https://github.com/opensearch-project/OpenSearch/issues/8610")
     public void testDeleteMultipleShallowCopySnapshotsCase2() throws Exception {
         disableRepoConsistencyCheck("Remote store repository is being used in the test");
-        FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE);
-
         final Path remoteStoreRepoPath = randomRepoPath();
         internalCluster().startClusterManagerOnlyNode(remoteStoreClusterSettings(REMOTE_REPO_NAME, remoteStoreRepoPath));
         final String dataNode = internalCluster().startDataOnlyNode(remoteStoreClusterSettings(REMOTE_REPO_NAME, remoteStoreRepoPath));
@@ -228,8 +220,6 @@ public void testDeleteMultipleShallowCopySnapshotsCase2() throws Exception {
     @AwaitsFix(bugUrl = "https://github.com/opensearch-project/OpenSearch/issues/8610")
     public void testDeleteMultipleShallowCopySnapshotsCase3() throws Exception {
         disableRepoConsistencyCheck("Remote store repository is being used in the test");
-        FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE);
-
         final Path remoteStoreRepoPath = randomRepoPath();
         internalCluster().startClusterManagerOnlyNode(remoteStoreClusterSettings(REMOTE_REPO_NAME, remoteStoreRepoPath));
         internalCluster().startDataOnlyNode(remoteStoreClusterSettings(REMOTE_REPO_NAME, remoteStoreRepoPath));
@@ -288,8 +278,6 @@ public void testDeleteMultipleShallowCopySnapshotsCase3() throws Exception {
 
     public void testRemoteStoreCleanupForDeletedIndex() throws Exception {
         disableRepoConsistencyCheck("Remote store repository is being used in the test");
-        FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE);
-
         final Path remoteStoreRepoPath = randomRepoPath();
         internalCluster().startClusterManagerOnlyNode(remoteStoreClusterSettings(REMOTE_REPO_NAME, remoteStoreRepoPath));
         internalCluster().startDataOnlyNode(remoteStoreClusterSettings(REMOTE_REPO_NAME, remoteStoreRepoPath));