[Backport 2.x] Introduce Remote translog feature flag (#4158) (#4857)

* Introduce Remote translog feature flag (#4158) * Introduce Remote translog feature flag * Limit combinations of remote segment and translog store to valid ones Signed-off-by: Bukhtawar Khan <[email protected]> (cherry picked from commit a469a3c) * Add CHANGELOG entry Signed-off-by: Andrew Ross <[email protected]> Signed-off-by: Andrew Ross <[email protected]> Co-authored-by: Bukhtawar Khan <[email protected]>
opensearch-project · Oct 21, 2022 · 057b096 · 057b096
1 parent 1511aa8
commit 057b096
Show file tree

Hide file tree

Showing 5 changed files with 110 additions and 7 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -17,6 +17,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Support for labels on version bump PRs, skip label support for changelog verifier ([#4391](https://github.com/opensearch-project/OpenSearch/pull/4391))
 - Add a new node role 'search' which is dedicated to provide search capability ([#4689](https://github.com/opensearch-project/OpenSearch/pull/4689))
 - Introduce experimental searchable snapshot API ([#4680](https://github.com/opensearch-project/OpenSearch/pull/4680))
+- Introduce Remote translog feature flag([#4158](https://github.com/opensearch-project/OpenSearch/pull/4158))
 ### Dependencies
 - Bumps `com.diffplug.spotless` from 6.9.1 to 6.10.0
 - Bumps `xmlbeans` from 5.1.0 to 5.1.1

diff --git a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java
@@ -287,6 +287,7 @@ 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";
     /**
      * Used to specify if the index data should be persisted in the remote store.
      */
@@ -367,6 +368,34 @@ 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 String SETTING_AUTO_EXPAND_REPLICAS = "index.auto_expand_replicas";
     public static final Setting<AutoExpandReplicas> INDEX_AUTO_EXPAND_REPLICAS_SETTING = AutoExpandReplicas.SETTING;
 

diff --git a/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java b/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java
@@ -223,7 +223,11 @@ public final class IndexScopedSettings extends AbstractScopedSettings {
         FeatureFlags.REPLICATION_TYPE,
         List.of(IndexMetadata.INDEX_REPLICATION_TYPE_SETTING),
         FeatureFlags.REMOTE_STORE,
-        Arrays.asList(IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING, IndexMetadata.INDEX_REMOTE_STORE_REPOSITORY_SETTING),
+        List.of(
+            IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING,
+            IndexMetadata.INDEX_REMOTE_TRANSLOG_STORE_ENABLED_SETTING,
+            IndexMetadata.INDEX_REMOTE_STORE_REPOSITORY_SETTING
+        ),
         FeatureFlags.SEARCHABLE_SNAPSHOT,
         List.of(
             IndexSettings.SEARCHABLE_SNAPSHOT_REPOSITORY,

diff --git a/server/src/main/java/org/opensearch/index/IndexSettings.java b/server/src/main/java/org/opensearch/index/IndexSettings.java
@@ -584,6 +584,7 @@ public final class IndexSettings {
     private final ReplicationType replicationType;
     private final boolean isRemoteStoreEnabled;
     private final String remoteStoreRepository;
+    private final boolean isRemoteTranslogStoreEnabled;
     // volatile fields are updated via #updateIndexMetadata(IndexMetadata) under lock
     private volatile Settings settings;
     private volatile IndexMetadata indexMetadata;
@@ -745,6 +746,7 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
         replicationType = ReplicationType.parseString(settings.get(IndexMetadata.SETTING_REPLICATION_TYPE));
         isRemoteStoreEnabled = settings.getAsBoolean(IndexMetadata.SETTING_REMOTE_STORE_ENABLED, false);
         remoteStoreRepository = settings.get(IndexMetadata.SETTING_REMOTE_STORE_REPOSITORY);
+        isRemoteTranslogStoreEnabled = settings.getAsBoolean(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_ENABLED, false);
         this.searchThrottled = INDEX_SEARCH_THROTTLED.get(settings);
         this.queryStringLenient = QUERY_STRING_LENIENT_SETTING.get(settings);
         this.queryStringAnalyzeWildcard = QUERY_STRING_ANALYZE_WILDCARD.get(nodeSettings);
@@ -1003,6 +1005,13 @@ public String getRemoteStoreRepository() {
         return remoteStoreRepository;
     }
 
+    /**
+     * Returns if remote translog store is enabled for this index.
+     */
+    public boolean isRemoteTranslogStoreEnabled() {
+        return isRemoteTranslogStoreEnabled;
+    }
+
     /**
      * Returns the node settings. The settings returned from {@link #getSettings()} are a merged version of the
      * index settings and the node settings where node settings are overwritten by index settings.

diff --git a/server/src/test/java/org/opensearch/index/IndexSettingsTests.java b/server/src/test/java/org/opensearch/index/IndexSettingsTests.java
@@ -32,7 +32,6 @@
 
 package org.opensearch.index;
 
-import org.opensearch.LegacyESVersion;
 import org.opensearch.Version;
 import org.opensearch.cluster.metadata.IndexMetadata;
 import org.opensearch.common.settings.AbstractScopedSettings;
@@ -724,18 +723,15 @@ public void testUpdateSoftDeletesFails() {
     public void testSoftDeletesDefaultSetting() {
         // enabled by default on 7.0+ or later
         {
-            Version createdVersion = VersionUtils.randomVersionBetween(random(), LegacyESVersion.V_7_0_0, Version.CURRENT);
+            Version createdVersion = VersionUtils.randomVersionBetween(random(), Version.V_1_0_0, Version.CURRENT);
             Settings settings = Settings.builder().put(IndexMetadata.SETTING_INDEX_VERSION_CREATED.getKey(), createdVersion).build();
             assertTrue(IndexSettings.INDEX_SOFT_DELETES_SETTING.get(settings));
         }
     }
 
     public void testIgnoreTranslogRetentionSettingsIfSoftDeletesEnabled() {
         Settings.Builder settings = Settings.builder()
-            .put(
-                IndexMetadata.SETTING_VERSION_CREATED,
-                VersionUtils.randomVersionBetween(random(), LegacyESVersion.V_7_4_0, Version.CURRENT)
-            );
+            .put(IndexMetadata.SETTING_VERSION_CREATED, VersionUtils.randomVersionBetween(random(), Version.V_1_0_0, Version.CURRENT));
         if (randomBoolean()) {
             settings.put(IndexSettings.INDEX_TRANSLOG_RETENTION_AGE_SETTING.getKey(), randomPositiveTimeValue());
         }
@@ -780,6 +776,39 @@ public void testRemoteStoreExplicitSetting() {
         assertTrue(settings.isRemoteStoreEnabled());
     }
 
+    public void testRemoteTranslogStoreDefaultSetting() {
+        IndexMetadata metadata = newIndexMeta(
+            "index",
+            Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT).build()
+        );
+        IndexSettings settings = new IndexSettings(metadata, Settings.EMPTY);
+        assertFalse(settings.isRemoteTranslogStoreEnabled());
+    }
+
+    public void testRemoteTranslogStoreExplicitSetting() {
+        IndexMetadata metadata = newIndexMeta(
+            "index",
+            Settings.builder()
+                .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
+                .put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_ENABLED, true)
+                .build()
+        );
+        IndexSettings settings = new IndexSettings(metadata, Settings.EMPTY);
+        assertTrue(settings.isRemoteTranslogStoreEnabled());
+    }
+
+    public void testRemoteTranslogStoreNullSetting() {
+        Settings indexSettings = Settings.builder()
+            .put("index.remote_store.translog.enabled", "null")
+            .put("index.remote_store.enabled", randomBoolean())
+            .build();
+        IllegalArgumentException iae = expectThrows(
+            IllegalArgumentException.class,
+            () -> IndexMetadata.INDEX_REMOTE_TRANSLOG_STORE_ENABLED_SETTING.get(indexSettings)
+        );
+        assertEquals("Failed to parse value [null] as only [true] or [false] are allowed.", iae.getMessage());
+    }
+
     public void testUpdateRemoteStoreFails() {
         Set<Setting<?>> remoteStoreSettingSet = new HashSet<>();
         remoteStoreSettingSet.add(IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING);
@@ -796,6 +825,37 @@ public void testUpdateRemoteStoreFails() {
         assertEquals(error.getMessage(), "final index setting [index.remote_store.enabled], not updateable");
     }
 
+    public void testUpdateRemoteTranslogStoreFails() {
+        Set<Setting<?>> remoteStoreSettingSet = new HashSet<>();
+        remoteStoreSettingSet.add(IndexMetadata.INDEX_REMOTE_TRANSLOG_STORE_ENABLED_SETTING);
+        IndexScopedSettings settings = new IndexScopedSettings(Settings.EMPTY, remoteStoreSettingSet);
+        IllegalArgumentException error = expectThrows(
+            IllegalArgumentException.class,
+            () -> settings.updateSettings(
+                Settings.builder().put("index.remote_store.translog.enabled", randomBoolean()).build(),
+                Settings.builder(),
+                Settings.builder(),
+                "index"
+            )
+        );
+        assertEquals(error.getMessage(), "final index setting [index.remote_store.translog.enabled], not updateable");
+    }
+
+    public void testEnablingRemoteTranslogStoreFailsWhenRemoteSegmentDisabled() {
+        Settings indexSettings = Settings.builder()
+            .put("index.remote_store.translog.enabled", true)
+            .put("index.remote_store.enabled", false)
+            .build();
+        IllegalArgumentException iae = expectThrows(
+            IllegalArgumentException.class,
+            () -> IndexMetadata.INDEX_REMOTE_TRANSLOG_STORE_ENABLED_SETTING.get(indexSettings)
+        );
+        assertEquals(
+            "Settings index.remote_store.translog.enabled can ont be set/enabled when index.remote_store.enabled is set to true",
+            iae.getMessage()
+        );
+    }
+
     public void testEnablingRemoteStoreFailsWhenReplicationTypeIsDocument() {
         Settings indexSettings = Settings.builder()
             .put("index.replication.type", ReplicationType.DOCUMENT)