[ML][Inference] Adding _stats endpoint for inference #48492

benwtrent · 2019-10-24T20:16:00Z

This adds the _stats endpoint for inference.

In the future, it might make sense to add cache stats or something to this.

Right now it only provides ingest stats and the total number of pipelines that reference the trained models. I decided against displaying the individual node stats as that seemed too granular, and that information is already accessible via a node/_stats/ingest call.

This _stats endpoint is a focus around the ingest pipelines that contain inference processors.

I think future work around "loaded" or "cached" or not might be useful as well.

elasticmachine · 2019-10-24T20:16:03Z

Pinging @elastic/ml-core (:ml)

benwtrent · 2019-10-24T20:17:47Z

server/src/main/java/org/elasticsearch/ingest/IngestStats.java

@jakelandis I added equals and hashCode objects here as I needed them for testing and for hashing the objects.

Adding them seemed like a no brainer to me, but I don't want to change this code without somebody who works on ingest giving their blessing.

this part of the change LGTM (didn't review the other parts)

hendrikmuhs

LGTM, added some minor suggestions

hendrikmuhs · 2019-10-29T09:00:15Z

...n/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetTrainedModelsStatsAction.java

+                builder.field(PIPELINE_COUNT.getPreferredName(), pipelineCount);
+                if (pipelineCount > 0) {
+                    // Ingest stats is a fragment
+                    builder.value(ingestStats);


I think it's better to call ingestStats.toXContent(builder, params);

hendrikmuhs · 2019-10-29T09:03:47Z

...n/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetTrainedModelsStatsAction.java

+            public TrainedModelStats(String modelId, IngestStats ingestStats, int pipelineCount) {
+                this.modelId = Objects.requireNonNull(modelId);
+                this.ingestStats = ingestStats == null ? EMPTY_INGEST_STATS : ingestStats;
+                this.pipelineCount = pipelineCount;


nit: paranoia, as we use VInt which is strictly >0, maybe assert > 0

hendrikmuhs · 2019-10-29T09:07:02Z

...n/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetTrainedModelsStatsAction.java

+            public TrainedModelStats(StreamInput in) throws IOException {
+                modelId = in.readString();
+                ingestStats = new IngestStats(in);
+                pipelineCount = in.readVInt();


what about swaping pipelineCount and ingestStats and only read/write the stats object if pipelineCount > 0??

hendrikmuhs · 2019-10-29T09:11:15Z

...n/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetTrainedModelsStatsAction.java

+                Collections.emptyList(),
+                Collections.emptyMap());
+
+            public TrainedModelStats(String modelId, IngestStats ingestStats, int pipelineCount) {


not sure: pipelineCount is int, modelCount below is long - should we use the same type for consistency?

pipelineCount is int because it is the size of the pipeline stats array. Java returns an int from a size() call.

As for modelCount it is the total number of models matched by the provided ids. If _all is provided, then the total number of models is returned.

Additionally, the underlying QueryPage object assumes that the count value is long.

hendrikmuhs · 2019-10-29T10:20:15Z

...ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportGetTrainedModelsStatsAction.java

+                .size(request.getPageParams().getSize());
+        }
+        sourceBuilder.trackTotalHits(true)
+            // we only care about the item IDS, there is no need to load large model definitions.


nit: "item IDS" -> "item id's" ??

…ture/ml-inference-add-_stats-endpoint

* [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

benwtrent added >non-issue :ml Machine learning v8.0.0 v7.6.0 labels Oct 24, 2019

benwtrent commented Oct 24, 2019

View reviewed changes

[ML][Inference] Adding _stats endpoint for inference

91ff7b5

benwtrent force-pushed the feature/ml-inference-add-_stats-endpoint branch from 9fdf346 to 91ff7b5 Compare October 24, 2019 20:31

benwtrent added 2 commits October 25, 2019 12:55

updating tests and returned format

73a57e3

setting origin of stats request

660439d

hendrikmuhs approved these changes Oct 29, 2019

View reviewed changes

benwtrent added 4 commits October 29, 2019 07:27

addressing PR comments

367033d

Merge remote-tracking branch 'upstream/feature/ml-inference' into fea…

7abbd4b

…ture/ml-inference-add-_stats-endpoint

removing unused import

efb4062

fixing method call

9044262

benwtrent merged commit c2946d1 into elastic:feature/ml-inference Oct 29, 2019

benwtrent mentioned this pull request Nov 18, 2019

[7.x][ML] ML Model Inference Ingest Processor (#49052) #49257

Merged

benwtrent deleted the feature/ml-inference-add-_stats-endpoint branch November 18, 2019 17:12

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[ML][Inference] Adding _stats endpoint for inference #48492

[ML][Inference] Adding _stats endpoint for inference #48492

Uh oh!

benwtrent commented Oct 24, 2019

Uh oh!

elasticmachine commented Oct 24, 2019

Uh oh!

benwtrent Oct 24, 2019

Uh oh!

jakelandis Oct 24, 2019

Uh oh!

hendrikmuhs left a comment

Uh oh!

hendrikmuhs Oct 29, 2019

Uh oh!

hendrikmuhs Oct 29, 2019

Uh oh!

hendrikmuhs Oct 29, 2019

Uh oh!

hendrikmuhs Oct 29, 2019

Uh oh!

benwtrent Oct 29, 2019

Uh oh!

hendrikmuhs Oct 29, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[ML][Inference] Adding _stats endpoint for inference #48492

[ML][Inference] Adding _stats endpoint for inference #48492

Uh oh!

Conversation

benwtrent commented Oct 24, 2019

Uh oh!

elasticmachine commented Oct 24, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hendrikmuhs left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants