From a9471f749ec811f4519f3a875e618cc9a402960f Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Tue, 18 Jul 2023 12:12:43 -0400 Subject: [PATCH] Remove SEARCH_PIPELINE feature flag (#8738) (#8749) (cherry picked from commit 1164221ee2b8ba3560f0ff492309867beea28433) Signed-off-by: Louis Chu Signed-off-by: github-actions[bot] Co-authored-by: github-actions[bot] --- client/rest-high-level/build.gradle | 2 -- distribution/src/config/opensearch.yml | 6 ------ modules/search-pipeline-common/build.gradle | 3 --- .../common/SearchPipelineCommonIT.java | 7 ------- qa/smoke-test-multinode/build.gradle | 1 - rest-api-spec/build.gradle | 1 - .../org/opensearch/action/ActionModule.java | 8 +++----- .../common/settings/FeatureFlagSettings.java | 1 - .../opensearch/common/util/FeatureFlags.java | 8 -------- .../org/opensearch/index/IndexSettings.java | 14 +------------ .../main/java/org/opensearch/node/Node.java | 4 +--- .../search/builder/SearchSourceBuilder.java | 20 +++++++++---------- .../pipeline/SearchPipelineService.java | 15 ++------------ .../opensearch/index/IndexSettingsTests.java | 19 ------------------ .../pipeline/SearchPipelineServiceTests.java | 12 ++++------- .../snapshots/SnapshotResiliencyTests.java | 3 +-- .../search/RandomSearchRequestGenerator.java | 3 +-- 17 files changed, 22 insertions(+), 105 deletions(-) diff --git a/client/rest-high-level/build.gradle b/client/rest-high-level/build.gradle index 4f4d4042269b0..cc8262ff6d9ad 100644 --- a/client/rest-high-level/build.gradle +++ b/client/rest-high-level/build.gradle @@ -106,6 +106,4 @@ testClusters.all { extraConfigFile nodeTrustStore.name, nodeTrustStore extraConfigFile pkiTrustCert.name, pkiTrustCert - // Enable APIs behind feature flags - setting 'opensearch.experimental.feature.search_pipeline.enabled', 'true' } diff --git a/distribution/src/config/opensearch.yml b/distribution/src/config/opensearch.yml index 8c4160db98857..99e5fb6f9a226 100644 --- a/distribution/src/config/opensearch.yml +++ b/distribution/src/config/opensearch.yml @@ -128,12 +128,6 @@ ${path.logs} #opensearch.experimental.feature.extensions.enabled: false # # -# Gates the search pipeline feature. This feature enables configurable processors -# for search requests and search responses, similar to ingest pipelines. -# -#opensearch.experimental.feature.search_pipeline.enabled: false -# -# # Gates the concurrent segment search feature. This feature enables concurrent segment search in a separate # index searcher threadpool. # diff --git a/modules/search-pipeline-common/build.gradle b/modules/search-pipeline-common/build.gradle index fe3c097ff6886..657392d884e97 100644 --- a/modules/search-pipeline-common/build.gradle +++ b/modules/search-pipeline-common/build.gradle @@ -28,9 +28,6 @@ restResources { } } -testClusters.all { - setting 'opensearch.experimental.feature.search_pipeline.enabled', 'true' -} thirdPartyAudit.ignoreMissingClasses( // from log4j diff --git a/modules/search-pipeline-common/src/internalClusterTest/java/org/opensearch/search/pipeline/common/SearchPipelineCommonIT.java b/modules/search-pipeline-common/src/internalClusterTest/java/org/opensearch/search/pipeline/common/SearchPipelineCommonIT.java index 2c7ffb4648a7d..730a49b8351c3 100644 --- a/modules/search-pipeline-common/src/internalClusterTest/java/org/opensearch/search/pipeline/common/SearchPipelineCommonIT.java +++ b/modules/search-pipeline-common/src/internalClusterTest/java/org/opensearch/search/pipeline/common/SearchPipelineCommonIT.java @@ -19,8 +19,6 @@ import org.opensearch.action.search.SearchResponse; import org.opensearch.action.support.master.AcknowledgedResponse; import org.opensearch.common.bytes.BytesArray; -import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.common.xcontent.XContentType; import org.opensearch.index.query.MatchAllQueryBuilder; import org.opensearch.plugins.Plugin; @@ -35,11 +33,6 @@ @OpenSearchIntegTestCase.SuiteScopeTestCase public class SearchPipelineCommonIT extends OpenSearchIntegTestCase { - @Override - protected Settings featureFlagSettings() { - return Settings.builder().put(FeatureFlags.SEARCH_PIPELINE, "true").build(); - } - @Override protected Collection> nodePlugins() { return List.of(SearchPipelineCommonModulePlugin.class); diff --git a/qa/smoke-test-multinode/build.gradle b/qa/smoke-test-multinode/build.gradle index 8f13e4bf8736e..25261f5e3ff7d 100644 --- a/qa/smoke-test-multinode/build.gradle +++ b/qa/smoke-test-multinode/build.gradle @@ -43,7 +43,6 @@ File repo = file("$buildDir/testclusters/repo") testClusters.integTest { numberOfNodes = 2 setting 'path.repo', repo.absolutePath - setting 'opensearch.experimental.feature.search_pipeline.enabled', 'true' } integTest { diff --git a/rest-api-spec/build.gradle b/rest-api-spec/build.gradle index dcc62d11568ae..535749a5e0c14 100644 --- a/rest-api-spec/build.gradle +++ b/rest-api-spec/build.gradle @@ -28,7 +28,6 @@ artifacts { testClusters.all { module ':modules:mapper-extras' - setting 'opensearch.experimental.feature.search_pipeline.enabled', 'true' } test.enabled = false diff --git a/server/src/main/java/org/opensearch/action/ActionModule.java b/server/src/main/java/org/opensearch/action/ActionModule.java index 0f036bb80c4c2..32980a5bdb273 100644 --- a/server/src/main/java/org/opensearch/action/ActionModule.java +++ b/server/src/main/java/org/opensearch/action/ActionModule.java @@ -946,11 +946,9 @@ public void initRestHandlers(Supplier nodesInCluster) { registerHandler.accept(new RestDeleteDecommissionStateAction()); // Search pipelines API - if (FeatureFlags.isEnabled(FeatureFlags.SEARCH_PIPELINE)) { - registerHandler.accept(new RestPutSearchPipelineAction()); - registerHandler.accept(new RestGetSearchPipelineAction()); - registerHandler.accept(new RestDeleteSearchPipelineAction()); - } + registerHandler.accept(new RestPutSearchPipelineAction()); + registerHandler.accept(new RestGetSearchPipelineAction()); + registerHandler.accept(new RestDeleteSearchPipelineAction()); // Extensions API if (FeatureFlags.isEnabled(FeatureFlags.EXTENSIONS)) { 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 dae66c79c63ec..ed01347b115ea 100644 --- a/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java @@ -38,7 +38,6 @@ protected FeatureFlagSettings( FeatureFlags.REMOTE_STORE_SETTING, FeatureFlags.EXTENSIONS_SETTING, FeatureFlags.IDENTITY_SETTING, - FeatureFlags.SEARCH_PIPELINE_SETTING, FeatureFlags.CONCURRENT_SEGMENT_SEARCH_SETTING, FeatureFlags.TELEMETRY_SETTING ) 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 915f231a1ea49..e2663b56c5cca 100644 --- a/server/src/main/java/org/opensearch/common/util/FeatureFlags.java +++ b/server/src/main/java/org/opensearch/common/util/FeatureFlags.java @@ -46,12 +46,6 @@ public class FeatureFlags { */ public static final String EXTENSIONS = "opensearch.experimental.feature.extensions.enabled"; - /** - * Gates the search pipeline features during initial development. - * Once the feature is complete and ready for release, this feature flag can be removed. - */ - public static final String SEARCH_PIPELINE = "opensearch.experimental.feature.search_pipeline.enabled"; - /** * Gates the functionality of identity. */ @@ -106,8 +100,6 @@ public static boolean isEnabled(String featureFlagName) { public static final Setting EXTENSIONS_SETTING = Setting.boolSetting(EXTENSIONS, false, Property.NodeScope); - public static final Setting SEARCH_PIPELINE_SETTING = Setting.boolSetting(SEARCH_PIPELINE, true, Property.NodeScope); - public static final Setting IDENTITY_SETTING = Setting.boolSetting(IDENTITY, false, Property.NodeScope); public static final Setting TELEMETRY_SETTING = Setting.boolSetting(TELEMETRY, false, Property.NodeScope); diff --git a/server/src/main/java/org/opensearch/index/IndexSettings.java b/server/src/main/java/org/opensearch/index/IndexSettings.java index ced4b8b1d1464..905a2c2ea84ae 100644 --- a/server/src/main/java/org/opensearch/index/IndexSettings.java +++ b/server/src/main/java/org/opensearch/index/IndexSettings.java @@ -42,7 +42,6 @@ import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Setting.Property; import org.opensearch.common.settings.Settings; -import org.opensearch.common.settings.SettingsException; import org.opensearch.common.unit.ByteSizeUnit; import org.opensearch.common.unit.ByteSizeValue; import org.opensearch.common.unit.TimeValue; @@ -64,7 +63,6 @@ import java.util.function.UnaryOperator; import static org.opensearch.common.util.FeatureFlags.SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY; -import static org.opensearch.common.util.FeatureFlags.SEARCH_PIPELINE; import static org.opensearch.index.mapper.MapperService.INDEX_MAPPING_DEPTH_LIMIT_SETTING; import static org.opensearch.index.mapper.MapperService.INDEX_MAPPING_FIELD_NAME_LENGTH_LIMIT_SETTING; import static org.opensearch.index.mapper.MapperService.INDEX_MAPPING_NESTED_DOCS_LIMIT_SETTING; @@ -1624,16 +1622,6 @@ public String getDefaultSearchPipeline() { } public void setDefaultSearchPipeline(String defaultSearchPipeline) { - if (FeatureFlags.isEnabled(SEARCH_PIPELINE)) { - this.defaultSearchPipeline = defaultSearchPipeline; - } else { - throw new SettingsException( - "Unable to update setting: " - + DEFAULT_SEARCH_PIPELINE.getKey() - + ". This is an experimental feature that is currently disabled, please enable the " - + SEARCH_PIPELINE - + " feature flag first." - ); - } + this.defaultSearchPipeline = defaultSearchPipeline; } } diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index 0c9cd4deed806..de51b91979dd8 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -260,7 +260,6 @@ import java.util.stream.Stream; import static java.util.stream.Collectors.toList; -import static org.opensearch.common.util.FeatureFlags.SEARCH_PIPELINE; import static org.opensearch.common.util.FeatureFlags.TELEMETRY; import static org.opensearch.env.NodeEnvironment.collectFileCacheDataPath; import static org.opensearch.index.ShardIndexingPressureSettings.SHARD_INDEXING_PRESSURE_ENABLED_ATTRIBUTE_KEY; @@ -982,8 +981,7 @@ protected Node( xContentRegistry, namedWriteableRegistry, pluginsService.filterPlugins(SearchPipelinePlugin.class), - client, - FeatureFlags.isEnabled(SEARCH_PIPELINE) + client ); final TaskCancellationMonitoringSettings taskCancellationMonitoringSettings = new TaskCancellationMonitoringSettings( settings, diff --git a/server/src/main/java/org/opensearch/search/builder/SearchSourceBuilder.java b/server/src/main/java/org/opensearch/search/builder/SearchSourceBuilder.java index ee47078a0d34c..dfd0fa7d8fd88 100644 --- a/server/src/main/java/org/opensearch/search/builder/SearchSourceBuilder.java +++ b/server/src/main/java/org/opensearch/search/builder/SearchSourceBuilder.java @@ -37,7 +37,6 @@ import org.opensearch.Version; import org.opensearch.common.Booleans; import org.opensearch.common.Nullable; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.common.ParsingException; import org.opensearch.common.io.stream.StreamInput; import org.opensearch.common.io.stream.StreamOutput; @@ -1267,16 +1266,15 @@ public void parseXContent(XContentParser parser, boolean checkTrailingTokens) th collapse = CollapseBuilder.fromXContent(parser); } else if (POINT_IN_TIME.match(currentFieldName, parser.getDeprecationHandler())) { pointInTimeBuilder = PointInTimeBuilder.fromXContent(parser); - } else if (FeatureFlags.isEnabled(FeatureFlags.SEARCH_PIPELINE) - && SEARCH_PIPELINE.match(currentFieldName, parser.getDeprecationHandler())) { - searchPipelineSource = parser.mapOrdered(); - } else { - throw new ParsingException( - parser.getTokenLocation(), - "Unknown key for a " + token + " in [" + currentFieldName + "].", - parser.getTokenLocation() - ); - } + } else if (SEARCH_PIPELINE.match(currentFieldName, parser.getDeprecationHandler())) { + searchPipelineSource = parser.mapOrdered(); + } else { + throw new ParsingException( + parser.getTokenLocation(), + "Unknown key for a " + token + " in [" + currentFieldName + "].", + parser.getTokenLocation() + ); + } } else if (token == XContentParser.Token.START_ARRAY) { if (STORED_FIELDS_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { storedFieldsContext = StoredFieldsContext.fromXContent(STORED_FIELDS_FIELD.getPreferredName(), parser); diff --git a/server/src/main/java/org/opensearch/search/pipeline/SearchPipelineService.java b/server/src/main/java/org/opensearch/search/pipeline/SearchPipelineService.java index 83a7a0564467e..584530a3f89ae 100644 --- a/server/src/main/java/org/opensearch/search/pipeline/SearchPipelineService.java +++ b/server/src/main/java/org/opensearch/search/pipeline/SearchPipelineService.java @@ -85,8 +85,6 @@ public class SearchPipelineService implements ClusterStateApplier, ReportingServ private final OperationMetrics totalRequestProcessingMetrics = new OperationMetrics(); private final OperationMetrics totalResponseProcessingMetrics = new OperationMetrics(); - private final boolean isEnabled; - public SearchPipelineService( ClusterService clusterService, ThreadPool threadPool, @@ -96,8 +94,7 @@ public SearchPipelineService( NamedXContentRegistry namedXContentRegistry, NamedWriteableRegistry namedWriteableRegistry, List searchPipelinePlugins, - Client client, - boolean isEnabled + Client client ) { this.clusterService = clusterService; this.scriptService = scriptService; @@ -123,7 +120,6 @@ public SearchPipelineService( ); putPipelineTaskKey = clusterService.registerClusterManagerTask(ClusterManagerTaskKeys.PUT_SEARCH_PIPELINE_KEY, true); deletePipelineTaskKey = clusterService.registerClusterManagerTask(ClusterManagerTaskKeys.DELETE_SEARCH_PIPELINE_KEY, true); - this.isEnabled = isEnabled; } private static Map> processorFactories( @@ -233,10 +229,6 @@ public void putPipeline( PutSearchPipelineRequest request, ActionListener listener ) throws Exception { - if (isEnabled == false) { - throw new IllegalArgumentException("Experimental search pipeline feature is not enabled"); - } - validatePipeline(searchPipelineInfos, request); clusterService.submitStateUpdateTask( "put-search-pipeline-" + request.getId(), @@ -371,9 +363,6 @@ static ClusterState innerDelete(DeleteSearchPipelineRequest request, ClusterStat public PipelinedRequest resolvePipeline(SearchRequest searchRequest) { Pipeline pipeline = Pipeline.NO_OP_PIPELINE; - if (isEnabled == false) { - return new PipelinedRequest(pipeline, searchRequest); - } if (searchRequest.source() != null && searchRequest.source().searchPipelineSource() != null) { // Pipeline defined in search request (ad hoc pipeline). if (searchRequest.pipeline() != null) { @@ -401,7 +390,7 @@ public PipelinedRequest resolvePipeline(SearchRequest searchRequest) { if (searchRequest.pipeline() != null) { // Named pipeline specified for the request pipelineId = searchRequest.pipeline(); - } else if (searchRequest.indices() != null && searchRequest.indices().length == 1) { + } else if (state != null && searchRequest.indices() != null && searchRequest.indices().length == 1) { // Check for index default pipeline IndexMetadata indexMetadata = state.metadata().index(searchRequest.indices()[0]); if (indexMetadata != null) { diff --git a/server/src/test/java/org/opensearch/index/IndexSettingsTests.java b/server/src/test/java/org/opensearch/index/IndexSettingsTests.java index 66e86d51b10d9..6f312334ae665 100644 --- a/server/src/test/java/org/opensearch/index/IndexSettingsTests.java +++ b/server/src/test/java/org/opensearch/index/IndexSettingsTests.java @@ -40,7 +40,6 @@ import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Setting.Property; import org.opensearch.common.settings.Settings; -import org.opensearch.common.settings.SettingsException; import org.opensearch.common.unit.ByteSizeValue; import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.FeatureFlags; @@ -1110,7 +1109,6 @@ public void testExtendedCompatibilityVersionWithoutFeatureFlag() { @SuppressForbidden(reason = "sets the SEARCH_PIPELINE feature flag") public void testDefaultSearchPipeline() throws Exception { - FeatureFlagSetter.set(FeatureFlags.SEARCH_PIPELINE); IndexMetadata metadata = newIndexMeta( "index", Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT).build() @@ -1127,21 +1125,4 @@ public void testDefaultSearchPipeline() throws Exception { settings.updateIndexMetadata(metadata); assertEquals("foo", settings.getDefaultSearchPipeline()); } - - public void testDefaultSearchPipelineWithoutFeatureFlag() { - IndexMetadata metadata = newIndexMeta( - "index", - Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT).build() - ); - IndexSettings settings = new IndexSettings(metadata, Settings.EMPTY); - assertEquals(SearchPipelineService.NOOP_PIPELINE_ID, settings.getDefaultSearchPipeline()); - IndexMetadata updatedMetadata = newIndexMeta( - "index", - Settings.builder() - .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) - .put(IndexSettings.DEFAULT_SEARCH_PIPELINE.getKey(), "foo") - .build() - ); - assertThrows(SettingsException.class, () -> settings.updateIndexMetadata(updatedMetadata)); - } } diff --git a/server/src/test/java/org/opensearch/search/pipeline/SearchPipelineServiceTests.java b/server/src/test/java/org/opensearch/search/pipeline/SearchPipelineServiceTests.java index 0103d4c677b00..a90eb3595e156 100644 --- a/server/src/test/java/org/opensearch/search/pipeline/SearchPipelineServiceTests.java +++ b/server/src/test/java/org/opensearch/search/pipeline/SearchPipelineServiceTests.java @@ -112,8 +112,7 @@ public void testSearchPipelinePlugin() { this.xContentRegistry(), this.writableRegistry(), List.of(DUMMY_PLUGIN), - client, - false + client ); Map> requestProcessorFactories = searchPipelineService .getRequestProcessorFactories(); @@ -138,8 +137,7 @@ public void testSearchPipelinePluginDuplicate() { this.xContentRegistry(), this.writableRegistry(), List.of(DUMMY_PLUGIN, DUMMY_PLUGIN), - client, - false + client ) ); assertTrue(e.getMessage(), e.getMessage().contains(" already registered")); @@ -156,8 +154,7 @@ public void testResolveSearchPipelineDoesNotExist() { this.xContentRegistry(), this.writableRegistry(), List.of(DUMMY_PLUGIN), - client, - true + client ); final SearchRequest searchRequest = new SearchRequest("_index").pipeline("bar"); IllegalArgumentException e = expectThrows( @@ -386,8 +383,7 @@ public Map> getSearchPhas } }), - client, - true + client ); } diff --git a/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java b/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java index 88899a1b282af..2ce1b05c2507d 100644 --- a/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java +++ b/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java @@ -2063,8 +2063,7 @@ public void onFailure(final Exception e) { namedXContentRegistry, namedWriteableRegistry, List.of(), - client, - false + client ) ) ); diff --git a/test/framework/src/main/java/org/opensearch/search/RandomSearchRequestGenerator.java b/test/framework/src/main/java/org/opensearch/search/RandomSearchRequestGenerator.java index 9dec1f2ba739c..f39277b5cf3a0 100644 --- a/test/framework/src/main/java/org/opensearch/search/RandomSearchRequestGenerator.java +++ b/test/framework/src/main/java/org/opensearch/search/RandomSearchRequestGenerator.java @@ -38,7 +38,6 @@ import org.opensearch.common.bytes.BytesReference; import org.opensearch.common.text.Text; import org.opensearch.common.unit.TimeValue; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.xcontent.DeprecationHandler; import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.core.xcontent.XContentBuilder; @@ -406,7 +405,7 @@ public static SearchSourceBuilder randomSearchSourceBuilder( } builder.pointInTimeBuilder(pit); } - if (FeatureFlags.isEnabled(FeatureFlags.SEARCH_PIPELINE) && randomBoolean()) { + if (randomBoolean()) { builder.searchPipelineSource(new HashMap<>()); } return builder;