From 8cf64b287d534b1aa1263aae4a2c7f220fdda03d Mon Sep 17 00:00:00 2001 From: Lakshya Taragi Date: Tue, 16 Apr 2024 17:15:33 +0530 Subject: [PATCH 1/8] Handle allocation for NONE direction Signed-off-by: Lakshya Taragi --- .../metadata/MetadataCreateIndexService.java | 2 +- ...RemoteStoreMigrationAllocationDecider.java | 18 +- ...eStoreMigrationAllocationDeciderTests.java | 337 ++++++++---------- 3 files changed, 159 insertions(+), 198 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java index b31985a260361..121f8d935cf48 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java @@ -1664,7 +1664,7 @@ public static void validateRefreshIntervalSettings(Settings requestSettings, Clu * @param clusterSettings cluster setting */ static void validateTranslogDurabilitySettings(Settings requestSettings, ClusterSettings clusterSettings, Settings settings) { - if (isRemoteDataAttributePresent(settings) == false + if ((isRemoteDataAttributePresent(settings) == false && isMigratingToRemoteStore(clusterSettings) == false) || IndexSettings.INDEX_TRANSLOG_DURABILITY_SETTING.exists(requestSettings) == false || clusterSettings.get(IndicesService.CLUSTER_REMOTE_INDEX_RESTRICT_ASYNC_DURABILITY_SETTING) == false) { return; 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 7d40aacb71e25..c3f7a5cd518b8 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 @@ -95,18 +95,24 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, Routing ); } - if (migrationDirection.equals(Direction.REMOTE_STORE) == false) { - // docrep migration direction is currently not supported + IndexMetadata indexMetadata = allocation.metadata().getIndexSafe(shardRouting.index()); + boolean remoteStoreBackedIndex = IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING.get(indexMetadata.getSettings()); + + if (migrationDirection.equals(Direction.NONE)) { + boolean isNoDecision = (remoteStoreBackedIndex && targetNode.isRemoteStoreNode() == false) + || (remoteStoreBackedIndex == false && targetNode.isRemoteStoreNode()); + String reason = String.format(Locale.ROOT, " for %sremote store backed index", remoteStoreBackedIndex ? "" : "non "); return allocation.decision( - Decision.YES, + isNoDecision ? Decision.NO : Decision.YES, NAME, - getDecisionDetails(true, shardRouting, targetNode, " for non remote_store direction") + getDecisionDetails(!isNoDecision, shardRouting, targetNode, reason) ); + } else if (migrationDirection.equals(Direction.DOCREP)) { + // docrep migration direction is currently not supported + return allocation.decision(Decision.YES, NAME, getDecisionDetails(true, shardRouting, targetNode, " for DOCREP direction")); } // check for remote store backed indices - IndexMetadata indexMetadata = allocation.metadata().getIndexSafe(shardRouting.index()); - boolean remoteStoreBackedIndex = IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING.get(indexMetadata.getSettings()); if (remoteStoreBackedIndex && targetNode.isRemoteStoreNode() == false) { // allocations and relocations must be to a remote node String reason = String.format( 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 3e130a42952e4..ee4dbe9738e04 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 @@ -70,6 +70,8 @@ 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; 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.hamcrest.core.Is.is; @@ -89,7 +91,7 @@ public class RemoteStoreMigrationAllocationDeciderTests extends OpenSearchAlloca .build(); private final Settings remoteStoreDirectionSettings = Settings.builder() - .put(MIGRATION_DIRECTION_SETTING.getKey(), RemoteStoreNodeService.Direction.REMOTE_STORE) + .put(MIGRATION_DIRECTION_SETTING.getKey(), REMOTE_STORE) .build(); private final Settings docrepDirectionSettings = Settings.builder() .put(MIGRATION_DIRECTION_SETTING.getKey(), RemoteStoreNodeService.Direction.DOCREP) @@ -106,7 +108,9 @@ public class RemoteStoreMigrationAllocationDeciderTests extends OpenSearchAlloca private Metadata metadata; private RoutingTable routingTable = null; - private void beforeAllocation() { + private ShardId shardId = new ShardId(TEST_INDEX, "_na_", 0); + + private void beforeAllocation(String direction) { FeatureFlags.initializeFeatureFlags(directionEnabledNodeSettings); if (isRemoteStoreBackedIndex == null) { isRemoteStoreBackedIndex = randomBoolean(); @@ -116,11 +120,7 @@ private void beforeAllocation() { String compatibilityMode = isMixedMode ? RemoteStoreNodeService.CompatibilityMode.MIXED.mode : RemoteStoreNodeService.CompatibilityMode.STRICT.mode; - customSettings = getCustomSettings( - RemoteStoreNodeService.Direction.REMOTE_STORE.direction, - compatibilityMode, - indexMetadataBuilder - ); + customSettings = getCustomSettings(direction, compatibilityMode, indexMetadataBuilder); if (routingTable != null) { metadata = Metadata.builder().put(indexMetadataBuilder).build(); @@ -149,6 +149,35 @@ private void beforeAllocation() { routingAllocation.debugDecision(true); } + private void prepareRoutingTable(boolean isReplicaAllocation, String primaryShardNodeId) { + routingTable = RoutingTable.builder() + .add( + IndexRoutingTable.builder(shardId.getIndex()) + .addIndexShard( + new IndexShardRoutingTable.Builder(shardId).addShard( + TestShardRouting.newShardRouting( + shardId.getIndexName(), + shardId.getId(), + (isReplicaAllocation ? primaryShardNodeId : null), + true, + (isReplicaAllocation ? ShardRoutingState.STARTED : ShardRoutingState.UNASSIGNED) + ) + ) + .addShard( + TestShardRouting.newShardRouting( + shardId.getIndexName(), + shardId.getId(), + null, + false, + ShardRoutingState.UNASSIGNED + ) + ) + .build() + ) + ) + .build(); + } + // tests for primary shard copy allocation with MIXED mode and REMOTE_STORE direction public void testDontAllocateNewPrimaryShardOnNonRemoteNodeForMixedModeAndRemoteStoreDirection() { @@ -166,7 +195,7 @@ public void testDontAllocateNewPrimaryShardOnNonRemoteNodeForMixedModeAndRemoteS .localNodeId(remoteNode.getId()) .build(); - beforeAllocation(); + beforeAllocation(REMOTE_STORE.direction); ShardRouting primaryShardRouting = clusterState.getRoutingTable().shardRoutingTable(TEST_INDEX, 0).primaryShard(); RoutingNode nonRemoteRoutingNode = clusterState.getRoutingNodes().node(nonRemoteNode.getId()); @@ -196,7 +225,7 @@ public void testAllocateNewPrimaryShardOnRemoteNodeForMixedModeAndRemoteStoreDir .localNodeId(remoteNode.getId()) .build(); - beforeAllocation(); + beforeAllocation(REMOTE_STORE.direction); ShardRouting primaryShardRouting = clusterState.getRoutingTable().shardRoutingTable(TEST_INDEX, 0).primaryShard(); RoutingNode remoteRoutingNode = clusterState.getRoutingNodes().node(remoteNode.getId()); @@ -216,39 +245,11 @@ public void testDontAllocateNewReplicaShardOnRemoteNodeIfPrimaryShardOnNonRemote replicaCount = 1; isMixedMode = true; - ShardId shardId = new ShardId(TEST_INDEX, "_na_", 0); - DiscoveryNode nonRemoteNode = getNonRemoteNode(); DiscoveryNode remoteNode = getRemoteNode(); - routingTable = RoutingTable.builder() - .add( - IndexRoutingTable.builder(shardId.getIndex()) - .addIndexShard( - new IndexShardRoutingTable.Builder(shardId).addShard( - // primary on non-remote node - TestShardRouting.newShardRouting( - shardId.getIndexName(), - shardId.getId(), - nonRemoteNode.getId(), - true, - ShardRoutingState.STARTED - ) - ) - .addShard( - // new replica's allocation - TestShardRouting.newShardRouting( - shardId.getIndexName(), - shardId.getId(), - null, - false, - ShardRoutingState.UNASSIGNED - ) - ) - .build() - ) - ) - .build(); + // primary on non-remote node, new replica's allocation + prepareRoutingTable(true, nonRemoteNode.getId()); discoveryNodes = DiscoveryNodes.builder() .add(nonRemoteNode) @@ -257,7 +258,7 @@ public void testDontAllocateNewReplicaShardOnRemoteNodeIfPrimaryShardOnNonRemote .localNodeId(remoteNode.getId()) .build(); - beforeAllocation(); + beforeAllocation(REMOTE_STORE.direction); assertEquals(2, clusterState.getRoutingTable().allShards().size()); ShardRouting replicaShardRouting = clusterState.getRoutingTable().shardRoutingTable(TEST_INDEX, 0).replicaShards().get(0); @@ -278,40 +279,12 @@ public void testAllocateNewReplicaShardOnRemoteNodeIfPrimaryShardOnRemoteNodeFor replicaCount = 1; isMixedMode = true; - ShardId shardId = new ShardId(TEST_INDEX, "_na_", 0); - DiscoveryNode remoteNode1 = getRemoteNode(); DiscoveryNode remoteNode2 = getRemoteNode(); DiscoveryNode nonRemoteNode = getNonRemoteNode(); - routingTable = RoutingTable.builder() - .add( - IndexRoutingTable.builder(shardId.getIndex()) - .addIndexShard( - new IndexShardRoutingTable.Builder(shardId).addShard( - // primary on remote node - TestShardRouting.newShardRouting( - shardId.getIndexName(), - shardId.getId(), - remoteNode1.getId(), - true, - ShardRoutingState.STARTED - ) - ) - .addShard( - // new replica's allocation - TestShardRouting.newShardRouting( - shardId.getIndexName(), - shardId.getId(), - null, - false, - ShardRoutingState.UNASSIGNED - ) - ) - .build() - ) - ) - .build(); + // primary on remote node, new replica's allocation + prepareRoutingTable(true, remoteNode1.getId()); discoveryNodes = DiscoveryNodes.builder() .add(remoteNode1) @@ -322,7 +295,7 @@ public void testAllocateNewReplicaShardOnRemoteNodeIfPrimaryShardOnRemoteNodeFor .localNodeId(nonRemoteNode.getId()) .build(); - beforeAllocation(); + beforeAllocation(REMOTE_STORE.direction); assertEquals(2, clusterState.getRoutingTable().allShards().size()); ShardRouting replicaShardRouting = clusterState.getRoutingTable().shardRoutingTable(TEST_INDEX, 0).replicaShards().get(0); @@ -343,40 +316,12 @@ public void testAllocateNewReplicaShardOnNonRemoteNodeIfPrimaryShardOnNonRemoteN replicaCount = 1; isMixedMode = true; - ShardId shardId = new ShardId(TEST_INDEX, "_na_", 0); - DiscoveryNode remoteNode = getRemoteNode(); DiscoveryNode nonRemoteNode1 = getNonRemoteNode(); DiscoveryNode nonRemoteNode2 = getNonRemoteNode(); - routingTable = RoutingTable.builder() - .add( - IndexRoutingTable.builder(shardId.getIndex()) - .addIndexShard( - new IndexShardRoutingTable.Builder(shardId).addShard( - // primary shard on non-remote node - TestShardRouting.newShardRouting( - shardId.getIndexName(), - shardId.getId(), - nonRemoteNode1.getId(), - true, - ShardRoutingState.STARTED - ) - ) - .addShard( - // new replica's allocation - TestShardRouting.newShardRouting( - shardId.getIndexName(), - shardId.getId(), - null, - false, - ShardRoutingState.UNASSIGNED - ) - ) - .build() - ) - ) - .build(); + // primary shard on non-remote node, new replica's allocation + prepareRoutingTable(true, nonRemoteNode1.getId()); discoveryNodes = DiscoveryNodes.builder() .add(remoteNode) @@ -387,7 +332,7 @@ public void testAllocateNewReplicaShardOnNonRemoteNodeIfPrimaryShardOnNonRemoteN .localNodeId(nonRemoteNode2.getId()) .build(); - beforeAllocation(); + beforeAllocation(REMOTE_STORE.direction); assertEquals(2, clusterState.getRoutingTable().allShards().size()); @@ -411,39 +356,11 @@ public void testAllocateNewReplicaShardOnNonRemoteNodeIfPrimaryShardOnRemoteNode replicaCount = 1; isMixedMode = true; - ShardId shardId = new ShardId(TEST_INDEX, "_na_", 0); - DiscoveryNode nonRemoteNode = getNonRemoteNode(); DiscoveryNode remoteNode = getRemoteNode(); - routingTable = RoutingTable.builder() - .add( - IndexRoutingTable.builder(shardId.getIndex()) - .addIndexShard( - new IndexShardRoutingTable.Builder(shardId).addShard( - // primary shard on non-remote node - TestShardRouting.newShardRouting( - shardId.getIndexName(), - shardId.getId(), - remoteNode.getId(), - true, - ShardRoutingState.STARTED - ) - ) - .addShard( - // new replica's allocation - TestShardRouting.newShardRouting( - shardId.getIndexName(), - shardId.getId(), - null, - false, - ShardRoutingState.UNASSIGNED - ) - ) - .build() - ) - ) - .build(); + // primary shard on remote node, new replica's allocation + prepareRoutingTable(true, remoteNode.getId()); discoveryNodes = DiscoveryNodes.builder() .add(nonRemoteNode) @@ -452,7 +369,7 @@ public void testAllocateNewReplicaShardOnNonRemoteNodeIfPrimaryShardOnRemoteNode .localNodeId(remoteNode.getId()) .build(); - beforeAllocation(); + beforeAllocation(REMOTE_STORE.direction); assertEquals(2, clusterState.getRoutingTable().allShards().size()); ShardRouting replicaShardRouting = clusterState.getRoutingTable().shardRoutingTable(TEST_INDEX, 0).replicaShards().get(0); @@ -478,39 +395,12 @@ public void testAlwaysAllocateNewShardForStrictMode() { isMixedMode = false; isRemoteStoreBackedIndex = false; - ShardId shardId = new ShardId(TEST_INDEX, "_na_", 0); - DiscoveryNode nonRemoteNode1 = getNonRemoteNode(); DiscoveryNode nonRemoteNode2 = getNonRemoteNode(); boolean isReplicaAllocation = randomBoolean(); - routingTable = RoutingTable.builder() - .add( - IndexRoutingTable.builder(shardId.getIndex()) - .addIndexShard( - new IndexShardRoutingTable.Builder(shardId).addShard( - TestShardRouting.newShardRouting( - shardId.getIndexName(), - shardId.getId(), - (isReplicaAllocation ? nonRemoteNode1.getId() : null), - true, - (isReplicaAllocation ? ShardRoutingState.STARTED : ShardRoutingState.UNASSIGNED) - ) - ) - .addShard( - TestShardRouting.newShardRouting( - shardId.getIndexName(), - shardId.getId(), - null, - false, - ShardRoutingState.UNASSIGNED - ) - ) - .build() - ) - ) - .build(); + prepareRoutingTable(isReplicaAllocation, nonRemoteNode1.getId()); discoveryNodes = DiscoveryNodes.builder() .add(nonRemoteNode1) @@ -519,7 +409,7 @@ public void testAlwaysAllocateNewShardForStrictMode() { .localNodeId(nonRemoteNode2.getId()) .build(); - beforeAllocation(); + beforeAllocation(REMOTE_STORE.direction); assertEquals(2, clusterState.getRoutingTable().allShards().size()); @@ -543,33 +433,7 @@ public void testAlwaysAllocateNewShardForStrictMode() { DiscoveryNode remoteNode1 = getRemoteNode(); DiscoveryNode remoteNode2 = getRemoteNode(); - routingTable = RoutingTable.builder() - .add( - IndexRoutingTable.builder(shardId.getIndex()) - .addIndexShard( - new IndexShardRoutingTable.Builder(shardId).addShard( - TestShardRouting.newShardRouting( - shardId.getIndexName(), - shardId.getId(), - (isReplicaAllocation ? remoteNode1.getId() : null), - true, - (isReplicaAllocation ? ShardRoutingState.STARTED : ShardRoutingState.UNASSIGNED) - ) - ) - .addShard( - // new replica's allocation - TestShardRouting.newShardRouting( - shardId.getIndexName(), - shardId.getId(), - null, - false, - ShardRoutingState.UNASSIGNED - ) - ) - .build() - ) - ) - .build(); + prepareRoutingTable(isReplicaAllocation, remoteNode1.getId()); discoveryNodes = DiscoveryNodes.builder() .add(remoteNode1) @@ -578,7 +442,7 @@ public void testAlwaysAllocateNewShardForStrictMode() { .localNodeId(remoteNode2.getId()) .build(); - beforeAllocation(); + beforeAllocation(REMOTE_STORE.direction); assertEquals(2, clusterState.getRoutingTable().allShards().size()); @@ -598,6 +462,97 @@ public void testAlwaysAllocateNewShardForStrictMode() { assertThat(decision.getExplanation().toLowerCase(Locale.ROOT), is(reason)); } + // test for NONE direction + public void testAllocationForNoneDirection() { + shardCount = 1; + replicaCount = 1; + isMixedMode = true; + isRemoteStoreBackedIndex = false; // non-remote store backed index + + DiscoveryNode remoteNode1 = getRemoteNode(); + DiscoveryNode remoteNode2 = getRemoteNode(); + DiscoveryNode nonRemoteNode1 = getNonRemoteNode(); + DiscoveryNode nonRemoteNode2 = getNonRemoteNode(); + + boolean isReplicaAllocation = randomBoolean(); + + prepareRoutingTable(isReplicaAllocation, nonRemoteNode1.getId()); + + discoveryNodes = DiscoveryNodes.builder() + .add(remoteNode1) + .localNodeId(remoteNode1.getId()) + .add(remoteNode2) + .localNodeId(remoteNode2.getId()) + .add(nonRemoteNode1) + .localNodeId(nonRemoteNode1.getId()) + .add(nonRemoteNode2) + .localNodeId(nonRemoteNode2.getId()) + .build(); + + beforeAllocation(NONE.direction); + assertEquals(2, clusterState.getRoutingTable().allShards().size()); + + ShardRouting shardRouting = clusterState.getRoutingTable().shardRoutingTable(TEST_INDEX, 0).primaryShard(); + if (isReplicaAllocation) { + shardRouting = clusterState.getRoutingTable().shardRoutingTable(TEST_INDEX, 0).replicaShards().get(0); + } + RoutingNode nonRemoteRoutingNode = clusterState.getRoutingNodes().node(nonRemoteNode2.getId()); + RoutingNode remoteRoutingNode = clusterState.getRoutingNodes().node(remoteNode2.getId()); + + // allocation decision for non-remote node for non-remote store backed index + Decision decision = remoteStoreMigrationAllocationDecider.canAllocate(shardRouting, nonRemoteRoutingNode, routingAllocation); + assertThat(decision.type(), is(Decision.Type.YES)); + String reason = String.format( + Locale.ROOT, + "[none migration_direction]: %s shard copy can be allocated to a non-remote node for non remote store backed index", + (isReplicaAllocation ? "replica" : "primary") + ); + assertThat(decision.getExplanation().toLowerCase(Locale.ROOT), is(reason)); + + // allocation decision for remote node for non-remote store backed index + decision = remoteStoreMigrationAllocationDecider.canAllocate(shardRouting, remoteRoutingNode, routingAllocation); + assertThat(decision.type(), is(Decision.Type.NO)); + reason = String.format( + Locale.ROOT, + "[none migration_direction]: %s shard copy can not be allocated to a remote node for non remote store backed index", + (isReplicaAllocation ? "replica" : "primary") + ); + assertThat(decision.getExplanation().toLowerCase(Locale.ROOT), is(reason)); + + isRemoteStoreBackedIndex = true; // remote store backed index + prepareRoutingTable(isReplicaAllocation, remoteNode1.getId()); + + beforeAllocation(NONE.direction); + assertEquals(2, clusterState.getRoutingTable().allShards().size()); + + shardRouting = clusterState.getRoutingTable().shardRoutingTable(TEST_INDEX, 0).primaryShard(); + if (isReplicaAllocation) { + shardRouting = clusterState.getRoutingTable().shardRoutingTable(TEST_INDEX, 0).replicaShards().get(0); + } + nonRemoteRoutingNode = clusterState.getRoutingNodes().node(nonRemoteNode2.getId()); + remoteRoutingNode = clusterState.getRoutingNodes().node(remoteNode2.getId()); + + // allocation decision for non-remote node for remote store backed index + decision = remoteStoreMigrationAllocationDecider.canAllocate(shardRouting, nonRemoteRoutingNode, routingAllocation); + assertThat(decision.type(), is(Decision.Type.NO)); + reason = String.format( + Locale.ROOT, + "[none migration_direction]: %s shard copy can not be allocated to a non-remote node for remote store backed index", + (isReplicaAllocation ? "replica" : "primary") + ); + assertThat(decision.getExplanation().toLowerCase(Locale.ROOT), is(reason)); + + // allocation decision for remote node for remote store backed index + decision = remoteStoreMigrationAllocationDecider.canAllocate(shardRouting, remoteRoutingNode, routingAllocation); + assertThat(decision.type(), is(Decision.Type.YES)); + reason = String.format( + Locale.ROOT, + "[none migration_direction]: %s shard copy can be allocated to a remote node for remote store backed index", + (isReplicaAllocation ? "replica" : "primary") + ); + assertThat(decision.getExplanation().toLowerCase(Locale.ROOT), is(reason)); + } + // prepare index metadata for test-index private IndexMetadata.Builder getIndexMetadataBuilder(boolean isRemoteStoreBackedIndex, int shardCount, int replicaCount) { Settings.Builder builder = settings(Version.CURRENT); @@ -614,7 +569,7 @@ private IndexMetadata.Builder getIndexMetadataBuilder(boolean isRemoteStoreBacke private Settings getCustomSettings(String direction, String compatibilityMode, IndexMetadata.Builder indexMetadataBuilder) { Settings.Builder builder = Settings.builder(); // direction settings - if (direction.toLowerCase(Locale.ROOT).equals(RemoteStoreNodeService.Direction.REMOTE_STORE.direction)) { + if (direction.toLowerCase(Locale.ROOT).equals(REMOTE_STORE.direction)) { builder.put(remoteStoreDirectionSettings); } else if (direction.toLowerCase(Locale.ROOT).equals(RemoteStoreNodeService.Direction.DOCREP.direction)) { builder.put(docrepDirectionSettings); From c80c5d1f4707514f10126ad32f8ceb153978b2c9 Mon Sep 17 00:00:00 2001 From: Lakshya Taragi Date: Mon, 22 Apr 2024 17:03:53 +0530 Subject: [PATCH 2/8] Add IT Signed-off-by: Lakshya Taragi --- ...eMigrationShardAllocationBaseTestCase.java | 39 ++++++- ...RemoteStoreMigrationShardAllocationIT.java | 106 ++++++++++++++++++ 2 files changed, 143 insertions(+), 2 deletions(-) create mode 100644 server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationIT.java diff --git a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationBaseTestCase.java b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationBaseTestCase.java index ffcab9483485d..56521be8f47f5 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationBaseTestCase.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationBaseTestCase.java @@ -8,6 +8,7 @@ package org.opensearch.remotemigration; +import org.opensearch.action.admin.cluster.allocation.ClusterAllocationExplanation; import org.opensearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; import org.opensearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse; import org.opensearch.cluster.metadata.IndexMetadata; @@ -15,6 +16,12 @@ import org.opensearch.cluster.node.DiscoveryNodes; import org.opensearch.cluster.routing.IndexShardRoutingTable; import org.opensearch.cluster.routing.ShardRouting; +import org.opensearch.cluster.routing.ShardRoutingState; +import org.opensearch.cluster.routing.allocation.AllocateUnassignedDecision; +import org.opensearch.cluster.routing.allocation.MoveDecision; +import org.opensearch.cluster.routing.allocation.NodeAllocationResult; +import org.opensearch.cluster.routing.allocation.decider.Decision; +import org.opensearch.common.Nullable; import org.opensearch.common.settings.Settings; import org.opensearch.core.rest.RestStatus; import org.opensearch.index.IndexSettings; @@ -22,6 +29,7 @@ import org.opensearch.snapshots.SnapshotInfo; import org.opensearch.snapshots.SnapshotState; +import java.util.List; import java.util.Map; import java.util.Optional; @@ -47,7 +55,7 @@ protected void setClusterMode(String mode) { } // set the migration direction for cluster [remote_store, docrep, none] - public void setDirection(String direction) { + protected void setDirection(String direction) { updateSettingsRequest.persistentSettings(Settings.builder().put(MIGRATION_DIRECTION_SETTING.getKey(), direction)); assertAcked(internalCluster().client().admin().cluster().updateSettings(updateSettingsRequest).actionGet()); } @@ -79,7 +87,7 @@ protected String allNodesExcept(String except) { return exclude.toString(); } - // create a new test index + // create a new test index with un-allocated primary and no replicas protected void prepareIndexWithoutReplica(Optional name) { String indexName = name.orElse(TEST_INDEX); internalCluster().client() @@ -96,6 +104,33 @@ protected void prepareIndexWithoutReplica(Optional name) { .actionGet(); } + // create a new test index with allocated primary and 1 unallocated replica + public void prepareIndexWithAllocatedPrimary(DiscoveryNode primaryShardNode, Optional name) { + String indexName = name.orElse(TEST_INDEX); + internalCluster().client() + .admin() + .indices() + .prepareCreate(indexName) + .setSettings( + Settings.builder() + .put("index.number_of_shards", 1) + .put("index.number_of_replicas", 1) + .put("index.routing.allocation.include._name", primaryShardNode.getName()) + .put("index.routing.allocation.exclude._name", allNodesExcept(primaryShardNode.getName())) + ) + .setWaitForActiveShards(ActiveShardCount.ONE) + .execute() + .actionGet(); + + ensureYellowAndNoInitializingShards(TEST_INDEX); + + logger.info(" --> verify allocation of primary shard"); + assertAllocation(true, primaryShardNode); + + logger.info(" --> verify non-allocation of replica shard"); + assertNonAllocation(false); + } + protected ShardRouting getShardRouting(boolean isPrimary) { IndexShardRoutingTable table = internalCluster().client() .admin() diff --git a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationIT.java b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationIT.java new file mode 100644 index 0000000000000..e50e5b343c266 --- /dev/null +++ b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationIT.java @@ -0,0 +1,106 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.remotemigration; + +import org.opensearch.client.Client; +import org.opensearch.cluster.node.DiscoveryNode; +import org.opensearch.cluster.routing.ShardRouting; +import org.opensearch.cluster.routing.allocation.decider.Decision; +import org.opensearch.test.OpenSearchIntegTestCase; + +import java.util.Locale; +import java.util.Optional; + +import static org.opensearch.node.remotestore.RemoteStoreNodeService.CompatibilityMode.MIXED; + +@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0) +public class RemoteStoreMigrationShardAllocationIT extends RemoteStoreMigrationShardAllocationBaseTestCase { + + public static final String NAME = "remote_store_migration"; + + private Client client; + + // test for shard allocation decisions for MIXED mode and NONE direction + public void testAllocationForRemoteStoreBackedIndexForNoneDirectionAndMixedMode() throws Exception { + logger.info("Initialize cluster"); + initializeCluster(true); + + logger.info("Add data nodes"); + String remoteNodeName1 = internalCluster().startDataOnlyNode(); + String remoteNodeName2 = internalCluster().startDataOnlyNode(); + internalCluster().validateClusterFormed(); + DiscoveryNode remoteNode1 = assertNodeInCluster(remoteNodeName1); + DiscoveryNode remoteNode2 = assertNodeInCluster(remoteNodeName2); + + logger.info("Prepare test index"); + boolean isReplicaAllocation = randomBoolean(); + if (isReplicaAllocation) { + prepareIndexWithAllocatedPrimary(remoteNode1, Optional.empty()); + } else { + prepareIndexWithoutReplica(Optional.empty()); + } + assertRemoteStoreBackedIndex(TEST_INDEX); + + logger.info("Switch to MIXED cluster compatibility mode"); + setClusterMode(MIXED.mode); + addRemote = false; + String docrepNodeName = internalCluster().startDataOnlyNode(); + internalCluster().validateClusterFormed(); + DiscoveryNode docrepNode = assertNodeInCluster(docrepNodeName); + + logger.info("Verify decision for allocation on docrep node"); + prepareDecisions(); + Decision decision = getDecisionForTargetNode(docrepNode, !isReplicaAllocation, false, false); + assertEquals(Decision.Type.NO, decision.type()); + String expectedReason = String.format( + Locale.ROOT, + "[none migration_direction]: %s shard copy can not be allocated to a non-remote node for remote store backed index", + (isReplicaAllocation ? "replica" : "primary") + ); + assertEquals(expectedReason, decision.getExplanation().toLowerCase(Locale.ROOT)); + + logger.info("Attempt allocation of shard on non-remote node"); + attemptAllocation(docrepNodeName); + + logger.info("Verify non-allocation of shard"); + assertNonAllocation(!isReplicaAllocation); + + logger.info("Verify decision for allocation on remote node"); + decision = getDecisionForTargetNode(remoteNode2, !isReplicaAllocation, true, false); + assertEquals(Decision.Type.YES, decision.type()); + expectedReason = String.format( + Locale.ROOT, + "[none migration_direction]: %s shard copy can be allocated to a remote node for remote store backed index", + (isReplicaAllocation ? "replica" : "primary") + ); + assertEquals(expectedReason, decision.getExplanation().toLowerCase(Locale.ROOT)); + + logger.info("Attempt free allocation of shard on remote node"); + attemptAllocation(null); + + logger.info("Verify successful allocation of shard"); + if (!isReplicaAllocation) { + ensureGreen(TEST_INDEX); + } else { + ensureYellowAndNoInitializingShards(TEST_INDEX); + } + assertAllocation(!isReplicaAllocation, null); + logger.info("Verify allocation on one of the remote nodes"); + ShardRouting shardRouting = getShardRouting(!isReplicaAllocation); + assertTrue(shardRouting.currentNodeId().equals(remoteNode1.getId()) || shardRouting.currentNodeId().equals(remoteNode2.getId())); + } + + // bootstrap a cluster + private void initializeCluster(boolean remoteClusterManager) { + addRemote = remoteClusterManager; + internalCluster().startClusterManagerOnlyNode(); + client = internalCluster().client(); + } + +} From ca8dfc61e5fc34ebad1d840ca624311b4ed7b4c0 Mon Sep 17 00:00:00 2001 From: Lakshya Taragi Date: Mon, 22 Apr 2024 17:20:00 +0530 Subject: [PATCH 3/8] Make IT generic Signed-off-by: Lakshya Taragi --- ...RemoteStoreMigrationShardAllocationIT.java | 71 ++++++++++++------- 1 file changed, 46 insertions(+), 25 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationIT.java b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationIT.java index e50e5b343c266..1e82404116dfe 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationIT.java @@ -27,61 +27,80 @@ public class RemoteStoreMigrationShardAllocationIT extends RemoteStoreMigrationS private Client client; // test for shard allocation decisions for MIXED mode and NONE direction - public void testAllocationForRemoteStoreBackedIndexForNoneDirectionAndMixedMode() throws Exception { + public void testAllocationForNoneDirectionAndMixedMode() throws Exception { + boolean isRemoteStoreBackedIndex = randomBoolean(); + boolean isReplicaAllocation = randomBoolean(); + logger.info( + String.format( + Locale.ROOT, + "Test for allocation decisions for %s shard of a %s store backed index under NONE direction", + (isReplicaAllocation ? "replica" : "primary"), + (isRemoteStoreBackedIndex ? "remote" : "non remote") + ) + ); + logger.info("Initialize cluster"); - initializeCluster(true); + initializeCluster(isRemoteStoreBackedIndex); logger.info("Add data nodes"); - String remoteNodeName1 = internalCluster().startDataOnlyNode(); - String remoteNodeName2 = internalCluster().startDataOnlyNode(); + String previousNodeName1 = internalCluster().startDataOnlyNode(); + String previousNodeName2 = internalCluster().startDataOnlyNode(); internalCluster().validateClusterFormed(); - DiscoveryNode remoteNode1 = assertNodeInCluster(remoteNodeName1); - DiscoveryNode remoteNode2 = assertNodeInCluster(remoteNodeName2); + DiscoveryNode previousNode1 = assertNodeInCluster(previousNodeName1); + DiscoveryNode previousNode2 = assertNodeInCluster(previousNodeName2); logger.info("Prepare test index"); - boolean isReplicaAllocation = randomBoolean(); if (isReplicaAllocation) { - prepareIndexWithAllocatedPrimary(remoteNode1, Optional.empty()); + prepareIndexWithAllocatedPrimary(previousNode1, Optional.empty()); } else { prepareIndexWithoutReplica(Optional.empty()); } - assertRemoteStoreBackedIndex(TEST_INDEX); + + if (isRemoteStoreBackedIndex) { + assertRemoteStoreBackedIndex(TEST_INDEX); + } else { + assertNonRemoteStoreBackedIndex(TEST_INDEX); + } logger.info("Switch to MIXED cluster compatibility mode"); setClusterMode(MIXED.mode); - addRemote = false; - String docrepNodeName = internalCluster().startDataOnlyNode(); + addRemote = !addRemote; + String newNodeName = internalCluster().startDataOnlyNode(); internalCluster().validateClusterFormed(); - DiscoveryNode docrepNode = assertNodeInCluster(docrepNodeName); + DiscoveryNode newNode = assertNodeInCluster(newNodeName); - logger.info("Verify decision for allocation on docrep node"); + logger.info("Verify decision for allocation on the new node"); prepareDecisions(); - Decision decision = getDecisionForTargetNode(docrepNode, !isReplicaAllocation, false, false); + Decision decision = getDecisionForTargetNode(newNode, !isReplicaAllocation, false, false); assertEquals(Decision.Type.NO, decision.type()); String expectedReason = String.format( Locale.ROOT, - "[none migration_direction]: %s shard copy can not be allocated to a non-remote node for remote store backed index", - (isReplicaAllocation ? "replica" : "primary") + "[none migration_direction]: %s shard copy can not be allocated to a %s node for %s store backed index", + (isReplicaAllocation ? "replica" : "primary"), + (isRemoteStoreBackedIndex ? "non-remote" : "remote"), + (isRemoteStoreBackedIndex ? "remote" : "non remote") ); assertEquals(expectedReason, decision.getExplanation().toLowerCase(Locale.ROOT)); - logger.info("Attempt allocation of shard on non-remote node"); - attemptAllocation(docrepNodeName); + logger.info("Attempt allocation of shard on new node"); + attemptAllocation(newNodeName); logger.info("Verify non-allocation of shard"); assertNonAllocation(!isReplicaAllocation); - logger.info("Verify decision for allocation on remote node"); - decision = getDecisionForTargetNode(remoteNode2, !isReplicaAllocation, true, false); + logger.info("Verify decision for allocation on previous node"); + decision = getDecisionForTargetNode(previousNode2, !isReplicaAllocation, true, false); assertEquals(Decision.Type.YES, decision.type()); expectedReason = String.format( Locale.ROOT, - "[none migration_direction]: %s shard copy can be allocated to a remote node for remote store backed index", - (isReplicaAllocation ? "replica" : "primary") + "[none migration_direction]: %s shard copy can be allocated to a %s node for %s store backed index", + (isReplicaAllocation ? "replica" : "primary"), + (isRemoteStoreBackedIndex ? "remote" : "non-remote"), + (isRemoteStoreBackedIndex ? "remote" : "non remote") ); assertEquals(expectedReason, decision.getExplanation().toLowerCase(Locale.ROOT)); - logger.info("Attempt free allocation of shard on remote node"); + logger.info("Attempt free allocation of shard"); attemptAllocation(null); logger.info("Verify successful allocation of shard"); @@ -91,9 +110,11 @@ public void testAllocationForRemoteStoreBackedIndexForNoneDirectionAndMixedMode( ensureYellowAndNoInitializingShards(TEST_INDEX); } assertAllocation(!isReplicaAllocation, null); - logger.info("Verify allocation on one of the remote nodes"); + logger.info("Verify allocation on one of the previous nodes"); ShardRouting shardRouting = getShardRouting(!isReplicaAllocation); - assertTrue(shardRouting.currentNodeId().equals(remoteNode1.getId()) || shardRouting.currentNodeId().equals(remoteNode2.getId())); + assertTrue( + shardRouting.currentNodeId().equals(previousNode1.getId()) || shardRouting.currentNodeId().equals(previousNode2.getId()) + ); } // bootstrap a cluster From cf0a5f1bc78f9a5b8ae0e182bd6897e51e81672c Mon Sep 17 00:00:00 2001 From: Lakshya Taragi Date: Tue, 23 Apr 2024 12:57:40 +0530 Subject: [PATCH 4/8] Fix Failing ITs Signed-off-by: Lakshya Taragi --- .../remotemigration/RemotePrimaryRelocationIT.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemotePrimaryRelocationIT.java b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemotePrimaryRelocationIT.java index 8f6c1e2d9a68c..293691ace2edd 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemotePrimaryRelocationIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemotePrimaryRelocationIT.java @@ -91,6 +91,10 @@ public void testRemotePrimaryRelocation() throws Exception { int finalCurrentDoc1 = currentDoc; waitUntil(() -> numAutoGenDocs.get() > finalCurrentDoc1 + 5); + // Change direction to remote store + updateSettingsRequest.persistentSettings(Settings.builder().put(MIGRATION_DIRECTION_SETTING.getKey(), "remote_store")); + assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet()); + logger.info("--> relocating from {} to {} ", docRepNodes, remoteNode); client().admin() .cluster() @@ -179,6 +183,10 @@ public void testMixedModeRelocation_RemoteSeedingFail() throws Exception { .setTransientSettings(Settings.builder().put(RecoverySettings.INDICES_INTERNAL_REMOTE_UPLOAD_TIMEOUT.getKey(), "10s")) .get(); + // Change direction to remote store + updateSettingsRequest.persistentSettings(Settings.builder().put(MIGRATION_DIRECTION_SETTING.getKey(), "remote_store")); + assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet()); + logger.info("--> relocating from {} to {} ", docRepNode, remoteNode); client().admin().cluster().prepareReroute().add(new MoveAllocationCommand("test", 0, docRepNode, remoteNode)).execute().actionGet(); ClusterHealthResponse clusterHealthResponse = client().admin() From 3f91c3969cf7c26e5469afd58e111ca7720c8224 Mon Sep 17 00:00:00 2001 From: Lakshya Taragi Date: Wed, 24 Apr 2024 16:56:15 +0530 Subject: [PATCH 5/8] Address comments Signed-off-by: Lakshya Taragi --- ...RemoteStoreMigrationAllocationDecider.java | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) 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 c3f7a5cd518b8..4fc5fff805663 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 @@ -96,12 +96,12 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, Routing } IndexMetadata indexMetadata = allocation.metadata().getIndexSafe(shardRouting.index()); - boolean remoteStoreBackedIndex = IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING.get(indexMetadata.getSettings()); + boolean remoteSettingsBackedIndex = IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING.get(indexMetadata.getSettings()); if (migrationDirection.equals(Direction.NONE)) { - boolean isNoDecision = (remoteStoreBackedIndex && targetNode.isRemoteStoreNode() == false) - || (remoteStoreBackedIndex == false && targetNode.isRemoteStoreNode()); - String reason = String.format(Locale.ROOT, " for %sremote store backed index", remoteStoreBackedIndex ? "" : "non "); + // remote backed indices on docrep nodes and non remote backed indices on remote nodes are not allowed + boolean isNoDecision = remoteSettingsBackedIndex ^ targetNode.isRemoteStoreNode(); + String reason = String.format(Locale.ROOT, " for %sremote store backed index", remoteSettingsBackedIndex ? "" : "non "); return allocation.decision( isNoDecision ? Decision.NO : Decision.YES, NAME, @@ -110,23 +110,23 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, Routing } else if (migrationDirection.equals(Direction.DOCREP)) { // docrep migration direction is currently not supported return allocation.decision(Decision.YES, NAME, getDecisionDetails(true, shardRouting, targetNode, " for DOCREP direction")); - } - - // check for remote store backed indices - if (remoteStoreBackedIndex && targetNode.isRemoteStoreNode() == false) { - // allocations and relocations must be to a remote node - String reason = String.format( - Locale.ROOT, - " because a remote store backed index's shard copy can only be %s to a remote node", - ((shardRouting.assignedToNode() == false) ? "allocated" : "relocated") - ); - return allocation.decision(Decision.NO, NAME, getDecisionDetails(false, shardRouting, targetNode, reason)); - } + } else { + // check for remote store backed indices + if (remoteSettingsBackedIndex && targetNode.isRemoteStoreNode() == false) { + // allocations and relocations must be to a remote node + String reason = String.format( + Locale.ROOT, + " because a remote store backed index's shard copy can only be %s to a remote node", + ((shardRouting.assignedToNode() == false) ? "allocated" : "relocated") + ); + return allocation.decision(Decision.NO, NAME, getDecisionDetails(false, shardRouting, targetNode, reason)); + } - if (shardRouting.primary()) { - return primaryShardDecision(shardRouting, targetNode, allocation); + if (shardRouting.primary()) { + return primaryShardDecision(shardRouting, targetNode, allocation); + } + return replicaShardDecision(shardRouting, targetNode, allocation); } - return replicaShardDecision(shardRouting, targetNode, allocation); } // handle scenarios for allocation of a new shard's primary copy From d1df1a7018461ab829769af0ee04ef1167f18aaf Mon Sep 17 00:00:00 2001 From: Lakshya Taragi Date: Thu, 25 Apr 2024 17:34:17 +0530 Subject: [PATCH 6/8] Rebase correction Signed-off-by: Lakshya Taragi --- ...eMigrationShardAllocationBaseTestCase.java | 104 ++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationBaseTestCase.java b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationBaseTestCase.java index 56521be8f47f5..fdd0d3ca1190f 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationBaseTestCase.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationBaseTestCase.java @@ -11,6 +11,7 @@ import org.opensearch.action.admin.cluster.allocation.ClusterAllocationExplanation; import org.opensearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; import org.opensearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse; +import org.opensearch.action.support.ActiveShardCount; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.node.DiscoveryNodes; @@ -145,6 +146,108 @@ protected ShardRouting getShardRouting(boolean isPrimary) { return (isPrimary ? table.primaryShard() : table.replicaShards().get(0)); } + // obtain decision for allocation/relocation of a shard to a given node + protected Decision getDecisionForTargetNode( + DiscoveryNode targetNode, + boolean isPrimary, + boolean includeYesDecisions, + boolean isRelocation + ) { + ClusterAllocationExplanation explanation = internalCluster().client() + .admin() + .cluster() + .prepareAllocationExplain() + .setIndex(TEST_INDEX) + .setShard(0) + .setPrimary(isPrimary) + .setIncludeYesDecisions(includeYesDecisions) + .get() + .getExplanation(); + + Decision requiredDecision = null; + List nodeAllocationResults; + if (isRelocation) { + MoveDecision moveDecision = explanation.getShardAllocationDecision().getMoveDecision(); + nodeAllocationResults = moveDecision.getNodeDecisions(); + } else { + AllocateUnassignedDecision allocateUnassignedDecision = explanation.getShardAllocationDecision().getAllocateDecision(); + nodeAllocationResults = allocateUnassignedDecision.getNodeDecisions(); + } + + for (NodeAllocationResult nodeAllocationResult : nodeAllocationResults) { + if (nodeAllocationResult.getNode().equals(targetNode)) { + for (Decision decision : nodeAllocationResult.getCanAllocateDecision().getDecisions()) { + if (decision.label().equals(NAME)) { + requiredDecision = decision; + break; + } + } + } + } + + assertNotNull(requiredDecision); + return requiredDecision; + } + + // get allocation and relocation decisions for all nodes + protected void prepareDecisions() { + internalCluster().client() + .admin() + .indices() + .prepareUpdateSettings(TEST_INDEX) + .setSettings(Settings.builder().put("index.routing.allocation.exclude._name", allNodesExcept(null))) + .execute() + .actionGet(); + } + + protected void attemptAllocation(@Nullable String targetNodeName) { + Settings.Builder settingsBuilder; + if (targetNodeName != null) { + settingsBuilder = Settings.builder() + .put("index.routing.allocation.include._name", targetNodeName) + .put("index.routing.allocation.exclude._name", allNodesExcept(targetNodeName)); + } else { + String clusterManagerNodeName = internalCluster().client() + .admin() + .cluster() + .prepareState() + .execute() + .actionGet() + .getState() + .getNodes() + .getClusterManagerNode() + .getName(); + // to allocate freely among all nodes other than cluster-manager node + settingsBuilder = Settings.builder() + .put("index.routing.allocation.include._name", allNodesExcept(clusterManagerNodeName)) + .put("index.routing.allocation.exclude._name", clusterManagerNodeName); + } + internalCluster().client().admin().indices().prepareUpdateSettings(TEST_INDEX).setSettings(settingsBuilder).execute().actionGet(); + } + + // verify that shard does not exist at targetNode + protected void assertNonAllocation(boolean isPrimary) { + if (isPrimary) { + ensureRed(TEST_INDEX); + } else { + ensureYellowAndNoInitializingShards(TEST_INDEX); + } + ShardRouting shardRouting = getShardRouting(isPrimary); + assertFalse(shardRouting.active()); + assertNull(shardRouting.currentNodeId()); + assertEquals(ShardRoutingState.UNASSIGNED, shardRouting.state()); + } + + // verify that shard exists at targetNode + protected void assertAllocation(boolean isPrimary, @Nullable DiscoveryNode targetNode) { + ShardRouting shardRouting = getShardRouting(isPrimary); + assertTrue(shardRouting.active()); + assertNotNull(shardRouting.currentNodeId()); + if (targetNode != null) { + assertEquals(shardRouting.currentNodeId(), targetNode.getId()); + } + } + // create a snapshot public static SnapshotInfo createSnapshot(String snapshotRepoName, String snapshotName, String... indices) { SnapshotInfo snapshotInfo = internalCluster().client() @@ -229,4 +332,5 @@ public static void assertRemoteStoreBackedIndex(String indexName) { INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING.get(indexSettings) ); } + } From bd1f7176b88b8b65240d87504d52f077be9ba486 Mon Sep 17 00:00:00 2001 From: Lakshya Taragi Date: Tue, 30 Apr 2024 18:22:30 +0530 Subject: [PATCH 7/8] Add remote store migration allocation ITs Signed-off-by: Lakshya Taragi --- .../RemoteMigrationAllocationDeciderIT.java | 418 +++++++++++++++++- ...eMigrationShardAllocationBaseTestCase.java | 40 +- ...RemoteStoreMigrationShardAllocationIT.java | 127 ------ 3 files changed, 448 insertions(+), 137 deletions(-) delete mode 100644 server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationIT.java diff --git a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteMigrationAllocationDeciderIT.java b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteMigrationAllocationDeciderIT.java index de425ffc63816..9c3aec58b6eed 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteMigrationAllocationDeciderIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteMigrationAllocationDeciderIT.java @@ -11,8 +11,11 @@ import org.opensearch.action.admin.cluster.health.ClusterHealthResponse; import org.opensearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.cluster.node.DiscoveryNode; +import org.opensearch.cluster.routing.ShardRouting; import org.opensearch.cluster.routing.UnassignedInfo; import org.opensearch.cluster.routing.allocation.command.MoveAllocationCommand; +import org.opensearch.cluster.routing.allocation.decider.Decision; import org.opensearch.common.Priority; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; @@ -21,13 +24,17 @@ import java.io.IOException; import java.util.List; +import java.util.Locale; +import java.util.Optional; +import static org.opensearch.node.remotestore.RemoteStoreNodeService.CompatibilityMode.MIXED; +import static org.opensearch.node.remotestore.RemoteStoreNodeService.Direction.REMOTE_STORE; 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.test.hamcrest.OpenSearchAssertions.assertAcked; @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0) -public class RemoteMigrationAllocationDeciderIT extends MigrationBaseTestCase { +public class RemoteMigrationAllocationDeciderIT extends RemoteStoreMigrationShardAllocationBaseTestCase { // When the primary is on doc rep node, existing replica copy can get allocated on excluded docrep node. public void testFilterAllocationSkipsReplica() throws IOException { @@ -127,4 +134,413 @@ public void testFilterAllocationSkipsReplicaOnExcludedNode() throws IOException assertTrue(clusterHealthResponse.isTimedOut()); ensureYellow("test"); } + + // When under mixed mode and remote_store direction, a primary shard can only be allocated to a remote node + + public void testNewPrimaryShardAllocationForRemoteStoreMigration() throws Exception { + logger.info("Initialize cluster"); + internalCluster().startClusterManagerOnlyNode(); + + logger.info("Add non-remote data node"); + String nonRemoteNodeName = internalCluster().startDataOnlyNode(); + internalCluster().validateClusterFormed(); + DiscoveryNode nonRemoteNode = assertNodeInCluster(nonRemoteNodeName); + + logger.info("Set mixed mode and remote_store direction"); + setClusterMode(MIXED.mode); + setDirection(REMOTE_STORE.direction); + + logger.info("Verify expected decision for allocating a new primary shard on a non-remote node"); + prepareIndexWithoutReplica(Optional.empty()); + Decision decision = getDecisionForTargetNode(nonRemoteNode, true, true, false); + assertEquals(Decision.Type.NO, decision.type()); + assertEquals( + "[remote_store migration_direction]: primary shard copy can not be allocated to a non-remote node", + decision.getExplanation().toLowerCase(Locale.ROOT) + ); + + logger.info("Attempt allocation on non-remote node"); + attemptAllocation(null); + + logger.info("Verify non-allocation of primary shard on non-remote node"); + assertNonAllocation(true); + + logger.info("Add remote data node"); + setAddRemote(true); + String remoteNodeName = internalCluster().startDataOnlyNode(); + internalCluster().validateClusterFormed(); + DiscoveryNode remoteNode = assertNodeInCluster(remoteNodeName); + + logger.info("Verify expected decision for allocating a new primary shard on a remote node"); + excludeAllNodes(); + decision = getDecisionForTargetNode(remoteNode, true, true, false); + assertEquals(Decision.Type.YES, decision.type()); + assertEquals( + "[remote_store migration_direction]: primary shard copy can be allocated to a remote node", + decision.getExplanation().toLowerCase(Locale.ROOT) + ); + + logger.info("Attempt free allocation"); + attemptAllocation(null); + ensureGreen(TEST_INDEX); + + logger.info("Verify allocation of primary shard on remote node"); + assertAllocation(true, remoteNode); + } + + // When under mixed mode and remote_store direction, a replica shard can only be allocated to a remote node if the primary has relocated + // to another remote node + + public void testNewReplicaShardAllocationIfPrimaryShardOnNonRemoteNodeForRemoteStoreMigration() throws Exception { + logger.info("Initialize cluster"); + internalCluster().startClusterManagerOnlyNode(); + + logger.info("Add non-remote data node"); + String nonRemoteNodeName1 = internalCluster().startDataOnlyNode(); + internalCluster().validateClusterFormed(); + DiscoveryNode nonRemoteNode1 = assertNodeInCluster(nonRemoteNodeName1); + + logger.info("Allocate primary shard on non-remote node"); + prepareIndexWithAllocatedPrimary(nonRemoteNode1, Optional.empty()); + + logger.info("Add remote data node"); + setClusterMode(MIXED.mode); + setAddRemote(true); + String remoteNodeName = internalCluster().startDataOnlyNode(); + internalCluster().validateClusterFormed(); + DiscoveryNode remoteNode = assertNodeInCluster(remoteNodeName); + + logger.info("Set remote_store direction"); + setDirection(REMOTE_STORE.direction); + + logger.info("Verify expected decision for allocating a replica shard on a remote node"); + excludeAllNodes(); + Decision decision = getDecisionForTargetNode(remoteNode, false, true, false); + assertEquals(Decision.Type.NO, decision.type()); + assertEquals( + "[remote_store migration_direction]: replica shard copy can not be allocated to a remote node since primary shard copy is not yet migrated to remote", + decision.getExplanation().toLowerCase(Locale.ROOT) + ); + + logger.info("Attempt free allocation of replica shard"); + attemptAllocation(null); + + logger.info("Verify non-allocation of replica shard"); + assertNonAllocation(false); + + logger.info("Add another non-remote data node"); + setAddRemote(false); + String nonRemoteNodeName2 = internalCluster().startDataOnlyNode(); + internalCluster().validateClusterFormed(); + DiscoveryNode nonRemoteNode2 = assertNodeInCluster(nonRemoteNodeName2); + + logger.info("Verify expected decision for allocating the replica shard on a non-remote node"); + excludeAllNodes(); + decision = getDecisionForTargetNode(nonRemoteNode2, false, true, false); + assertEquals(Decision.Type.YES, 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("Attempt free allocation of replica shard"); + attemptAllocation(null); + ensureGreen(TEST_INDEX); + + logger.info("Verify allocation of replica shard on non-remote node"); + assertAllocation(false, nonRemoteNode2); + } + + public void testNewReplicaShardAllocationIfPrimaryShardOnRemoteNodeForRemoteStoreMigration() throws Exception { + logger.info("Initialize cluster"); + internalCluster().startClusterManagerOnlyNode(); + + logger.info("Add non-remote data node"); + String nonRemoteNodeName = internalCluster().startDataOnlyNode(); + internalCluster().validateClusterFormed(); + DiscoveryNode nonRemoteNode = assertNodeInCluster(nonRemoteNodeName); + + logger.info("Allocate primary shard on non-remote node"); + createIndex(TEST_INDEX, 0); + ensureGreen(TEST_INDEX); + assertAllocation(true, nonRemoteNode); + + logger.info("Set mixed mode"); + setClusterMode(MIXED.mode); + + logger.info("Add remote data node"); + setAddRemote(true); + String remoteNodeName1 = internalCluster().startDataOnlyNode(); + internalCluster().validateClusterFormed(); + DiscoveryNode remoteNode1 = assertNodeInCluster(remoteNodeName1); + + logger.info("Set remote_store direction"); + setDirection(REMOTE_STORE.direction); + + logger.info("Relocate primary shard to remote node"); + includeAllNodes(); + assertAcked( + internalCluster().client() + .admin() + .cluster() + .prepareReroute() + .add(new MoveAllocationCommand(TEST_INDEX, 0, nonRemoteNodeName, remoteNodeName1)) + .get() + ); + ensureGreen(TEST_INDEX); + assertAllocation(true, remoteNode1); + + logger.info("Verify expected decision for allocating a replica shard on non-remote node"); + excludeAllNodes(); + assertAcked( + internalCluster().client() + .admin() + .indices() + .prepareUpdateSettings() + .setIndices(TEST_INDEX) + .setSettings(Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1).build()) + .get() + ); + ensureYellowAndNoInitializingShards(TEST_INDEX); + Decision decision = getDecisionForTargetNode(nonRemoteNode, false, true, false); + assertEquals(Decision.Type.YES, 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("Attempt free allocation of replica shard"); + attemptAllocation(null); + + logger.info("Verify allocation of replica shard on non-remote node"); + ensureGreen(TEST_INDEX); + assertAllocation(false, nonRemoteNode); + + logger.info("Add another remote data node"); + String remoteNodeName2 = internalCluster().startDataOnlyNode(); + internalCluster().validateClusterFormed(); + DiscoveryNode remoteNode2 = assertNodeInCluster(remoteNodeName2); + + logger.info("Verify expected decision for allocating a replica shard on remote node"); + excludeAllNodes(); + assertAcked( + internalCluster().client() + .admin() + .indices() + .prepareUpdateSettings() + .setIndices(TEST_INDEX) + .setSettings(Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 2).build()) + .get() + ); + ensureYellowAndNoInitializingShards(TEST_INDEX); + 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 free allocation of replica shard"); + attemptAllocation(null); + + logger.info("Verify allocation of replica shard on remote node"); + ensureGreen(TEST_INDEX); + assertAllocation(false, remoteNode2); + } + + // When under strict mode, a shard can be allocated to any node + + public void testAlwaysAllocateNewShardForStrictMode() throws Exception { + boolean isRemoteCluster = randomBoolean(); + boolean isReplicaAllocation = randomBoolean(); + + logger.info("Initialize cluster and add nodes"); + setAddRemote(isRemoteCluster); + internalCluster().startClusterManagerOnlyNode(); + String nodeName1 = internalCluster().startDataOnlyNode(); + String nodeName2 = internalCluster().startDataOnlyNode(); + internalCluster().validateClusterFormed(); + DiscoveryNode node1 = assertNodeInCluster(nodeName1); + DiscoveryNode node2 = assertNodeInCluster(nodeName2); + + if (isReplicaAllocation) { + prepareIndexWithAllocatedPrimary(node1, Optional.empty()); + } else { + prepareIndexWithoutReplica(Optional.empty()); + } + + if (isRemoteCluster) { + assertRemoteStoreBackedIndex(TEST_INDEX); + } else { + assertNonRemoteStoreBackedIndex(TEST_INDEX); + } + + logger.info("Verify expected decision for allocation of a shard"); + excludeAllNodes(); + Decision decision = getDecisionForTargetNode( + isReplicaAllocation ? node2 : randomFrom(node1, node2), + !isReplicaAllocation, + true, + false + ); + assertEquals(Decision.Type.YES, decision.type()); + String expectedReason = String.format( + Locale.ROOT, + "[none migration_direction]: %s shard copy can be allocated to a %s node for strict compatibility mode", + (isReplicaAllocation ? "replica" : "primary"), + (isRemoteCluster ? "remote" : "non-remote") + ); + assertEquals(expectedReason, decision.getExplanation().toLowerCase(Locale.ROOT)); + + logger.info("Attempt free allocation"); + attemptAllocation(null); + ensureGreen(TEST_INDEX); + + logger.info("Verify allocation of shard"); + assertAllocation(!isReplicaAllocation, !isReplicaAllocation ? null : node2); + } + + // When under mixed mode and remote_store direction, shard of a remote store backed index can not be allocated to a non-remote node + + public void testRemoteStoreBackedIndexShardAllocationForRemoteStoreMigration() throws Exception { + logger.info("Initialize cluster"); + internalCluster().startClusterManagerOnlyNode(); + + logger.info("Set mixed mode"); + setClusterMode(MIXED.mode); + + logger.info("Add remote and non-remote nodes"); + String nonRemoteNodeName = internalCluster().startDataOnlyNode(); + setAddRemote(true); + String remoteNodeName = internalCluster().startDataOnlyNode(); + internalCluster().validateClusterFormed(); + DiscoveryNode remoteNode = assertNodeInCluster(remoteNodeName); + DiscoveryNode nonRemoteNode = assertNodeInCluster(nonRemoteNodeName); + + logger.info("Set remote_store direction"); + setDirection(REMOTE_STORE.direction); + + boolean isReplicaAllocation = randomBoolean(); + if (isReplicaAllocation) { + logger.info("Create index with primary allocated on remote node"); + prepareIndexWithAllocatedPrimary(remoteNode, Optional.empty()); + } else { + logger.info("Create index with unallocated primary"); + prepareIndexWithoutReplica(Optional.empty()); + } + + logger.info("Verify remote store backed index"); + assertRemoteStoreBackedIndex(TEST_INDEX); + + logger.info("Verify expected decision for allocation of shard on a non-remote node"); + excludeAllNodes(); + Decision decision = getDecisionForTargetNode(nonRemoteNode, !isReplicaAllocation, false, false); + assertEquals(Decision.Type.NO, decision.type()); + String expectedReason = String.format( + Locale.ROOT, + "[remote_store migration_direction]: %s shard copy can not be allocated to a non-remote node because a remote store backed index's shard copy can only be allocated to a remote node", + (isReplicaAllocation ? "replica" : "primary") + ); + assertEquals(expectedReason, decision.getExplanation().toLowerCase(Locale.ROOT)); + + logger.info("Attempt allocation of shard on non-remote node"); + attemptAllocation(nonRemoteNodeName); + + logger.info("Verify non-allocation of shard"); + assertNonAllocation(!isReplicaAllocation); + } + + // When under mixed mode and none direction, allocate shard of a remote store backed index to a remote node and shard of a non remote + // store backed index to a non-remote node only + + public void testAllocationForNoneDirectionAndMixedMode() throws Exception { + boolean isRemoteStoreBackedIndex = randomBoolean(); + boolean isReplicaAllocation = randomBoolean(); + logger.info( + String.format( + Locale.ROOT, + "Test for allocation decisions for %s shard of a %s store backed index under NONE direction", + (isReplicaAllocation ? "replica" : "primary"), + (isRemoteStoreBackedIndex ? "remote" : "non remote") + ) + ); + + logger.info("Initialize cluster"); + setAddRemote(isRemoteStoreBackedIndex); + internalCluster().startClusterManagerOnlyNode(); + + logger.info("Add data nodes"); + String previousNodeName1 = internalCluster().startDataOnlyNode(); + String previousNodeName2 = internalCluster().startDataOnlyNode(); + internalCluster().validateClusterFormed(); + DiscoveryNode previousNode1 = assertNodeInCluster(previousNodeName1); + DiscoveryNode previousNode2 = assertNodeInCluster(previousNodeName2); + + logger.info("Prepare test index"); + if (isReplicaAllocation) { + prepareIndexWithAllocatedPrimary(previousNode1, Optional.empty()); + } else { + prepareIndexWithoutReplica(Optional.empty()); + } + + if (isRemoteStoreBackedIndex) { + assertRemoteStoreBackedIndex(TEST_INDEX); + } else { + assertNonRemoteStoreBackedIndex(TEST_INDEX); + } + + logger.info("Switch to MIXED cluster compatibility mode"); + setClusterMode(MIXED.mode); + setAddRemote(!addRemote); + String newNodeName = internalCluster().startDataOnlyNode(); + internalCluster().validateClusterFormed(); + DiscoveryNode newNode = assertNodeInCluster(newNodeName); + + logger.info("Verify decision for allocation on the new node"); + excludeAllNodes(); + Decision decision = getDecisionForTargetNode(newNode, !isReplicaAllocation, false, false); + assertEquals(Decision.Type.NO, decision.type()); + String expectedReason = String.format( + Locale.ROOT, + "[none migration_direction]: %s shard copy can not be allocated to a %s node for %s store backed index", + (isReplicaAllocation ? "replica" : "primary"), + (isRemoteStoreBackedIndex ? "non-remote" : "remote"), + (isRemoteStoreBackedIndex ? "remote" : "non remote") + ); + assertEquals(expectedReason, decision.getExplanation().toLowerCase(Locale.ROOT)); + + logger.info("Attempt allocation of shard on new node"); + attemptAllocation(newNodeName); + + logger.info("Verify non-allocation of shard"); + assertNonAllocation(!isReplicaAllocation); + + logger.info("Verify decision for allocation on previous node"); + decision = getDecisionForTargetNode(previousNode2, !isReplicaAllocation, true, false); + assertEquals(Decision.Type.YES, decision.type()); + expectedReason = String.format( + Locale.ROOT, + "[none migration_direction]: %s shard copy can be allocated to a %s node for %s store backed index", + (isReplicaAllocation ? "replica" : "primary"), + (isRemoteStoreBackedIndex ? "remote" : "non-remote"), + (isRemoteStoreBackedIndex ? "remote" : "non remote") + ); + assertEquals(expectedReason, decision.getExplanation().toLowerCase(Locale.ROOT)); + + logger.info("Attempt free allocation of shard"); + attemptAllocation(null); + + logger.info("Verify successful allocation of shard"); + if (!isReplicaAllocation) { + ensureGreen(TEST_INDEX); + } else { + ensureYellowAndNoInitializingShards(TEST_INDEX); + } + assertAllocation(!isReplicaAllocation, null); + logger.info("Verify allocation on one of the previous nodes"); + ShardRouting shardRouting = getShardRouting(!isReplicaAllocation); + assertTrue( + shardRouting.currentNodeId().equals(previousNode1.getId()) || shardRouting.currentNodeId().equals(previousNode2.getId()) + ); + } } diff --git a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationBaseTestCase.java b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationBaseTestCase.java index fdd0d3ca1190f..cf689aa554c8b 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationBaseTestCase.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationBaseTestCase.java @@ -190,14 +190,36 @@ protected Decision getDecisionForTargetNode( } // get allocation and relocation decisions for all nodes - protected void prepareDecisions() { - internalCluster().client() - .admin() - .indices() - .prepareUpdateSettings(TEST_INDEX) - .setSettings(Settings.builder().put("index.routing.allocation.exclude._name", allNodesExcept(null))) - .execute() - .actionGet(); + protected void excludeAllNodes() { + assertAcked( + internalCluster().client() + .admin() + .indices() + .prepareUpdateSettings(TEST_INDEX) + .setSettings( + Settings.builder() + .put("index.routing.allocation.include._name", "") + .put("index.routing.allocation.exclude._name", allNodesExcept(null)) + ) + .execute() + .actionGet() + ); + } + + protected void includeAllNodes() { + assertAcked( + internalCluster().client() + .admin() + .indices() + .prepareUpdateSettings(TEST_INDEX) + .setSettings( + Settings.builder() + .put("index.routing.allocation.exclude._name", "") + .put("index.routing.allocation.include._name", allNodesExcept(null)) + ) + .execute() + .actionGet() + ); } protected void attemptAllocation(@Nullable String targetNodeName) { @@ -244,7 +266,7 @@ protected void assertAllocation(boolean isPrimary, @Nullable DiscoveryNode targe assertTrue(shardRouting.active()); assertNotNull(shardRouting.currentNodeId()); if (targetNode != null) { - assertEquals(shardRouting.currentNodeId(), targetNode.getId()); + assertEquals(targetNode.getId(), shardRouting.currentNodeId()); } } diff --git a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationIT.java b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationIT.java deleted file mode 100644 index 1e82404116dfe..0000000000000 --- a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationIT.java +++ /dev/null @@ -1,127 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.remotemigration; - -import org.opensearch.client.Client; -import org.opensearch.cluster.node.DiscoveryNode; -import org.opensearch.cluster.routing.ShardRouting; -import org.opensearch.cluster.routing.allocation.decider.Decision; -import org.opensearch.test.OpenSearchIntegTestCase; - -import java.util.Locale; -import java.util.Optional; - -import static org.opensearch.node.remotestore.RemoteStoreNodeService.CompatibilityMode.MIXED; - -@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0) -public class RemoteStoreMigrationShardAllocationIT extends RemoteStoreMigrationShardAllocationBaseTestCase { - - public static final String NAME = "remote_store_migration"; - - private Client client; - - // test for shard allocation decisions for MIXED mode and NONE direction - public void testAllocationForNoneDirectionAndMixedMode() throws Exception { - boolean isRemoteStoreBackedIndex = randomBoolean(); - boolean isReplicaAllocation = randomBoolean(); - logger.info( - String.format( - Locale.ROOT, - "Test for allocation decisions for %s shard of a %s store backed index under NONE direction", - (isReplicaAllocation ? "replica" : "primary"), - (isRemoteStoreBackedIndex ? "remote" : "non remote") - ) - ); - - logger.info("Initialize cluster"); - initializeCluster(isRemoteStoreBackedIndex); - - logger.info("Add data nodes"); - String previousNodeName1 = internalCluster().startDataOnlyNode(); - String previousNodeName2 = internalCluster().startDataOnlyNode(); - internalCluster().validateClusterFormed(); - DiscoveryNode previousNode1 = assertNodeInCluster(previousNodeName1); - DiscoveryNode previousNode2 = assertNodeInCluster(previousNodeName2); - - logger.info("Prepare test index"); - if (isReplicaAllocation) { - prepareIndexWithAllocatedPrimary(previousNode1, Optional.empty()); - } else { - prepareIndexWithoutReplica(Optional.empty()); - } - - if (isRemoteStoreBackedIndex) { - assertRemoteStoreBackedIndex(TEST_INDEX); - } else { - assertNonRemoteStoreBackedIndex(TEST_INDEX); - } - - logger.info("Switch to MIXED cluster compatibility mode"); - setClusterMode(MIXED.mode); - addRemote = !addRemote; - String newNodeName = internalCluster().startDataOnlyNode(); - internalCluster().validateClusterFormed(); - DiscoveryNode newNode = assertNodeInCluster(newNodeName); - - logger.info("Verify decision for allocation on the new node"); - prepareDecisions(); - Decision decision = getDecisionForTargetNode(newNode, !isReplicaAllocation, false, false); - assertEquals(Decision.Type.NO, decision.type()); - String expectedReason = String.format( - Locale.ROOT, - "[none migration_direction]: %s shard copy can not be allocated to a %s node for %s store backed index", - (isReplicaAllocation ? "replica" : "primary"), - (isRemoteStoreBackedIndex ? "non-remote" : "remote"), - (isRemoteStoreBackedIndex ? "remote" : "non remote") - ); - assertEquals(expectedReason, decision.getExplanation().toLowerCase(Locale.ROOT)); - - logger.info("Attempt allocation of shard on new node"); - attemptAllocation(newNodeName); - - logger.info("Verify non-allocation of shard"); - assertNonAllocation(!isReplicaAllocation); - - logger.info("Verify decision for allocation on previous node"); - decision = getDecisionForTargetNode(previousNode2, !isReplicaAllocation, true, false); - assertEquals(Decision.Type.YES, decision.type()); - expectedReason = String.format( - Locale.ROOT, - "[none migration_direction]: %s shard copy can be allocated to a %s node for %s store backed index", - (isReplicaAllocation ? "replica" : "primary"), - (isRemoteStoreBackedIndex ? "remote" : "non-remote"), - (isRemoteStoreBackedIndex ? "remote" : "non remote") - ); - assertEquals(expectedReason, decision.getExplanation().toLowerCase(Locale.ROOT)); - - logger.info("Attempt free allocation of shard"); - attemptAllocation(null); - - logger.info("Verify successful allocation of shard"); - if (!isReplicaAllocation) { - ensureGreen(TEST_INDEX); - } else { - ensureYellowAndNoInitializingShards(TEST_INDEX); - } - assertAllocation(!isReplicaAllocation, null); - logger.info("Verify allocation on one of the previous nodes"); - ShardRouting shardRouting = getShardRouting(!isReplicaAllocation); - assertTrue( - shardRouting.currentNodeId().equals(previousNode1.getId()) || shardRouting.currentNodeId().equals(previousNode2.getId()) - ); - } - - // bootstrap a cluster - private void initializeCluster(boolean remoteClusterManager) { - addRemote = remoteClusterManager; - internalCluster().startClusterManagerOnlyNode(); - client = internalCluster().client(); - } - -} From 54bfe7314c6bcc4114e55a39d1c1deb041cad8f0 Mon Sep 17 00:00:00 2001 From: Lakshya Taragi Date: Tue, 30 Apr 2024 19:08:18 +0530 Subject: [PATCH 8/8] Fix replica allocation test Signed-off-by: Lakshya Taragi --- .../RemoteMigrationAllocationDeciderIT.java | 85 +++++++++---------- 1 file changed, 38 insertions(+), 47 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteMigrationAllocationDeciderIT.java b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteMigrationAllocationDeciderIT.java index 9c3aec58b6eed..eeb6a5a5626e4 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteMigrationAllocationDeciderIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteMigrationAllocationDeciderIT.java @@ -255,95 +255,86 @@ public void testNewReplicaShardAllocationIfPrimaryShardOnRemoteNodeForRemoteStor logger.info("Initialize cluster"); internalCluster().startClusterManagerOnlyNode(); - logger.info("Add non-remote data node"); - String nonRemoteNodeName = internalCluster().startDataOnlyNode(); + logger.info("Add non-remote data nodes"); + String nonRemoteNodeName1 = internalCluster().startDataOnlyNode(); + String nonRemoteNodeName2 = internalCluster().startDataOnlyNode(); internalCluster().validateClusterFormed(); - DiscoveryNode nonRemoteNode = assertNodeInCluster(nonRemoteNodeName); + DiscoveryNode nonRemoteNode1 = assertNodeInCluster(nonRemoteNodeName1); + DiscoveryNode nonRemoteNode2 = assertNodeInCluster(nonRemoteNodeName2); - logger.info("Allocate primary shard on non-remote node"); - createIndex(TEST_INDEX, 0); + logger.info("Allocate primary and replica shard on non-remote nodes"); + createIndex(TEST_INDEX, 1); ensureGreen(TEST_INDEX); - assertAllocation(true, nonRemoteNode); logger.info("Set mixed mode"); setClusterMode(MIXED.mode); - logger.info("Add remote data node"); + logger.info("Add remote data nodes"); setAddRemote(true); String remoteNodeName1 = internalCluster().startDataOnlyNode(); + String remoteNodeName2 = internalCluster().startDataOnlyNode(); internalCluster().validateClusterFormed(); DiscoveryNode remoteNode1 = assertNodeInCluster(remoteNodeName1); + DiscoveryNode remoteNode2 = assertNodeInCluster(remoteNodeName2); logger.info("Set remote_store direction"); setDirection(REMOTE_STORE.direction); logger.info("Relocate primary shard to remote node"); - includeAllNodes(); + DiscoveryNode initialPrimaryNode = primaryNodeName(TEST_INDEX).equals(nonRemoteNodeName1) ? nonRemoteNode1 : nonRemoteNode2; + DiscoveryNode initialReplicaNode = initialPrimaryNode.equals(nonRemoteNode1) ? nonRemoteNode2 : nonRemoteNode1; assertAcked( internalCluster().client() .admin() .cluster() .prepareReroute() - .add(new MoveAllocationCommand(TEST_INDEX, 0, nonRemoteNodeName, remoteNodeName1)) + .add(new MoveAllocationCommand(TEST_INDEX, 0, initialPrimaryNode.getName(), remoteNodeName1)) .get() ); ensureGreen(TEST_INDEX); assertAllocation(true, remoteNode1); - logger.info("Verify expected decision for allocating a replica shard on non-remote node"); - excludeAllNodes(); - assertAcked( - internalCluster().client() - .admin() - .indices() - .prepareUpdateSettings() - .setIndices(TEST_INDEX) - .setSettings(Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1).build()) - .get() - ); - ensureYellowAndNoInitializingShards(TEST_INDEX); - Decision decision = getDecisionForTargetNode(nonRemoteNode, false, true, false); + logger.info("Verify expected decision for relocating a replica shard on non-remote node"); + Decision decision = getDecisionForTargetNode(initialPrimaryNode, false, true, true); assertEquals(Decision.Type.YES, decision.type()); assertEquals( - "[remote_store migration_direction]: replica shard copy can be allocated to a non-remote node", + "[remote_store migration_direction]: replica shard copy can be relocated to a non-remote node", decision.getExplanation().toLowerCase(Locale.ROOT) ); - logger.info("Attempt free allocation of replica shard"); - attemptAllocation(null); - - logger.info("Verify allocation of replica shard on non-remote node"); - ensureGreen(TEST_INDEX); - assertAllocation(false, nonRemoteNode); - - logger.info("Add another remote data node"); - String remoteNodeName2 = internalCluster().startDataOnlyNode(); - internalCluster().validateClusterFormed(); - DiscoveryNode remoteNode2 = assertNodeInCluster(remoteNodeName2); - - logger.info("Verify expected decision for allocating a replica shard on remote node"); - excludeAllNodes(); + logger.info("Attempt relocation of replica shard to non-remote node"); assertAcked( internalCluster().client() .admin() - .indices() - .prepareUpdateSettings() - .setIndices(TEST_INDEX) - .setSettings(Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 2).build()) + .cluster() + .prepareReroute() + .add(new MoveAllocationCommand(TEST_INDEX, 0, initialReplicaNode.getName(), initialPrimaryNode.getName())) .get() ); - ensureYellowAndNoInitializingShards(TEST_INDEX); - decision = getDecisionForTargetNode(remoteNode2, false, true, false); + + logger.info("Verify relocation of replica shard to non-remote node"); + ensureGreen(TEST_INDEX); + assertAllocation(false, initialPrimaryNode); + + logger.info("Verify expected decision for relocating a replica shard on remote node"); + decision = getDecisionForTargetNode(remoteNode2, false, true, true); 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", + "[remote_store migration_direction]: replica shard copy can be relocated to a remote node since primary shard copy has been migrated to remote", decision.getExplanation().toLowerCase(Locale.ROOT) ); - logger.info("Attempt free allocation of replica shard"); - attemptAllocation(null); + logger.info("Attempt relocation of replica shard to remote node"); + assertAcked( + internalCluster().client() + .admin() + .cluster() + .prepareReroute() + .add(new MoveAllocationCommand(TEST_INDEX, 0, initialPrimaryNode.getName(), remoteNodeName2)) + .get() + ); - logger.info("Verify allocation of replica shard on remote node"); + logger.info("Verify relocation of replica shard to non-remote node"); ensureGreen(TEST_INDEX); assertAllocation(false, remoteNode2); }