From 523e537368079c3f74bc5776d9fba5c6ec1f7f91 Mon Sep 17 00:00:00 2001
From: "Mark D. Roth" <roth@google.com>
Date: Thu, 21 Feb 2019 12:11:00 -0800
Subject: [PATCH] Start connectivity watches before updating connectivity
 state.

---
 .../lb_policy/pick_first/pick_first.cc        | 20 ++++++++++---------
 .../lb_policy/round_robin/round_robin.cc      |  7 ++++---
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc b/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc
index 58bf3d89f21..c222b7ba292 100644
--- a/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc
+++ b/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc
@@ -288,8 +288,8 @@ void PickFirst::UpdateLocked(const grpc_channel_args& args,
     GRPC_ERROR_UNREF(error);
     if (state == GRPC_CHANNEL_READY) {
       subchannel_list_ = std::move(subchannel_list);
-      sd->ProcessUnselectedReadyLocked();
       sd->StartConnectivityWatchLocked();
+      sd->ProcessUnselectedReadyLocked();
       // If there was a previously pending update (which may or may
       // not have contained the currently selected subchannel), drop
       // it, so that it doesn't override what we've done here.
@@ -421,9 +421,9 @@ void PickFirst::PickFirstSubchannelData::ProcessConnectivityChangeLocked(
   //    select in place of the current one.
   switch (connectivity_state) {
     case GRPC_CHANNEL_READY: {
-      ProcessUnselectedReadyLocked();
       // Renew notification.
       RenewConnectivityWatchLocked();
+      ProcessUnselectedReadyLocked();
       break;
     }
     case GRPC_CHANNEL_TRANSIENT_FAILURE: {
@@ -502,16 +502,18 @@ void PickFirst::PickFirstSubchannelData::ProcessUnselectedReadyLocked() {
 void PickFirst::PickFirstSubchannelData::
     CheckConnectivityStateAndStartWatchingLocked() {
   PickFirst* p = static_cast<PickFirst*>(subchannel_list()->policy());
+  // Check current state.
   grpc_error* error = GRPC_ERROR_NONE;
-  if (p->selected_ != this &&
-      CheckConnectivityStateLocked(&error) == GRPC_CHANNEL_READY) {
-    // We must process the READY subchannel before we start watching it.
-    // Otherwise, we won't know it's READY because we will be waiting for its
-    // connectivity state to change from READY.
-    ProcessUnselectedReadyLocked();
-  }
+  grpc_connectivity_state current_state = CheckConnectivityStateLocked(&error);
   GRPC_ERROR_UNREF(error);
+  // Start watch.
   StartConnectivityWatchLocked();
+  // If current state is READY, select the subchannel now, since we started
+  // watching from this state and will not get a notification of it
+  // transitioning into this state.
+  if (p->selected_ != this && current_state == GRPC_CHANNEL_READY) {
+    ProcessUnselectedReadyLocked();
+  }
 }
 
 //
diff --git a/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc b/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc
index f92c2d4ba59..b9250f92033 100644
--- a/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc
+++ b/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc
@@ -325,14 +325,14 @@ void RoundRobin::RoundRobinSubchannelList::StartWatchingLocked() {
       subchannel(i)->UpdateConnectivityStateLocked(state, error);
     }
   }
-  // Now set the LB policy's state based on the subchannels' states.
-  UpdateRoundRobinStateFromSubchannelStateCountsLocked();
   // Start connectivity watch for each subchannel.
   for (size_t i = 0; i < num_subchannels(); i++) {
     if (subchannel(i)->subchannel() != nullptr) {
       subchannel(i)->StartConnectivityWatchLocked();
     }
   }
+  // Now set the LB policy's state based on the subchannels' states.
+  UpdateRoundRobinStateFromSubchannelStateCountsLocked();
 }
 
 void RoundRobin::RoundRobinSubchannelList::UpdateStateCountersLocked(
@@ -468,11 +468,12 @@ void RoundRobin::RoundRobinSubchannelData::ProcessConnectivityChangeLocked(
     }
     p->channel_control_helper()->RequestReresolution();
   }
+  // Renew connectivity watch.
+  RenewConnectivityWatchLocked();
   // Update state counters.
   UpdateConnectivityStateLocked(connectivity_state, error);
   // Update overall state and renew notification.
   subchannel_list()->UpdateRoundRobinStateFromSubchannelStateCountsLocked();
-  RenewConnectivityWatchLocked();
 }
 
 void RoundRobin::UpdateLocked(const grpc_channel_args& args,