From 866b3fda292a3e4672d4ce7884d70d4ca7df0f07 Mon Sep 17 00:00:00 2001 From: Xuanqi He Date: Thu, 20 Aug 2026 12:12:01 -0400 Subject: [PATCH 1/3] Fix EC2 throttling not being retried and reported as insufficient capacity 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. --- CHANGELOG.md | 7 + src/slurm_plugin/fleet_manager.py | 29 +++++ tests/slurm_plugin/test_fleet_manager.py | 137 +++++++++++++++++++- tests/slurm_plugin/test_instance_manager.py | 32 +++++ 4 files changed, 203 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b42536f5..09197689 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,13 @@ aws-parallelcluster-node CHANGELOG This file is used to list changes made in each version of the aws-parallelcluster-node package. +3.17.0 +------ + +**BUG FIXES** +- Fix an issue where EC2 throttling during compute node launch is not retried and is reported as insufficient capacity + when using Multiple Instance Types or multiple subnets. + 3.16.0 ------ diff --git a/src/slurm_plugin/fleet_manager.py b/src/slurm_plugin/fleet_manager.py index 37fb37ab..82516165 100644 --- a/src/slurm_plugin/fleet_manager.py +++ b/src/slurm_plugin/fleet_manager.py @@ -30,6 +30,18 @@ INSTANCE_INFO_RETRIEVAL_TIMEOUT_DEFAULT = 90 INSTANCE_INFO_RETRIEVAL_MAX_BACKOFF = 30 +# The only error code EC2 returns when a launch request is throttled, see +# https://docs.aws.amazon.com/ec2/latest/devguide/ec2-api-throttling.html +LAUNCH_THROTTLING_ERROR_CODE = "RequestLimitExceeded" + +# 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. Other UnfulfillableCapacity +# messages, such as the one about MinTargetCapacity constraints, do describe a real cause and are kept. +UNFULFILLED_OVERRIDE_ERROR = ( + "UnfulfillableCapacity", + "Failed to fulfill capacity. Please review errors in the response.", +) + class EC2Instance: def __init__(self, id, private_ip, hostname, all_private_ips, launch_time): @@ -430,6 +442,23 @@ def _launch_instances(self, launch_params): if partial_instance_ids: logger.error("Unable to retrieve instance info for instances: %s", partial_instance_ids) + if not instances: + # Every error is logged above, but only the ones describing a cause are worth reporting. + real_errors = [ + err + for err in err_list + if (err.get("ErrorCode"), err.get("ErrorMessage")) != UNFULFILLED_OVERRIDE_ERROR + ] + if real_errors: + err_list = real_errors + # Throttling is reported even alongside other causes, so that the launch is retried with backoff + # instead of the nodes being recorded as insufficient capacity, which fails the compute resource over. + throttling = next( + (err for err in err_list if err.get("ErrorCode") == LAUNCH_THROTTLING_ERROR_CODE), None + ) + if throttling: + raise LaunchInstancesError(throttling.get("ErrorCode"), throttling.get("ErrorMessage")) + # Any other cause is reported only when the response is unambiguous, unchanged from before. if not instances and len(err_list) == 1: raise LaunchInstancesError(err_list[0].get("ErrorCode"), err_list[0].get("ErrorMessage")) return {"Instances": instances} diff --git a/tests/slurm_plugin/test_fleet_manager.py b/tests/slurm_plugin/test_fleet_manager.py index 988f3f21..a75ca1b9 100644 --- a/tests/slurm_plugin/test_fleet_manager.py +++ b/tests/slurm_plugin/test_fleet_manager.py @@ -29,6 +29,23 @@ from tests.common import FLEET_CONFIG, MockedBoto3Request +UNFULFILLED_OVERRIDE = { + "ErrorCode": "UnfulfillableCapacity", + "ErrorMessage": "Failed to fulfill capacity. Please review errors in the response.", +} +UNSUPPORTED_ERROR = {"ErrorCode": "Unsupported", "ErrorMessage": "Not supported in this AZ."} +MIN_TARGET_CAPACITY_ERROR = { + "ErrorCode": "UnfulfillableCapacity", + "ErrorMessage": "Unable to fulfill request due to MinTargetCapacity constraints. Please adjust your request.", +} + + +def _raises_launch_error(err_list): + """Mirror when _launch_instances turns a CreateFleet response with no instances into an exception.""" + real = [err for err in err_list if err != UNFULFILLED_OVERRIDE] or err_list + return any(err.get("ErrorCode") == "RequestLimitExceeded" for err in real) or len(real) == 1 + + def _expected_describe_attempts(timeout): """Compute DescribeInstances attempts for a never-converging instance, mirroring _get_instances_info.""" attempts = 0 @@ -735,6 +752,31 @@ def test_evaluate_launch_params( ], [], ), + # create-fleet - throttling reported together with one generic error per override + ( + test_on_demand_params, + [ + MockedBoto3Request( + method="create_fleet", + response={ + "Instances": [], + "Errors": [ + {"ErrorCode": "RequestLimitExceeded", "ErrorMessage": "Request limit exceeded."}, + ] + + [ + { + "ErrorCode": "UnfulfillableCapacity", + "ErrorMessage": "Failed to fulfill capacity. Please review errors in the response.", + } + ] + * 35, + "ResponseMetadata": {"RequestId": "37633199-bcc6-4a88-89e3-89d859d76096"}, + }, + expected_params=test_on_demand_params, + ), + ], + [], + ), ], ids=[ "fleet_spot", @@ -743,6 +785,7 @@ def test_evaluate_launch_params( "fleet_capacity_block", "fleet_throttling", "fleet_multiple_errors", + "fleet_throttling_with_generic_errors", ], ) def test_launch_instances( @@ -765,10 +808,10 @@ def test_launch_instances( with pytest.raises(Exception) as e: fleet_manager._launch_instances(launch_params) assert isinstance(e, ClientError) - elif len(expected_assigned_nodes) == 0 and len(mocked_boto3_request[0].response.get("Errors")) == 1: + elif not expected_assigned_nodes and _raises_launch_error(mocked_boto3_request[0].response.get("Errors", [])): with pytest.raises(LaunchInstancesError) as e: fleet_manager._launch_instances(launch_params) - assert isinstance(e, LaunchInstancesError) + assert_that(e.value.code).is_equal_to(mocked_boto3_request[0].response.get("Errors")[0].get("ErrorCode")) else: assigned_nodes = fleet_manager._launch_instances(launch_params) assert_that(assigned_nodes.get("Instances", [])).is_equal_to(expected_assigned_nodes) @@ -1326,3 +1369,93 @@ def test_launch_ec2_instances(self, mocker, count, job_id): fleet_manager._evaluate_launch_params.assert_called_once_with(count) fleet_manager._launch_instances.assert_called_once() + + def test_launch_ec2_instances_retries_on_throttling(self, mocker): + """Verify CreateFleet throttling is retried, also when the response carries one error per override.""" + mocker.patch("time.sleep") + fleet_manager = FleetManagerFactory.get_manager( + "hit", "region", "boto3_config", FLEET_CONFIG, "queue2", "fleet-ondemand", True, {}, {} + ) + mocker.patch.object(fleet_manager, "_evaluate_launch_params", return_value={}) + launched_instance_info = { + "InstanceId": "i-12345", + "PrivateIpAddress": "ip-1", + "PrivateDnsName": "hostname", + "LaunchTime": datetime(2020, 1, 1, tzinfo=timezone.utc), + "NetworkInterfaces": [ + {"Attachment": {"DeviceIndex": 0, "NetworkCardIndex": 0}, "PrivateIpAddress": "ip-1"}, + ], + } + mocker.patch.object( + fleet_manager, + "_get_instances_info", + side_effect=lambda instance_ids: ([launched_instance_info] if instance_ids else [], []), + ) + throttled_response = { + "Instances": [], + "Errors": [{"ErrorCode": "RequestLimitExceeded", "ErrorMessage": "Request limit exceeded."}] + + [ + { + "ErrorCode": "UnfulfillableCapacity", + "ErrorMessage": "Failed to fulfill capacity. Please review errors in the response.", + } + ] + * 35, + "ResponseMetadata": {"RequestId": "1234-abcde"}, + } + create_fleet = mocker.patch( + "slurm_plugin.fleet_manager.create_fleet", + side_effect=[throttled_response, {"Instances": [{"InstanceIds": ["i-12345"]}]}], + ) + + launched = fleet_manager.launch_ec2_instances(1) + + assert_that(create_fleet.call_count).is_equal_to(2) + assert_that(launched).is_length(1) + + @pytest.mark.parametrize( + ("err_list", "expected_error_code"), + [ + # The real cause is reported even though every other override adds an entry pointing back at it. + ([UNSUPPORTED_ERROR] + [UNFULFILLED_OVERRIDE] * 35, "Unsupported"), + # The entries pointing at the real cause carry no order guarantee. + ([UNFULFILLED_OVERRIDE] * 35 + [UNSUPPORTED_ERROR], "Unsupported"), + # UnfulfillableCapacity messages other than that one do describe a cause and are kept. + ([MIN_TARGET_CAPACITY_ERROR] + [UNFULFILLED_OVERRIDE] * 35, "UnfulfillableCapacity"), + # A single override is unaffected, whatever the entry says. + ([UNFULFILLED_OVERRIDE], "UnfulfillableCapacity"), + ([MIN_TARGET_CAPACITY_ERROR], "UnfulfillableCapacity"), + # Nothing to prefer: reporting stays as it was, so no cause is claimed. + ([UNFULFILLED_OVERRIDE] * 36, None), + ([UNSUPPORTED_ERROR, {"ErrorCode": "VcpuLimitExceeded", "ErrorMessage": "vCPU limit"}], None), + ], + ids=[ + "real_cause_first", + "real_cause_last", + "min_target_capacity_kept", + "single_override_unfulfilled", + "single_override_min_target_capacity", + "only_unfulfilled_overrides", + "two_real_causes", + ], + ) + def test_launch_instances_reports_the_cause_among_unfulfilled_overrides( + self, mocker, err_list, expected_error_code + ): + """An entry is added per override that was not fulfilled; only a real cause is worth reporting.""" + fleet_manager = FleetManagerFactory.get_manager( + "hit", "region", "boto3_config", FLEET_CONFIG, "queue2", "fleet-ondemand", False, {}, {} + ) + mocker.patch.object(fleet_manager, "_evaluate_launch_params", return_value={}) + mocker.patch.object(fleet_manager, "_get_instances_info", return_value=([], [])) + mocker.patch( + "slurm_plugin.fleet_manager.create_fleet", + return_value={"Instances": [], "Errors": err_list, "ResponseMetadata": {"RequestId": "1234-abcde"}}, + ) + + if expected_error_code: + with pytest.raises(LaunchInstancesError) as e: + fleet_manager._launch_instances({}) + assert_that(e.value.code).is_equal_to(expected_error_code) + else: + assert_that(fleet_manager._launch_instances({})).is_equal_to({"Instances": []}) diff --git a/tests/slurm_plugin/test_instance_manager.py b/tests/slurm_plugin/test_instance_manager.py index d7ae8834..58cecb14 100644 --- a/tests/slurm_plugin/test_instance_manager.py +++ b/tests/slurm_plugin/test_instance_manager.py @@ -4033,6 +4033,38 @@ def test_launch_instances( assert_that(instances_launched).is_equal_to(expected_instances_launched) assert_that(instance_manager.failed_nodes).is_equal_to(expected_failed_nodes) + def test_launch_instances_reports_throttling_not_insufficient_capacity(self, mocker, instance_manager): + """A throttled CreateFleet must not be recorded as insufficient capacity, which would fail over. + + CreateFleet reports one error per launch template override, so the throttling entry comes with a + generic entry for every other override. + """ + mocker.patch("time.sleep") + mocker.patch( + "slurm_plugin.fleet_manager.create_fleet", + return_value={ + "Instances": [], + "Errors": [{"ErrorCode": "RequestLimitExceeded", "ErrorMessage": "Request limit exceeded."}] + + [ + { + "ErrorCode": "UnfulfillableCapacity", + "ErrorMessage": "Failed to fulfill capacity. Please review errors in the response.", + } + ] + * 29, + "ResponseMetadata": {"RequestId": "1234-abcde"}, + }, + ) + + instance_manager._launch_instances( + job=None, + nodes_to_launch={"queue2": {"fleet-ondemand": ["queue2-dy-fleet-ondemand-1"]}}, + launch_batch_size=1, + scaling_strategy=ScalingStrategy.BEST_EFFORT, + ) + + assert_that(instance_manager.failed_nodes).is_equal_to({"RequestLimitExceeded": {"queue2-dy-fleet-ondemand-1"}}) + @pytest.mark.parametrize( "job_list, launch_batch_size, assign_node_batch_size, update_node_address, " "expected_single_nodes_no_oversubscribe, scaling_strategy", From c65b2bf3402a3835957e699ef38089e2cd2bb082 Mon Sep 17 00:00:00 2001 From: Xuanqi He Date: Thu, 20 Aug 2026 15:53:04 -0400 Subject: [PATCH 2/3] Refine the comments. --- src/slurm_plugin/fleet_manager.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/slurm_plugin/fleet_manager.py b/src/slurm_plugin/fleet_manager.py index 82516165..14e2613d 100644 --- a/src/slurm_plugin/fleet_manager.py +++ b/src/slurm_plugin/fleet_manager.py @@ -35,8 +35,7 @@ LAUNCH_THROTTLING_ERROR_CODE = "RequestLimitExceeded" # 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. Other UnfulfillableCapacity -# messages, such as the one about MinTargetCapacity constraints, do describe a real cause and are kept. +# code and message pair, which points at the real error rather than being one. UNFULFILLED_OVERRIDE_ERROR = ( "UnfulfillableCapacity", "Failed to fulfill capacity. Please review errors in the response.", @@ -443,7 +442,7 @@ def _launch_instances(self, launch_params): logger.error("Unable to retrieve instance info for instances: %s", partial_instance_ids) if not instances: - # Every error is logged above, but only the ones describing a cause are worth reporting. + # Drop the entries that only point at the real error, unless the response carries nothing else. real_errors = [ err for err in err_list @@ -451,8 +450,9 @@ def _launch_instances(self, launch_params): ] if real_errors: err_list = real_errors - # Throttling is reported even alongside other causes, so that the launch is retried with backoff - # instead of the nodes being recorded as insufficient capacity, which fails the compute resource over. + # A single cause is normally left. Should there be several, prefer throttling as a safety net: it is + # the only cause that resolves on its own, and reporting it as insufficient capacity would instead + # fail the compute resource over. throttling = next( (err for err in err_list if err.get("ErrorCode") == LAUNCH_THROTTLING_ERROR_CODE), None ) From 32288dc82a6a2e8aca7c6bb444230a27ed913ca4 Mon Sep 17 00:00:00 2001 From: Xuanqi He Date: Fri, 21 Aug 2026 16:19:05 -0400 Subject: [PATCH 3/3] Add a second saftey net: Report the first CreateFleet error when several remain --- src/slurm_plugin/fleet_manager.py | 6 ++++-- tests/slurm_plugin/test_fleet_manager.py | 13 +++++++------ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/slurm_plugin/fleet_manager.py b/src/slurm_plugin/fleet_manager.py index 14e2613d..14195b1e 100644 --- a/src/slurm_plugin/fleet_manager.py +++ b/src/slurm_plugin/fleet_manager.py @@ -458,8 +458,10 @@ def _launch_instances(self, launch_params): ) if throttling: raise LaunchInstancesError(throttling.get("ErrorCode"), throttling.get("ErrorMessage")) - # Any other cause is reported only when the response is unambiguous, unchanged from before. - if not instances and len(err_list) == 1: + # Normally a single cause is left. Reporting the first one of several is a second safety net: the + # caller otherwise records a hardcoded InsufficientInstanceCapacity, and any code EC2 actually + # returned is more useful than an invented one, whichever of them the response happens to list first. + if not instances and err_list: raise LaunchInstancesError(err_list[0].get("ErrorCode"), err_list[0].get("ErrorMessage")) return {"Instances": instances} except ClientError as e: diff --git a/tests/slurm_plugin/test_fleet_manager.py b/tests/slurm_plugin/test_fleet_manager.py index a75ca1b9..4b12274a 100644 --- a/tests/slurm_plugin/test_fleet_manager.py +++ b/tests/slurm_plugin/test_fleet_manager.py @@ -28,7 +28,6 @@ from tests.common import FLEET_CONFIG, MockedBoto3Request - UNFULFILLED_OVERRIDE = { "ErrorCode": "UnfulfillableCapacity", "ErrorMessage": "Failed to fulfill capacity. Please review errors in the response.", @@ -42,8 +41,7 @@ def _raises_launch_error(err_list): """Mirror when _launch_instances turns a CreateFleet response with no instances into an exception.""" - real = [err for err in err_list if err != UNFULFILLED_OVERRIDE] or err_list - return any(err.get("ErrorCode") == "RequestLimitExceeded" for err in real) or len(real) == 1 + return bool(err_list) def _expected_describe_attempts(timeout): @@ -1425,9 +1423,11 @@ def test_launch_ec2_instances_retries_on_throttling(self, mocker): # A single override is unaffected, whatever the entry says. ([UNFULFILLED_OVERRIDE], "UnfulfillableCapacity"), ([MIN_TARGET_CAPACITY_ERROR], "UnfulfillableCapacity"), - # Nothing to prefer: reporting stays as it was, so no cause is claimed. - ([UNFULFILLED_OVERRIDE] * 36, None), - ([UNSUPPORTED_ERROR, {"ErrorCode": "VcpuLimitExceeded", "ErrorMessage": "vCPU limit"}], None), + # Nothing to prefer: report the first entry rather than let a hardcoded code be recorded. + ([UNFULFILLED_OVERRIDE] * 36, "UnfulfillableCapacity"), + ([UNSUPPORTED_ERROR, {"ErrorCode": "VcpuLimitExceeded", "ErrorMessage": "vCPU limit"}], "Unsupported"), + # An empty error list is the only case left to the caller, which records insufficient capacity. + ([], None), ], ids=[ "real_cause_first", @@ -1437,6 +1437,7 @@ def test_launch_ec2_instances_retries_on_throttling(self, mocker): "single_override_min_target_capacity", "only_unfulfilled_overrides", "two_real_causes", + "no_errors_reported", ], ) def test_launch_instances_reports_the_cause_among_unfulfilled_overrides(