Merge pull request #13121 from dgquintas/backoff_spec

Improvements to C Core's backoff code and API

4 files changed, 25 insertions, 21 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 85e76e68b5..03116b420c 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
@@ -1266,7 +1266,8 @@ static void maybe_restart_lb_call(grpc_exec_ctx *exec_ctx,
   } else if (!glb_policy->shutting_down) {
     /* if we aren't shutting down, restart the LB client call after some time */
     grpc_millis next_try =
-        grpc_backoff_step(exec_ctx, &glb_policy->lb_call_backoff_state);
+        grpc_backoff_step(exec_ctx, &glb_policy->lb_call_backoff_state)
+            .next_attempt_start_time;
     if (GRPC_TRACER_ON(grpc_lb_glb_trace)) {
       gpr_log(GPR_DEBUG, "Connection to LB server lost (grpclb: %p)...",
               (void *)glb_policy);
@@ -1431,7 +1432,7 @@ static void lb_call_init_locked(grpc_exec_ctx *exec_ctx,
                     grpc_combiner_scheduler(glb_policy->base.combiner));
 
   grpc_backoff_init(&glb_policy->lb_call_backoff_state,
-                    GRPC_GRPCLB_INITIAL_CONNECT_BACKOFF_SECONDS,
+                    GRPC_GRPCLB_INITIAL_CONNECT_BACKOFF_SECONDS * 1000,
                     GRPC_GRPCLB_RECONNECT_BACKOFF_MULTIPLIER,
                     GRPC_GRPCLB_RECONNECT_JITTER,
                     GRPC_GRPCLB_MIN_CONNECT_TIMEOUT_SECONDS * 1000,
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 5f7ab987cb..a1ddaee499 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
@@ -271,7 +271,8 @@ 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 = grpc_backoff_step(exec_ctx, &r->backoff_state);
+    grpc_millis next_try =
+        grpc_backoff_step(exec_ctx, &r->backoff_state).next_attempt_start_time;
     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));
@@ -379,11 +380,11 @@ 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_backoff_init(&r->backoff_state, GRPC_DNS_INITIAL_CONNECT_BACKOFF_SECONDS,
-                    GRPC_DNS_RECONNECT_BACKOFF_MULTIPLIER,
-                    GRPC_DNS_RECONNECT_JITTER,
-                    GRPC_DNS_MIN_CONNECT_TIMEOUT_SECONDS * 1000,
-                    GRPC_DNS_RECONNECT_MAX_BACKOFF_SECONDS * 1000);
+  grpc_backoff_init(
+      &r->backoff_state, GRPC_DNS_INITIAL_CONNECT_BACKOFF_SECONDS * 1000,
+      GRPC_DNS_RECONNECT_BACKOFF_MULTIPLIER, GRPC_DNS_RECONNECT_JITTER,
+      GRPC_DNS_MIN_CONNECT_TIMEOUT_SECONDS * 1000,
+      GRPC_DNS_RECONNECT_MAX_BACKOFF_SECONDS * 1000);
   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 e669b6dfc7..62aead5517 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
@@ -170,7 +170,8 @@ 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 = grpc_backoff_step(exec_ctx, &r->backoff_state);
+    grpc_millis next_try =
+        grpc_backoff_step(exec_ctx, &r->backoff_state).next_attempt_start_time;
     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));
@@ -256,11 +257,11 @@ 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_backoff_init(&r->backoff_state, GRPC_DNS_INITIAL_CONNECT_BACKOFF_SECONDS,
-                    GRPC_DNS_RECONNECT_BACKOFF_MULTIPLIER,
-                    GRPC_DNS_RECONNECT_JITTER,
-                    GRPC_DNS_MIN_CONNECT_TIMEOUT_SECONDS * 1000,
-                    GRPC_DNS_RECONNECT_MAX_BACKOFF_SECONDS * 1000);
+  grpc_backoff_init(
+      &r->backoff_state, GRPC_DNS_INITIAL_CONNECT_BACKOFF_SECONDS * 1000,
+      GRPC_DNS_RECONNECT_BACKOFF_MULTIPLIER, GRPC_DNS_RECONNECT_JITTER,
+      GRPC_DNS_MIN_CONNECT_TIMEOUT_SECONDS * 1000,
+      GRPC_DNS_RECONNECT_MAX_BACKOFF_SECONDS * 1000);
   return &r->base;
 }
 
diff --git a/src/core/ext/filters/client_channel/subchannel.cc b/src/core/ext/filters/client_channel/subchannel.cc
index 5710a22178..b954e1b879 100644
--- a/src/core/ext/filters/client_channel/subchannel.cc
+++ b/src/core/ext/filters/client_channel/subchannel.cc
@@ -117,10 +117,10 @@ struct grpc_subchannel {
 
   external_state_watcher root_external_state_watcher;
 
-  /** next connect attempt time */
-  grpc_millis next_attempt;
   /** backoff state */
   grpc_backoff backoff_state;
+  grpc_backoff_result backoff_result;
+
   /** do we have an active alarm? */
   bool have_alarm;
   /** have we started the backoff loop */
@@ -380,7 +380,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->next_attempt;
+  args.deadline = c->backoff_result.current_deadline;
   args.channel_args = c->args;
 
   grpc_connectivity_state_set(exec_ctx, &c->state_tracker,
@@ -428,7 +428,7 @@ 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->next_attempt = grpc_backoff_step(exec_ctx, &c->backoff_state);
+    c->backoff_result = grpc_backoff_step(exec_ctx, &c->backoff_state);
     continue_connect_locked(exec_ctx, c);
     gpr_mu_unlock(&c->mu);
   } else {
@@ -465,20 +465,21 @@ static void maybe_start_connecting_locked(grpc_exec_ctx *exec_ctx,
 
   if (!c->backoff_begun) {
     c->backoff_begun = true;
-    c->next_attempt = grpc_backoff_begin(exec_ctx, &c->backoff_state);
+    c->backoff_result = grpc_backoff_begin(exec_ctx, &c->backoff_state);
     continue_connect_locked(exec_ctx, c);
   } else {
     GPR_ASSERT(!c->have_alarm);
     c->have_alarm = true;
     const grpc_millis time_til_next =
-        c->next_attempt - grpc_exec_ctx_now(exec_ctx);
+        c->backoff_result.next_attempt_start_time - 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->next_attempt, &c->on_alarm);
+    grpc_timer_init(exec_ctx, &c->alarm,
+                    c->backoff_result.next_attempt_start_time, &c->on_alarm);
   }
 }