Skip to content

Commit

Permalink
Refactor: rename settings, change logic to provide simplicity in over…
Browse files Browse the repository at this point in the history
…riding settings

Signed-off-by: Varun Bansal <bansvaru@amazon.com>
  • Loading branch information
linuxpi committed Apr 11, 2023
1 parent d611076 commit 3151753
Show file tree
Hide file tree
Showing 8 changed files with 354 additions and 86 deletions.
9 changes: 6 additions & 3 deletions distribution/src/config/opensearch.yml
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,16 @@ ${path.logs}
#
# ---------------------------------- Remote Store -----------------------------------
# Controls whether cluster imposes index creation only with remote store enabled
# cluster.remote_store.enabled.force: true
# cluster.remote_store.enabled: true
#
# Repository to use for segment upload while enforcing remote store for an index
# cluster.remote_store.repository.segment.default: my-repo-1
# cluster.remote_store.repository: my-repo-1
#
# Controls whether cluster imposes index creation only with translog remote store enabled
# cluster.remote_store.translog.enabled: true
#
# Repository to use for translog upload while enforcing remote store for an index
# cluster.remote_store.repository.translog.default: my-repo-1
# cluster.remote_store.translog.repository: my-repo-1
#
# ---------------------------------- Experimental Features -----------------------------------
#
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/*
* 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.remotestore;

import org.opensearch.action.admin.indices.get.GetIndexRequest;
import org.opensearch.action.admin.indices.get.GetIndexResponse;
import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.common.settings.Settings;
import org.opensearch.indices.replication.common.ReplicationType;
import org.opensearch.test.OpenSearchIntegTestCase;

import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_REPOSITORY;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE;
import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_STORE_ENABLED_SETTING;
import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_STORE_REPOSITORY_SETTING;
import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_TRANSLOG_REPOSITORY_SETTING;
import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_TRANSLOG_STORE_ENABLED_SETTING;
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;

@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST)
public class CreateRemoteIndexTranslogDisabledIT extends CreateRemoteIndexIT {

@Override
protected Settings nodeSettings(int nodeOriginal) {
Settings settings = super.nodeSettings(nodeOriginal);
Settings.Builder builder = Settings.builder()
.put(CLUSTER_REMOTE_STORE_ENABLED_SETTING.getKey(), true)
.put(CLUSTER_REMOTE_STORE_REPOSITORY_SETTING.getKey(), "my-segment-repo-1")
.put(settings);
builder.remove(CLUSTER_REMOTE_TRANSLOG_STORE_ENABLED_SETTING.getKey());
builder.remove(CLUSTER_REMOTE_TRANSLOG_REPOSITORY_SETTING.getKey());
return builder.build();
}

public void testRemoteStoreEnabledByUserWithRemoteRepo() throws Exception {
final int numReplicas = internalCluster().numDataNodes();
Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 3)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas)
.put(SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT)
.put(SETTING_REMOTE_STORE_ENABLED, true)
.put(SETTING_REMOTE_STORE_REPOSITORY, "my-custom-repo")
.build();

assertAcked(client().admin().indices().prepareCreate("test-idx-1").setSettings(settings).get());
GetIndexResponse getIndexResponse = client().admin()
.indices()
.getIndex(new GetIndexRequest().indices("test-idx-1").includeDefaults(true))
.get();
Settings indexSettings = getIndexResponse.settings().get("test-idx-1");
verifyRemoteStoreIndexSettings(indexSettings, "true", "my-custom-repo", null, null, ReplicationType.SEGMENT.toString(), null);
}

public void testDefaultRemoteStoreNoUserOverride() throws Exception {
final int numReplicas = internalCluster().numDataNodes();
Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 3)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas)
.build();
assertAcked(client().admin().indices().prepareCreate("test-idx-1").setSettings(settings).get());
GetIndexResponse getIndexResponse = client().admin()
.indices()
.getIndex(new GetIndexRequest().indices("test-idx-1").includeDefaults(true))
.get();
Settings indexSettings = getIndexResponse.settings().get("test-idx-1");
verifyRemoteStoreIndexSettings(indexSettings, "true", "my-segment-repo-1", null, null, ReplicationType.SEGMENT.toString(), null);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@
import org.opensearch.common.settings.IndexScopedSettings;
import org.opensearch.common.settings.Setting;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.FeatureFlags;
import org.opensearch.core.xcontent.NamedXContentRegistry;
import org.opensearch.common.xcontent.XContentHelper;
import org.opensearch.env.Environment;
Expand Down Expand Up @@ -122,6 +121,8 @@
import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING;
import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING;
import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING;
import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_REMOTE_STORE_REPOSITORY_SETTING;
import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_REMOTE_TRANSLOG_REPOSITORY_SETTING;
import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_REMOTE_TRANSLOG_STORE_ENABLED_SETTING;
import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_REPLICATION_TYPE_SETTING;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_AUTO_EXPAND_REPLICAS;
Expand All @@ -135,9 +136,10 @@
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE;
import static org.opensearch.cluster.metadata.Metadata.DEFAULT_REPLICA_COUNT_SETTING;
import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_STORE_DEFAULT_SEGMENT_REPO_SETTING;
import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_STORE_DEFAULT_TRANSLOG_REPO_SETTING;
import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_STORE_INDEX_FORCE_SETTING;
import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_STORE_REPOSITORY_SETTING;
import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_TRANSLOG_REPOSITORY_SETTING;
import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_STORE_ENABLED_SETTING;
import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_TRANSLOG_STORE_ENABLED_SETTING;

/**
* Service responsible for submitting create index requests
Expand Down Expand Up @@ -929,34 +931,45 @@ static Settings aggregateIndexSettings(
* @param clusterSettings cluster level settings
*/
private static void updateRemoteStoreSettings(Settings.Builder settingsBuilder, Settings requestSettings, Settings clusterSettings) {
if (CLUSTER_REMOTE_STORE_INDEX_FORCE_SETTING.get(clusterSettings)) {
if (CLUSTER_REMOTE_STORE_ENABLED_SETTING.get(clusterSettings)) {
// Verify if we can create a remote store based index based on user provided settings
if ((INDEX_REPLICATION_TYPE_SETTING.exists(requestSettings)
&& !INDEX_REPLICATION_TYPE_SETTING.get(requestSettings).equals(ReplicationType.SEGMENT))
|| (INDEX_REMOTE_STORE_ENABLED_SETTING.exists(requestSettings)
&& !INDEX_REMOTE_STORE_ENABLED_SETTING.get(requestSettings))) {
if (!canCreateRemoteStoreIndex(requestSettings)) {
return;
}
String remoteStoreRepo = requestSettings.get(SETTING_REMOTE_STORE_REPOSITORY);
if (remoteStoreRepo == null) {
remoteStoreRepo = CLUSTER_REMOTE_STORE_DEFAULT_SEGMENT_REPO_SETTING.get(clusterSettings);

settingsBuilder.put(SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT).put(SETTING_REMOTE_STORE_ENABLED, true);

String remoteStoreRepo;
if (Objects.equals(requestSettings.get(INDEX_REMOTE_STORE_ENABLED_SETTING.getKey()), "true")) {
remoteStoreRepo = requestSettings.get(INDEX_REMOTE_STORE_REPOSITORY_SETTING.getKey());
} else {
remoteStoreRepo = CLUSTER_REMOTE_STORE_REPOSITORY_SETTING.get(clusterSettings);
}
settingsBuilder.put(SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT)
.put(SETTING_REMOTE_STORE_ENABLED, true)
.put(SETTING_REMOTE_STORE_REPOSITORY, remoteStoreRepo);

if (!Objects.equals(requestSettings.get(INDEX_REMOTE_TRANSLOG_STORE_ENABLED_SETTING.getKey()), "false")) {
String translogStoreRepo = requestSettings.get(SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY);
;
if (translogStoreRepo == null) {
translogStoreRepo = CLUSTER_REMOTE_STORE_DEFAULT_TRANSLOG_REPO_SETTING.get(clusterSettings);
settingsBuilder.put(SETTING_REMOTE_STORE_REPOSITORY, remoteStoreRepo);

boolean translogStoreEnabled = false;
String translogStoreRepo = null;
if (Objects.equals(requestSettings.get(INDEX_REMOTE_TRANSLOG_STORE_ENABLED_SETTING.getKey()), "true")) {
translogStoreEnabled = true;
translogStoreRepo = requestSettings.get(INDEX_REMOTE_TRANSLOG_REPOSITORY_SETTING.getKey());
} else if (!Objects.equals(requestSettings.get(INDEX_REMOTE_TRANSLOG_STORE_ENABLED_SETTING.getKey()), "false")
&& CLUSTER_REMOTE_TRANSLOG_STORE_ENABLED_SETTING.get(clusterSettings)) {
translogStoreEnabled = true;
translogStoreRepo = CLUSTER_REMOTE_TRANSLOG_REPOSITORY_SETTING.get(clusterSettings);
}
settingsBuilder.put(SETTING_REMOTE_TRANSLOG_STORE_ENABLED, true)
if (translogStoreEnabled) {
settingsBuilder.put(SETTING_REMOTE_TRANSLOG_STORE_ENABLED, translogStoreEnabled)
.put(SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, translogStoreRepo);
}
}
}

private static boolean canCreateRemoteStoreIndex(Settings requestSettings) {
return (!INDEX_REPLICATION_TYPE_SETTING.exists(requestSettings)
|| INDEX_REPLICATION_TYPE_SETTING.get(requestSettings).equals(ReplicationType.SEGMENT))
&& (!INDEX_REMOTE_STORE_ENABLED_SETTING.exists(requestSettings) || INDEX_REMOTE_STORE_ENABLED_SETTING.get(requestSettings));
}

public static void validateStoreTypeSettings(Settings settings) {
// deprecate simplefs store type:
if (IndexModule.Type.SIMPLEFS.match(IndexModule.INDEX_STORE_TYPE_SETTING.get(settings))) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -643,16 +643,17 @@ public void apply(Settings value, Settings current, Settings previous) {
* is ready for production release, the feature flag can be removed, and the
* setting should be moved to {@link #BUILT_IN_CLUSTER_SETTINGS}.
*/
public static final Map<String, List<Setting>> FEATURE_FLAGGED_CLUSTER_SETTINGS = Map.of(
FeatureFlags.SEARCHABLE_SNAPSHOT,
public static final Map<List<String>, List<Setting>> FEATURE_FLAGGED_CLUSTER_SETTINGS = Map.of(
List.of(FeatureFlags.SEARCHABLE_SNAPSHOT),
List.of(Node.NODE_SEARCH_CACHE_SIZE_SETTING),
FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL,
List.of(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL),
List.of(IndicesService.CLUSTER_REPLICATION_TYPE_SETTING),
FeatureFlags.REMOTE_STORE,
List.of(FeatureFlags.REMOTE_STORE, FeatureFlags.REPLICATION_TYPE),
List.of(
IndicesService.CLUSTER_REMOTE_STORE_INDEX_FORCE_SETTING,
IndicesService.CLUSTER_REMOTE_STORE_DEFAULT_SEGMENT_REPO_SETTING,
IndicesService.CLUSTER_REMOTE_STORE_DEFAULT_TRANSLOG_REPO_SETTING
IndicesService.CLUSTER_REMOTE_STORE_ENABLED_SETTING,
IndicesService.CLUSTER_REMOTE_STORE_REPOSITORY_SETTING,
IndicesService.CLUSTER_REMOTE_TRANSLOG_STORE_ENABLED_SETTING,
IndicesService.CLUSTER_REMOTE_TRANSLOG_REPOSITORY_SETTING
)
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,6 @@
import java.util.stream.Collectors;
import java.util.stream.IntStream;

import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_STORE_DEFAULT_SEGMENT_REPO_SETTING;
import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_STORE_DEFAULT_TRANSLOG_REPO_SETTING;
import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_STORE_INDEX_FORCE_SETTING;

/**
* A module that binds the provided settings to the {@link Settings} interface.
*
Expand Down Expand Up @@ -95,8 +91,8 @@ public SettingsModule(
registerSetting(setting);
}

for (Map.Entry<String, List<Setting>> featureFlaggedSetting : ClusterSettings.FEATURE_FLAGGED_CLUSTER_SETTINGS.entrySet()) {
if (FeatureFlags.isEnabled(featureFlaggedSetting.getKey())) {
for (Map.Entry<List<String>, List<Setting>> featureFlaggedSetting : ClusterSettings.FEATURE_FLAGGED_CLUSTER_SETTINGS.entrySet()) {
if (featureFlaggedSetting.getKey().stream().allMatch(FeatureFlags::isEnabled)) {
featureFlaggedSetting.getValue().forEach(this::registerSetting);
}
}
Expand Down
22 changes: 16 additions & 6 deletions server/src/main/java/org/opensearch/indices/IndicesService.java
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,8 @@ public class IndicesService extends AbstractLifecycleComponent
/**
* Used to specify if all indexes are to create with remote store enabled by default
*/
public static final Setting<Boolean> CLUSTER_REMOTE_STORE_INDEX_FORCE_SETTING = Setting.boolSetting(
"cluster.remote_store.enabled.force",
public static final Setting<Boolean> CLUSTER_REMOTE_STORE_ENABLED_SETTING = Setting.boolSetting(
"cluster.remote_store.enabled",
false,
Property.NodeScope,
Property.Final
Expand All @@ -257,18 +257,28 @@ public class IndicesService extends AbstractLifecycleComponent
/**
* Used to specify default repo to use for segment upload for remote store backed indices
*/
public static final Setting<String> CLUSTER_REMOTE_STORE_DEFAULT_SEGMENT_REPO_SETTING = Setting.simpleString(
"cluster.remote_store.repository.segment.default",
public static final Setting<String> CLUSTER_REMOTE_STORE_REPOSITORY_SETTING = Setting.simpleString(
"cluster.remote_store.repository",
"",
Property.NodeScope,
Property.Final
);

/**
* Used to specify if all indexes are to create with translog remote store enabled by default
*/
public static final Setting<Boolean> CLUSTER_REMOTE_TRANSLOG_STORE_ENABLED_SETTING = Setting.boolSetting(
"cluster.remote_store.translog.enabled",
false,
Property.NodeScope,
Property.Final
);

/**
* Used to specify default repo to use for translog upload for remote store backed indices
*/
public static final Setting<String> CLUSTER_REMOTE_STORE_DEFAULT_TRANSLOG_REPO_SETTING = Setting.simpleString(
"cluster.remote_store.repository.translog.default",
public static final Setting<String> CLUSTER_REMOTE_TRANSLOG_REPOSITORY_SETTING = Setting.simpleString(
"cluster.remote_store.translog.repository",
"",
Property.NodeScope,
Property.Final
Expand Down
Loading

0 comments on commit 3151753

Please sign in to comment.