Skip to content

Commit

Permalink
Split changes
Browse files Browse the repository at this point in the history 
Signed-off-by: Lakshya Taragi <[email protected]>
  • Loading branch information
@ltaragi
ltaragi committed Apr 8, 2024
1 parent 9f0d981 commit 155f5b3
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 76 deletions.
14 changes: 0 additions & 14 deletions ...rnalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationAllocationIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,13 +171,6 @@ public void testAllocateNewReplicaShardOnRemoteNodeIfPrimaryShardOnRemoteNodeFor
"[remote_store migration_direction]: replica shard copy can be allocated to a remote node since primary shard copy has been migrated to remote",
decision.getExplanation().toLowerCase(Locale.ROOT)
);

logger.info(" --> attempt allocation of replica shard the other remote node");
attemptAllocation(remoteNodeName2);
ensureGreen(TEST_INDEX);

logger.info(" --> verify allocation of replica shard");
assertAllocation(false, remoteNode2);
}

public void testAllocateNewReplicaShardOnNonRemoteNodeIfPrimaryShardOnNonRemoteNodeForMixedModeAndRemoteStoreDirection()
Expand Down Expand Up @@ -252,13 +245,6 @@ public void testAllocateNewReplicaShardOnNonRemoteNodeIfPrimaryShardOnRemoteNode
"[remote_store migration_direction]: replica shard copy can be allocated to a non-remote node",
decision.getExplanation().toLowerCase(Locale.ROOT)
);

logger.info(" --> allocate replica shard on non-remote node");
attemptAllocation(nonRemoteNodeName);
ensureGreen(TEST_INDEX);

logger.info(" --> verify allocation of replica shard");
assertAllocation(false, nonRemoteNode);
}

// test for STRICT mode
Expand Down
33 changes: 0 additions & 33 deletions ...ClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationSettingsUpdateIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,9 @@
import org.opensearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse;
import org.opensearch.client.Client;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.settings.SettingsException;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.index.IndexSettings;
import org.opensearch.indices.replication.common.ReplicationType;
import org.opensearch.test.InternalTestCluster;
import org.opensearch.test.OpenSearchIntegTestCase;

import java.util.Optional;
Expand Down Expand Up @@ -78,37 +76,6 @@ public void testNewIndexIsRemoteStoreBackedForRemoteStoreDirectionAndMixedMode()
assertRemoteStoreBackedIndex(indexName2);
}

// compatibility mode setting test

public void testSwitchToStrictMode() throws Exception {
logger.info(" --> initialize cluster");
initializeCluster(false);

logger.info(" --> create a mixed mode cluster");
setClusterMode(MIXED.mode);
addRemote = true;
String remoteNodeName = internalCluster().startNode();
addRemote = false;
String nonRemoteNodeName = internalCluster().startNode();
internalCluster().validateClusterFormed();
assertNodeInCluster(remoteNodeName);
assertNodeInCluster(nonRemoteNodeName);

logger.info(" --> attempt switching to strict mode");
SettingsException exception = assertThrows(SettingsException.class, () -> setClusterMode(STRICT.mode));
assertEquals(
"can not switch to STRICT compatibility mode when the cluster contains both remote and non-remote nodes",
exception.getMessage()
);

logger.info(" --> stop remote node");
internalCluster().stopRandomNode(InternalTestCluster.nameFilter(remoteNodeName));
ensureStableCluster(2);

logger.info(" --> attempt switching to strict mode");
setClusterMode(STRICT.mode);
}

// verify that the created index is not remote store backed
private void assertNonRemoteStoreBackedIndex(String indexName) {
Settings indexSettings = client.admin().indices().prepareGetIndex().execute().actionGet().getSettings().get(indexName);
Expand Down
29 changes: 0 additions & 29 deletions ...va/org/opensearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,12 @@
import org.opensearch.common.Priority;
import org.opensearch.common.inject.Inject;
import org.opensearch.common.settings.ClusterSettings;
import org.opensearch.common.settings.SettingsException;
import org.opensearch.core.action.ActionListener;
import org.opensearch.core.common.io.stream.StreamInput;
import org.opensearch.node.remotestore.RemoteStoreNodeService;
import org.opensearch.threadpool.ThreadPool;
import org.opensearch.transport.TransportService;

import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;

/**
* Transport action for updating cluster settings
Expand Down Expand Up @@ -142,7 +137,6 @@ protected void clusterManagerOperation(
final ClusterState state,
final ActionListener<ClusterUpdateSettingsResponse> listener
) {
validateCompatibilityModeSettingRequest(request, state);
final SettingsUpdater updater = new SettingsUpdater(clusterSettings);
clusterService.submitStateUpdateTask(
"cluster_update_settings",
Expand Down Expand Up @@ -270,27 +264,4 @@ public ClusterState execute(final ClusterState currentState) {
);
}

/**
* Verifies that while trying to switch to STRICT compatibility mode, all nodes must be of the
* same type (all remote or all non-remote). If not, it throws SettingsException error
* @param request cluster settings update request, for settings to be updated and new values
* @param clusterState current state of cluster, for information on nodes
*/
private void validateCompatibilityModeSettingRequest(ClusterUpdateSettingsRequest request, ClusterState clusterState) {
if (RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING.exists(request.persistentSettings())) {
String value = request.persistentSettings().get(RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey());
if (value.equals(RemoteStoreNodeService.CompatibilityMode.STRICT.mode)) {
List<DiscoveryNode> discoveryNodeList = new ArrayList<>(clusterState.nodes().getNodes().values());
Optional<DiscoveryNode> remoteNode = discoveryNodeList.stream().filter(DiscoveryNode::isRemoteStoreNode).findFirst();
Optional<DiscoveryNode> nonRemoteNode = discoveryNodeList.stream()
.filter(dn -> dn.isRemoteStoreNode() == false)
.findFirst();
if (remoteNode.isPresent() && nonRemoteNode.isPresent()) {
throw new SettingsException(
"can not switch to STRICT compatibility mode when the cluster contains both remote and non-remote nodes"
);
}
}
}
}
}
132 changes: 132 additions & 0 deletions server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,15 @@
import org.opensearch.cluster.routing.allocation.decider.AwarenessAllocationDecider;
import org.opensearch.cluster.routing.allocation.decider.MaxRetryAllocationDecider;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.UUIDs;
import org.opensearch.common.compress.CompressedXContent;
import org.opensearch.common.settings.ClusterSettings;
import org.opensearch.common.settings.IndexScopedSettings;
import org.opensearch.common.settings.Setting;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.unit.TimeValue;
import org.opensearch.common.util.BigArrays;
import org.opensearch.common.util.FeatureFlags;
import org.opensearch.common.xcontent.XContentFactory;
import org.opensearch.core.index.Index;
import org.opensearch.core.xcontent.NamedXContentRegistry;
Expand All @@ -82,6 +84,7 @@
import org.opensearch.indices.SystemIndexDescriptor;
import org.opensearch.indices.SystemIndices;
import org.opensearch.indices.replication.common.ReplicationType;
import org.opensearch.node.remotestore.RemoteStoreNodeService;
import org.opensearch.snapshots.EmptySnapshotsInfoService;
import org.opensearch.test.ClusterServiceUtils;
import org.opensearch.test.OpenSearchTestCase;
Expand Down Expand Up @@ -136,6 +139,7 @@
import static org.opensearch.cluster.metadata.MetadataCreateIndexService.getIndexNumberOfRoutingShards;
import static org.opensearch.cluster.metadata.MetadataCreateIndexService.parseV1Mappings;
import static org.opensearch.cluster.metadata.MetadataCreateIndexService.resolveAndValidateAliases;
import static org.opensearch.common.util.FeatureFlags.REMOTE_STORE_MIGRATION_EXPERIMENTAL;
import static org.opensearch.index.IndexModule.INDEX_STORE_TYPE_SETTING;
import static org.opensearch.index.IndexSettings.INDEX_REFRESH_INTERVAL_SETTING;
import static org.opensearch.index.IndexSettings.INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING;
Expand All @@ -150,6 +154,8 @@
import static org.opensearch.node.Node.NODE_ATTRIBUTES;
import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY;
import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY;
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.Matchers.containsString;
import static org.hamcrest.Matchers.endsWith;
import static org.hamcrest.Matchers.equalTo;
Expand Down Expand Up @@ -1551,6 +1557,132 @@ public void testRemoteStoreOverrideTranslogRepoIndexSettings() {
}));
}

public void testNewIndexIsRemoteStoreBackedForRemoteStoreDirectionAndMixedMode() {
FeatureFlags.initializeFeatureFlags(Settings.builder().put(REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true").build());

// non-remote cluster manager node
DiscoveryNode nonRemoteClusterManagerNode = new DiscoveryNode(
UUIDs.base64UUID(),
buildNewFakeTransportAddress(),
emptyMap(),
singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE),
Version.CURRENT
);

DiscoveryNodes discoveryNodes = DiscoveryNodes.builder()
.add(nonRemoteClusterManagerNode)
.localNodeId(nonRemoteClusterManagerNode.getId())
.build();

ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT).nodes(discoveryNodes).build();

ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);

request = new CreateIndexClusterStateUpdateRequest("create index", "test", "test");

Settings indexSettings = aggregateIndexSettings(
clusterState,
request,
Settings.EMPTY,
null,
Settings.EMPTY,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet(),
clusterSettings
);

verifyRemoteStoreIndexSettings(
indexSettings,
null,
null,
null,
ReplicationType.DOCUMENT.toString(),
IndexSettings.DEFAULT_REMOTE_TRANSLOG_BUFFER_INTERVAL
);

// remote data node
Map<String, String> attributes = new HashMap<>();
attributes.put(REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY, "my-segment-repo-1");
attributes.put(REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY, "my-translog-repo-1");
DiscoveryNode remoteDataNode = new DiscoveryNode(
UUIDs.base64UUID(),
buildNewFakeTransportAddress(),
attributes,
Set.of(DiscoveryNodeRole.INGEST_ROLE, DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE, DiscoveryNodeRole.DATA_ROLE),
Version.CURRENT
);

discoveryNodes = DiscoveryNodes.builder(discoveryNodes).add(remoteDataNode).localNodeId(remoteDataNode.getId()).build();

clusterState = ClusterState.builder(ClusterName.DEFAULT).nodes(discoveryNodes).build();

Settings remoteStoreMigrationSettings = Settings.builder()
.put(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey(), RemoteStoreNodeService.CompatibilityMode.MIXED)
.put(MIGRATION_DIRECTION_SETTING.getKey(), RemoteStoreNodeService.Direction.REMOTE_STORE)
.build();

clusterSettings = new ClusterSettings(remoteStoreMigrationSettings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);

indexSettings = aggregateIndexSettings(
clusterState,
request,
Settings.EMPTY,
null,
Settings.EMPTY,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet(),
clusterSettings
);

verifyRemoteStoreIndexSettings(
indexSettings,
"true",
"my-segment-repo-1",
"my-translog-repo-1",
ReplicationType.SEGMENT.toString(),
IndexSettings.DEFAULT_REMOTE_TRANSLOG_BUFFER_INTERVAL
);

Map<String, String> missingTranslogAttribute = Map.of(REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY, "my-segment-repo-1");

DiscoveryNodes finalDiscoveryNodes = DiscoveryNodes.builder()
.add(nonRemoteClusterManagerNode)
.add(
new DiscoveryNode(
UUIDs.base64UUID(),
buildNewFakeTransportAddress(),
missingTranslogAttribute,
Set.of(DiscoveryNodeRole.INGEST_ROLE, DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE, DiscoveryNodeRole.DATA_ROLE),
Version.CURRENT
)
)
.build();

ClusterState finalClusterState = ClusterState.builder(ClusterName.DEFAULT).nodes(finalDiscoveryNodes).build();
ClusterSettings finalClusterSettings = clusterSettings;

final IndexCreationException error = expectThrows(IndexCreationException.class, () -> {
aggregateIndexSettings(
finalClusterState,
request,
Settings.EMPTY,
null,
Settings.EMPTY,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet(),
finalClusterSettings
);
});

assertThat(
error.getCause().getMessage(),
containsString("Cluster is migrating to remote store but no remote node found, failing index creation")
);
}

public void testBuildIndexMetadata() {
IndexMetadata sourceIndexMetadata = IndexMetadata.builder("parent")
.settings(Settings.builder().put("index.version.created", Version.CURRENT).build())
Expand Down

0 comments on commit 155f5b3

Please sign in to comment.