Skip to content

Commit 799b98c

Browse files
TingDaoKTingDaoK
authored
Merge branch 'main' into pause-for-get
2 parents 9295f1d + 5eac79f commit 799b98c

File tree

6 files changed

+190
-40
lines changed

6 files changed

+190
-40
lines changed

‎include/aws/s3/exports.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* SPDX-License-Identifier: Apache-2.0.
77
*/
88

9-
#if defined(USE_WINDOWS_DLL_SEMANTICS) || defined(WIN32)
9+
#if defined(AWS_CRT_USE_WINDOWS_DLL_SEMANTICS) || defined(_WIN32)
1010
# ifdef AWS_S3_USE_IMPORT_EXPORT
1111
# ifdef AWS_S3_EXPORTS
1212
# define AWS_S3_API __declspec(dllexport)
@@ -18,12 +18,12 @@
1818
# endif /*USE_IMPORT_EXPORT */
1919

2020
#else
21-
# if ((__GNUC__ >= 4) || defined(__clang__)) && defined(AWS_S3_USE_IMPORT_EXPORT) && defined(AWS_S3_EXPORTS)
21+
# if defined(AWS_S3_USE_IMPORT_EXPORT) && defined(AWS_S3_EXPORTS)
2222
# define AWS_S3_API __attribute__((visibility("default")))
2323
# else
2424
# define AWS_S3_API
25-
# endif /* __GNUC__ >= 4 || defined(__clang__) */
25+
# endif
2626

27-
#endif /* defined(USE_WINDOWS_DLL_SEMANTICS) || defined(WIN32) */
27+
#endif /* defined(AWS_CRT_USE_WINDOWS_DLL_SEMANTICS) || defined(_WIN32) */
2828

2929
#endif /* AWS_S3_EXPORTS_H */

‎include/aws/s3/private/s3_client_impl.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ struct aws_http_connection;
2626
struct aws_http_connection_manager;
2727
struct aws_host_resolver;
2828
struct aws_s3_endpoint;
29+
struct aws_priority_queue;
2930

3031
enum aws_s3_connection_finish_code {
3132
AWS_S3_CONNECTION_FINISH_CODE_SUCCESS,
@@ -185,6 +186,8 @@ struct aws_s3_upload_part_timeout_stats {
185186
struct {
186187
uint64_t sum_ns;
187188
uint64_t num_samples;
189+
bool collecting_p90;
190+
struct aws_priority_queue p90_samples;
188191
} initial_request_time;
189192

190193
/* Track the timeout rate. */

‎source/s3.c‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <aws/s3/s3_client.h>
1010

1111
#include <aws/auth/auth.h>
12+
#include <aws/checksums/checksums.h>
1213
#include <aws/common/error.h>
1314
#include <aws/common/hash_table.h>
1415
#include <aws/http/http.h>
@@ -195,6 +196,7 @@ void aws_s3_library_init(struct aws_allocator *allocator) {
195196

196197
aws_auth_library_init(s_library_allocator);
197198
aws_http_library_init(s_library_allocator);
199+
aws_checksums_library_init(s_library_allocator);
198200

199201
aws_register_error_info(&s_error_list);
200202
aws_register_log_subject_info_list(&s_s3_log_subject_list);
@@ -231,5 +233,6 @@ void aws_s3_library_clean_up(void) {
231233
aws_unregister_error_info(&s_error_list);
232234
aws_http_library_clean_up();
233235
aws_auth_library_clean_up();
236+
aws_checksums_library_clean_up();
234237
s_library_allocator = NULL;
235238
}

‎source/s3_client.c‎

Lines changed: 58 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <aws/common/clock.h>
2323
#include <aws/common/device_random.h>
2424
#include <aws/common/json.h>
25+
#include <aws/common/priority_queue.h>
2526
#include <aws/common/string.h>
2627
#include <aws/common/system_info.h>
2728
#include <aws/http/connection.h>
@@ -768,6 +769,9 @@ static void s_s3_client_finish_destroy_default(struct aws_s3_client *client) {
768769
void *shutdown_user_data = client->shutdown_callback_user_data;
769770

770771
aws_s3_buffer_pool_destroy(client->buffer_pool);
772+
if (client->synced_data.upload_part_stats.initial_request_time.collecting_p90) {
773+
aws_priority_queue_clean_up(&client->synced_data.upload_part_stats.initial_request_time.p90_samples);
774+
}
771775

772776
aws_mem_release(client->allocator, client->network_interface_names_cursor_array);
773777
for (size_t i = 0; i < client->num_network_interface_names; i++) {
@@ -2492,6 +2496,13 @@ static uint64_t s_upload_timeout_threshold_ns = 5000000000; /* 5 Secs */
24922496
const size_t g_expect_timeout_offset_ms =
24932497
700; /* 0.7 Secs. From experiments on c5n.18xlarge machine for 30 GiB upload, it gave us best performance. */
24942498

2499+
static int s_compare_uint64_min_heap(const void *a, const void *b) {
2500+
uint64_t arg1 = *(const uint64_t *)a;
2501+
uint64_t arg2 = *(const uint64_t *)b;
2502+
/* Use a min-heap to get the P90, which will be the min of the largest 10% of data. */
2503+
return arg1 > arg2;
2504+
}
2505+
24952506
/**
24962507
* The upload timeout optimization: explained.
24972508
*
@@ -2512,10 +2523,10 @@ const size_t g_expect_timeout_offset_ms =
25122523
* would be better off waiting 5sec for the response, vs re-uploading the whole request.
25132524
*
25142525
* The current algorithm:
2515-
* 1. Start without a timeout value. After 10 requests completed, we know the average of how long the
2516-
* request takes. We decide if it's worth to set a timeout value or not. (If the average of request takes more than
2517-
* 5 secs or not) TODO: if the client have different part size, this doesn't make sense
2518-
* 2. If it is worth to retry, start with a default timeout value, 1 sec.
2526+
* 1. Start without a timeout value. After max(10, # of ideal connections) requests completed, we know the average of
2527+
* how long the request takes. We decide if it's worth to set a timeout value or not. (If the average of request
2528+
* takes more than 5 secs or not).
2529+
* 2. If it is worth to retry, start with a timeout value from max(1 sec, P90 of the initial samples).
25192530
* 3. If a request finishes successfully, use the average response_to_first_byte_time + g_expect_timeout_offset_ms as
25202531
* our expected timeout value. (TODO: The real expected timeout value should be a P99 of all the requests.)
25212532
* 3.1 Adjust the current timeout value against the expected timeout value, via 0.99 * <current timeout> + 0.01 *
@@ -2548,28 +2559,67 @@ void aws_s3_client_update_upload_part_timeout(
25482559
uint64_t updated_timeout_ns = 0;
25492560
uint64_t expect_timeout_offset_ns =
25502561
aws_timestamp_convert(g_expect_timeout_offset_ms, AWS_TIMESTAMP_MILLIS, AWS_TIMESTAMP_NANOS, NULL);
2562+
// Default to ideal connection count, but at least collect 10 samples.
2563+
uint32_t num_samples_to_collect = aws_max_u32(client->ideal_connection_count, 10);
25512564

25522565
switch (finished_error_code) {
25532566
case AWS_ERROR_SUCCESS:
25542567
/* We only interested in request succeed */
25552568
stats->num_successful_upload_requests = aws_add_u64_saturating(stats->num_successful_upload_requests, 1);
2556-
if (stats->num_successful_upload_requests <= 10) {
2569+
if (stats->num_successful_upload_requests <= num_samples_to_collect) {
25572570
/* Gether the data */
25582571
uint64_t request_time_ns =
25592572
metrics->time_metrics.receive_end_timestamp_ns - metrics->time_metrics.send_start_timestamp_ns;
25602573
stats->initial_request_time.sum_ns =
25612574
aws_add_u64_saturating(stats->initial_request_time.sum_ns, request_time_ns);
25622575
++stats->initial_request_time.num_samples;
2563-
if (stats->num_successful_upload_requests == 10) {
2576+
if (!stats->initial_request_time.collecting_p90) {
2577+
/* Initialize the priority queue to collect the p90 of the initial requests. */
2578+
stats->initial_request_time.collecting_p90 = true;
2579+
size_t queue_size = num_samples_to_collect / 10;
2580+
AWS_ASSERT(queue_size > 0);
2581+
aws_priority_queue_init_dynamic(
2582+
&stats->initial_request_time.p90_samples,
2583+
client->allocator,
2584+
queue_size,
2585+
sizeof(uint64_t),
2586+
s_compare_uint64_min_heap);
2587+
} else {
2588+
/* check if the queue is full, if full pop and top and push the next. */
2589+
size_t current_size = aws_priority_queue_size(&stats->initial_request_time.p90_samples);
2590+
size_t current_capacity = aws_priority_queue_capacity(&stats->initial_request_time.p90_samples);
2591+
if (current_size == current_capacity) {
2592+
uint64_t *min = NULL;
2593+
aws_priority_queue_top(&stats->initial_request_time.p90_samples, (void **)&min);
2594+
if (request_time_ns > *min) {
2595+
/* Push the data into the queue if it's larger than the min of the queue. */
2596+
uint64_t pop_val = 0;
2597+
aws_priority_queue_pop(&stats->initial_request_time.p90_samples, &pop_val);
2598+
aws_priority_queue_push(&stats->initial_request_time.p90_samples, &request_time_ns);
2599+
}
2600+
} else {
2601+
aws_priority_queue_push(&stats->initial_request_time.p90_samples, &request_time_ns);
2602+
}
2603+
}
2604+
2605+
if (stats->num_successful_upload_requests == client->ideal_connection_count) {
25642606
/* Decide we need a timeout or not */
25652607
uint64_t average_request_time_ns =
25662608
stats->initial_request_time.sum_ns / stats->initial_request_time.num_samples;
25672609
if (average_request_time_ns >= s_upload_timeout_threshold_ns) {
25682610
/* We don't need a timeout, as retry will be slower than just wait for the server to response */
25692611
stats->stop_timeout = true;
25702612
} else {
2571-
/* Start the timeout at 1 secs */
2572-
aws_atomic_store_int(&client->upload_timeout_ms, 1000);
2613+
/* Start the timeout at th p90 of the initial samples. */
2614+
uint64_t *p90_ns = NULL;
2615+
aws_priority_queue_top(&stats->initial_request_time.p90_samples, (void **)&p90_ns);
2616+
uint64_t p90_ms =
2617+
aws_timestamp_convert(*p90_ns, AWS_TIMESTAMP_NANOS, AWS_TIMESTAMP_MILLIS, NULL);
2618+
uint64_t init_upload_timeout_ms = aws_max_u64(p90_ms, 1000 /*1sec*/);
2619+
aws_atomic_store_int(&client->upload_timeout_ms, (size_t)init_upload_timeout_ms);
2620+
/* Clean up the queue now, as not needed anymore. */
2621+
aws_priority_queue_clean_up(&stats->initial_request_time.p90_samples);
2622+
stats->initial_request_time.collecting_p90 = false;
25732623
}
25742624
}
25752625
goto unlock;

‎source/s3_endpoint_resolver/aws_s3_endpoint_resolver_partition.c‎

Lines changed: 43 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -204,33 +204,52 @@ static const char s_generated_array[] = {
204204
'"', 's', 'u', 'p', 'p', 'o', 'r', 't', 's', 'F', 'I', 'P', 'S', '"', ':', 't', 'r', 'u', 'e', '}',
205205
',', '"', 'r', 'e', 'g', 'i', 'o', 'n', 'R', 'e', 'g', 'e', 'x', '"', ':', '"', '^', 'e', 'u', '\\',
206206
'\\', '-', 'i', 's', 'o', 'e', '\\', '\\', '-', '\\', '\\', 'w', '+', '\\', '\\', '-', '\\', '\\', 'd', '+',
207-
'$', '"', ',', '"', 'r', 'e', 'g', 'i', 'o', 'n', 's', '"', ':', '{', '"', 'e', 'u', '-', 'i', 's',
208-
'o', 'e', '-', 'w', 'e', 's', 't', '-', '1', '"', ':', '{', '"', 'd', 'e', 's', 'c', 'r', 'i', 'p',
209-
't', 'i', 'o', 'n', '"', ':', '"', 'E', 'U', ' ', 'I', 'S', 'O', 'E', ' ', 'W', 'e', 's', 't', '"',
210-
'}', '}', '}', ',', '{', '"', 'i', 'd', '"', ':', '"', 'a', 'w', 's', '-', 'i', 's', 'o', '-', 'f',
211-
'"', ',', '"', 'o', 'u', 't', 'p', 'u', 't', 's', '"', ':', '{', '"', 'd', 'n', 's', 'S', 'u', 'f',
212-
'f', 'i', 'x', '"', ':', '"', 'c', 's', 'p', '.', 'h', 'c', 'i', '.', 'i', 'c', '.', 'g', 'o', 'v',
213-
'"', ',', '"', 'd', 'u', 'a', 'l', 'S', 't', 'a', 'c', 'k', 'D', 'n', 's', 'S', 'u', 'f', 'f', 'i',
214-
'x', '"', ':', '"', 'c', 's', 'p', '.', 'h', 'c', 'i', '.', 'i', 'c', '.', 'g', 'o', 'v', '"', ',',
207+
'$', '"', ',', '"', 'r', 'e', 'g', 'i', 'o', 'n', 's', '"', ':', '{', '"', 'a', 'w', 's', '-', 'i',
208+
's', 'o', '-', 'e', '-', 'g', 'l', 'o', 'b', 'a', 'l', '"', ':', '{', '"', 'd', 'e', 's', 'c', 'r',
209+
'i', 'p', 't', 'i', 'o', 'n', '"', ':', '"', 'A', 'W', 'S', ' ', 'I', 'S', 'O', 'E', ' ', '(', 'E',
210+
'u', 'r', 'o', 'p', 'e', ')', ' ', 'g', 'l', 'o', 'b', 'a', 'l', ' ', 'r', 'e', 'g', 'i', 'o', 'n',
211+
'"', '}', ',', '"', 'e', 'u', '-', 'i', 's', 'o', 'e', '-', 'w', 'e', 's', 't', '-', '1', '"', ':',
212+
'{', '"', 'd', 'e', 's', 'c', 'r', 'i', 'p', 't', 'i', 'o', 'n', '"', ':', '"', 'E', 'U', ' ', 'I',
213+
'S', 'O', 'E', ' ', 'W', 'e', 's', 't', '"', '}', '}', '}', ',', '{', '"', 'i', 'd', '"', ':', '"',
214+
'a', 'w', 's', '-', 'i', 's', 'o', '-', 'f', '"', ',', '"', 'o', 'u', 't', 'p', 'u', 't', 's', '"',
215+
':', '{', '"', 'd', 'n', 's', 'S', 'u', 'f', 'f', 'i', 'x', '"', ':', '"', 'c', 's', 'p', '.', 'h',
216+
'c', 'i', '.', 'i', 'c', '.', 'g', 'o', 'v', '"', ',', '"', 'd', 'u', 'a', 'l', 'S', 't', 'a', 'c',
217+
'k', 'D', 'n', 's', 'S', 'u', 'f', 'f', 'i', 'x', '"', ':', '"', 'c', 's', 'p', '.', 'h', 'c', 'i',
218+
'.', 'i', 'c', '.', 'g', 'o', 'v', '"', ',', '"', 'i', 'm', 'p', 'l', 'i', 'c', 'i', 't', 'G', 'l',
219+
'o', 'b', 'a', 'l', 'R', 'e', 'g', 'i', 'o', 'n', '"', ':', '"', 'u', 's', '-', 'i', 's', 'o', 'f',
220+
'-', 's', 'o', 'u', 't', 'h', '-', '1', '"', ',', '"', 'n', 'a', 'm', 'e', '"', ':', '"', 'a', 'w',
221+
's', '-', 'i', 's', 'o', '-', 'f', '"', ',', '"', 's', 'u', 'p', 'p', 'o', 'r', 't', 's', 'D', 'u',
222+
'a', 'l', 'S', 't', 'a', 'c', 'k', '"', ':', 'f', 'a', 'l', 's', 'e', ',', '"', 's', 'u', 'p', 'p',
223+
'o', 'r', 't', 's', 'F', 'I', 'P', 'S', '"', ':', 't', 'r', 'u', 'e', '}', ',', '"', 'r', 'e', 'g',
224+
'i', 'o', 'n', 'R', 'e', 'g', 'e', 'x', '"', ':', '"', '^', 'u', 's', '\\', '\\', '-', 'i', 's', 'o',
225+
'f', '\\', '\\', '-', '\\', '\\', 'w', '+', '\\', '\\', '-', '\\', '\\', 'd', '+', '$', '"', ',', '"', 'r',
226+
'e', 'g', 'i', 'o', 'n', 's', '"', ':', '{', '"', 'a', 'w', 's', '-', 'i', 's', 'o', '-', 'f', '-',
227+
'g', 'l', 'o', 'b', 'a', 'l', '"', ':', '{', '"', 'd', 'e', 's', 'c', 'r', 'i', 'p', 't', 'i', 'o',
228+
'n', '"', ':', '"', 'A', 'W', 'S', ' ', 'I', 'S', 'O', 'F', ' ', 'g', 'l', 'o', 'b', 'a', 'l', ' ',
229+
'r', 'e', 'g', 'i', 'o', 'n', '"', '}', ',', '"', 'u', 's', '-', 'i', 's', 'o', 'f', '-', 'e', 'a',
230+
's', 't', '-', '1', '"', ':', '{', '"', 'd', 'e', 's', 'c', 'r', 'i', 'p', 't', 'i', 'o', 'n', '"',
231+
':', '"', 'U', 'S', ' ', 'I', 'S', 'O', 'F', ' ', 'E', 'A', 'S', 'T', '"', '}', ',', '"', 'u', 's',
232+
'-', 'i', 's', 'o', 'f', '-', 's', 'o', 'u', 't', 'h', '-', '1', '"', ':', '{', '"', 'd', 'e', 's',
233+
'c', 'r', 'i', 'p', 't', 'i', 'o', 'n', '"', ':', '"', 'U', 'S', ' ', 'I', 'S', 'O', 'F', ' ', 'S',
234+
'O', 'U', 'T', 'H', '"', '}', '}', '}', ',', '{', '"', 'i', 'd', '"', ':', '"', 'a', 'w', 's', '-',
235+
'e', 'u', 's', 'c', '"', ',', '"', 'o', 'u', 't', 'p', 'u', 't', 's', '"', ':', '{', '"', 'd', 'n',
236+
's', 'S', 'u', 'f', 'f', 'i', 'x', '"', ':', '"', 'a', 'm', 'a', 'z', 'o', 'n', 'a', 'w', 's', '.',
237+
'e', 'u', '"', ',', '"', 'd', 'u', 'a', 'l', 'S', 't', 'a', 'c', 'k', 'D', 'n', 's', 'S', 'u', 'f',
238+
'f', 'i', 'x', '"', ':', '"', 'a', 'm', 'a', 'z', 'o', 'n', 'a', 'w', 's', '.', 'e', 'u', '"', ',',
215239
'"', 'i', 'm', 'p', 'l', 'i', 'c', 'i', 't', 'G', 'l', 'o', 'b', 'a', 'l', 'R', 'e', 'g', 'i', 'o',
216-
'n', '"', ':', '"', 'u', 's', '-', 'i', 's', 'o', 'f', '-', 's', 'o', 'u', 't', 'h', '-', '1', '"',
217-
',', '"', 'n', 'a', 'm', 'e', '"', ':', '"', 'a', 'w', 's', '-', 'i', 's', 'o', '-', 'f', '"', ',',
218-
'"', 's', 'u', 'p', 'p', 'o', 'r', 't', 's', 'D', 'u', 'a', 'l', 'S', 't', 'a', 'c', 'k', '"', ':',
219-
'f', 'a', 'l', 's', 'e', ',', '"', 's', 'u', 'p', 'p', 'o', 'r', 't', 's', 'F', 'I', 'P', 'S', '"',
220-
':', 't', 'r', 'u', 'e', '}', ',', '"', 'r', 'e', 'g', 'i', 'o', 'n', 'R', 'e', 'g', 'e', 'x', '"',
221-
':', '"', '^', 'u', 's', '\\', '\\', '-', 'i', 's', 'o', 'f', '\\', '\\', '-', '\\', '\\', 'w', '+', '\\',
240+
'n', '"', ':', '"', 'e', 'u', 's', 'c', '-', 'd', 'e', '-', 'e', 'a', 's', 't', '-', '1', '"', ',',
241+
'"', 'n', 'a', 'm', 'e', '"', ':', '"', 'a', 'w', 's', '-', 'e', 'u', 's', 'c', '"', ',', '"', 's',
242+
'u', 'p', 'p', 'o', 'r', 't', 's', 'D', 'u', 'a', 'l', 'S', 't', 'a', 'c', 'k', '"', ':', 'f', 'a',
243+
'l', 's', 'e', ',', '"', 's', 'u', 'p', 'p', 'o', 'r', 't', 's', 'F', 'I', 'P', 'S', '"', ':', 't',
244+
'r', 'u', 'e', '}', ',', '"', 'r', 'e', 'g', 'i', 'o', 'n', 'R', 'e', 'g', 'e', 'x', '"', ':', '"',
245+
'^', 'e', 'u', 's', 'c', '\\', '\\', '-', '(', 'd', 'e', ')', '\\', '\\', '-', '\\', '\\', 'w', '+', '\\',
222246
'\\', '-', '\\', '\\', 'd', '+', '$', '"', ',', '"', 'r', 'e', 'g', 'i', 'o', 'n', 's', '"', ':', '{',
223-
'"', 'a', 'w', 's', '-', 'i', 's', 'o', '-', 'f', '-', 'g', 'l', 'o', 'b', 'a', 'l', '"', ':', '{',
224-
'"', 'd', 'e', 's', 'c', 'r', 'i', 'p', 't', 'i', 'o', 'n', '"', ':', '"', 'A', 'W', 'S', ' ', 'I',
225-
'S', 'O', 'F', ' ', 'g', 'l', 'o', 'b', 'a', 'l', ' ', 'r', 'e', 'g', 'i', 'o', 'n', '"', '}', ',',
226-
'"', 'u', 's', '-', 'i', 's', 'o', 'f', '-', 'e', 'a', 's', 't', '-', '1', '"', ':', '{', '"', 'd',
227-
'e', 's', 'c', 'r', 'i', 'p', 't', 'i', 'o', 'n', '"', ':', '"', 'U', 'S', ' ', 'I', 'S', 'O', 'F',
228-
' ', 'E', 'A', 'S', 'T', '"', '}', ',', '"', 'u', 's', '-', 'i', 's', 'o', 'f', '-', 's', 'o', 'u',
229-
't', 'h', '-', '1', '"', ':', '{', '"', 'd', 'e', 's', 'c', 'r', 'i', 'p', 't', 'i', 'o', 'n', '"',
230-
':', '"', 'U', 'S', ' ', 'I', 'S', 'O', 'F', ' ', 'S', 'O', 'U', 'T', 'H', '"', '}', '}', '}', ']',
231-
',', '"', 'v', 'e', 'r', 's', 'i', 'o', 'n', '"', ':', '"', '1', '.', '1', '"', '}'};
247+
'"', 'e', 'u', 's', 'c', '-', 'd', 'e', '-', 'e', 'a', 's', 't', '-', '1', '"', ':', '{', '"', 'd',
248+
'e', 's', 'c', 'r', 'i', 'p', 't', 'i', 'o', 'n', '"', ':', '"', 'E', 'U', ' ', '(', 'G', 'e', 'r',
249+
'm', 'a', 'n', 'y', ')', '"', '}', '}', '}', ']', ',', '"', 'v', 'e', 'r', 's', 'i', 'o', 'n', '"',
250+
':', '"', '1', '.', '1', '"', '}'};
232251

233252
const struct aws_byte_cursor aws_s3_endpoint_resolver_partitions = {
234-
.len = 4357,
253+
.len = 4727,
235254
.ptr = (uint8_t *) s_generated_array
236255
};

0 commit comments

Comments
 (0)