Commit 585e9bc7 authored by Mike Starr's avatar Mike Starr Committed by Facebook Github Bot

Make folly::retrying exponential backoff time behavior more consistent

Summary:
If the value for backoff_max is less than backoff_min, we will see inconsistent behavior where the backoff_min sleep duration is used for the first 57 attempts, then backoff_max will be used.

In our case, we hit an issue where 0 was passed for backoff_max. We would see backoff_min sleep times for the first 57 attempts, then immediately drop to 0 sleep time which led to no retry wait at all.

If backoff_max < backoff_min, there's no way to respect both, but this makes the behavior consistent across all tries.

Reviewed By: yfeldblum

Differential Revision: D15282681

fbshipit-source-id: d9dd7b2498d77e4a01c9454a639b612ed9e26519
parent a88cfa0b
...@@ -148,7 +148,7 @@ Duration retryingJitteredExponentialBackoffDur( ...@@ -148,7 +148,7 @@ Duration retryingJitteredExponentialBackoffDur(
auto jitter = std::exp(dist(rng)); auto jitter = std::exp(dist(rng));
auto backoff_rep = jitter * backoff_min.count() * std::pow(2, n - 1); auto backoff_rep = jitter * backoff_min.count() * std::pow(2, n - 1);
if (UNLIKELY(backoff_rep >= std::numeric_limits<Duration::rep>::max())) { if (UNLIKELY(backoff_rep >= std::numeric_limits<Duration::rep>::max())) {
return backoff_max; return std::max(backoff_min, backoff_max);
} }
auto backoff = Duration(Duration::rep(backoff_rep)); auto backoff = Duration(Duration::rep(backoff_rep));
return std::max(backoff_min, std::min(backoff_max, backoff)); return std::max(backoff_min, std::min(backoff_max, backoff));
......
...@@ -236,6 +236,58 @@ TEST(RetryingTest, large_retries) { ...@@ -236,6 +236,58 @@ TEST(RetryingTest, large_retries) {
} }
} }
TEST(RetryingTest, retryingJitteredExponentialBackoffDur) {
mt19937_64 rng(0);
auto backoffMin = milliseconds(100);
auto backoffMax = milliseconds(1000);
EXPECT_EQ(
100,
futures::detail::retryingJitteredExponentialBackoffDur(
1, backoffMin, backoffMax, 0, rng)
.count());
EXPECT_EQ(
200,
futures::detail::retryingJitteredExponentialBackoffDur(
2, backoffMin, backoffMax, 0, rng)
.count());
EXPECT_EQ(
400,
futures::detail::retryingJitteredExponentialBackoffDur(
3, backoffMin, backoffMax, 0, rng)
.count());
EXPECT_EQ(
800,
futures::detail::retryingJitteredExponentialBackoffDur(
4, backoffMin, backoffMax, 0, rng)
.count());
EXPECT_EQ(
1000,
futures::detail::retryingJitteredExponentialBackoffDur(
5, backoffMin, backoffMax, 0, rng)
.count());
// Invalid usage: backoffMin > backoffMax
backoffMax = milliseconds(0);
EXPECT_EQ(
100,
futures::detail::retryingJitteredExponentialBackoffDur(
1, backoffMin, backoffMax, 0, rng)
.count());
EXPECT_EQ(
100,
futures::detail::retryingJitteredExponentialBackoffDur(
1000, backoffMin, backoffMax, 0, rng)
.count());
}
/* /*
TEST(RetryingTest, policy_sleep_cancel) { TEST(RetryingTest, policy_sleep_cancel) {
multiAttemptExpectDurationWithin(5, milliseconds(0), milliseconds(10), []{ multiAttemptExpectDurationWithin(5, milliseconds(0), milliseconds(10), []{
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment