From e5a70db4b35e7aa0d70c5a2c366f7e43f4f9d289 Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Mon, 25 Apr 2022 14:54:13 -0400 Subject: [PATCH] Adds the replication type index setting, alongside a formal notion of feature flags (#3037) (#3071) * This change formalizes the notion of feature flags, and adds a "replication type" setting that will differentiate between document and segment replication, gated by a feature flag. Since seg-rep is currently an incomplete implementation, the feature flag ensures that the setting is not visible to users without explicitly setting a system property. We can then continue to merge seg-rep related changes from the feature branch to `main` safely hidden behind the feature flag gate. Signed-off-by: Kartik Ganesh * Update security policy for testing feature flags Signed-off-by: Nicholas Walter Knize Co-authored-by: Nicholas Walter Knize (cherry picked from commit ee7b731975d9044d3239ee3a4fe97f50d45af3e5) Co-authored-by: Kartik Ganesh --- .../cluster/metadata/IndexMetadata.java | 13 ++++++ .../common/settings/IndexScopedSettings.java | 11 +++++ .../common/settings/SettingsModule.java | 7 +++ .../opensearch/common/util/FeatureFlags.java | 32 ++++++++++++++ .../org/opensearch/index/IndexSettings.java | 10 +++++ .../replication/common/ReplicationType.java | 30 +++++++++++++ .../org/opensearch/bootstrap/security.policy | 3 ++ .../common/util/FeatureFlagTests.java | 43 +++++++++++++++++++ 8 files changed, 149 insertions(+) create mode 100644 server/src/main/java/org/opensearch/common/util/FeatureFlags.java create mode 100644 server/src/main/java/org/opensearch/indices/replication/common/ReplicationType.java create mode 100644 server/src/test/java/org/opensearch/common/util/FeatureFlagTests.java 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 6510c57060fe0..9139cbac2b0be 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java @@ -70,6 +70,7 @@ import org.opensearch.index.mapper.MapperService; import org.opensearch.index.seqno.SequenceNumbers; import org.opensearch.index.shard.ShardId; +import org.opensearch.indices.replication.common.ReplicationType; import org.opensearch.rest.RestStatus; import java.io.IOException; @@ -260,6 +261,18 @@ public Iterator> settings() { Property.IndexScope ); + /** + * Used to specify the replication type for the index. By default, document replication is used. + */ + public static final String SETTING_REPLICATION_TYPE = "index.replication.type"; + public static final Setting INDEX_REPLICATION_TYPE_SETTING = new Setting<>( + SETTING_REPLICATION_TYPE, + ReplicationType.DOCUMENT.toString(), + ReplicationType::parseString, + 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 528d6cc9f5e23..68e1b5b598d40 100644 --- a/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java @@ -40,6 +40,7 @@ import org.opensearch.cluster.routing.allocation.decider.ShardsLimitAllocationDecider; import org.opensearch.common.logging.Loggers; import org.opensearch.common.settings.Setting.Property; +import org.opensearch.common.util.FeatureFlags; import org.opensearch.index.IndexModule; import org.opensearch.index.IndexSettings; import org.opensearch.index.IndexSortConfig; @@ -207,6 +208,16 @@ public final class IndexScopedSettings extends AbstractScopedSettings { ) ); + /** + * Map of feature flag name to feature-flagged index setting. Once each feature + * is ready for production release, the feature flag can be removed, and the + * setting should be moved to {@link #BUILT_IN_INDEX_SETTINGS}. + */ + public static final Map FEATURE_FLAGGED_INDEX_SETTINGS = Map.of( + FeatureFlags.REPLICATION_TYPE, + IndexMetadata.INDEX_REPLICATION_TYPE_SETTING + ); + public static final IndexScopedSettings DEFAULT_SCOPED_SETTINGS = new IndexScopedSettings(Settings.EMPTY, BUILT_IN_INDEX_SETTINGS); public IndexScopedSettings(Settings settings, Set> settingsSet) { diff --git a/server/src/main/java/org/opensearch/common/settings/SettingsModule.java b/server/src/main/java/org/opensearch/common/settings/SettingsModule.java index 79ee0bf9f975a..0874814f940d4 100644 --- a/server/src/main/java/org/opensearch/common/settings/SettingsModule.java +++ b/server/src/main/java/org/opensearch/common/settings/SettingsModule.java @@ -37,6 +37,7 @@ import org.opensearch.common.Strings; import org.opensearch.common.inject.Binder; import org.opensearch.common.inject.Module; +import org.opensearch.common.util.FeatureFlags; import org.opensearch.common.xcontent.ToXContent; import org.opensearch.common.xcontent.XContentBuilder; import org.opensearch.common.xcontent.XContentType; @@ -85,6 +86,12 @@ public SettingsModule( registerSetting(setting); } + for (Map.Entry featureFlaggedSetting : IndexScopedSettings.FEATURE_FLAGGED_INDEX_SETTINGS.entrySet()) { + if (FeatureFlags.isEnabled(featureFlaggedSetting.getKey())) { + registerSetting(featureFlaggedSetting.getValue()); + } + } + for (Setting setting : additionalSettings) { registerSetting(setting); } diff --git a/server/src/main/java/org/opensearch/common/util/FeatureFlags.java b/server/src/main/java/org/opensearch/common/util/FeatureFlags.java new file mode 100644 index 0000000000000..34c613f5423d0 --- /dev/null +++ b/server/src/main/java/org/opensearch/common/util/FeatureFlags.java @@ -0,0 +1,32 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.common.util; + +/** + * Utility class to manage feature flags. Feature flags are system properties that must be set on the JVM. + * These are used to gate the visibility/availability of incomplete features. Fore more information, see + * https://featureflags.io/feature-flag-introduction/ + */ +public class FeatureFlags { + + /** + * Gates the visibility of the index setting that allows changing of replication type. + * Once the feature is ready for production release, this feature flag can be removed. + */ + public static final String REPLICATION_TYPE = "opensearch.experimental.feature.replication_type.enabled"; + + /** + * Used to test feature flags whose values are expected to be booleans. + * This method returns true if the value is "true" (case-insensitive), + * and false otherwise. + */ + public static boolean isEnabled(String featureFlagName) { + return "true".equalsIgnoreCase(System.getProperty(featureFlagName)); + } +} diff --git a/server/src/main/java/org/opensearch/index/IndexSettings.java b/server/src/main/java/org/opensearch/index/IndexSettings.java index aa69417af1897..8ba9c47902115 100644 --- a/server/src/main/java/org/opensearch/index/IndexSettings.java +++ b/server/src/main/java/org/opensearch/index/IndexSettings.java @@ -46,6 +46,7 @@ import org.opensearch.common.unit.ByteSizeValue; import org.opensearch.common.unit.TimeValue; import org.opensearch.index.translog.Translog; +import org.opensearch.indices.replication.common.ReplicationType; import org.opensearch.ingest.IngestService; import org.opensearch.node.Node; @@ -530,6 +531,7 @@ public final class IndexSettings { private final String nodeName; private final Settings nodeSettings; private final int numberOfShards; + private final ReplicationType replicationType; // volatile fields are updated via #updateIndexMetadata(IndexMetadata) under lock private volatile Settings settings; private volatile IndexMetadata indexMetadata; @@ -681,6 +683,7 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti nodeName = Node.NODE_NAME_SETTING.get(settings); this.indexMetadata = indexMetadata; numberOfShards = settings.getAsInt(IndexMetadata.SETTING_NUMBER_OF_SHARDS, null); + replicationType = ReplicationType.parseString(settings.get(IndexMetadata.SETTING_REPLICATION_TYPE)); this.searchThrottled = INDEX_SEARCH_THROTTLED.get(settings); this.queryStringLenient = QUERY_STRING_LENIENT_SETTING.get(settings); @@ -915,6 +918,13 @@ public int getNumberOfReplicas() { return settings.getAsInt(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, null); } + /** + * Returns true if segment replication is enabled on the index. + */ + public boolean isSegRepEnabled() { + return ReplicationType.SEGMENT.equals(replicationType); + } + /** * 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/indices/replication/common/ReplicationType.java b/server/src/main/java/org/opensearch/indices/replication/common/ReplicationType.java new file mode 100644 index 0000000000000..98d68d67ba5e3 --- /dev/null +++ b/server/src/main/java/org/opensearch/indices/replication/common/ReplicationType.java @@ -0,0 +1,30 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.indices.replication.common; + +/** + * Enumerates the types of replication strategies supported by OpenSearch. + * For more information, see https://github.com/opensearch-project/OpenSearch/issues/1694 + */ +public enum ReplicationType { + + DOCUMENT, + SEGMENT; + + public static ReplicationType parseString(String replicationType) { + try { + return ReplicationType.valueOf(replicationType); + } catch (IllegalArgumentException e) { + throw new IllegalArgumentException("Could not parse ReplicationStrategy for [" + replicationType + "]"); + } catch (NullPointerException npe) { + // return a default value for null input + return DOCUMENT; + } + } +} diff --git a/server/src/main/resources/org/opensearch/bootstrap/security.policy b/server/src/main/resources/org/opensearch/bootstrap/security.policy index 05d648212bc40..3671782b9d12f 100644 --- a/server/src/main/resources/org/opensearch/bootstrap/security.policy +++ b/server/src/main/resources/org/opensearch/bootstrap/security.policy @@ -120,6 +120,9 @@ grant { // TODO: set this with gradle or some other way that repros with seed? permission java.util.PropertyPermission "processors.override", "write"; + // needed for feature flags + permission java.util.PropertyPermission "opensearch.experimental.feature.*", "write"; + // TODO: these simply trigger a noisy warning if its unable to clear the properties // fix that in randomizedtesting permission java.util.PropertyPermission "junit4.childvm.count", "write"; diff --git a/server/src/test/java/org/opensearch/common/util/FeatureFlagTests.java b/server/src/test/java/org/opensearch/common/util/FeatureFlagTests.java new file mode 100644 index 0000000000000..1084f9c658db4 --- /dev/null +++ b/server/src/test/java/org/opensearch/common/util/FeatureFlagTests.java @@ -0,0 +1,43 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.common.util; + +import org.junit.BeforeClass; +import org.opensearch.common.SuppressForbidden; +import org.opensearch.test.OpenSearchTestCase; + +import java.security.AccessController; +import java.security.PrivilegedAction; + +public class FeatureFlagTests extends OpenSearchTestCase { + + @SuppressForbidden(reason = "sets the feature flag") + @BeforeClass + public static void enableFeature() { + AccessController.doPrivileged((PrivilegedAction) () -> System.setProperty(FeatureFlags.REPLICATION_TYPE, "true")); + } + + public void testReplicationTypeFeatureFlag() { + String replicationTypeFlag = FeatureFlags.REPLICATION_TYPE; + assertNotNull(System.getProperty(replicationTypeFlag)); + assertTrue(FeatureFlags.isEnabled(replicationTypeFlag)); + } + + public void testMissingFeatureFlag() { + String testFlag = "missingFeatureFlag"; + assertNull(System.getProperty(testFlag)); + assertFalse(FeatureFlags.isEnabled(testFlag)); + } + + public void testNonBooleanFeatureFlag() { + String javaVersionProperty = "java.version"; + assertNotNull(System.getProperty(javaVersionProperty)); + assertFalse(FeatureFlags.isEnabled(javaVersionProperty)); + } +}