Skip to content

Commit d8217bf

Browse files
author
Aditya Khera
committed
Removing FeatureFlags.MERGED_SEGMENT_WARMER_EXPERIMENTAL_FLAG
Signed-off-by: Aditya Khera <kheraadi@amazon.com>
1 parent d7e9e09 commit d8217bf

File tree

15 files changed

+11
-112
lines changed

15 files changed

+11
-112
lines changed

server/src/internalClusterTest/java/org/opensearch/indices/replication/MergedSegmentWarmerIT.java

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import org.opensearch.action.support.WriteRequest;
1919
import org.opensearch.common.settings.Settings;
2020
import org.opensearch.common.unit.TimeValue;
21-
import org.opensearch.common.util.FeatureFlags;
2221
import org.opensearch.common.util.set.Sets;
2322
import org.opensearch.index.IndexSettings;
2423
import org.opensearch.index.TieredMergePolicyProvider;
@@ -54,13 +53,6 @@ protected Settings nodeSettings(int nodeOrdinal) {
5453
.build();
5554
}
5655

57-
@Override
58-
protected Settings featureFlagSettings() {
59-
Settings.Builder featureSettings = Settings.builder();
60-
featureSettings.put(FeatureFlags.MERGED_SEGMENT_WARMER_EXPERIMENTAL_FLAG, true);
61-
return featureSettings.build();
62-
}
63-
6456
public void testPrimaryNodeRestart() throws Exception {
6557
logger.info("--> start nodes");
6658
internalCluster().startNode();

server/src/internalClusterTest/java/org/opensearch/indices/replication/RemoteStoreMergedSegmentWarmerIT.java

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
import org.opensearch.action.support.WriteRequest;
1414
import org.opensearch.action.support.replication.TransportReplicationAction;
1515
import org.opensearch.common.settings.Settings;
16-
import org.opensearch.common.util.FeatureFlags;
1716
import org.opensearch.index.IndexSettings;
1817
import org.opensearch.index.TieredMergePolicyProvider;
1918
import org.opensearch.indices.recovery.RecoverySettings;
@@ -47,13 +46,6 @@ protected Settings nodeSettings(int nodeOrdinal) {
4746
.build();
4847
}
4948

50-
@Override
51-
protected Settings featureFlagSettings() {
52-
Settings.Builder featureSettings = Settings.builder();
53-
featureSettings.put(FeatureFlags.MERGED_SEGMENT_WARMER_EXPERIMENTAL_FLAG, true);
54-
return featureSettings.build();
55-
}
56-
5749
@Before
5850
public void setup() {
5951
internalCluster().startClusterManagerOnlyNode();

server/src/internalClusterTest/java/org/opensearch/merge/MergeStatsIT.java

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import org.opensearch.cluster.node.DiscoveryNode;
2525
import org.opensearch.cluster.routing.allocation.decider.ShardsLimitAllocationDecider;
2626
import org.opensearch.common.settings.Settings;
27-
import org.opensearch.common.util.FeatureFlags;
2827
import org.opensearch.core.common.unit.ByteSizeValue;
2928
import org.opensearch.index.merge.MergeStats;
3029
import org.opensearch.index.merge.MergedSegmentWarmerStats;
@@ -64,13 +63,6 @@ public Settings indexSettings() {
6463
.build();
6564
}
6665

67-
@Override
68-
protected Settings featureFlagSettings() {
69-
Settings.Builder featureSettings = Settings.builder();
70-
featureSettings.put(FeatureFlags.MERGED_SEGMENT_WARMER_EXPERIMENTAL_FLAG, true);
71-
return featureSettings.build();
72-
}
73-
7466
private void setup() {
7567
internalCluster().startNodes(2);
7668
}

server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ protected FeatureFlagSettings(
3838
FeatureFlags.APPLICATION_BASED_CONFIGURATION_TEMPLATES_SETTING,
3939
FeatureFlags.TERM_VERSION_PRECOMMIT_ENABLE_SETTING,
4040
FeatureFlags.ARROW_STREAMS_SETTING,
41-
FeatureFlags.STREAM_TRANSPORT_SETTING,
42-
FeatureFlags.MERGED_SEGMENT_WARMER_EXPERIMENTAL_SETTING
41+
FeatureFlags.STREAM_TRANSPORT_SETTING
4342
);
4443
}

server/src/main/java/org/opensearch/common/util/FeatureFlags.java

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,6 @@ public class FeatureFlags {
6363
*/
6464
public static final String BACKGROUND_TASK_EXECUTION_EXPERIMENTAL = FEATURE_FLAG_PREFIX + "task.background.enabled";
6565

66-
/**
67-
* Gates the functionality of merged segment warmer in local/remote segment replication.
68-
* Once the feature is ready for release, this feature flag can be removed.
69-
*/
70-
public static final String MERGED_SEGMENT_WARMER_EXPERIMENTAL_FLAG = "opensearch.experimental.feature.merged_segment_warmer.enabled";
71-
7266
public static final Setting<Boolean> REMOTE_STORE_MIGRATION_EXPERIMENTAL_SETTING = Setting.boolSetting(
7367
REMOTE_STORE_MIGRATION_EXPERIMENTAL,
7468
false,
@@ -91,12 +85,6 @@ public class FeatureFlags {
9185
Property.NodeScope
9286
);
9387

94-
public static final Setting<Boolean> MERGED_SEGMENT_WARMER_EXPERIMENTAL_SETTING = Setting.boolSetting(
95-
MERGED_SEGMENT_WARMER_EXPERIMENTAL_FLAG,
96-
false,
97-
Property.NodeScope
98-
);
99-
10088
/**
10189
* Gates the functionality of application based configuration templates.
10290
*/
@@ -145,7 +133,6 @@ static class FeatureFlagsImpl {
145133
put(TERM_VERSION_PRECOMMIT_ENABLE_SETTING, TERM_VERSION_PRECOMMIT_ENABLE_SETTING.getDefault(Settings.EMPTY));
146134
put(ARROW_STREAMS_SETTING, ARROW_STREAMS_SETTING.getDefault(Settings.EMPTY));
147135
put(STREAM_TRANSPORT_SETTING, STREAM_TRANSPORT_SETTING.getDefault(Settings.EMPTY));
148-
put(MERGED_SEGMENT_WARMER_EXPERIMENTAL_SETTING, MERGED_SEGMENT_WARMER_EXPERIMENTAL_SETTING.getDefault(Settings.EMPTY));
149136
}
150137
};
151138

server/src/main/java/org/opensearch/index/IndexService.java

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -341,9 +341,8 @@ public IndexService(
341341
this.retentionLeaseSyncTask = new AsyncRetentionLeaseSyncTask(this);
342342
}
343343
this.asyncReplicationTask = new AsyncReplicationTask(this);
344-
if (FeatureFlags.isEnabled(FeatureFlags.MERGED_SEGMENT_WARMER_EXPERIMENTAL_SETTING)) {
345-
this.asyncPublishReferencedSegmentsTask = new AsyncPublishReferencedSegmentsTask(this);
346-
}
344+
this.asyncPublishReferencedSegmentsTask = new AsyncPublishReferencedSegmentsTask(this);
345+
347346
this.translogFactorySupplier = translogFactorySupplier;
348347
this.recoverySettings = recoverySettings;
349348
this.remoteStoreSettings = remoteStoreSettings;
@@ -1201,9 +1200,7 @@ public synchronized void updateMetadata(final IndexMetadata currentIndexMetadata
12011200
onRefreshIntervalChange();
12021201
updateFsyncTaskIfNecessary();
12031202
updateReplicationTask();
1204-
if (FeatureFlags.isEnabled(FeatureFlags.MERGED_SEGMENT_WARMER_EXPERIMENTAL_SETTING)) {
1205-
updatePublishReferencedSegmentsTask();
1206-
}
1203+
updatePublishReferencedSegmentsTask();
12071204
}
12081205

12091206
metadataListeners.forEach(c -> c.accept(newIndexMetadata));

server/src/main/java/org/opensearch/index/engine/InternalEngine.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,6 @@
8585
import org.opensearch.common.lucene.uid.VersionsAndSeqNoResolver.DocIdAndSeqNo;
8686
import org.opensearch.common.metrics.CounterMetric;
8787
import org.opensearch.common.unit.TimeValue;
88-
import org.opensearch.common.util.FeatureFlags;
8988
import org.opensearch.common.util.concurrent.AbstractRunnable;
9089
import org.opensearch.common.util.concurrent.KeyedLock;
9190
import org.opensearch.common.util.concurrent.ReleasableLock;
@@ -2398,8 +2397,7 @@ private IndexWriterConfig getIndexWriterConfig() {
23982397
if (config().getLeafSorter() != null) {
23992398
iwc.setLeafSorter(config().getLeafSorter()); // The default segment search order
24002399
}
2401-
if (FeatureFlags.isEnabled(FeatureFlags.MERGED_SEGMENT_WARMER_EXPERIMENTAL_SETTING)
2402-
&& config().getIndexSettings().isSegRepEnabledOrRemoteNode()) {
2400+
if (config().getIndexSettings().isSegRepEnabledOrRemoteNode()) {
24032401
assert null != config().getIndexReaderWarmer();
24042402
iwc.setMergedSegmentWarmer(config().getIndexReaderWarmer());
24052403
}

server/src/main/java/org/opensearch/index/engine/MergedSegmentWarmerFactory.java

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
import org.apache.lucene.index.IndexWriter;
1212
import org.opensearch.cluster.service.ClusterService;
1313
import org.opensearch.common.annotation.ExperimentalApi;
14-
import org.opensearch.common.util.FeatureFlags;
1514
import org.opensearch.index.shard.IndexShard;
1615
import org.opensearch.indices.recovery.RecoverySettings;
1716
import org.opensearch.transport.TransportService;
@@ -35,12 +34,10 @@ public MergedSegmentWarmerFactory(TransportService transportService, RecoverySet
3534
}
3635

3736
public IndexWriter.IndexReaderWarmer get(IndexShard shard) {
38-
if (FeatureFlags.isEnabled(FeatureFlags.MERGED_SEGMENT_WARMER_EXPERIMENTAL_FLAG) == false
39-
|| shard.indexSettings().isDocumentReplication()) {
37+
if (shard.indexSettings().isDocumentReplication()) {
4038
// MergedSegmentWarmerFactory#get is called by IndexShard#newEngineConfig on the initialization of a new indexShard and
41-
// in cases of updates to shard state.
42-
// 1. IndexWriter.IndexReaderWarmer should be null when IndexMetadata.INDEX_REPLICATION_TYPE_SETTING == ReplicationType.DOCUMENT
43-
// 2. IndexWriter.IndexReaderWarmer should be null when the FeatureFlags.MERGED_SEGMENT_WARMER_EXPERIMENTAL_FLAG == false
39+
// in case of updates to shard state.
40+
// - IndexWriter.IndexReaderWarmer should be null when IndexMetadata.INDEX_REPLICATION_TYPE_SETTING == ReplicationType.DOCUMENT
4441
return null;
4542
} else if (shard.indexSettings().isSegRepLocalEnabled() || shard.indexSettings().isRemoteStoreEnabled()) {
4643
return new MergedSegmentWarmer(transportService, recoverySettings, clusterService, shard);

server/src/main/java/org/opensearch/indices/recovery/RecoverySettings.java

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,8 @@
4040
import org.opensearch.common.settings.ClusterSettings;
4141
import org.opensearch.common.settings.Setting;
4242
import org.opensearch.common.settings.Setting.Property;
43-
import org.opensearch.common.settings.Setting.Validator;
4443
import org.opensearch.common.settings.Settings;
4544
import org.opensearch.common.unit.TimeValue;
46-
import org.opensearch.common.util.FeatureFlags;
4745
import org.opensearch.common.util.concurrent.OpenSearchExecutors;
4846
import org.opensearch.core.common.unit.ByteSizeUnit;
4947
import org.opensearch.core.common.unit.ByteSizeValue;
@@ -83,18 +81,6 @@ public class RecoverySettings {
8381
public static final Setting<Boolean> INDICES_MERGED_SEGMENT_REPLICATION_WARMER_ENABLED_SETTING = Setting.boolSetting(
8482
"indices.replication.merged_segment_warmer_enabled",
8583
false,
86-
new Validator<Boolean>() {
87-
@Override
88-
public void validate(Boolean value) {
89-
if (FeatureFlags.isEnabled(FeatureFlags.MERGED_SEGMENT_WARMER_EXPERIMENTAL_FLAG) == false && value == true) {
90-
throw new IllegalArgumentException(
91-
"FeatureFlag "
92-
+ FeatureFlags.MERGED_SEGMENT_WARMER_EXPERIMENTAL_FLAG
93-
+ " must be enabled to set this property to true."
94-
);
95-
}
96-
}
97-
},
9884
Property.Dynamic,
9985
Property.NodeScope
10086
);

server/src/main/java/org/opensearch/node/Node.java

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1682,16 +1682,10 @@ protected Node(final Environment initialEnvironment, Collection<PluginInfo> clas
16821682
b.bind(MergedSegmentWarmerFactory.class).toInstance(mergedSegmentWarmerFactory);
16831683
b.bind(MappingTransformerRegistry.class).toInstance(mappingTransformerRegistry);
16841684
b.bind(AutoForceMergeManager.class).toInstance(autoForceMergeManager);
1685-
if (FeatureFlags.isEnabled(FeatureFlags.MERGED_SEGMENT_WARMER_EXPERIMENTAL_FLAG)) {
1686-
if (isRemoteDataAttributePresent(settings)) {
1687-
b.bind(MergedSegmentPublisher.PublishAction.class)
1688-
.to(RemoteStorePublishMergedSegmentAction.class)
1689-
.asEagerSingleton();
1690-
} else {
1691-
b.bind(MergedSegmentPublisher.PublishAction.class).to(PublishMergedSegmentAction.class).asEagerSingleton();
1692-
}
1685+
if (isRemoteDataAttributePresent(settings)) {
1686+
b.bind(MergedSegmentPublisher.PublishAction.class).to(RemoteStorePublishMergedSegmentAction.class).asEagerSingleton();
16931687
} else {
1694-
b.bind(MergedSegmentPublisher.PublishAction.class).toInstance((shard, checkpoint) -> {});
1688+
b.bind(MergedSegmentPublisher.PublishAction.class).to(PublishMergedSegmentAction.class).asEagerSingleton();
16951689
}
16961690
b.bind(MergedSegmentPublisher.class).asEagerSingleton();
16971691

0 commit comments

Comments
 (0)