From 9fc1b0fcbf87b1cc8abe47f82fbafe295eb548cc Mon Sep 17 00:00:00 2001 From: Lakshya Taragi Date: Mon, 4 Mar 2024 13:53:04 +0530 Subject: [PATCH] Remove featureflag and JoinTaskExecutor changes Signed-off-by: Lakshya Taragi --- .../RemoteStoreMigrationAllocationIT.java | 6 ++++-- .../org/opensearch/cluster/ClusterModule.java | 4 ---- .../cluster/coordination/JoinTaskExecutor.java | 15 +++------------ .../RemoteStoreMigrationAllocationDecider.java | 9 ++++++--- .../opensearch/cluster/ClusterModuleTests.java | 4 ---- ...emoteStoreMigrationAllocationDeciderTests.java | 12 +++++++++--- 6 files changed, 22 insertions(+), 28 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreMigrationAllocationIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreMigrationAllocationIT.java index 896d6792da73a..833138752940f 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreMigrationAllocationIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreMigrationAllocationIT.java @@ -30,7 +30,10 @@ import org.opensearch.plugins.Plugin; import org.opensearch.test.OpenSearchIntegTestCase; -import static org.opensearch.cluster.metadata.IndexMetadata.*; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED; import static org.opensearch.node.remotestore.RemoteStoreNodeService.MIGRATION_DIRECTION_SETTING; import static org.opensearch.node.remotestore.RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING; import static org.opensearch.remotestore.RemoteStoreBaseIntegTestCase.remoteStoreClusterSettings; @@ -789,7 +792,6 @@ private ClusterRerouteResponse relocateShard (String currentNodeName, String tar // verify that shard does not exist at targetNode private void assertNonAllocation (ShardRouting shardRouting) { - logger.info("*** shardRouting: " + shardRouting); assertFalse(shardRouting.active()); assertNull(shardRouting.currentNodeId()); assertEquals(ShardRoutingState.UNASSIGNED, shardRouting.state()); diff --git a/server/src/main/java/org/opensearch/cluster/ClusterModule.java b/server/src/main/java/org/opensearch/cluster/ClusterModule.java index acd1b7d464d80..ac5f1239d9cff 100644 --- a/server/src/main/java/org/opensearch/cluster/ClusterModule.java +++ b/server/src/main/java/org/opensearch/cluster/ClusterModule.java @@ -68,7 +68,6 @@ import org.opensearch.cluster.routing.allocation.decider.MaxRetryAllocationDecider; import org.opensearch.cluster.routing.allocation.decider.NodeLoadAwareAllocationDecider; import org.opensearch.cluster.routing.allocation.decider.NodeVersionAllocationDecider; -import org.opensearch.cluster.routing.allocation.decider.RemoteStoreMigrationAllocationDecider; import org.opensearch.cluster.routing.allocation.decider.RebalanceOnlyWhenActiveAllocationDecider; import org.opensearch.cluster.routing.allocation.decider.ReplicaAfterPrimaryActiveAllocationDecider; import org.opensearch.cluster.routing.allocation.decider.ResizeAllocationDecider; @@ -375,9 +374,6 @@ 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)); - } clusterPlugins.stream() .flatMap(p -> p.createAllocationDeciders(settings, clusterSettings).stream()) diff --git a/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java b/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java index bc94cc28ed8da..bc365b9872037 100644 --- a/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java +++ b/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java @@ -83,8 +83,6 @@ public class JoinTaskExecutor implements ClusterStateTaskExecutor execute(ClusterState currentState, List joiningNodes) throws Exception { final ClusterTasksResult.Builder results = ClusterTasksResult.builder(); + final DiscoveryNodes currentNodes = currentState.nodes(); boolean nodesChanged = false; ClusterState.Builder newState; @@ -513,12 +507,9 @@ private static void ensureRemoteStoreNodesCompatibility(DiscoveryNode joiningNod assert existingNodes.isEmpty() == false; - CompatibilityMode remoteStoreCompatibilityMode = REMOTE_STORE_COMPATIBILITY_MODE_SETTING.get(getCurrentNodeSettings()); - if (REMOTE_STORE_COMPATIBILITY_MODE_SETTING.exists(metadata.settings())) { - remoteStoreCompatibilityMode = REMOTE_STORE_COMPATIBILITY_MODE_SETTING.get(metadata.settings()); - } - + CompatibilityMode remoteStoreCompatibilityMode = REMOTE_STORE_COMPATIBILITY_MODE_SETTING.get(metadata.settings()); if (STRICT.equals(remoteStoreCompatibilityMode)) { + DiscoveryNode existingNode = existingNodes.get(0); if (joiningNode.isRemoteStoreNode()) { ensureRemoteStoreNodesCompatibility(joiningNode, existingNode); diff --git a/server/src/main/java/org/opensearch/cluster/routing/allocation/decider/RemoteStoreMigrationAllocationDecider.java b/server/src/main/java/org/opensearch/cluster/routing/allocation/decider/RemoteStoreMigrationAllocationDecider.java index c337da8a744fd..fb6472d5169fc 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/allocation/decider/RemoteStoreMigrationAllocationDecider.java +++ b/server/src/main/java/org/opensearch/cluster/routing/allocation/decider/RemoteStoreMigrationAllocationDecider.java @@ -43,10 +43,12 @@ import org.opensearch.node.remotestore.RemoteStoreNodeService.Direction; import org.opensearch.node.remotestore.RemoteStoreNodeService.CompatibilityMode; +import java.util.Locale; + /** - * An allocation decider to oversee shard allocation or relocation to facilitate remote-store migration: + * A new allocation decider for migration of document replication clusters to remote store backed clusters: * - For STRICT compatibility mode, the decision is always YES - * * - For remote_store_enabled indices, relocation or allocation/relocation can only be towards a remote node + * - For remote_store_enabled indices, relocation or allocation/relocation can only be towards a remote node * - For "REMOTE_STORE" migration direction: * - New primary shards can only be allocated to a remote node * - New replica shards can be allocated to a remote node iff the primary has been migrated/allocated to a remote node @@ -176,8 +178,9 @@ private String getDecisionDetails( String reason ) { return String.format( + Locale.ROOT, "[%s migration_direction]: %s shard copy %s be %s to a %s node%s", - migrationDirection, + migrationDirection.direction, (shardRouting.primary() ? "primary" : "replica"), (isYes ? "can" : "can not"), (!shardRouting.assignedToNode() ? "allocated" : "relocated"), diff --git a/server/src/test/java/org/opensearch/cluster/ClusterModuleTests.java b/server/src/test/java/org/opensearch/cluster/ClusterModuleTests.java index b30ebaf183084..a2655ba34ee0e 100644 --- a/server/src/test/java/org/opensearch/cluster/ClusterModuleTests.java +++ b/server/src/test/java/org/opensearch/cluster/ClusterModuleTests.java @@ -51,7 +51,6 @@ import org.opensearch.cluster.routing.allocation.decider.NodeLoadAwareAllocationDecider; import org.opensearch.cluster.routing.allocation.decider.NodeVersionAllocationDecider; import org.opensearch.cluster.routing.allocation.decider.RebalanceOnlyWhenActiveAllocationDecider; -import org.opensearch.cluster.routing.allocation.decider.RemoteStoreMigrationAllocationDecider; import org.opensearch.cluster.routing.allocation.decider.ReplicaAfterPrimaryActiveAllocationDecider; import org.opensearch.cluster.routing.allocation.decider.ResizeAllocationDecider; import org.opensearch.cluster.routing.allocation.decider.RestoreInProgressAllocationDecider; @@ -244,9 +243,6 @@ public void testAllocationDeciderOrder() { NodeLoadAwareAllocationDecider.class, TargetPoolAllocationDecider.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/routing/allocation/RemoteStoreMigrationAllocationDeciderTests.java b/server/src/test/java/org/opensearch/cluster/routing/allocation/RemoteStoreMigrationAllocationDeciderTests.java index a701db6c041ea..17c37275f8374 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 @@ -33,7 +33,9 @@ package org.opensearch.cluster.routing.allocation; import org.opensearch.Version; -import org.opensearch.cluster.*; +import org.opensearch.cluster.OpenSearchAllocationTestCase; +import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.ClusterName; import org.opensearch.cluster.routing.allocation.decider.RemoteStoreMigrationAllocationDecider; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.metadata.Metadata; @@ -48,7 +50,8 @@ import org.opensearch.cluster.routing.ShardRouting; import org.opensearch.cluster.routing.ShardRoutingState; import org.opensearch.cluster.routing.TestShardRouting; -import org.opensearch.cluster.routing.allocation.decider.*; +import org.opensearch.cluster.routing.allocation.decider.Decision; +import org.opensearch.cluster.routing.allocation.decider.AllocationDeciders; import org.opensearch.common.UUIDs; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.FeatureFlags; @@ -61,7 +64,10 @@ import java.util.Locale; import java.util.Map; -import static org.opensearch.cluster.metadata.IndexMetadata.*; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY; +import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED; 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.MIGRATION_DIRECTION_SETTING;