Skip to content

Commit

Permalink
Removing segrep experimental flag marking completion of intergation w…
Browse files Browse the repository at this point in the history
…ith remote store

Signed-off-by: Shourya Dutta Biswas <114977491+shourya035@users.noreply.github.com>
  • Loading branch information
shourya035 committed Sep 5, 2023
1 parent 3c183a3 commit 4ecfe6a
Show file tree
Hide file tree
Showing 17 changed files with 16 additions and 103 deletions.
11 changes: 0 additions & 11 deletions distribution/src/config/opensearch.yml
Original file line number Diff line number Diff line change
Expand Up @@ -99,17 +99,6 @@ ${path.logs}
#
# ---------------------------------- Experimental Features -----------------------------------
#
# Gates the visibility of the experimental segment replication features until they are production ready.
#
#opensearch.experimental.feature.segment_replication_experimental.enabled: false
#
#
# Gates the visibility of the index setting that allows persisting data to remote store along with local disk.
# Once the feature is ready for production release, this feature flag can be removed.
#
#opensearch.experimental.feature.remote_store.enabled: false
#
#
# Gates the functionality of a new parameter to the snapshot restore API
# that allows for creation of a new index type that searches a snapshot
# directly in a remote repository without restoring all index data to disk
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import org.opensearch.core.rest.RestStatus;
import org.opensearch.index.shard.IndexShard;
import org.opensearch.index.shard.IndexShardState;
import org.opensearch.indices.IndicesService;
import org.opensearch.indices.replication.SegmentReplicationBaseIT;
import org.opensearch.indices.replication.common.ReplicationType;
import org.opensearch.plugins.Plugin;
Expand All @@ -31,7 +30,6 @@
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import org.opensearch.common.Nullable;
import org.opensearch.common.lease.Releasable;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.FeatureFlags;
import org.opensearch.core.index.Index;
import org.opensearch.core.index.shard.ShardId;
import org.opensearch.index.IndexModule;
Expand Down Expand Up @@ -204,8 +203,7 @@ protected IndexShard getIndexShard(String node, String indexName) {
}

protected boolean segmentReplicationWithRemoteEnabled() {
return IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING.get(indexSettings()).booleanValue()
&& "true".equalsIgnoreCase(featureFlagSettings().get(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL));
return IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING.get(indexSettings()).booleanValue();
}

protected Releasable blockReplication(List<String> nodes, CountDownLatch latch) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,12 @@
import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.common.UUIDs;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.FeatureFlags;
import org.opensearch.core.util.FileSystemUtils;
import org.opensearch.index.IndexModule;
import org.opensearch.index.IndexSettings;
import org.opensearch.index.store.RemoteSegmentStoreDirectory;
import org.opensearch.indices.replication.common.ReplicationType;
import org.opensearch.snapshots.AbstractSnapshotIntegTestCase;
import org.opensearch.test.FeatureFlagSetter;
import org.junit.Before;

import java.io.IOException;
Expand All @@ -40,7 +38,6 @@ public abstract class AbstractRemoteStoreMockRepositoryIntegTestCase extends Abs

@Before
public void setup() {
FeatureFlagSetter.set(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL);
internalCluster().startClusterManagerOnlyNode(remoteStoreClusterSettings(REPOSITORY_NAME, TRANSLOG_REPOSITORY_NAME));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.FeatureFlags;
import org.opensearch.indices.recovery.IndexPrimaryRelocationIT;
import org.opensearch.indices.replication.common.ReplicationType;
import org.opensearch.test.OpenSearchIntegTestCase;
Expand Down Expand Up @@ -55,11 +54,6 @@ public Settings indexSettings() {
.build();
}

@Override
protected Settings featureFlagSettings() {
return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL, "true").build();
}

@AwaitsFix(bugUrl = "https://github.com/opensearch-project/OpenSearch/issues/9191")
public void testPrimaryRelocationWhileIndexing() throws Exception {
super.testPrimaryRelocationWhileIndexing();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.FeatureFlags;
import org.opensearch.index.IndexModule;
import org.opensearch.index.IndexSettings;
import org.opensearch.indices.recovery.IndexRecoveryIT;
Expand Down Expand Up @@ -38,11 +37,6 @@ protected Settings nodeSettings(int nodeOrdinal) {
return Settings.builder().put(super.nodeSettings(nodeOrdinal)).put(remoteStoreClusterSettings(REPOSITORY_NAME)).build();
}

@Override
protected Settings featureFlagSettings() {
return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL, "true").build();
}

@Before
@Override
public void setUp() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.common.UUIDs;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.FeatureFlags;
import org.opensearch.index.IndexModule;
import org.opensearch.index.IndexSettings;
import org.opensearch.index.mapper.MapperService;
Expand Down Expand Up @@ -114,11 +113,6 @@ protected Settings nodeSettings(int nodeOrdinal) {
.build();
}

@Override
protected Settings featureFlagSettings() {
return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL, "true").build();
}

public Settings indexSettings() {
return defaultIndexSettings();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
package org.opensearch.remotestore;

import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.FeatureFlags;
import org.opensearch.indices.replication.SegmentReplicationIT;
import org.opensearch.test.OpenSearchIntegTestCase;
import org.junit.After;
Expand All @@ -22,9 +21,6 @@

/**
* This class runs Segment Replication Integ test suite with remote store enabled.
* Setup is similar to SegmentReplicationRemoteStoreIT but this also enables the segment replication using remote store which
* is behind SEGMENT_REPLICATION_EXPERIMENTAL flag. After this is moved out of experimental, we can combine and keep only one
* test suite for Segment and Remote store integration tests.
*/
@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0)
public class SegmentReplicationUsingRemoteStoreIT extends SegmentReplicationIT {
Expand All @@ -40,11 +36,6 @@ protected boolean segmentReplicationWithRemoteEnabled() {
return true;
}

@Override
protected Settings featureFlagSettings() {
return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL, "true").build();
}

@Before
public void setup() {
internalCluster().startClusterManagerOnlyNode();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
package org.opensearch.remotestore;

import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.FeatureFlags;
import org.opensearch.index.SegmentReplicationPressureIT;
import org.opensearch.test.OpenSearchIntegTestCase;
import org.junit.After;
Expand All @@ -22,8 +21,6 @@

/**
* This class executes the SegmentReplicationPressureIT suite with remote store integration enabled.
* Setup is similar to SegmentReplicationPressureIT but this also enables the segment replication using remote store which
* is behind SEGMENT_REPLICATION_EXPERIMENTAL flag.
*/
@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0)
public class SegmentReplicationWithRemoteStorePressureIT extends SegmentReplicationPressureIT {
Expand All @@ -35,11 +32,6 @@ protected boolean segmentReplicationWithRemoteEnabled() {
return true;
}

@Override
protected Settings featureFlagSettings() {
return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL, "true").build();
}

@Override
protected Settings nodeSettings(int nodeOrdinal) {
return Settings.builder().put(super.nodeSettings(nodeOrdinal)).put(remoteStoreClusterSettings(REPOSITORY_NAME)).build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
import org.opensearch.cluster.SnapshotsInProgress;
import org.opensearch.common.action.ActionFuture;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.FeatureFlags;
import org.opensearch.threadpool.ThreadPool;

import java.nio.file.Path;
Expand All @@ -56,7 +55,6 @@ protected Settings nodeSettings(int nodeOrdinal) {
return Settings.builder()
.put(super.nodeSettings(nodeOrdinal))
.put(ThreadPool.ESTIMATED_TIME_INTERVAL_SETTING.getKey(), 0) // We have tests that check by-timestamp order
.put(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL, "true")
.put(remoteStoreClusterSettings("remote-store-repo-name"))
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ protected FeatureFlagSettings(
public static final Set<Setting<?>> BUILT_IN_FEATURE_FLAGS = Collections.unmodifiableSet(
new HashSet<>(
Arrays.asList(
FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL_SETTING,
FeatureFlags.EXTENSIONS_SETTING,
FeatureFlags.IDENTITY_SETTING,
FeatureFlags.CONCURRENT_SEGMENT_SEARCH_SETTING,
Expand Down
13 changes: 0 additions & 13 deletions server/src/main/java/org/opensearch/common/util/FeatureFlags.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,6 @@
* @opensearch.internal
*/
public class FeatureFlags {

/**
* Gates the visibility of the segment replication experimental features that allows users to test unreleased beta features.
*/
public static final String SEGMENT_REPLICATION_EXPERIMENTAL =
"opensearch.experimental.feature.segment_replication_experimental.enabled";

/**
* Gates the ability for Searchable Snapshots to read snapshots that are older than the
* guaranteed backward compatibility for OpenSearch (one prior major version) on a best effort basis.
Expand Down Expand Up @@ -84,12 +77,6 @@ public static boolean isEnabled(String featureFlagName) {
return settings != null && settings.getAsBoolean(featureFlagName, false);
}

public static final Setting<Boolean> SEGMENT_REPLICATION_EXPERIMENTAL_SETTING = Setting.boolSetting(
SEGMENT_REPLICATION_EXPERIMENTAL,
false,
Property.NodeScope
);

public static final Setting<Boolean> EXTENSIONS_SETTING = Setting.boolSetting(EXTENSIONS, false, Property.NodeScope);

public static final Setting<Boolean> IDENTITY_SETTING = Setting.boolSetting(IDENTITY, false, Property.NodeScope);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1052,7 +1052,7 @@ public boolean isSegRepLocalEnabled() {
}

public boolean isSegRepWithRemoteEnabled() {
return isSegRepEnabled() && isRemoteStoreEnabled() && FeatureFlags.isEnabled(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL);
return isSegRepEnabled() && isRemoteStoreEnabled();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1365,7 +1365,13 @@ public void testRemoteStoreOverrideSegmentRepoIndexSettings() {
assertThat(validationErrors.size(), is(1));
assertThat(
validationErrors.get(0),
is(String.format(Locale.ROOT, "private index setting [%s] can not be set explicitly", SETTING_REMOTE_SEGMENT_STORE_REPOSITORY))
is(
String.format(
Locale.ROOT,
"private index setting [%s] can not be set explicitly",
SETTING_REMOTE_SEGMENT_STORE_REPOSITORY
)
)
);
}));
}
Expand Down Expand Up @@ -1398,7 +1404,13 @@ public void testRemoteStoreOverrideTranslogRepoIndexSettings() {
assertThat(validationErrors.size(), is(1));
assertThat(
validationErrors.get(0),
is(String.format(Locale.ROOT, "private index setting [%s] can not be set explicitly", SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY))
is(
String.format(
Locale.ROOT,
"private index setting [%s] can not be set explicitly",
SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY
)
)
);
}));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.common.concurrent.GatedCloseable;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.FeatureFlags;
import org.opensearch.core.action.ActionListener;
import org.opensearch.index.engine.DocIdSeqNoAndSource;
import org.opensearch.index.engine.Engine;
Expand All @@ -34,7 +33,6 @@
import org.opensearch.indices.replication.common.ReplicationType;
import org.hamcrest.MatcherAssert;
import org.junit.Assert;
import org.junit.Before;

import java.io.IOException;
import java.nio.file.Path;
Expand Down Expand Up @@ -65,14 +63,6 @@ public class RemoteIndexShardTests extends SegmentReplicationIndexShardTests {
.put(IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, REPOSITORY_NAME)
.build();

@Before
public void setup() {
// Todo: Remove feature flag once remote store integration with segrep goes GA
FeatureFlags.initializeFeatureFlags(
Settings.builder().put(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL_SETTING.getKey(), "true").build()
);
}

protected Settings getIndexSettings() {
return settings;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import org.opensearch.cluster.routing.ShardRouting;
import org.opensearch.cluster.routing.ShardRoutingState;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.FeatureFlags;
import org.opensearch.index.IndexSettings;
import org.opensearch.index.engine.DocIdSeqNoAndSource;
import org.opensearch.index.engine.NRTReplicationEngine;
Expand All @@ -25,7 +24,6 @@
import org.opensearch.indices.recovery.RecoveryTarget;
import org.opensearch.indices.replication.common.ReplicationType;
import org.junit.Assert;
import org.junit.Before;

import java.io.IOException;
import java.nio.file.Path;
Expand All @@ -43,14 +41,6 @@ public class ReplicaRecoveryWithRemoteTranslogOnPrimaryTests extends OpenSearchI
.put(IndexSettings.INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING.getKey(), "100ms")
.build();

@Before
public void setup() {
// Todo: Remove feature flag once remote store integration with segrep goes GA
FeatureFlags.initializeFeatureFlags(
Settings.builder().put(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL_SETTING.getKey(), "true").build()
);
}

public void testStartSequenceForReplicaRecovery() throws Exception {
final Path remoteDir = createTempDir();
final String indexMapping = "{ \"" + MapperService.SINGLE_MAPPING_NAME + "\": {} }";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,13 @@

import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.FeatureFlags;
import org.opensearch.index.IndexSettings;
import org.opensearch.index.engine.NRTReplicationEngineFactory;
import org.opensearch.index.mapper.MapperService;
import org.opensearch.index.replication.OpenSearchIndexLevelReplicationTestCase;
import org.opensearch.index.seqno.ReplicationTracker;
import org.opensearch.index.shard.IndexShard;
import org.opensearch.indices.replication.common.ReplicationType;
import org.junit.Before;

import java.nio.file.Path;

Expand All @@ -31,14 +29,6 @@ public class RemoteStorePeerRecoverySourceHandlerTests extends OpenSearchIndexLe
.put(IndexSettings.INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING.getKey(), "100ms")
.build();

@Before
public void setup() {
// Todo: Remove feature flag once remote store integration with segrep goes GA
FeatureFlags.initializeFeatureFlags(
Settings.builder().put(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL_SETTING.getKey(), "true").build()
);
}

public void testReplicaShardRecoveryUptoLastFlushedCommit() throws Exception {
final Path remoteDir = createTempDir();
final String indexMapping = "{ \"" + MapperService.SINGLE_MAPPING_NAME + "\": {} }";
Expand Down

0 comments on commit 4ecfe6a

Please sign in to comment.