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 0883a4ed6c3..be00e871b8e 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 @@ -294,7 +294,7 @@ void PickFirst::UpdateLocked(const grpc_channel_args& args) { return; } const grpc_lb_addresses* addresses = - (const grpc_lb_addresses*)arg->value.pointer.p; + static_cast(arg->value.pointer.p); if (grpc_lb_pick_first_trace.enabled()) { gpr_log(GPR_INFO, "Pick First %p received update with %" PRIuPTR " addresses", this, @@ -344,7 +344,6 @@ void PickFirst::UpdateLocked(const grpc_channel_args& args) { subchannel_list->num_subchannels()); } if (selected_->connected_subchannel() != nullptr) { -// FIXME: restructure to work more like RR? sd->SetConnectedSubchannelFromLocked(selected_); } selected_ = sd; 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 8b843b16c00..6d9c382c64b 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 @@ -94,13 +94,14 @@ class SubchannelData { return curr_connectivity_state_; } -// FIXME: remove - // An alternative to SetConnectedSubchannelFromSubchannelLocked() for - // cases where we are retaining a connected 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. + // 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. @@ -164,11 +165,12 @@ class SubchannelData { // Implementations can use connectivity_state() to get the new // connectivity state. // Implementations must invoke either StopConnectivityWatch() or again - // call StartConnectivityWatch() before returning. + // call StartOrRenewConnectivityWatch() before returning. virtual void ProcessConnectivityChangeLocked(grpc_error* error) GRPC_ABSTRACT; private: -// FIXME: document + // Updates connected_subchannel_ based on pending_connectivity_state_unsafe_. + // Returns true if the connectivity state should be reported. bool UpdateConnectedSubchannelLocked(); static void OnConnectivityChangedLocked(void* arg, grpc_error* error);