diff --git a/include/aws/s3/s3_client.h b/include/aws/s3/s3_client.h index 7aefdb9c3..c95ab0a9b 100644 --- a/include/aws/s3/s3_client.h +++ b/include/aws/s3/s3_client.h @@ -393,8 +393,9 @@ struct aws_s3_client_config { * Optional. * Size of parts the object will be downloaded or uploaded in, in bytes. * This only affects AWS_S3_META_REQUEST_TYPE_GET_OBJECT and AWS_S3_META_REQUEST_TYPE_PUT_OBJECT. - * If set, this should be at least 5MiB. - * If not set, this defaults to 8MiB. + * If not set, this defaults to 8 MiB. + * The client will adjust the part size for AWS_S3_META_REQUEST_TYPE_PUT_OBJECT if needed for service limits (max + * number of parts per upload is 10,000, minimum upload part size is 5 MiB). * * You can also set this per meta-request, via `aws_s3_meta_request_options.part_size`. */ @@ -569,8 +570,6 @@ struct aws_s3_checksum_config { * 3) If the data will be be produced in asynchronous chunks, set `send_async_stream`. */ struct aws_s3_meta_request_options { - /* TODO: The meta request options cannot control the request to be split or not. Should consider to add one */ - /* The type of meta request we will be trying to accelerate. */ enum aws_s3_meta_request_type type; @@ -631,8 +630,10 @@ struct aws_s3_meta_request_options { * Optional. * Size of parts the object will be downloaded or uploaded in, in bytes. * This only affects AWS_S3_META_REQUEST_TYPE_GET_OBJECT and AWS_S3_META_REQUEST_TYPE_PUT_OBJECT. - * If set, this should be at least 5MiB. * If not set, the value from `aws_s3_client_config.part_size` is used, which defaults to 8MiB. + * + * The client will adjust the part size for AWS_S3_META_REQUEST_TYPE_PUT_OBJECT if needed for service limits (max + * number of parts per upload is 10,000, minimum upload part size is 5 MiB). */ uint64_t part_size; @@ -645,7 +646,7 @@ struct aws_s3_meta_request_options { * If set, this should be at least `part_size`. * If not set, `part_size` will be used as the threshold. * If both `part_size` and `multipart_upload_threshold` are not set, - * the values from `aws_s3_client_config` are used, which default to 8MiB. + * the values from `aws_s3_client_config` are used. */ uint64_t multipart_upload_threshold; diff --git a/source/s3_client.c b/source/s3_client.c index 6d3f36adf..1eda4b64d 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -347,7 +347,11 @@ struct aws_s3_client *aws_s3_client_new( size_t part_size; if (client_config->part_size != 0) { - part_size = (size_t)client_config->part_size; + if (client_config->part_size > SIZE_MAX) { + part_size = SIZE_MAX; + } else { + part_size = (size_t)client_config->part_size; + } } else { part_size = s_default_part_size; } @@ -424,6 +428,8 @@ struct aws_s3_client *aws_s3_client_new( if (client_config->multipart_upload_threshold != 0) { *((uint64_t *)&client->multipart_upload_threshold) = client_config->multipart_upload_threshold; + } else { + *((uint64_t *)&client->multipart_upload_threshold) = part_size; } if (client_config->max_part_size < client_config->part_size) { @@ -1151,6 +1157,7 @@ static struct aws_s3_meta_request *s_s3_client_meta_request_factory_default( aws_raise_error(AWS_ERROR_INVALID_ARGUMENT); return NULL; } + size_t part_size_config = options->part_size == 0 ? client->part_size : options->part_size; /* Call the appropriate meta-request new function. */ switch (options->type) { @@ -1169,7 +1176,7 @@ static struct aws_s3_meta_request *s_s3_client_meta_request_factory_default( options); } - return aws_s3_meta_request_auto_ranged_get_new(client->allocator, client, client->part_size, options); + return aws_s3_meta_request_auto_ranged_get_new(client->allocator, client, part_size_config, options); } case AWS_S3_META_REQUEST_TYPE_PUT_OBJECT: { if (body_source_count == 0) { @@ -1182,19 +1189,17 @@ static struct aws_s3_meta_request *s_s3_client_meta_request_factory_default( } if (options->resume_token == NULL) { - - size_t client_part_size = client->part_size; uint64_t client_max_part_size = client->max_part_size; - if (client_part_size < g_s3_min_upload_part_size) { + if (part_size_config < g_s3_min_upload_part_size) { AWS_LOGF_WARN( AWS_LS_S3_META_REQUEST, - "Client config part size of %" PRIu64 " is less than the minimum upload part size of %" PRIu64 + "Config part size of %" PRIu64 " is less than the minimum upload part size of %" PRIu64 ". Using to the minimum part-size for upload.", - (uint64_t)client_part_size, + (uint64_t)part_size_config, (uint64_t)g_s3_min_upload_part_size); - client_part_size = g_s3_min_upload_part_size; + part_size_config = g_s3_min_upload_part_size; } if (client_max_part_size < (uint64_t)g_s3_min_upload_part_size) { @@ -1208,8 +1213,15 @@ static struct aws_s3_meta_request *s_s3_client_meta_request_factory_default( client_max_part_size = (uint64_t)g_s3_min_upload_part_size; } - uint64_t multipart_upload_threshold = - client->multipart_upload_threshold == 0 ? client_part_size : client->multipart_upload_threshold; + + uint64_t multipart_upload_threshold = client->multipart_upload_threshold; + if (options->multipart_upload_threshold != 0) { + /* If the threshold is set for the meta request, use it */ + multipart_upload_threshold = options->multipart_upload_threshold; + } else if (options->part_size != 0) { + /* If the threshold is not set, but the part size is set for the meta request, us it */ + multipart_upload_threshold = options->part_size; + } if (content_length_found && content_length <= multipart_upload_threshold) { return aws_s3_meta_request_default_new( @@ -1233,17 +1245,29 @@ static struct aws_s3_meta_request *s_s3_client_meta_request_factory_default( } } - size_t part_size = client_part_size; + size_t adjusted_part_size = part_size_config; uint32_t num_parts = 0; if (content_length_found) { if (aws_s3_calculate_optimal_mpu_part_size_and_num_parts( - content_length, client_part_size, client_max_part_size, &part_size, &num_parts)) { + content_length, part_size_config, client_max_part_size, &adjusted_part_size, &num_parts)) { return NULL; } } + if (adjusted_part_size != part_size_config) { + AWS_LOGF_DEBUG( + AWS_LS_S3_META_REQUEST, + "The multipart upload part size has been adjusted to %" PRIu64 "", + (uint64_t)adjusted_part_size); + } return aws_s3_meta_request_auto_ranged_put_new( - client->allocator, client, part_size, content_length_found, content_length, num_parts, options); + client->allocator, + client, + adjusted_part_size, + content_length_found, + content_length, + num_parts, + options); } else { /* else using resume token */ if (!content_length_found) { AWS_LOGF_ERROR( diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index a4f81da52..7a6fc02dd 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -327,6 +327,8 @@ add_test_case(test_s3_buffer_pool_too_small) add_net_test_case(test_s3_put_object_buffer_pool_trim) add_net_test_case(client_update_upload_part_timeout) +add_net_test_case(client_meta_request_override_part_size) +add_net_test_case(client_meta_request_override_multipart_upload_threshold) set(TEST_BINARY_NAME ${PROJECT_NAME}-tests) generate_test_driver(${TEST_BINARY_NAME}) diff --git a/tests/s3_client_test.c b/tests/s3_client_test.c index 2436b1610..3b4f478d2 100644 --- a/tests/s3_client_test.c +++ b/tests/s3_client_test.c @@ -246,3 +246,144 @@ TEST_CASE(client_update_upload_part_timeout) { aws_s3_tester_clean_up(&tester); return AWS_OP_SUCCESS; } + +/* Test meta request can override the part size as expected */ +TEST_CASE(client_meta_request_override_part_size) { + (void)ctx; + struct aws_s3_tester tester; + AWS_ZERO_STRUCT(tester); + ASSERT_SUCCESS(aws_s3_tester_init(allocator, &tester)); + struct aws_s3_client *client = NULL; + struct aws_s3_tester_client_options client_options = { + .part_size = MB_TO_BYTES(8), + .tls_usage = AWS_S3_TLS_DISABLED, + }; + ASSERT_SUCCESS(aws_s3_tester_client_new(&tester, &client_options, &client)); + + struct aws_string *host_name = + aws_s3_tester_build_endpoint_string(allocator, &g_test_bucket_name, &g_test_s3_region); + struct aws_byte_cursor host_cur = aws_byte_cursor_from_string(host_name); + struct aws_byte_cursor test_object_path = aws_byte_cursor_from_c_str("/mytest"); + + size_t override_part_size = MB_TO_BYTES(10); + size_t content_length = + MB_TO_BYTES(20); /* Let the content length larger than the override part size to make sure we do MPU */ + + /* MPU put object */ + struct aws_input_stream_tester_options stream_options = { + .autogen_length = content_length, + }; + struct aws_input_stream *input_stream = aws_input_stream_new_tester(allocator, &stream_options); + + struct aws_http_message *put_messages = aws_s3_test_put_object_request_new( + allocator, &host_cur, g_test_body_content_type, test_object_path, input_stream, 0 /*flags*/); + + struct aws_s3_meta_request_options meta_request_options = { + .message = put_messages, + .type = AWS_S3_META_REQUEST_TYPE_PUT_OBJECT, + .part_size = override_part_size, + }; + struct aws_s3_meta_request *put_meta_request = client->vtable->meta_request_factory(client, &meta_request_options); + ASSERT_UINT_EQUALS(put_meta_request->part_size, override_part_size); + + /* auto ranged Get Object */ + struct aws_http_message *get_message = aws_s3_test_get_object_request_new( + allocator, aws_byte_cursor_from_string(host_name), g_pre_existing_object_1MB); + + struct aws_s3_meta_request_options get_meta_request_options = { + .message = get_message, + .type = AWS_S3_META_REQUEST_TYPE_GET_OBJECT, + .part_size = override_part_size, + }; + + struct aws_s3_meta_request *get_meta_request = + client->vtable->meta_request_factory(client, &get_meta_request_options); + ASSERT_UINT_EQUALS(get_meta_request->part_size, override_part_size); + + aws_http_message_release(put_messages); + aws_s3_meta_request_release(put_meta_request); + aws_http_message_release(get_message); + aws_s3_meta_request_release(get_meta_request); + aws_string_destroy(host_name); + aws_s3_client_release(client); + aws_input_stream_release(input_stream); + aws_s3_tester_clean_up(&tester); + + return AWS_OP_SUCCESS; +} + +/* Test meta request can override the multipart upload threshold as expected */ +TEST_CASE(client_meta_request_override_multipart_upload_threshold) { + (void)ctx; + struct aws_s3_tester tester; + ASSERT_SUCCESS(aws_s3_tester_init(allocator, &tester)); + + struct aws_s3_client_config client_config = { + .part_size = MB_TO_BYTES(8), + .multipart_upload_threshold = MB_TO_BYTES(15), + }; + + ASSERT_SUCCESS(aws_s3_tester_bind_client( + &tester, &client_config, AWS_S3_TESTER_BIND_CLIENT_REGION | AWS_S3_TESTER_BIND_CLIENT_SIGNING)); + + struct aws_s3_client *client = aws_s3_client_new(allocator, &client_config); + + ASSERT_TRUE(client != NULL); + + struct aws_string *host_name = + aws_s3_tester_build_endpoint_string(allocator, &g_test_bucket_name, &g_test_s3_region); + struct aws_byte_cursor host_cur = aws_byte_cursor_from_string(host_name); + struct aws_byte_cursor test_object_path = aws_byte_cursor_from_c_str("/mytest"); + + size_t override_multipart_upload_threshold = MB_TO_BYTES(20); + size_t content_length = + MB_TO_BYTES(20); /* Let the content length larger than the override part size to make sure we do MPU */ + + /* MPU put object */ + struct aws_input_stream_tester_options stream_options = { + .autogen_length = content_length, + }; + struct aws_input_stream *input_stream = aws_input_stream_new_tester(allocator, &stream_options); + + struct aws_http_message *put_messages = aws_s3_test_put_object_request_new( + allocator, &host_cur, g_test_body_content_type, test_object_path, input_stream, 0 /*flags*/); + + { + /* Content length is smaller than the override multipart_upload_threshold */ + struct aws_s3_meta_request_options meta_request_options = { + .message = put_messages, + .type = AWS_S3_META_REQUEST_TYPE_PUT_OBJECT, + .multipart_upload_threshold = override_multipart_upload_threshold, + }; + struct aws_s3_meta_request *put_meta_request = + client->vtable->meta_request_factory(client, &meta_request_options); + + /* Part size will be 0, as we don't use MPU */ + ASSERT_UINT_EQUALS(put_meta_request->part_size, 0); + aws_s3_meta_request_release(put_meta_request); + } + + { + /* meta request override the part size, so the override part size will be used as the multipart upload threshold + */ + struct aws_s3_meta_request_options meta_request_options = { + .message = put_messages, + .type = AWS_S3_META_REQUEST_TYPE_PUT_OBJECT, + .part_size = override_multipart_upload_threshold, + }; + struct aws_s3_meta_request *put_meta_request = + client->vtable->meta_request_factory(client, &meta_request_options); + + /* Part size will be 0, as we don't use MPU */ + ASSERT_UINT_EQUALS(put_meta_request->part_size, 0); + aws_s3_meta_request_release(put_meta_request); + } + + aws_http_message_release(put_messages); + aws_string_destroy(host_name); + aws_s3_client_release(client); + aws_input_stream_release(input_stream); + aws_s3_tester_clean_up(&tester); + + return AWS_OP_SUCCESS; +}