Skip to content

Commit 417433d

Browse files
author
Krish
committed
undo some of the changes
1 parent e2a3f86 commit 417433d

File tree

2 files changed

+40
-5
lines changed

2 files changed

+40
-5
lines changed

source/s3_client.c

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -192,9 +192,10 @@ void aws_s3_set_dns_ttl(size_t ttl) {
192192

193193
/**
194194
* Determine how many connections are ideal by dividing target-throughput by throughput-per-connection.
195-
* TODO: we have begun altering the calculation behind this. get_ideal_connection_number no longer provides
196-
* the basis for the number of connections we use for a particular meta request. It will soon be removed
197-
* as part of future work towards moving to a dynamic connection allocation implementation.
195+
* We have begun altering the calculation behind this. get_ideal_connection_number no longer provides
196+
* the basis for the number of connections we use for a particular meta request. However, we may use lesser
197+
* number of connections that the ideal_connection_count incase of endpoints which may have higher connection
198+
* throughput allowing us to use the memory and connection more efficiently.
198199
**/
199200
static uint32_t s_get_ideal_connection_number_from_throughput(double throughput_gps) {
200201
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_
206207

207208
/* Returns the max number of connections allowed.
208209
*
209-
* When meta request is NULL, this will return the overall allowed number of connections based on the clinet
210+
* When meta request is NULL, this will return the overall allowed number of connections based on the client
210211
* configurations.
211212
*
212213
* 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(
217218
struct aws_s3_meta_request *meta_request) {
218219
AWS_PRECONDITION(client);
219220

220-
uint32_t max_active_connections = g_max_num_connections;
221+
uint32_t max_active_connections = client->ideal_connection_count;
221222
if (client->max_active_connections_override > 0 &&
222223
client->max_active_connections_override < max_active_connections) {
223224
max_active_connections = client->max_active_connections_override;

tests/s3_data_plane_tests.c

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,7 @@ static int s_test_s3_client_get_max_active_connections(struct aws_allocator *all
315315

316316
struct aws_s3_client *mock_client = aws_s3_tester_mock_client_new(&tester);
317317
*((uint32_t *)&mock_client->max_active_connections_override) = 0;
318+
*((uint32_t *)&mock_client->ideal_connection_count) = 100;
318319
mock_client->client_bootstrap = &mock_client_bootstrap;
319320
mock_client->vtable->get_host_address_count = s_test_get_max_active_connections_host_address_count;
320321

@@ -329,10 +330,25 @@ static int s_test_s3_client_get_max_active_connections(struct aws_allocator *all
329330

330331
s_test_max_active_connections_host_count = 2;
331332

333+
/* Behavior should not be affected by max_active_connections_override since it is 0, and should just be in relation
334+
* to ideal-connection-count. */
335+
{
336+
ASSERT_TRUE(aws_s3_client_get_max_active_connections(mock_client, NULL) == mock_client->ideal_connection_count);
337+
338+
for (size_t i = 0; i < AWS_S3_META_REQUEST_TYPE_MAX; ++i) {
339+
ASSERT_TRUE(
340+
aws_s3_client_get_max_active_connections(mock_client, mock_meta_requests[i]) ==
341+
mock_client->ideal_connection_count);
342+
}
343+
}
344+
332345
/* Max active connections override should now cap the calculated amount of active connections. */
333346
{
334347
*((uint32_t *)&mock_client->max_active_connections_override) = 3;
335348

349+
/* Assert that override is low enough to have effect */
350+
ASSERT_TRUE(mock_client->max_active_connections_override < mock_client->ideal_connection_count);
351+
336352
ASSERT_TRUE(
337353
aws_s3_client_get_max_active_connections(mock_client, NULL) ==
338354
mock_client->max_active_connections_override);
@@ -346,6 +362,24 @@ static int s_test_s3_client_get_max_active_connections(struct aws_allocator *all
346362
}
347363
}
348364

365+
/* Max active connections override should be ignored since the calculated amount of max connections is less. */
366+
{
367+
*((uint32_t *)&mock_client->max_active_connections_override) = 100000;
368+
369+
/* Assert that override is NOT low enough to have effect */
370+
ASSERT_TRUE(mock_client->max_active_connections_override > mock_client->ideal_connection_count);
371+
372+
ASSERT_TRUE(aws_s3_client_get_max_active_connections(mock_client, NULL) == mock_client->ideal_connection_count);
373+
374+
for (size_t i = 0; i < AWS_S3_META_REQUEST_TYPE_MAX; ++i) {
375+
ASSERT_TRUE(mock_client->max_active_connections_override > mock_client->ideal_connection_count);
376+
377+
ASSERT_TRUE(
378+
aws_s3_client_get_max_active_connections(mock_client, mock_meta_requests[i]) ==
379+
mock_client->ideal_connection_count);
380+
}
381+
}
382+
349383
for (size_t i = 0; i < AWS_S3_META_REQUEST_TYPE_MAX; ++i) {
350384
mock_meta_requests[i] = aws_s3_meta_request_release(mock_meta_requests[i]);
351385
}

0 commit comments

Comments
 (0)