Skip to content

Commit

Permalink
Add validation while updating CompatibilityMode setting (#13080)
Browse files Browse the repository at this point in the history
Signed-off-by: Lakshya Taragi <[email protected]>
  • Loading branch information
ltaragi authored Apr 16, 2024
1 parent 5e72e1d commit 695fbde
Show file tree
Hide file tree
Showing 4 changed files with 308 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@
import org.opensearch.client.Client;
import org.opensearch.cluster.metadata.IndexMetadata;
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.snapshots.SnapshotInfo;
import org.opensearch.snapshots.SnapshotState;
import org.opensearch.test.InternalTestCluster;
import org.opensearch.test.OpenSearchIntegTestCase;

import java.nio.file.Path;
Expand All @@ -28,6 +30,7 @@
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE;
import static org.opensearch.index.IndexSettings.INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING;
import static org.opensearch.node.remotestore.RemoteStoreNodeService.CompatibilityMode.MIXED;
import static org.opensearch.node.remotestore.RemoteStoreNodeService.CompatibilityMode.STRICT;
import static org.opensearch.node.remotestore.RemoteStoreNodeService.Direction.REMOTE_STORE;
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;

Expand Down Expand Up @@ -140,6 +143,37 @@ public void testNewRestoredIndexIsRemoteStoreBackedForRemoteStoreDirectionAndMix
assertRemoteStoreBackedIndex(restoredIndexName2);
}

// 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 so that cluster had only non-remote nodes");
internalCluster().stopRandomNode(InternalTestCluster.nameFilter(remoteNodeName));
ensureStableCluster(2);

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

// restore indices from a snapshot
private void restoreSnapshot(String snapshotRepoName, String snapshotName, String restoredIndexName) {
RestoreSnapshotResponse restoreSnapshotResponse = client.admin()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
import org.opensearch.cluster.metadata.Metadata;
import org.opensearch.cluster.node.DiscoveryNode;
import org.opensearch.cluster.node.DiscoveryNodes;
import org.opensearch.cluster.routing.allocation.AllocationService;
import org.opensearch.cluster.service.ClusterManagerTaskKeys;
import org.opensearch.cluster.service.ClusterManagerTaskThrottler;
Expand All @@ -53,12 +54,18 @@
import org.opensearch.common.Priority;
import org.opensearch.common.inject.Inject;
import org.opensearch.common.settings.ClusterSettings;
import org.opensearch.common.settings.Settings;
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.Locale;
import java.util.Set;
import java.util.stream.Collectors;

/**
* Transport action for updating cluster settings
Expand Down Expand Up @@ -251,6 +258,7 @@ public void onFailure(String source, Exception e) {

@Override
public ClusterState execute(final ClusterState currentState) {
validateCompatibilityModeSettingRequest(request, state);
final ClusterState clusterState = updater.updateSettings(
currentState,
clusterSettings.upgradeSettings(request.transientSettings()),
Expand All @@ -264,4 +272,49 @@ public ClusterState execute(final ClusterState currentState) {
);
}

/**
* Runs various checks associated with changing cluster compatibility mode
* @param request cluster settings update request, for settings to be updated and new values
* @param clusterState current state of cluster, for information on nodes
*/
public void validateCompatibilityModeSettingRequest(ClusterUpdateSettingsRequest request, ClusterState clusterState) {
Settings settings = Settings.builder().put(request.persistentSettings()).put(request.transientSettings()).build();
if (RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING.exists(settings)) {
String value = settings.get(RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey()).toLowerCase(Locale.ROOT);
validateAllNodesOfSameVersion(clusterState.nodes());
if (value.equals(RemoteStoreNodeService.CompatibilityMode.STRICT.mode)) {
validateAllNodesOfSameType(clusterState.nodes());
}
}
}

/**
* Verifies that while trying to change the compatibility mode, all nodes must have the same version.
* If not, it throws SettingsException error
* @param discoveryNodes current discovery nodes in the cluster
*/
private void validateAllNodesOfSameVersion(DiscoveryNodes discoveryNodes) {
if (discoveryNodes.getMaxNodeVersion().equals(discoveryNodes.getMinNodeVersion()) == false) {
throw new SettingsException("can not change the compatibility mode when all the nodes in cluster are not of the same version");
}
}

/**
* 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 discoveryNodes current discovery nodes in the cluster
*/
private void validateAllNodesOfSameType(DiscoveryNodes discoveryNodes) {
Set<Boolean> nodeTypes = discoveryNodes.getNodes()
.values()
.stream()
.map(DiscoveryNode::isRemoteStoreNode)
.collect(Collectors.toSet());
if (nodeTypes.size() != 1) {
throw new SettingsException(
"can not switch to STRICT compatibility mode when the cluster contains both remote and non-remote nodes"
);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -680,9 +680,9 @@ private Settings getOverrideSettingsInternal() {
// We will use whatever replication strategy provided by user or from snapshot metadata unless
// cluster is remote store enabled or user have restricted a specific replication type in the
// cluster. If cluster is undergoing remote store migration, replication strategy is strictly SEGMENT type
if (RemoteStoreNodeAttribute.isRemoteStoreAttributePresent(clusterService.getSettings()) == true
|| clusterSettings.get(IndicesService.CLUSTER_INDEX_RESTRICT_REPLICATION_TYPE_SETTING) == true
|| RemoteStoreNodeService.isMigratingToRemoteStore(clusterSettings) == true) {
if (RemoteStoreNodeAttribute.isRemoteStoreAttributePresent(clusterService.getSettings())
|| clusterSettings.get(IndicesService.CLUSTER_INDEX_RESTRICT_REPLICATION_TYPE_SETTING)
|| RemoteStoreNodeService.isMigratingToRemoteStore(clusterSettings)) {
MetadataCreateIndexService.updateReplicationStrategy(
settingsBuilder,
request.indexSettings(),
Expand Down
Loading

0 comments on commit 695fbde

Please sign in to comment.