Skip to content

Commit

Permalink
Disabled auto_expand_replicas setting when search replica is enabled
Browse files Browse the repository at this point in the history
Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>
  • Loading branch information
vinaykpud committed Dec 28, 2024
1 parent b67cdf4 commit c6a1d64
Show file tree
Hide file tree
Showing 3 changed files with 148 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1469,6 +1469,7 @@ public void validateIndexSettings(String indexName, final Settings settings, fin
throws IndexCreationException {
List<String> validationErrors = getIndexSettingsValidationErrors(settings, forbidPrivateIndexSettings, indexName);
validateIndexReplicationTypeSettings(settings, clusterService.getClusterSettings()).ifPresent(validationErrors::add);
validateAutoExpandReplicaConflictInRequest(settings).ifPresent(validationErrors::add);
validateErrors(indexName, validationErrors);
}

Expand Down Expand Up @@ -1738,6 +1739,63 @@ public static int calculateNumRoutingShards(int numShards, Version indexVersionC
return numShards * 1 << numSplits;
}

/**
* Validates that the settings do not conflict with each other.
*
* @param requestSettings settings passed in during index create request
* @return the validation error if there is one, otherwise {@code Optional.empty()}
*/
public static Optional<String> validateAutoExpandReplicaConflictInRequest(Settings requestSettings) {
boolean hasRequestAutoExpand = AutoExpandReplicas.SETTING.get(requestSettings).isEnabled();
boolean hasRequestSearchReplica = INDEX_NUMBER_OF_SEARCH_REPLICAS_SETTING.get(requestSettings) > 0;
if (hasRequestAutoExpand && hasRequestSearchReplica) {
return Optional.of(
"Cannot set both ["
+ SETTING_AUTO_EXPAND_REPLICAS
+ "] and ["
+ SETTING_NUMBER_OF_SEARCH_REPLICAS
+ "]. These settings are mutually exclusive."
);
}
return Optional.empty();
}

/**
* Validates that the settings do not conflict with each other.
* Check conflicts between request and existing settings
* @param requestSettings
* @param indexSettings
* @return
*/
public static Optional<String> validateAutoExpandReplicaConflictWithIndex(Settings requestSettings, Settings indexSettings) {
boolean hasRequestAutoExpand = AutoExpandReplicas.SETTING.get(requestSettings).isEnabled();
boolean hasRequestSearchReplica = INDEX_NUMBER_OF_SEARCH_REPLICAS_SETTING.get(requestSettings) > 0;
boolean hasIndexAutoExpand = AutoExpandReplicas.SETTING.get(indexSettings).isEnabled();
boolean hasIndexSearchReplica = INDEX_NUMBER_OF_SEARCH_REPLICAS_SETTING.get(indexSettings) > 0;

if (hasRequestAutoExpand && hasIndexSearchReplica) {
return Optional.of(
"Cannot set ["
+ AutoExpandReplicas.SETTING.getKey()
+ "] because ["
+ INDEX_NUMBER_OF_SEARCH_REPLICAS_SETTING.getKey()
+ "] is set"
+ ". These settings are mutually exclusive."
);
}

if (hasRequestSearchReplica && hasIndexAutoExpand) {
return Optional.of(
"Cannot set ["
+ INDEX_NUMBER_OF_SEARCH_REPLICAS_SETTING.getKey()
+ "] because ["
+ AutoExpandReplicas.SETTING.getKey()
+ "] is configured. These settings are mutually exclusive."
);
}
return Optional.empty();
}

public static void validateTranslogRetentionSettings(Settings indexSettings) {
if (IndexSettings.INDEX_SOFT_DELETES_SETTING.get(indexSettings)) {
if (IndexSettings.INDEX_TRANSLOG_RETENTION_AGE_SETTING.exists(indexSettings)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@
import static org.opensearch.action.support.ContextPreservingActionListener.wrapPreservingContext;
import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_REPLICATION_TYPE_SETTING;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SEARCH_REPLICAS;
import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateAutoExpandReplicaConflictInRequest;
import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateAutoExpandReplicaConflictWithIndex;
import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateOverlap;
import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateRefreshIntervalSettings;
import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateTranslogDurabilitySettings;
Expand Down Expand Up @@ -229,6 +231,10 @@ public ClusterState execute(ClusterState currentState) {
metadata.getSettings()
).ifPresent(validationErrors::add);

validateAutoExpandReplicaConflictInRequest(normalizedSettings).ifPresent(validationErrors::add);
validateAutoExpandReplicaConflictWithIndex(normalizedSettings, metadata.getSettings()).ifPresent(
validationErrors::add
);
}

if (validationErrors.size() > 0) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/*
* 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.cluster.metadata;

import org.opensearch.common.settings.Settings;
import org.opensearch.test.OpenSearchTestCase;

/**
* AutoExpandReplica Disabled for Search Replica
*/
public class DisableAutoExpandReplicaForSearchReplicaTests extends OpenSearchTestCase {

public void testWhenAutoExpandReplicaAndSearchReplicaIsSetInTheRequest() {
Settings conflictRequestSettings = Settings.builder()
.put("index.number_of_search_only_replicas", 1)
.put("index.auto_expand_replicas", "0-all")
.build();

assertEquals(
"Cannot set both [index.auto_expand_replicas] and [index.number_of_search_only_replicas]. These settings are mutually exclusive.",
MetadataCreateIndexService.validateAutoExpandReplicaConflictInRequest(conflictRequestSettings).get()
);
}

public void testWhenOnlyAutoExpandReplicaIsSetInTheRequest() {
Settings reqestSettings = Settings.builder()
.put("index.number_of_search_only_replicas", 0)
.put("index.auto_expand_replicas", "0-all")
.build();

assertTrue(MetadataCreateIndexService.validateAutoExpandReplicaConflictInRequest(reqestSettings).isEmpty());
}

public void testWhenAutoExpandReplicaEnabledForTheIndex() {
Settings reqestSettings = Settings.builder().put("index.number_of_search_only_replicas", 1).build();

Settings indexSettings = Settings.builder().put("index.auto_expand_replicas", "0-all").build();

assertEquals(
"Cannot set [index.number_of_search_only_replicas] because [index.auto_expand_replicas] is configured. These settings are mutually exclusive.",
MetadataCreateIndexService.validateAutoExpandReplicaConflictWithIndex(reqestSettings, indexSettings).get()
);
}

public void testWhenSearchReplicaEnabledForTheIndex() {
Settings reqestSettings = Settings.builder().put("index.auto_expand_replicas", "0-all").build();

Settings indexSettings = Settings.builder().put("index.number_of_search_only_replicas", 1).build();

assertEquals(
"Cannot set [index.auto_expand_replicas] because [index.number_of_search_only_replicas] is set. These settings are mutually exclusive.",
MetadataCreateIndexService.validateAutoExpandReplicaConflictWithIndex(reqestSettings, indexSettings).get()
);
}

public void testWhenSearchReplicaSetZeroForTheIndex() {
Settings reqestSettings = Settings.builder().put("index.auto_expand_replicas", "0-all").build();

Settings indexSettings = Settings.builder().put("index.number_of_search_only_replicas", 0).build();

assertTrue(MetadataCreateIndexService.validateAutoExpandReplicaConflictWithIndex(reqestSettings, indexSettings).isEmpty());
}

public void testIfSearchReplicaEnabledEnablingAutoExpandReplicasAndDisablingSearchReplicasNotPossibleInSingleRequest() {
Settings reqestSettings = Settings.builder()
.put("index.auto_expand_replicas", "0-all")
.put("index.number_of_search_only_replicas", 0)
.build();

Settings indexSettings = Settings.builder().put("index.number_of_search_only_replicas", 1).build();

assertEquals(
"Cannot set [index.auto_expand_replicas] because [index.number_of_search_only_replicas] "
+ "is set. These settings are mutually exclusive.",
MetadataCreateIndexService.validateAutoExpandReplicaConflictWithIndex(reqestSettings, indexSettings).get()
);
}
}

0 comments on commit c6a1d64

Please sign in to comment.