Skip to content

Commit

Permalink
remove remote translog setting and restrict to remote only index crea…
Browse files Browse the repository at this point in the history
…tion

Signed-off-by: bansvaru <bansvaru@amazon.com>
  • Loading branch information
linuxpi committed Jul 18, 2023
1 parent ca74aac commit 4999d3e
Show file tree
Hide file tree
Showing 24 changed files with 92 additions and 589 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import org.opensearch.test.OpenSearchIntegTestCase;

import static org.hamcrest.Matchers.containsString;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_ENABLED;
import static org.opensearch.indices.IndicesService.CLUSTER_REPLICATION_TYPE_SETTING;
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;

Expand All @@ -33,31 +32,6 @@ protected Settings nodeSettings(int nodeOriginal) {
return builder.build();
}

@Override
public void testRemoteStoreTranslogDisabledByUser() throws Exception {
Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
.put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT)
.put(SETTING_REMOTE_TRANSLOG_STORE_ENABLED, false)
.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",
"false",
null,
ReplicationType.SEGMENT.toString(),
IndexSettings.DEFAULT_REMOTE_TRANSLOG_BUFFER_INTERVAL
);
}

@Override
public void testDefaultRemoteStoreNoUserOverride() throws Exception {
Settings settings = Settings.builder()
Expand Down Expand Up @@ -90,7 +64,6 @@ public void testDefaultRemoteStoreNoUserOverrideExceptReplicationTypeSegment() t
indexSettings,
"true",
"my-segment-repo-1",
"true",
"my-translog-repo-1",
ReplicationType.SEGMENT.toString(),
IndexSettings.DEFAULT_REMOTE_TRANSLOG_BUFFER_INTERVAL
Expand Down

Large diffs are not rendered by default.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ public Settings indexSettings() {
.put(IndexModule.INDEX_QUERY_CACHE_ENABLED_SETTING.getKey(), false)
.put(IndexMetadata.SETTING_REMOTE_STORE_ENABLED, true)
.put(IndexMetadata.SETTING_REMOTE_STORE_REPOSITORY, REPOSITORY_NAME)
.put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_ENABLED, true)
.put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, REPOSITORY_NAME)
.put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), "300s")
.put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ protected Settings remoteStoreIndexSettings(int numberOfReplicas, long totalFiel
protected Settings remoteTranslogIndexSettings(int numberOfReplicas, int numberOfShards) {
return Settings.builder()
.put(remoteStoreIndexSettings(numberOfReplicas, numberOfShards))
.put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_ENABLED, true)
.put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, REPOSITORY_NAME)
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ public Settings indexSettings() {
return Settings.builder()
.put(super.indexSettings())
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, shard_count)
.put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_ENABLED, true)
.put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, REPOSITORY_NAME)
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ public Settings indexSettings() {
.put(super.indexSettings())
.put(IndexMetadata.SETTING_REMOTE_STORE_ENABLED, true)
.put(IndexMetadata.SETTING_REMOTE_STORE_REPOSITORY, REPOSITORY_NAME)
.put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_ENABLED, true)
.put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, REPOSITORY_NAME)
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ public Settings indexSettings() {
.put(super.indexSettings())
.put(IndexMetadata.SETTING_REMOTE_STORE_ENABLED, true)
.put(IndexMetadata.SETTING_REMOTE_STORE_REPOSITORY, REPOSITORY_NAME)
.put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_ENABLED, false)
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ public Settings indexSettings() {
.put(super.indexSettings())
.put(IndexMetadata.SETTING_REMOTE_STORE_ENABLED, true)
.put(IndexMetadata.SETTING_REMOTE_STORE_REPOSITORY, REPOSITORY_NAME)
.put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_ENABLED, false)
.put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT)
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ public void testRestoreOperationsShallowCopyEnabled(boolean remoteTranslogEnable
.cluster()
.prepareRestoreSnapshot(snapshotRepoName, snapshotName1)
.setWaitForCompletion(false)
.setIgnoreIndexSettings(IndexMetadata.SETTING_REMOTE_STORE_ENABLED, IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_ENABLED)
.setIgnoreIndexSettings(IndexMetadata.SETTING_REMOTE_STORE_ENABLED)
.setIndices(indexName1)
.setRenamePattern(indexName1)
.setRenameReplacement(restoredIndexName1Seg)
Expand All @@ -322,11 +322,7 @@ public void testRestoreOperationsShallowCopyEnabled(boolean remoteTranslogEnable
.cluster()
.prepareRestoreSnapshot(snapshotRepoName, snapshotName1)
.setWaitForCompletion(false)
.setIgnoreIndexSettings(
IndexMetadata.SETTING_REMOTE_STORE_ENABLED,
IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_ENABLED,
IndexMetadata.SETTING_REPLICATION_TYPE
)
.setIgnoreIndexSettings(IndexMetadata.SETTING_REMOTE_STORE_ENABLED, IndexMetadata.SETTING_REPLICATION_TYPE)
.setIndices(indexName1)
.setRenamePattern(indexName1)
.setRenameReplacement(restoredIndexName1Doc)
Expand Down Expand Up @@ -552,9 +548,7 @@ private Settings.Builder getIndexSettings(
.put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT);
}
if (enableRemoteTranslog) {
settingsBuilder.put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_ENABLED, true)
.put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, remoteStoreRepo)
.build();
settingsBuilder.put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, remoteStoreRepo).build();
}
return settingsBuilder;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,8 +293,6 @@ public Iterator<Setting<?>> settings() {

public static final String SETTING_REMOTE_STORE_REPOSITORY = "index.remote_store.repository";

public static final String SETTING_REMOTE_TRANSLOG_STORE_ENABLED = "index.remote_store.translog.enabled";

public static final String SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY = "index.remote_store.translog.repository";

/**
Expand Down Expand Up @@ -377,34 +375,6 @@ private static void validateRemoteStoreSettingEnabled(final Map<Setting<?>, Obje
}
}

/**
* Used to specify if the index translog operations should be persisted in the remote store.
*/
public static final Setting<Boolean> INDEX_REMOTE_TRANSLOG_STORE_ENABLED_SETTING = Setting.boolSetting(
SETTING_REMOTE_TRANSLOG_STORE_ENABLED,
false,
new Setting.Validator<>() {

@Override
public void validate(final Boolean value) {}

@Override
public void validate(final Boolean value, final Map<Setting<?>, Object> settings) {
if (value == true) {
validateRemoteStoreSettingEnabled(settings, INDEX_REMOTE_TRANSLOG_STORE_ENABLED_SETTING);
}
}

@Override
public Iterator<Setting<?>> settings() {
final List<Setting<?>> settings = Collections.singletonList(INDEX_REMOTE_STORE_ENABLED_SETTING);
return settings.iterator();
}
},
Property.IndexScope,
Property.Final
);

public static final Setting<String> INDEX_REMOTE_TRANSLOG_REPOSITORY_SETTING = Setting.simpleString(
SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY,
new Setting.Validator<>() {
Expand All @@ -419,13 +389,13 @@ public void validate(final String value, final Map<Setting<?>, Object> settings)
"Setting " + INDEX_REMOTE_TRANSLOG_REPOSITORY_SETTING.getKey() + " should be provided with non-empty repository ID"
);
} else {
final Boolean isRemoteTranslogStoreEnabled = (Boolean) settings.get(INDEX_REMOTE_TRANSLOG_STORE_ENABLED_SETTING);
final Boolean isRemoteTranslogStoreEnabled = (Boolean) settings.get(INDEX_REMOTE_STORE_ENABLED_SETTING);
if (isRemoteTranslogStoreEnabled == null || isRemoteTranslogStoreEnabled == false) {
throw new IllegalArgumentException(
"Settings "
+ INDEX_REMOTE_TRANSLOG_REPOSITORY_SETTING.getKey()
+ " can only be set/enabled when "
+ INDEX_REMOTE_TRANSLOG_STORE_ENABLED_SETTING.getKey()
+ INDEX_REMOTE_STORE_ENABLED_SETTING.getKey()
+ " is set to true"
);
}
Expand All @@ -434,7 +404,7 @@ public void validate(final String value, final Map<Setting<?>, Object> settings)

@Override
public Iterator<Setting<?>> settings() {
final List<Setting<?>> settings = Collections.singletonList(INDEX_REMOTE_TRANSLOG_STORE_ENABLED_SETTING);
final List<Setting<?>> settings = Collections.singletonList(INDEX_REMOTE_STORE_ENABLED_SETTING);
return settings.iterator();
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@
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 @@ -123,7 +122,6 @@
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;
Expand All @@ -132,14 +130,12 @@
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;
import static org.opensearch.indices.IndicesService.CLUSTER_REPLICATION_TYPE_SETTING;

/**
Expand Down Expand Up @@ -957,10 +953,11 @@ private static void updateRemoteStoreSettings(Settings.Builder settingsBuilder,
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) == false) {
return;
throw new IllegalArgumentException("cannot control remote store index settings");
}

// Verify index has replication type as SEGMENT
// this validation should not be needed if we don't allow remote enabled nodes to bootup with replication type document
if (ReplicationType.DOCUMENT.equals(ReplicationType.parseString(settingsBuilder.get(SETTING_REPLICATION_TYPE)))) {
throw new IllegalArgumentException(
"Cannot enable ["
Expand All @@ -973,37 +970,17 @@ private static void updateRemoteStoreSettings(Settings.Builder settingsBuilder,
}

settingsBuilder.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_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") == 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);
}
settingsBuilder.put(SETTING_REMOTE_STORE_REPOSITORY, CLUSTER_REMOTE_STORE_REPOSITORY_SETTING.get(clusterSettings));
settingsBuilder.put(SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, CLUSTER_REMOTE_TRANSLOG_REPOSITORY_SETTING.get(clusterSettings));
}
}

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

public static void validateStoreTypeSettings(Settings settings) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,6 @@ public void apply(Settings value, Settings current, Settings previous) {
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
),
List.of(FeatureFlags.CONCURRENT_SEGMENT_SEARCH),
Expand Down
Loading

0 comments on commit 4999d3e

Please sign in to comment.