Skip to content

Commit

Permalink
Updated the validation for search only replica settings
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
vinaykpud committed Jan 22, 2025
1 parent 22b9ac0 commit bc88825
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 22 deletions.
15 changes: 7 additions & 8 deletions server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1051,9 +1051,6 @@ static Settings aggregateIndexSettings(

updateReplicationStrategy(indexSettingsBuilder, request.settings(), settings, combinedTemplateSettings, clusterSettings);
updateRemoteStoreSettings(indexSettingsBuilder, currentState, clusterSettings, settings, request.index());
if (FeatureFlags.isEnabled(FeatureFlags.READER_WRITER_SPLIT_EXPERIMENTAL_SETTING)) {
updateSearchOnlyReplicas(request.settings(), indexSettingsBuilder);
}

if (sourceMetadata != null) {
assert request.resizeType() != null;
Expand Down Expand Up @@ -1090,18 +1087,20 @@ static Settings aggregateIndexSettings(
validateRefreshIntervalSettings(request.settings(), clusterSettings);
validateTranslogFlushIntervalSettingsForCompositeIndex(request.settings(), clusterSettings);
validateTranslogDurabilitySettings(request.settings(), clusterSettings, settings);
if (FeatureFlags.isEnabled(FeatureFlags.READER_WRITER_SPLIT_EXPERIMENTAL_SETTING)) {
validateSearchOnlyReplicasSettings(indexSettings);
}
return indexSettings;
}

private static void updateSearchOnlyReplicas(Settings requestSettings, Settings.Builder builder) {
if (INDEX_NUMBER_OF_SEARCH_REPLICAS_SETTING.exists(builder) && builder.get(SETTING_NUMBER_OF_SEARCH_REPLICAS) != null) {
if (INDEX_NUMBER_OF_SEARCH_REPLICAS_SETTING.get(requestSettings) > 0
&& Boolean.parseBoolean(builder.get(SETTING_REMOTE_STORE_ENABLED)) == false) {
private static void validateSearchOnlyReplicasSettings(Settings indexSettings) {
if (INDEX_NUMBER_OF_SEARCH_REPLICAS_SETTING.exists(indexSettings) && indexSettings.get(SETTING_NUMBER_OF_SEARCH_REPLICAS) != null) {
if (INDEX_NUMBER_OF_SEARCH_REPLICAS_SETTING.get(indexSettings) > 0
&& Boolean.parseBoolean(indexSettings.get(SETTING_REMOTE_STORE_ENABLED)) == false) {
throw new IllegalArgumentException(
"To set " + SETTING_NUMBER_OF_SEARCH_REPLICAS + ", " + SETTING_REMOTE_STORE_ENABLED + " must be set to true"
);
}
builder.put(SETTING_NUMBER_OF_SEARCH_REPLICAS, INDEX_NUMBER_OF_SEARCH_REPLICAS_SETTING.get(requestSettings));
}
}

Expand Down
86 changes: 72 additions & 14 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 @@ -32,6 +32,9 @@

package org.opensearch.cluster.metadata;

import org.hamcrest.Matchers;
import org.junit.After;
import org.junit.Before;
import org.opensearch.ExceptionsHelper;
import org.opensearch.ResourceAlreadyExistsException;
import org.opensearch.Version;
Expand Down Expand Up @@ -98,14 +101,12 @@
import org.opensearch.repositories.blobstore.BlobStoreRepository;
import org.opensearch.snapshots.EmptySnapshotsInfoService;
import org.opensearch.test.ClusterServiceUtils;
import org.opensearch.test.FeatureFlagSetter;
import org.opensearch.test.OpenSearchTestCase;
import org.opensearch.test.VersionUtils;
import org.opensearch.test.gateway.TestGatewayAllocator;
import org.opensearch.threadpool.TestThreadPool;
import org.opensearch.threadpool.ThreadPool;
import org.hamcrest.Matchers;
import org.junit.After;
import org.junit.Before;

import java.io.IOException;
import java.util.ArrayList;
Expand Down Expand Up @@ -136,12 +137,24 @@
import static java.util.Collections.emptyMap;
import static java.util.Collections.singleton;
import static java.util.Collections.singletonList;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.endsWith;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasKey;
import static org.hamcrest.Matchers.hasValue;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
import static org.hamcrest.Matchers.startsWith;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING;
import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING;
import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING;
import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_READ_ONLY_BLOCK;
import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_REPLICATION_TYPE_SETTING;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SEARCH_REPLICAS;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_READ_ONLY;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY;
Expand All @@ -155,6 +168,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.READER_WRITER_SPLIT_EXPERIMENTAL;
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_MERGE_POLICY;
Expand All @@ -175,17 +189,6 @@
import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.getRemoteStoreTranslogRepo;
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;
import static org.hamcrest.Matchers.hasKey;
import static org.hamcrest.Matchers.hasValue;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
import static org.hamcrest.Matchers.startsWith;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class MetadataCreateIndexServiceTests extends OpenSearchTestCase {

Expand Down Expand Up @@ -1963,6 +1966,61 @@ public void testValidateTranslogRetentionSettings() {
);
}

public void testValidateSearchOnlySettingsWhenRemoteRepoNotConfiguredThrowsException() {
FeatureFlagSetter.set(READER_WRITER_SPLIT_EXPERIMENTAL);
request = new CreateIndexClusterStateUpdateRequest("create index", "test", "test");
final Settings.Builder requestSettings = Settings.builder();
requestSettings.put(SETTING_NUMBER_OF_SEARCH_REPLICAS, "1");
requestSettings.put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT);
request.settings(requestSettings.build());

IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, ()->
aggregateIndexSettings(
ClusterState.EMPTY_STATE,
request,
Settings.EMPTY,
null,
Settings.EMPTY,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet(),
clusterSettings
));

assertEquals("To set index.number_of_search_only_replicas, " +
"index.remote_store.enabled must be set to true",
exception.getMessage()
);
}

public void testValidateSearchOnlySettingsWhenRemoteRepoConfigured() {
FeatureFlagSetter.set(READER_WRITER_SPLIT_EXPERIMENTAL);
ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT)
.nodes(DiscoveryNodes.builder().add(getRemoteNode()).build())
.build();
Settings settings = Settings.builder().put(translogRepositoryNameAttributeKey, "my-translog-repo-1").build();

request = new CreateIndexClusterStateUpdateRequest("create index", "test", "test");
final Settings.Builder requestSettings = Settings.builder();
requestSettings.put(SETTING_NUMBER_OF_SEARCH_REPLICAS, "1");
requestSettings.put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT);
request.settings(requestSettings.build());

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

assertEquals("1", aggregatedIndexSettings.get(SETTING_NUMBER_OF_SEARCH_REPLICAS));
}

public void testIndexLifecycleNameSetting() {
// see: https://github.com/opensearch-project/OpenSearch/issues/1019
final Settings ilnSetting = Settings.builder().put("index.lifecycle.name", "dummy").build();
Expand Down

0 comments on commit bc88825

Please sign in to comment.