Remove SetConnectedSubchannelFromLocked().
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 1fecdeb..2d9e4af 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
@@ -332,18 +332,23 @@
this, selected_->subchannel(), i,
subchannel_list->num_subchannels());
}
- if (selected_->connected_subchannel() != nullptr) {
- sd->SetConnectedSubchannelFromLocked(selected_);
+ // Make sure it's in state READY. It might not be if we grabbed
+ // the combiner while a connectivity state notification
+ // informing us otherwise is pending.
+ // Note that CheckConnectivityStateLocked() also takes a ref to
+ // the connected subchannel.
+ grpc_error* error = GRPC_ERROR_NONE;
+ if (sd->CheckConnectivityStateLocked(&error) == GRPC_CHANNEL_READY) {
+ selected_ = sd;
+ subchannel_list_ = std::move(subchannel_list);
+ DestroyUnselectedSubchannelsLocked();
+ sd->StartConnectivityWatchLocked();
+ // 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.
+ latest_pending_subchannel_list_.reset();
+ return;
}
- selected_ = sd;
- subchannel_list_ = std::move(subchannel_list);
- DestroyUnselectedSubchannelsLocked();
- sd->StartConnectivityWatchLocked();
- // 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.
- latest_pending_subchannel_list_.reset();
- return;
}
}
// Not keeping the previous selected subchannel, so set the latest
diff --git a/src/core/ext/filters/client_channel/lb_policy/subchannel_list.h b/src/core/ext/filters/client_channel/lb_policy/subchannel_list.h
index bad50c4..276a2f0 100644
--- a/src/core/ext/filters/client_channel/lb_policy/subchannel_list.h
+++ b/src/core/ext/filters/client_channel/lb_policy/subchannel_list.h
@@ -89,20 +89,6 @@
return connected_subchannel_.get();
}
-// FIXME: remove
- // Used to set the connected subchannel in cases where we are retaining a
- // subchannel from a previous subchannel list. This is slightly more
- // efficient than getting the connected subchannel from the subchannel,
- // because that approach requires the use of a mutex, whereas this one
- // only mutates a refcount.
- // TODO(roth): This method is a bit of a hack and is used only in
- // pick_first. When we have time, find a way to remove this, possibly
- // by making pick_first work more like round_robin.
- void SetConnectedSubchannelFromLocked(SubchannelData* other) {
- GPR_ASSERT(subchannel_ == other->subchannel_);
- connected_subchannel_ = other->connected_subchannel_; // Adds ref.
- }
-
// Synchronously checks the subchannel's connectivity state.
// Must not be called while there is a connectivity notification
// pending (i.e., between calling StartConnectivityWatchLocked() or