PR Comments

reviewable/pr13494/r8
David Garcia Quintas 7 years ago
parent a12efc0a3d
commit 8df0cc3363
  1. 2
      src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc
  2. 2
      src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc
  3. 2
      src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc
  4. 8
      src/core/ext/filters/client_channel/subchannel.cc
  5. 13
      src/core/lib/backoff/backoff.h
  6. 10
      test/core/backoff/backoff_test.cc

@ -120,7 +120,6 @@
#include "src/core/lib/surface/channel_init.h"
#include "src/core/lib/transport/static_metadata.h"
#define GRPC_GRPCLB_MIN_CONNECT_TIMEOUT_SECONDS 20
#define GRPC_GRPCLB_INITIAL_CONNECT_BACKOFF_SECONDS 1
#define GRPC_GRPCLB_RECONNECT_BACKOFF_MULTIPLIER 1.6
#define GRPC_GRPCLB_RECONNECT_MAX_BACKOFF_SECONDS 120
@ -1465,7 +1464,6 @@ static void lb_call_init_locked(grpc_exec_ctx* exec_ctx,
.set_initial_backoff(GRPC_GRPCLB_INITIAL_CONNECT_BACKOFF_SECONDS * 1000)
.set_multiplier(GRPC_GRPCLB_RECONNECT_BACKOFF_MULTIPLIER)
.set_jitter(GRPC_GRPCLB_RECONNECT_JITTER)
.set_min_connect_timeout(GRPC_GRPCLB_MIN_CONNECT_TIMEOUT_SECONDS * 1000)
.set_max_backoff(GRPC_GRPCLB_RECONNECT_MAX_BACKOFF_SECONDS * 1000);
glb_policy->lb_call_backoff.Init(backoff_options);

@ -44,7 +44,6 @@
#include "src/core/lib/support/string.h"
#include "src/core/lib/transport/service_config.h"
#define GRPC_DNS_MIN_CONNECT_TIMEOUT_SECONDS 1
#define GRPC_DNS_INITIAL_CONNECT_BACKOFF_SECONDS 1
#define GRPC_DNS_RECONNECT_BACKOFF_MULTIPLIER 1.6
#define GRPC_DNS_RECONNECT_MAX_BACKOFF_SECONDS 120
@ -386,7 +385,6 @@ static grpc_resolver* dns_ares_create(grpc_exec_ctx* exec_ctx,
.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));
GRPC_CLOSURE_INIT(&r->dns_ares_on_retry_timer_locked,

@ -36,7 +36,6 @@
#include "src/core/lib/support/manual_constructor.h"
#include "src/core/lib/support/string.h"
#define GRPC_DNS_MIN_CONNECT_TIMEOUT_SECONDS 1
#define GRPC_DNS_INITIAL_CONNECT_BACKOFF_SECONDS 1
#define GRPC_DNS_RECONNECT_BACKOFF_MULTIPLIER 1.6
#define GRPC_DNS_RECONNECT_MAX_BACKOFF_SECONDS 120
@ -262,7 +261,6 @@ static grpc_resolver* dns_create(grpc_exec_ctx* exec_ctx,
.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));
return &r->base;

@ -290,8 +290,6 @@ 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_TIMEOUT_SECONDS * 1000;
int max_backoff_ms = GRPC_SUBCHANNEL_RECONNECT_MAX_BACKOFF_SECONDS * 1000;
bool fixed_reconnect_backoff = false;
if (args != nullptr) {
@ -299,14 +297,9 @@ static grpc_core::BackOff::Options extract_backoff_options(
if (0 == strcmp(args->args[i].key,
"grpc.testing.fixed_reconnect_backoff_ms")) {
fixed_reconnect_backoff = true;
initial_backoff_ms = min_connect_timeout_ms = max_backoff_ms =
grpc_channel_arg_get_integer(&args->args[i],
{initial_backoff_ms, 100, INT_MAX});
} else if (0 ==
strcmp(args->args[i].key, GRPC_ARG_MIN_RECONNECT_BACKOFF_MS)) {
fixed_reconnect_backoff = false;
min_connect_timeout_ms = grpc_channel_arg_get_integer(
&args->args[i], {min_connect_timeout_ms, 100, INT_MAX});
} else if (0 ==
strcmp(args->args[i].key, GRPC_ARG_MAX_RECONNECT_BACKOFF_MS)) {
fixed_reconnect_backoff = false;
@ -327,7 +320,6 @@ static grpc_core::BackOff::Options extract_backoff_options(
: GRPC_SUBCHANNEL_RECONNECT_BACKOFF_MULTIPLIER)
.set_jitter(fixed_reconnect_backoff ? 0.0
: GRPC_SUBCHANNEL_RECONNECT_JITTER)
.set_min_connect_timeout(min_connect_timeout_ms)
.set_max_backoff(max_backoff_ms);
return backoff_options;
}

@ -33,10 +33,10 @@ class BackOff {
explicit BackOff(const Options& options);
/// Begin retry loop: returns the deadline to be used for the next attempt,
/// following the backoff / strategy.
/// 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.
/// Step a retry loop: 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.
@ -58,10 +58,6 @@ class BackOff {
jitter_ = jitter;
return *this;
}
Options& set_min_connect_timeout(grpc_millis min_connect_timeout) {
min_connect_timeout_ = min_connect_timeout;
return *this;
}
Options& set_max_backoff(grpc_millis max_backoff) {
max_backoff_ = max_backoff;
return *this;
@ -72,8 +68,6 @@ class BackOff {
double multiplier() const { return multiplier_; }
/// amount to randomize backoffs
double jitter() const { return jitter_; }
/// minimum time between retries
grpc_millis min_connect_timeout() const { return min_connect_timeout_; }
/// maximum time between retries
grpc_millis max_backoff() const { return max_backoff_; }
@ -81,7 +75,6 @@ class BackOff {
grpc_millis initial_backoff_;
double multiplier_;
double jitter_;
grpc_millis min_connect_timeout_;
grpc_millis max_backoff_;
}; // class Options

@ -36,13 +36,11 @@ TEST(BackOffTest, ConstantBackOff) {
const grpc_millis initial_backoff = 200;
const double multiplier = 1.0;
const double jitter = 0.0;
const grpc_millis min_connect_timeout = 100;
const grpc_millis max_backoff = 1000;
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);
@ -54,7 +52,6 @@ TEST(BackOffTest, ConstantBackOff) {
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);
}
@ -63,13 +60,11 @@ TEST(BackOffTest, MinConnect) {
const grpc_millis initial_backoff = 100;
const double multiplier = 1.0;
const double jitter = 0.0;
const grpc_millis min_connect_timeout = 200;
const grpc_millis max_backoff = 1000;
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);
grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT;
@ -82,13 +77,11 @@ TEST(BackOffTest, NoJitterBackOff) {
const grpc_millis initial_backoff = 2;
const double multiplier = 2.0;
const double jitter = 0.0;
const grpc_millis min_connect_timeout = 1;
const grpc_millis max_backoff = 513;
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);
// x_1 = 2
@ -140,14 +133,12 @@ TEST(BackOffTest, JitterBackOff) {
const grpc_millis initial_backoff = 500;
grpc_millis current_backoff = initial_backoff;
const grpc_millis max_backoff = 1000;
const grpc_millis min_connect_timeout = 100;
const double multiplier = 1.0;
const double jitter = 0.1;
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);
@ -175,7 +166,6 @@ TEST(BackOffTest, JitterBackOff) {
(grpc_millis)((double)current_backoff * (1 - jitter));
expected_next_upper_bound =
(grpc_millis)((double)current_backoff * (1 + jitter));
exec_ctx.now = next;
}
grpc_exec_ctx_finish(&exec_ctx);
}

Loading…
Cancel
Save