Merge pull request #24215 from markdroth/subchannel_attributes

Don't keep address attributes on the resulting subchannels.

src/core/ext/filters/client_channel/client_channel.cc

@@ -882,9 +882,6 @@ class CallData {
 // ChannelData::SubchannelWrapper
 //
 
-using ServerAddressAttributeMap =
-    std::map<const char*, std::unique_ptr<ServerAddress::AttributeInterface>>;
-
 // This class is a wrapper for Subchannel that hides details of the
 // channel's implementation (such as the health check service name and
 // connected subchannel) from the LB policy API.
@@ -896,13 +893,11 @@ using ServerAddressAttributeMap =
 class ChannelData::SubchannelWrapper : public SubchannelInterface {
  public:
   SubchannelWrapper(ChannelData* chand, Subchannel* subchannel,
-                    grpc_core::UniquePtr<char> health_check_service_name,
-                    ServerAddressAttributeMap attributes)
+                    grpc_core::UniquePtr<char> health_check_service_name)
       : SubchannelInterface(&grpc_client_channel_routing_trace),
         chand_(chand),
         subchannel_(subchannel),
-        health_check_service_name_(std::move(health_check_service_name)),
-        attributes_(std::move(attributes)) {
+        health_check_service_name_(std::move(health_check_service_name)) {
     if (GRPC_TRACE_FLAG_ENABLED(grpc_client_channel_routing_trace)) {
       gpr_log(GPR_INFO,
               "chand=%p: creating subchannel wrapper %p for subchannel %p",
@@ -984,13 +979,6 @@ class ChannelData::SubchannelWrapper : public SubchannelInterface {
     return subchannel_->channel_args();
   }
 
-  const ServerAddress::AttributeInterface* GetAttribute(
-      const char* key) const override {
-    auto it = attributes_.find(key);
-    if (it == attributes_.end()) return nullptr;
-    return it->second.get();
-  }
-
   void ThrottleKeepaliveTime(int new_keepalive_time) {
     subchannel_->ThrottleKeepaliveTime(new_keepalive_time);
   }
@@ -1188,7 +1176,6 @@ class ChannelData::SubchannelWrapper : public SubchannelInterface {
   ChannelData* chand_;
   Subchannel* subchannel_;
   grpc_core::UniquePtr<char> health_check_service_name_;
-  ServerAddressAttributeMap attributes_;
   // Maps from the address of the watcher passed to us by the LB policy
   // to the address of the WrapperWatcher that we passed to the underlying
   // subchannel.  This is needed so that when the LB policy calls
@@ -1363,18 +1350,6 @@ class ChannelData::ConnectivityWatcherRemover {
 // ChannelData::ClientChannelControlHelper
 //
 
-}  // namespace
-
-// Allows accessing the attributes from a ServerAddress.
-class ChannelServerAddressPeer {
- public:
-  static ServerAddressAttributeMap GetAttributes(ServerAddress* address) {
-    return std::move(address->attributes_);
-  }
-};
-
-namespace {
-
 class ChannelData::ClientChannelControlHelper
     : public LoadBalancingPolicy::ChannelControlHelper {
  public:
@@ -1426,8 +1401,7 @@ class ChannelData::ClientChannelControlHelper
     subchannel->ThrottleKeepaliveTime(chand_->keepalive_time_);
     // Create and return wrapper for the subchannel.
     return MakeRefCounted<SubchannelWrapper>(
-        chand_, subchannel, std::move(health_check_service_name),
-        ChannelServerAddressPeer::GetAttributes(&address));
+        chand_, subchannel, std::move(health_check_service_name));
   }
 
   void UpdateState(

src/core/ext/filters/client_channel/lb_policy/subchannel_list.h

@@ -363,9 +363,9 @@ SubchannelList<SubchannelListType, SubchannelDataType>::SubchannelList(
   }
   subchannels_.reserve(addresses.size());
   // Create a subchannel for each address.
-  for (const ServerAddress& address : addresses) {
+  for (ServerAddress& address : addresses) {
     RefCountedPtr<SubchannelInterface> subchannel =
-        helper->CreateSubchannel(std::move(address), args);
+        helper->CreateSubchannel(address, args);
     if (subchannel == nullptr) {
       // Subchannel could not be created.
       if (GRPC_TRACE_FLAG_ENABLED(*tracer_)) {
@@ -383,7 +383,7 @@ SubchannelList<SubchannelListType, SubchannelDataType>::SubchannelList(
               tracer_->name(), policy_, this, subchannels_.size(),
               subchannel.get(), address.ToString().c_str());
     }
-    subchannels_.emplace_back(this, address, std::move(subchannel));
+    subchannels_.emplace_back(this, std::move(address), std::move(subchannel));
   }
 }
 

src/core/ext/filters/client_channel/server_address.h

@@ -97,10 +97,6 @@ class ServerAddress {
   std::string ToString() const;
 
  private:
-  // Allows the channel to access the attributes without knowing the keys.
-  // (We intentionally do not allow LB policies to do this.)
-  friend class ChannelServerAddressPeer;
-
   grpc_resolved_address address_;
   grpc_channel_args* args_;
   std::map<const char*, std::unique_ptr<AttributeInterface>> attributes_;

src/core/ext/filters/client_channel/subchannel_interface.h

@@ -21,9 +21,10 @@
 
 #include <grpc/support/port_platform.h>
 
-#include "src/core/ext/filters/client_channel/server_address.h"
+#include <grpc/impl/codegen/connectivity_state.h>
+#include <grpc/impl/codegen/grpc_types.h>
+
 #include "src/core/lib/gprpp/ref_counted.h"
-#include "src/core/lib/gprpp/ref_counted_ptr.h"
 
 namespace grpc_core {
 
@@ -88,11 +89,6 @@ class SubchannelInterface : public RefCounted<SubchannelInterface> {
 
   // TODO(roth): Need a better non-grpc-specific abstraction here.
   virtual const grpc_channel_args* channel_args() = 0;
-
-  // Allows accessing the attributes associated with the address for
-  // this subchannel.
-  virtual const ServerAddress::AttributeInterface* GetAttribute(
-      const char* key) const = 0;
 };
 
 // A class that delegates to another subchannel, to be used in cases
@@ -124,10 +120,6 @@ class DelegatingSubchannel : public SubchannelInterface {
   const grpc_channel_args* channel_args() override {
     return wrapped_subchannel_->channel_args();
   }
-  const ServerAddress::AttributeInterface* GetAttribute(
-      const char* key) const override {
-    return wrapped_subchannel_->GetAttribute(key);
-  }
 
  private:
   RefCountedPtr<SubchannelInterface> wrapped_subchannel_;