Skip to content

Commit

Permalink
Move setting from Built-in to feature flag
Browse files Browse the repository at this point in the history
Signed-off-by: Ashish Singh <ssashish@amazon.com>
  • Loading branch information
ashking94 committed Jan 20, 2023
1 parent 01973d7 commit 415a1cf
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
import org.opensearch.common.settings.Setting;
import org.opensearch.common.settings.Setting.Property;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.unit.TimeValue;
import org.opensearch.common.xcontent.ToXContent;
import org.opensearch.common.xcontent.ToXContentFragment;
import org.opensearch.common.xcontent.XContentBuilder;
Expand Down Expand Up @@ -301,6 +302,8 @@ public Iterator<Setting<?>> settings() {

public static final String SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY = "index.remote_store.translog.repository";

public static final String SETTING_REMOTE_TRANSLOG_BUFFER_INTERVAL = "index.remote_store.translog.buffer_interval";

/**
* Used to specify if the index data should be persisted in the remote store.
*/
Expand Down Expand Up @@ -446,6 +449,45 @@ public Iterator<Setting<?>> settings() {
Property.Final
);

public static final Setting<TimeValue> INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING = Setting.timeSetting(
SETTING_REMOTE_TRANSLOG_BUFFER_INTERVAL,
TimeValue.timeValueMillis(100),
TimeValue.timeValueMillis(50),
new Setting.Validator<>() {

@Override
public void validate(final TimeValue value) {}

@Override
public void validate(final TimeValue value, final Map<Setting<?>, Object> settings) {
if (value == null) {
throw new IllegalArgumentException(
"Setting " + SETTING_REMOTE_TRANSLOG_BUFFER_INTERVAL + " should be provided with a valid time value"
);
} else {
final Boolean isRemoteTranslogStoreEnabled = (Boolean) settings.get(INDEX_REMOTE_TRANSLOG_STORE_ENABLED_SETTING);
if (isRemoteTranslogStoreEnabled == null || isRemoteTranslogStoreEnabled == false) {
throw new IllegalArgumentException(
"Settings "
+ SETTING_REMOTE_TRANSLOG_BUFFER_INTERVAL
+ " can only be set when "
+ SETTING_REMOTE_TRANSLOG_STORE_ENABLED
+ " is set to true"
);
}
}
}

@Override
public Iterator<Setting<?>> settings() {
final List<Setting<?>> settings = Collections.singletonList(INDEX_REMOTE_TRANSLOG_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<AutoExpandReplicas> INDEX_AUTO_EXPAND_REPLICAS_SETTING = AutoExpandReplicas.SETTING;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,6 @@ public final class IndexScopedSettings extends AbstractScopedSettings {
IndexSettings.MAX_ANALYZED_OFFSET_SETTING,
IndexSettings.MAX_TERMS_COUNT_SETTING,
IndexSettings.INDEX_TRANSLOG_SYNC_INTERVAL_SETTING,
IndexSettings.INDEX_REMOTE_STORE_TRANSLOG_BUFFER_INTERVAL_SETTING,
IndexSettings.DEFAULT_FIELD_SETTING,
IndexSettings.QUERY_STRING_LENIENT_SETTING,
IndexSettings.ALLOW_UNMAPPED,
Expand Down Expand Up @@ -228,7 +227,8 @@ public final class IndexScopedSettings extends AbstractScopedSettings {
IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING,
IndexMetadata.INDEX_REMOTE_STORE_REPOSITORY_SETTING,
IndexMetadata.INDEX_REMOTE_TRANSLOG_STORE_ENABLED_SETTING,
IndexMetadata.INDEX_REMOTE_TRANSLOG_REPOSITORY_SETTING
IndexMetadata.INDEX_REMOTE_TRANSLOG_REPOSITORY_SETTING,
IndexMetadata.INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING
),
FeatureFlags.SEARCHABLE_SNAPSHOT,
List.of(
Expand Down
27 changes: 27 additions & 0 deletions server/src/main/java/org/opensearch/common/settings/Setting.java
Original file line number Diff line number Diff line change
Expand Up @@ -2073,6 +2073,23 @@ public static Setting<TimeValue> timeSetting(
);
}

public static Setting<TimeValue> timeSetting(
final String key,
Function<Settings, TimeValue> defaultValue,
final TimeValue minValue,
final Validator<TimeValue> validator,
final Property... properties
) {
final SimpleKey simpleKey = new SimpleKey(key);
return new Setting<>(
simpleKey,
s -> defaultValue.apply(s).getStringRep(),
minTimeValueParser(key, minValue, isFiltered(properties)),
validator,
properties
);
}

public static Setting<TimeValue> timeSetting(
final String key,
Function<Settings, TimeValue> defaultValue,
Expand Down Expand Up @@ -2173,6 +2190,16 @@ public static Setting<TimeValue> timeSetting(String key, Setting<TimeValue> fall
return new Setting<>(key, fallbackSetting, (s) -> TimeValue.parseTimeValue(s, key), properties);
}

public static Setting<TimeValue> timeSetting(
String key,
TimeValue defaultValue,
TimeValue minValue,
Validator<TimeValue> validator,
Property... properties
) {
return timeSetting(key, (s) -> defaultValue, minValue, validator, properties);
}

public static Setting<TimeValue> timeSetting(
String key,
Setting<TimeValue> fallBackSetting,
Expand Down
21 changes: 5 additions & 16 deletions server/src/main/java/org/opensearch/index/IndexSettings.java
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,6 @@ public final class IndexSettings {
Property.Dynamic,
Property.IndexScope
);
public static final Setting<TimeValue> INDEX_REMOTE_STORE_TRANSLOG_BUFFER_INTERVAL_SETTING = Setting.timeSetting(
"index.remote_store.translog.buffer_interval",
TimeValue.timeValueMillis(100),
TimeValue.timeValueMillis(1),
Property.Dynamic,
Property.IndexScope
);
public static final Setting<TimeValue> INDEX_SEARCH_IDLE_AFTER = Setting.timeSetting(
"index.search.idle.after",
TimeValue.timeValueSeconds(30),
Expand Down Expand Up @@ -593,6 +586,7 @@ public final class IndexSettings {
private final ReplicationType replicationType;
private final boolean isRemoteStoreEnabled;
private final boolean isRemoteTranslogStoreEnabled;
private volatile TimeValue bufferInterval;
private final String remoteStoreTranslogRepository;
private final String remoteStoreRepository;
private final boolean isRemoteSnapshot;
Expand All @@ -608,7 +602,6 @@ public final class IndexSettings {
private final boolean defaultAllowUnmappedFields;
private volatile Translog.Durability durability;
private volatile TimeValue syncInterval;
private volatile TimeValue bufferInterval;
private volatile TimeValue refreshInterval;
private volatile ByteSizeValue flushThresholdSize;
private volatile TimeValue translogRetentionAge;
Expand Down Expand Up @@ -761,6 +754,7 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
isRemoteStoreEnabled = settings.getAsBoolean(IndexMetadata.SETTING_REMOTE_STORE_ENABLED, false);
isRemoteTranslogStoreEnabled = settings.getAsBoolean(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_ENABLED, false);
remoteStoreTranslogRepository = settings.get(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY);
bufferInterval = settings.getAsTime(IndexMetadata.SETTING_REMOTE_TRANSLOG_BUFFER_INTERVAL, TimeValue.timeValueMillis(100));
remoteStoreRepository = settings.get(IndexMetadata.SETTING_REMOTE_STORE_REPOSITORY);
isRemoteSnapshot = IndexModule.Type.REMOTE_SNAPSHOT.match(this.settings);

Expand All @@ -778,7 +772,6 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
this.durability = scopedSettings.get(INDEX_TRANSLOG_DURABILITY_SETTING);
defaultFields = scopedSettings.get(DEFAULT_FIELD_SETTING);
syncInterval = INDEX_TRANSLOG_SYNC_INTERVAL_SETTING.get(settings);
bufferInterval = INDEX_REMOTE_STORE_TRANSLOG_BUFFER_INTERVAL_SETTING.get(settings);
refreshInterval = scopedSettings.get(INDEX_REFRESH_INTERVAL_SETTING);
flushThresholdSize = scopedSettings.get(INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING);
generationThresholdSize = scopedSettings.get(INDEX_TRANSLOG_GENERATION_THRESHOLD_SIZE_SETTING);
Expand Down Expand Up @@ -854,7 +847,6 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
scopedSettings.addSettingsUpdateConsumer(MergeSchedulerConfig.AUTO_THROTTLE_SETTING, mergeSchedulerConfig::setAutoThrottle);
scopedSettings.addSettingsUpdateConsumer(INDEX_TRANSLOG_DURABILITY_SETTING, this::setTranslogDurability);
scopedSettings.addSettingsUpdateConsumer(INDEX_TRANSLOG_SYNC_INTERVAL_SETTING, this::setTranslogSyncInterval);
scopedSettings.addSettingsUpdateConsumer(INDEX_REMOTE_STORE_TRANSLOG_BUFFER_INTERVAL_SETTING, this::setBufferInterval);
scopedSettings.addSettingsUpdateConsumer(MAX_RESULT_WINDOW_SETTING, this::setMaxResultWindow);
scopedSettings.addSettingsUpdateConsumer(MAX_INNER_RESULT_WINDOW_SETTING, this::setMaxInnerResultWindow);
scopedSettings.addSettingsUpdateConsumer(MAX_ADJACENCY_MATRIX_FILTERS_SETTING, this::setMaxAdjacencyMatrixFilters);
Expand Down Expand Up @@ -1146,17 +1138,14 @@ public void setTranslogSyncInterval(TimeValue translogSyncInterval) {
}

/**
* TODO
* @return
* Returns the translog sync/upload buffer interval when remote translog store is enabled and index setting
* {@code index.translog.durability} is set as {@code request}.
* @return the buffer interval.
*/
public TimeValue getBufferInterval() {
return bufferInterval;
}

/**
* TODO
* @param bufferInterval
*/
public void setBufferInterval(TimeValue bufferInterval) {
this.bufferInterval = bufferInterval;
}
Expand Down
28 changes: 28 additions & 0 deletions server/src/test/java/org/opensearch/index/IndexSettingsTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -964,11 +964,13 @@ public void testRemoteTranslogExplicitSetting() {
.put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
.put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_ENABLED, true)
.put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, "tlog-store")
.put(IndexMetadata.INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING.getKey(), "200ms")
.build()
);
IndexSettings settings = new IndexSettings(metadata, Settings.EMPTY);
assertNull(settings.getRemoteStoreRepository());
assertEquals("tlog-store", settings.getRemoteStoreTranslogRepository());
assertEquals(TimeValue.timeValueMillis(200), settings.getBufferInterval());
}

public void testSetRemoteTranslogRepositoryFailsWhenRemoteTranslogIsNotEnabled() {
Expand Down Expand Up @@ -1000,6 +1002,32 @@ public void testSetRemoteTranslogRepositoryFailsWhenEmptyString() {
assertEquals("Setting index.remote_store.translog.repository should be provided with non-empty repository ID", iae.getMessage());
}

public void testSetRemoteTranslogBufferIntervalDefaultSetting() {
Version createdVersion = VersionUtils.randomVersionBetween(random(), Version.V_2_0_0, Version.CURRENT);
Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_INDEX_VERSION_CREATED.getKey(), createdVersion)
.put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_ENABLED, true)
.build();
assertTrue(IndexSettings.INDEX_SOFT_DELETES_SETTING.get(settings));
assertEquals(TimeValue.timeValueMillis(100), IndexMetadata.INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING.get(settings));
}

public void testSetRemoteTranslogBufferIntervalFailsWhenRemoteTranslogIsNotEnabled() {
Settings indexSettings = Settings.builder()
.put("index.replication.type", ReplicationType.SEGMENT)
.put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_ENABLED, false)
.put(IndexMetadata.INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING.getKey(), "200ms")
.build();
IllegalArgumentException iae = expectThrows(
IllegalArgumentException.class,
() -> IndexMetadata.INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING.get(indexSettings)
);
assertEquals(
"Settings index.remote_store.translog.buffer_interval can only be set when index.remote_store.translog.enabled is set to true",
iae.getMessage()
);
}

@SuppressForbidden(reason = "sets the SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY feature flag")
public void testExtendedCompatibilityVersionForRemoteSnapshot() throws Exception {
try (FeatureFlagSetter f = FeatureFlagSetter.set(FeatureFlags.SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY)) {
Expand Down

0 comments on commit 415a1cf

Please sign in to comment.