From 6ecd1e45e7c6fb956b96e4e4c09e469da164f7d4 Mon Sep 17 00:00:00 2001 From: Steve Yoo Date: Mon, 14 Sep 2026 08:24:47 -0400 Subject: [PATCH 1/3] Conditionally set default crt target_throughput --- .../next-release/enhancement-crt-44299.json | 5 + .../next-release/enhancement-crt-53269.json | 5 + awscli/customizations/s3/factory.py | 79 ++++++++- tests/unit/customizations/s3/test_factory.py | 159 +++++++++++++++++- 4 files changed, 243 insertions(+), 5 deletions(-) create mode 100644 .changes/next-release/enhancement-crt-44299.json create mode 100644 .changes/next-release/enhancement-crt-53269.json diff --git a/.changes/next-release/enhancement-crt-44299.json b/.changes/next-release/enhancement-crt-44299.json new file mode 100644 index 000000000000..8588ab3341b4 --- /dev/null +++ b/.changes/next-release/enhancement-crt-44299.json @@ -0,0 +1,5 @@ +{ + "type": "enhancement", + "category": "crt", + "description": "Enforce minimum 10gbps target throughput for explicitly configured crt environments" +} diff --git a/.changes/next-release/enhancement-crt-53269.json b/.changes/next-release/enhancement-crt-53269.json new file mode 100644 index 000000000000..6401268c6786 --- /dev/null +++ b/.changes/next-release/enhancement-crt-53269.json @@ -0,0 +1,5 @@ +{ + "type": "enhancement", + "category": "crt", + "description": "Set lower 4gbps target throughput default for non-EC2 hosts." +} diff --git a/awscli/customizations/s3/factory.py b/awscli/customizations/s3/factory.py index 188493dbecd3..81dac67458c4 100644 --- a/awscli/customizations/s3/factory.py +++ b/awscli/customizations/s3/factory.py @@ -44,6 +44,20 @@ # disabling retries, so it cannot honor a single attempt. MIN_CRT_MAX_ATTEMPTS = 2 +# Throughput target, in gigabits per second, for hosts the crt client has not +# been tuned for. Staying at 4 keeps it in its smallest memory pool tier. +UNTUNED_TARGET_THROUGHPUT_GBPS = 4.0 + +# Largest part size the crt client accepts in its smallest memory pool tier. It +# rejects a part size over half the pool, and rejects it while constructing the +# client, so a larger chunksize would fail every transfer rather than just +# large ones. +UNTUNED_MAX_MULTIPART_CHUNKSIZE = 128 * (1024**2) + +# Throughput target, in gigabits per second, to fall back to rather than +# accepting a lower recommendation from the crt. +MINIMUM_TARGET_THROUGHPUT_GBPS = 10.0 + WARN_IGNORED = 'warn_ignored' EXCLUDE_FROM_AUTO = 'exclude_from_auto' @@ -69,6 +83,10 @@ } +def _gbps_to_bytes_per_sec(gbps): + return int(gbps * 1_000_000_000 / 8) + + class ClientFactory: def __init__(self, session): self._session = session @@ -171,8 +189,24 @@ def _get_unsupported_settings(self, params, runtime_config): unsupported.append('uploads from a non-seekable stream') if self._is_retries_disabled(runtime_config): unsupported.append('max_attempts = 1') + if self._is_multipart_chunksize_too_large(runtime_config): + unsupported.append( + f'multipart_chunksize over ' + f'{UNTUNED_MAX_MULTIPART_CHUNKSIZE} on this system' + ) return unsupported + def _is_multipart_chunksize_too_large(self, runtime_config): + # Only the pool for untuned systems is small enough to reject a + # configurable chunksize. + if not self._is_untuned_system(): + return False + return ( + runtime_config.is_explicitly_set('multipart_chunksize') + and runtime_config['multipart_chunksize'] + > UNTUNED_MAX_MULTIPART_CHUNKSIZE + ) + def _is_retries_disabled(self, runtime_config): max_attempts = self._resolve_max_attempts(runtime_config) return max_attempts is not None and max_attempts < MIN_CRT_MAX_ATTEMPTS @@ -309,7 +343,7 @@ def _create_crt_client( endpoint_url = params.get('endpoint_url') if endpoint_url and urlparse.urlparse(endpoint_url).scheme == 'http': create_crt_client_kwargs['use_ssl'] = False - target_throughput = runtime_config.get('target_bandwidth', None) + target_throughput = self._resolve_target_throughput(runtime_config) if target_throughput: create_crt_client_kwargs['target_throughput'] = target_throughput create_crt_client_kwargs.update(config_kwargs) @@ -349,12 +383,49 @@ def _resolve_crt_client_config_kwargs(self, runtime_config): kwargs['retry_options'] = {'max_retries': max_attempts - 1} return kwargs - def _should_use_transfer_config_defaults(self, runtime_config): - preferred = runtime_config.get('preferred_transfer_client') - if preferred == constants.CRT_TRANSFER_CLIENT: + def _resolve_target_throughput(self, runtime_config): + target_throughput = runtime_config.get('target_bandwidth') + if target_throughput is not None: + return target_throughput + if self._is_preferring_crt_client(runtime_config): + # Users who opted into the crt transfer client keep the throughput + # they get today, even on hosts the crt recommends less for. + recommended = awscrt.s3.get_recommended_throughput_target_gbps() + return _gbps_to_bytes_per_sec( + max(recommended or 0, MINIMUM_TARGET_THROUGHPUT_GBPS) + ) + if self._is_newly_eligible_for_crt_client(runtime_config) and ( + self._is_untuned_system() + ): + # The crt client sizes its memory pool from the throughput target. + # Without a recommendation it assumes 10gbps, which maps to a max + # pool size of 2GiB. Newly-eligible hosts that auto-resolve to crt + # may not be able to afford 2GiB, so it sets the maximum throughput + # that maps to the smallest 256MiB tier. + return _gbps_to_bytes_per_sec(UNTUNED_TARGET_THROUGHPUT_GBPS) + return None + + def _is_untuned_system(self): + # The crt client has no throughput recommendation for systems it has + # not been tuned for. + return awscrt.s3.get_recommended_throughput_target_gbps() is None + + def _is_preferring_crt_client(self, runtime_config): + return ( + runtime_config.get('preferred_transfer_client') + == constants.CRT_TRANSFER_CLIENT + ) + + def _is_newly_eligible_for_crt_client(self, runtime_config): + if self._is_preferring_crt_client(runtime_config): return False return not awscrt.s3.is_optimized_for_system() + def _should_use_transfer_config_defaults(self, runtime_config): + # Configurations that already resolve to the crt transfer client keep + # its defaults so their behavior is unchanged. + return self._is_newly_eligible_for_crt_client(runtime_config) + def _create_crt_request_serializer(self, params): return BotocoreCRTRequestSerializer( self._session, diff --git a/tests/unit/customizations/s3/test_factory.py b/tests/unit/customizations/s3/test_factory.py index e5517561e279..a21dd715a1e8 100644 --- a/tests/unit/customizations/s3/test_factory.py +++ b/tests/unit/customizations/s3/test_factory.py @@ -25,6 +25,8 @@ from awscli.customizations.s3 import constants from awscli.customizations.s3.factory import ( ADAPTIVE_RETRY_MODE, + MINIMUM_TARGET_THROUGHPUT_GBPS, + UNTUNED_MAX_MULTIPART_CHUNKSIZE, ClientFactory, TransferManagerFactory, ) @@ -573,7 +575,7 @@ def test_fio_options_configure_for_crt_manager(self, mock_crt_client): mock_crt_client.call_args[1]['fio_options'], expected_fio_options ) - @mock.patch('s3transfer.crt.get_recommended_throughput_target_gbps') + @mock.patch('awscrt.s3.get_recommended_throughput_target_gbps') @mock.patch('s3transfer.crt.S3Client') def test_target_bandwidth_uses_crt_recommended_throughput( self, mock_crt_client, mock_get_target_gbps @@ -795,6 +797,7 @@ def resolve_client_type( auto_resolve_factory, s3_params, mock_crt_is_optimized_for_system, + mock_crt_recommended_throughput, mock_crt_lock_held, ): def _resolve(**kwargs): @@ -959,6 +962,52 @@ def test_does_not_warn_when_classic_explicitly_preferred( assert capsys.readouterr().err == '' +@pytest.fixture +def mock_crt_get_ec2_instance_type(): + with mock.patch('awscrt.s3.get_ec2_instance_type') as mock_instance_type: + mock_instance_type.return_value = None + yield mock_instance_type + + +@pytest.fixture +def mock_crt_recommended_throughput(): + # The factory and s3transfer each hold their own reference, and which one + # resolves the target depends on the transfer client being created. + with ( + mock.patch( + 'awscrt.s3.get_recommended_throughput_target_gbps' + ) as mock_recommended, + mock.patch( + 's3transfer.crt.get_recommended_throughput_target_gbps', + new=mock_recommended, + ), + ): + mock_recommended.return_value = None + yield mock_recommended + + +@pytest.fixture +def crt_s3_client_kwargs( + auto_resolve_factory, + s3_params, + mock_crt_is_optimized_for_system, + mock_crt_get_ec2_instance_type, + mock_crt_recommended_throughput, + mock_crt_s3_client, + mock_crt_process_lock, +): + """Creates a crt transfer manager and returns the S3Client kwargs""" + + def _create(**kwargs): + runtime_config = RuntimeConfig().build_config(**kwargs) + auto_resolve_factory._create_crt_transfer_manager( + s3_params, runtime_config + ) + return mock_crt_s3_client.call_args[1] + + return _create + + @pytest.fixture def crt_client_kwargs(auto_resolve_factory, mock_crt_is_optimized_for_system): def _resolve(**kwargs): @@ -1261,3 +1310,111 @@ def test_maps_configured_max_attempts_when_crt_explicitly_preferred( preferred_transfer_client=constants.CRT_TRANSFER_CLIENT ) assert kwargs['retry_options'] == {'max_retries': 4} + + +class TestTargetThroughput: + def test_targets_less_when_crt_has_no_recommendation( + self, crt_s3_client_kwargs + ): + assert crt_s3_client_kwargs()['throughput_target_gbps'] == 4.0 + + def test_defers_to_crt_recommendation_when_it_has_one( + self, crt_s3_client_kwargs, mock_crt_recommended_throughput + ): + mock_crt_recommended_throughput.return_value = 50.0 + assert crt_s3_client_kwargs()['throughput_target_gbps'] == 50.0 + + def test_targets_less_on_ec2_hosts_crt_cannot_recommend_for( + self, crt_s3_client_kwargs, mock_crt_get_ec2_instance_type + ): + # Being on EC2 does not mean the crt client sized a pool for this + # host, so the instance type must not decide the throughput target. + mock_crt_get_ec2_instance_type.return_value = 't3.micro' + assert crt_s3_client_kwargs()['throughput_target_gbps'] == 4.0 + + def test_configured_target_bandwidth_wins(self, crt_s3_client_kwargs): + kwargs = crt_s3_client_kwargs(target_bandwidth=1_250_000_000) + assert kwargs['throughput_target_gbps'] == 10.0 + + def test_floors_throughput_when_crt_explicitly_preferred( + self, crt_s3_client_kwargs, mock_crt_recommended_throughput + ): + mock_crt_recommended_throughput.return_value = 3.0 + + kwargs = crt_s3_client_kwargs( + preferred_transfer_client=constants.CRT_TRANSFER_CLIENT + ) + assert ( + kwargs['throughput_target_gbps'] == MINIMUM_TARGET_THROUGHPUT_GBPS + ) + + def test_keeps_higher_recommendation_when_crt_explicitly_preferred( + self, crt_s3_client_kwargs, mock_crt_recommended_throughput + ): + mock_crt_recommended_throughput.return_value = 50.0 + kwargs = crt_s3_client_kwargs( + preferred_transfer_client=constants.CRT_TRANSFER_CLIENT + ) + assert kwargs['throughput_target_gbps'] == 50.0 + + def test_does_not_floor_configured_target_bandwidth( + self, crt_s3_client_kwargs + ): + kwargs = crt_s3_client_kwargs( + preferred_transfer_client=constants.CRT_TRANSFER_CLIENT, + target_bandwidth=125_000_000, + ) + assert kwargs['throughput_target_gbps'] == 1.0 + + def test_defers_to_crt_on_optimized_host( + self, + crt_s3_client_kwargs, + mock_crt_is_optimized_for_system, + mock_crt_recommended_throughput, + ): + mock_crt_is_optimized_for_system.return_value = True + mock_crt_recommended_throughput.return_value = 3.0 + assert crt_s3_client_kwargs()['throughput_target_gbps'] == 3.0 + + +class TestLargeMultipartChunksize: + def test_resolves_to_classic_when_chunksize_exceeds_pool( + self, resolve_client_type + ): + assert ( + resolve_client_type( + multipart_chunksize=UNTUNED_MAX_MULTIPART_CHUNKSIZE + 1 + ) + == constants.CLASSIC_TRANSFER_CLIENT + ) + + def test_resolves_to_crt_at_the_largest_accepted_chunksize( + self, resolve_client_type + ): + assert ( + resolve_client_type( + multipart_chunksize=UNTUNED_MAX_MULTIPART_CHUNKSIZE + ) + == constants.CRT_TRANSFER_CLIENT + ) + + def test_resolves_to_crt_on_tuned_systems( + self, resolve_client_type, mock_crt_recommended_throughput + ): + # A tuned system gets a pool large enough for the configured chunksize. + mock_crt_recommended_throughput.return_value = 50.0 + assert ( + resolve_client_type( + multipart_chunksize=UNTUNED_MAX_MULTIPART_CHUNKSIZE * 8 + ) + == constants.CRT_TRANSFER_CLIENT + ) + + def test_does_not_warn_when_falling_back_for_chunksize( + self, resolve_client_type, capsys + ): + # Classic honors the configured chunksize, so nothing is lost. + resolve_client_type( + multipart_chunksize=UNTUNED_MAX_MULTIPART_CHUNKSIZE + 1 + ) + assert capsys.readouterr().err == '' From 162ea13f1f6247b93d42872a1be5cf4020e02f13 Mon Sep 17 00:00:00 2001 From: Steve Yoo Date: Tue, 15 Sep 2026 14:51:58 -0400 Subject: [PATCH 2/3] Fix part size handling --- awscli/customizations/s3/factory.py | 63 ++++++----- tests/unit/customizations/s3/test_factory.py | 109 +++++++++++++------ 2 files changed, 113 insertions(+), 59 deletions(-) diff --git a/awscli/customizations/s3/factory.py b/awscli/customizations/s3/factory.py index 81dac67458c4..bc500ab4885b 100644 --- a/awscli/customizations/s3/factory.py +++ b/awscli/customizations/s3/factory.py @@ -32,6 +32,7 @@ from awscli.customizations.s3 import constants from awscli.customizations.s3.transferconfig import ( DEFAULTS, + InvalidConfigError, create_transfer_config_from_runtime_config, ) from awscli.customizations.utils import uni_print @@ -48,16 +49,17 @@ # been tuned for. Staying at 4 keeps it in its smallest memory pool tier. UNTUNED_TARGET_THROUGHPUT_GBPS = 4.0 -# Largest part size the crt client accepts in its smallest memory pool tier. It -# rejects a part size over half the pool, and rejects it while constructing the -# client, so a larger chunksize would fail every transfer rather than just -# large ones. -UNTUNED_MAX_MULTIPART_CHUNKSIZE = 128 * (1024**2) - # Throughput target, in gigabits per second, to fall back to rather than # accepting a lower recommendation from the crt. MINIMUM_TARGET_THROUGHPUT_GBPS = 10.0 +# The crt client rejects a part size over half of its memory pool while it is +# being constructed. The pool is sized from the throughput target, and neither +# the sizing nor the limit is exposed, so the only way to know a multipart +# chunksize does not fit is to build the client and see. awscrt raises a plain +# RuntimeError for this, leaving the error code as the only thing to match on. +CRT_PART_SIZE_EXCEEDS_MEMORY_LIMIT = 14371 + WARN_IGNORED = 'warn_ignored' EXCLUDE_FROM_AUTO = 'exclude_from_auto' @@ -121,13 +123,38 @@ def create_transfer_manager( client_type = self._compute_transfer_client_type( params, runtime_config ) - self.warn_unsupported_settings(client_type, runtime_config) if client_type == constants.CRT_TRANSFER_CLIENT: + transfer_manager = self._try_create_crt_transfer_manager( + params, runtime_config + ) + if transfer_manager is not None: + self.warn_unsupported_settings(client_type, runtime_config) + return transfer_manager + client_type = constants.CLASSIC_TRANSFER_CLIENT + self.warn_unsupported_settings(client_type, runtime_config) + return self._create_classic_transfer_manager( + params, runtime_config, botocore_client + ) + + def _try_create_crt_transfer_manager(self, params, runtime_config): + try: return self._create_crt_transfer_manager(params, runtime_config) - else: - return self._create_classic_transfer_manager( - params, runtime_config, botocore_client + except RuntimeError as e: + if str(CRT_PART_SIZE_EXCEEDS_MEMORY_LIMIT) not in str(e): + raise + if self._is_preferring_crt_client(runtime_config): + raise InvalidConfigError( + f'The configured multipart_chunksize is too large for the ' + f"'{constants.CRT_TRANSFER_CLIENT}' s3 transfer client. " + f'Lower multipart_chunksize or raise the ' + f'memory available to the transfer client by setting the ' + f'AWS_CRT_S3_MEMORY_LIMIT_IN_GIB environment variable.' + ) from e + LOGGER.debug( + f'Not using the crt s3 transfer client because the configured ' + f'multipart_chunksize does not fit its memory pool: {e}' ) + return None def _compute_transfer_client_type(self, params, runtime_config): if params.get('paths_type') == 's3s3': @@ -189,24 +216,8 @@ def _get_unsupported_settings(self, params, runtime_config): unsupported.append('uploads from a non-seekable stream') if self._is_retries_disabled(runtime_config): unsupported.append('max_attempts = 1') - if self._is_multipart_chunksize_too_large(runtime_config): - unsupported.append( - f'multipart_chunksize over ' - f'{UNTUNED_MAX_MULTIPART_CHUNKSIZE} on this system' - ) return unsupported - def _is_multipart_chunksize_too_large(self, runtime_config): - # Only the pool for untuned systems is small enough to reject a - # configurable chunksize. - if not self._is_untuned_system(): - return False - return ( - runtime_config.is_explicitly_set('multipart_chunksize') - and runtime_config['multipart_chunksize'] - > UNTUNED_MAX_MULTIPART_CHUNKSIZE - ) - def _is_retries_disabled(self, runtime_config): max_attempts = self._resolve_max_attempts(runtime_config) return max_attempts is not None and max_attempts < MIN_CRT_MAX_ATTEMPTS diff --git a/tests/unit/customizations/s3/test_factory.py b/tests/unit/customizations/s3/test_factory.py index a21dd715a1e8..500b45804355 100644 --- a/tests/unit/customizations/s3/test_factory.py +++ b/tests/unit/customizations/s3/test_factory.py @@ -10,6 +10,7 @@ # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF # ANY KIND, either express or implied. See the License for the specific # language governing permissions and limitations under the License. +import awscrt.exceptions import awscrt.s3 import pytest import s3transfer.crt @@ -25,12 +26,15 @@ from awscli.customizations.s3 import constants from awscli.customizations.s3.factory import ( ADAPTIVE_RETRY_MODE, + CRT_PART_SIZE_EXCEEDS_MEMORY_LIMIT, MINIMUM_TARGET_THROUGHPUT_GBPS, - UNTUNED_MAX_MULTIPART_CHUNKSIZE, ClientFactory, TransferManagerFactory, ) -from awscli.customizations.s3.transferconfig import RuntimeConfig +from awscli.customizations.s3.transferconfig import ( + InvalidConfigError, + RuntimeConfig, +) from awscli.testutils import FileCreator, mock, unittest @@ -1377,44 +1381,83 @@ def test_defers_to_crt_on_optimized_host( assert crt_s3_client_kwargs()['throughput_target_gbps'] == 3.0 -class TestLargeMultipartChunksize: - def test_resolves_to_classic_when_chunksize_exceeds_pool( - self, resolve_client_type - ): - assert ( - resolve_client_type( - multipart_chunksize=UNTUNED_MAX_MULTIPART_CHUNKSIZE + 1 - ) - == constants.CLASSIC_TRANSFER_CLIENT +class TestChunksizeExceedingCrtMemoryPool: + """The crt client only reports an oversized chunksize while constructing.""" + + @pytest.fixture + def part_size_error(self): + return RuntimeError( + f'{CRT_PART_SIZE_EXCEEDS_MEMORY_LIMIT} ' + f'(AWS_ERROR_S3_PART_SIZE_EXCEEDS_MEMORY_LIMIT): Part size ' + f'exceeds the configured memory limit.' ) - def test_resolves_to_crt_at_the_largest_accepted_chunksize( - self, resolve_client_type - ): - assert ( - resolve_client_type( - multipart_chunksize=UNTUNED_MAX_MULTIPART_CHUNKSIZE + @pytest.fixture + def create_manager(self, auto_resolve_factory, s3_params): + def _create(**kwargs): + runtime_config = RuntimeConfig().build_config(**kwargs) + return auto_resolve_factory.create_transfer_manager( + s3_params, runtime_config, mock.Mock() ) - == constants.CRT_TRANSFER_CLIENT - ) - def test_resolves_to_crt_on_tuned_systems( - self, resolve_client_type, mock_crt_recommended_throughput - ): - # A tuned system gets a pool large enough for the configured chunksize. - mock_crt_recommended_throughput.return_value = 50.0 + return _create + + @pytest.fixture + def crt_manager_raises(self, auto_resolve_factory, part_size_error): + with mock.patch.object( + auto_resolve_factory, + '_create_crt_transfer_manager', + side_effect=part_size_error, + ) as mock_create: + yield mock_create + + def test_error_code_still_means_what_we_match_on(self): + # Guards against awscrt renumbering the code out from under us. assert ( - resolve_client_type( - multipart_chunksize=UNTUNED_MAX_MULTIPART_CHUNKSIZE * 8 - ) - == constants.CRT_TRANSFER_CLIENT + awscrt.exceptions.from_code( + CRT_PART_SIZE_EXCEEDS_MEMORY_LIMIT + ).name + == 'AWS_ERROR_S3_PART_SIZE_EXCEEDS_MEMORY_LIMIT' ) - def test_does_not_warn_when_falling_back_for_chunksize( - self, resolve_client_type, capsys + def test_falls_back_to_classic_when_auto_resolved( + self, create_manager, crt_manager_raises, mock_crt_lock_held + ): + assert isinstance(create_manager(), TransferManager) + + def test_does_not_warn_when_falling_back( + self, create_manager, crt_manager_raises, mock_crt_lock_held, capsys ): # Classic honors the configured chunksize, so nothing is lost. - resolve_client_type( - multipart_chunksize=UNTUNED_MAX_MULTIPART_CHUNKSIZE + 1 - ) + create_manager() assert capsys.readouterr().err == '' + + def test_raises_when_crt_explicitly_preferred( + self, create_manager, crt_manager_raises + ): + with pytest.raises(InvalidConfigError) as excinfo: + create_manager( + preferred_transfer_client=constants.CRT_TRANSFER_CLIENT + ) + message = str(excinfo.value) + assert 'multipart_chunksize' in message + assert 'AWS_CRT_S3_MEMORY_LIMIT_IN_GIB' in message + # Explicit crt must never be told to switch to classic. + assert constants.CLASSIC_TRANSFER_CLIENT not in message + + def test_reraises_unrelated_runtime_errors( + self, create_manager, auto_resolve_factory, mock_crt_lock_held + ): + with mock.patch.object( + auto_resolve_factory, + '_create_crt_transfer_manager', + side_effect=RuntimeError('something else entirely'), + ): + with pytest.raises(RuntimeError, match='something else entirely'): + create_manager() + + def test_uses_crt_when_the_chunksize_fits( + self, create_manager, mock_crt_lock_held, mock_crt_s3_client + ): + manager = create_manager(multipart_chunksize=8 * 1024 * 1024) + assert not isinstance(manager, TransferManager) From ae0bf95a730693c3aaff639465ca4089ec4eedab Mon Sep 17 00:00:00 2001 From: Steve Yoo Date: Wed, 16 Sep 2026 13:55:52 -0400 Subject: [PATCH 3/3] Release lock on fallback --- awscli/customizations/s3/factory.py | 2 ++ awscli/s3transfer/crt.py | 11 +++++++++++ tests/unit/customizations/s3/test_factory.py | 17 ++++++++++++++--- 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/awscli/customizations/s3/factory.py b/awscli/customizations/s3/factory.py index bc500ab4885b..a47b9681e0b7 100644 --- a/awscli/customizations/s3/factory.py +++ b/awscli/customizations/s3/factory.py @@ -25,6 +25,7 @@ acquire_crt_s3_process_lock, create_crt_client_bootstrap, create_s3_crt_client, + release_crt_s3_process_lock, ) from s3transfer.manager import TransferManager @@ -154,6 +155,7 @@ def _try_create_crt_transfer_manager(self, params, runtime_config): f'Not using the crt s3 transfer client because the configured ' f'multipart_chunksize does not fit its memory pool: {e}' ) + release_crt_s3_process_lock() return None def _compute_transfer_client_type(self, params, runtime_config): diff --git a/awscli/s3transfer/crt.py b/awscli/s3transfer/crt.py index 625412d1db2f..9105c63baefe 100644 --- a/awscli/s3transfer/crt.py +++ b/awscli/s3transfer/crt.py @@ -96,6 +96,17 @@ def acquire_crt_s3_process_lock(name): return CRT_S3_PROCESS_LOCK +def release_crt_s3_process_lock(): + # Acquiring the lock signals to other processes that this one is using the + # CRT S3 client, so a process that acquired it and then did not use the + # client has to release it. Otherwise it denies the client to every other + # process of the same application for the rest of its lifetime. + global CRT_S3_PROCESS_LOCK + if CRT_S3_PROCESS_LOCK is not None: + CRT_S3_PROCESS_LOCK.release() + CRT_S3_PROCESS_LOCK = None + + def create_s3_crt_client( region, crt_credentials_provider=None, diff --git a/tests/unit/customizations/s3/test_factory.py b/tests/unit/customizations/s3/test_factory.py index 500b45804355..9e895fea8da3 100644 --- a/tests/unit/customizations/s3/test_factory.py +++ b/tests/unit/customizations/s3/test_factory.py @@ -32,9 +32,9 @@ TransferManagerFactory, ) from awscli.customizations.s3.transferconfig import ( - InvalidConfigError, - RuntimeConfig, + InvalidConfigError as InvalidTransferConfigError, ) +from awscli.customizations.s3.transferconfig import RuntimeConfig from awscli.testutils import FileCreator, mock, unittest @@ -1425,6 +1425,17 @@ def test_falls_back_to_classic_when_auto_resolved( ): assert isinstance(create_manager(), TransferManager) + def test_releases_process_lock_when_falling_back( + self, create_manager, crt_manager_raises, mock_crt_lock_held + ): + # Holding the lock while running classic denies the crt client to + # every other process of the same application. + with mock.patch( + 'awscli.customizations.s3.factory.release_crt_s3_process_lock' + ) as mock_release: + create_manager() + assert mock_release.called + def test_does_not_warn_when_falling_back( self, create_manager, crt_manager_raises, mock_crt_lock_held, capsys ): @@ -1435,7 +1446,7 @@ def test_does_not_warn_when_falling_back( def test_raises_when_crt_explicitly_preferred( self, create_manager, crt_manager_raises ): - with pytest.raises(InvalidConfigError) as excinfo: + with pytest.raises(InvalidTransferConfigError) as excinfo: create_manager( preferred_transfer_client=constants.CRT_TRANSFER_CLIENT )