[Remote Store] Add index specific setting for remote repository (#4253)

* Add index specific setting for remote repository * Fix for failover incremental uploads Signed-off-by: Sachin Kale <kalsac@amazon.com>
opensearch-project · Sep 6, 2022 · ff2e4bf · ff2e4bf
1 parent 4ed0995
commit ff2e4bf
Show file tree

Hide file tree

Showing 7 changed files with 139 additions and 13 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -22,6 +22,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Use RemoteSegmentStoreDirectory instead of RemoteDirectory ([#4240](https://github.com/opensearch-project/OpenSearch/pull/4240))
 - Plugin ZIP publication groupId value is configurable ([#4156](https://github.com/opensearch-project/OpenSearch/pull/4156))
 - Update to Netty 4.1.80.Final ([#4359](https://github.com/opensearch-project/OpenSearch/pull/4359))
+- Add index specific setting for remote repository ([#4253](https://github.com/opensearch-project/OpenSearch/pull/4253))
 
 ### Deprecated
 

diff --git a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java
@@ -285,6 +285,8 @@ public Iterator<Setting<?>> settings() {
 
     public static final String SETTING_REMOTE_STORE_ENABLED = "index.remote_store.enabled";
 
+    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.
@@ -322,6 +324,50 @@ public Iterator<Setting<?>> settings() {
         Property.Final
     );
 
+    /**
+     * Used to specify remote store repository to use for this index.
+     */
+    public static final Setting<String> INDEX_REMOTE_STORE_REPOSITORY_SETTING = Setting.simpleString(
+        SETTING_REMOTE_STORE_REPOSITORY,
+        new Setting.Validator<>() {
+
+            @Override
+            public void validate(final String value) {}
+
+            @Override
+            public void validate(final String value, final Map<Setting<?>, Object> settings) {
+                if (value == null || value.isEmpty()) {
+                    throw new IllegalArgumentException(
+                        "Setting " + INDEX_REMOTE_STORE_REPOSITORY_SETTING.getKey() + " should be provided with non-empty repository ID"
+                    );
+                } else {
+                    validateRemoteStoreSettingEnabled(settings, INDEX_REMOTE_STORE_REPOSITORY_SETTING);
+                }
+            }
+
+            @Override
+            public Iterator<Setting<?>> settings() {
+                final List<Setting<?>> settings = Collections.singletonList(INDEX_REMOTE_STORE_ENABLED_SETTING);
+                return settings.iterator();
+            }
+        },
+        Property.IndexScope,
+        Property.Final
+    );
+
+    private static void validateRemoteStoreSettingEnabled(final Map<Setting<?>, Object> settings, Setting<?> setting) {
+        final Boolean isRemoteSegmentStoreEnabled = (Boolean) settings.get(INDEX_REMOTE_STORE_ENABLED_SETTING);
+        if (isRemoteSegmentStoreEnabled == false) {
+            throw new IllegalArgumentException(
+                "Settings "
+                    + setting.getKey()
+                    + " can ont be set/enabled when "
+                    + INDEX_REMOTE_STORE_ENABLED_SETTING.getKey()
+                    + " is set to true"
+            );
+        }
+    }
+
     /**
      * Used to specify if the index translog operations should be persisted in the remote store.
      */
@@ -335,16 +381,8 @@ public void validate(final Boolean value) {}
 
             @Override
             public void validate(final Boolean value, final Map<Setting<?>, Object> settings) {
-                final Boolean isRemoteSegmentStoreEnabled = (Boolean) settings.get(INDEX_REMOTE_STORE_ENABLED_SETTING);
-                if (isRemoteSegmentStoreEnabled == false && value == true) {
-                    throw new IllegalArgumentException(
-                        "Settings "
-                            + INDEX_REMOTE_TRANSLOG_STORE_ENABLED_SETTING.getKey()
-                            + " cannot be enabled when "
-                            + INDEX_REMOTE_STORE_ENABLED_SETTING.getKey()
-                            + " is set to "
-                            + settings.get(INDEX_REMOTE_STORE_ENABLED_SETTING)
-                    );
+                if (value == true) {
+                    validateRemoteStoreSettingEnabled(settings, INDEX_REMOTE_TRANSLOG_STORE_ENABLED_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,
         Collections.singletonList(IndexMetadata.INDEX_REPLICATION_TYPE_SETTING),
         FeatureFlags.REMOTE_STORE,
-        Arrays.asList(IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING, IndexMetadata.INDEX_REMOTE_TRANSLOG_STORE_ENABLED_SETTING)
+        Arrays.asList(
+            IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING,
+            IndexMetadata.INDEX_REMOTE_TRANSLOG_STORE_ENABLED_SETTING,
+            IndexMetadata.INDEX_REMOTE_STORE_REPOSITORY_SETTING
+        )
     );
 
     public static final IndexScopedSettings DEFAULT_SCOPED_SETTINGS = new IndexScopedSettings(Settings.EMPTY, BUILT_IN_INDEX_SETTINGS);

diff --git a/server/src/main/java/org/opensearch/index/IndexService.java b/server/src/main/java/org/opensearch/index/IndexService.java
@@ -511,7 +511,7 @@ public synchronized IndexShard createShard(
             Store remoteStore = null;
             if (this.indexSettings.isRemoteStoreEnabled()) {
                 Directory remoteDirectory = remoteDirectoryFactory.newDirectory(
-                    clusterService.state().metadata().clusterUUID(),
+                    this.indexSettings.getRemoteStoreRepository(),
                     this.indexSettings,
                     path
                 );

diff --git a/server/src/main/java/org/opensearch/index/IndexSettings.java b/server/src/main/java/org/opensearch/index/IndexSettings.java
@@ -560,6 +560,7 @@ public final class IndexSettings {
     private final ReplicationType replicationType;
     private final boolean isRemoteStoreEnabled;
     private final boolean isRemoteTranslogStoreEnabled;
+    private final String remoteStoreRepository;
     // volatile fields are updated via #updateIndexMetadata(IndexMetadata) under lock
     private volatile Settings settings;
     private volatile IndexMetadata indexMetadata;
@@ -721,6 +722,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);
         isRemoteTranslogStoreEnabled = settings.getAsBoolean(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_ENABLED, false);
+        remoteStoreRepository = settings.get(IndexMetadata.SETTING_REMOTE_STORE_REPOSITORY);
         this.searchThrottled = INDEX_SEARCH_THROTTLED.get(settings);
         this.queryStringLenient = QUERY_STRING_LENIENT_SETTING.get(settings);
         this.queryStringAnalyzeWildcard = QUERY_STRING_ANALYZE_WILDCARD.get(nodeSettings);
@@ -979,6 +981,13 @@ public boolean isRemoteTranslogStoreEnabled() {
         return isRemoteTranslogStoreEnabled;
     }
 
+    /**
+     * Returns if remote store is enabled for this index.
+     */
+    public String getRemoteStoreRepository() {
+        return remoteStoreRepository;
+    }
+
     /**
      * 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/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java b/server/src/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java
@@ -59,6 +59,13 @@ public RemoteStoreRefreshListener(IndexShard indexShard) {
             .getDelegate()).getDelegate();
         this.primaryTerm = indexShard.getOperationPrimaryTerm();
         localSegmentChecksumMap = new HashMap<>();
+        if (indexShard.shardRouting.primary()) {
+            try {
+                this.remoteDirectory.init();
+            } catch (IOException e) {
+                logger.error("Exception while initialising RemoteSegmentStoreDirectory", e);
+            }
+        }
     }
 
     @Override

diff --git a/server/src/test/java/org/opensearch/index/IndexSettingsTests.java b/server/src/test/java/org/opensearch/index/IndexSettingsTests.java
@@ -851,7 +851,7 @@ public void testEnablingRemoteTranslogStoreFailsWhenRemoteSegmentDisabled() {
             () -> IndexMetadata.INDEX_REMOTE_TRANSLOG_STORE_ENABLED_SETTING.get(indexSettings)
         );
         assertEquals(
-            "Settings index.remote_store.translog.enabled cannot be enabled when index.remote_store.enabled is set to false",
+            "Settings index.remote_store.translog.enabled can ont be set/enabled when index.remote_store.enabled is set to true",
             iae.getMessage()
         );
     }
@@ -876,4 +876,71 @@ public void testEnablingRemoteStoreFailsWhenReplicationTypeIsDefault() {
         );
         assertEquals("To enable index.remote_store.enabled, index.replication.type should be set to SEGMENT", iae.getMessage());
     }
+
+    public void testRemoteRepositoryDefaultSetting() {
+        IndexMetadata metadata = newIndexMeta(
+            "index",
+            Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT).build()
+        );
+        IndexSettings settings = new IndexSettings(metadata, Settings.EMPTY);
+        assertNull(settings.getRemoteStoreRepository());
+    }
+
+    public void testRemoteRepositoryExplicitSetting() {
+        IndexMetadata metadata = newIndexMeta(
+            "index",
+            Settings.builder()
+                .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
+                .put(IndexMetadata.SETTING_REMOTE_STORE_ENABLED, true)
+                .put(IndexMetadata.SETTING_REMOTE_STORE_REPOSITORY, "repo1")
+                .build()
+        );
+        IndexSettings settings = new IndexSettings(metadata, Settings.EMPTY);
+        assertEquals("repo1", settings.getRemoteStoreRepository());
+    }
+
+    public void testUpdateRemoteRepositoryFails() {
+        Set<Setting<?>> remoteStoreSettingSet = new HashSet<>();
+        remoteStoreSettingSet.add(IndexMetadata.INDEX_REMOTE_STORE_REPOSITORY_SETTING);
+        IndexScopedSettings settings = new IndexScopedSettings(Settings.EMPTY, remoteStoreSettingSet);
+        IllegalArgumentException error = expectThrows(
+            IllegalArgumentException.class,
+            () -> settings.updateSettings(
+                Settings.builder().put("index.remote_store.repository", randomUnicodeOfLength(10)).build(),
+                Settings.builder(),
+                Settings.builder(),
+                "index"
+            )
+        );
+        assertEquals(error.getMessage(), "final index setting [index.remote_store.repository], not updateable");
+    }
+
+    public void testSetRemoteRepositoryFailsWhenRemoteStoreIsNotEnabled() {
+        Settings indexSettings = Settings.builder()
+            .put("index.replication.type", ReplicationType.SEGMENT)
+            .put("index.remote_store.enabled", false)
+            .put("index.remote_store.repository", "repo1")
+            .build();
+        IllegalArgumentException iae = expectThrows(
+            IllegalArgumentException.class,
+            () -> IndexMetadata.INDEX_REMOTE_STORE_REPOSITORY_SETTING.get(indexSettings)
+        );
+        assertEquals(
+            "Settings index.remote_store.repository can ont be set/enabled when index.remote_store.enabled is set to true",
+            iae.getMessage()
+        );
+    }
+
+    public void testSetRemoteRepositoryFailsWhenEmptyString() {
+        Settings indexSettings = Settings.builder()
+            .put("index.replication.type", ReplicationType.SEGMENT)
+            .put("index.remote_store.enabled", false)
+            .put("index.remote_store.repository", "")
+            .build();
+        IllegalArgumentException iae = expectThrows(
+            IllegalArgumentException.class,
+            () -> IndexMetadata.INDEX_REMOTE_STORE_REPOSITORY_SETTING.get(indexSettings)
+        );
+        assertEquals("Setting index.remote_store.repository should be provided with non-empty repository ID", iae.getMessage());
+    }
 }