From ec775883c7b530d6779ddc5090644b5568b9e8f3 Mon Sep 17 00:00:00 2001 From: Krish <> Date: Mon, 24 Nov 2025 10:20:55 -0800 Subject: [PATCH 01/21] setup --- include/aws/s3/private/s3_client_impl.h | 3 + include/aws/s3/private/s3_meta_request_impl.h | 3 + source/s3_client.c | 114 +++++++++++++++++- 3 files changed, 119 insertions(+), 1 deletion(-) diff --git a/include/aws/s3/private/s3_client_impl.h b/include/aws/s3/private/s3_client_impl.h index 0fb87ca57..110ad1056 100644 --- a/include/aws/s3/private/s3_client_impl.h +++ b/include/aws/s3/private/s3_client_impl.h @@ -275,6 +275,9 @@ struct aws_s3_client { /* The calculated ideal number of HTTP connections, based on throughput target and throughput per connection. */ const uint32_t ideal_connection_count; + /* Tokens added when meta requests are created and subtracted when requests use the tokens to allocate connections */ + struct struct_atomic_var token_bucket; + /** * For multi-part upload, content-md5 will be calculated if the AWS_MR_CONTENT_MD5_ENABLED is specified * or initial request has content-md5 header. diff --git a/include/aws/s3/private/s3_meta_request_impl.h b/include/aws/s3/private/s3_meta_request_impl.h index e9f3b5dc7..1779a7ac9 100644 --- a/include/aws/s3/private/s3_meta_request_impl.h +++ b/include/aws/s3/private/s3_meta_request_impl.h @@ -191,6 +191,9 @@ struct aws_s3_meta_request { enum aws_s3_meta_request_type type; struct aws_string *s3express_session_host; + /* Estimated size of a meta request. for file downloads, discovery request reveal size. + * In other cases, we preemptively knew the size or will never know the size of the object. */ + size_t object_size; /* Is the meta request made to s3express bucket or not. */ bool is_express; /* If the buffer pool optimized for the specific size or not. */ diff --git a/source/s3_client.c b/source/s3_client.c index 5d9370a06..5e6b33912 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -75,6 +75,36 @@ const uint32_t g_min_num_connections = 10; /* Magic value based on: 10 was old b * be 2500 Gbps. */ const uint32_t g_max_num_connections = 10000; +/* This is a first pass at a token based implementation, the calculations are approximate and can be improved in the + * future. The idea is to scale the number of connections we require up and down based on the different requests we + * receive and hence dynamically scale the maximum number of connections we need to open. One token is equivalent to + * 1Mbps of throughput. */ + +/* All throughput values are in MBps and provided by S3 team */ + +// 90 MBps +const uint32_t s_s3_download_throughput_per_connection_mbps = 90 * 8; +// 20 MBps +const uint32_t s_s3_upload_throughput_per_connection_mbps = 20 * 8; +// 150 MBps +const uint32_t s_s3_express_download_throughput_per_connection_mbps = 150 * 8; +// 100 MBps +const uint32_t s_s3_express_upload_throughput_per_connection_mbps = 100 * 8; + +/* All latency values are in milliseconds (ms) and provided by S3 team */ +// 30ms +const uint32_t s_s3_p50_request_latency_ms = 30; +// 4ms +const uint32_t s_s3_express_p50_request_latency_ms = 4; + +const uint32_t s_s3_client_minimum_concurrent_requests = 8; + +/* Currently the ideal part size is 8MB and hence the value set. + * However, this is subject to change due to newer part sizes and adjustments. */ +const uint32_t s_ideal_part_size = 8 * 8; + +const uint32_t s_s3_minimum_tokens = s_ideal_part_size * 8 * s_s3_client_minimum_concurrent_requests; + /** * Default max part size is 5GiB as the server limit. */ @@ -206,6 +236,88 @@ uint32_t aws_s3_client_get_max_active_connections( return max_active_connections; } +/* Initialize token bucket based on target throughput */ +void aws_s3_client_init_tokens(struct aws_s3_client *client, double target_throughput_gbps) { + AWS_PRECONDITION(client); + if (target_throughput_gbps == 0.0) target_throughput_gbps = 150.0; + aws_atomic_store_int(&client->token_bucket, aws_max_u32(target_throughput_gbps * 1024, s_s3_minimum_tokens)); +} + +/* Releases tokens back after request is complete. */ +void aws_s3_client_release_tokens( + struct aws_s3_client *client, + struct aws_s3_request *request) { + AWS_PRECONDITION(client); + AWS_PRECONDITION(request); + + uint32_t tokens = 0; + + switch (request->request_type) { + case AWS_S3_REQUEST_TYPE_GET_OBJECT: { + if (request->meta_request->is_express) { + tokens = aws_min_u32(ceil((request->buffer_size * 1000) / s_s3_express_p50_request_latency_ms), s_s3_express_download_throughput_per_connection_mbps); + } else { + tokens = aws_min_u32(ceil((request->buffer_size * 1000) / s_s3_p50_request_latency_ms), s_s3_download_throughput_per_connection_mbps); + } + break; + } + case AWS_S3_REQUEST_TYPE_PUT_OBJECT: { + if (request->meta_request->is_express) { + tokens = aws_min_u32(ceil((request->buffer_size * 1000) / s_s3_express_p50_request_latency_ms), s_s3_express_upload_throughput_per_connection_mbps); + } else { + tokens = aws_min_u32(ceil((request->buffer_size * 1000) / s_s3_p50_request_latency_ms), s_s3_upload_throughput_per_connection_mbps); + } + break; + } + default: { + tokens = g_default_min_tokens; + } + } + + // do we need error handling here? + aws_atomic_fetch_add(client->token_bucket, tokens); +} + +/* Returns true or false based on whether the request was able to avail the required amount of tokens. + * TODO: try to introduce a scalability factor instead of using pure latency. */ +bool aws_s3_client_acquire_tokens( + struct aws_s3_client *client, + struct aws_s3_request *request) { + AWS_PRECONDITION(client); + AWS_PRECONDITION(request); + + uint32_t required_tokens = 0; + + switch (request->request_type) { + case AWS_S3_REQUEST_TYPE_GET_OBJECT: { + if (request->meta_request->is_express) { + required_tokens = aws_min_u32(ceil((request->buffer_size * 1000) / s_s3_express_p50_request_latency_ms), s_s3_express_download_throughput_per_connection_mbps); + } else { + required_tokens = aws_min_u32(ceil((request->buffer_size * 1000) / s_s3_p50_request_latency_ms), s_s3_download_throughput_per_connection_mbps); + } + break; + } + case AWS_S3_REQUEST_TYPE_PUT_OBJECT: { + if (request->meta_request->is_express) { + required_tokens = aws_min_u32(ceil((request->buffer_size * 1000) / s_s3_express_p50_request_latency_ms), s_s3_express_upload_throughput_per_connection_mbps); + } else { + required_tokens = aws_min_u32(ceil((request->buffer_size * 1000) / s_s3_p50_request_latency_ms), s_s3_upload_throughput_per_connection_mbps); + } + break; + } + default: { + required_tokens = g_default_min_tokens; + } + } + + if ((uint32_t *) aws_atomic_load_int(&client->token_bucket) > required_tokens) { + // do we need error handling here? + aws_atomic_fetch_sub(client->token_bucket, required_tokens); + return true; + } + return false; +} + /* Returns the max number of requests allowed to be in memory */ uint32_t aws_s3_client_get_max_requests_in_flight(struct aws_s3_client *client) { AWS_PRECONDITION(client); @@ -2286,7 +2398,7 @@ void aws_s3_client_update_connections_threaded(struct aws_s3_client *client) { s_s3_client_meta_request_finished_request(client, meta_request, request, AWS_ERROR_S3_CANCELED); request = aws_s3_request_release(request); - } else if ((uint32_t)aws_atomic_load_int(&meta_request->num_requests_network) < max_active_connections) { + } else if (aws_s3_avail_tokens(client, request)) { /* Make sure it's above the max request level limitation. */ s_s3_client_create_connection_for_request(client, request); } else { From 72fcc1fbb7eef4fec260fa5ca68466e9636d51e6 Mon Sep 17 00:00:00 2001 From: Krish <> Date: Mon, 24 Nov 2025 10:29:35 -0800 Subject: [PATCH 02/21] convert bytes to megabits --- source/s3_client.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/source/s3_client.c b/source/s3_client.c index 5e6b33912..06c478d5f 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -255,17 +255,17 @@ void aws_s3_client_release_tokens( switch (request->request_type) { case AWS_S3_REQUEST_TYPE_GET_OBJECT: { if (request->meta_request->is_express) { - tokens = aws_min_u32(ceil((request->buffer_size * 1000) / s_s3_express_p50_request_latency_ms), s_s3_express_download_throughput_per_connection_mbps); + tokens = aws_min_u32(ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_express_p50_request_latency_ms)), s_s3_express_download_throughput_per_connection_mbps); } else { - tokens = aws_min_u32(ceil((request->buffer_size * 1000) / s_s3_p50_request_latency_ms), s_s3_download_throughput_per_connection_mbps); + tokens = aws_min_u32(ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_p50_request_latency_ms)), s_s3_download_throughput_per_connection_mbps); } break; } case AWS_S3_REQUEST_TYPE_PUT_OBJECT: { if (request->meta_request->is_express) { - tokens = aws_min_u32(ceil((request->buffer_size * 1000) / s_s3_express_p50_request_latency_ms), s_s3_express_upload_throughput_per_connection_mbps); + tokens = aws_min_u32(ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_express_p50_request_latency_ms)), s_s3_express_upload_throughput_per_connection_mbps); } else { - tokens = aws_min_u32(ceil((request->buffer_size * 1000) / s_s3_p50_request_latency_ms), s_s3_upload_throughput_per_connection_mbps); + tokens = aws_min_u32(ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_p50_request_latency_ms)), s_s3_upload_throughput_per_connection_mbps); } break; } @@ -291,17 +291,17 @@ bool aws_s3_client_acquire_tokens( switch (request->request_type) { case AWS_S3_REQUEST_TYPE_GET_OBJECT: { if (request->meta_request->is_express) { - required_tokens = aws_min_u32(ceil((request->buffer_size * 1000) / s_s3_express_p50_request_latency_ms), s_s3_express_download_throughput_per_connection_mbps); + required_tokens = aws_min_u32(ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_express_p50_request_latency_ms)), s_s3_express_download_throughput_per_connection_mbps); } else { - required_tokens = aws_min_u32(ceil((request->buffer_size * 1000) / s_s3_p50_request_latency_ms), s_s3_download_throughput_per_connection_mbps); + required_tokens = aws_min_u32(ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_p50_request_latency_ms)), s_s3_download_throughput_per_connection_mbps); } break; } case AWS_S3_REQUEST_TYPE_PUT_OBJECT: { if (request->meta_request->is_express) { - required_tokens = aws_min_u32(ceil((request->buffer_size * 1000) / s_s3_express_p50_request_latency_ms), s_s3_express_upload_throughput_per_connection_mbps); + required_tokens = aws_min_u32(ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_express_p50_request_latency_ms)), s_s3_express_upload_throughput_per_connection_mbps); } else { - required_tokens = aws_min_u32(ceil((request->buffer_size * 1000) / s_s3_p50_request_latency_ms), s_s3_upload_throughput_per_connection_mbps); + required_tokens = aws_min_u32(ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_p50_request_latency_ms)), s_s3_upload_throughput_per_connection_mbps); } break; } From 5b6a70ae6768559061497755fb31c533c49f9afd Mon Sep 17 00:00:00 2001 From: Krish <> Date: Mon, 24 Nov 2025 11:09:23 -0800 Subject: [PATCH 03/21] acquire and release tokens --- source/s3_client.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/source/s3_client.c b/source/s3_client.c index 06c478d5f..8b6e89064 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -237,14 +237,14 @@ uint32_t aws_s3_client_get_max_active_connections( } /* Initialize token bucket based on target throughput */ -void aws_s3_client_init_tokens(struct aws_s3_client *client, double target_throughput_gbps) { +void s_s3_client_init_tokens(struct aws_s3_client *client, double target_throughput_gbps) { AWS_PRECONDITION(client); if (target_throughput_gbps == 0.0) target_throughput_gbps = 150.0; aws_atomic_store_int(&client->token_bucket, aws_max_u32(target_throughput_gbps * 1024, s_s3_minimum_tokens)); } /* Releases tokens back after request is complete. */ -void aws_s3_client_release_tokens( +void s_s3_client_release_tokens( struct aws_s3_client *client, struct aws_s3_request *request) { AWS_PRECONDITION(client); @@ -280,7 +280,7 @@ void aws_s3_client_release_tokens( /* Returns true or false based on whether the request was able to avail the required amount of tokens. * TODO: try to introduce a scalability factor instead of using pure latency. */ -bool aws_s3_client_acquire_tokens( +bool s_s3_client_acquire_tokens( struct aws_s3_client *client, struct aws_s3_request *request) { AWS_PRECONDITION(client); @@ -533,6 +533,8 @@ struct aws_s3_client *aws_s3_client_new( *(uint32_t *)&client->ideal_connection_count = aws_max_u32( g_min_num_connections, s_get_ideal_connection_number_from_throughput(client->throughput_target_gbps)); + s_s3_client_init_tokens(client, client->throughput_target_gbps); + size_t part_size = (size_t)g_default_part_size_fallback; if (client_config->part_size != 0) { if (client_config->part_size > SIZE_MAX) { @@ -2398,7 +2400,7 @@ void aws_s3_client_update_connections_threaded(struct aws_s3_client *client) { s_s3_client_meta_request_finished_request(client, meta_request, request, AWS_ERROR_S3_CANCELED); request = aws_s3_request_release(request); - } else if (aws_s3_avail_tokens(client, request)) { + } else if (s_s3_client_acquire_tokens(client, request)) { /* Make sure it's above the max request level limitation. */ s_s3_client_create_connection_for_request(client, request); } else { @@ -2724,6 +2726,9 @@ void aws_s3_client_notify_connection_finished( request->send_data.metrics->time_metrics.s3_request_last_attempt_end_timestamp_ns - request->send_data.metrics->time_metrics.s3_request_first_attempt_start_timestamp_ns; + // release tokens acquired for the request + s_s3_client_release_tokens(client, request); + if (connection->retry_token != NULL) { /* If we have a retry token and successfully finished, record that success. */ if (finish_code == AWS_S3_CONNECTION_FINISH_CODE_SUCCESS) { From 6e25a4f97ffd2d0562d18343b029741528558acb Mon Sep 17 00:00:00 2001 From: Krish <> Date: Mon, 24 Nov 2025 11:15:25 -0800 Subject: [PATCH 04/21] lint --- include/aws/s3/private/s3_client_impl.h | 3 +- source/s3_client.c | 49 ++++++++++++++++--------- 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/include/aws/s3/private/s3_client_impl.h b/include/aws/s3/private/s3_client_impl.h index 110ad1056..0e4032ca5 100644 --- a/include/aws/s3/private/s3_client_impl.h +++ b/include/aws/s3/private/s3_client_impl.h @@ -275,7 +275,8 @@ struct aws_s3_client { /* The calculated ideal number of HTTP connections, based on throughput target and throughput per connection. */ const uint32_t ideal_connection_count; - /* Tokens added when meta requests are created and subtracted when requests use the tokens to allocate connections */ + /* Tokens added when meta requests are created and subtracted when requests use the tokens to allocate connections + */ struct struct_atomic_var token_bucket; /** diff --git a/source/s3_client.c b/source/s3_client.c index 8b6e89064..046eb5bbe 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -76,7 +76,7 @@ const uint32_t g_min_num_connections = 10; /* Magic value based on: 10 was old b const uint32_t g_max_num_connections = 10000; /* This is a first pass at a token based implementation, the calculations are approximate and can be improved in the - * future. The idea is to scale the number of connections we require up and down based on the different requests we + * future. The idea is to scale the number of connections we require up and down based on the different requests we * receive and hence dynamically scale the maximum number of connections we need to open. One token is equivalent to * 1Mbps of throughput. */ @@ -103,7 +103,7 @@ const uint32_t s_s3_client_minimum_concurrent_requests = 8; * However, this is subject to change due to newer part sizes and adjustments. */ const uint32_t s_ideal_part_size = 8 * 8; -const uint32_t s_s3_minimum_tokens = s_ideal_part_size * 8 * s_s3_client_minimum_concurrent_requests; +const uint32_t s_s3_minimum_tokens = s_ideal_part_size * 8 * s_s3_client_minimum_concurrent_requests; /** * Default max part size is 5GiB as the server limit. @@ -239,14 +239,13 @@ uint32_t aws_s3_client_get_max_active_connections( /* Initialize token bucket based on target throughput */ void s_s3_client_init_tokens(struct aws_s3_client *client, double target_throughput_gbps) { AWS_PRECONDITION(client); - if (target_throughput_gbps == 0.0) target_throughput_gbps = 150.0; + if (target_throughput_gbps == 0.0) + target_throughput_gbps = 150.0; aws_atomic_store_int(&client->token_bucket, aws_max_u32(target_throughput_gbps * 1024, s_s3_minimum_tokens)); } /* Releases tokens back after request is complete. */ -void s_s3_client_release_tokens( - struct aws_s3_client *client, - struct aws_s3_request *request) { +void s_s3_client_release_tokens(struct aws_s3_client *client, struct aws_s3_request *request) { AWS_PRECONDITION(client); AWS_PRECONDITION(request); @@ -255,17 +254,25 @@ void s_s3_client_release_tokens( switch (request->request_type) { case AWS_S3_REQUEST_TYPE_GET_OBJECT: { if (request->meta_request->is_express) { - tokens = aws_min_u32(ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_express_p50_request_latency_ms)), s_s3_express_download_throughput_per_connection_mbps); + tokens = aws_min_u32( + ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_express_p50_request_latency_ms)), + s_s3_express_download_throughput_per_connection_mbps); } else { - tokens = aws_min_u32(ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_p50_request_latency_ms)), s_s3_download_throughput_per_connection_mbps); + tokens = aws_min_u32( + ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_p50_request_latency_ms)), + s_s3_download_throughput_per_connection_mbps); } break; } case AWS_S3_REQUEST_TYPE_PUT_OBJECT: { if (request->meta_request->is_express) { - tokens = aws_min_u32(ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_express_p50_request_latency_ms)), s_s3_express_upload_throughput_per_connection_mbps); + tokens = aws_min_u32( + ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_express_p50_request_latency_ms)), + s_s3_express_upload_throughput_per_connection_mbps); } else { - tokens = aws_min_u32(ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_p50_request_latency_ms)), s_s3_upload_throughput_per_connection_mbps); + tokens = aws_min_u32( + ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_p50_request_latency_ms)), + s_s3_upload_throughput_per_connection_mbps); } break; } @@ -280,9 +287,7 @@ void s_s3_client_release_tokens( /* Returns true or false based on whether the request was able to avail the required amount of tokens. * TODO: try to introduce a scalability factor instead of using pure latency. */ -bool s_s3_client_acquire_tokens( - struct aws_s3_client *client, - struct aws_s3_request *request) { +bool s_s3_client_acquire_tokens(struct aws_s3_client *client, struct aws_s3_request *request) { AWS_PRECONDITION(client); AWS_PRECONDITION(request); @@ -291,17 +296,25 @@ bool s_s3_client_acquire_tokens( switch (request->request_type) { case AWS_S3_REQUEST_TYPE_GET_OBJECT: { if (request->meta_request->is_express) { - required_tokens = aws_min_u32(ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_express_p50_request_latency_ms)), s_s3_express_download_throughput_per_connection_mbps); + required_tokens = aws_min_u32( + ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_express_p50_request_latency_ms)), + s_s3_express_download_throughput_per_connection_mbps); } else { - required_tokens = aws_min_u32(ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_p50_request_latency_ms)), s_s3_download_throughput_per_connection_mbps); + required_tokens = aws_min_u32( + ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_p50_request_latency_ms)), + s_s3_download_throughput_per_connection_mbps); } break; } case AWS_S3_REQUEST_TYPE_PUT_OBJECT: { if (request->meta_request->is_express) { - required_tokens = aws_min_u32(ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_express_p50_request_latency_ms)), s_s3_express_upload_throughput_per_connection_mbps); + required_tokens = aws_min_u32( + ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_express_p50_request_latency_ms)), + s_s3_express_upload_throughput_per_connection_mbps); } else { - required_tokens = aws_min_u32(ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_p50_request_latency_ms)), s_s3_upload_throughput_per_connection_mbps); + required_tokens = aws_min_u32( + ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_p50_request_latency_ms)), + s_s3_upload_throughput_per_connection_mbps); } break; } @@ -310,7 +323,7 @@ bool s_s3_client_acquire_tokens( } } - if ((uint32_t *) aws_atomic_load_int(&client->token_bucket) > required_tokens) { + if ((uint32_t *)aws_atomic_load_int(&client->token_bucket) > required_tokens) { // do we need error handling here? aws_atomic_fetch_sub(client->token_bucket, required_tokens); return true; From 258edb9d567e824cf518b7986030c421ecf61324 Mon Sep 17 00:00:00 2001 From: Krish <> Date: Mon, 24 Nov 2025 11:37:04 -0800 Subject: [PATCH 05/21] corrections --- include/aws/s3/private/s3_client_impl.h | 2 +- source/s3_client.c | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/include/aws/s3/private/s3_client_impl.h b/include/aws/s3/private/s3_client_impl.h index 0e4032ca5..141fd558c 100644 --- a/include/aws/s3/private/s3_client_impl.h +++ b/include/aws/s3/private/s3_client_impl.h @@ -277,7 +277,7 @@ struct aws_s3_client { /* Tokens added when meta requests are created and subtracted when requests use the tokens to allocate connections */ - struct struct_atomic_var token_bucket; + struct aws_atomic_var token_bucket; /** * For multi-part upload, content-md5 will be calculated if the AWS_MR_CONTENT_MD5_ENABLED is specified diff --git a/source/s3_client.c b/source/s3_client.c index 046eb5bbe..57a08f525 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -277,12 +277,12 @@ void s_s3_client_release_tokens(struct aws_s3_client *client, struct aws_s3_requ break; } default: { - tokens = g_default_min_tokens; + tokens = s_s3_minimum_tokens; } } // do we need error handling here? - aws_atomic_fetch_add(client->token_bucket, tokens); + aws_atomic_fetch_add(&client->token_bucket, tokens); } /* Returns true or false based on whether the request was able to avail the required amount of tokens. @@ -319,13 +319,13 @@ bool s_s3_client_acquire_tokens(struct aws_s3_client *client, struct aws_s3_requ break; } default: { - required_tokens = g_default_min_tokens; + required_tokens = s_s3_minimum_tokens; } } - if ((uint32_t *)aws_atomic_load_int(&client->token_bucket) > required_tokens) { + if ((uint32_t)aws_atomic_load_int(&client->token_bucket) > required_tokens) { // do we need error handling here? - aws_atomic_fetch_sub(client->token_bucket, required_tokens); + aws_atomic_fetch_sub(&client->token_bucket, required_tokens); return true; } return false; @@ -2393,7 +2393,7 @@ void aws_s3_client_update_connections_threaded(struct aws_s3_client *client) { struct aws_s3_request *request = aws_s3_client_dequeue_request_threaded(client); struct aws_s3_meta_request *meta_request = request->meta_request; - const uint32_t max_active_connections = aws_s3_client_get_max_active_connections(client, meta_request); + if (request->is_noop) { /* If request is no-op, finishes and cleans up the request */ s_s3_client_meta_request_finished_request(client, meta_request, request, AWS_ERROR_SUCCESS); From 73f3acb970e1363e90c175b4460acaf35464d372 Mon Sep 17 00:00:00 2001 From: Krish <> Date: Mon, 24 Nov 2025 15:12:42 -0800 Subject: [PATCH 06/21] fix math --- source/s3_client.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/source/s3_client.c b/source/s3_client.c index 57a08f525..72f828876 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -93,9 +93,9 @@ const uint32_t s_s3_express_upload_throughput_per_connection_mbps = 100 * 8; /* All latency values are in milliseconds (ms) and provided by S3 team */ // 30ms -const uint32_t s_s3_p50_request_latency_ms = 30; +const double s_s3_p50_request_latency_ms = 0.03; // 4ms -const uint32_t s_s3_express_p50_request_latency_ms = 4; +const double s_s3_express_p50_request_latency_ms = 0.004; const uint32_t s_s3_client_minimum_concurrent_requests = 8; @@ -239,8 +239,6 @@ uint32_t aws_s3_client_get_max_active_connections( /* Initialize token bucket based on target throughput */ void s_s3_client_init_tokens(struct aws_s3_client *client, double target_throughput_gbps) { AWS_PRECONDITION(client); - if (target_throughput_gbps == 0.0) - target_throughput_gbps = 150.0; aws_atomic_store_int(&client->token_bucket, aws_max_u32(target_throughput_gbps * 1024, s_s3_minimum_tokens)); } @@ -255,11 +253,11 @@ void s_s3_client_release_tokens(struct aws_s3_client *client, struct aws_s3_requ case AWS_S3_REQUEST_TYPE_GET_OBJECT: { if (request->meta_request->is_express) { tokens = aws_min_u32( - ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_express_p50_request_latency_ms)), + ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_express_p50_request_latency_ms)), s_s3_express_download_throughput_per_connection_mbps); } else { tokens = aws_min_u32( - ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_p50_request_latency_ms)), + ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_p50_request_latency_ms)), s_s3_download_throughput_per_connection_mbps); } break; @@ -267,11 +265,11 @@ void s_s3_client_release_tokens(struct aws_s3_client *client, struct aws_s3_requ case AWS_S3_REQUEST_TYPE_PUT_OBJECT: { if (request->meta_request->is_express) { tokens = aws_min_u32( - ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_express_p50_request_latency_ms)), + ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_express_p50_request_latency_ms)), s_s3_express_upload_throughput_per_connection_mbps); } else { tokens = aws_min_u32( - ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_p50_request_latency_ms)), + ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_p50_request_latency_ms)), s_s3_upload_throughput_per_connection_mbps); } break; @@ -297,11 +295,11 @@ bool s_s3_client_acquire_tokens(struct aws_s3_client *client, struct aws_s3_requ case AWS_S3_REQUEST_TYPE_GET_OBJECT: { if (request->meta_request->is_express) { required_tokens = aws_min_u32( - ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_express_p50_request_latency_ms)), + ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_express_p50_request_latency_ms)), s_s3_express_download_throughput_per_connection_mbps); } else { required_tokens = aws_min_u32( - ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_p50_request_latency_ms)), + ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_p50_request_latency_ms)), s_s3_download_throughput_per_connection_mbps); } break; @@ -309,11 +307,11 @@ bool s_s3_client_acquire_tokens(struct aws_s3_client *client, struct aws_s3_requ case AWS_S3_REQUEST_TYPE_PUT_OBJECT: { if (request->meta_request->is_express) { required_tokens = aws_min_u32( - ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_express_p50_request_latency_ms)), + ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_express_p50_request_latency_ms)), s_s3_express_upload_throughput_per_connection_mbps); } else { required_tokens = aws_min_u32( - ceil((request->buffer_size * 1000) / (MB_TO_BYTES(8) * s_s3_p50_request_latency_ms)), + ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_p50_request_latency_ms)), s_s3_upload_throughput_per_connection_mbps); } break; From 37a0ff6471e2835ce796e143c99730d13c2c2db4 Mon Sep 17 00:00:00 2001 From: Krish <> Date: Mon, 24 Nov 2025 16:19:12 -0800 Subject: [PATCH 07/21] more corrections --- source/s3_client.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/source/s3_client.c b/source/s3_client.c index 72f828876..e30d4365b 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -101,9 +101,11 @@ const uint32_t s_s3_client_minimum_concurrent_requests = 8; /* Currently the ideal part size is 8MB and hence the value set. * However, this is subject to change due to newer part sizes and adjustments. */ -const uint32_t s_ideal_part_size = 8 * 8; +const uint32_t s_ideal_part_size = 8; -const uint32_t s_s3_minimum_tokens = s_ideal_part_size * 8 * s_s3_client_minimum_concurrent_requests; +const uint32_t s_s3_minimum_tokens = + s_ideal_part_size * 8 * + s_s3_client_minimum_concurrent_requests; /* x 8 to convert to megabits and match token unit */ /** * Default max part size is 5GiB as the server limit. @@ -262,7 +264,7 @@ void s_s3_client_release_tokens(struct aws_s3_client *client, struct aws_s3_requ } break; } - case AWS_S3_REQUEST_TYPE_PUT_OBJECT: { + case AWS_S3_REQUEST_TYPE_UPLOAD_PART: { if (request->meta_request->is_express) { tokens = aws_min_u32( ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_express_p50_request_latency_ms)), @@ -304,7 +306,7 @@ bool s_s3_client_acquire_tokens(struct aws_s3_client *client, struct aws_s3_requ } break; } - case AWS_S3_REQUEST_TYPE_PUT_OBJECT: { + case AWS_S3_REQUEST_TYPE_UPLOAD_PART: { if (request->meta_request->is_express) { required_tokens = aws_min_u32( ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_express_p50_request_latency_ms)), From 3185861d2d529519be7df38a30a1fc61e69dbce1 Mon Sep 17 00:00:00 2001 From: Krish <> Date: Tue, 25 Nov 2025 14:39:57 -0800 Subject: [PATCH 08/21] test fixes --- include/aws/s3/private/s3_client_impl.h | 5 +++-- source/s3_client.c | 17 +++++++++++++++++ tests/s3_max_active_connections_override_test.c | 2 +- tests/s3_tester.c | 3 +++ 4 files changed, 24 insertions(+), 3 deletions(-) diff --git a/include/aws/s3/private/s3_client_impl.h b/include/aws/s3/private/s3_client_impl.h index 141fd558c..282a47c7e 100644 --- a/include/aws/s3/private/s3_client_impl.h +++ b/include/aws/s3/private/s3_client_impl.h @@ -289,8 +289,6 @@ struct aws_s3_client { /* Hard limit on max connections set through the client config. */ const uint32_t max_active_connections_override; - struct aws_atomic_var max_allowed_connections; - /* Retry strategy used for scheduling request retries. */ struct aws_retry_strategy *retry_strategy; @@ -367,6 +365,9 @@ struct aws_s3_client { /* Number of requests being sent/received over network. */ struct aws_atomic_var num_requests_network_io[AWS_S3_META_REQUEST_TYPE_MAX]; + /* Total number of requests on the network over all the meta requests. */ + struct aws_atomic_var num_requests_network_total; + /* Number of requests sitting in their meta request priority queue, waiting to be streamed. */ struct aws_atomic_var num_requests_stream_queued_waiting; diff --git a/source/s3_client.c b/source/s3_client.c index e30d4365b..2b7880f50 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -291,6 +291,21 @@ bool s_s3_client_acquire_tokens(struct aws_s3_client *client, struct aws_s3_requ AWS_PRECONDITION(client); AWS_PRECONDITION(request); + // We ensure we do not violate the user set max-connections limit + if ((uint32_t)aws_atomic_load_int(&client->stats.num_requests_network_total) >= + client->max_active_connections_override && + client->max_active_connections_override > 0) { + return false; + } + + struct aws_s3_meta_request *meta_request = request->meta_request; + if (meta_request && + (uint32_t)aws_atomic_load_int(&meta_request->num_requests_network) >= + meta_request->max_active_connections_override && + meta_request->max_active_connections_override > 0) { + return false; + } + uint32_t required_tokens = 0; switch (request->request_type) { @@ -2463,6 +2478,7 @@ static void s_s3_client_create_connection_for_request_default( aws_atomic_fetch_add(&meta_request->num_requests_network, 1); aws_atomic_fetch_add(&client->stats.num_requests_network_io[meta_request->type], 1); + aws_atomic_fetch_add(&client->stats.num_requests_network_total, 1); struct aws_s3_connection *connection = aws_mem_calloc(client->allocator, 1, sizeof(struct aws_s3_connection)); @@ -2761,6 +2777,7 @@ void aws_s3_client_notify_connection_finished( } aws_atomic_fetch_sub(&meta_request->num_requests_network, 1); aws_atomic_fetch_sub(&client->stats.num_requests_network_io[meta_request->type], 1); + aws_atomic_fetch_sub(&client->stats.num_requests_network_total, 1); s_s3_client_meta_request_finished_request(client, meta_request, request, error_code); diff --git a/tests/s3_max_active_connections_override_test.c b/tests/s3_max_active_connections_override_test.c index 2c09a47e2..3ef893a34 100644 --- a/tests/s3_max_active_connections_override_test.c +++ b/tests/s3_max_active_connections_override_test.c @@ -239,7 +239,7 @@ TEST_CASE(s3_max_active_connections_override_enforced) { * TODO: this test seems a bit flaky. Sometime the peak we collect is like one more than expected. Maybe some race * conditions that release and acquire happening. Check it against either the expected or expected + 1 for now. */ - ASSERT_TRUE(peak == options.max_active_connections_override || peak == options.max_active_connections_override + 1); + ASSERT_TRUE(peak <= options.max_active_connections_override + 1); aws_input_stream_destroy(input_stream); aws_string_destroy(host_name); diff --git a/tests/s3_tester.c b/tests/s3_tester.c index 7fec8f7ec..13a9784ba 100644 --- a/tests/s3_tester.c +++ b/tests/s3_tester.c @@ -895,6 +895,9 @@ struct aws_s3_client *aws_s3_tester_mock_client_new(struct aws_s3_tester *tester aws_atomic_init_int(&mock_client->stats.num_requests_stream_queued_waiting, 0); aws_atomic_init_int(&mock_client->stats.num_requests_streaming_response, 0); + // create tokens for mock use + aws_atomic_init_int(&mock_client->token_bucket, 1000000); + return mock_client; } From 8799bc4b3ef79a67e84f8641a3174a7ee5605e55 Mon Sep 17 00:00:00 2001 From: Krish <> Date: Tue, 25 Nov 2025 14:57:03 -0800 Subject: [PATCH 09/21] fix build issues --- source/s3_client.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/source/s3_client.c b/source/s3_client.c index 2b7880f50..8b8a51e11 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -97,15 +97,14 @@ const double s_s3_p50_request_latency_ms = 0.03; // 4ms const double s_s3_express_p50_request_latency_ms = 0.004; -const uint32_t s_s3_client_minimum_concurrent_requests = 8; - /* Currently the ideal part size is 8MB and hence the value set. * However, this is subject to change due to newer part sizes and adjustments. */ -const uint32_t s_ideal_part_size = 8; +static const uint32_t s_ideal_part_size = 8; + +static const uint32_t s_s3_client_minimum_concurrent_requests = 8; -const uint32_t s_s3_minimum_tokens = - s_ideal_part_size * 8 * - s_s3_client_minimum_concurrent_requests; /* x 8 to convert to megabits and match token unit */ +/* x 8 to convert to megabits and match token unit */ +const uint32_t s_s3_minimum_tokens = s_ideal_part_size * 8 * s_s3_client_minimum_concurrent_requests; /** * Default max part size is 5GiB as the server limit. From 0d6368c55fa3f4ee1f47831240a109908378bd7e Mon Sep 17 00:00:00 2001 From: Krish <> Date: Tue, 25 Nov 2025 15:04:26 -0800 Subject: [PATCH 10/21] unconst the const --- source/s3_client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/s3_client.c b/source/s3_client.c index 8b8a51e11..8412ef2a8 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -104,7 +104,7 @@ static const uint32_t s_ideal_part_size = 8; static const uint32_t s_s3_client_minimum_concurrent_requests = 8; /* x 8 to convert to megabits and match token unit */ -const uint32_t s_s3_minimum_tokens = s_ideal_part_size * 8 * s_s3_client_minimum_concurrent_requests; +static uint32_t s_s3_minimum_tokens = s_ideal_part_size * 8 * s_s3_client_minimum_concurrent_requests; /** * Default max part size is 5GiB as the server limit. From 562da6642fd6f60b986b9f00f903818f87101ea6 Mon Sep 17 00:00:00 2001 From: Krish <> Date: Tue, 25 Nov 2025 15:08:15 -0800 Subject: [PATCH 11/21] dynamic the static --- source/s3_client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/s3_client.c b/source/s3_client.c index 8412ef2a8..f6ea96157 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -104,7 +104,7 @@ static const uint32_t s_ideal_part_size = 8; static const uint32_t s_s3_client_minimum_concurrent_requests = 8; /* x 8 to convert to megabits and match token unit */ -static uint32_t s_s3_minimum_tokens = s_ideal_part_size * 8 * s_s3_client_minimum_concurrent_requests; +uint32_t s_s3_minimum_tokens = s_ideal_part_size * 8 * s_s3_client_minimum_concurrent_requests; /** * Default max part size is 5GiB as the server limit. From 8e063c76acf99347f2cc6aa5a4f5183775d17bca Mon Sep 17 00:00:00 2001 From: Krish <> Date: Tue, 25 Nov 2025 15:13:41 -0800 Subject: [PATCH 12/21] more unstatic --- source/s3_client.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/s3_client.c b/source/s3_client.c index f6ea96157..90fdd9cdd 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -99,9 +99,9 @@ const double s_s3_express_p50_request_latency_ms = 0.004; /* Currently the ideal part size is 8MB and hence the value set. * However, this is subject to change due to newer part sizes and adjustments. */ -static const uint32_t s_ideal_part_size = 8; +const uint32_t s_ideal_part_size = 8; -static const uint32_t s_s3_client_minimum_concurrent_requests = 8; +const uint32_t s_s3_client_minimum_concurrent_requests = 8; /* x 8 to convert to megabits and match token unit */ uint32_t s_s3_minimum_tokens = s_ideal_part_size * 8 * s_s3_client_minimum_concurrent_requests; From d0c0f016131c29ff140f8276a53fdb6db24b34ab Mon Sep 17 00:00:00 2001 From: Krish <> Date: Tue, 25 Nov 2025 15:53:53 -0800 Subject: [PATCH 13/21] define instead --- source/s3_client.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/source/s3_client.c b/source/s3_client.c index 90fdd9cdd..a3ef431ef 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -44,6 +44,9 @@ #include #include +#define S_IDEAL_PART_SIZE 8 +#define S_S3_CLIENT_MINIMUM_CONCURRENT_REQUESTS 8 + #ifdef _MSC_VER # pragma warning(disable : 4232) /* function pointer to dll symbol */ #endif /* _MSC_VER */ @@ -99,12 +102,8 @@ const double s_s3_express_p50_request_latency_ms = 0.004; /* Currently the ideal part size is 8MB and hence the value set. * However, this is subject to change due to newer part sizes and adjustments. */ -const uint32_t s_ideal_part_size = 8; - -const uint32_t s_s3_client_minimum_concurrent_requests = 8; -/* x 8 to convert to megabits and match token unit */ -uint32_t s_s3_minimum_tokens = s_ideal_part_size * 8 * s_s3_client_minimum_concurrent_requests; +const uint32_t s_s3_minimum_tokens = S_IDEAL_PART_SIZE * 8 * S_S3_CLIENT_MINIMUM_CONCURRENT_REQUESTS; /** * Default max part size is 5GiB as the server limit. From cdbdcb80e7f2c04b762d49bd1be4e30befcfc043 Mon Sep 17 00:00:00 2001 From: Krish <> Date: Tue, 25 Nov 2025 16:15:06 -0800 Subject: [PATCH 14/21] typecast to pass windows --- source/s3_client.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/source/s3_client.c b/source/s3_client.c index a3ef431ef..4bab42b97 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -239,7 +239,8 @@ uint32_t aws_s3_client_get_max_active_connections( /* Initialize token bucket based on target throughput */ void s_s3_client_init_tokens(struct aws_s3_client *client, double target_throughput_gbps) { AWS_PRECONDITION(client); - aws_atomic_store_int(&client->token_bucket, aws_max_u32(target_throughput_gbps * 1024, s_s3_minimum_tokens)); + aws_atomic_store_int( + &client->token_bucket, aws_max_u32((uint32_t)target_throughput_gbps * 1024, s_s3_minimum_tokens)); } /* Releases tokens back after request is complete. */ @@ -253,11 +254,11 @@ void s_s3_client_release_tokens(struct aws_s3_client *client, struct aws_s3_requ case AWS_S3_REQUEST_TYPE_GET_OBJECT: { if (request->meta_request->is_express) { tokens = aws_min_u32( - ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_express_p50_request_latency_ms)), + (uint32_t)ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_express_p50_request_latency_ms)), s_s3_express_download_throughput_per_connection_mbps); } else { tokens = aws_min_u32( - ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_p50_request_latency_ms)), + (uint32_t)ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_p50_request_latency_ms)), s_s3_download_throughput_per_connection_mbps); } break; @@ -265,11 +266,11 @@ void s_s3_client_release_tokens(struct aws_s3_client *client, struct aws_s3_requ case AWS_S3_REQUEST_TYPE_UPLOAD_PART: { if (request->meta_request->is_express) { tokens = aws_min_u32( - ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_express_p50_request_latency_ms)), + (uint32_t)ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_express_p50_request_latency_ms)), s_s3_express_upload_throughput_per_connection_mbps); } else { tokens = aws_min_u32( - ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_p50_request_latency_ms)), + (uint32_t)ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_p50_request_latency_ms)), s_s3_upload_throughput_per_connection_mbps); } break; @@ -310,11 +311,11 @@ bool s_s3_client_acquire_tokens(struct aws_s3_client *client, struct aws_s3_requ case AWS_S3_REQUEST_TYPE_GET_OBJECT: { if (request->meta_request->is_express) { required_tokens = aws_min_u32( - ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_express_p50_request_latency_ms)), + (uint32_t)ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_express_p50_request_latency_ms)), s_s3_express_download_throughput_per_connection_mbps); } else { required_tokens = aws_min_u32( - ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_p50_request_latency_ms)), + (uint32_t)ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_p50_request_latency_ms)), s_s3_download_throughput_per_connection_mbps); } break; @@ -322,11 +323,11 @@ bool s_s3_client_acquire_tokens(struct aws_s3_client *client, struct aws_s3_requ case AWS_S3_REQUEST_TYPE_UPLOAD_PART: { if (request->meta_request->is_express) { required_tokens = aws_min_u32( - ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_express_p50_request_latency_ms)), + (uint32_t)ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_express_p50_request_latency_ms)), s_s3_express_upload_throughput_per_connection_mbps); } else { required_tokens = aws_min_u32( - ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_p50_request_latency_ms)), + (uint32_t)ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_p50_request_latency_ms)), s_s3_upload_throughput_per_connection_mbps); } break; From a30d41706a9d5e09f8edda05c14262c240450a35 Mon Sep 17 00:00:00 2001 From: Krish <> Date: Wed, 26 Nov 2025 15:39:12 -0800 Subject: [PATCH 15/21] addressing comments 1 --- include/aws/s3/private/s3_request.h | 4 + source/s3_client.c | 116 +++++++++++++++------------- 2 files changed, 66 insertions(+), 54 deletions(-) diff --git a/include/aws/s3/private/s3_request.h b/include/aws/s3/private/s3_request.h index af8b85655..09dbefc1e 100644 --- a/include/aws/s3/private/s3_request.h +++ b/include/aws/s3/private/s3_request.h @@ -255,6 +255,10 @@ struct aws_s3_request { /* The upload_timeout used. Zero, if the request is not a upload part */ size_t upload_timeout_ms; + /* The number of tokens used to send the request. Initialized to zero until connection needs to be created. In case + * of failure, we also set it back to zero until the retry requires a connection again. */ + uint32_t tokens_used; + /* Number of times aws_s3_meta_request_prepare has been called for a request. During the first call to the virtual * prepare function, this will be 0.*/ uint32_t num_times_prepared; diff --git a/source/s3_client.c b/source/s3_client.c index 4bab42b97..0f12da7ad 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -44,9 +44,6 @@ #include #include -#define S_IDEAL_PART_SIZE 8 -#define S_S3_CLIENT_MINIMUM_CONCURRENT_REQUESTS 8 - #ifdef _MSC_VER # pragma warning(disable : 4232) /* function pointer to dll symbol */ #endif /* _MSC_VER */ @@ -78,10 +75,12 @@ const uint32_t g_min_num_connections = 10; /* Magic value based on: 10 was old b * be 2500 Gbps. */ const uint32_t g_max_num_connections = 10000; -/* This is a first pass at a token based implementation, the calculations are approximate and can be improved in the - * future. The idea is to scale the number of connections we require up and down based on the different requests we - * receive and hence dynamically scale the maximum number of connections we need to open. One token is equivalent to - * 1Mbps of throughput. */ +/* + * This is a token based implementation dor dynamic scaling of connections. + * The calculations are approximate and can be improved in the future. The idea is to scale the number of connections + * we require up and down based on the different requests we receive and hence dynamically scale the maximum number + * of connections we need to open. One token is equivalent to 1Mbps of throughput. + */ /* All throughput values are in MBps and provided by S3 team */ @@ -100,10 +99,12 @@ const double s_s3_p50_request_latency_ms = 0.03; // 4ms const double s_s3_express_p50_request_latency_ms = 0.004; -/* Currently the ideal part size is 8MB and hence the value set. - * However, this is subject to change due to newer part sizes and adjustments. */ - -const uint32_t s_s3_minimum_tokens = S_IDEAL_PART_SIZE * 8 * S_S3_CLIENT_MINIMUM_CONCURRENT_REQUESTS; +/* + * Represents the minimum number of tokens a particular request might use irrespective of payload size or throughput + * achieved. This is required to hard limit the number of connections we open for extremely small request sizes. The + * number below is arbitrary until we come up with more sophisticated math. + */ +const uint32_t s_s3_minimum_tokens = 500; /** * Default max part size is 5GiB as the server limit. @@ -186,8 +187,9 @@ void aws_s3_set_dns_ttl(size_t ttl) { /** * Determine how many connections are ideal by dividing target-throughput by throughput-per-connection. - * TODO: we may consider to alter this, upload to regular s3 can use more connections, and s3 express - * can use less connections to reach the target throughput.. + * TODO: we have begun altering the calculation behind this. get_ideal_connection_number no longer provides + * the basis for the number of connections we use for a particular meta request. It will soon be removed + * as part of future work towards moving to a dynamic connection allocation implementation. **/ static uint32_t s_get_ideal_connection_number_from_throughput(double throughput_gps) { double ideal_connection_count_double = throughput_gps / s_throughput_per_connection_gbps; @@ -237,10 +239,11 @@ uint32_t aws_s3_client_get_max_active_connections( } /* Initialize token bucket based on target throughput */ -void s_s3_client_init_tokens(struct aws_s3_client *client, double target_throughput_gbps) { +void s_s3_client_init_tokens(struct aws_s3_client *client) { AWS_PRECONDITION(client); + aws_atomic_store_int( - &client->token_bucket, aws_max_u32((uint32_t)target_throughput_gbps * 1024, s_s3_minimum_tokens)); + &client->token_bucket, aws_max_u32((uint32_t)client->throughput_target_gbps * 1024, s_s3_minimum_tokens)); } /* Releases tokens back after request is complete. */ @@ -248,45 +251,13 @@ void s_s3_client_release_tokens(struct aws_s3_client *client, struct aws_s3_requ AWS_PRECONDITION(client); AWS_PRECONDITION(request); - uint32_t tokens = 0; - - switch (request->request_type) { - case AWS_S3_REQUEST_TYPE_GET_OBJECT: { - if (request->meta_request->is_express) { - tokens = aws_min_u32( - (uint32_t)ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_express_p50_request_latency_ms)), - s_s3_express_download_throughput_per_connection_mbps); - } else { - tokens = aws_min_u32( - (uint32_t)ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_p50_request_latency_ms)), - s_s3_download_throughput_per_connection_mbps); - } - break; - } - case AWS_S3_REQUEST_TYPE_UPLOAD_PART: { - if (request->meta_request->is_express) { - tokens = aws_min_u32( - (uint32_t)ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_express_p50_request_latency_ms)), - s_s3_express_upload_throughput_per_connection_mbps); - } else { - tokens = aws_min_u32( - (uint32_t)ceil(request->buffer_size * 8 / (MB_TO_BYTES(1) * s_s3_p50_request_latency_ms)), - s_s3_upload_throughput_per_connection_mbps); - } - break; - } - default: { - tokens = s_s3_minimum_tokens; - } - } - - // do we need error handling here? - aws_atomic_fetch_add(&client->token_bucket, tokens); + aws_atomic_fetch_add(&client->token_bucket, request->tokens_used); + request->tokens_used = 0; } -/* Returns true or false based on whether the request was able to avail the required amount of tokens. - * TODO: try to introduce a scalability factor instead of using pure latency. */ -bool s_s3_client_acquire_tokens(struct aws_s3_client *client, struct aws_s3_request *request) { +/* Checks to ensure we are not violating user configured connection limits althought we are dynamically increasing + * and decreasing connections */ +bool s_check_connection_limits(struct aws_s3_client *client, struct aws_s3_request *request) { AWS_PRECONDITION(client); AWS_PRECONDITION(request); @@ -305,8 +276,33 @@ bool s_s3_client_acquire_tokens(struct aws_s3_client *client, struct aws_s3_requ return false; } + return true; +} + +/* Returns true or false based on whether the request was able to avail the required amount of tokens. + * TODO: try to introduce a scalability factor instead of using pure latency. */ +bool s_s3_client_acquire_tokens(struct aws_s3_client *client, struct aws_s3_request *request) { + AWS_PRECONDITION(client); + AWS_PRECONDITION(request); + uint32_t required_tokens = 0; + /* + * In each of the following cases, we determine the number of tokens required using the following formula, (One + * token is equivalent to attaining 1Mbps of target throughput):- + * + * For each operation (upload/download) and each service (s3/s3express), we have a hardcoded attainable throughput + * per connection value obtained from S3. also the latency involved in the respective services. + * + * The attained throughput per request is atmost the attainable throughput of the respective service-operation or + * when the payload size is small enough the delivery time is neglegible and hence close to payload/latency. + * + * The tokens used is basically the minimum of the two since larger objects end up getting closer to the attainable + * throughput and the smaller object max out at a theoretical limit of what it can attain. + * + * This calculation is an approximation of reality and might continuously improve based on S3 performance. + */ + switch (request->request_type) { case AWS_S3_REQUEST_TYPE_GET_OBJECT: { if (request->meta_request->is_express) { @@ -340,6 +336,7 @@ bool s_s3_client_acquire_tokens(struct aws_s3_client *client, struct aws_s3_requ if ((uint32_t)aws_atomic_load_int(&client->token_bucket) > required_tokens) { // do we need error handling here? aws_atomic_fetch_sub(&client->token_bucket, required_tokens); + request->tokens_used = required_tokens; return true; } return false; @@ -560,7 +557,7 @@ struct aws_s3_client *aws_s3_client_new( *(uint32_t *)&client->ideal_connection_count = aws_max_u32( g_min_num_connections, s_get_ideal_connection_number_from_throughput(client->throughput_target_gbps)); - s_s3_client_init_tokens(client, client->throughput_target_gbps); + s_s3_client_init_tokens(client); size_t part_size = (size_t)g_default_part_size_fallback; if (client_config->part_size != 0) { @@ -1967,12 +1964,20 @@ static void s_s3_client_process_work_default(struct aws_s3_client *client) { uint32_t total_approx_requests = num_requests_network_io + num_requests_stream_queued_waiting + num_requests_streaming_response + num_requests_being_prepared + client->threaded_data.request_queue_size; + + uint32_t total_tokens = client->throughput_target_gbps * 1024; + + uint32_t available_tokens = (uint32_t)aws_atomic_load_int(&client->token_bucket); + + uint32_t used_tokens = total_tokens - available_tokens; + AWS_LOGF( s_log_level_client_stats, AWS_LS_S3_CLIENT_STATS, "id=%p Requests-in-flight(approx/exact):%d/%d Requests-preparing:%d Requests-queued:%d " "Requests-network(get/put/default/total):%d/%d/%d/%d Requests-streaming-waiting:%d " "Requests-streaming-response:%d " + "Total Tokens: %d, Tokens Available: %d, Tokens Used: %d" " Endpoints(in-table/allocated):%d/%d", (void *)client, total_approx_requests, @@ -1985,6 +1990,9 @@ static void s_s3_client_process_work_default(struct aws_s3_client *client) { num_requests_network_io, num_requests_stream_queued_waiting, num_requests_streaming_response, + total_tokens, + available_tokens, + used_tokens, num_endpoints_in_table, num_endpoints_allocated); } @@ -2427,7 +2435,7 @@ void aws_s3_client_update_connections_threaded(struct aws_s3_client *client) { s_s3_client_meta_request_finished_request(client, meta_request, request, AWS_ERROR_S3_CANCELED); request = aws_s3_request_release(request); - } else if (s_s3_client_acquire_tokens(client, request)) { + } else if (s_check_connection_limits(client, request) && s_s3_client_acquire_tokens(client, request)) { /* Make sure it's above the max request level limitation. */ s_s3_client_create_connection_for_request(client, request); } else { From 714d235057f709966aae0c99173100130b5287fe Mon Sep 17 00:00:00 2001 From: Krish <> Date: Wed, 26 Nov 2025 18:19:14 -0800 Subject: [PATCH 16/21] add uint32_t --- source/s3_client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/s3_client.c b/source/s3_client.c index 0f12da7ad..bccafa13a 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -1965,7 +1965,7 @@ static void s_s3_client_process_work_default(struct aws_s3_client *client) { num_requests_streaming_response + num_requests_being_prepared + client->threaded_data.request_queue_size; - uint32_t total_tokens = client->throughput_target_gbps * 1024; + uint32_t total_tokens = (uint32_t)client->throughput_target_gbps * 1024; uint32_t available_tokens = (uint32_t)aws_atomic_load_int(&client->token_bucket); From 64025f106df1af0205a9893091e33dedf2cc9271 Mon Sep 17 00:00:00 2001 From: Krish <> Date: Fri, 28 Nov 2025 11:12:24 -0800 Subject: [PATCH 17/21] add default tokens separate from minimum --- source/s3_client.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/source/s3_client.c b/source/s3_client.c index bccafa13a..0a997c94a 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -104,8 +104,13 @@ const double s_s3_express_p50_request_latency_ms = 0.004; * achieved. This is required to hard limit the number of connections we open for extremely small request sizes. The * number below is arbitrary until we come up with more sophisticated math. */ -const uint32_t s_s3_minimum_tokens = 500; +const uint32_t s_s3_minimum_tokens = 10; +/* + * Represents a rough estimate of the tokens used by a request we do not have the idea of type or payload for. + * This is hard to set and hence is an approximation. + */ +const uint32_t s_s3_default_tokens = 500; /** * Default max part size is 5GiB as the server limit. */ @@ -242,8 +247,7 @@ uint32_t aws_s3_client_get_max_active_connections( void s_s3_client_init_tokens(struct aws_s3_client *client) { AWS_PRECONDITION(client); - aws_atomic_store_int( - &client->token_bucket, aws_max_u32((uint32_t)client->throughput_target_gbps * 1024, s_s3_minimum_tokens)); + aws_atomic_store_int(&client->token_bucket, (uint32_t)client->throughput_target_gbps * 1024); } /* Releases tokens back after request is complete. */ @@ -329,10 +333,13 @@ bool s_s3_client_acquire_tokens(struct aws_s3_client *client, struct aws_s3_requ break; } default: { - required_tokens = s_s3_minimum_tokens; + required_tokens = s_s3_default_tokens; } } + // Ensure we are using atleast minimum number of tokens irrespective of payload size. + required_tokens = aws_max_u32(required_tokens, s_s3_minimum_tokens); + if ((uint32_t)aws_atomic_load_int(&client->token_bucket) > required_tokens) { // do we need error handling here? aws_atomic_fetch_sub(&client->token_bucket, required_tokens); From 640088c0f6dfcbd9e11c721a95703333f89ec9a3 Mon Sep 17 00:00:00 2001 From: Krish <> Date: Mon, 1 Dec 2025 12:22:02 -0800 Subject: [PATCH 18/21] address comments --- source/s3_client.c | 41 +++++++++++------------------------------ 1 file changed, 11 insertions(+), 30 deletions(-) diff --git a/source/s3_client.c b/source/s3_client.c index 0a997c94a..3419b4ee6 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -217,7 +217,7 @@ uint32_t aws_s3_client_get_max_active_connections( struct aws_s3_meta_request *meta_request) { AWS_PRECONDITION(client); - uint32_t max_active_connections = client->ideal_connection_count; + uint32_t max_active_connections = g_max_num_connections; if (client->max_active_connections_override > 0 && client->max_active_connections_override < max_active_connections) { max_active_connections = client->max_active_connections_override; @@ -244,14 +244,16 @@ uint32_t aws_s3_client_get_max_active_connections( } /* Initialize token bucket based on target throughput */ -void s_s3_client_init_tokens(struct aws_s3_client *client) { +static void s_s3_client_init_tokens(struct aws_s3_client *client) { AWS_PRECONDITION(client); - aws_atomic_store_int(&client->token_bucket, (uint32_t)client->throughput_target_gbps * 1024); + uint32_t target_throughput_mbps = 0; + aws_mul_u32_checked(client->throughput_target_gbps, 1024, &target_throughput_mbps); + aws_atomic_store_int(&client->token_bucket, target_throughput_mbps); } /* Releases tokens back after request is complete. */ -void s_s3_client_release_tokens(struct aws_s3_client *client, struct aws_s3_request *request) { +static void s_s3_client_release_tokens(struct aws_s3_client *client, struct aws_s3_request *request) { AWS_PRECONDITION(client); AWS_PRECONDITION(request); @@ -259,33 +261,9 @@ void s_s3_client_release_tokens(struct aws_s3_client *client, struct aws_s3_requ request->tokens_used = 0; } -/* Checks to ensure we are not violating user configured connection limits althought we are dynamically increasing - * and decreasing connections */ -bool s_check_connection_limits(struct aws_s3_client *client, struct aws_s3_request *request) { - AWS_PRECONDITION(client); - AWS_PRECONDITION(request); - - // We ensure we do not violate the user set max-connections limit - if ((uint32_t)aws_atomic_load_int(&client->stats.num_requests_network_total) >= - client->max_active_connections_override && - client->max_active_connections_override > 0) { - return false; - } - - struct aws_s3_meta_request *meta_request = request->meta_request; - if (meta_request && - (uint32_t)aws_atomic_load_int(&meta_request->num_requests_network) >= - meta_request->max_active_connections_override && - meta_request->max_active_connections_override > 0) { - return false; - } - - return true; -} - /* Returns true or false based on whether the request was able to avail the required amount of tokens. * TODO: try to introduce a scalability factor instead of using pure latency. */ -bool s_s3_client_acquire_tokens(struct aws_s3_client *client, struct aws_s3_request *request) { +static bool s_s3_client_acquire_tokens(struct aws_s3_client *client, struct aws_s3_request *request) { AWS_PRECONDITION(client); AWS_PRECONDITION(request); @@ -2442,7 +2420,10 @@ void aws_s3_client_update_connections_threaded(struct aws_s3_client *client) { s_s3_client_meta_request_finished_request(client, meta_request, request, AWS_ERROR_S3_CANCELED); request = aws_s3_request_release(request); - } else if (s_check_connection_limits(client, request) && s_s3_client_acquire_tokens(client, request)) { + } else if ( + (uint32_t)aws_atomic_load_int(&meta_request->num_requests_network) < + (uint32_t)aws_s3_client_get_max_active_connections(client, meta_request) && + s_s3_client_acquire_tokens(client, request)) { /* Make sure it's above the max request level limitation. */ s_s3_client_create_connection_for_request(client, request); } else { From e2a3f861d5505e8876513ea9c82d243c49d5e48a Mon Sep 17 00:00:00 2001 From: Krish <> Date: Mon, 1 Dec 2025 13:57:53 -0800 Subject: [PATCH 19/21] fix test to not use ideal_connection_count --- tests/s3_data_plane_tests.c | 34 ---------------------------------- 1 file changed, 34 deletions(-) diff --git a/tests/s3_data_plane_tests.c b/tests/s3_data_plane_tests.c index ade775cc1..46ff0209f 100644 --- a/tests/s3_data_plane_tests.c +++ b/tests/s3_data_plane_tests.c @@ -315,7 +315,6 @@ static int s_test_s3_client_get_max_active_connections(struct aws_allocator *all struct aws_s3_client *mock_client = aws_s3_tester_mock_client_new(&tester); *((uint32_t *)&mock_client->max_active_connections_override) = 0; - *((uint32_t *)&mock_client->ideal_connection_count) = 100; mock_client->client_bootstrap = &mock_client_bootstrap; mock_client->vtable->get_host_address_count = s_test_get_max_active_connections_host_address_count; @@ -330,25 +329,10 @@ static int s_test_s3_client_get_max_active_connections(struct aws_allocator *all s_test_max_active_connections_host_count = 2; - /* Behavior should not be affected by max_active_connections_override since it is 0, and should just be in relation - * to ideal-connection-count. */ - { - ASSERT_TRUE(aws_s3_client_get_max_active_connections(mock_client, NULL) == mock_client->ideal_connection_count); - - for (size_t i = 0; i < AWS_S3_META_REQUEST_TYPE_MAX; ++i) { - ASSERT_TRUE( - aws_s3_client_get_max_active_connections(mock_client, mock_meta_requests[i]) == - mock_client->ideal_connection_count); - } - } - /* Max active connections override should now cap the calculated amount of active connections. */ { *((uint32_t *)&mock_client->max_active_connections_override) = 3; - /* Assert that override is low enough to have effect */ - ASSERT_TRUE(mock_client->max_active_connections_override < mock_client->ideal_connection_count); - ASSERT_TRUE( aws_s3_client_get_max_active_connections(mock_client, NULL) == mock_client->max_active_connections_override); @@ -362,24 +346,6 @@ static int s_test_s3_client_get_max_active_connections(struct aws_allocator *all } } - /* Max active connections override should be ignored since the calculated amount of max connections is less. */ - { - *((uint32_t *)&mock_client->max_active_connections_override) = 100000; - - /* Assert that override is NOT low enough to have effect */ - ASSERT_TRUE(mock_client->max_active_connections_override > mock_client->ideal_connection_count); - - ASSERT_TRUE(aws_s3_client_get_max_active_connections(mock_client, NULL) == mock_client->ideal_connection_count); - - for (size_t i = 0; i < AWS_S3_META_REQUEST_TYPE_MAX; ++i) { - ASSERT_TRUE(mock_client->max_active_connections_override > mock_client->ideal_connection_count); - - ASSERT_TRUE( - aws_s3_client_get_max_active_connections(mock_client, mock_meta_requests[i]) == - mock_client->ideal_connection_count); - } - } - for (size_t i = 0; i < AWS_S3_META_REQUEST_TYPE_MAX; ++i) { mock_meta_requests[i] = aws_s3_meta_request_release(mock_meta_requests[i]); } From 417433dac3c52a4965e66ee512485dc6c8b0b391 Mon Sep 17 00:00:00 2001 From: Krish <> Date: Mon, 1 Dec 2025 17:22:32 -0800 Subject: [PATCH 20/21] undo some of the changes --- source/s3_client.c | 11 ++++++----- tests/s3_data_plane_tests.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/source/s3_client.c b/source/s3_client.c index 3419b4ee6..212a77c86 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -192,9 +192,10 @@ void aws_s3_set_dns_ttl(size_t ttl) { /** * Determine how many connections are ideal by dividing target-throughput by throughput-per-connection. - * TODO: we have begun altering the calculation behind this. get_ideal_connection_number no longer provides - * the basis for the number of connections we use for a particular meta request. It will soon be removed - * as part of future work towards moving to a dynamic connection allocation implementation. + * We have begun altering the calculation behind this. get_ideal_connection_number no longer provides + * the basis for the number of connections we use for a particular meta request. However, we may use lesser + * number of connections that the ideal_connection_count incase of endpoints which may have higher connection + * throughput allowing us to use the memory and connection more efficiently. **/ static uint32_t s_get_ideal_connection_number_from_throughput(double throughput_gps) { double ideal_connection_count_double = throughput_gps / s_throughput_per_connection_gbps; @@ -206,7 +207,7 @@ static uint32_t s_get_ideal_connection_number_from_throughput(double throughput_ /* Returns the max number of connections allowed. * - * When meta request is NULL, this will return the overall allowed number of connections based on the clinet + * When meta request is NULL, this will return the overall allowed number of connections based on the client * configurations. * * If meta_request is not NULL, this will return the number of connections allowed based on the meta request @@ -217,7 +218,7 @@ uint32_t aws_s3_client_get_max_active_connections( struct aws_s3_meta_request *meta_request) { AWS_PRECONDITION(client); - uint32_t max_active_connections = g_max_num_connections; + uint32_t max_active_connections = client->ideal_connection_count; if (client->max_active_connections_override > 0 && client->max_active_connections_override < max_active_connections) { max_active_connections = client->max_active_connections_override; diff --git a/tests/s3_data_plane_tests.c b/tests/s3_data_plane_tests.c index 46ff0209f..ade775cc1 100644 --- a/tests/s3_data_plane_tests.c +++ b/tests/s3_data_plane_tests.c @@ -315,6 +315,7 @@ static int s_test_s3_client_get_max_active_connections(struct aws_allocator *all struct aws_s3_client *mock_client = aws_s3_tester_mock_client_new(&tester); *((uint32_t *)&mock_client->max_active_connections_override) = 0; + *((uint32_t *)&mock_client->ideal_connection_count) = 100; mock_client->client_bootstrap = &mock_client_bootstrap; mock_client->vtable->get_host_address_count = s_test_get_max_active_connections_host_address_count; @@ -329,10 +330,25 @@ static int s_test_s3_client_get_max_active_connections(struct aws_allocator *all s_test_max_active_connections_host_count = 2; + /* Behavior should not be affected by max_active_connections_override since it is 0, and should just be in relation + * to ideal-connection-count. */ + { + ASSERT_TRUE(aws_s3_client_get_max_active_connections(mock_client, NULL) == mock_client->ideal_connection_count); + + for (size_t i = 0; i < AWS_S3_META_REQUEST_TYPE_MAX; ++i) { + ASSERT_TRUE( + aws_s3_client_get_max_active_connections(mock_client, mock_meta_requests[i]) == + mock_client->ideal_connection_count); + } + } + /* Max active connections override should now cap the calculated amount of active connections. */ { *((uint32_t *)&mock_client->max_active_connections_override) = 3; + /* Assert that override is low enough to have effect */ + ASSERT_TRUE(mock_client->max_active_connections_override < mock_client->ideal_connection_count); + ASSERT_TRUE( aws_s3_client_get_max_active_connections(mock_client, NULL) == mock_client->max_active_connections_override); @@ -346,6 +362,24 @@ static int s_test_s3_client_get_max_active_connections(struct aws_allocator *all } } + /* Max active connections override should be ignored since the calculated amount of max connections is less. */ + { + *((uint32_t *)&mock_client->max_active_connections_override) = 100000; + + /* Assert that override is NOT low enough to have effect */ + ASSERT_TRUE(mock_client->max_active_connections_override > mock_client->ideal_connection_count); + + ASSERT_TRUE(aws_s3_client_get_max_active_connections(mock_client, NULL) == mock_client->ideal_connection_count); + + for (size_t i = 0; i < AWS_S3_META_REQUEST_TYPE_MAX; ++i) { + ASSERT_TRUE(mock_client->max_active_connections_override > mock_client->ideal_connection_count); + + ASSERT_TRUE( + aws_s3_client_get_max_active_connections(mock_client, mock_meta_requests[i]) == + mock_client->ideal_connection_count); + } + } + for (size_t i = 0; i < AWS_S3_META_REQUEST_TYPE_MAX; ++i) { mock_meta_requests[i] = aws_s3_meta_request_release(mock_meta_requests[i]); } From 982ba7feae27b1c1b220cdef33e767351bcb7868 Mon Sep 17 00:00:00 2001 From: Krish <> Date: Tue, 2 Dec 2025 09:31:13 -0800 Subject: [PATCH 21/21] convert to uint32_t --- source/s3_client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/s3_client.c b/source/s3_client.c index 212a77c86..b26f8ef03 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -249,7 +249,7 @@ static void s_s3_client_init_tokens(struct aws_s3_client *client) { AWS_PRECONDITION(client); uint32_t target_throughput_mbps = 0; - aws_mul_u32_checked(client->throughput_target_gbps, 1024, &target_throughput_mbps); + aws_mul_u32_checked((uint32_t)client->throughput_target_gbps, 1024, &target_throughput_mbps); aws_atomic_store_int(&client->token_bucket, target_throughput_mbps); }