Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
Signed-off-by: Lakshya Taragi <[email protected]>
  • Loading branch information
ltaragi committed Apr 8, 2024
1 parent 8b05f9d commit 6d434aa
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -387,15 +387,25 @@ public void initializeCluster(boolean remoteClusterManager) {
client = internalCluster().client();
}

// assign settings to be updated randomly as persistent or transient
private static void randomlyAssignPersistentOrTransient(Settings.Builder settingsBuilder) {
boolean isPersistentSetting = randomBoolean();
if (isPersistentSetting) {
updateSettingsRequest.persistentSettings(settingsBuilder);
} else {
updateSettingsRequest.transientSettings(settingsBuilder);
}
}

// set the compatibility mode of cluster [strict, mixed]
public static void setClusterMode(String mode) {
updateSettingsRequest.persistentSettings(Settings.builder().put(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey(), mode));
randomlyAssignPersistentOrTransient(Settings.builder().put(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey(), mode));
assertAcked(internalCluster().client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
}

// set the migration direction for cluster [remote_store, docrep, none]
public static void setDirection(String direction) {
updateSettingsRequest.persistentSettings(Settings.builder().put(MIGRATION_DIRECTION_SETTING.getKey(), direction));
randomlyAssignPersistentOrTransient(Settings.builder().put(MIGRATION_DIRECTION_SETTING.getKey(), direction));
assertAcked(internalCluster().client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.opensearch.OpenSearchException;
import org.opensearch.Version;
import org.opensearch.action.support.ActionFilters;
import org.opensearch.action.support.clustermanager.TransportClusterManagerNodeAction;
import org.opensearch.cluster.AckedClusterStateUpdateTask;
Expand All @@ -46,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 @@ -54,6 +54,7 @@
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;
Expand All @@ -62,9 +63,7 @@
import org.opensearch.transport.TransportService;

import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import java.util.Locale;
import java.util.Set;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -145,7 +144,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 @@ -260,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 @@ -279,42 +278,39 @@ public ClusterState execute(final ClusterState currentState) {
* @param clusterState current state of cluster, for information on nodes
*/
public static 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())
.toLowerCase();
List<DiscoveryNode> discoveryNodeList = new ArrayList<>(clusterState.nodes().getNodes().values());
validateAllNodesOfSameVersion(discoveryNodeList);
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(discoveryNodeList);
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 discoveryNodeList list of the current discovery nodes in the cluster
* @param discoveryNodes current discovery nodes in the cluster
*/
private static void validateAllNodesOfSameVersion(List<DiscoveryNode> discoveryNodeList) {
Set<Version> versions = discoveryNodeList.stream().map(DiscoveryNode::getVersion).collect(Collectors.toSet());
if (versions.size() != 1) {
throw new SettingsException(
"can not change the compatibility mode when all the nodes in cluster are not of the same version. Present versions: "
+ versions
);
private static 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 discoveryNodeList list of the current discovery nodes in the cluster
* @param discoveryNodes current discovery nodes in the cluster
*/
private static void validateAllNodesOfSameType(List<DiscoveryNode> discoveryNodeList) {
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()) {
private static 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"
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -830,9 +830,7 @@ public void testDontAllowSwitchingCompatibilityModeForClusterWithMultipleVersion
);
assertThat(
exception.getMessage(),
containsString(
"can not change the compatibility mode when all the nodes in cluster are not of the same version. Present versions: ["
)
containsString("can not change the compatibility mode when all the nodes in cluster are not of the same version")
);

// changing compatibility mode when all nodes are of the same version
Expand Down

0 comments on commit 6d434aa

Please sign in to comment.