-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
[ML] improve trained model stats API performance #87978
[ML] improve trained model stats API performance #87978
Conversation
Hi @benwtrent, I've created a changelog YAML for you. |
Pinging @elastic/ml-core (Team:ML) |
Reviewer, issue #87931 is focused on making the API cancellable. I will do that in a separate PR to reduce the review space and code churn :). |
@elasticmachine update branch |
I know we have a report saying that this got slow after upgrading from 7.17 to 8.x, but I cannot see why this couldn't have been just as slow in 7.17 assuming the same ingest pipelines existed. Is there a good explanation for why the 7.17 get trained model stats code would not have suffered from the problem of slow Grok processor instantiation? If there isn't then I think this fix should be backported to 7.17.6 (and 8.3.1 too, to get it out to 8.x users sooner). |
@droberts195 the only thing I can think of is that there may be more solutions/apps in Kibana and Elasticsearch making more "system managed" ingest pipelines in 8 vs 7. So, the increase in the number of pipelines pushed this over the edge. Doing a quick search of any "grok" specific changes, indicates nothing that would make stuff incredibly slower. It is true, any user with a large number of pipelines (especially complicated GROK ones), may run into this problem. I am good with backporting it to 7.17 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM apart from a couple of minor nits
.../plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/ingest/InferenceProcessor.java
Show resolved
Hide resolved
.../ml/src/test/java/org/elasticsearch/xpack/ml/utils/InferenceProcessorInfoExtractorTests.java
Outdated
Show resolved
Hide resolved
.../ml/src/test/java/org/elasticsearch/xpack/ml/utils/InferenceProcessorInfoExtractorTests.java
Outdated
Show resolved
Hide resolved
.../plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/ingest/InferenceProcessor.java
Outdated
Show resolved
Hide resolved
@elasticmachine update branch |
@elasticmachine update branch |
@elasticmachine update branch |
Previous, get trained model stats API would build every pipeline defined in cluster state. This is problematic when MANY pipelines are defined. Especially if those pipelines take some time to parse (consider GROK). This improvement is part of fixing: elastic#87931
💔 Backport failed
You can use sqren/backport to manually backport by running |
Previous, get trained model stats API would build every pipeline defined in cluster state. This is problematic when MANY pipelines are defined. Especially if those pipelines take some time to parse (consider GROK). This improvement is part of fixing: #87931
Previous, get trained model stats API would build every pipeline defined in cluster state. This is problematic when MANY pipelines are defined. Especially if those pipelines take some time to parse (consider GROK). This improvement is part of fixing: elastic#87931
Previous, get trained model stats API would build every pipeline defined in cluster state. This is problematic when MANY pipelines are defined. Especially if those pipelines take some time to parse (consider GROK). This improvement is part of fixing: #87931
Previous, get trained model stats API would build every pipeline defined in cluster state.
This is problematic when MANY pipelines are defined. Especially if those pipelines take some time to parse (consider
GROK
).This improvement is part of fixing: #87931