From 0f91e513d9dc9bee529701ba254933eb7be07b38 Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Mon, 4 Dec 2017 16:12:54 -0800 Subject: Cleaned up API. Backoff now returns a single value: the time of the next retry --- .../client_channel/lb_policy/grpclb/grpclb.cc | 7 +- .../resolver/dns/c_ares/dns_resolver_ares.cc | 8 +- .../resolver/dns/native/dns_resolver.cc | 8 +- src/core/ext/filters/client_channel/subchannel.cc | 32 +++-- src/core/lib/backoff/backoff.cc | 27 ++-- src/core/lib/backoff/backoff.h | 25 ++-- test/core/backoff/backoff_test.cc | 144 +++++++++------------ 7 files changed, 112 insertions(+), 139 deletions(-) diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc index cc55925758..3c4e7d0270 100644 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc +++ b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc @@ -398,7 +398,7 @@ typedef struct glb_lb_policy { grpc_slice lb_call_status_details; /** LB call retry backoff state */ - grpc_core::ManualConstructor lb_call_backoff; + grpc_core::ManualConstructor lb_call_backoff; /** LB call retry timer */ grpc_timer lb_call_retry_timer; @@ -1291,8 +1291,7 @@ static void maybe_restart_lb_call(grpc_exec_ctx* exec_ctx, glb_policy->updating_lb_call = false; } else if (!glb_policy->shutting_down) { /* if we aren't shutting down, restart the LB client call after some time */ - grpc_millis next_try = - glb_policy->lb_call_backoff->Step(exec_ctx).next_attempt_start_time; + grpc_millis next_try = glb_policy->lb_call_backoff->Step(exec_ctx); if (grpc_lb_glb_trace.enabled()) { gpr_log(GPR_DEBUG, "[grpclb %p] Connection to LB server lost...", glb_policy); @@ -1461,7 +1460,7 @@ static void lb_call_init_locked(grpc_exec_ctx* exec_ctx, lb_on_response_received_locked, glb_policy, grpc_combiner_scheduler(glb_policy->base.combiner)); - grpc_core::Backoff::Options backoff_options; + grpc_core::BackOff::Options backoff_options; backoff_options .set_initial_backoff(GRPC_GRPCLB_INITIAL_CONNECT_BACKOFF_SECONDS * 1000) .set_multiplier(GRPC_GRPCLB_RECONNECT_BACKOFF_MULTIPLIER) diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc b/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc index ff6689b736..c3d0e87a83 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc @@ -90,7 +90,7 @@ typedef struct { bool have_retry_timer; grpc_timer retry_timer; /** retry backoff state */ - grpc_core::ManualConstructor backoff; + grpc_core::ManualConstructor backoff; /** currently resolving addresses */ grpc_lb_addresses* lb_addresses; @@ -273,7 +273,7 @@ static void dns_ares_on_resolved_locked(grpc_exec_ctx* exec_ctx, void* arg, } else { const char* msg = grpc_error_string(error); gpr_log(GPR_DEBUG, "dns resolution failed: %s", msg); - grpc_millis next_try = r->backoff->Step(exec_ctx).next_attempt_start_time; + grpc_millis next_try = r->backoff->Step(exec_ctx); grpc_millis timeout = next_try - grpc_exec_ctx_now(exec_ctx); gpr_log(GPR_INFO, "dns resolution failed (will retry): %s", grpc_error_string(error)); @@ -381,14 +381,14 @@ static grpc_resolver* dns_ares_create(grpc_exec_ctx* exec_ctx, grpc_pollset_set_add_pollset_set(exec_ctx, r->interested_parties, args->pollset_set); } - grpc_core::Backoff::Options backoff_options; + grpc_core::BackOff::Options backoff_options; backoff_options .set_initial_backoff(GRPC_DNS_INITIAL_CONNECT_BACKOFF_SECONDS * 1000) .set_multiplier(GRPC_DNS_RECONNECT_BACKOFF_MULTIPLIER) .set_jitter(GRPC_DNS_RECONNECT_JITTER) .set_min_connect_timeout(GRPC_DNS_MIN_CONNECT_TIMEOUT_SECONDS * 1000) .set_max_backoff(GRPC_DNS_RECONNECT_MAX_BACKOFF_SECONDS * 1000); - r->backoff.Init(grpc_core::Backoff(backoff_options)); + r->backoff.Init(grpc_core::BackOff(backoff_options)); GRPC_CLOSURE_INIT(&r->dns_ares_on_retry_timer_locked, dns_ares_on_retry_timer_locked, r, grpc_combiner_scheduler(r->base.combiner)); diff --git a/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc b/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc index 81f292f4d9..e1f5ca87d4 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc @@ -71,7 +71,7 @@ typedef struct { grpc_timer retry_timer; grpc_closure on_retry; /** retry backoff state */ - grpc_core::ManualConstructor backoff; + grpc_core::ManualConstructor backoff; /** currently resolving addresses */ grpc_resolved_addresses* addresses; @@ -171,7 +171,7 @@ static void dns_on_resolved_locked(grpc_exec_ctx* exec_ctx, void* arg, grpc_resolved_addresses_destroy(r->addresses); grpc_lb_addresses_destroy(exec_ctx, addresses); } else { - grpc_millis next_try = r->backoff->Step(exec_ctx).next_attempt_start_time; + grpc_millis next_try = r->backoff->Step(exec_ctx); grpc_millis timeout = next_try - grpc_exec_ctx_now(exec_ctx); gpr_log(GPR_INFO, "dns resolution failed (will retry): %s", grpc_error_string(error)); @@ -257,14 +257,14 @@ static grpc_resolver* dns_create(grpc_exec_ctx* exec_ctx, grpc_pollset_set_add_pollset_set(exec_ctx, r->interested_parties, args->pollset_set); } - grpc_core::Backoff::Options backoff_options; + grpc_core::BackOff::Options backoff_options; backoff_options .set_initial_backoff(GRPC_DNS_INITIAL_CONNECT_BACKOFF_SECONDS * 1000) .set_multiplier(GRPC_DNS_RECONNECT_BACKOFF_MULTIPLIER) .set_jitter(GRPC_DNS_RECONNECT_JITTER) .set_min_connect_timeout(GRPC_DNS_MIN_CONNECT_TIMEOUT_SECONDS * 1000) .set_max_backoff(GRPC_DNS_RECONNECT_MAX_BACKOFF_SECONDS * 1000); - r->backoff.Init(grpc_core::Backoff(backoff_options)); + r->backoff.Init(grpc_core::BackOff(backoff_options)); return &r->base; } diff --git a/src/core/ext/filters/client_channel/subchannel.cc b/src/core/ext/filters/client_channel/subchannel.cc index 945c237d2c..dbf66ae71a 100644 --- a/src/core/ext/filters/client_channel/subchannel.cc +++ b/src/core/ext/filters/client_channel/subchannel.cc @@ -20,7 +20,9 @@ #include #include -#include + +#include +#include #include #include @@ -49,7 +51,7 @@ #define GRPC_SUBCHANNEL_INITIAL_CONNECT_BACKOFF_SECONDS 1 #define GRPC_SUBCHANNEL_RECONNECT_BACKOFF_MULTIPLIER 1.6 -#define GRPC_SUBCHANNEL_RECONNECT_MIN_BACKOFF_SECONDS 20 +#define GRPC_SUBCHANNEL_RECONNECT_MIN_TIMEOUT_SECONDS 20 #define GRPC_SUBCHANNEL_RECONNECT_MAX_BACKOFF_SECONDS 120 #define GRPC_SUBCHANNEL_RECONNECT_JITTER 0.2 @@ -119,8 +121,8 @@ struct grpc_subchannel { external_state_watcher root_external_state_watcher; /** backoff state */ - grpc_core::ManualConstructor backoff; - grpc_core::Backoff::Result backoff_result; + grpc_core::ManualConstructor backoff; + grpc_millis next_attempt_deadline; /** do we have an active alarm? */ bool have_alarm; @@ -284,12 +286,12 @@ void grpc_subchannel_weak_unref(grpc_exec_ctx* exec_ctx, } } -static grpc_core::Backoff::Options extract_backoff_options( +static grpc_core::BackOff::Options extract_backoff_options( const grpc_channel_args* args) { int initial_backoff_ms = GRPC_SUBCHANNEL_INITIAL_CONNECT_BACKOFF_SECONDS * 1000; int min_connect_timeout_ms = - GRPC_SUBCHANNEL_RECONNECT_MIN_BACKOFF_SECONDS * 1000; + GRPC_SUBCHANNEL_RECONNECT_MIN_TIMEOUT_SECONDS * 1000; int max_backoff_ms = GRPC_SUBCHANNEL_RECONNECT_MAX_BACKOFF_SECONDS * 1000; bool fixed_reconnect_backoff = false; if (args != nullptr) { @@ -318,7 +320,7 @@ static grpc_core::Backoff::Options extract_backoff_options( } } } - grpc_core::Backoff::Options backoff_options; + grpc_core::BackOff::Options backoff_options; backoff_options.set_initial_backoff(initial_backoff_ms) .set_multiplier(fixed_reconnect_backoff ? 1.0 @@ -392,7 +394,7 @@ static void continue_connect_locked(grpc_exec_ctx* exec_ctx, grpc_connect_in_args args; args.interested_parties = c->pollset_set; - args.deadline = c->backoff_result.current_deadline; + args.deadline = c->next_attempt_deadline; args.channel_args = c->args; grpc_connectivity_state_set(exec_ctx, &c->state_tracker, @@ -440,7 +442,11 @@ static void on_alarm(grpc_exec_ctx* exec_ctx, void* arg, grpc_error* error) { } if (error == GRPC_ERROR_NONE) { gpr_log(GPR_INFO, "Failed to connect to channel, retrying"); - c->backoff_result = c->backoff->Step(exec_ctx); + const grpc_millis min_deadline = + (GRPC_SUBCHANNEL_RECONNECT_MIN_TIMEOUT_SECONDS * 1000) + + grpc_exec_ctx_now(exec_ctx); + c->next_attempt_deadline = + std::max(c->backoff->Step(exec_ctx), min_deadline); continue_connect_locked(exec_ctx, c); gpr_mu_unlock(&c->mu); } else { @@ -477,21 +483,21 @@ static void maybe_start_connecting_locked(grpc_exec_ctx* exec_ctx, if (!c->backoff_begun) { c->backoff_begun = true; - c->backoff_result = c->backoff->Begin(exec_ctx); + c->next_attempt_deadline = c->backoff->Begin(exec_ctx); continue_connect_locked(exec_ctx, c); } else { GPR_ASSERT(!c->have_alarm); c->have_alarm = true; const grpc_millis time_til_next = - c->backoff_result.next_attempt_start_time - grpc_exec_ctx_now(exec_ctx); + c->next_attempt_deadline - grpc_exec_ctx_now(exec_ctx); if (time_til_next <= 0) { gpr_log(GPR_INFO, "Retry immediately"); } else { gpr_log(GPR_INFO, "Retry in %" PRIdPTR " milliseconds", time_til_next); } GRPC_CLOSURE_INIT(&c->on_alarm, on_alarm, c, grpc_schedule_on_exec_ctx); - grpc_timer_init(exec_ctx, &c->alarm, - c->backoff_result.next_attempt_start_time, &c->on_alarm); + grpc_timer_init(exec_ctx, &c->alarm, c->next_attempt_deadline, + &c->on_alarm); } } diff --git a/src/core/lib/backoff/backoff.cc b/src/core/lib/backoff/backoff.cc index a5128a7e6e..5553e6b3f3 100644 --- a/src/core/lib/backoff/backoff.cc +++ b/src/core/lib/backoff/backoff.cc @@ -29,8 +29,9 @@ namespace { /* Generate a random number between 0 and 1. We roll our own RNG because seeding * rand() modifies a global variable we have no control over. */ double generate_uniform_random_number(uint32_t* rng_state) { - *rng_state = (1103515245 * *rng_state + 12345) % ((uint32_t)1 << 31); - return *rng_state / (double)((uint32_t)1 << 31); + constexpr uint32_t two_raise_31 = uint32_t(1) << 31; + *rng_state = (1103515245 * *rng_state + 12345) % two_raise_31; + return *rng_state / static_cast(two_raise_31); } double generate_uniform_random_number_between(uint32_t* rng_state, double a, @@ -42,35 +43,29 @@ double generate_uniform_random_number_between(uint32_t* rng_state, double a, } } // namespace -Backoff::Backoff(const Options& options) : options_(options) { - rng_state_ = (unsigned int)gpr_now(GPR_CLOCK_REALTIME).tv_nsec; +BackOff::BackOff(const Options& options) : options_(options) { + rng_state_ = static_cast(gpr_now(GPR_CLOCK_REALTIME).tv_nsec); } -Backoff::Result Backoff::Begin(grpc_exec_ctx* exec_ctx) { +grpc_millis BackOff::Begin(grpc_exec_ctx* exec_ctx) { current_backoff_ = options_.initial_backoff(); - const grpc_millis initial_timeout = - std::max(options_.initial_backoff(), options_.min_connect_timeout()); - const grpc_millis now = grpc_exec_ctx_now(exec_ctx); - return Backoff::Result{now + initial_timeout, now + current_backoff_}; + return current_backoff_ + grpc_exec_ctx_now(exec_ctx); } -Backoff::Result Backoff::Step(grpc_exec_ctx* exec_ctx) { +grpc_millis BackOff::Step(grpc_exec_ctx* exec_ctx) { current_backoff_ = (grpc_millis)(std::min(current_backoff_ * options_.multiplier(), (double)options_.max_backoff())); const double jitter = generate_uniform_random_number_between( &rng_state_, -options_.jitter() * current_backoff_, options_.jitter() * current_backoff_); - const grpc_millis current_timeout = std::max( - (grpc_millis)(current_backoff_ + jitter), options_.min_connect_timeout()); const grpc_millis next_timeout = std::min( (grpc_millis)(current_backoff_ + jitter), options_.max_backoff()); - const grpc_millis now = grpc_exec_ctx_now(exec_ctx); - return Backoff::Result{now + current_timeout, now + next_timeout}; + return next_timeout + grpc_exec_ctx_now(exec_ctx); } -void Backoff::Reset() { current_backoff_ = options_.initial_backoff(); } +void BackOff::Reset() { current_backoff_ = options_.initial_backoff(); } -void Backoff::SetRandomSeed(uint32_t seed) { rng_state_ = seed; } +void BackOff::SetRandomSeed(uint32_t seed) { rng_state_ = seed; } } // namespace grpc_core diff --git a/src/core/lib/backoff/backoff.h b/src/core/lib/backoff/backoff.h index 3e5241c226..5ba05e1d75 100644 --- a/src/core/lib/backoff/backoff.h +++ b/src/core/lib/backoff/backoff.h @@ -25,20 +25,19 @@ namespace grpc_core { /// Implementation of the backoff mechanism described in /// doc/connection-backoff.md -class Backoff { +class BackOff { public: class Options; - struct Result; /// Initialize backoff machinery - does not need to be destroyed - explicit Backoff(const Options& options); + explicit BackOff(const Options& options); - /// Begin retry loop: returns the deadlines to be used for the current attempt - /// and the subsequent retry, if any. - Result Begin(grpc_exec_ctx* exec_ctx); - /// Step a retry loop: returns the deadlines to be used for the current - /// attempt and the subsequent retry, if any. - Result Step(grpc_exec_ctx* exec_ctx); + /// Begin retry loop: returns the deadline to be used for the next attempt, + /// following the backoff / strategy. + grpc_millis Begin(grpc_exec_ctx* exec_ctx); + /// Step a retry loop: returns returns the deadline to be used for the next + /// attempt, / following the backoff / strategy. + grpc_millis Step(grpc_exec_ctx* exec_ctx); /// Reset the backoff, so the next grpc_backoff_step will be a /// grpc_backoff_begin. void Reset(); @@ -86,14 +85,6 @@ class Backoff { grpc_millis max_backoff_; }; // class Options - struct Result { - /// Deadline to be used for the current attempt. - grpc_millis current_deadline; - /// Deadline to be used for the next attempt, following the backoff - /// strategy. - grpc_millis next_attempt_start_time; - }; - private: const Options options_; /// current delay before retries diff --git a/test/core/backoff/backoff_test.cc b/test/core/backoff/backoff_test.cc index 478d3b325d..e0b72cfce0 100644 --- a/test/core/backoff/backoff_test.cc +++ b/test/core/backoff/backoff_test.cc @@ -18,6 +18,8 @@ #include "src/core/lib/backoff/backoff.h" +#include + #include #include @@ -28,7 +30,7 @@ namespace grpc { namespace testing { namespace { -using grpc_core::Backoff; +using grpc_core::BackOff; TEST(BackOffTest, ConstantBackOff) { const grpc_millis initial_backoff = 200; @@ -36,29 +38,23 @@ TEST(BackOffTest, ConstantBackOff) { const double jitter = 0.0; const grpc_millis min_connect_timeout = 100; const grpc_millis max_backoff = 1000; - Backoff::Options options; + BackOff::Options options; options.set_initial_backoff(initial_backoff) .set_multiplier(multiplier) .set_jitter(jitter) .set_min_connect_timeout(min_connect_timeout) .set_max_backoff(max_backoff); - Backoff backoff(options); + BackOff backoff(options); grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; - Backoff::Result next_deadlines = backoff.Begin(&exec_ctx); - GPR_ASSERT(next_deadlines.current_deadline - grpc_exec_ctx_now(&exec_ctx) == - initial_backoff); - GPR_ASSERT(next_deadlines.next_attempt_start_time - - grpc_exec_ctx_now(&exec_ctx) == - initial_backoff); + grpc_millis next_attempt_start_time = backoff.Begin(&exec_ctx); + EXPECT_EQ(next_attempt_start_time - grpc_exec_ctx_now(&exec_ctx), + initial_backoff); for (int i = 0; i < 10000; i++) { - next_deadlines = backoff.Step(&exec_ctx); - GPR_ASSERT(next_deadlines.current_deadline - grpc_exec_ctx_now(&exec_ctx) == - initial_backoff); - GPR_ASSERT(next_deadlines.next_attempt_start_time - - grpc_exec_ctx_now(&exec_ctx) == - initial_backoff); - exec_ctx.now = next_deadlines.current_deadline; + next_attempt_start_time = backoff.Step(&exec_ctx); + EXPECT_EQ(next_attempt_start_time - grpc_exec_ctx_now(&exec_ctx), + initial_backoff); + exec_ctx.now = next_attempt_start_time; } grpc_exec_ctx_finish(&exec_ctx); } @@ -69,23 +65,16 @@ TEST(BackOffTest, MinConnect) { const double jitter = 0.0; const grpc_millis min_connect_timeout = 200; const grpc_millis max_backoff = 1000; - Backoff::Options options; + BackOff::Options options; options.set_initial_backoff(initial_backoff) .set_multiplier(multiplier) .set_jitter(jitter) .set_min_connect_timeout(min_connect_timeout) .set_max_backoff(max_backoff); - Backoff backoff(options); + BackOff backoff(options); grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; - Backoff::Result next = backoff.Begin(&exec_ctx); - // Because the min_connect_timeout > initial_backoff, current_deadline is used - // as the deadline for the current attempt. - GPR_ASSERT(next.current_deadline - grpc_exec_ctx_now(&exec_ctx) == - min_connect_timeout); - // ... while, if the current attempt fails, the next one will happen after - // initial_backoff. - GPR_ASSERT(next.next_attempt_start_time - grpc_exec_ctx_now(&exec_ctx) == - initial_backoff); + grpc_millis next = backoff.Begin(&exec_ctx); + EXPECT_EQ(next - grpc_exec_ctx_now(&exec_ctx), initial_backoff); grpc_exec_ctx_finish(&exec_ctx); } @@ -95,57 +84,55 @@ TEST(BackOffTest, NoJitterBackOff) { const double jitter = 0.0; const grpc_millis min_connect_timeout = 1; const grpc_millis max_backoff = 513; - Backoff::Options options; + BackOff::Options options; options.set_initial_backoff(initial_backoff) .set_multiplier(multiplier) .set_jitter(jitter) .set_min_connect_timeout(min_connect_timeout) .set_max_backoff(max_backoff); - Backoff backoff(options); + BackOff backoff(options); // x_1 = 2 // x_n = 2**i + x_{i-1} ( = 2**(n+1) - 2 ) grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; exec_ctx.now = 0; exec_ctx.now_is_valid = true; - Backoff::Result next_deadlines = backoff.Begin(&exec_ctx); - GPR_ASSERT(next_deadlines.current_deadline == - next_deadlines.next_attempt_start_time); - GPR_ASSERT(next_deadlines.current_deadline == 2); - exec_ctx.now = next_deadlines.current_deadline; - next_deadlines = backoff.Step(&exec_ctx); - GPR_ASSERT(next_deadlines.current_deadline == 6); - exec_ctx.now = next_deadlines.current_deadline; - next_deadlines = backoff.Step(&exec_ctx); - GPR_ASSERT(next_deadlines.current_deadline == 14); - exec_ctx.now = next_deadlines.current_deadline; - next_deadlines = backoff.Step(&exec_ctx); - GPR_ASSERT(next_deadlines.current_deadline == 30); - exec_ctx.now = next_deadlines.current_deadline; - next_deadlines = backoff.Step(&exec_ctx); - GPR_ASSERT(next_deadlines.current_deadline == 62); - exec_ctx.now = next_deadlines.current_deadline; - next_deadlines = backoff.Step(&exec_ctx); - GPR_ASSERT(next_deadlines.current_deadline == 126); - exec_ctx.now = next_deadlines.current_deadline; - next_deadlines = backoff.Step(&exec_ctx); - GPR_ASSERT(next_deadlines.current_deadline == 254); - exec_ctx.now = next_deadlines.current_deadline; - next_deadlines = backoff.Step(&exec_ctx); - GPR_ASSERT(next_deadlines.current_deadline == 510); - exec_ctx.now = next_deadlines.current_deadline; - next_deadlines = backoff.Step(&exec_ctx); - GPR_ASSERT(next_deadlines.current_deadline == 1022); - exec_ctx.now = next_deadlines.current_deadline; - next_deadlines = backoff.Step(&exec_ctx); + grpc_millis next = backoff.Begin(&exec_ctx); + EXPECT_EQ(next, 2); + exec_ctx.now = next; + next = backoff.Step(&exec_ctx); + EXPECT_EQ(next, 6); + exec_ctx.now = next; + next = backoff.Step(&exec_ctx); + EXPECT_EQ(next, 14); + exec_ctx.now = next; + next = backoff.Step(&exec_ctx); + EXPECT_EQ(next, 30); + exec_ctx.now = next; + next = backoff.Step(&exec_ctx); + EXPECT_EQ(next, 62); + exec_ctx.now = next; + next = backoff.Step(&exec_ctx); + EXPECT_EQ(next, 126); + exec_ctx.now = next; + next = backoff.Step(&exec_ctx); + EXPECT_EQ(next, 254); + exec_ctx.now = next; + next = backoff.Step(&exec_ctx); + EXPECT_EQ(next, 510); + exec_ctx.now = next; + next = backoff.Step(&exec_ctx); + EXPECT_EQ(next, 1022); + exec_ctx.now = next; + next = backoff.Step(&exec_ctx); // Hit the maximum timeout. From this point onwards, retries will increase // only by max timeout. - GPR_ASSERT(next_deadlines.current_deadline == 1535); - exec_ctx.now = next_deadlines.current_deadline; - next_deadlines = backoff.Step(&exec_ctx); - GPR_ASSERT(next_deadlines.current_deadline == 2048); - exec_ctx.now = next_deadlines.current_deadline; - next_deadlines = backoff.Step(&exec_ctx); - GPR_ASSERT(next_deadlines.current_deadline == 2561); + EXPECT_EQ(next, 1535); + exec_ctx.now = next; + next = backoff.Step(&exec_ctx); + EXPECT_EQ(next, 2048); + exec_ctx.now = next; + next = backoff.Step(&exec_ctx); + EXPECT_EQ(next, 2561); grpc_exec_ctx_finish(&exec_ctx); } @@ -156,23 +143,19 @@ TEST(BackOffTest, JitterBackOff) { const grpc_millis min_connect_timeout = 100; const double multiplier = 1.0; const double jitter = 0.1; - Backoff::Options options; + BackOff::Options options; options.set_initial_backoff(initial_backoff) .set_multiplier(multiplier) .set_jitter(jitter) .set_min_connect_timeout(min_connect_timeout) .set_max_backoff(max_backoff); - Backoff backoff(options); + BackOff backoff(options); backoff.SetRandomSeed(0); // force consistent PRNG grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; - Backoff::Result next_deadlines = backoff.Begin(&exec_ctx); - GPR_ASSERT(next_deadlines.current_deadline - grpc_exec_ctx_now(&exec_ctx) == - initial_backoff); - GPR_ASSERT(next_deadlines.next_attempt_start_time - - grpc_exec_ctx_now(&exec_ctx) == - initial_backoff); + grpc_millis next = backoff.Begin(&exec_ctx); + EXPECT_EQ(next - grpc_exec_ctx_now(&exec_ctx), initial_backoff); grpc_millis expected_next_lower_bound = (grpc_millis)((double)current_backoff * (1 - jitter)); @@ -180,20 +163,19 @@ TEST(BackOffTest, JitterBackOff) { (grpc_millis)((double)current_backoff * (1 + jitter)); for (int i = 0; i < 10000; i++) { - next_deadlines = backoff.Step(&exec_ctx); + next = backoff.Step(&exec_ctx); // next-now must be within (jitter*100)% of the current backoff (which // increases by * multiplier up to max_backoff). - const grpc_millis timeout_millis = - next_deadlines.current_deadline - grpc_exec_ctx_now(&exec_ctx); - GPR_ASSERT(timeout_millis >= expected_next_lower_bound); - GPR_ASSERT(timeout_millis <= expected_next_upper_bound); - current_backoff = GPR_MIN( + const grpc_millis timeout_millis = next - grpc_exec_ctx_now(&exec_ctx); + EXPECT_GE(timeout_millis, expected_next_lower_bound); + EXPECT_LE(timeout_millis, expected_next_upper_bound); + current_backoff = std::min( (grpc_millis)((double)current_backoff * multiplier), max_backoff); expected_next_lower_bound = (grpc_millis)((double)current_backoff * (1 - jitter)); expected_next_upper_bound = (grpc_millis)((double)current_backoff * (1 + jitter)); - exec_ctx.now = next_deadlines.current_deadline; + exec_ctx.now = next; } grpc_exec_ctx_finish(&exec_ctx); } -- cgit v1.2.3