[ML] Fail model deployment if all allocations cannot be provided #88656
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When we cannot scale up (autoscaling is disabled) we should fail requests to start a trained model deployment whose allocations cannot be provided.
|
Pinging @elastic/ml-core (Team:ML) |
benwtrent
reviewed
Jul 20, 2022
Comment on lines
513
to
519
| if (maxLazyMLNodes <= nodes.size() | ||
| && trainedModelAssignment.isSatisfied(nodes.stream().map(DiscoveryNode::getId).collect(Collectors.toSet())) == false) { | ||
| String msg = "Could not start deployment because there are not enough resources to provide all requested allocations"; | ||
| logger.debug(() -> format("[%s] %s", modelId, msg)); | ||
| exception = new ElasticsearchStatusException(msg, RestStatus.TOO_MANY_REQUESTS); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
We should also check to make sure that the current node size is less than the max node size (vertical scaling vs horizontal).
There was a problem hiding this comment.
Well spotted. I've added a commit that factors out a isScalingPossible and uses it in both places we make that check.
droberts195
reviewed
Jul 21, 2022
| @@ -527,6 +532,15 @@ public boolean test(ClusterState clusterState) { | |||
| ); | |||
| return false; | |||
| } | |||
|
|
|||
| private boolean isScalingPossible(List<DiscoveryNode> nodes) { | |||
There was a problem hiding this comment.
I think you should add a comment that this only considers memory, and in the future it would be nice to consider CPU too.
It means there's a discrepancy in how we handle different situations:
- If autoscaling is enabled, a cluster is scaled to maximum size, there are 2 free CPUs, and you ask to start a deployment that needs 3 CPUs then you get told you cannot.
- If autoscaling is enabled, a cluster is scaled to one step below its maximum size, there are no free CPUs, and you ask to start a deployment that needs 100000 CPUs then that's fine - we start it, the cluster scales to its maximum size and the deployment goes ahead but with far fewer than 100000 CPUs allocated to it.
While autoscaling doesn't understand CPU there's not a lot we can do about this, but it's worth at least adding comments to acknowledge where we are today.
There was a problem hiding this comment.
I have added a TODO comment.
benwtrent
approved these changes
Jul 21, 2022
dimitris-athanasiou
deleted the
fail-starting-deployment-if-model-cannot-be-satisfied
branch
weizijun
added a commit
to weizijun/elasticsearch
that referenced
this pull request
Jul 22, 2022
* upstream/master: (40 commits) Fix CI job naming [ML] disallow autoscaling downscaling in two trained model assignment scenarios (elastic#88623) Add "Vector Search" area to changelog schema [DOCS] Update API key API (elastic#88499) Enable the pipeline on the feature branch (elastic#88672) Adding the ability to register a PeerFinderListener to Coordinator (elastic#88626) [DOCS] Fix transform painless example syntax (elastic#88364) [ML] Muting InternalCategorizationAggregationTests testReduceRandom (elastic#88685) Fix double rounding errors for disk usage (elastic#88683) Replace health request with a state observer. (elastic#88641) [ML] Fail model deployment if all allocations cannot be provided (elastic#88656) Upgrade to OpenJDK 18.0.2+9 (elastic#88675) [ML] make bucket_correlation aggregation generally available (elastic#88655) Adding cardinality support for random_sampler agg (elastic#86838) Use custom task instead of generic AckedClusterStateUpdateTask (elastic#88643) Reinstate test cluster throttling behavior (elastic#88664) Mute testReadBlobWithPrematureConnectionClose Simplify plugin descriptor tests (elastic#88659) Add CI job for testing more job parallelism [ML] make deployment infer requests fully cancellable (elastic#88649) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When we cannot scale up (autoscaling is disabled) we should fail
requests to start a trained model deployment whose allocations
cannot be provided.