remove remote translog setting and restrict to remote only index crea…

…tion Signed-off-by: bansvaru <[email protected]>
opensearch-project · Jul 17, 2023 · f281cb9 · f281cb9
1 parent ca74aac
commit f281cb9
Show file tree

Hide file tree

Showing 14 changed files with 29 additions and 236 deletions.
diff --git a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java
@@ -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";
 
     /**
@@ -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<>() {
@@ -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"
                         );
                     }
@@ -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();
             }
         },

diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java
@@ -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;
@@ -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;
@@ -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;
 
 /**
@@ -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 ["
@@ -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) {

diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java
@@ -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),

diff --git a/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java b/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java
@@ -236,7 +236,6 @@ public final class IndexScopedSettings extends AbstractScopedSettings {
         List.of(
             IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING,
             IndexMetadata.INDEX_REMOTE_STORE_REPOSITORY_SETTING,
-            IndexMetadata.INDEX_REMOTE_TRANSLOG_STORE_ENABLED_SETTING,
             IndexMetadata.INDEX_REMOTE_TRANSLOG_REPOSITORY_SETTING
         ),
         FeatureFlags.CONCURRENT_SEGMENT_SEARCH,

diff --git a/server/src/main/java/org/opensearch/index/IndexSettings.java b/server/src/main/java/org/opensearch/index/IndexSettings.java
@@ -614,7 +614,6 @@ public final class IndexSettings {
     private final int numberOfShards;
     private final ReplicationType replicationType;
     private final boolean isRemoteStoreEnabled;
-    private final boolean isRemoteTranslogStoreEnabled;
     private volatile TimeValue remoteTranslogUploadBufferInterval;
     private final String remoteStoreTranslogRepository;
     private final String remoteStoreRepository;
@@ -784,7 +783,6 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
         numberOfShards = settings.getAsInt(IndexMetadata.SETTING_NUMBER_OF_SHARDS, null);
         replicationType = IndexMetadata.INDEX_REPLICATION_TYPE_SETTING.get(settings);
         isRemoteStoreEnabled = settings.getAsBoolean(IndexMetadata.SETTING_REMOTE_STORE_ENABLED, false);
-        isRemoteTranslogStoreEnabled = settings.getAsBoolean(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_ENABLED, false);
         remoteStoreTranslogRepository = settings.get(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY);
         remoteTranslogUploadBufferInterval = INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING.get(settings);
         remoteStoreRepository = settings.get(IndexMetadata.SETTING_REMOTE_STORE_REPOSITORY);
@@ -1072,7 +1070,7 @@ public boolean isRemoteStoreEnabled() {
      * Returns if remote translog store is enabled for this index.
      */
     public boolean isRemoteTranslogStoreEnabled() {
-        return isRemoteTranslogStoreEnabled;
+        return isRemoteStoreEnabled;
     }
 
     /**

diff --git a/server/src/main/java/org/opensearch/indices/IndicesService.java b/server/src/main/java/org/opensearch/indices/IndicesService.java
@@ -263,16 +263,6 @@ public class IndicesService extends AbstractLifecycleComponent
         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
      */