Skip to content

Commit

Permalink
[Remote Store] Add index specific setting for remote repository (open…
Browse files Browse the repository at this point in the history
…search-project#4253)

* Add index specific setting for remote repository
* Fix for failover incremental uploads

Signed-off-by: Sachin Kale <kalsac@amazon.com>
  • Loading branch information
sachinpkale authored and pranikum committed Sep 25, 2022
1 parent 7024c22 commit 55338b1
Show file tree
Hide file tree
Showing 7 changed files with 139 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
*/
Expand All @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
Expand Down
9 changes: 9 additions & 0 deletions server/src/main/java/org/opensearch/index/IndexSettings.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
);
}
Expand All @@ -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());
}
}

0 comments on commit 55338b1

Please sign in to comment.