Skip to content

Commit

Permalink
Adds the replication type index setting, alongside a formal notion of…
Browse files Browse the repository at this point in the history
… feature flags (#3037)

* 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 <gkart@amazon.com>

* Update security policy for testing feature flags

Signed-off-by: Nicholas Walter Knize <nknize@apache.org>

Co-authored-by: Nicholas Walter Knize <nknize@apache.org>
(cherry picked from commit ee7b731)
  • Loading branch information
kartg authored and github-actions[bot] committed Apr 25, 2022
1 parent 7b02dc0 commit d1539b7
Show file tree
Hide file tree
Showing 8 changed files with 149 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -260,6 +261,18 @@ public Iterator<Setting<?>> 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<ReplicationType> 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<AutoExpandReplicas> INDEX_AUTO_EXPAND_REPLICAS_SETTING = AutoExpandReplicas.SETTING;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<String, Setting> 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<Setting<?>> settingsSet) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -85,6 +86,12 @@ public SettingsModule(
registerSetting(setting);
}

for (Map.Entry<String, Setting> featureFlaggedSetting : IndexScopedSettings.FEATURE_FLAGGED_INDEX_SETTINGS.entrySet()) {
if (FeatureFlags.isEnabled(featureFlaggedSetting.getKey())) {
registerSetting(featureFlaggedSetting.getValue());
}
}

for (Setting<?> setting : additionalSettings) {
registerSetting(setting);
}
Expand Down
32 changes: 32 additions & 0 deletions server/src/main/java/org/opensearch/common/util/FeatureFlags.java
Original file line number Diff line number Diff line change
@@ -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));
}
}
10 changes: 10 additions & 0 deletions server/src/main/java/org/opensearch/index/IndexSettings.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
Original file line number Diff line number Diff line change
@@ -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<String>) () -> 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));
}
}

0 comments on commit d1539b7

Please sign in to comment.