Fix ml autoscaling for zero allocations #114982
Conversation
jan-elastic
added
:ml
|
Pinging @elastic/ml-core (Team:ML) |
| if (assignment.getNodeRoutingTable().isEmpty() == false | ||
| && assignment.getNodeRoutingTable().values().stream().allMatch(r -> r.getState().consumesMemory() == false)) { | ||
| // Ignore states that don't consume memory, for example all allocations are failed or stopped | ||
| // if the node routing table is empty, then it will match the above condition, but it needs to be handled in the next branch | ||
| continue; | ||
| } | ||
|
|
||
| if (assignment.getNodeRoutingTable().isEmpty() == false) { |
There was a problem hiding this comment.
Everything down here in this file is just indentation.
If you want to review it, I'd recommend settings -> hide whitespace
jan-elastic
force-pushed
the
fix-ml-autoscaling-for-zero-allocations
branch
from
9714a73 to
d4d069d
Compare
| @@ -623,6 +623,9 @@ public String getDeploymentId() { | |||
| * @return the estimated memory (in bytes) required for the model deployment to run | |||
| */ | |||
| public long estimateMemoryUsageBytes() { | |||
| if (numberOfAllocations == 0) { | |||
There was a problem hiding this comment.
This method is on TaskParams - StartTrainedModelDeploymentAction.TaskParams. estimateMemoryUsageBytes()
There is another public method StartTrainedModelDeploymentAction.estimateMemoryUsageBytes() on line 792 that needs this check.
If StartTrainedModelDeploymentAction.estimateMemoryUsageBytes() can return - then line 635 + (cacheSize.getBytes() - modelBytes); needs a Max.(0,...) to ensure the return is non-negative
There was a problem hiding this comment.
Fixed StartTrainedModelDeploymentAction.estimateMemoryUsageBytes
I don't think the Max is necessary. StartTrainedModelDeploymentAction.estimateMemoryUsageBytes returns 0 only if the number of allocations is 0, in which case the TaskParams.estimateMemoryUsageBytes already returns 0.
💔 Backport failedThe backport operation could not be completed due to the following error: You can use sqren/backport to manually backport by running |
* Fix estimated memory usage for a model with zero allocations. * Ignore number of threads of models with zero allocations in autoscaling decisions. * Add some long overdue comments. * Another estimateMemoryUsageBytes fix
* Fix estimated memory usage for a model with zero allocations. * Ignore number of threads of models with zero allocations in autoscaling decisions. * Add some long overdue comments. * Another estimateMemoryUsageBytes fix
* Fix estimated memory usage for a model with zero allocations. * Ignore number of threads of models with zero allocations in autoscaling decisions. * Add some long overdue comments. * Another estimateMemoryUsageBytes fix
Fixes: #114930