[ML][Inference] adding license checks #49056
Conversation
|
Pinging @elastic/ml-core (:ml) |
|
run elasticsearch-ci/1 |
|
|
||
| @Override | ||
| public void licenseStateChanged() { | ||
| this.inferenceAllowed = licenseState.isMachineLearningAllowed(); |
There was a problem hiding this comment.
What is the mechanism which updates licenseState?
Am I right that it is a mutable object and the license listener notifies licenseStateChanged method after the internal state of licenseState object changes?
There was a problem hiding this comment.
Yes, it is a singleton that gets mutated in place. See other examples in org.elasticsearch.xpack.ml.InvalidLicenseEnforcer
x-pack/plugin/ml/src/test/java/org/elasticsearch/license/MachineLearningLicensingTests.java
Outdated
Show resolved
Hide resolved
x-pack/plugin/ml/src/test/java/org/elasticsearch/license/MachineLearningLicensingTests.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| @Override | ||
| protected void doExecute(Task task, InferModelAction.Request request, ActionListener<InferModelAction.Response> listener) { | ||
|
|
||
| if (licenseState.isMachineLearningAllowed() == false) { |
There was a problem hiding this comment.
Do other actions (get, delete) already have this license check?
There was a problem hiding this comment.
No, I don't think they should either. They don't really provide licensed value IMO. This is similar to how we treat anomaly jobs as well.
Co-Authored-By: Przemysław Witek <przemyslaw.witek@elastic.co>
| @@ -342,10 +342,16 @@ | |||
|
|
|||
| @Override | |||
| public Map<String, Processor.Factory> getProcessors(Processor.Parameters parameters) { | |||
| if (this.enabled == false) { | |||
| return Collections.emptyMap(); | |||
There was a problem hiding this comment.
This needs some due diligence. If there was an ingest pipeline in the cluster state containing an inference ingest processor and the cluster was restarted with xpack.ml.enabled: false what would happen?
The tentative plan for restoring full cluster snapshots in Cloud in the future is to disable all X-Pack plugins during the snapshot restore, which will lead to this exact situation.
If there's any doubt about what will happen it might be safer to allow the ingest processors to exist but just have them fail on every document they process (via the failure response from the infer model action) if the license is invalid.
There was a problem hiding this comment.
@droberts195 aren't xpack.ml.enabled and the license checks to different things? Additionally, this is exactly what the enrich project has done (if enrich is disabled, do not provide the processors). I will reach out to core features to see what they think about this.
There was a problem hiding this comment.
@droberts195 pipelines are stored as maps in the cluster state and are not fully marshaled until the pipeline is created on the ingest node. So, clusterstate restoration and pipeline instantiation are two different steps.
If there was an ingest pipeline in the cluster state containing an inference ingest processor and the cluster was restarted with
xpack.ml.enabled: falsewhat would happen?
The cluster will start up fine, the pipeline will just fail to be instantiated on the node.
There was a problem hiding this comment.
OK great. Sounds like it's not a problem then and the code can stay as it is.
* [ML][Inference] adds lazy model loader and inference (#47410) This adds a couple of things: - A model loader service that is accessible via transport calls. This service will load in models and cache them. They will stay loaded until a processor no longer references them - A Model class and its first sub-class LocalModel. Used to cache model information and run inference. - Transport action and handler for requests to infer against a local model Related Feature PRs: * [ML][Inference] Adjust inference configuration option API (#47812) * [ML][Inference] adds logistic_regression output aggregator (#48075) * [ML][Inference] Adding read/del trained models (#47882) * [ML][Inference] Adding inference ingest processor (#47859) * [ML][Inference] fixing classification inference for ensemble (#48463) * [ML][Inference] Adding model memory estimations (#48323) * [ML][Inference] adding more options to inference processor (#48545) * [ML][Inference] handle string values better in feature extraction (#48584) * [ML][Inference] Adding _stats endpoint for inference (#48492) * [ML][Inference] add inference processors and trained models to usage (#47869) * [ML][Inference] add new flag for optionally including model definition (#48718) * [ML][Inference] adding license checks (#49056) * [ML][Inference] Adding memory and compute estimates to inference (#48955)
* [ML][Inference] adds lazy model loader and inference (elastic#47410) This adds a couple of things: - A model loader service that is accessible via transport calls. This service will load in models and cache them. They will stay loaded until a processor no longer references them - A Model class and its first sub-class LocalModel. Used to cache model information and run inference. - Transport action and handler for requests to infer against a local model Related Feature PRs: * [ML][Inference] Adjust inference configuration option API (elastic#47812) * [ML][Inference] adds logistic_regression output aggregator (elastic#48075) * [ML][Inference] Adding read/del trained models (elastic#47882) * [ML][Inference] Adding inference ingest processor (elastic#47859) * [ML][Inference] fixing classification inference for ensemble (elastic#48463) * [ML][Inference] Adding model memory estimations (elastic#48323) * [ML][Inference] adding more options to inference processor (elastic#48545) * [ML][Inference] handle string values better in feature extraction (elastic#48584) * [ML][Inference] Adding _stats endpoint for inference (elastic#48492) * [ML][Inference] add inference processors and trained models to usage (elastic#47869) * [ML][Inference] add new flag for optionally including model definition (elastic#48718) * [ML][Inference] adding license checks (elastic#49056) * [ML][Inference] Adding memory and compute estimates to inference (elastic#48955)
* [ML] ML Model Inference Ingest Processor (#49052) * [ML][Inference] adds lazy model loader and inference (#47410) This adds a couple of things: - A model loader service that is accessible via transport calls. This service will load in models and cache them. They will stay loaded until a processor no longer references them - A Model class and its first sub-class LocalModel. Used to cache model information and run inference. - Transport action and handler for requests to infer against a local model Related Feature PRs: * [ML][Inference] Adjust inference configuration option API (#47812) * [ML][Inference] adds logistic_regression output aggregator (#48075) * [ML][Inference] Adding read/del trained models (#47882) * [ML][Inference] Adding inference ingest processor (#47859) * [ML][Inference] fixing classification inference for ensemble (#48463) * [ML][Inference] Adding model memory estimations (#48323) * [ML][Inference] adding more options to inference processor (#48545) * [ML][Inference] handle string values better in feature extraction (#48584) * [ML][Inference] Adding _stats endpoint for inference (#48492) * [ML][Inference] add inference processors and trained models to usage (#47869) * [ML][Inference] add new flag for optionally including model definition (#48718) * [ML][Inference] adding license checks (#49056) * [ML][Inference] Adding memory and compute estimates to inference (#48955) * fixing version of indexed docs for model inference
This adds license checks for the inference processor creation and the inference action.