Skip to content

Commit e0fe6d8

Browse files
committed
in case of retry, let's record the succeed only metrics as well
1 parent 2e900b4 commit e0fe6d8

File tree

3 files changed

+25
-11
lines changed

3 files changed

+25
-11
lines changed

tests/s3_data_plane_tests.c

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2029,7 +2029,7 @@ static int s_test_s3_get_object_part(struct aws_allocator *allocator, void *ctx)
20292029

20302030
ASSERT_UINT_EQUALS(AWS_ERROR_SUCCESS, meta_request_test_results.finished_error_code);
20312031
/* Only one request was made to get the second part of the object */
2032-
ASSERT_UINT_EQUALS(1, aws_array_list_length(&meta_request_test_results.synced_data.metrics));
2032+
ASSERT_UINT_EQUALS(1, aws_array_list_length(&meta_request_test_results.synced_data.succeed_metrics));
20332033

20342034
aws_s3_meta_request_test_results_clean_up(&meta_request_test_results);
20352035

@@ -4997,7 +4997,7 @@ static int s_check_metrics_helper(
49974997
struct aws_s3_request_metrics *metrics = NULL;
49984998

49994999
/* First metrics should be the CreateMPU */
5000-
ASSERT_SUCCESS(aws_array_list_get_at(&test_results->synced_data.metrics, (void **)&metrics, index));
5000+
ASSERT_SUCCESS(aws_array_list_get_at(&test_results->synced_data.succeed_metrics, (void **)&metrics, index));
50015001
enum aws_s3_request_type out_request_type = AWS_S3_REQUEST_TYPE_UNKNOWN;
50025002
aws_s3_request_metrics_get_request_type(metrics, &out_request_type);
50035003
uint32_t part_number = 0;
@@ -5087,7 +5087,7 @@ static int s_test_s3_round_trip_dynamic_range_size_download_multipart(struct aws
50875087
ASSERT_TRUE(test_results.did_validate);
50885088
ASSERT_UINT_EQUALS(AWS_SCA_CRC32, test_results.validation_algorithm);
50895089
/* The tests has been done, we are safe to touch the synced data from test results. */
5090-
ASSERT_UINT_EQUALS(3, aws_array_list_length(&test_results.synced_data.metrics));
5090+
ASSERT_UINT_EQUALS(3, aws_array_list_length(&test_results.synced_data.succeed_metrics));
50915091
/* First request made was head object and the range should be 0 */
50925092
ASSERT_SUCCESS(s_check_metrics_helper(&test_results, 0, AWS_S3_REQUEST_TYPE_HEAD_OBJECT, 0, 0, 0));
50935093
/* Second request made should be get with range and range from 0 to stored part size -1. */
@@ -5118,7 +5118,7 @@ static int s_test_s3_round_trip_dynamic_range_size_download_multipart(struct aws
51185118
aws_s3_tester_send_meta_request_with_options(&tester, &get_options_without_checksum, &test_results));
51195119
ASSERT_FALSE(test_results.did_validate);
51205120
/* The tests has been done, we are safe to touch the synced data from test results. */
5121-
ASSERT_UINT_EQUALS(3, aws_array_list_length(&test_results.synced_data.metrics));
5121+
ASSERT_UINT_EQUALS(3, aws_array_list_length(&test_results.synced_data.succeed_metrics));
51225122
/* First request made was Get object and the range should be 0 to default range - 1 */
51235123
ASSERT_SUCCESS(s_check_metrics_helper(
51245124
&test_results, 0, AWS_S3_REQUEST_TYPE_GET_OBJECT, 1, 0, (size_t)g_default_part_size_fallback - 1));
@@ -5153,7 +5153,7 @@ static int s_test_s3_round_trip_dynamic_range_size_download_multipart(struct aws
51535153
ASSERT_FALSE(test_results.did_validate);
51545154
/* The tests has been done, we are safe to touch the synced data from test results. */
51555155
/* 4 parts in total for 30MiB with 8MiB part */
5156-
ASSERT_UINT_EQUALS(4, aws_array_list_length(&test_results.synced_data.metrics));
5156+
ASSERT_UINT_EQUALS(4, aws_array_list_length(&test_results.synced_data.succeed_metrics));
51575157
aws_s3_meta_request_test_results_clean_up(&test_results);
51585158
}
51595159
/* TODO: The MPU file still works with dynamic part size */
@@ -5245,7 +5245,7 @@ static int s_test_s3_round_trip_dynamic_range_size_download_single_part(struct a
52455245
ASSERT_TRUE(test_results.did_validate);
52465246
ASSERT_UINT_EQUALS(AWS_SCA_CRC32, test_results.validation_algorithm);
52475247
/* The tests has been done, we are safe to touch the synced data from test results. */
5248-
ASSERT_UINT_EQUALS(2, aws_array_list_length(&test_results.synced_data.metrics));
5248+
ASSERT_UINT_EQUALS(2, aws_array_list_length(&test_results.synced_data.succeed_metrics));
52495249
/* First request made was head object and the range should be 0 */
52505250
ASSERT_SUCCESS(s_check_metrics_helper(&test_results, 0, AWS_S3_REQUEST_TYPE_HEAD_OBJECT, 0, 0, 0));
52515251
/* Second request made should be get with range and range from 0 to optimal part size. */
@@ -5274,7 +5274,7 @@ static int s_test_s3_round_trip_dynamic_range_size_download_single_part(struct a
52745274
aws_s3_tester_send_meta_request_with_options(&tester, &get_options_without_checksum, &test_results));
52755275
ASSERT_FALSE(test_results.did_validate);
52765276
/* The tests has been done, we are safe to touch the synced data from test results. */
5277-
ASSERT_UINT_EQUALS(2, aws_array_list_length(&test_results.synced_data.metrics));
5277+
ASSERT_UINT_EQUALS(2, aws_array_list_length(&test_results.synced_data.succeed_metrics));
52785278
/* First request made was Get object and the range should be 0 to default range - 1 */
52795279
ASSERT_SUCCESS(s_check_metrics_helper(
52805280
&test_results, 0, AWS_S3_REQUEST_TYPE_GET_OBJECT, 1, 0, (size_t)g_default_part_size_fallback - 1));
@@ -5308,7 +5308,7 @@ static int s_test_s3_round_trip_dynamic_range_size_download_single_part(struct a
53085308
ASSERT_FALSE(test_results.did_validate);
53095309
/* The tests has been done, we are safe to touch the synced data from test results. */
53105310
/* 4 parts in total for 30MiB with 8MiB part */
5311-
ASSERT_UINT_EQUALS(4, aws_array_list_length(&test_results.synced_data.metrics));
5311+
ASSERT_UINT_EQUALS(4, aws_array_list_length(&test_results.synced_data.succeed_metrics));
53125312
aws_s3_meta_request_test_results_clean_up(&test_results);
53135313
}
53145314
/* TODO: The MPU file still works with dynamic part size */
@@ -5496,9 +5496,9 @@ static int s_test_s3_meta_request_default(struct aws_allocator *allocator, void
54965496

54975497
/* Check the size of the metrics should be the same as the number of
54985498
requests, which should be 1 */
5499-
ASSERT_UINT_EQUALS(1, aws_array_list_length(&meta_request_test_results.synced_data.metrics));
5499+
ASSERT_UINT_EQUALS(1, aws_array_list_length(&meta_request_test_results.synced_data.succeed_metrics));
55005500
struct aws_s3_request_metrics *metrics = NULL;
5501-
aws_array_list_back(&meta_request_test_results.synced_data.metrics, (void **)&metrics);
5501+
aws_array_list_back(&meta_request_test_results.synced_data.succeed_metrics, (void **)&metrics);
55025502

55035503
ASSERT_SUCCESS(aws_s3_tester_validate_get_object_results(&meta_request_test_results, 0));
55045504

tests/s3_tester.c

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ static void s_s3_test_meta_request_telemetry(
251251
aws_s3_request_metrics_get_request_type(metrics, &request_type);
252252
uint32_t retry_attempt = aws_s3_request_metrics_get_retry_attempt(metrics);
253253

254-
if (aws_s3_request_metrics_get_error_code(metrics) == 200 && retry_attempt == 0 &&
254+
if (aws_s3_request_metrics_get_error_code(metrics) == AWS_ERROR_SUCCESS && retry_attempt == 0 &&
255255
(request_type == AWS_S3_REQUEST_TYPE_GET_OBJECT || request_type == AWS_S3_REQUEST_TYPE_UPLOAD_PART)) {
256256
uint64_t start_time = 0;
257257
uint64_t end_time = 0;
@@ -266,6 +266,9 @@ static void s_s3_test_meta_request_telemetry(
266266

267267
aws_s3_tester_lock_synced_data(tester);
268268
aws_array_list_push_back(&meta_request_test_results->synced_data.metrics, &metrics);
269+
if (aws_s3_request_metrics_get_error_code(metrics) == AWS_ERROR_SUCCESS) {
270+
aws_array_list_push_back(&meta_request_test_results->synced_data.succeed_metrics, &metrics);
271+
}
269272
aws_s3_request_metrics_acquire(metrics);
270273
aws_s3_tester_unlock_synced_data(tester);
271274
}
@@ -560,6 +563,8 @@ void aws_s3_meta_request_test_results_init(
560563
aws_atomic_init_int(&test_meta_request->received_body_size_delta, 0);
561564
aws_array_list_init_dynamic(
562565
&test_meta_request->synced_data.metrics, allocator, 4, sizeof(struct aws_s3_request_metrics *));
566+
aws_array_list_init_dynamic(
567+
&test_meta_request->synced_data.succeed_metrics, allocator, 4, sizeof(struct aws_s3_request_metrics *));
563568
}
564569

565570
void aws_s3_meta_request_test_results_clean_up(struct aws_s3_meta_request_test_results *test_meta_request) {
@@ -577,6 +582,13 @@ void aws_s3_meta_request_test_results_clean_up(struct aws_s3_meta_request_test_r
577582
aws_s3_request_metrics_release(metrics);
578583
}
579584
aws_array_list_clean_up(&test_meta_request->synced_data.metrics);
585+
while (aws_array_list_length(&test_meta_request->synced_data.succeed_metrics) > 0) {
586+
struct aws_s3_request_metrics *metrics = NULL;
587+
aws_array_list_back(&test_meta_request->synced_data.succeed_metrics, (void **)&metrics);
588+
aws_array_list_pop_back(&test_meta_request->synced_data.succeed_metrics);
589+
aws_s3_request_metrics_release(metrics);
590+
}
591+
aws_array_list_clean_up(&test_meta_request->synced_data.succeed_metrics);
580592

581593
for (size_t i = 0; i < test_meta_request->upload_review.part_count; ++i) {
582594
aws_string_destroy(test_meta_request->upload_review.part_checksums_array[i]);

tests/s3_tester.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,8 @@ struct aws_s3_meta_request_test_results {
286286
struct {
287287
/* The array_list of `struct aws_s3_request_metrics *` */
288288
struct aws_array_list metrics;
289+
/* The array_list of `struct aws_s3_request_metrics *` that the request succeed, to avoid retries counts. */
290+
struct aws_array_list succeed_metrics;
289291
} synced_data;
290292

291293
/* record data from the upload_review_callback */

0 commit comments

Comments
 (0)