From a960ec3d7205942021b1379614fbecf1de48754f Mon Sep 17 00:00:00 2001 From: Kunal Kotwani Date: Thu, 13 Apr 2023 15:59:45 -0700 Subject: [PATCH] Remove feature flag for searchable snapshots (#7117) (#7150) Signed-off-by: Kunal Kotwani (cherry picked from commit 1c8421805955341ab692b003416b45b8d55ccd24) --- CHANGELOG.md | 1 + .../snapshots/SearchableSnapshotIT.java | 6 --- .../admin/cluster/node/stats/NodeStats.java | 7 ++- .../restore/RestoreSnapshotRequest.java | 21 +++----- .../put/TransportUpdateSettingsAction.java | 49 ++++++++----------- .../org/opensearch/cluster/ClusterModule.java | 5 +- .../cluster/routing/RecoverySource.java | 5 +- .../allocator/BalancedShardsAllocator.java | 12 ++--- .../allocator/LocalShardsBalancer.java | 49 +++++++------------ .../common/settings/ClusterSettings.java | 7 +-- .../common/settings/FeatureFlagSettings.java | 1 - .../common/settings/IndexScopedSettings.java | 13 +++-- .../opensearch/common/util/FeatureFlags.java | 10 ---- .../org/opensearch/monitor/fs/FsInfo.java | 9 ++-- .../opensearch/snapshots/RestoreService.java | 5 +- .../cluster/ClusterModuleTests.java | 4 +- .../RemoteShardsBalancerBaseTestCase.java | 13 ----- .../java/org/opensearch/node/NodeTests.java | 2 - 18 files changed, 78 insertions(+), 141 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b04f998f684c1..c94d4545d9b7a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add experimental support for ZSTD compression. ([#3577](https://github.com/opensearch-project/OpenSearch/pull/3577)) - [Segment Replication] Add point in time and scroll query compatibility. ([#6644](https://github.com/opensearch-project/OpenSearch/pull/6644)) - Add retry delay as dynamic setting for cluster maanger throttling. ([#6998](https://github.com/opensearch-project/OpenSearch/pull/6998)) +- Introduce full support for searchable snapshots ([#5087](https://github.com/opensearch-project/OpenSearch/issues/5087)) ### Dependencies - Bump `org.apache.logging.log4j:log4j-core` from 2.18.0 to 2.20.0 ([#6490](https://github.com/opensearch-project/OpenSearch/pull/6490)) diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/SearchableSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/SearchableSnapshotIT.java index 23f41f4be51ef..4493137ca81e7 100644 --- a/server/src/internalClusterTest/java/org/opensearch/snapshots/SearchableSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/snapshots/SearchableSnapshotIT.java @@ -26,7 +26,6 @@ import org.opensearch.common.io.PathUtils; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.ByteSizeUnit; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.index.Index; import org.opensearch.index.IndexNotFoundException; import org.opensearch.index.store.remote.file.CleanerDaemonThreadLeakFilter; @@ -60,11 +59,6 @@ protected boolean addMockInternalEngine() { return false; } - @Override - protected Settings featureFlagSettings() { - return Settings.builder().put(FeatureFlags.SEARCHABLE_SNAPSHOT, "true").build(); - } - @Override protected Settings.Builder randomRepositorySettings() { final Settings.Builder settings = Settings.builder(); diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/node/stats/NodeStats.java b/server/src/main/java/org/opensearch/action/admin/cluster/node/stats/NodeStats.java index 837464f2ac419..02bb41302d2f8 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/node/stats/NodeStats.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/node/stats/NodeStats.java @@ -42,7 +42,6 @@ import org.opensearch.common.Nullable; import org.opensearch.common.io.stream.StreamInput; import org.opensearch.common.io.stream.StreamOutput; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.xcontent.ToXContentFragment; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.discovery.DiscoveryStats; @@ -189,7 +188,7 @@ public NodeStats(StreamInput in) throws IOException { } else { weightedRoutingStats = null; } - if (in.getVersion().onOrAfter(Version.V_2_7_0) && FeatureFlags.isEnabled(FeatureFlags.SEARCHABLE_SNAPSHOT)) { + if (in.getVersion().onOrAfter(Version.V_2_7_0)) { fileCacheStats = in.readOptionalWriteable(FileCacheStats::new); } else { fileCacheStats = null; @@ -409,7 +408,7 @@ public void writeTo(StreamOutput out) throws IOException { if (out.getVersion().onOrAfter(Version.V_2_6_0)) { out.writeOptionalWriteable(weightedRoutingStats); } - if (out.getVersion().onOrAfter(Version.V_2_7_0) && FeatureFlags.isEnabled(FeatureFlags.SEARCHABLE_SNAPSHOT)) { + if (out.getVersion().onOrAfter(Version.V_2_7_0)) { out.writeOptionalWriteable(fileCacheStats); } } @@ -493,7 +492,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws if (getWeightedRoutingStats() != null) { getWeightedRoutingStats().toXContent(builder, params); } - if (getFileCacheStats() != null && FeatureFlags.isEnabled(FeatureFlags.SEARCHABLE_SNAPSHOT)) { + if (getFileCacheStats() != null) { getFileCacheStats().toXContent(builder, params); } diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java b/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java index e9c227d211d28..0f135ca6d9cde 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java @@ -43,7 +43,6 @@ import org.opensearch.common.io.stream.StreamOutput; import org.opensearch.common.logging.DeprecationLogger; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.xcontent.ToXContentObject; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.common.xcontent.XContentType; @@ -152,7 +151,7 @@ public RestoreSnapshotRequest(StreamInput in) throws IOException { if (in.getVersion().onOrAfter(LegacyESVersion.V_7_10_0)) { snapshotUuid = in.readOptionalString(); } - if (FeatureFlags.isEnabled(FeatureFlags.SEARCHABLE_SNAPSHOT) && in.getVersion().onOrAfter(Version.V_2_4_0)) { + if (in.getVersion().onOrAfter(Version.V_2_7_0)) { storageType = in.readEnum(StorageType.class); } } @@ -182,7 +181,7 @@ public void writeTo(StreamOutput out) throws IOException { "restricting the snapshot UUID is forbidden in a cluster with version [" + out.getVersion() + "] nodes" ); } - if (FeatureFlags.isEnabled(FeatureFlags.SEARCHABLE_SNAPSHOT) && out.getVersion().onOrAfter(Version.V_2_4_0)) { + if (out.getVersion().onOrAfter(Version.V_2_7_0)) { out.writeEnum(storageType); } } @@ -595,17 +594,13 @@ public RestoreSnapshotRequest source(Map source) { throw new IllegalArgumentException("malformed ignore_index_settings section, should be an array of strings"); } } else if (name.equals("storage_type")) { - if (FeatureFlags.isEnabled(FeatureFlags.SEARCHABLE_SNAPSHOT)) { - if (entry.getValue() instanceof String) { - storageType(StorageType.fromString((String) entry.getValue())); - } else { - throw new IllegalArgumentException("malformed storage_type"); - } + + if (entry.getValue() instanceof String) { + storageType(StorageType.fromString((String) entry.getValue())); } else { - throw new IllegalArgumentException( - "Unsupported parameter " + name + ". Feature flag is not enabled for this experimental feature" - ); + throw new IllegalArgumentException("malformed storage_type"); } + } else { if (IndicesOptions.isIndicesOptions(name) == false) { throw new IllegalArgumentException("Unknown parameter " + name); @@ -648,7 +643,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.value(ignoreIndexSetting); } builder.endArray(); - if (FeatureFlags.isEnabled(FeatureFlags.SEARCHABLE_SNAPSHOT) && storageType != null) { + if (storageType != null) { storageType.toXContent(builder); } builder.endObject(); diff --git a/server/src/main/java/org/opensearch/action/admin/indices/settings/put/TransportUpdateSettingsAction.java b/server/src/main/java/org/opensearch/action/admin/indices/settings/put/TransportUpdateSettingsAction.java index a1c3b9341fd40..66be780d22e03 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/settings/put/TransportUpdateSettingsAction.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/settings/put/TransportUpdateSettingsAction.java @@ -49,7 +49,6 @@ import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.inject.Inject; import org.opensearch.common.io.stream.StreamInput; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.index.Index; import org.opensearch.index.IndexModule; import org.opensearch.threadpool.ThreadPool; @@ -126,38 +125,30 @@ protected ClusterBlockException checkBlock(UpdateSettingsRequest request, Cluste return null; } - if (FeatureFlags.isEnabled(FeatureFlags.SEARCHABLE_SNAPSHOT)) { - final Index[] requestIndices = indexNameExpressionResolver.concreteIndices(state, request); - boolean allowSearchableSnapshotSettingsUpdate = true; - // check if all indices in the request are remote snapshot - for (Index index : requestIndices) { - if (state.blocks().indexBlocked(ClusterBlockLevel.METADATA_WRITE, index.getName())) { - allowSearchableSnapshotSettingsUpdate = allowSearchableSnapshotSettingsUpdate - && IndexModule.Type.REMOTE_SNAPSHOT.match( - state.getMetadata().getIndexSafe(index).getSettings().get(INDEX_STORE_TYPE_SETTING.getKey()) - ); - } + final Index[] requestIndices = indexNameExpressionResolver.concreteIndices(state, request); + boolean allowSearchableSnapshotSettingsUpdate = true; + // check if all indices in the request are remote snapshot + for (Index index : requestIndices) { + if (state.blocks().indexBlocked(ClusterBlockLevel.METADATA_WRITE, index.getName())) { + allowSearchableSnapshotSettingsUpdate = allowSearchableSnapshotSettingsUpdate + && IndexModule.Type.REMOTE_SNAPSHOT.match( + state.getMetadata().getIndexSafe(index).getSettings().get(INDEX_STORE_TYPE_SETTING.getKey()) + ); } - // check if all settings in the request are in the allow list - if (allowSearchableSnapshotSettingsUpdate) { - for (String setting : request.settings().keySet()) { - allowSearchableSnapshotSettingsUpdate = allowSearchableSnapshotSettingsUpdate - && (ALLOWLIST_REMOTE_SNAPSHOT_SETTINGS.contains(setting) - || Stream.of(ALLOWLIST_REMOTE_SNAPSHOT_SETTINGS_PREFIXES).anyMatch(setting::startsWith)); - } + } + // check if all settings in the request are in the allow list + if (allowSearchableSnapshotSettingsUpdate) { + for (String setting : request.settings().keySet()) { + allowSearchableSnapshotSettingsUpdate = allowSearchableSnapshotSettingsUpdate + && (ALLOWLIST_REMOTE_SNAPSHOT_SETTINGS.contains(setting) + || Stream.of(ALLOWLIST_REMOTE_SNAPSHOT_SETTINGS_PREFIXES).anyMatch(setting::startsWith)); } - - return allowSearchableSnapshotSettingsUpdate - ? null - : state.blocks() - .indicesBlockedException( - ClusterBlockLevel.METADATA_WRITE, - indexNameExpressionResolver.concreteIndexNames(state, request) - ); } - return state.blocks() - .indicesBlockedException(ClusterBlockLevel.METADATA_WRITE, indexNameExpressionResolver.concreteIndexNames(state, request)); + return allowSearchableSnapshotSettingsUpdate + ? null + : state.blocks() + .indicesBlockedException(ClusterBlockLevel.METADATA_WRITE, indexNameExpressionResolver.concreteIndexNames(state, request)); } @Override diff --git a/server/src/main/java/org/opensearch/cluster/ClusterModule.java b/server/src/main/java/org/opensearch/cluster/ClusterModule.java index 28961d24c111a..8a4e17e5c0dc3 100644 --- a/server/src/main/java/org/opensearch/cluster/ClusterModule.java +++ b/server/src/main/java/org/opensearch/cluster/ClusterModule.java @@ -86,7 +86,6 @@ import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Setting.Property; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.common.util.set.Sets; import org.opensearch.core.xcontent.NamedXContentRegistry; @@ -370,9 +369,7 @@ public static Collection createAllocationDeciders( addAllocationDecider(deciders, new ShardsLimitAllocationDecider(settings, clusterSettings)); addAllocationDecider(deciders, new AwarenessAllocationDecider(settings, clusterSettings)); addAllocationDecider(deciders, new NodeLoadAwareAllocationDecider(settings, clusterSettings)); - if (FeatureFlags.isEnabled(FeatureFlags.SEARCHABLE_SNAPSHOT)) { - addAllocationDecider(deciders, new TargetPoolAllocationDecider()); - } + addAllocationDecider(deciders, new TargetPoolAllocationDecider()); clusterPlugins.stream() .flatMap(p -> p.createAllocationDeciders(settings, clusterSettings).stream()) diff --git a/server/src/main/java/org/opensearch/cluster/routing/RecoverySource.java b/server/src/main/java/org/opensearch/cluster/routing/RecoverySource.java index c96d104aafc43..dd7d00e29dd26 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/RecoverySource.java +++ b/server/src/main/java/org/opensearch/cluster/routing/RecoverySource.java @@ -38,7 +38,6 @@ import org.opensearch.common.io.stream.StreamInput; import org.opensearch.common.io.stream.StreamOutput; import org.opensearch.common.io.stream.Writeable; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.ToXContentObject; import org.opensearch.core.xcontent.XContentBuilder; @@ -287,7 +286,7 @@ public SnapshotRecoverySource( } else { index = new IndexId(in.readString(), IndexMetadata.INDEX_UUID_NA_VALUE); } - if (FeatureFlags.isEnabled(FeatureFlags.SEARCHABLE_SNAPSHOT) && in.getVersion().onOrAfter(Version.V_2_4_0)) { + if (in.getVersion().onOrAfter(Version.V_2_7_0)) { isSearchableSnapshot = in.readBoolean(); } else { isSearchableSnapshot = false; @@ -330,7 +329,7 @@ protected void writeAdditionalFields(StreamOutput out) throws IOException { } else { out.writeString(index.getName()); } - if (FeatureFlags.isEnabled(FeatureFlags.SEARCHABLE_SNAPSHOT) && out.getVersion().onOrAfter(Version.V_2_4_0)) { + if (out.getVersion().onOrAfter(Version.V_2_7_0)) { out.writeBoolean(isSearchableSnapshot); } } diff --git a/server/src/main/java/org/opensearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java b/server/src/main/java/org/opensearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java index 6ba8e5d893bc0..59d7fab59c266 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java +++ b/server/src/main/java/org/opensearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java @@ -52,7 +52,6 @@ import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Setting.Property; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import java.util.HashMap; import java.util.HashSet; @@ -193,12 +192,11 @@ public void allocate(RoutingAllocation allocation) { localShardsBalancer.moveShards(); localShardsBalancer.balance(); - if (FeatureFlags.isEnabled(FeatureFlags.SEARCHABLE_SNAPSHOT)) { - final ShardsBalancer remoteShardsBalancer = new RemoteShardsBalancer(logger, allocation); - remoteShardsBalancer.allocateUnassigned(); - remoteShardsBalancer.moveShards(); - remoteShardsBalancer.balance(); - } + final ShardsBalancer remoteShardsBalancer = new RemoteShardsBalancer(logger, allocation); + remoteShardsBalancer.allocateUnassigned(); + remoteShardsBalancer.moveShards(); + remoteShardsBalancer.balance(); + } @Override diff --git a/server/src/main/java/org/opensearch/cluster/routing/allocation/allocator/LocalShardsBalancer.java b/server/src/main/java/org/opensearch/cluster/routing/allocation/allocator/LocalShardsBalancer.java index 0830f26be87f0..225c6d1ce3c55 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/allocation/allocator/LocalShardsBalancer.java +++ b/server/src/main/java/org/opensearch/cluster/routing/allocation/allocator/LocalShardsBalancer.java @@ -28,7 +28,6 @@ import org.opensearch.cluster.routing.allocation.decider.Decision; import org.opensearch.cluster.routing.allocation.decider.DiskThresholdDecider; import org.opensearch.common.collect.Tuple; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.gateway.PriorityComparator; import java.util.ArrayList; @@ -126,11 +125,8 @@ public float avgPrimaryShardsPerNode() { */ @Override public float avgShardsPerNode() { - if (FeatureFlags.isEnabled(FeatureFlags.SEARCHABLE_SNAPSHOT)) { - float totalShards = nodes.values().stream().map(BalancedShardsAllocator.ModelNode::numShards).reduce(0, Integer::sum); - return totalShards / nodes.size(); - } - return avgShardsPerNode; + float totalShards = nodes.values().stream().map(BalancedShardsAllocator.ModelNode::numShards).reduce(0, Integer::sum); + return totalShards / nodes.size(); } /** @@ -200,8 +196,7 @@ void balance() { */ @Override MoveDecision decideRebalance(final ShardRouting shard) { - if (FeatureFlags.isEnabled(FeatureFlags.SEARCHABLE_SNAPSHOT) - && RoutingPool.REMOTE_CAPABLE.equals(RoutingPool.getShardPool(shard, allocation))) { + if (RoutingPool.REMOTE_CAPABLE.equals(RoutingPool.getShardPool(shard, allocation))) { return MoveDecision.NOT_TAKEN; } @@ -474,18 +469,14 @@ private void balanceByWeights() { * to the nodes we relocated them from. */ private String[] buildWeightOrderedIndices() { - final String[] indices; - if (FeatureFlags.isEnabled(FeatureFlags.SEARCHABLE_SNAPSHOT)) { - final List localIndices = new ArrayList<>(); - for (String index : allocation.routingTable().indicesRouting().keys().toArray(String.class)) { - if (RoutingPool.LOCAL_ONLY.equals(RoutingPool.getIndexPool(metadata.index(index)))) { - localIndices.add(index); - } + + final List localIndices = new ArrayList<>(); + for (String index : allocation.routingTable().indicesRouting().keys().toArray(String.class)) { + if (RoutingPool.LOCAL_ONLY.equals(RoutingPool.getIndexPool(metadata.index(index)))) { + localIndices.add(index); } - indices = localIndices.toArray(new String[0]); - } else { - indices = allocation.routingTable().indicesRouting().keys().toArray(String.class); } + final String[] indices = localIndices.toArray(new String[0]); final float[] deltas = new float[indices.length]; for (int i = 0; i < deltas.length; i++) { @@ -578,8 +569,7 @@ void moveShards() { ShardRouting shardRouting = it.next(); - if (FeatureFlags.isEnabled(FeatureFlags.SEARCHABLE_SNAPSHOT) - && RoutingPool.REMOTE_CAPABLE.equals(RoutingPool.getShardPool(shardRouting, allocation))) { + if (RoutingPool.REMOTE_CAPABLE.equals(RoutingPool.getShardPool(shardRouting, allocation))) { continue; } @@ -643,8 +633,7 @@ void moveShards() { */ @Override MoveDecision decideMove(final ShardRouting shardRouting) { - if (FeatureFlags.isEnabled(FeatureFlags.SEARCHABLE_SNAPSHOT) - && RoutingPool.REMOTE_CAPABLE.equals(RoutingPool.getShardPool(shardRouting, allocation))) { + if (RoutingPool.REMOTE_CAPABLE.equals(RoutingPool.getShardPool(shardRouting, allocation))) { return MoveDecision.NOT_TAKEN; } @@ -791,15 +780,13 @@ void allocateUnassigned() { * the next replica. If we could not find a node to allocate (0,R,IDX1) we move all it's replicas to ignoreUnassigned. */ ShardRouting[] unassignedShards = unassigned.drain(); - if (FeatureFlags.isEnabled(FeatureFlags.SEARCHABLE_SNAPSHOT)) { - List allUnassignedShards = Arrays.stream(unassignedShards).collect(Collectors.toList()); - List localUnassignedShards = allUnassignedShards.stream() - .filter(shard -> RoutingPool.LOCAL_ONLY.equals(RoutingPool.getShardPool(shard, allocation))) - .collect(Collectors.toList()); - allUnassignedShards.removeAll(localUnassignedShards); - allUnassignedShards.forEach(shard -> routingNodes.unassigned().add(shard)); - unassignedShards = localUnassignedShards.toArray(new ShardRouting[localUnassignedShards.size()]); - } + List allUnassignedShards = Arrays.stream(unassignedShards).collect(Collectors.toList()); + List localUnassignedShards = allUnassignedShards.stream() + .filter(shard -> RoutingPool.LOCAL_ONLY.equals(RoutingPool.getShardPool(shard, allocation))) + .collect(Collectors.toList()); + allUnassignedShards.removeAll(localUnassignedShards); + allUnassignedShards.forEach(shard -> routingNodes.unassigned().add(shard)); + unassignedShards = localUnassignedShards.toArray(new ShardRouting[0]); ShardRouting[] primary = unassignedShards; ShardRouting[] secondary = new ShardRouting[primary.length]; int secondaryLength = 0; diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java index f58cd0f52f858..dd4fd00ce9781 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -634,7 +634,10 @@ public void apply(Settings value, Settings current, Settings previous) { SegmentReplicationPressureService.SEGMENT_REPLICATION_INDEXING_PRESSURE_ENABLED, SegmentReplicationPressureService.MAX_INDEXING_CHECKPOINTS, SegmentReplicationPressureService.MAX_REPLICATION_TIME_SETTING, - SegmentReplicationPressureService.MAX_ALLOWED_STALE_SHARDS + SegmentReplicationPressureService.MAX_ALLOWED_STALE_SHARDS, + + // Settings related to Searchable Snapshots + Node.NODE_SEARCH_CACHE_SIZE_SETTING ) ) ); @@ -647,8 +650,6 @@ public void apply(Settings value, Settings current, Settings previous) { * setting should be moved to {@link #BUILT_IN_CLUSTER_SETTINGS}. */ public static final Map> FEATURE_FLAGGED_CLUSTER_SETTINGS = Map.of( - FeatureFlags.SEARCHABLE_SNAPSHOT, - List.of(Node.NODE_SEARCH_CACHE_SIZE_SETTING), FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL, List.of(IndicesService.CLUSTER_REPLICATION_TYPE_SETTING) ); diff --git a/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java b/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java index 1b5070d90e58c..1a778da814d72 100644 --- a/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java @@ -37,7 +37,6 @@ protected FeatureFlagSettings( FeatureFlags.REPLICATION_TYPE_SETTING, FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL_SETTING, FeatureFlags.REMOTE_STORE_SETTING, - FeatureFlags.SEARCHABLE_SNAPSHOT_SETTING, FeatureFlags.EXTENSIONS_SETTING, FeatureFlags.SEARCH_PIPELINE_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 a81c330177129..48bd03abf41fb 100644 --- a/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java @@ -197,6 +197,12 @@ public final class IndexScopedSettings extends AbstractScopedSettings { IndexSettings.INDEX_MERGE_ON_FLUSH_MAX_FULL_FLUSH_MERGE_WAIT_TIME, IndexSettings.INDEX_MERGE_ON_FLUSH_POLICY, + // Settings for Searchable Snapshots + IndexSettings.SEARCHABLE_SNAPSHOT_REPOSITORY, + IndexSettings.SEARCHABLE_SNAPSHOT_INDEX_ID, + IndexSettings.SEARCHABLE_SNAPSHOT_ID_NAME, + IndexSettings.SEARCHABLE_SNAPSHOT_ID_UUID, + // validate that built-in similarities don't get redefined Setting.groupSetting("index.similarity.", (s) -> { Map groups = s.getAsGroups(); @@ -229,13 +235,6 @@ public final class IndexScopedSettings extends AbstractScopedSettings { IndexMetadata.INDEX_REMOTE_TRANSLOG_STORE_ENABLED_SETTING, IndexMetadata.INDEX_REMOTE_TRANSLOG_REPOSITORY_SETTING, IndexMetadata.INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING - ), - FeatureFlags.SEARCHABLE_SNAPSHOT, - List.of( - IndexSettings.SEARCHABLE_SNAPSHOT_REPOSITORY, - IndexSettings.SEARCHABLE_SNAPSHOT_INDEX_ID, - IndexSettings.SEARCHABLE_SNAPSHOT_ID_NAME, - IndexSettings.SEARCHABLE_SNAPSHOT_ID_UUID ) ); diff --git a/server/src/main/java/org/opensearch/common/util/FeatureFlags.java b/server/src/main/java/org/opensearch/common/util/FeatureFlags.java index 203b62f5595cd..2c4f111bdcd24 100644 --- a/server/src/main/java/org/opensearch/common/util/FeatureFlags.java +++ b/server/src/main/java/org/opensearch/common/util/FeatureFlags.java @@ -39,14 +39,6 @@ public class FeatureFlags { */ public static final String REMOTE_STORE = "opensearch.experimental.feature.remote_store.enabled"; - /** - * 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 - * ahead of time. - */ - public static final String SEARCHABLE_SNAPSHOT = "opensearch.experimental.feature.searchable_snapshot.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. @@ -104,8 +96,6 @@ public static boolean isEnabled(String featureFlagName) { public static final Setting REMOTE_STORE_SETTING = Setting.boolSetting(REMOTE_STORE, false, Property.NodeScope); - public static final Setting SEARCHABLE_SNAPSHOT_SETTING = Setting.boolSetting(SEARCHABLE_SNAPSHOT, false, Property.NodeScope); - public static final Setting EXTENSIONS_SETTING = Setting.boolSetting(EXTENSIONS, false, Property.NodeScope); public static final Setting SEARCH_PIPELINE_SETTING = Setting.boolSetting(SEARCH_PIPELINE, false, Property.NodeScope); diff --git a/server/src/main/java/org/opensearch/monitor/fs/FsInfo.java b/server/src/main/java/org/opensearch/monitor/fs/FsInfo.java index d57f722a4fc19..8e9be95279bd6 100644 --- a/server/src/main/java/org/opensearch/monitor/fs/FsInfo.java +++ b/server/src/main/java/org/opensearch/monitor/fs/FsInfo.java @@ -40,7 +40,6 @@ import org.opensearch.common.io.stream.StreamOutput; import org.opensearch.common.io.stream.Writeable; import org.opensearch.common.unit.ByteSizeValue; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.xcontent.ToXContentFragment; import org.opensearch.core.xcontent.ToXContentObject; import org.opensearch.core.xcontent.XContentBuilder; @@ -97,7 +96,7 @@ public Path(StreamInput in) throws IOException { total = in.readLong(); free = in.readLong(); available = in.readLong(); - if (FeatureFlags.isEnabled(FeatureFlags.SEARCHABLE_SNAPSHOT) && in.getVersion().onOrAfter(Version.V_2_7_0)) { + if (in.getVersion().onOrAfter(Version.V_2_7_0)) { fileCacheReserved = in.readLong(); fileCacheUtilized = in.readLong(); } @@ -111,7 +110,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeLong(total); out.writeLong(free); out.writeLong(available); - if (FeatureFlags.isEnabled(FeatureFlags.SEARCHABLE_SNAPSHOT) && out.getVersion().onOrAfter(Version.V_2_7_0)) { + if (out.getVersion().onOrAfter(Version.V_2_7_0)) { out.writeLong(fileCacheReserved); out.writeLong(fileCacheUtilized); } @@ -208,10 +207,10 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws if (available != -1) { builder.humanReadableField(Fields.AVAILABLE_IN_BYTES, Fields.AVAILABLE, getAvailable()); } - if (FeatureFlags.isEnabled(FeatureFlags.SEARCHABLE_SNAPSHOT) && fileCacheReserved != -1) { + if (fileCacheReserved != -1) { builder.humanReadableField(Fields.CACHE_RESERVED_IN_BYTES, Fields.CACHE_RESERVED, getFileCacheReserved()); } - if (FeatureFlags.isEnabled(FeatureFlags.SEARCHABLE_SNAPSHOT) && fileCacheReserved != 0) { + if (fileCacheReserved != 0) { builder.humanReadableField(Fields.CACHE_UTILIZED, Fields.CACHE_UTILIZED_IN_BYTES, getFileCacheUtilized()); } diff --git a/server/src/main/java/org/opensearch/snapshots/RestoreService.java b/server/src/main/java/org/opensearch/snapshots/RestoreService.java index 7d9b15f2ef296..b2eaa92abab4d 100644 --- a/server/src/main/java/org/opensearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/opensearch/snapshots/RestoreService.java @@ -455,8 +455,9 @@ public ClusterState execute(ClusterState currentState) { request.indexSettings(), request.ignoreIndexSettings() ); - final boolean isSearchableSnapshot = FeatureFlags.isEnabled(FeatureFlags.SEARCHABLE_SNAPSHOT) - && IndexModule.Type.REMOTE_SNAPSHOT.match(request.storageType().toString()); + final boolean isSearchableSnapshot = IndexModule.Type.REMOTE_SNAPSHOT.match( + request.storageType().toString() + ); if (isSearchableSnapshot) { snapshotIndexMetadata = addSnapshotToIndexSettings( snapshotIndexMetadata, diff --git a/server/src/test/java/org/opensearch/cluster/ClusterModuleTests.java b/server/src/test/java/org/opensearch/cluster/ClusterModuleTests.java index 004b784311b54..535444cd866b8 100644 --- a/server/src/test/java/org/opensearch/cluster/ClusterModuleTests.java +++ b/server/src/test/java/org/opensearch/cluster/ClusterModuleTests.java @@ -57,6 +57,7 @@ import org.opensearch.cluster.routing.allocation.decider.SameShardAllocationDecider; import org.opensearch.cluster.routing.allocation.decider.ShardsLimitAllocationDecider; import org.opensearch.cluster.routing.allocation.decider.SnapshotInProgressAllocationDecider; +import org.opensearch.cluster.routing.allocation.decider.TargetPoolAllocationDecider; import org.opensearch.cluster.routing.allocation.decider.ThrottlingAllocationDecider; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.inject.ModuleTestCase; @@ -238,7 +239,8 @@ public void testAllocationDeciderOrder() { ThrottlingAllocationDecider.class, ShardsLimitAllocationDecider.class, AwarenessAllocationDecider.class, - NodeLoadAwareAllocationDecider.class + NodeLoadAwareAllocationDecider.class, + TargetPoolAllocationDecider.class ); Collection deciders = ClusterModule.createAllocationDeciders( Settings.EMPTY, diff --git a/server/src/test/java/org/opensearch/cluster/routing/allocation/RemoteShardsBalancerBaseTestCase.java b/server/src/test/java/org/opensearch/cluster/routing/allocation/RemoteShardsBalancerBaseTestCase.java index ae9799545e6af..67278c56b2f78 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/allocation/RemoteShardsBalancerBaseTestCase.java +++ b/server/src/test/java/org/opensearch/cluster/routing/allocation/RemoteShardsBalancerBaseTestCase.java @@ -8,8 +8,6 @@ package org.opensearch.cluster.routing.allocation; -import org.junit.AfterClass; -import org.junit.BeforeClass; import org.opensearch.Version; import org.opensearch.cluster.ClusterInfo; import org.opensearch.cluster.ClusterModule; @@ -34,7 +32,6 @@ import org.opensearch.common.collect.ImmutableOpenMap; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.index.IndexModule; import org.opensearch.test.gateway.TestGatewayAllocator; @@ -68,16 +65,6 @@ public abstract class RemoteShardsBalancerBaseTestCase extends OpenSearchAllocat protected ClusterSettings EMPTY_CLUSTER_SETTINGS = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - @BeforeClass - public static void setup() { - System.setProperty(FeatureFlags.SEARCHABLE_SNAPSHOT, "true"); - } - - @AfterClass - public static void teardown() { - System.setProperty(FeatureFlags.SEARCHABLE_SNAPSHOT, "false"); - } - public String getNodeId(int id, boolean isRemote, String prefix) { if (isRemote) { return REMOTE_NODE_PREFIX + "-" + prefix + "-" + id; diff --git a/server/src/test/java/org/opensearch/node/NodeTests.java b/server/src/test/java/org/opensearch/node/NodeTests.java index ebafaca1a8f98..ae8028c143498 100644 --- a/server/src/test/java/org/opensearch/node/NodeTests.java +++ b/server/src/test/java/org/opensearch/node/NodeTests.java @@ -44,7 +44,6 @@ import org.opensearch.common.transport.BoundTransportAddress; import org.opensearch.common.unit.ByteSizeUnit; import org.opensearch.common.unit.ByteSizeValue; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.env.Environment; import org.opensearch.env.NodeEnvironment; import org.opensearch.index.IndexService; @@ -363,7 +362,6 @@ public void testCreateWithFileCache() throws Exception { ByteSizeValue cacheSize = new ByteSizeValue(16, ByteSizeUnit.GB); Settings searchRoleSettingsWithConfig = baseSettings().put(searchRoleSettings) .put(Node.NODE_SEARCH_CACHE_SIZE_SETTING.getKey(), cacheSize) - .put(FeatureFlags.SEARCHABLE_SNAPSHOT, "true") .build(); Settings onlySearchRoleSettings = Settings.builder() .put(searchRoleSettingsWithConfig)