Skip to content

Commit

Permalink
Reject Resize index requests during remote store migration (opensearc…
Browse files Browse the repository at this point in the history
…h-project#12686)

Signed-off-by: Shubh Sahu <shubhvs@amazon.com>
  • Loading branch information
astute-decipher authored Apr 4, 2024
1 parent 17641f6 commit 9ab31f5
Show file tree
Hide file tree
Showing 4 changed files with 418 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Detect breaking changes on pull requests ([#9044](https://github.com/opensearch-project/OpenSearch/pull/9044))
- Add cluster primary balance contraint for rebalancing with buffer ([#12656](https://github.com/opensearch-project/OpenSearch/pull/12656))
- [Remote Store] Make translog transfer timeout configurable ([#12704](https://github.com/opensearch-project/OpenSearch/pull/12704))
- Reject Resize index requests (i.e, split, shrink and clone), While DocRep to SegRep migration is in progress.([#12686](https://github.com/opensearch-project/OpenSearch/pull/12686))

### Dependencies
- Bump `org.apache.commons:commons-configuration2` from 2.10.0 to 2.10.1 ([#12896](https://github.com/opensearch-project/OpenSearch/pull/12896))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,208 @@
/*
* 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.action.admin.cluster.settings.ClusterUpdateSettingsRequest;
import org.opensearch.action.admin.indices.shrink.ResizeType;
import org.opensearch.action.support.ActiveShardCount;
import org.opensearch.client.Client;
import org.opensearch.common.settings.Settings;
import org.opensearch.indices.replication.common.ReplicationType;
import org.opensearch.test.OpenSearchIntegTestCase;

import java.util.List;

import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE;
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, autoManageMasterNodes = false)
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";

/*
* This test will verify the resize request failure, when cluster mode is mixed
* and index is on DocRep node, and migration to remote store is in progress.
* */
public void testFailResizeIndexWhileDocRepToRemoteStoreMigration() throws Exception {

internalCluster().setBootstrapClusterManagerNodeIndex(0);
List<String> 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
addRemote = false;
String nonRemoteNodeName = internalCluster().startNode();

addRemote = true;
String remoteNodeName = internalCluster().startNode();

logger.info("-->Create index on non-remote node and SETTING_REMOTE_STORE_ENABLED is false. 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", nonRemoteNodeName)
.put("index.routing.allocation.exclude._name", remoteNodeName)
)
.setWaitForActiveShards(ActiveShardCount.ALL)
.execute()
.actionGet();

updateSettingsRequest.persistentSettings(Settings.builder().put(MIGRATION_DIRECTION_SETTING.getKey(), REMOTE_STORE_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 " + resizeType + " is not allowed as remote migration mode is mixed" + " and index is remote store disabled"
);
}

/*
* This test will verify the resize request failure, when cluster mode is mixed
* and index is on Remote Store node, and migration to DocRep node is in progress.
* */
public void testFailResizeIndexWhileRemoteStoreToDocRepMigration() throws Exception {

addRemote = true;
internalCluster().setBootstrapClusterManagerNodeIndex(0);
List<String> 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 " + resizeType + " is not allowed as remote migration mode is mixed" + " and index is remote store enabled"
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import org.opensearch.cluster.node.DiscoveryNode;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.inject.Inject;
import org.opensearch.common.settings.ClusterSettings;
import org.opensearch.common.settings.Settings;
import org.opensearch.core.action.ActionListener;
import org.opensearch.core.common.io.stream.StreamInput;
Expand All @@ -57,6 +58,9 @@
import org.opensearch.index.IndexSettings;
import org.opensearch.index.shard.DocsStats;
import org.opensearch.index.store.StoreStats;
import org.opensearch.node.remotestore.RemoteStoreNodeService;
import org.opensearch.node.remotestore.RemoteStoreNodeService.CompatibilityMode;
import org.opensearch.node.remotestore.RemoteStoreNodeService.Direction;
import org.opensearch.threadpool.ThreadPool;
import org.opensearch.transport.TransportService;

Expand All @@ -67,6 +71,7 @@
import java.util.function.IntFunction;

import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED;

/**
* Main class to initiate resizing (shrink / split) an index into a new index
Expand Down Expand Up @@ -140,8 +145,8 @@ protected void clusterManagerOperation(
// there is no need to fetch docs stats for split but we keep it simple and do it anyway for simplicity of the code
final String sourceIndex = indexNameExpressionResolver.resolveDateMathExpression(resizeRequest.getSourceIndex());
final String targetIndex = indexNameExpressionResolver.resolveDateMathExpression(resizeRequest.getTargetIndexRequest().index());

IndexMetadata indexMetadata = state.metadata().index(sourceIndex);
ClusterSettings clusterSettings = clusterService.getClusterSettings();
if (resizeRequest.getResizeType().equals(ResizeType.SHRINK)
&& state.metadata().isSegmentReplicationEnabled(sourceIndex)
&& indexMetadata != null
Expand All @@ -161,7 +166,7 @@ protected void clusterManagerOperation(
CreateIndexClusterStateUpdateRequest updateRequest = prepareCreateIndexRequest(resizeRequest, state, i -> {
IndexShardStats shard = indicesStatsResponse.getIndex(sourceIndex).getIndexShards().get(i);
return shard == null ? null : shard.getPrimary().getDocs();
}, indicesStatsResponse.getPrimaries().store, sourceIndex, targetIndex);
}, indicesStatsResponse.getPrimaries().store, clusterSettings, sourceIndex, targetIndex);

if (indicesStatsResponse.getIndex(sourceIndex)
.getTotal()
Expand Down Expand Up @@ -200,7 +205,7 @@ protected void clusterManagerOperation(
CreateIndexClusterStateUpdateRequest updateRequest = prepareCreateIndexRequest(resizeRequest, state, i -> {
IndexShardStats shard = indicesStatsResponse.getIndex(sourceIndex).getIndexShards().get(i);
return shard == null ? null : shard.getPrimary().getDocs();
}, indicesStatsResponse.getPrimaries().store, sourceIndex, targetIndex);
}, indicesStatsResponse.getPrimaries().store, clusterSettings, sourceIndex, targetIndex);
createIndexService.createIndex(
updateRequest,
ActionListener.map(
Expand All @@ -223,6 +228,7 @@ static CreateIndexClusterStateUpdateRequest prepareCreateIndexRequest(
final ClusterState state,
final IntFunction<DocsStats> perShardDocStats,
final StoreStats primaryShardsStoreStats,
final ClusterSettings clusterSettings,
String sourceIndexName,
String targetIndexName
) {
Expand All @@ -231,6 +237,7 @@ static CreateIndexClusterStateUpdateRequest prepareCreateIndexRequest(
if (metadata == null) {
throw new IndexNotFoundException(sourceIndexName);
}
validateRemoteMigrationModeSettings(resizeRequest.getResizeType(), metadata, clusterSettings);
final Settings.Builder targetIndexSettingsBuilder = Settings.builder()
.put(targetIndex.settings())
.normalizePrefix(IndexMetadata.INDEX_SETTING_PREFIX);
Expand Down Expand Up @@ -368,4 +375,39 @@ protected static int calculateTargetIndexShardsNum(
protected String getClusterManagerActionName(DiscoveryNode node) {
return super.getClusterManagerActionName(node);
}

/**
* Reject resize request if cluster mode is [Mixed] and migration direction is [RemoteStore] and index is not on
* REMOTE_STORE_ENABLED node or [DocRep] and index is on REMOTE_STORE_ENABLED node.
* @param type resize type
* @param sourceIndexMetadata source index's metadata
* @param clusterSettings cluster settings
* @throws IllegalStateException if cluster mode is [Mixed] and migration direction is [RemoteStore] or [DocRep] and
* index's SETTING_REMOTE_STORE_ENABLED is not equal to the migration direction's value.
* For example, if migration direction is [RemoteStore] and index's SETTING_REMOTE_STORE_ENABLED
* is false, then throw IllegalStateException. If migration direction is [DocRep] and
* index's SETTING_REMOTE_STORE_ENABLED is true, then throw IllegalStateException.
*/
private static void validateRemoteMigrationModeSettings(
final ResizeType type,
IndexMetadata sourceIndexMetadata,
ClusterSettings clusterSettings
) {
CompatibilityMode compatibilityMode = clusterSettings.get(RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING);
if (compatibilityMode == CompatibilityMode.MIXED) {
boolean isRemoteStoreEnabled = sourceIndexMetadata.getSettings().getAsBoolean(SETTING_REMOTE_STORE_ENABLED, false);
Direction migrationDirection = clusterSettings.get(RemoteStoreNodeService.MIGRATION_DIRECTION_SETTING);
boolean invalidConfiguration = (migrationDirection == Direction.REMOTE_STORE && isRemoteStoreEnabled == false)
|| (migrationDirection == Direction.DOCREP && isRemoteStoreEnabled);
if (invalidConfiguration) {
throw new IllegalStateException(
"Index "
+ type
+ " is not allowed as remote migration mode is mixed"
+ " and index is remote store "
+ (isRemoteStoreEnabled ? "enabled" : "disabled")
);
}
}
}
}
Loading

0 comments on commit 9ab31f5

Please sign in to comment.