From 4c23b6b7239cea5532a3deb0c796323238a91a50 Mon Sep 17 00:00:00 2001 From: Lakshya Taragi Date: Mon, 1 Apr 2024 16:13:21 +0530 Subject: [PATCH] Split changes Signed-off-by: Lakshya Taragi --- .../RemoteStoreMigrationAllocationIT.java | 82 ------------------- .../RemoteStoreMigrationSettingsUpdateIT.java | 33 -------- .../TransportClusterUpdateSettingsAction.java | 29 ------- 3 files changed, 144 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationAllocationIT.java b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationAllocationIT.java index 3ab2cf1920549..bbc75a56d1b6f 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationAllocationIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationAllocationIT.java @@ -139,47 +139,6 @@ public void testDontAllocateNewReplicaShardOnRemoteNodeIfPrimaryShardOnNonRemote assertNonAllocation(false); } - public void testAllocateNewReplicaShardOnRemoteNodeIfPrimaryShardOnRemoteNodeForMixedModeAndRemoteStoreDirection() throws Exception { - logger.info(" --> initialize cluster"); - initializeCluster(false); - - logger.info(" --> set mixed cluster compatibility mode"); - setClusterMode(MIXED.mode); - - logger.info(" --> add remote and non-remote nodes"); - addRemote = true; - String remoteNodeName1 = internalCluster().startNode(); - String remoteNodeName2 = internalCluster().startNode(); - addRemote = false; - String nonRemoteNodeName = internalCluster().startNode(); - internalCluster().validateClusterFormed(); - DiscoveryNode remoteNode1 = assertNodeInCluster(remoteNodeName1); - DiscoveryNode remoteNode2 = assertNodeInCluster(remoteNodeName2); - DiscoveryNode nonRemoteNode = assertNodeInCluster(nonRemoteNodeName); - - logger.info(" --> allocate primary shard on remote node"); - prepareIndexWithAllocatedPrimary(remoteNode1, Optional.empty()); - - logger.info(" --> set remote_store direction"); - setDirection(REMOTE_STORE.direction); - - logger.info(" --> verify expected decision for replica shard"); - prepareDecisions(); - Decision decision = getDecisionForTargetNode(remoteNode2, false, true, false); - assertEquals(Decision.Type.YES, decision.type()); - assertEquals( - "[remote_store migration_direction]: replica shard copy can be allocated to a remote node since primary shard copy has been migrated to remote", - decision.getExplanation().toLowerCase(Locale.ROOT) - ); - - logger.info(" --> attempt allocation of replica shard the other remote node"); - attemptAllocation(remoteNodeName2); - ensureGreen(TEST_INDEX); - - logger.info(" --> verify allocation of replica shard"); - assertAllocation(false, remoteNode2); - } - public void testAllocateNewReplicaShardOnNonRemoteNodeIfPrimaryShardOnNonRemoteNodeForMixedModeAndRemoteStoreDirection() throws Exception { logger.info(" --> initialize cluster"); @@ -220,47 +179,6 @@ public void testAllocateNewReplicaShardOnNonRemoteNodeIfPrimaryShardOnNonRemoteN assertAllocation(false, nonRemoteNode2); } - public void testAllocateNewReplicaShardOnNonRemoteNodeIfPrimaryShardOnRemoteNodeForMixedModeAndRemoteStoreDirection() throws Exception { - logger.info(" --> initialize cluster"); - initializeCluster(false); - - logger.info(" --> set mixed cluster compatibility mode"); - - logger.info(" --> add remote and non-remote nodes"); - setClusterMode(MIXED.mode); - addRemote = false; - String nonRemoteNodeName = internalCluster().startNode(); - addRemote = true; - String remoteNodeName = internalCluster().startNode(); - internalCluster().validateClusterFormed(); - DiscoveryNode nonRemoteNode = assertNodeInCluster(nonRemoteNodeName); - DiscoveryNode remoteNode = assertNodeInCluster(remoteNodeName); - - logger.info(" --> allocate primary on remote node"); - prepareIndexWithAllocatedPrimary(remoteNode, Optional.empty()); - - logger.info(" --> set remote_store direction"); - setDirection(REMOTE_STORE.direction); - - logger.info(" --> verify expected decision for replica shard"); - prepareDecisions(); - Decision decision = getDecisionForTargetNode(nonRemoteNode, false, true, false); - - Decision.Type expectedType = Decision.Type.YES; - assertEquals(expectedType, decision.type()); - assertEquals( - "[remote_store migration_direction]: replica shard copy can be allocated to a non-remote node", - decision.getExplanation().toLowerCase(Locale.ROOT) - ); - - logger.info(" --> allocate replica shard on non-remote node"); - attemptAllocation(nonRemoteNodeName); - ensureGreen(TEST_INDEX); - - logger.info(" --> verify allocation of replica shard"); - assertAllocation(false, nonRemoteNode); - } - // test for STRICT mode public void testAlwaysAllocateNewShardForStrictMode() throws Exception { diff --git a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationSettingsUpdateIT.java b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationSettingsUpdateIT.java index c8dacd497dce5..f93080b63c8d9 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationSettingsUpdateIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationSettingsUpdateIT.java @@ -11,11 +11,9 @@ import org.opensearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse; import org.opensearch.client.Client; import org.opensearch.common.settings.Settings; -import org.opensearch.common.settings.SettingsException; import org.opensearch.core.rest.RestStatus; import org.opensearch.index.IndexSettings; import org.opensearch.indices.replication.common.ReplicationType; -import org.opensearch.test.InternalTestCluster; import org.opensearch.test.OpenSearchIntegTestCase; import java.util.Optional; @@ -78,37 +76,6 @@ public void testNewIndexIsRemoteStoreBackedForRemoteStoreDirectionAndMixedMode() assertRemoteStoreBackedIndex(indexName2); } - // compatibility mode setting test - - public void testSwitchToStrictMode() throws Exception { - logger.info(" --> initialize cluster"); - initializeCluster(false); - - logger.info(" --> create a mixed mode cluster"); - setClusterMode(MIXED.mode); - addRemote = true; - String remoteNodeName = internalCluster().startNode(); - addRemote = false; - String nonRemoteNodeName = internalCluster().startNode(); - internalCluster().validateClusterFormed(); - assertNodeInCluster(remoteNodeName); - assertNodeInCluster(nonRemoteNodeName); - - logger.info(" --> attempt switching to strict mode"); - SettingsException exception = assertThrows(SettingsException.class, () -> setClusterMode(STRICT.mode)); - assertEquals( - "can not switch to STRICT compatibility mode when the cluster contains both remote and non-remote nodes", - exception.getMessage() - ); - - logger.info(" --> stop remote node"); - internalCluster().stopRandomNode(InternalTestCluster.nameFilter(remoteNodeName)); - ensureStableCluster(2); - - logger.info(" --> attempt switching to strict mode"); - setClusterMode(STRICT.mode); - } - // verify that the created index is not remote store backed private void assertNonRemoteStoreBackedIndex(String indexName) { Settings indexSettings = client.admin().indices().prepareGetIndex().execute().actionGet().getSettings().get(indexName); diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java b/server/src/main/java/org/opensearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java index 5ce83ff79758d..2f3cc77b05550 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java @@ -53,17 +53,12 @@ import org.opensearch.common.Priority; import org.opensearch.common.inject.Inject; import org.opensearch.common.settings.ClusterSettings; -import org.opensearch.common.settings.SettingsException; import org.opensearch.core.action.ActionListener; import org.opensearch.core.common.io.stream.StreamInput; -import org.opensearch.node.remotestore.RemoteStoreNodeService; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.TransportService; import java.io.IOException; -import java.util.ArrayList; -import java.util.List; -import java.util.Optional; /** * Transport action for updating cluster settings @@ -142,7 +137,6 @@ protected void clusterManagerOperation( final ClusterState state, final ActionListener listener ) { - validateCompatibilityModeSettingRequest(request, state); final SettingsUpdater updater = new SettingsUpdater(clusterSettings); clusterService.submitStateUpdateTask( "cluster_update_settings", @@ -270,27 +264,4 @@ public ClusterState execute(final ClusterState currentState) { ); } - /** - * Verifies that while trying to switch to STRICT compatibility mode, all nodes must be of the - * same type (all remote or all non-remote). If not, it throws SettingsException error - * @param request cluster settings update request, for settings to be updated and new values - * @param clusterState current state of cluster, for information on nodes - */ - private void validateCompatibilityModeSettingRequest(ClusterUpdateSettingsRequest request, ClusterState clusterState) { - if (RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING.exists(request.persistentSettings())) { - String value = request.persistentSettings().get(RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey()); - if (value.equals(RemoteStoreNodeService.CompatibilityMode.STRICT.mode)) { - List discoveryNodeList = new ArrayList<>(clusterState.nodes().getNodes().values()); - Optional remoteNode = discoveryNodeList.stream().filter(DiscoveryNode::isRemoteStoreNode).findFirst(); - Optional nonRemoteNode = discoveryNodeList.stream() - .filter(dn -> dn.isRemoteStoreNode() == false) - .findFirst(); - if (remoteNode.isPresent() && nonRemoteNode.isPresent()) { - throw new SettingsException( - "can not switch to STRICT compatibility mode when the cluster contains both remote and non-remote nodes" - ); - } - } - } - } }