Remove SEARCH_PIPELINE feature flag (opensearch-project#8513)

* Remove SEARCH_PIPELINE feature flag Signed-off-by: Louis Chu <[email protected]> * Fix gradle check Signed-off-by: Louis Chu <[email protected]> * Bugfix on search service implicit execution Signed-off-by: Louis Chu <[email protected]> * Set the flag for bwc test Signed-off-by: Louis Chu <[email protected]> * Remove feature flag setting on qa Signed-off-by: Louis Chu <[email protected]> --------- Signed-off-by: Louis Chu <[email protected]>
dreamer-89 · Jul 18, 2023 · 57d5e90 · 57d5e90
1 parent 2f830be
commit 57d5e90
Show file tree

Hide file tree

Showing 18 changed files with 22 additions and 105 deletions.
diff --git a/client/rest-high-level/build.gradle b/client/rest-high-level/build.gradle
@@ -106,8 +106,6 @@ testClusters.all {
   extraConfigFile nodeTrustStore.name, nodeTrustStore
   extraConfigFile pkiTrustCert.name, pkiTrustCert
 
-  // Enable APIs behind feature flags
-  setting 'opensearch.experimental.feature.search_pipeline.enabled', 'true'
 }
 
 thirdPartyAudit.ignoreMissingClasses(

diff --git 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
@@ -28,9 +28,6 @@ restResources {
   }
 }
 
-testClusters.all {
-  setting 'opensearch.experimental.feature.search_pipeline.enabled', 'true'
-}
 
 thirdPartyAudit.ignoreMissingClasses(
   // from log4j

diff --git a/...nternalClusterTest/java/org/opensearch/search/pipeline/common/SearchPipelineCommonIT.java b/...nternalClusterTest/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.core.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<Class<? extends Plugin>> nodePlugins() {
         return List.of(SearchPipelineCommonModulePlugin.class);

diff --git a/qa/mixed-cluster/build.gradle b/qa/mixed-cluster/build.gradle
@@ -65,7 +65,6 @@ for (Version bwcVersion : BuildParams.bwcVersions.wireCompatible) {
       numberOfNodes = 4
 
       setting 'path.repo', "${buildDir}/cluster/shared/repo/${baseName}"
-      setting 'opensearch.experimental.feature.search_pipeline.enabled', 'true'
     }
   }
 

diff --git 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
@@ -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
@@ -946,11 +946,9 @@ public void initRestHandlers(Supplier<DiscoveryNodes> 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
@@ -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
@@ -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<Boolean> EXTENSIONS_SETTING = Setting.boolSetting(EXTENSIONS, false, Property.NodeScope);
 
-    public static final Setting<Boolean> SEARCH_PIPELINE_SETTING = Setting.boolSetting(SEARCH_PIPELINE, true, Property.NodeScope);
-
     public static final Setting<Boolean> IDENTITY_SETTING = Setting.boolSetting(IDENTITY, false, Property.NodeScope);
 
     public static final Setting<Boolean> 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
@@ -41,7 +41,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;
@@ -1625,16 +1623,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
@@ -258,7 +258,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;
@@ -980,8 +979,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
@@ -36,7 +36,6 @@
 import org.opensearch.Version;
 import org.opensearch.common.Booleans;
 import org.opensearch.common.Nullable;
-import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.core.common.ParsingException;
 import org.opensearch.core.common.io.stream.StreamInput;
 import org.opensearch.core.common.io.stream.StreamOutput;
@@ -1250,16 +1249,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
@@ -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<SearchPipelinePlugin> 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 <T extends Processor> Map<String, Processor.Factory<T>> processorFactories(
@@ -233,10 +229,6 @@ public void putPipeline(
         PutSearchPipelineRequest request,
         ActionListener<AcknowledgedResponse> 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
@@ -1110,7 +1110,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 +1126,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
@@ -112,8 +112,7 @@ public void testSearchPipelinePlugin() {
             this.xContentRegistry(),
             this.writableRegistry(),
             List.of(DUMMY_PLUGIN),
-            client,
-            false
+            client
         );
         Map<String, Processor.Factory<SearchRequestProcessor>> 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<String, Processor.Factory<SearchPhaseResultsProcessor>> 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
@@ -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
@@ -38,7 +38,6 @@
 import org.opensearch.core.common.bytes.BytesReference;
 import org.opensearch.core.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;