Skip to content

Commit 695fbde

Browse files
ltaragiltaragi
authored
Add validation while updating CompatibilityMode setting (opensearch-project#13080)
Signed-off-by: Lakshya Taragi <lakshya.taragi@gmail.com>
1 parent 5e72e1d commit 695fbde

File tree

4 files changed

+308
-3
lines changed

4 files changed

+308
-3
lines changed

‎server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationSettingsUpdateIT.java

+34
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,13 @@
1212
import org.opensearch.client.Client;
1313
import org.opensearch.cluster.metadata.IndexMetadata;
1414
import org.opensearch.common.settings.Settings;
15+
import org.opensearch.common.settings.SettingsException;
1516
import org.opensearch.core.rest.RestStatus;
1617
import org.opensearch.index.IndexSettings;
1718
import org.opensearch.indices.replication.common.ReplicationType;
1819
import org.opensearch.snapshots.SnapshotInfo;
1920
import org.opensearch.snapshots.SnapshotState;
21+
import org.opensearch.test.InternalTestCluster;
2022
import org.opensearch.test.OpenSearchIntegTestCase;
2123

2224
import java.nio.file.Path;
@@ -28,6 +30,7 @@
2830
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE;
2931
import static org.opensearch.index.IndexSettings.INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING;
3032
import static org.opensearch.node.remotestore.RemoteStoreNodeService.CompatibilityMode.MIXED;
33+
import static org.opensearch.node.remotestore.RemoteStoreNodeService.CompatibilityMode.STRICT;
3134
import static org.opensearch.node.remotestore.RemoteStoreNodeService.Direction.REMOTE_STORE;
3235
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
3336

@@ -140,6 +143,37 @@ public void testNewRestoredIndexIsRemoteStoreBackedForRemoteStoreDirectionAndMix
140143
assertRemoteStoreBackedIndex(restoredIndexName2);
141144
}
142145

146+
// compatibility mode setting test
147+
148+
public void testSwitchToStrictMode() throws Exception {
149+
logger.info(" --> initialize cluster");
150+
initializeCluster(false);
151+
152+
logger.info(" --> create a mixed mode cluster");
153+
setClusterMode(MIXED.mode);
154+
addRemote = true;
155+
String remoteNodeName = internalCluster().startNode();
156+
addRemote = false;
157+
String nonRemoteNodeName = internalCluster().startNode();
158+
internalCluster().validateClusterFormed();
159+
assertNodeInCluster(remoteNodeName);
160+
assertNodeInCluster(nonRemoteNodeName);
161+
162+
logger.info(" --> attempt switching to strict mode");
163+
SettingsException exception = assertThrows(SettingsException.class, () -> setClusterMode(STRICT.mode));
164+
assertEquals(
165+
"can not switch to STRICT compatibility mode when the cluster contains both remote and non-remote nodes",
166+
exception.getMessage()
167+
);
168+
169+
logger.info(" --> stop remote node so that cluster had only non-remote nodes");
170+
internalCluster().stopRandomNode(InternalTestCluster.nameFilter(remoteNodeName));
171+
ensureStableCluster(2);
172+
173+
logger.info(" --> attempt switching to strict mode");
174+
setClusterMode(STRICT.mode);
175+
}
176+
143177
// restore indices from a snapshot
144178
private void restoreSnapshot(String snapshotRepoName, String snapshotName, String restoredIndexName) {
145179
RestoreSnapshotResponse restoreSnapshotResponse = client.admin()

‎server/src/main/java/org/opensearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java

+53
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
4646
import org.opensearch.cluster.metadata.Metadata;
4747
import org.opensearch.cluster.node.DiscoveryNode;
48+
import org.opensearch.cluster.node.DiscoveryNodes;
4849
import org.opensearch.cluster.routing.allocation.AllocationService;
4950
import org.opensearch.cluster.service.ClusterManagerTaskKeys;
5051
import org.opensearch.cluster.service.ClusterManagerTaskThrottler;
@@ -53,12 +54,18 @@
5354
import org.opensearch.common.Priority;
5455
import org.opensearch.common.inject.Inject;
5556
import org.opensearch.common.settings.ClusterSettings;
57+
import org.opensearch.common.settings.Settings;
58+
import org.opensearch.common.settings.SettingsException;
5659
import org.opensearch.core.action.ActionListener;
5760
import org.opensearch.core.common.io.stream.StreamInput;
61+
import org.opensearch.node.remotestore.RemoteStoreNodeService;
5862
import org.opensearch.threadpool.ThreadPool;
5963
import org.opensearch.transport.TransportService;
6064

6165
import java.io.IOException;
66+
import java.util.Locale;
67+
import java.util.Set;
68+
import java.util.stream.Collectors;
6269

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

252259
@Override
253260
public ClusterState execute(final ClusterState currentState) {
261+
validateCompatibilityModeSettingRequest(request, state);
254262
final ClusterState clusterState = updater.updateSettings(
255263
currentState,
256264
clusterSettings.upgradeSettings(request.transientSettings()),
@@ -264,4 +272,49 @@ public ClusterState execute(final ClusterState currentState) {
264272
);
265273
}
266274

275+
/**
276+
* Runs various checks associated with changing cluster compatibility mode
277+
* @param request cluster settings update request, for settings to be updated and new values
278+
* @param clusterState current state of cluster, for information on nodes
279+
*/
280+
public void validateCompatibilityModeSettingRequest(ClusterUpdateSettingsRequest request, ClusterState clusterState) {
281+
Settings settings = Settings.builder().put(request.persistentSettings()).put(request.transientSettings()).build();
282+
if (RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING.exists(settings)) {
283+
String value = settings.get(RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey()).toLowerCase(Locale.ROOT);
284+
validateAllNodesOfSameVersion(clusterState.nodes());
285+
if (value.equals(RemoteStoreNodeService.CompatibilityMode.STRICT.mode)) {
286+
validateAllNodesOfSameType(clusterState.nodes());
287+
}
288+
}
289+
}
290+
291+
/**
292+
* Verifies that while trying to change the compatibility mode, all nodes must have the same version.
293+
* If not, it throws SettingsException error
294+
* @param discoveryNodes current discovery nodes in the cluster
295+
*/
296+
private void validateAllNodesOfSameVersion(DiscoveryNodes discoveryNodes) {
297+
if (discoveryNodes.getMaxNodeVersion().equals(discoveryNodes.getMinNodeVersion()) == false) {
298+
throw new SettingsException("can not change the compatibility mode when all the nodes in cluster are not of the same version");
299+
}
300+
}
301+
302+
/**
303+
* Verifies that while trying to switch to STRICT compatibility mode, all nodes must be of the
304+
* same type (all remote or all non-remote). If not, it throws SettingsException error
305+
* @param discoveryNodes current discovery nodes in the cluster
306+
*/
307+
private void validateAllNodesOfSameType(DiscoveryNodes discoveryNodes) {
308+
Set<Boolean> nodeTypes = discoveryNodes.getNodes()
309+
.values()
310+
.stream()
311+
.map(DiscoveryNode::isRemoteStoreNode)
312+
.collect(Collectors.toSet());
313+
if (nodeTypes.size() != 1) {
314+
throw new SettingsException(
315+
"can not switch to STRICT compatibility mode when the cluster contains both remote and non-remote nodes"
316+
);
317+
}
318+
}
319+
267320
}

‎server/src/main/java/org/opensearch/snapshots/RestoreService.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -680,9 +680,9 @@ private Settings getOverrideSettingsInternal() {
680680
// We will use whatever replication strategy provided by user or from snapshot metadata unless
681681
// cluster is remote store enabled or user have restricted a specific replication type in the
682682
// cluster. If cluster is undergoing remote store migration, replication strategy is strictly SEGMENT type
683-
if (RemoteStoreNodeAttribute.isRemoteStoreAttributePresent(clusterService.getSettings()) == true
684-
|| clusterSettings.get(IndicesService.CLUSTER_INDEX_RESTRICT_REPLICATION_TYPE_SETTING) == true
685-
|| RemoteStoreNodeService.isMigratingToRemoteStore(clusterSettings) == true) {
683+
if (RemoteStoreNodeAttribute.isRemoteStoreAttributePresent(clusterService.getSettings())
684+
|| clusterSettings.get(IndicesService.CLUSTER_INDEX_RESTRICT_REPLICATION_TYPE_SETTING)
685+
|| RemoteStoreNodeService.isMigratingToRemoteStore(clusterSettings)) {
686686
MetadataCreateIndexService.updateReplicationStrategy(
687687
settingsBuilder,
688688
request.indexSettings(),

0 commit comments

Comments
 (0)