Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove SEARCH_PIPELINE feature flag (#8513) #8738

Merged
merged 1 commit into from
Jul 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions client/rest-high-level/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,4 @@ testClusters.all {
extraConfigFile nodeTrustStore.name, nodeTrustStore
extraConfigFile pkiTrustCert.name, pkiTrustCert

// Enable APIs behind feature flags
setting 'opensearch.experimental.feature.search_pipeline.enabled', 'true'
}
6 changes: 0 additions & 6 deletions distribution/src/config/opensearch.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
#
Expand Down
3 changes: 0 additions & 3 deletions modules/search-pipeline-common/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@ restResources {
}
}

testClusters.all {
setting 'opensearch.experimental.feature.search_pipeline.enabled', 'true'
}

thirdPartyAudit.ignoreMissingClasses(
// from log4j
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down
1 change: 0 additions & 1 deletion qa/smoke-test-multinode/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 0 additions & 1 deletion rest-api-spec/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ artifacts {

testClusters.all {
module ':modules:mapper-extras'
setting 'opensearch.experimental.feature.search_pipeline.enabled', 'true'
}

test.enabled = false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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);
Expand Down
14 changes: 1 addition & 13 deletions server/src/main/java/org/opensearch/index/IndexSettings.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -1621,16 +1619,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;
}
}
4 changes: 1 addition & 3 deletions server/src/main/java/org/opensearch/node/Node.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -982,8 +981,7 @@ protected Node(
xContentRegistry,
namedWriteableRegistry,
pluginsService.filterPlugins(SearchPipelinePlugin.class),
client,
FeatureFlags.isEnabled(SEARCH_PIPELINE)
client
);
final TaskCancellationMonitoringSettings taskCancellationMonitoringSettings = new TaskCancellationMonitoringSettings(
settings,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
Expand All @@ -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(
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
Expand All @@ -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));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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"));
Expand All @@ -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(
Expand Down Expand Up @@ -386,8 +383,7 @@ public Map<String, Processor.Factory<SearchPhaseResultsProcessor>> getSearchPhas
}

}),
client,
true
client
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2063,8 +2063,7 @@ public void onFailure(final Exception e) {
namedXContentRegistry,
namedWriteableRegistry,
List.of(),
client,
false
client
)
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
Loading