From 057b096f272e08c60e0f3623e2e4344e5d7d8f82 Mon Sep 17 00:00:00 2001 From: Andrew Ross Date: Fri, 21 Oct 2022 12:47:40 -0700 Subject: [PATCH] [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 (cherry picked from commit a469a3cbcb7a7294a81bf7e4122968c2a9b32ca4) * Add CHANGELOG entry Signed-off-by: Andrew Ross Signed-off-by: Andrew Ross Co-authored-by: Bukhtawar Khan --- CHANGELOG.md | 1 + .../cluster/metadata/IndexMetadata.java | 29 ++++++++ .../common/settings/IndexScopedSettings.java | 6 +- .../org/opensearch/index/IndexSettings.java | 9 +++ .../opensearch/index/IndexSettingsTests.java | 72 +++++++++++++++++-- 5 files changed, 110 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c1881821210b2..7dcf2f721d8b2 100644 --- 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 index 72697c44295cf..cd1c92a8b109f 100644 --- 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> 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, Obje } } + /** + * Used to specify if the index translog operations should be persisted in the remote store. + */ + public static final Setting 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, Object> settings) { + if (value == true) { + validateRemoteStoreSettingEnabled(settings, INDEX_REMOTE_TRANSLOG_STORE_ENABLED_SETTING); + } + } + + @Override + public Iterator> settings() { + final List> 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 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 index 201bd76b9aada..079fc38415328 100644 --- 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 index e9d64d39aef35..72602077150b4 100644 --- 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 index 2d232cccef8b2..de5ef8851ae80 100644 --- 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,7 +723,7 @@ 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)); } @@ -732,10 +731,7 @@ public void testSoftDeletesDefaultSetting() { 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> 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> 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)