From 3b3a9489f9b8b7f0bdf42638fb70a60f56d37996 Mon Sep 17 00:00:00 2001 From: Gaurav Bafna Date: Sun, 9 Jun 2024 16:18:09 +0530 Subject: [PATCH] Move Remote Store Migration from DocRep to GA. Modify remote store migration setting name. Signed-off-by: Gaurav Bafna --- CHANGELOG.md | 1 + .../remotemigration/MigrationBaseTestCase.java | 6 ------ .../ResizeIndexMigrationTestCase.java | 6 ++++++ .../org/opensearch/cluster/ClusterModule.java | 5 +---- .../opensearch/common/util/FeatureFlags.java | 2 +- .../remotestore/RemoteStoreNodeService.java | 18 ++++-------------- .../opensearch/cluster/ClusterModuleTests.java | 7 ++----- .../coordination/JoinTaskExecutorTests.java | 7 ------- ...teStoreMigrationAllocationDeciderTests.java | 8 -------- .../decider/FilterAllocationDeciderTests.java | 3 --- 10 files changed, 15 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 539f5a6628dac..21148d55aab09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [Query Insights] Add exporter support for top n queries ([#12982](https://github.com/opensearch-project/OpenSearch/pull/12982)) - [Query Insights] Add X-Opaque-Id to search request metadata for top n queries ([#13374](https://github.com/opensearch-project/OpenSearch/pull/13374)) - Add support for query level resource usage tracking ([#13172](https://github.com/opensearch-project/OpenSearch/pull/13172)) +- Move Remote Store Migration from DocRep to GA ([#14100](https://github.com/opensearch-project/OpenSearch/pull/14100)) ### Dependencies - Bump `com.github.spullara.mustache.java:compiler` from 0.9.10 to 0.9.13 ([#13329](https://github.com/opensearch-project/OpenSearch/pull/13329), [#13559](https://github.com/opensearch-project/OpenSearch/pull/13559)) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotemigration/MigrationBaseTestCase.java b/server/src/internalClusterTest/java/org/opensearch/remotemigration/MigrationBaseTestCase.java index b65f6f056aae6..901b36f872622 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotemigration/MigrationBaseTestCase.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotemigration/MigrationBaseTestCase.java @@ -21,7 +21,6 @@ import org.opensearch.cluster.routing.RoutingNode; import org.opensearch.common.UUIDs; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.repositories.fs.ReloadableFsRepository; import org.opensearch.test.OpenSearchIntegTestCase; import org.junit.Before; @@ -87,11 +86,6 @@ protected Settings nodeSettings(int nodeOrdinal) { } } - @Override - protected Settings featureFlagSettings() { - return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true").build(); - } - protected void setFailRate(String repoName, int value) throws ExecutionException, InterruptedException { GetRepositoriesRequest gr = new GetRepositoriesRequest(new String[] { repoName }); GetRepositoriesResponse res = client().admin().cluster().getRepositories(gr).get(); diff --git a/server/src/internalClusterTest/java/org/opensearch/remotemigration/ResizeIndexMigrationTestCase.java b/server/src/internalClusterTest/java/org/opensearch/remotemigration/ResizeIndexMigrationTestCase.java index b817906a8f828..b804e6dbc1231 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotemigration/ResizeIndexMigrationTestCase.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotemigration/ResizeIndexMigrationTestCase.java @@ -12,6 +12,7 @@ import org.opensearch.action.admin.indices.shrink.ResizeType; import org.opensearch.action.support.ActiveShardCount; import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.FeatureFlags; import org.opensearch.indices.replication.common.ReplicationType; import org.opensearch.test.OpenSearchIntegTestCase; @@ -27,6 +28,11 @@ public class ResizeIndexMigrationTestCase extends MigrationBaseTestCase { private final static String DOC_REP_DIRECTION = "docrep"; private final static String MIXED_MODE = "mixed"; + @Override + protected Settings featureFlagSettings() { + return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true").build(); + } + /* * This test will verify the resize request failure, when cluster mode is mixed * and index is on DocRep node, and migration to remote store is in progress. diff --git a/server/src/main/java/org/opensearch/cluster/ClusterModule.java b/server/src/main/java/org/opensearch/cluster/ClusterModule.java index f56c906db1002..c7fd263bda56a 100644 --- a/server/src/main/java/org/opensearch/cluster/ClusterModule.java +++ b/server/src/main/java/org/opensearch/cluster/ClusterModule.java @@ -84,7 +84,6 @@ import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Setting.Property; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.common.util.set.Sets; import org.opensearch.core.ParseField; @@ -384,9 +383,7 @@ public static Collection createAllocationDeciders( addAllocationDecider(deciders, new AwarenessAllocationDecider(settings, clusterSettings)); addAllocationDecider(deciders, new NodeLoadAwareAllocationDecider(settings, clusterSettings)); addAllocationDecider(deciders, new TargetPoolAllocationDecider()); - if (FeatureFlags.isEnabled(FeatureFlags.REMOTE_STORE_MIGRATION_EXPERIMENTAL_SETTING)) { - addAllocationDecider(deciders, new RemoteStoreMigrationAllocationDecider(settings, clusterSettings)); - } + addAllocationDecider(deciders, new RemoteStoreMigrationAllocationDecider(settings, clusterSettings)); clusterPlugins.stream() .flatMap(p -> p.createAllocationDeciders(settings, clusterSettings).stream()) diff --git a/server/src/main/java/org/opensearch/common/util/FeatureFlags.java b/server/src/main/java/org/opensearch/common/util/FeatureFlags.java index 82f43921d2d28..6c6e2f2d600f0 100644 --- a/server/src/main/java/org/opensearch/common/util/FeatureFlags.java +++ b/server/src/main/java/org/opensearch/common/util/FeatureFlags.java @@ -23,7 +23,7 @@ */ public class FeatureFlags { /** - * Gates the visibility of the remote store migration support from docrep . + * Gates the visibility of the remote store to docrep migration. */ public static final String REMOTE_STORE_MIGRATION_EXPERIMENTAL = "opensearch.experimental.feature.remote_store.migration.enabled"; diff --git a/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeService.java b/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeService.java index 874c9408de6c5..cc5d8b0e62e90 100644 --- a/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeService.java +++ b/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeService.java @@ -42,31 +42,21 @@ public class RemoteStoreNodeService { private final Supplier repositoriesService; private final ThreadPool threadPool; public static final Setting REMOTE_STORE_COMPATIBILITY_MODE_SETTING = new Setting<>( - "remote_store.compatibility_mode", + "cluster.remote_store.compatibility_mode", CompatibilityMode.STRICT.name(), CompatibilityMode::parseString, - value -> { - if (value == CompatibilityMode.MIXED - && FeatureFlags.isEnabled(FeatureFlags.REMOTE_STORE_MIGRATION_EXPERIMENTAL_SETTING) == false) { - throw new IllegalArgumentException( - " mixed mode is under an experimental feature and can be activated only by enabling " - + REMOTE_STORE_MIGRATION_EXPERIMENTAL - + " feature flag in the JVM options " - ); - } - }, Setting.Property.Dynamic, Setting.Property.NodeScope ); public static final Setting MIGRATION_DIRECTION_SETTING = new Setting<>( - "migration.direction", + "cluster.migration.direction", Direction.NONE.name(), Direction::parseString, value -> { - if (value != Direction.NONE && FeatureFlags.isEnabled(FeatureFlags.REMOTE_STORE_MIGRATION_EXPERIMENTAL_SETTING) == false) { + if (value == Direction.DOCREP && FeatureFlags.isEnabled(FeatureFlags.REMOTE_STORE_MIGRATION_EXPERIMENTAL_SETTING) == false) { throw new IllegalArgumentException( - " migration.direction is under an experimental feature and can be activated only by enabling " + " remote store to docrep migration.direction is under an experimental feature and can be activated only by enabling " + REMOTE_STORE_MIGRATION_EXPERIMENTAL + " feature flag in the JVM options " ); diff --git a/server/src/test/java/org/opensearch/cluster/ClusterModuleTests.java b/server/src/test/java/org/opensearch/cluster/ClusterModuleTests.java index ae35d37fe77b2..f2d99a51f1c9a 100644 --- a/server/src/test/java/org/opensearch/cluster/ClusterModuleTests.java +++ b/server/src/test/java/org/opensearch/cluster/ClusterModuleTests.java @@ -68,7 +68,6 @@ import org.opensearch.common.settings.Setting.Property; import org.opensearch.common.settings.Settings; import org.opensearch.common.settings.SettingsModule; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.gateway.GatewayAllocator; import org.opensearch.plugins.ClusterPlugin; @@ -253,11 +252,9 @@ public void testAllocationDeciderOrder() { ShardsLimitAllocationDecider.class, AwarenessAllocationDecider.class, NodeLoadAwareAllocationDecider.class, - TargetPoolAllocationDecider.class + TargetPoolAllocationDecider.class, + RemoteStoreMigrationAllocationDecider.class ); - if (FeatureFlags.isEnabled(FeatureFlags.REMOTE_STORE_MIGRATION_EXPERIMENTAL_SETTING)) { - expectedDeciders.add(RemoteStoreMigrationAllocationDecider.class); - } Collection deciders = ClusterModule.createAllocationDeciders( Settings.EMPTY, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), diff --git a/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java b/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java index 9cb1bd0b57132..9f463673aa6a6 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java @@ -420,8 +420,6 @@ public void testRemoteStoreNodeJoiningNonRemoteStoreClusterMixedMode() { .put(MIGRATION_DIRECTION_SETTING.getKey(), RemoteStoreNodeService.Direction.REMOTE_STORE) .put(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey(), "mixed") .build(); - final Settings nodeSettings = Settings.builder().put(REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true").build(); - FeatureFlags.initializeFeatureFlags(nodeSettings); Metadata metadata = Metadata.builder().persistentSettings(settings).build(); ClusterState currentState = ClusterState.builder(ClusterName.DEFAULT) .nodes(DiscoveryNodes.builder().add(existingNode).localNodeId(existingNode.getId()).build()) @@ -439,8 +437,6 @@ public void testAllTypesNodeJoiningRemoteStoreClusterMixedMode() { .put(MIGRATION_DIRECTION_SETTING.getKey(), RemoteStoreNodeService.Direction.REMOTE_STORE) .put(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey(), "mixed") .build(); - final Settings nodeSettings = Settings.builder().put(REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true").build(); - FeatureFlags.initializeFeatureFlags(nodeSettings); Metadata metadata = Metadata.builder().persistentSettings(settings).build(); ClusterState currentState = ClusterState.builder(ClusterName.DEFAULT) .nodes( @@ -888,9 +884,6 @@ public void testUpdatesClusterStateWithMultiNodeClusterAndSameRepository() throw } public void testNodeJoinInMixedMode() { - Settings nodeSettings = Settings.builder().put(REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true").build(); - FeatureFlags.initializeFeatureFlags(nodeSettings); - List versions = allOpenSearchVersions(); assert versions.size() >= 2 : "test requires at least two open search versions"; Version baseVersion = versions.get(versions.size() - 2); diff --git a/server/src/test/java/org/opensearch/cluster/routing/allocation/RemoteStoreMigrationAllocationDeciderTests.java b/server/src/test/java/org/opensearch/cluster/routing/allocation/RemoteStoreMigrationAllocationDeciderTests.java index ee4dbe9738e04..5b29922f2400c 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/allocation/RemoteStoreMigrationAllocationDeciderTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/allocation/RemoteStoreMigrationAllocationDeciderTests.java @@ -54,7 +54,6 @@ import org.opensearch.common.UUIDs; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.index.shard.ShardId; import org.opensearch.indices.replication.common.ReplicationType; import org.opensearch.node.remotestore.RemoteStoreNodeService; @@ -68,7 +67,6 @@ import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE; -import static org.opensearch.common.util.FeatureFlags.REMOTE_STORE_MIGRATION_EXPERIMENTAL; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeService.Direction.NONE; import static org.opensearch.node.remotestore.RemoteStoreNodeService.Direction.REMOTE_STORE; @@ -81,8 +79,6 @@ public class RemoteStoreMigrationAllocationDeciderTests extends OpenSearchAlloca private final static String TEST_INDEX = "test_index"; private final static String TEST_REPO = "test_repo"; - private final Settings directionEnabledNodeSettings = Settings.builder().put(REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true").build(); - private final Settings strictModeCompatibilitySettings = Settings.builder() .put(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey(), RemoteStoreNodeService.CompatibilityMode.STRICT) .build(); @@ -111,7 +107,6 @@ public class RemoteStoreMigrationAllocationDeciderTests extends OpenSearchAlloca private ShardId shardId = new ShardId(TEST_INDEX, "_na_", 0); private void beforeAllocation(String direction) { - FeatureFlags.initializeFeatureFlags(directionEnabledNodeSettings); if (isRemoteStoreBackedIndex == null) { isRemoteStoreBackedIndex = randomBoolean(); } @@ -584,9 +579,6 @@ private Settings getCustomSettings(String direction, String compatibilityMode, I // index metadata settings builder.put(indexMetadataBuilder.build().getSettings()); - - builder.put(directionEnabledNodeSettings); - return builder.build(); } diff --git a/server/src/test/java/org/opensearch/cluster/routing/allocation/decider/FilterAllocationDeciderTests.java b/server/src/test/java/org/opensearch/cluster/routing/allocation/decider/FilterAllocationDeciderTests.java index e8273d294f24f..2e303887e0f1b 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/allocation/decider/FilterAllocationDeciderTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/allocation/decider/FilterAllocationDeciderTests.java @@ -51,7 +51,6 @@ import org.opensearch.common.settings.IndexScopedSettings; import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.node.remotestore.RemoteStoreNodeService; import org.opensearch.snapshots.EmptySnapshotsInfoService; import org.opensearch.test.gateway.TestGatewayAllocator; @@ -64,7 +63,6 @@ import static org.opensearch.cluster.routing.ShardRoutingState.INITIALIZING; import static org.opensearch.cluster.routing.ShardRoutingState.STARTED; import static org.opensearch.cluster.routing.ShardRoutingState.UNASSIGNED; -import static org.opensearch.common.util.FeatureFlags.REMOTE_STORE_MIGRATION_EXPERIMENTAL; import static org.opensearch.node.remotestore.RemoteStoreNodeService.MIGRATION_DIRECTION_SETTING; import static org.opensearch.node.remotestore.RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING; @@ -416,7 +414,6 @@ public void testWildcardIPFilter() { public void testMixedModeRemoteStoreAllocation() { // For mixed mode remote store direction cluster's existing indices replica creation , // we don't consider filter allocation decider for replica of existing indices - FeatureFlags.initializeFeatureFlags(Settings.builder().put(REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true").build()); ClusterSettings clusterSettings = new ClusterSettings(Settings.builder().build(), ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); Settings initialSettings = Settings.builder() .put("cluster.routing.allocation.exclude._id", "node2")