Fix mpu part size round off error for large files (#289)

Author: waahm7

include/aws/s3/private/s3_util.h

@@ -236,6 +236,20 @@ int aws_s3_parse_content_length_response_header(
 AWS_S3_API
 uint32_t aws_s3_get_num_parts(size_t part_size, uint64_t object_range_start, uint64_t object_range_end);
 
+/**
+ * Calculates the optimal part size and num parts given the 'content_length' and 'client_part_size'.
+ * This will increase the part size to stay within S3's number of parts.
+ * If the required part size exceeds the 'client_max_part_size' or
+ * if the system cannot support the required part size, it will raise an 'AWS_ERROR_INVALID_ARGUMENT' argument.
+ */
+AWS_S3_API
+int aws_s3_calculate_optimal_mpu_part_size_and_num_parts(
+    uint64_t content_length,
+    size_t client_part_size,
+    uint64_t client_max_part_size,
+    size_t *out_part_size,
+    uint32_t *out_num_parts);
+
 /* Calculates the part range for a part given overall object range, size of each part, and the part's number. Note: part
  * numbers begin at one. This takes into account aligning part-ranges on part_size. Intended to be used in conjunction
  * with aws_s3_get_num_parts. part_number should be less than or equal to the result of aws_s3_get_num_parts. */

source/s3_client.c

@@ -1040,47 +1040,13 @@ static struct aws_s3_meta_request *s_s3_client_meta_request_factory_default(
                     }
                 }
 
-                uint64_t part_size_uint64 = content_length / (uint64_t)g_s3_max_num_upload_parts;
-
-                if (part_size_uint64 > SIZE_MAX) {
-                    AWS_LOGF_ERROR(
-                        AWS_LS_S3_META_REQUEST,
-                        "Could not create auto-ranged-put meta request; required part size of %" PRIu64
-                        " bytes is too large for platform.",
-                        part_size_uint64);
-
-                    aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
-                    return NULL;
-                }
-
-                size_t part_size = (size_t)part_size_uint64;
-
-                if (part_size > client_max_part_size) {
-                    AWS_LOGF_ERROR(
-                        AWS_LS_S3_META_REQUEST,
-                        "Could not create auto-ranged-put meta request; required part size for put request is %" PRIu64
-                        ", but current maximum part size is %" PRIu64,
-                        (uint64_t)part_size,
-                        (uint64_t)client_max_part_size);
-                    aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
+                size_t part_size;
+                uint32_t num_parts;
+                if (aws_s3_calculate_optimal_mpu_part_size_and_num_parts(
+                        content_length, client_part_size, client_max_part_size, &part_size, &num_parts)) {
                     return NULL;
                 }
 
-                if (part_size < client_part_size) {
-                    part_size = client_part_size;
-                }
-                if (content_length < part_size) {
-                    /* When the content length is smaller than part size and larger than the threshold, we set one part
-                     * with the whole length */
-                    part_size = (size_t)content_length;
-                }
-
-                uint32_t num_parts = (uint32_t)(content_length / part_size);
-
-                if ((content_length % part_size) > 0) {
-                    ++num_parts;
-                }
-
                 return aws_s3_meta_request_auto_ranged_put_new(
                     client->allocator, client, part_size, content_length, num_parts, options);
             } else {

source/s3_util.c

@@ -547,6 +547,65 @@ void aws_s3_get_part_range(
     }
 }
 
+int aws_s3_calculate_optimal_mpu_part_size_and_num_parts(
+    uint64_t content_length,
+    size_t client_part_size,
+    uint64_t client_max_part_size,
+    size_t *out_part_size,
+    uint32_t *out_num_parts) {
+
+    AWS_FATAL_ASSERT(out_part_size);
+    AWS_FATAL_ASSERT(out_num_parts);
+
+    uint64_t part_size_uint64 = content_length / (uint64_t)g_s3_max_num_upload_parts;
+
+    if ((content_length % g_s3_max_num_upload_parts) > 0) {
+        ++part_size_uint64;
+    }
+
+    if (part_size_uint64 > SIZE_MAX) {
+        AWS_LOGF_ERROR(
+            AWS_LS_S3_META_REQUEST,
+            "Could not create auto-ranged-put meta request; required part size of %" PRIu64
+            " bytes is too large for platform.",
+            part_size_uint64);
+
+        return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
+    }
+
+    size_t part_size = (size_t)part_size_uint64;
+
+    if (part_size > client_max_part_size) {
+        AWS_LOGF_ERROR(
+            AWS_LS_S3_META_REQUEST,
+            "Could not create auto-ranged-put meta request; required part size for put request is %" PRIu64
+            ", but current maximum part size is %" PRIu64,
+            (uint64_t)part_size,
+            (uint64_t)client_max_part_size);
+        return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
+    }
+
+    if (part_size < client_part_size) {
+        part_size = client_part_size;
+    }
+
+    if (content_length < part_size) {
+        /* When the content length is smaller than part size and larger than the threshold, we set one part
+         * with the whole length */
+        part_size = (size_t)content_length;
+    }
+
+    uint32_t num_parts = (uint32_t)(content_length / part_size);
+    if ((content_length % part_size) > 0) {
+        ++num_parts;
+    }
+    AWS_ASSERT(num_parts <= g_s3_max_num_upload_parts);
+
+    *out_part_size = part_size;
+    *out_num_parts = num_parts;
+    return AWS_OP_SUCCESS;
+}
+
 int aws_s3_crt_error_code_from_server_error_code_string(const struct aws_string *error_code_string) {
     if (aws_string_eq_byte_cursor(error_code_string, &g_s3_slow_down_error_code)) {
         return AWS_ERROR_S3_SLOW_DOWN;

tests/CMakeLists.txt

@@ -160,6 +160,7 @@ add_test_case(test_s3_strip_quotes)
 add_test_case(test_s3_parse_content_range_response_header)
 add_test_case(test_s3_parse_content_length_response_header)
 add_test_case(test_s3_get_num_parts_and_get_part_range)
+add_test_case(test_s3_mpu_get_part_size_and_num_parts)
 add_test_case(test_s3_aws_xml_get_top_level_tag_with_root_name)
 add_test_case(test_add_user_agent_header)
 

tests/s3_util_tests.c

@@ -364,6 +364,152 @@ static int s_test_s3_get_num_parts_and_get_part_range(struct aws_allocator *allo
     return 0;
 }
 
+struct s3_request_part_config_example {
+    const char *name;
+    uint64_t content_length;
+    size_t client_part_size;
+    uint64_t client_max_part_size;
+    size_t expected_part_size;
+    uint32_t expected_num_parts;
+};
+
+AWS_TEST_CASE(test_s3_mpu_get_part_size_and_num_parts, s_test_s3_mpu_get_part_size_and_num_parts)
+static int s_test_s3_mpu_get_part_size_and_num_parts(struct aws_allocator *allocator, void *ctx) {
+    (void)allocator;
+    (void)ctx;
+    uint64_t default_max_part_size = 5368709120ULL;
+
+    const struct s3_request_part_config_example valid_request_part_config[] = {
+        {
+            .name = "simple case",
+            .content_length = MB_TO_BYTES((uint64_t)10000),
+            .client_part_size = MB_TO_BYTES(5),
+            .client_max_part_size = default_max_part_size,
+            .expected_part_size = 5242880,
+            .expected_num_parts = 2000,
+        },
+        {
+            .name = "large content length with small part size",
+            .content_length = MB_TO_BYTES((uint64_t)990000),
+            .client_part_size = MB_TO_BYTES(5),
+            .client_max_part_size = default_max_part_size,
+            .expected_part_size = 103809024,
+            .expected_num_parts = 10000,
+        },
+        {
+
+            .name = "large content length with large part size",
+            .content_length = MB_TO_BYTES((uint64_t)1000000),
+            .client_part_size = MB_TO_BYTES(500),
+            .client_max_part_size = default_max_part_size,
+            .expected_part_size = MB_TO_BYTES(500),
+            .expected_num_parts = 2000,
+        },
+        {
+            .name = "large odd content length",
+            .content_length = 1044013645824,
+            .client_part_size = 5242880,
+            .client_max_part_size = default_max_part_size,
+            .expected_part_size = 104401365,
+            .expected_num_parts = 10000,
+        },
+        {
+            .name = "10k parts",
+            .content_length = MB_TO_BYTES((uint64_t)50000),
+            .client_part_size = MB_TO_BYTES(5),
+            .client_max_part_size = default_max_part_size,
+            .expected_part_size = MB_TO_BYTES(5),
+            .expected_num_parts = 10000,
+        },
+        {
+            .name = "10k - 1 parts",
+            .content_length = 49995,
+            .client_part_size = 5,
+            .client_max_part_size = default_max_part_size,
+            .expected_part_size = 5,
+            .expected_num_parts = 9999,
+        },
+        {
+            .name = "10k with small last part",
+            .content_length = 49998,
+            .client_part_size = 5,
+            .client_max_part_size = default_max_part_size,
+            .expected_part_size = 5,
+            .expected_num_parts = 10000,
+        },
+        {
+            .name = "10k + 1 parts",
+            .content_length = 50001,
+            .client_part_size = 5,
+            .client_max_part_size = default_max_part_size,
+            .expected_part_size = 6,
+            .expected_num_parts = 8334,
+
+        },
+        {
+            .name = "bump content length",
+            .content_length = 100000,
+            .client_part_size = 5,
+            .client_max_part_size = default_max_part_size,
+            .expected_part_size = 10,
+            .expected_num_parts = 10000,
+        },
+        {
+            .name = "bump content length with non-zero mod",
+            .content_length = 999999,
+            .client_part_size = 5,
+            .client_max_part_size = default_max_part_size,
+            .expected_part_size = 100,
+            .expected_num_parts = 10000,
+        },
+        {
+            .name = "5 tb content length",
+            .content_length = MB_TO_BYTES((uint64_t)5 * 1024 * 1024),
+            .client_part_size = MB_TO_BYTES((uint64_t)5),
+            .client_max_part_size = default_max_part_size,
+            .expected_part_size = 549755814,
+            .expected_num_parts = 10000,
+        },
+    };
+    for (size_t i = 0; i < AWS_ARRAY_SIZE(valid_request_part_config); ++i) {
+        AWS_LOGF_INFO(AWS_LS_S3_GENERAL, "valid example [%zu]: %s\n", i, valid_request_part_config[i].name);
+
+        uint64_t content_length = valid_request_part_config[i].content_length;
+        size_t part_size;
+        uint32_t num_parts;
+
+        ASSERT_SUCCESS(aws_s3_calculate_optimal_mpu_part_size_and_num_parts(
+            content_length,
+            valid_request_part_config[i].client_part_size,
+            valid_request_part_config[i].client_max_part_size,
+            &part_size,
+            &num_parts));
+        ASSERT_INT_EQUALS(valid_request_part_config[i].expected_part_size, part_size);
+        ASSERT_INT_EQUALS(valid_request_part_config[i].expected_num_parts, num_parts);
+    }
+
+    /* Invalid cases */
+    const struct s3_request_part_config_example invalid_request_part_config[] = {{
+        .name = "max part < required part size",
+        .content_length = 900000,
+        .client_part_size = 5,
+        .client_max_part_size = 10,
+    }};
+
+    for (size_t i = 0; i < AWS_ARRAY_SIZE(invalid_request_part_config); ++i) {
+        printf("invalid example [%zu]: %s\n", i, invalid_request_part_config[i].name);
+        size_t part_size;
+        uint32_t num_parts;
+        ASSERT_FAILS(aws_s3_calculate_optimal_mpu_part_size_and_num_parts(
+            invalid_request_part_config[i].content_length,
+            invalid_request_part_config[i].client_part_size,
+            invalid_request_part_config[i].client_max_part_size,
+            &part_size,
+            &num_parts));
+    }
+    return AWS_OP_SUCCESS;
+}
+
 AWS_TEST_CASE(test_s3_aws_xml_get_top_level_tag_with_root_name, s_test_s3_aws_xml_get_top_level_tag_with_root_name)
 static int s_test_s3_aws_xml_get_top_level_tag_with_root_name(struct aws_allocator *allocator, void *ctx) {
     (void)allocator;