Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reject Resize index requests. (i.e, split, shrink and… #12686

Merged
merged 12 commits into from
Apr 4, 2024
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 {
astute-decipher marked this conversation as resolved.
Show resolved Hide resolved

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 @@
// 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();

Check warning on line 149 in server/src/main/java/org/opensearch/action/admin/indices/shrink/TransportResizeAction.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/action/admin/indices/shrink/TransportResizeAction.java#L149

Added line #L149 was not covered by tests
if (resizeRequest.getResizeType().equals(ResizeType.SHRINK)
&& state.metadata().isSegmentReplicationEnabled(sourceIndex)
&& indexMetadata != null
Expand All @@ -161,7 +166,7 @@
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);

Check warning on line 169 in server/src/main/java/org/opensearch/action/admin/indices/shrink/TransportResizeAction.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/action/admin/indices/shrink/TransportResizeAction.java#L169

Added line #L169 was not covered by tests

if (indicesStatsResponse.getIndex(sourceIndex)
.getTotal()
Expand Down Expand Up @@ -200,7 +205,7 @@
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);

Check warning on line 208 in server/src/main/java/org/opensearch/action/admin/indices/shrink/TransportResizeAction.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/action/admin/indices/shrink/TransportResizeAction.java#L208

Added line #L208 was not covered by tests
createIndexService.createIndex(
updateRequest,
ActionListener.map(
Expand All @@ -223,6 +228,7 @@
final ClusterState state,
final IntFunction<DocsStats> perShardDocStats,
final StoreStats primaryShardsStoreStats,
final ClusterSettings clusterSettings,
String sourceIndexName,
String targetIndexName
) {
Expand All @@ -231,6 +237,7 @@
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 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(

Check warning on line 403 in server/src/main/java/org/opensearch/action/admin/indices/shrink/TransportResizeAction.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/action/admin/indices/shrink/TransportResizeAction.java#L403

Added line #L403 was not covered by tests
"Index "
+ type
+ " is not allowed as remote migration mode is mixed"
+ " and index is remote store "
+ (isRemoteStoreEnabled ? "enabled" : "disabled")
);
}
}
}
}
Loading
Loading