From ad4086ab9c22d3922cc5f078b845cd2119101ea8 Mon Sep 17 00:00:00 2001 From: Dengke Tang Date: Wed, 29 Nov 2023 14:25:18 -0800 Subject: [PATCH] comments address --- source/s3_client.c | 19 ++++++++++--------- source/s3express_credentials_provider.c | 3 +-- tests/CMakeLists.txt | 15 ++++++++------- tests/s3_data_plane_tests.c | 4 ++-- .../s3_mock_server_s3express_provider_test.c | 2 +- tests/s3_s3express_client_test.c | 18 ++++++++++-------- 6 files changed, 32 insertions(+), 29 deletions(-) diff --git a/source/s3_client.c b/source/s3_client.c index 360240322..549a1a2da 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -1606,9 +1606,9 @@ static void s_s3_client_prepare_callback_queue_request( int error_code, void *user_data); -static bool s_meta_request_create_more_requests( - struct aws_s3_meta_request *meta_request, +static bool s_s3_client_should_update_meta_request( struct aws_s3_client *client, + struct aws_s3_meta_request *meta_request, uint32_t num_requests_in_flight, const uint32_t max_requests_in_flight, const uint32_t max_requests_prepare) { @@ -1622,17 +1622,18 @@ static bool s_meta_request_create_more_requests( } /** - * If: - * * Number of being-prepared + already-prepared-and-queued requests is more than the max that can + * If number of being-prepared + already-prepared-and-queued requests is more than the max that can * be in the preparation stage. - * * Total number of requests tracked by the client is more than the max tracked ("in flight") + * Or total number of requests tracked by the client is more than the max tracked ("in flight") * requests. * * We cannot create more requests for this meta request. */ if ((client->threaded_data.num_requests_being_prepared + client->threaded_data.request_queue_size) >= - max_requests_prepare || - num_requests_in_flight >= max_requests_in_flight) { + max_requests_prepare) { + return false; + } + if (num_requests_in_flight >= max_requests_in_flight) { return false; } @@ -1686,8 +1687,8 @@ void aws_s3_client_update_meta_requests_threaded(struct aws_s3_client *client) { struct aws_s3_meta_request *meta_request = AWS_CONTAINER_OF(meta_request_node, struct aws_s3_meta_request, client_process_work_threaded_data); - if (!s_meta_request_create_more_requests( - meta_request, client, num_requests_in_flight, max_requests_in_flight, max_requests_prepare)) { + if (!s_s3_client_should_update_meta_request( + client, meta_request, num_requests_in_flight, max_requests_in_flight, max_requests_prepare)) { /* Move the meta request to be processed from next loop. */ aws_linked_list_remove(&meta_request->client_process_work_threaded_data.node); diff --git a/source/s3express_credentials_provider.c b/source/s3express_credentials_provider.c index 2bb5c3160..1dbf9b969 100644 --- a/source/s3express_credentials_provider.c +++ b/source/s3express_credentials_provider.c @@ -497,7 +497,6 @@ static struct aws_s3express_session_creator *s_session_creator_new( }; aws_byte_buf_init(&session_creator->response_buf, provider->allocator, 512); - struct aws_byte_cursor operation_name = aws_byte_cursor_from_c_str("CreateSession"); struct aws_s3_meta_request_options options = { .message = request, .type = AWS_S3_META_REQUEST_TYPE_DEFAULT, @@ -507,7 +506,7 @@ static struct aws_s3express_session_creator *s_session_creator_new( /* Override endpoint only for tests. */ .endpoint = impl->mock_test.endpoint_override ? impl->mock_test.endpoint_override : NULL, .user_data = session_creator, - .operation_name = operation_name, + .operation_name = aws_byte_cursor_from_c_str("CreateSession"), }; session_creator->synced_data.meta_request = aws_s3_client_make_meta_request(impl->client, &options); AWS_FATAL_ASSERT(session_creator->synced_data.meta_request); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index f4331498a..c827a0ade 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -294,13 +294,14 @@ if(ENABLE_MOCK_SERVER_TESTS) add_net_test_case(request_time_too_skewed_mock_server) endif() -add_net_test_case(s3express_provider_long_running) -add_net_test_case(s3express_client_put_test_small) -add_net_test_case(s3express_client_put_test_large) -add_net_test_case(s3express_client_put_test_multiple) -add_net_test_case(s3express_client_put_long_running_test) -add_net_test_case(s3express_client_get_test) -add_net_test_case(s3express_client_get_test_multiple) +add_net_test_case(s3express_provider_long_running_session_refresh) + +add_net_test_case(s3express_client_put_object) +add_net_test_case(s3express_client_put_object_multipart) +add_net_test_case(s3express_client_put_object_multipart_multiple) +add_net_test_case(s3express_client_put_object_long_running_session_refresh) +add_net_test_case(s3express_client_get_object) +add_net_test_case(s3express_client_get_object_multiple) add_net_test_case(meta_request_auto_ranged_get_new_error_handling) add_net_test_case(meta_request_auto_ranged_put_new_error_handling) diff --git a/tests/s3_data_plane_tests.c b/tests/s3_data_plane_tests.c index cc1de27db..e879a9557 100644 --- a/tests/s3_data_plane_tests.c +++ b/tests/s3_data_plane_tests.c @@ -1711,7 +1711,8 @@ static int s_test_s3_multipart_put_object_with_acl(struct aws_allocator *allocat static int s_test_s3_put_object_multiple_helper(struct aws_allocator *allocator, bool file_on_disk) { -#define NUM_REQUESTS 5 + enum s_numbers { NUM_REQUESTS = 5 }; + struct aws_s3_meta_request *meta_requests[NUM_REQUESTS]; struct aws_s3_meta_request_test_results meta_request_test_results[NUM_REQUESTS]; struct aws_http_message *messages[NUM_REQUESTS]; @@ -1801,7 +1802,6 @@ static int s_test_s3_put_object_multiple_helper(struct aws_allocator *allocator, aws_string_destroy(filepath_str[i]); } } -#undef NUM_REQUESTS aws_string_destroy(host_name); host_name = NULL; diff --git a/tests/s3_mock_server_s3express_provider_test.c b/tests/s3_mock_server_s3express_provider_test.c index 6e988bc7d..693c9fdf8 100644 --- a/tests/s3_mock_server_s3express_provider_test.c +++ b/tests/s3_mock_server_s3express_provider_test.c @@ -581,7 +581,7 @@ TEST_CASE(s3express_provider_stress_mock_server) { return AWS_OP_SUCCESS; } -TEST_CASE(s3express_provider_long_running) { +TEST_CASE(s3express_provider_long_running_session_refresh) { (void)ctx; struct aws_s3_tester tester; diff --git a/tests/s3_s3express_client_test.c b/tests/s3_s3express_client_test.c index 8bdba09f8..cad5b81d2 100644 --- a/tests/s3_s3express_client_test.c +++ b/tests/s3_s3express_client_test.c @@ -256,20 +256,21 @@ static int s_s3express_client_put_test_helper(struct aws_allocator *allocator, s return AWS_OP_SUCCESS; } -TEST_CASE(s3express_client_put_test_small) { +TEST_CASE(s3express_client_put_object) { (void)ctx; return s_s3express_client_put_test_helper(allocator, MB_TO_BYTES(1)); } -TEST_CASE(s3express_client_put_test_large) { +TEST_CASE(s3express_client_put_object_multipart) { (void)ctx; return s_s3express_client_put_test_helper(allocator, MB_TO_BYTES(100)); } -TEST_CASE(s3express_client_put_test_multiple) { +TEST_CASE(s3express_client_put_object_multipart_multiple) { (void)ctx; -#define NUM_REQUESTS 100 + enum s_numbers { NUM_REQUESTS = 100 }; + struct aws_s3_meta_request *meta_requests[NUM_REQUESTS]; struct aws_s3_meta_request_test_results meta_request_test_results[NUM_REQUESTS]; struct aws_input_stream *input_streams[NUM_REQUESTS]; @@ -303,6 +304,7 @@ TEST_CASE(s3express_client_put_test_multiple) { struct aws_byte_cursor request_region = region_cursor; struct aws_byte_cursor request_host = host_cursor; if (i % 2 == 0) { + /* Make half of request to east1 and rest half to west2 */ request_region = west2_region_cursor; request_host = west2_host_cursor; } @@ -348,7 +350,7 @@ TEST_CASE(s3express_client_put_test_multiple) { for (size_t i = 0; i < NUM_REQUESTS; ++i) { aws_input_stream_release(input_streams[i]); } -#undef NUM_REQUESTS + aws_s3_client_release(client); aws_s3_tester_clean_up(&tester); return AWS_OP_SUCCESS; @@ -388,7 +390,7 @@ struct aws_s3express_credentials_provider *s_s3express_provider_mock_factory( } /* Long running test to make sure our refresh works properly */ -TEST_CASE(s3express_client_put_long_running_test) { +TEST_CASE(s3express_client_put_object_long_running_session_refresh) { (void)ctx; struct aws_s3_tester tester; @@ -463,7 +465,7 @@ TEST_CASE(s3express_client_put_long_running_test) { return AWS_OP_SUCCESS; } -TEST_CASE(s3express_client_get_test) { +TEST_CASE(s3express_client_get_object) { (void)ctx; struct aws_s3_tester tester; @@ -518,7 +520,7 @@ TEST_CASE(s3express_client_get_test) { return AWS_OP_SUCCESS; } -TEST_CASE(s3express_client_get_test_multiple) { +TEST_CASE(s3express_client_get_object_multiple) { (void)ctx; struct aws_s3_meta_request *meta_requests[100];