comments address

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);

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);

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)

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;

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;

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];