-
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] allow for larger models in the inference step for data frame analytics #76116
[ML] allow for larger models in the inference step for data frame analytics #76116
Conversation
Pinging @elastic/ml-core (Team:ML) |
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
TrainedModelConfig config = loadModelFromResource(modelId, false).build().ensureParsedDefinition(xContentRegistry); | ||
TrainedModelConfig config = loadModelFromResource(modelId, false) | ||
.build() | ||
.ensureParsedDefinitionUnsafe(xContentRegistry); |
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.
This should respect the value of the unsafe
parameter the same as line 432
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.
@davidkyle I don't think it should. Models in the resourceFiles are provided in the jar distribution. So, I don't think we should ever check their stream length on parsing.
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.
So, I don't think we should ever check their stream length on parsing.
Because we know how big they are? Why not enforce that with a simple check.
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.
It is superfluous to me as the only way to adjust this resource is to modify the resource files directly on disk and since we control these resource models, we already know and trust their sizes.
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
💔 Backport failed
To backport manually run: |
…lytics (elastic#76116) When a user creates a Data frame analytics model, it is possible that the inference step fails due to he model being too large to fit in the JVM. Example error messages: ``` [foo] failed running inference on model [foo-1628085713000]; cause was [Data too large, data for [foo-1628085713000] would be [...], which is larger than the limit of [...]] ``` ``` [foo] failed running inference on model [foo-1628085713000]; cause was [Cannot parse model definition as the content is larger than the maximum stream size of [...] bytes. Max stream size is 10% of the JVM heap or 1GB whichever is smallest] ``` This commit partially addresses these error by allowing the circuit breaker to handle the OOM prevention. Since the model was recently created by an internal process, this is acceptable. relates to elastic#76093
…lytics (#76116) (#76256) When a user creates a Data frame analytics model, it is possible that the inference step fails due to he model being too large to fit in the JVM. Example error messages: ``` [foo] failed running inference on model [foo-1628085713000]; cause was [Data too large, data for [foo-1628085713000] would be [...], which is larger than the limit of [...]] ``` ``` [foo] failed running inference on model [foo-1628085713000]; cause was [Cannot parse model definition as the content is larger than the maximum stream size of [...] bytes. Max stream size is 10% of the JVM heap or 1GB whichever is smallest] ``` This commit partially addresses these error by allowing the circuit breaker to handle the OOM prevention. Since the model was recently created by an internal process, this is acceptable. relates to #76093
When a user creates a Data frame analytics model, it is possible that the inference step fails due to he model being too large to fit in the JVM.
Example error messages:
This commit partially addresses these error by allowing the circuit breaker to handle the OOM prevention. Since the model was recently created by an internal process, this is acceptable.
relates to #76093