Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Remote Store] Add support to create index with remote store by default #6932

Merged
merged 10 commits into from
Apr 12, 2023
13 changes: 13 additions & 0 deletions distribution/src/config/opensearch.yml
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,19 @@ ${path.logs}
#
#action.destructive_requires_name: true
#
# ---------------------------------- Remote Store -----------------------------------
# Controls whether cluster imposes index creation only with remote store enabled
# cluster.remote_store.enabled: true
#
# Repository to use for segment upload while enforcing remote store for an index
# cluster.remote_store.repository: my-repo-1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will it throw exception if remote repo is not setup and prevent index creation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. we have checks during index creation

#
# 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.translog.repository: my-repo-1
#
# ---------------------------------- Experimental Features -----------------------------------
#
# Gates the visibility of the index setting that allows changing of replication type.
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 @@ -381,7 +381,7 @@ private static void validateRemoteStoreSettingEnabled(final Map<Setting<?>, Obje
throw new IllegalArgumentException(
"Settings "
+ setting.getKey()
+ " can ont be set/enabled when "
+ " can only be set/enabled when "
+ INDEX_REMOTE_STORE_ENABLED_SETTING.getKey()
+ " is set to true"
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@
import org.opensearch.indices.InvalidIndexNameException;
import org.opensearch.indices.ShardLimitValidator;
import org.opensearch.indices.SystemIndices;
import org.opensearch.indices.replication.common.ReplicationType;
import org.opensearch.threadpool.ThreadPool;

import java.io.IOException;
Expand All @@ -104,6 +105,7 @@
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.atomic.AtomicInteger;
Expand All @@ -118,12 +120,26 @@
import static java.util.stream.Collectors.toList;
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;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_CREATION_DATE;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_INDEX_UUID;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS;
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_REMOTE_TRANSLOG_STORE_ENABLED;
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_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 @@ -874,6 +890,8 @@ static Settings aggregateIndexSettings(
indexSettingsBuilder.put(IndexMetadata.SETTING_INDEX_PROVIDED_NAME, request.getProvidedName());
indexSettingsBuilder.put(SETTING_INDEX_UUID, UUIDs.randomBase64UUID());

updateRemoteStoreSettings(indexSettingsBuilder, request.settings(), settings);

if (sourceMetadata != null) {
assert request.resizeType() != null;
prepareResizeIndexSettings(
Expand Down Expand Up @@ -906,6 +924,52 @@ static Settings aggregateIndexSettings(
return indexSettings;
}

/**
* Updates index settings to enable remote store by default based on cluster level settings
* @param settingsBuilder index settings builder to be updated with relevant settings
* @param requestSettings settings passed in during index create request
* @param clusterSettings cluster level settings
*/
private static void updateRemoteStoreSettings(Settings.Builder settingsBuilder, Settings requestSettings, Settings 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 (!canCreateRemoteStoreIndex(requestSettings)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: canCreateRemoteStoreIndex(requestSettings) == false (Applicable to all the places where we are using ! in if condition)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure

return;
}

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we adding Segment Replication setting explicitly here? Are we sure that replication type will always be segment at this point?
As I understand, Segment replication also has a cluster level setting. settingsBuilder should be updated by cluster and index level setting of segment replication prior to updateRemoteStoreSettings

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if there is an index level setting for segment replication then it would have been put by the user. We already have check for it.
For cluster level setting, if its set to SEGMENT, we dont have any problem. If its set to DOCUMENT then we are overriding and sending SEGMENT as replication.
I'll update to throw an error incase of cluster level settings being document


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_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);
}
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,11 +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(IndicesService.CLUSTER_REPLICATION_TYPE_SETTING)
List.of(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL),
List.of(IndicesService.CLUSTER_REPLICATION_TYPE_SETTING),
List.of(FeatureFlags.REMOTE_STORE, FeatureFlags.REPLICATION_TYPE),
List.of(
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 @@ -91,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
40 changes: 40 additions & 0 deletions server/src/main/java/org/opensearch/indices/IndicesService.java
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,46 @@ public class IndicesService extends AbstractLifecycleComponent
Property.Final
);

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

/**
* Used to specify default repo to use for segment upload for remote store backed indices
*/
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_TRANSLOG_REPOSITORY_SETTING = Setting.simpleString(
"cluster.remote_store.translog.repository",
"",
Property.NodeScope,
Property.Final
);

/**
* The node's settings.
*/
Expand Down
Loading