Skip to content

Commit

Permalink
Use a settings instead of a feature flag for LTR.
Browse files Browse the repository at this point in the history
  • Loading branch information
afoucret committed Dec 12, 2023
1 parent 8ee4721 commit a135a54
Show file tree
Hide file tree
Showing 8 changed files with 41 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
*/
public enum FeatureFlag {
TIME_SERIES_MODE("es.index_mode_feature_flag_registered=true", Version.fromString("8.0.0"), null),
LEARNING_TO_RANK("es.learning_to_rank_feature_flag_enabled=true", Version.fromString("8.12.0"), null),
FAILURE_STORE_ENABLED("es.failure_store_feature_flag_enabled=true", Version.fromString("8.12.0"), null);

public final String systemProperty;
Expand Down
3 changes: 1 addition & 2 deletions x-pack/plugin/ml/qa/basic-multi-node/build.gradle
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import org.elasticsearch.gradle.Version
import org.elasticsearch.gradle.internal.info.BuildParams

apply plugin: 'elasticsearch.legacy-java-rest-test'
Expand All @@ -17,7 +16,7 @@ testClusters.configureEach {
setting 'xpack.license.self_generated.type', 'trial'
setting 'indices.lifecycle.history_index_enabled', 'false'
setting 'slm.history_index_enabled', 'false'
requiresFeature 'es.learning_to_rank_feature_flag_enabled', Version.fromString("8.12.0")
setting 'xpack.ml.learning_to_rank.enabled', 'true'
}

if (BuildParams.inFipsJvm){
Expand Down
3 changes: 1 addition & 2 deletions x-pack/plugin/ml/qa/ml-with-security/build.gradle
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import org.elasticsearch.gradle.Version
apply plugin: 'elasticsearch.legacy-yaml-rest-test'

dependencies {
Expand Down Expand Up @@ -258,5 +257,5 @@ testClusters.configureEach {
user username: "no_ml", password: "x-pack-test-password", role: "minimal"
setting 'xpack.license.self_generated.type', 'trial'
setting 'xpack.security.enabled', 'true'
requiresFeature 'es.learning_to_rank_feature_flag_enabled', Version.fromString("8.12.0")
setting 'xpack.ml.learning_to_rank.enabled', 'true'
}
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,6 @@
import org.elasticsearch.xpack.ml.inference.ingest.InferenceProcessor;
import org.elasticsearch.xpack.ml.inference.loadingservice.ModelLoadingService;
import org.elasticsearch.xpack.ml.inference.ltr.LearningToRankRescorerBuilder;
import org.elasticsearch.xpack.ml.inference.ltr.LearningToRankRescorerFeature;
import org.elasticsearch.xpack.ml.inference.ltr.LearningToRankService;
import org.elasticsearch.xpack.ml.inference.modelsize.MlModelSizeNamedXContentProvider;
import org.elasticsearch.xpack.ml.inference.persistence.TrainedModelProvider;
Expand Down Expand Up @@ -890,7 +889,7 @@ private static void reportClashingNodeAttribute(String attrName) {

@Override
public List<RescorerSpec<?>> getRescorers() {
if (enabled && LearningToRankRescorerFeature.isEnabled()) {
if (enabled && learningToRankService.get().isEnabled()) {
return List.of(
new RescorerSpec<>(
LearningToRankRescorerBuilder.NAME,
Expand Down Expand Up @@ -1125,7 +1124,7 @@ public Collection<?> createComponents(PluginServices services) {
this.modelLoadingService.set(modelLoadingService);

this.learningToRankService.set(
new LearningToRankService(modelLoadingService, trainedModelProvider, services.scriptService(), services.xContentRegistry())
new LearningToRankService(modelLoadingService, trainedModelProvider, services.scriptService(), services.xContentRegistry(), settings)
);

this.deploymentManager.set(
Expand Down Expand Up @@ -1801,7 +1800,7 @@ public List<NamedXContentRegistry.Entry> getNamedXContent() {
);
namedXContent.addAll(new CorrelationNamedContentProvider().getNamedXContentParsers());
// LTR Combine with Inference named content provider when feature flag is removed
if (LearningToRankRescorerFeature.isEnabled()) {
if (learningToRankService.get().isEnabled()) {
namedXContent.addAll(new MlLTRNamedXContentProvider().getNamedXContentParsers());
}
return namedXContent;
Expand Down Expand Up @@ -1889,7 +1888,7 @@ public List<NamedWriteableRegistry.Entry> getNamedWriteables() {
namedWriteables.addAll(new CorrelationNamedContentProvider().getNamedWriteables());
namedWriteables.addAll(new ChangePointNamedContentProvider().getNamedWriteables());
// LTR Combine with Inference named content provider when feature flag is removed
if (LearningToRankRescorerFeature.isEnabled()) {
if (learningToRankService.get().isEnabled()) {
namedWriteables.addAll(new MlLTRNamedXContentProvider().getNamedWriteables());
}
return namedWriteables;
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@

package org.elasticsearch.xpack.ml.inference.ltr;

import org.elasticsearch.Build;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.query.MatchAllQueryBuilder;
import org.elasticsearch.index.query.MatchNoneQueryBuilder;
import org.elasticsearch.index.query.QueryBuilder;
Expand Down Expand Up @@ -52,33 +55,60 @@
import static org.elasticsearch.xpack.core.ml.job.messages.Messages.INFERENCE_CONFIG_QUERY_BAD_FORMAT;

public class LearningToRankService {
/**
* This setting behave like a feature flag for the learning to rank feature but can be set by an operator on self-managed environment.
* The feature is enabled by default only for snapshots builds.
*
* Before enabling by default, ensure transport serialization is all corrected for future BWC.
*
* See {@link LearningToRankRescorerBuilder}
*/
public static Setting<Boolean> LEARNING_TO_RANK_ENABLED = Setting.boolSetting("xpack.ml.learning_to_rank.enabled",
Build.current().isSnapshot(),
Setting.Property.OperatorDynamic,
Setting.Property.NodeScope
);

private static final Map<String, String> SCRIPT_OPTIONS = Map.ofEntries(
entry(MustacheScriptEngine.DETECT_MISSING_PARAMS_OPTION, Boolean.TRUE.toString())
);
private final ModelLoadingService modelLoadingService;
private final TrainedModelProvider trainedModelProvider;
private final ScriptService scriptService;
private final XContentParserConfiguration parserConfiguration;
private final boolean enabled;

public LearningToRankService(
ModelLoadingService modelLoadingService,
TrainedModelProvider trainedModelProvider,
ScriptService scriptService,
NamedXContentRegistry xContentRegistry
NamedXContentRegistry xContentRegistry,
Settings settings
) {
this(modelLoadingService, trainedModelProvider, scriptService, XContentParserConfiguration.EMPTY.withRegistry(xContentRegistry));
this(modelLoadingService, trainedModelProvider, scriptService, XContentParserConfiguration.EMPTY.withRegistry(xContentRegistry), settings);
}

LearningToRankService(
ModelLoadingService modelLoadingService,
TrainedModelProvider trainedModelProvider,
ScriptService scriptService,
XContentParserConfiguration parserConfiguration
XContentParserConfiguration parserConfiguration,
Settings settings
) {
this.modelLoadingService = modelLoadingService;
this.scriptService = scriptService;
this.trainedModelProvider = trainedModelProvider;
this.parserConfiguration = parserConfiguration;
this.enabled = LEARNING_TO_RANK_ENABLED.get(settings);
}

/**
* Check if the learning to rank is enabled or not.
*
* @return Whether the LTR feature is enabled or not.
*/
public boolean isEnabled() {
return enabled;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ private LearningToRankService getTestLearningToRankService(LearningToRankConfig
}

private LearningToRankService getTestLearningToRankService(TrainedModelProvider trainedModelProvider) {
return new LearningToRankService(mockModelLoadingService(), trainedModelProvider, getTestScriptService(), xContentRegistry());
return new LearningToRankService(mockModelLoadingService(), trainedModelProvider, getTestScriptService(), xContentRegistry(), Settings.EMPTY);
}

private ScriptService getTestScriptService() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public class XPackRestIT extends AbstractXPackRestTest {
.setting("xpack.searchable.snapshot.shared_cache.region_size", "256KB")
.user("x_pack_rest_user", "x-pack-test-password")
.feature(FeatureFlag.TIME_SERIES_MODE)
.feature(FeatureFlag.LEARNING_TO_RANK)
.setting("xpack.ml.learning_to_rank.enabled", "true")
.configFile("testnode.pem", Resource.fromClasspath("org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.pem"))
.configFile("testnode.crt", Resource.fromClasspath("org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.crt"))
.configFile("service_tokens", Resource.fromClasspath("service_tokens"))
Expand Down

0 comments on commit a135a54

Please sign in to comment.