-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Fix scale up for model allocations #115189
Conversation
Pinging @elastic/ml-core (Team:ML) |
@@ -177,7 +177,7 @@ public boolean isEmpty() { | |||
return anomalyDetectionTasks.isEmpty() | |||
&& snapshotUpgradeTasks.isEmpty() | |||
&& dataframeAnalyticsTasks.isEmpty() | |||
&& modelAssignments.values().stream().allMatch(assignment -> assignment.totalTargetAllocations() == 0); | |||
&& modelAssignments.values().stream().allMatch(assignment -> assignment.getTaskParams().getNumberOfAllocations() == 0); |
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 code is so confusing...
Anyway, assignment.totalTargetAllocations()
is the number of allocations assigned to ML nodes, but if there are no nodes, there are no assigned allocations. assignment.getTaskParams().getNumberOfAllocations()
is the number that's wanted.
I've also added a test for this.
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
@@ -177,7 +177,7 @@ public boolean isEmpty() { | |||
return anomalyDetectionTasks.isEmpty() | |||
&& snapshotUpgradeTasks.isEmpty() | |||
&& dataframeAnalyticsTasks.isEmpty() | |||
&& modelAssignments.values().stream().allMatch(assignment -> assignment.totalTargetAllocations() == 0); | |||
&& modelAssignments.values().stream().allMatch(assignment -> assignment.getTaskParams().getNumberOfAllocations() == 0); |
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.
Oh because if there are no ml nodes in the cluster assignment.totalTargetAllocations() == 0
even if assignment.getTaskParams().getNumberOfAllocations() > 0
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.
Yes, indeed
## Summary Semantic text feature was disabled by default due to ML node unable to scale down. With the relevant [PR](elastic/elasticsearch#114323) & subsequent [fix](elastic/elasticsearch#115189) merged, ML node now auto scales when there is no activity. Therefore enabling semantic_text feature in es3. ### Testing instructions * start serverless instance * visit index management index details page -> mappings * Click Add field * Confirm `semantic_text` is shown in the field type form
Fixes: #114930