From f262296e1d55a5dd9d3d746fb3fd51ba2a16d034 Mon Sep 17 00:00:00 2001 From: Shubh Sahu Date: Mon, 1 Apr 2024 13:57:42 +0530 Subject: [PATCH] Addressed comments on PR by @gbbafna Signed-off-by: Shubh Sahu --- .../ResizeIndexMigrationTestCase.java | 97 ++++++++++++++++++- .../indices/shrink/TransportResizeAction.java | 30 +++--- .../shrink/TransportResizeActionTests.java | 60 ++++++++---- 3 files changed, 153 insertions(+), 34 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotemigration/ResizeIndexMigrationTestCase.java b/server/src/internalClusterTest/java/org/opensearch/remotemigration/ResizeIndexMigrationTestCase.java index db5979143e4cc..b84d546d5d4f9 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotemigration/ResizeIndexMigrationTestCase.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotemigration/ResizeIndexMigrationTestCase.java @@ -14,6 +14,7 @@ import org.opensearch.client.Client; import org.opensearch.common.settings.Settings; import org.opensearch.indices.replication.common.ReplicationType; +import org.opensearch.node.remotestore.RemoteStoreNodeService; import org.opensearch.test.OpenSearchIntegTestCase; import java.util.List; @@ -27,11 +28,12 @@ public class ResizeIndexMigrationTestCase extends MigrationBaseTestCase { private static final String TEST_INDEX = "test_index"; private final static String REMOTE_STORE_DIRECTION = "remote_store"; + private final static String DOC_REP_DIRECTION = "docrep"; private final static String NONE_DIRECTION = "none"; private final static String STRICT_MODE = "strict"; private final static String MIXED_MODE = "mixed"; - public void testFailResizeIndexWhileMigration() throws Exception { + public void testFailResizeIndexWhileDocRepToRemoteStoreMigration() throws Exception { internalCluster().setBootstrapClusterManagerNodeIndex(0); List cmNodes = internalCluster().startNodes(1); @@ -113,7 +115,98 @@ public void testFailResizeIndexWhileMigration() throws Exception { "index Resizing for type [" + resizeType + "] is not allowed as Cluster mode is [Mixed]" - + " and migration direction is [Remote Store]" + + " and migration direction is [REMOTE_STORE]" + + " and index's SETTING_REMOTE_STORE_ENABLED = " + + "false" + ); + } + + public void testFailResizeIndexWhileRemoteStoreToDocRepMigration() throws Exception { + + addRemote = true; + internalCluster().setBootstrapClusterManagerNodeIndex(0); + List cmNodes = internalCluster().startNodes(1); + Client client = internalCluster().client(cmNodes.get(0)); + ClusterUpdateSettingsRequest updateSettingsRequest = new ClusterUpdateSettingsRequest(); + updateSettingsRequest.persistentSettings(Settings.builder().put(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey(), MIXED_MODE)); + assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet()); + + // Adding a non remote and a remote node + String remoteNodeName = internalCluster().startNode(); + + addRemote = false; + String nonRemoteNodeName = internalCluster().startNode(); + + + logger.info("-->Create index on remote node and SETTING_REMOTE_STORE_ENABLED is true. Resize should not happen"); + Settings.Builder builder = Settings.builder().put(SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT); + client.admin() + .indices() + .prepareCreate(TEST_INDEX) + .setSettings( + builder.put("index.number_of_shards", 10) + .put("index.number_of_replicas", 0) + .put("index.routing.allocation.include._name", remoteNodeName) + .put("index.routing.allocation.exclude._name", nonRemoteNodeName) + ) + .setWaitForActiveShards(ActiveShardCount.ALL) + .execute() + .actionGet(); + + updateSettingsRequest.persistentSettings(Settings.builder().put(MIGRATION_DIRECTION_SETTING.getKey(), DOC_REP_DIRECTION)); + assertAcked(client.admin().cluster().updateSettings(updateSettingsRequest).actionGet()); + + ResizeType resizeType; + int resizeShardsNum; + String cause; + switch (randomIntBetween(0, 2)) { + case 0: + resizeType = ResizeType.SHRINK; + resizeShardsNum = 5; + cause = "shrink_index"; + break; + case 1: + resizeType = ResizeType.SPLIT; + resizeShardsNum = 20; + cause = "split_index"; + break; + default: + resizeType = ResizeType.CLONE; + resizeShardsNum = 10; + cause = "clone_index"; + } + + client.admin() + .indices() + .prepareUpdateSettings(TEST_INDEX) + .setSettings(Settings.builder().put("index.blocks.write", true)) + .execute() + .actionGet(); + + ensureGreen(TEST_INDEX); + + Settings.Builder resizeSettingsBuilder = Settings.builder() + .put("index.number_of_replicas", 0) + .put("index.number_of_shards", resizeShardsNum) + .putNull("index.blocks.write"); + + IllegalStateException ex = expectThrows( + IllegalStateException.class, + () -> client().admin() + .indices() + .prepareResizeIndex(TEST_INDEX, "first_split") + .setResizeType(resizeType) + .setSettings(resizeSettingsBuilder.build()) + .get() + ); + assertEquals( + ex.getMessage(), + "index Resizing for type [" + + resizeType + + "] is not allowed as Cluster mode is [Mixed]" + + " and migration direction is [DOCREP]" + + " and index's SETTING_REMOTE_STORE_ENABLED = " + + "true" ); } } diff --git a/server/src/main/java/org/opensearch/action/admin/indices/shrink/TransportResizeAction.java b/server/src/main/java/org/opensearch/action/admin/indices/shrink/TransportResizeAction.java index 92e64efb53e03..5139e6a5c21b6 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/shrink/TransportResizeAction.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/shrink/TransportResizeAction.java @@ -379,18 +379,24 @@ private static void validateClusterModeSettings( IndexMetadata sourceIndexMetadata, ClusterSettings clusterSettings ) { - boolean isMixed = clusterSettings.get(RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING) - .equals(RemoteStoreNodeService.CompatibilityMode.MIXED); - boolean isRemoteStoreMigrationDirection = clusterSettings.get(RemoteStoreNodeService.MIGRATION_DIRECTION_SETTING) - .equals(RemoteStoreNodeService.Direction.REMOTE_STORE); - boolean isRemoteStoreEnabled = sourceIndexMetadata.getSettings().getAsBoolean(SETTING_REMOTE_STORE_ENABLED, false); - if (isMixed && isRemoteStoreMigrationDirection && !isRemoteStoreEnabled) { - throw new IllegalStateException( - "index Resizing for type [" - + type - + "] is not allowed as Cluster mode is [Mixed]" - + " and migration direction is [Remote Store]" - ); + if (clusterSettings.get(RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING) + .equals(RemoteStoreNodeService.CompatibilityMode.MIXED)) { + boolean isRemoteStoreEnabled = sourceIndexMetadata.getSettings().getAsBoolean(SETTING_REMOTE_STORE_ENABLED, false); + if ((clusterSettings.get(RemoteStoreNodeService.MIGRATION_DIRECTION_SETTING).equals(RemoteStoreNodeService.Direction.REMOTE_STORE) + && isRemoteStoreEnabled == false) + || (clusterSettings.get(RemoteStoreNodeService.MIGRATION_DIRECTION_SETTING).equals(RemoteStoreNodeService.Direction.DOCREP) + && isRemoteStoreEnabled == true)) { + throw new IllegalStateException( + "index Resizing for type [" + + type + + "] is not allowed as Cluster mode is [Mixed]" + + " and migration direction is [" + + clusterSettings.get(RemoteStoreNodeService.MIGRATION_DIRECTION_SETTING) + + "]" + + " and index's SETTING_REMOTE_STORE_ENABLED = " + + isRemoteStoreEnabled + ); + } } } } diff --git a/server/src/test/java/org/opensearch/action/admin/indices/shrink/TransportResizeActionTests.java b/server/src/test/java/org/opensearch/action/admin/indices/shrink/TransportResizeActionTests.java index 20ffdac65636f..fbec551317c47 100644 --- a/server/src/test/java/org/opensearch/action/admin/indices/shrink/TransportResizeActionTests.java +++ b/server/src/test/java/org/opensearch/action/admin/indices/shrink/TransportResizeActionTests.java @@ -664,30 +664,50 @@ public void testResizeFailuresDuringMigration() { .settings(Settings.builder().put("index.number_of_shards", expectedShardsNum).put("index.blocks.read_only", false).build()); final ActiveShardCount activeShardCount = randomBoolean() ? ActiveShardCount.ALL : ActiveShardCount.ONE; resizeRequest.setWaitForActiveShards(activeShardCount); - // startsWith("index Resizing for type [") - if (compatibilityMode == CompatibilityMode.MIXED - && migrationDirection == RemoteStoreNodeService.Direction.REMOTE_STORE - && !isRemoteStoreEnabled) { - ClusterState finalState = clusterState; - IllegalStateException ise = expectThrows( - IllegalStateException.class, - () -> TransportResizeAction.prepareCreateIndexRequest( - new ResizeRequest("target", "source"), - finalState, + + if (compatibilityMode == CompatibilityMode.MIXED) { + if ((migrationDirection == RemoteStoreNodeService.Direction.REMOTE_STORE && isRemoteStoreEnabled == false) + || migrationDirection == RemoteStoreNodeService.Direction.DOCREP && isRemoteStoreEnabled == true ){ + ClusterState finalState = clusterState; + IllegalStateException ise = expectThrows( + IllegalStateException.class, + () -> TransportResizeAction.prepareCreateIndexRequest( + resizeRequest, + finalState, + (i) -> stats, + new StoreStats(between(1, 10000), between(1, 10000)), + clusterSettings, + "source", + "target" + ) + ); + assertEquals( + ise.getMessage(), + "index Resizing for type [" + + resizeType + + "] is not allowed as Cluster mode is [Mixed]" + + " and migration direction is [" + + migrationDirection + + "]" + + " and index's SETTING_REMOTE_STORE_ENABLED = " + + isRemoteStoreEnabled + ); + } else { + CreateIndexClusterStateUpdateRequest request = TransportResizeAction.prepareCreateIndexRequest( + resizeRequest, + clusterState, (i) -> stats, - new StoreStats(between(1, 10000), between(1, 10000)), + new StoreStats(100, between(1, 10000)), clusterSettings, "source", "target" - ) - ); - assertEquals( - ise.getMessage(), - "index Resizing for type [" - + resizeType - + "] is not allowed as Cluster mode is [Mixed]" - + " and migration direction is [Remote Store]" - ); + ); + assertNotNull(request.recoverFrom()); + assertEquals("source", request.recoverFrom().getName()); + assertEquals(String.valueOf(expectedShardsNum), request.settings().get("index.number_of_shards")); + assertEquals(cause, request.cause()); + assertEquals(request.waitForActiveShards(), activeShardCount); + } } else { CreateIndexClusterStateUpdateRequest request = TransportResizeAction.prepareCreateIndexRequest( resizeRequest,