Fix EC2 throttling during computenode launch is not retried and is reported as ICE when using multi InstanceTypes or SubnetIds - #734
Conversation
…acity
CreateFleet returns one error entry per launch template override, so with more than one instance type or
subnet a single failure also produces an ("UnfulfillableCapacity", "Failed to fulfill capacity. Please
review errors in the response.") entry for every other override. The pre-existing `len(err_list) == 1`
condition therefore never held, and every cause was flattened into a hardcoded
InsufficientInstanceCapacity, which is in EC2_ICE_ERROR_CODES and fails the compute resource over for
insufficient_capacity_timeout.
Drop those entries before choosing what to report, keeping the response as is when it carries nothing else.
Other UnfulfillableCapacity messages, notably the MinTargetCapacity one that all-or-nothing scaling
produces, do describe a cause and are kept. Throttling is preferred over any remaining cause so that a
launch which only needs a retry is not abandoned. Single-override compute resources are unaffected.
A throttled batch now consumes the retry budget on launch_ec2_instances (10 attempts, 810s of backoff),
which can delay _store_assigned_hostnames past the window compute nodes use to read their hostname from
DynamoDB.
hehe7318
changed the title
hehe7318
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #734 +/- ##
===========================================
+ Coverage 91.06% 91.09% +0.02%
===========================================
Files 20 20
Lines 3213 3222 +9
===========================================
+ Hits 2926 2935 +9
Misses 287 287
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
|
||
| # An override that CreateFleet did not fulfill because another override failed is reported with this exact | ||
| # code and message pair, which points at the real error rather than being one. | ||
| UNFULFILLED_OVERRIDE_ERROR = ( |
There was a problem hiding this comment.
What does "override" mean here
There was a problem hiding this comment.
It means the launch template "override".
| real_errors = [ | ||
| err | ||
| for err in err_list | ||
| if (err.get("ErrorCode"), err.get("ErrorMessage")) != UNFULFILLED_OVERRIDE_ERROR |
There was a problem hiding this comment.
UnfulfillableCapacity is real error to right?
According to https://docs.aws.amazon.com/ec2/latest/devguide/errors-overview.html:
At this time there isn't enough spare capacity to fulfill your request for Spot Instances. You can wait a few minutes to see whether capacity becomes available for your request. Alternatively, create a more flexible request. For example, include additional instance types, include additional Availability Zones, or use the capacity-optimized allocation strategy.
Is it safe to drop it?
There was a problem hiding this comment.
Yes it's safe. The filter matches the code and the error message as a pair, not the code
alone.
The code+message pair we dropped is:
("UnfulfillableCapacity", "Failed to fulfill capacity. Please review errors in the response.")
Every other UnfulfillableCapacity message passes through untouched, including
"Unable to fulfill request due to MinTargetCapacity constraints. Please adjust your request and try again.",
which is what all-or-nothing produces and is a genuine cause.
And the message itself is a evidence. "Please review errors in the response" means something if the response carries another error to review. As the sole entry it would be self-referential.
2 participants
Description of changes
Customer impact
Since #578, EC2 throttling during a compute node scale-up is never retried, and every launch failure is instead reported as insufficient capacity.
Preconditions: a compute resource with more than one instance type or more than one subnet, and a scale-up large enough to exhaust the
RunInstancesresource token bucket, which holds 1000 instances and refills at 2/s. See the number in ref link.On
best-effort(the default): a single throttled call disables the whole compute resource, not only the batch that failed. Measured withMaxCount 3000andsbatch -N 1500: 500 nodes went DOWN and the remaining 1500 gotTemporarily disabling node due to insufficient capacity— 2000 of 3000 nodes unusable for 600s (insufficient_capacity_timeout), with the job stuckPENDING.On
all-or-nothing: the compute resource cannot scale up reliably. Every round releases the instances already launched and starts over, so a large scale-up would fail.Root cause
CreateFleetreturns one entry inErrorsper launch template override, so a compute resource with more than one instance type or more than one subnet gets an entry for every override that was not fulfilled, not only for the one that actually failed. Those extra entries all carry the same pair:which points at the real error rather than being one.
With 36 overrides a single failure therefore arrives as 36 entries, so the pre-existing
len(err_list) == 1condition inEc2CreateFleetManager._launch_instancesnever holds. NoLaunchInstancesErroris raised, andInstanceManager._launch_ec2_instancesfalls back to a hardcodedInsufficientInstanceCapacity. Three things follow:launch_ec2_instancesalready implements the exponential backoff the EC2 documentation recommends, sized to wait for the bucket to refill a fulllaunch_max_batch_sizebatch, but no exception ever reaches it. The batch is abandoned after a single attempt even though retrying is all it needed.InsufficientInstanceCapacityis inSlurmNode.EC2_ICE_ERROR_CODES, soclustermgtdtreats the failure as a capacity shortage and disables the whole compute resource, not just the batch that failed.VcpuLimitExceeded, anUnsupportedinstance type or anUnauthorizedOperationis all flattened into insufficient capacity.The fix
len(err_list) == 1condition works again for multi-override compute resources. The response is left untouched when those entries are all it carries, so the worst case is today's behaviour.Tests
References
Checklist
developadd the branch name as prefix in the PR title (e.g.[release-3.6]).Please review the guidelines for contributing and Pull Request Instructions.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.