From 25f7d0f6f4a40dc969685192b45257d8c000a58d Mon Sep 17 00:00:00 2001
From: Juanli Shen <juanlishen@google.com>
Date: Fri, 17 Aug 2018 13:25:15 -0700
Subject: [PATCH] Revert to TRANSIENT_FAILURE during backoff

---
 .../ext/filters/client_channel/subchannel.cc  |  9 ++-------
 test/cpp/end2end/grpclb_end2end_test.cc       | 19 +++++++++++++++++++
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/src/core/ext/filters/client_channel/subchannel.cc b/src/core/ext/filters/client_channel/subchannel.cc
index 48c6030c895..0e40f42e189 100644
--- a/src/core/ext/filters/client_channel/subchannel.cc
+++ b/src/core/ext/filters/client_channel/subchannel.cc
@@ -404,6 +404,8 @@ static void continue_connect_locked(grpc_subchannel* c) {
   c->next_attempt_deadline = c->backoff->NextAttemptTime();
   args.deadline = std::max(c->next_attempt_deadline, min_deadline);
   args.channel_args = c->args;
+  grpc_connectivity_state_set(&c->state_tracker, GRPC_CHANNEL_CONNECTING,
+                              GRPC_ERROR_NONE, "connecting");
   grpc_connector_connect(c->connector, &args, &c->connecting_result,
                          &c->on_connected);
 }
@@ -478,8 +480,6 @@ static void maybe_start_connecting_locked(grpc_subchannel* c) {
   GRPC_SUBCHANNEL_WEAK_REF(c, "connecting");
   if (!c->backoff_begun) {
     c->backoff_begun = true;
-    grpc_connectivity_state_set(&c->state_tracker, GRPC_CHANNEL_CONNECTING,
-                                GRPC_ERROR_NONE, "connecting");
     continue_connect_locked(c);
   } else {
     GPR_ASSERT(!c->have_alarm);
@@ -494,11 +494,6 @@ static void maybe_start_connecting_locked(grpc_subchannel* c) {
     }
     GRPC_CLOSURE_INIT(&c->on_alarm, on_alarm, c, grpc_schedule_on_exec_ctx);
     grpc_timer_init(&c->alarm, c->next_attempt_deadline, &c->on_alarm);
-    // During backoff, we prefer the connectivity state of CONNECTING instead of
-    // TRANSIENT_FAILURE in order to prevent triggering re-resolution
-    // continuously in pick_first.
-    grpc_connectivity_state_set(&c->state_tracker, GRPC_CHANNEL_CONNECTING,
-                                GRPC_ERROR_NONE, "backoff");
   }
 }
 
diff --git a/test/cpp/end2end/grpclb_end2end_test.cc b/test/cpp/end2end/grpclb_end2end_test.cc
index 28f9ae6f40a..b69b861fcf4 100644
--- a/test/cpp/end2end/grpclb_end2end_test.cc
+++ b/test/cpp/end2end/grpclb_end2end_test.cc
@@ -734,6 +734,25 @@ TEST_F(SingleBalancerTest, InitiallyEmptyServerlist) {
   EXPECT_EQ(2U, balancer_servers_[0].service_->response_count());
 }
 
+TEST_F(SingleBalancerTest, AllServersUnreachableFailFast) {
+  SetNextResolutionAllBalancers();
+  const size_t kNumUnreachableServers = 5;
+  std::vector<int> ports;
+  for (size_t i = 0; i < kNumUnreachableServers; ++i) {
+    ports.push_back(grpc_pick_unused_port_or_die());
+  }
+  ScheduleResponseForBalancer(
+      0, BalancerServiceImpl::BuildResponseForBackends(ports, {}), 0);
+  const Status status = SendRpc();
+  // The error shouldn't be DEADLINE_EXCEEDED.
+  EXPECT_EQ(StatusCode::UNAVAILABLE, status.error_code());
+  balancers_[0]->NotifyDoneWithServerlists();
+  // The balancer got a single request.
+  EXPECT_EQ(1U, balancer_servers_[0].service_->request_count());
+  // and sent a single response.
+  EXPECT_EQ(1U, balancer_servers_[0].service_->response_count());
+}
+
 TEST_F(SingleBalancerTest, Fallback) {
   SetNextResolutionAllBalancers();
   const int kFallbackTimeoutMs = 200 * grpc_test_slowdown_factor();