diff --git a/include/aws/io/retry_strategy.h b/include/aws/io/retry_strategy.h index 8f15d207f..3986db4d4 100644 --- a/include/aws/io/retry_strategy.h +++ b/include/aws/io/retry_strategy.h @@ -112,6 +112,8 @@ struct aws_exponential_backoff_retry_options { size_t max_retries; /** Scaling factor to add for the backoff. Default is 25ms */ uint32_t backoff_scale_factor_ms; + /** Max retry backoff in seconds. Default is 20 seconds */ + uint32_t max_backoff_secs; /** Jitter mode to use, see comments for aws_exponential_backoff_jitter_mode. * Default is AWS_EXPONENTIAL_BACKOFF_JITTER_DEFAULT */ enum aws_exponential_backoff_jitter_mode jitter_mode; diff --git a/source/exponential_backoff_retry_strategy.c b/source/exponential_backoff_retry_strategy.c index 298cca9af..0b2742e65 100644 --- a/source/exponential_backoff_retry_strategy.c +++ b/source/exponential_backoff_retry_strategy.c @@ -26,6 +26,7 @@ struct exponential_backoff_retry_token { struct aws_atomic_var last_backoff; size_t max_retries; uint64_t backoff_scale_factor_ns; + uint64_t maximum_backoff_ns; enum aws_exponential_backoff_jitter_mode jitter_mode; /* Let's not make this worse by constantly moving across threads if we can help it */ struct aws_event_loop *bound_loop; @@ -139,6 +140,8 @@ static int s_exponential_retry_acquire_token( backoff_retry_token->max_retries = exponential_backoff_strategy->config.max_retries; backoff_retry_token->backoff_scale_factor_ns = aws_timestamp_convert( exponential_backoff_strategy->config.backoff_scale_factor_ms, AWS_TIMESTAMP_MILLIS, AWS_TIMESTAMP_NANOS, NULL); + backoff_retry_token->maximum_backoff_ns = aws_timestamp_convert( + exponential_backoff_strategy->config.max_backoff_secs, AWS_TIMESTAMP_SECS, AWS_TIMESTAMP_NANOS, NULL); backoff_retry_token->jitter_mode = exponential_backoff_strategy->config.jitter_mode; backoff_retry_token->generate_random = exponential_backoff_strategy->config.generate_random; backoff_retry_token->generate_random_impl = exponential_backoff_strategy->config.generate_random_impl; @@ -184,7 +187,8 @@ typedef uint64_t(compute_backoff_fn)(struct exponential_backoff_retry_token *tok static uint64_t s_compute_no_jitter(struct exponential_backoff_retry_token *token) { uint64_t retry_count = aws_min_u64(aws_atomic_load_int(&token->current_retry_count), 63); - return aws_mul_u64_saturating((uint64_t)1 << retry_count, token->backoff_scale_factor_ns); + uint64_t backoff_ns = aws_mul_u64_saturating((uint64_t)1 << retry_count, token->backoff_scale_factor_ns); + return aws_min_u64(backoff_ns, token->maximum_backoff_ns); } static uint64_t s_compute_full_jitter(struct exponential_backoff_retry_token *token) { @@ -198,8 +202,8 @@ static uint64_t s_compute_deccorelated_jitter(struct exponential_backoff_retry_t if (!last_backoff_val) { return s_compute_full_jitter(token); } - - return s_random_in_range(token->backoff_scale_factor_ns, aws_mul_u64_saturating(last_backoff_val, 3), token); + uint64_t backoff_ns = aws_min_u64(token->maximum_backoff_ns, aws_mul_u64_saturating(last_backoff_val, 3)); + return s_random_in_range(token->backoff_scale_factor_ns, backoff_ns, token); } static compute_backoff_fn *s_backoff_compute_table[] = { @@ -372,6 +376,10 @@ struct aws_retry_strategy *aws_retry_strategy_new_exponential_backoff( exponential_backoff_strategy->config.backoff_scale_factor_ms = 25; } + if (!exponential_backoff_strategy->config.max_backoff_secs) { + exponential_backoff_strategy->config.max_backoff_secs = 20; + } + if (config->shutdown_options) { exponential_backoff_strategy->shutdown_options = *config->shutdown_options; } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index e59e7b22a..c75bb0a5d 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -280,6 +280,7 @@ add_test_case(test_exponential_backoff_retry_too_many_retries_decorrelated_jitte add_test_case(test_exponential_backoff_retry_too_many_retries_default_jitter) add_test_case(test_exponential_backoff_retry_client_errors_do_not_count) add_test_case(test_exponential_backoff_retry_no_jitter_time_taken) +add_test_case(test_exponential_max_backoff_retry_no_jitter) add_test_case(test_exponential_backoff_retry_invalid_options) add_test_case(test_standard_retry_strategy_setup_shutdown) diff --git a/tests/exponential_backoff_retry_test.c b/tests/exponential_backoff_retry_test.c index 2d5c53089..a3bf7bde0 100644 --- a/tests/exponential_backoff_retry_test.c +++ b/tests/exponential_backoff_retry_test.c @@ -247,6 +247,63 @@ AWS_TEST_CASE( test_exponential_backoff_retry_no_jitter_time_taken, s_test_exponential_backoff_retry_no_jitter_time_taken_fn) +/* Test that in no jitter mode, max exponential backoff is actually applied as documented. */ +static int s_test_exponential_max_backoff_retry_no_jitter_fn(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + aws_io_library_init(allocator); + + struct aws_event_loop_group *el_group = aws_event_loop_group_new_default(allocator, 1, NULL); + struct aws_exponential_backoff_retry_options config = { + .max_retries = 3, + .jitter_mode = AWS_EXPONENTIAL_BACKOFF_JITTER_NONE, + .el_group = el_group, + .backoff_scale_factor_ms = 1000, + .max_backoff_secs = 3, + }; + + struct aws_retry_strategy *retry_strategy = aws_retry_strategy_new_exponential_backoff(allocator, &config); + ASSERT_NOT_NULL(retry_strategy); + + struct exponential_backoff_test_data test_data = { + .retry_count = 0, + .failure_error_code = 0, + .mutex = AWS_MUTEX_INIT, + .cvar = AWS_CONDITION_VARIABLE_INIT, + }; + + uint64_t before_time = 0; + ASSERT_SUCCESS(aws_high_res_clock_get_ticks(&before_time)); + ASSERT_SUCCESS(aws_mutex_lock(&test_data.mutex)); + ASSERT_SUCCESS(aws_retry_strategy_acquire_retry_token( + retry_strategy, NULL, s_too_many_retries_test_token_acquired, &test_data, 0)); + + ASSERT_SUCCESS(aws_condition_variable_wait_pred(&test_data.cvar, &test_data.mutex, s_retry_has_failed, &test_data)); + aws_mutex_unlock(&test_data.mutex); + uint64_t after_time = 0; + ASSERT_SUCCESS(aws_high_res_clock_get_ticks(&after_time)); + uint64_t backoff_scale_factor = + aws_timestamp_convert(config.backoff_scale_factor_ms, AWS_TIMESTAMP_MILLIS, AWS_TIMESTAMP_NANOS, NULL); + uint64_t max_backoff_scale_factor = + aws_timestamp_convert(config.max_backoff_secs, AWS_TIMESTAMP_SECS, AWS_TIMESTAMP_NANOS, NULL); + + uint64_t expected_interval = aws_min_u64(max_backoff_scale_factor, 1 * backoff_scale_factor) + + aws_min_u64(max_backoff_scale_factor, 2 * backoff_scale_factor) + + aws_min_u64(max_backoff_scale_factor, 4 * backoff_scale_factor); + ASSERT_TRUE(expected_interval <= after_time - before_time); + + ASSERT_UINT_EQUALS(config.max_retries, test_data.retry_count); + ASSERT_UINT_EQUALS(AWS_IO_MAX_RETRIES_EXCEEDED, test_data.failure_error_code); + + aws_retry_strategy_release(retry_strategy); + aws_event_loop_group_release(el_group); + + aws_io_library_clean_up(); + + return AWS_OP_SUCCESS; +} +AWS_TEST_CASE(test_exponential_max_backoff_retry_no_jitter, s_test_exponential_max_backoff_retry_no_jitter_fn) + /* verify that invalid options cause a failure at creation time. */ static int s_test_exponential_backoff_retry_invalid_options_fn(struct aws_allocator *allocator, void *ctx) { (void)ctx;