Browse Source

Minor cleanups.

Mark D. Roth 7 years ago
parent
commit
3e74a18668

+ 1 - 2
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<const grpc_lb_addresses*>(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;

+ 11 - 9
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);