diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc | 27 | ||||
| -rw-r--r-- | src/core/ext/filters/client_channel/lb_policy/subchannel_list.h | 14 |
2 files changed, 16 insertions, 25 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 1fecdebccf..2d9e4af96c 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 @@ void PickFirst::UpdateLocked(const grpc_channel_args& args) { 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 bad50c461c..276a2f08a1 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 @@ class SubchannelData { 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 |