Skip to content

[ML] stop using isAllowedByLicense for model license checks #79908

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Oct 28, 2021
Merged

[ML] stop using isAllowedByLicense for model license checks #79908

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
050184f
[ML] stop using isAllowedByLicense for model license checks
benwtrent Oct 27, 2021
52d1dce
Merge remote-tracking branch 'upstream/master' into feature/remove-ml…
benwtrent Oct 28, 2021
bba0132
formatting
benwtrent Oct 28, 2021
02fed70
addressing PR comments
benwtrent Oct 28, 2021
dee500e
Apply suggestions from code review
benwtrent Oct 28, 2021
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
23 changes: 23 additions & 0 deletions x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/MachineLearningField.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,17 @@ public final class MachineLearningField {
License.OperationMode.PLATINUM
);

public static final LicensedFeature.Momentary ML_MODEL_INFERENCE_PLATINUM_FEATURE = LicensedFeature.momentary(
MachineLearningField.ML_FEATURE_FAMILY,
"model-inference-platinum-check",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@benwtrent Followup thought: this string is meant to be sent to users. Is there something we could use that doesn't have the license level or "check" in it? It's meant to be a short descriptor of what the feature is. How do these two license levels (basic vs platinum) for model inference differ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rjernst you are 100% right here. I thought about this a tad more and simplified our checks.

Since we were doing API license checks (Plat), we only need to verify if the model is basic or not. So, this new feature tracking isn't required.

See here: #80031

License.OperationMode.PLATINUM
);
public static final LicensedFeature.Momentary ML_MODEL_INFERENCE_BASIC_FEATURE = LicensedFeature.momentary(
MachineLearningField.ML_FEATURE_FAMILY,
"model-inference-basic-check",
License.OperationMode.BASIC
);

private MachineLearningField() {}

public static String valuesToId(String... values) {
Expand All @@ -45,4 +56,16 @@ public static String valuesToId(String... values) {
System.arraycopy(Numbers.longToBytes(hash.h2), 0, hashedBytes, 8, 8);
return new BigInteger(hashedBytes) + "_" + combined.length();
}

public static LicensedFeature.Momentary featureFromLicenseLevel(License.OperationMode mode) {
switch (mode) {
case BASIC:
return ML_MODEL_INFERENCE_BASIC_FEATURE;
case PLATINUM:
return ML_MODEL_INFERENCE_PLATINUM_FEATURE;
default:
assert false: "Unrecognized licensing mode for inference models [" + mode + "]";
return ML_MODEL_INFERENCE_PLATINUM_FEATURE;
}
}
}
1 change: 1 addition & 0 deletions ...ugin/core/src/main/java/org/elasticsearch/xpack/core/ml/inference/TrainedModelConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,7 @@ public long getEstimatedOperations() {
}

//TODO if we ever support anything other than "basic" and platinum, we need to adjust our feature tracking logic
// Additionally, see `MachineLearningField.featureFromLicenseLevel` for handling modes
public License.OperationMode getLicenseLevel() {
return licenseLevel;
}
Expand Down
10 changes: 8 additions & 2 deletions ...ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportInternalInferModelAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.license.LicenseUtils;
import org.elasticsearch.license.LicensedFeature;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.tasks.Task;
Expand All @@ -42,6 +43,7 @@

import static org.elasticsearch.xpack.core.ClientHelper.ML_ORIGIN;
import static org.elasticsearch.xpack.core.ClientHelper.executeAsyncWithOrigin;
import static org.elasticsearch.xpack.core.ml.MachineLearningField.featureFromLicenseLevel;


public class TransportInternalInferModelAction extends HandledTransportAction<Request, Response> {
Expand Down Expand Up @@ -79,8 +81,10 @@ protected void doExecute(Task task, Request request, ActionListener<Response> li
} else {
trainedModelProvider.getTrainedModel(request.getModelId(), GetTrainedModelsAction.Includes.empty(), ActionListener.wrap(
trainedModelConfig -> {
responseBuilder.setLicensed(licenseState.isAllowedByLicense(trainedModelConfig.getLicenseLevel()));
if (licenseState.isAllowedByLicense(trainedModelConfig.getLicenseLevel()) || request.isPreviouslyLicensed()) {
LicensedFeature.Momentary check = featureFromLicenseLevel(trainedModelConfig.getLicenseLevel());
final boolean allowed = check.check(licenseState);
responseBuilder.setLicensed(allowed);
if (allowed || request.isPreviouslyLicensed()) {
doInfer(request, responseBuilder, listener);
} else {
listener.onFailure(LicenseUtils.newComplianceException(XPackField.MACHINE_LEARNING));
Expand Down Expand Up @@ -183,4 +187,6 @@ private void inferSingleDocAgainstAllocatedModel(
)
);
}


}
4 changes: 3 additions & 1 deletion .../src/main/java/org/elasticsearch/xpack/ml/action/TransportPutTrainedModelAliasAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import java.util.Set;
import java.util.function.Predicate;

import static org.elasticsearch.xpack.core.ml.MachineLearningField.featureFromLicenseLevel;
import static org.elasticsearch.xpack.core.ml.job.messages.Messages.TRAINED_MODEL_INPUTS_DIFFER_SIGNIFICANTLY;

public class TransportPutTrainedModelAliasAction extends AcknowledgedTransportMasterNodeAction<PutTrainedModelAliasAction.Request> {
Expand Down Expand Up @@ -90,7 +91,8 @@ protected void masterOperation(
ActionListener<AcknowledgedResponse> listener
) throws Exception {
final boolean mlSupported = MachineLearningField.ML_API_FEATURE.check(licenseState);
final Predicate<TrainedModelConfig> isLicensed = (model) -> mlSupported || licenseState.isAllowedByLicense(model.getLicenseLevel());
final Predicate<TrainedModelConfig> isLicensed = (model) -> mlSupported
|| featureFromLicenseLevel(model.getLicenseLevel()).check(licenseState);
final String oldModelId = ModelAliasMetadata.fromState(state).getModelId(request.getModelAlias());

if (oldModelId != null && (request.isReassign() == false)) {
Expand Down
3 changes: 2 additions & 1 deletion ...n/java/org/elasticsearch/xpack/ml/aggs/inference/InferencePipelineAggregationBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import java.util.function.Supplier;

import static org.elasticsearch.xcontent.ConstructingObjectParser.constructorArg;
import static org.elasticsearch.xpack.core.ml.MachineLearningField.featureFromLicenseLevel;
import static org.elasticsearch.xpack.ml.utils.SecondaryAuthorizationUtils.useSecondaryAuthIfAvailable;

public class InferencePipelineAggregationBuilder extends AbstractPipelineAggregationBuilder<InferencePipelineAggregationBuilder> {
Expand Down Expand Up @@ -231,7 +232,7 @@ public InferencePipelineAggregationBuilder rewrite(QueryRewriteContext context)
loadedModel.set(model);

boolean isLicensed = MachineLearningField.ML_API_FEATURE.check(licenseState) ||
licenseState.isAllowedByLicense(model.getLicenseLevel());
featureFromLicenseLevel(model.getLicenseLevel()).check(licenseState);
if (isLicensed) {
delegate.onResponse(null);
} else {
Expand Down