From 97dd4ae059e840cb754261c4c0fa13f64ba3da5a Mon Sep 17 00:00:00 2001 From: Gaurav Bafna <85113518+gbbafna@users.noreply.github.com> Date: Mon, 10 Jun 2024 19:41:16 +0530 Subject: [PATCH] Move Remote Store Migration from DocRep to GA (#14100) (#14122) Signed-off-by: Gaurav Bafna --- CHANGELOG.md | 1 + .../remotemigration/MigrationBaseTestCase.java | 6 ------ .../RemoteStoreMigrationTestCase.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 --- 11 files changed, 21 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7591babbb48c7..0eb375b14b6cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [Remote Store] Add support to disable flush based on translog reader count ([#14027](https://github.com/opensearch-project/OpenSearch/pull/14027)) - [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)) +- Move Remote Store Migration from DocRep to GA and modify remote migration settings name ([#14100](https://github.com/opensearch-project/OpenSearch/pull/14100)) - [Remote State] Add async remote state deletion task running on an interval, configurable by a setting ([#13995](https://github.com/opensearch-project/OpenSearch/pull/13995)) - Add remote routing table for remote state publication with experimental feature flag ([#13304](https://github.com/opensearch-project/OpenSearch/pull/13304)) 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/RemoteStoreMigrationTestCase.java b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationTestCase.java index 4b1c91f1d57ca..4e4f6da56d622 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationTestCase.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationTestCase.java @@ -17,6 +17,7 @@ import org.opensearch.common.Priority; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; +import org.opensearch.common.util.FeatureFlags; import org.opensearch.index.query.QueryBuilders; import org.opensearch.repositories.blobstore.BlobStoreRepository; import org.opensearch.snapshots.SnapshotInfo; @@ -42,6 +43,11 @@ protected int minimumNumberOfReplicas() { return 1; } + @Override + protected Settings featureFlagSettings() { + return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true").build(); + } + public void testMixedModeAddRemoteNodes() throws Exception { internalCluster().setBootstrapClusterManagerNodeIndex(0); List cmNodes = internalCluster().startNodes(1); 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 e3c4bc80eb4a7..93850cc046088 100644 --- a/server/src/main/java/org/opensearch/cluster/ClusterModule.java +++ b/server/src/main/java/org/opensearch/cluster/ClusterModule.java @@ -83,7 +83,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; @@ -379,9 +378,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 557e4dc2ca8c5..33431c5c45958 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; @@ -243,11 +242,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 f8f3ce76ae325..02cc87d334cc0 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java @@ -498,8 +498,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()) @@ -517,8 +515,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( @@ -970,9 +966,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")