diff --git a/src/core/ext/filters/client_channel/README.md b/src/core/ext/filters/client_channel/README.md index 9676a4535b2..ffb09fd34e7 100644 --- a/src/core/ext/filters/client_channel/README.md +++ b/src/core/ext/filters/client_channel/README.md @@ -4,7 +4,7 @@ Client Configuration Support for GRPC This library provides high level configuration machinery to construct client channels and load balance between them. -Each grpc_channel is created with a grpc_resolver. It is the resolver's duty +Each `grpc_channel` is created with a `Resolver`. It is the resolver's duty to resolve a name into a set of arguments for the channel. Such arguments might include: @@ -12,7 +12,7 @@ might include: - a load balancing policy to decide which server to send a request to - a set of filters to mutate outgoing requests (say, by adding metadata) -The resolver provides this data as a stream of grpc_channel_args objects to +The resolver provides this data as a stream of `grpc_channel_args` objects to the channel. We represent arguments as a stream so that they can be changed by the resolver during execution, by reacting to external events (such as new service configuration data being pushed to some store). @@ -21,11 +21,11 @@ new service configuration data being pushed to some store). Load Balancing -------------- -Load balancing configuration is provided by a grpc_lb_policy object. +Load balancing configuration is provided by a `LoadBalancingPolicy` object. The primary job of the load balancing policies is to pick a target server given only the initial metadata for a request. It does this by providing -a grpc_subchannel object to the owning channel. +a `ConnectedSubchannel` object to the owning channel. Sub-Channels @@ -38,9 +38,9 @@ decisions (for example, by avoiding disconnected backends). Configured sub-channels are fully setup to participate in the grpc data plane. Their behavior is specified by a set of grpc channel filters defined at their -construction. To customize this behavior, resolvers build -grpc_client_channel_factory objects, which use the decorator pattern to customize -construction arguments for concrete grpc_subchannel instances. +construction. To customize this behavior, transports build +`ClientChannelFactory` objects, which customize construction arguments for +concrete subchannel instances. Naming for GRPC diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index 3566ef8fb35..3fb32f7e823 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -107,8 +107,8 @@ typedef struct client_channel_channel_data { grpc_channel_stack* owning_stack; /** interested parties (owned) */ grpc_pollset_set* interested_parties; - // Client channel factory. Holds a ref. - grpc_client_channel_factory* client_channel_factory; + // Client channel factory. + grpc_core::ClientChannelFactory* client_channel_factory; // Subchannel pool. grpc_core::RefCountedPtr subchannel_pool; @@ -205,16 +205,15 @@ class ClientChannelControlHelper chand_->subchannel_pool.get()); grpc_channel_args* new_args = grpc_channel_args_copy_and_add(&args, &arg, 1); - Subchannel* subchannel = grpc_client_channel_factory_create_subchannel( - chand_->client_channel_factory, new_args); + Subchannel* subchannel = + chand_->client_channel_factory->CreateSubchannel(new_args); grpc_channel_args_destroy(new_args); return subchannel; } - grpc_channel* CreateChannel(const char* target, grpc_client_channel_type type, + grpc_channel* CreateChannel(const char* target, const grpc_channel_args& args) override { - return grpc_client_channel_factory_create_channel( - chand_->client_channel_factory, target, type, &args); + return chand_->client_channel_factory->CreateChannel(target, &args); } void UpdateState( @@ -420,19 +419,12 @@ static grpc_error* cc_init_channel_elem(grpc_channel_element* elem, arg = grpc_channel_args_find(args->channel_args, GRPC_ARG_ENABLE_RETRIES); chand->enable_retries = grpc_channel_arg_get_bool(arg, true); // Record client channel factory. - arg = grpc_channel_args_find(args->channel_args, - GRPC_ARG_CLIENT_CHANNEL_FACTORY); - if (arg == nullptr) { + chand->client_channel_factory = + grpc_core::ClientChannelFactory::GetFromChannelArgs(args->channel_args); + if (chand->client_channel_factory == nullptr) { return GRPC_ERROR_CREATE_FROM_STATIC_STRING( "Missing client channel factory in args for client channel filter"); } - if (arg->type != GRPC_ARG_POINTER) { - return GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "client channel factory arg must be a pointer"); - } - chand->client_channel_factory = - static_cast(arg->value.pointer.p); - grpc_client_channel_factory_ref(chand->client_channel_factory); // Get server name to resolve, using proxy mapper if needed. arg = grpc_channel_args_find(args->channel_args, GRPC_ARG_SERVER_URI); if (arg == nullptr) { @@ -509,9 +501,6 @@ static void cc_destroy_channel_elem(grpc_channel_element* elem) { // longer be any need to explicitly reset these smart pointer data members. chand->picker.reset(); chand->subchannel_pool.reset(); - if (chand->client_channel_factory != nullptr) { - grpc_client_channel_factory_unref(chand->client_channel_factory); - } chand->info_lb_policy_name.reset(); chand->info_service_config_json.reset(); chand->retry_throttle_data.reset(); diff --git a/src/core/ext/filters/client_channel/client_channel_factory.cc b/src/core/ext/filters/client_channel/client_channel_factory.cc index 8c558382fdf..671a38430ef 100644 --- a/src/core/ext/filters/client_channel/client_channel_factory.cc +++ b/src/core/ext/filters/client_channel/client_channel_factory.cc @@ -21,47 +21,35 @@ #include "src/core/ext/filters/client_channel/client_channel_factory.h" #include "src/core/lib/channel/channel_args.h" -void grpc_client_channel_factory_ref(grpc_client_channel_factory* factory) { - factory->vtable->ref(factory); -} +// Channel arg key for client channel factory. +#define GRPC_ARG_CLIENT_CHANNEL_FACTORY "grpc.client_channel_factory" -void grpc_client_channel_factory_unref(grpc_client_channel_factory* factory) { - factory->vtable->unref(factory); -} +namespace grpc_core { -grpc_core::Subchannel* grpc_client_channel_factory_create_subchannel( - grpc_client_channel_factory* factory, const grpc_channel_args* args) { - return factory->vtable->create_subchannel(factory, args); -} +namespace { -grpc_channel* grpc_client_channel_factory_create_channel( - grpc_client_channel_factory* factory, const char* target, - grpc_client_channel_type type, const grpc_channel_args* args) { - return factory->vtable->create_client_channel(factory, target, type, args); +void* factory_arg_copy(void* f) { return f; } +void factory_arg_destroy(void* f) {} +int factory_arg_cmp(void* factory1, void* factory2) { + return GPR_ICMP(factory1, factory2); } +const grpc_arg_pointer_vtable factory_arg_vtable = { + factory_arg_copy, factory_arg_destroy, factory_arg_cmp}; -static void* factory_arg_copy(void* factory) { - grpc_client_channel_factory_ref( - static_cast(factory)); - return factory; -} +} // namespace -static void factory_arg_destroy(void* factory) { - grpc_client_channel_factory_unref( - static_cast(factory)); +grpc_arg ClientChannelFactory::CreateChannelArg(ClientChannelFactory* factory) { + return grpc_channel_arg_pointer_create( + const_cast(GRPC_ARG_CLIENT_CHANNEL_FACTORY), factory, + &factory_arg_vtable); } -static int factory_arg_cmp(void* factory1, void* factory2) { - if (factory1 < factory2) return -1; - if (factory1 > factory2) return 1; - return 0; +ClientChannelFactory* ClientChannelFactory::GetFromChannelArgs( + const grpc_channel_args* args) { + const grpc_arg* arg = + grpc_channel_args_find(args, GRPC_ARG_CLIENT_CHANNEL_FACTORY); + if (arg == nullptr || arg->type != GRPC_ARG_POINTER) return nullptr; + return static_cast(arg->value.pointer.p); } -static const grpc_arg_pointer_vtable factory_arg_vtable = { - factory_arg_copy, factory_arg_destroy, factory_arg_cmp}; - -grpc_arg grpc_client_channel_factory_create_channel_arg( - grpc_client_channel_factory* factory) { - return grpc_channel_arg_pointer_create((char*)GRPC_ARG_CLIENT_CHANNEL_FACTORY, - factory, &factory_arg_vtable); -} +} // namespace grpc_core diff --git a/src/core/ext/filters/client_channel/client_channel_factory.h b/src/core/ext/filters/client_channel/client_channel_factory.h index 4b72aa46499..21f78a833df 100644 --- a/src/core/ext/filters/client_channel/client_channel_factory.h +++ b/src/core/ext/filters/client_channel/client_channel_factory.h @@ -24,51 +24,32 @@ #include #include "src/core/ext/filters/client_channel/subchannel.h" -#include "src/core/lib/channel/channel_stack.h" +#include "src/core/lib/gprpp/abstract.h" -// Channel arg key for client channel factory. -#define GRPC_ARG_CLIENT_CHANNEL_FACTORY "grpc.client_channel_factory" +namespace grpc_core { -typedef struct grpc_client_channel_factory grpc_client_channel_factory; -typedef struct grpc_client_channel_factory_vtable - grpc_client_channel_factory_vtable; +class ClientChannelFactory { + public: + virtual ~ClientChannelFactory() = default; -typedef enum { - GRPC_CLIENT_CHANNEL_TYPE_REGULAR, /** for the user-level regular calls */ - GRPC_CLIENT_CHANNEL_TYPE_LOAD_BALANCING, /** for communication with a load - balancing service */ -} grpc_client_channel_type; + // Creates a subchannel with the specified args. + virtual Subchannel* CreateSubchannel(const grpc_channel_args* args) + GRPC_ABSTRACT; -/** Constructor for new configured channels. - Creating decorators around this type is encouraged to adapt behavior. */ -struct grpc_client_channel_factory { - const grpc_client_channel_factory_vtable* vtable; -}; - -struct grpc_client_channel_factory_vtable { - void (*ref)(grpc_client_channel_factory* factory); - void (*unref)(grpc_client_channel_factory* factory); - grpc_core::Subchannel* (*create_subchannel)( - grpc_client_channel_factory* factory, const grpc_channel_args* args); - grpc_channel* (*create_client_channel)(grpc_client_channel_factory* factory, - const char* target, - grpc_client_channel_type type, - const grpc_channel_args* args); -}; + // Creates a channel for the specified target with the specified args. + virtual grpc_channel* CreateChannel( + const char* target, const grpc_channel_args* args) GRPC_ABSTRACT; -void grpc_client_channel_factory_ref(grpc_client_channel_factory* factory); -void grpc_client_channel_factory_unref(grpc_client_channel_factory* factory); + // Returns a channel arg containing the specified factory. + static grpc_arg CreateChannelArg(ClientChannelFactory* factory); -/** Create a new grpc_subchannel */ -grpc_core::Subchannel* grpc_client_channel_factory_create_subchannel( - grpc_client_channel_factory* factory, const grpc_channel_args* args); + // Returns the factory from args, or null if not found. + static ClientChannelFactory* GetFromChannelArgs( + const grpc_channel_args* args); -/** Create a new grpc_channel */ -grpc_channel* grpc_client_channel_factory_create_channel( - grpc_client_channel_factory* factory, const char* target, - grpc_client_channel_type type, const grpc_channel_args* args); + GRPC_ABSTRACT_BASE_CLASS +}; -grpc_arg grpc_client_channel_factory_create_channel_arg( - grpc_client_channel_factory* factory); +} // namespace grpc_core #endif /* GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_CLIENT_CHANNEL_FACTORY_H */ diff --git a/src/core/ext/filters/client_channel/lb_policy.h b/src/core/ext/filters/client_channel/lb_policy.h index 813bad1b7f0..1bb8c5e96c0 100644 --- a/src/core/ext/filters/client_channel/lb_policy.h +++ b/src/core/ext/filters/client_channel/lb_policy.h @@ -22,7 +22,6 @@ #include #include "src/core/ext/filters/client_channel/client_channel_channelz.h" -#include "src/core/ext/filters/client_channel/client_channel_factory.h" #include "src/core/ext/filters/client_channel/subchannel.h" #include "src/core/lib/gprpp/abstract.h" #include "src/core/lib/gprpp/orphanable.h" @@ -193,10 +192,9 @@ class LoadBalancingPolicy : public InternallyRefCounted { virtual Subchannel* CreateSubchannel(const grpc_channel_args& args) GRPC_ABSTRACT; - /// Creates a channel with the specified target, type, and channel args. + /// Creates a channel with the specified target and channel args. virtual grpc_channel* CreateChannel( - const char* target, grpc_client_channel_type type, - const grpc_channel_args& args) GRPC_ABSTRACT; + const char* target, const grpc_channel_args& args) GRPC_ABSTRACT; /// Sets the connectivity state and returns a new picker to be used /// by the client channel. diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc index 90398aac7f4..4cd4a51b9c5 100644 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc +++ b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc @@ -273,7 +273,6 @@ class GrpcLb : public LoadBalancingPolicy { Subchannel* CreateSubchannel(const grpc_channel_args& args) override; grpc_channel* CreateChannel(const char* target, - grpc_client_channel_type type, const grpc_channel_args& args) override; void UpdateState(grpc_connectivity_state state, grpc_error* state_error, UniquePtr picker) override; @@ -581,10 +580,9 @@ Subchannel* GrpcLb::Helper::CreateSubchannel(const grpc_channel_args& args) { } grpc_channel* GrpcLb::Helper::CreateChannel(const char* target, - grpc_client_channel_type type, const grpc_channel_args& args) { if (parent_->shutting_down_) return nullptr; - return parent_->channel_control_helper()->CreateChannel(target, type, args); + return parent_->channel_control_helper()->CreateChannel(target, args); } void GrpcLb::Helper::UpdateState(grpc_connectivity_state state, @@ -1305,8 +1303,8 @@ void GrpcLb::ProcessChannelArgsLocked(const grpc_channel_args& args) { if (lb_channel_ == nullptr) { char* uri_str; gpr_asprintf(&uri_str, "fake:///%s", server_name_); - lb_channel_ = channel_control_helper()->CreateChannel( - uri_str, GRPC_CLIENT_CHANNEL_TYPE_LOAD_BALANCING, *lb_channel_args); + lb_channel_ = + channel_control_helper()->CreateChannel(uri_str, *lb_channel_args); GPR_ASSERT(lb_channel_ != nullptr); grpc_core::channelz::ChannelNode* channel_node = grpc_channel_get_channelz_node(lb_channel_); diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/xds.cc b/src/core/ext/filters/client_channel/lb_policy/xds/xds.cc index e1291da50af..078aebc133e 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/xds.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/xds.cc @@ -223,7 +223,6 @@ class XdsLb : public LoadBalancingPolicy { Subchannel* CreateSubchannel(const grpc_channel_args& args) override; grpc_channel* CreateChannel(const char* target, - grpc_client_channel_type type, const grpc_channel_args& args) override; void UpdateState(grpc_connectivity_state state, grpc_error* state_error, UniquePtr picker) override; @@ -354,10 +353,9 @@ Subchannel* XdsLb::Helper::CreateSubchannel(const grpc_channel_args& args) { } grpc_channel* XdsLb::Helper::CreateChannel(const char* target, - grpc_client_channel_type type, const grpc_channel_args& args) { if (parent_->shutting_down_) return nullptr; - return parent_->channel_control_helper()->CreateChannel(target, type, args); + return parent_->channel_control_helper()->CreateChannel(target, args); } void XdsLb::Helper::UpdateState(grpc_connectivity_state state, @@ -1076,8 +1074,8 @@ void XdsLb::ProcessChannelArgsLocked(const grpc_channel_args& args) { char* uri_str; gpr_asprintf(&uri_str, "fake:///%s", server_name_); gpr_mu_lock(&lb_channel_mu_); - lb_channel_ = channel_control_helper()->CreateChannel( - uri_str, GRPC_CLIENT_CHANNEL_TYPE_LOAD_BALANCING, *lb_channel_args); + lb_channel_ = + channel_control_helper()->CreateChannel(uri_str, *lb_channel_args); gpr_mu_unlock(&lb_channel_mu_); GPR_ASSERT(lb_channel_ != nullptr); gpr_free(uri_str); diff --git a/src/core/ext/filters/client_channel/resolving_lb_policy.cc b/src/core/ext/filters/client_channel/resolving_lb_policy.cc index 02a7af54588..a02a7e8acdb 100644 --- a/src/core/ext/filters/client_channel/resolving_lb_policy.cc +++ b/src/core/ext/filters/client_channel/resolving_lb_policy.cc @@ -80,10 +80,10 @@ class ResolvingLoadBalancingPolicy::ResolvingControlHelper return parent_->channel_control_helper()->CreateSubchannel(args); } - grpc_channel* CreateChannel(const char* target, grpc_client_channel_type type, + grpc_channel* CreateChannel(const char* target, const grpc_channel_args& args) override { if (parent_->resolver_ == nullptr) return nullptr; // Shutting down. - return parent_->channel_control_helper()->CreateChannel(target, type, args); + return parent_->channel_control_helper()->CreateChannel(target, args); } void UpdateState(grpc_connectivity_state state, grpc_error* state_error, diff --git a/src/core/ext/transport/chttp2/client/insecure/channel_create.cc b/src/core/ext/transport/chttp2/client/insecure/channel_create.cc index 8aabcfa2000..d77799cef70 100644 --- a/src/core/ext/transport/chttp2/client/insecure/channel_create.cc +++ b/src/core/ext/transport/chttp2/client/insecure/channel_create.cc @@ -33,50 +33,53 @@ #include "src/core/lib/surface/api_trace.h" #include "src/core/lib/surface/channel.h" -static void client_channel_factory_ref( - grpc_client_channel_factory* cc_factory) {} +namespace grpc_core { -static void client_channel_factory_unref( - grpc_client_channel_factory* cc_factory) {} - -static grpc_core::Subchannel* client_channel_factory_create_subchannel( - grpc_client_channel_factory* cc_factory, const grpc_channel_args* args) { - grpc_channel_args* new_args = grpc_default_authority_add_if_not_present(args); - grpc_connector* connector = grpc_chttp2_connector_create(); - grpc_core::Subchannel* s = grpc_core::Subchannel::Create(connector, new_args); - grpc_connector_unref(connector); - grpc_channel_args_destroy(new_args); - return s; -} +class Chttp2InsecureClientChannelFactory : public ClientChannelFactory { + public: + Subchannel* CreateSubchannel(const grpc_channel_args* args) override { + grpc_channel_args* new_args = + grpc_default_authority_add_if_not_present(args); + grpc_connector* connector = grpc_chttp2_connector_create(); + Subchannel* s = Subchannel::Create(connector, new_args); + grpc_connector_unref(connector); + grpc_channel_args_destroy(new_args); + return s; + } -static grpc_channel* client_channel_factory_create_channel( - grpc_client_channel_factory* cc_factory, const char* target, - grpc_client_channel_type type, const grpc_channel_args* args) { - if (target == nullptr) { - gpr_log(GPR_ERROR, "cannot create channel with NULL target name"); - return nullptr; + grpc_channel* CreateChannel(const char* target, + const grpc_channel_args* args) override { + if (target == nullptr) { + gpr_log(GPR_ERROR, "cannot create channel with NULL target name"); + return nullptr; + } + // Add channel arg containing the server URI. + UniquePtr canonical_target = + ResolverRegistry::AddDefaultPrefixIfNeeded(target); + grpc_arg arg = grpc_channel_arg_string_create( + const_cast(GRPC_ARG_SERVER_URI), canonical_target.get()); + const char* to_remove[] = {GRPC_ARG_SERVER_URI}; + grpc_channel_args* new_args = + grpc_channel_args_copy_and_add_and_remove(args, to_remove, 1, &arg, 1); + grpc_channel* channel = + grpc_channel_create(target, new_args, GRPC_CLIENT_CHANNEL, nullptr); + grpc_channel_args_destroy(new_args); + return channel; } - // Add channel arg containing the server URI. - grpc_core::UniquePtr canonical_target = - grpc_core::ResolverRegistry::AddDefaultPrefixIfNeeded(target); - grpc_arg arg = grpc_channel_arg_string_create( - const_cast(GRPC_ARG_SERVER_URI), canonical_target.get()); - const char* to_remove[] = {GRPC_ARG_SERVER_URI}; - grpc_channel_args* new_args = - grpc_channel_args_copy_and_add_and_remove(args, to_remove, 1, &arg, 1); - grpc_channel* channel = - grpc_channel_create(target, new_args, GRPC_CLIENT_CHANNEL, nullptr); - grpc_channel_args_destroy(new_args); - return channel; -} +}; -static const grpc_client_channel_factory_vtable client_channel_factory_vtable = - {client_channel_factory_ref, client_channel_factory_unref, - client_channel_factory_create_subchannel, - client_channel_factory_create_channel}; +} // namespace grpc_core -static grpc_client_channel_factory client_channel_factory = { - &client_channel_factory_vtable}; +namespace { + +grpc_core::Chttp2InsecureClientChannelFactory* g_factory; +gpr_once g_factory_once; + +void FactoryInit() { + g_factory = grpc_core::New(); +} + +} // namespace /* Create a client channel: Asynchronously: - resolve target @@ -91,16 +94,13 @@ grpc_channel* grpc_insecure_channel_create(const char* target, (target, args, reserved)); GPR_ASSERT(reserved == nullptr); // Add channel arg containing the client channel factory. - grpc_arg arg = - grpc_client_channel_factory_create_channel_arg(&client_channel_factory); + gpr_once_init(&g_factory_once, FactoryInit); + grpc_arg arg = grpc_core::ClientChannelFactory::CreateChannelArg(g_factory); grpc_channel_args* new_args = grpc_channel_args_copy_and_add(args, &arg, 1); // Create channel. - grpc_channel* channel = client_channel_factory_create_channel( - &client_channel_factory, target, GRPC_CLIENT_CHANNEL_TYPE_REGULAR, - new_args); + grpc_channel* channel = g_factory->CreateChannel(target, new_args); // Clean up. grpc_channel_args_destroy(new_args); - return channel != nullptr ? channel : grpc_lame_client_channel_create( target, GRPC_STATUS_INTERNAL, diff --git a/src/core/ext/transport/chttp2/client/secure/secure_channel_create.cc b/src/core/ext/transport/chttp2/client/secure/secure_channel_create.cc index eb2fee2af91..6277859c59c 100644 --- a/src/core/ext/transport/chttp2/client/secure/secure_channel_create.cc +++ b/src/core/ext/transport/chttp2/client/secure/secure_channel_create.cc @@ -40,148 +40,148 @@ #include "src/core/lib/surface/channel.h" #include "src/core/lib/uri/uri_parser.h" -static void client_channel_factory_ref( - grpc_client_channel_factory* cc_factory) {} +namespace grpc_core { -static void client_channel_factory_unref( - grpc_client_channel_factory* cc_factory) {} - -static grpc_channel_args* get_secure_naming_channel_args( - const grpc_channel_args* args) { - grpc_channel_credentials* channel_credentials = - grpc_channel_credentials_find_in_args(args); - if (channel_credentials == nullptr) { - gpr_log(GPR_ERROR, - "Can't create subchannel: channel credentials missing for secure " - "channel."); - return nullptr; - } - // Make sure security connector does not already exist in args. - if (grpc_security_connector_find_in_args(args) != nullptr) { - gpr_log(GPR_ERROR, - "Can't create subchannel: security connector already present in " - "channel args."); - return nullptr; - } - // To which address are we connecting? By default, use the server URI. - const grpc_arg* server_uri_arg = - grpc_channel_args_find(args, GRPC_ARG_SERVER_URI); - const char* server_uri_str = grpc_channel_arg_get_string(server_uri_arg); - GPR_ASSERT(server_uri_str != nullptr); - grpc_uri* server_uri = - grpc_uri_parse(server_uri_str, true /* supress errors */); - GPR_ASSERT(server_uri != nullptr); - const grpc_core::TargetAuthorityTable* target_authority_table = - grpc_core::FindTargetAuthorityTableInArgs(args); - grpc_core::UniquePtr authority; - if (target_authority_table != nullptr) { - // Find the authority for the target. - const char* target_uri_str = - grpc_core::Subchannel::GetUriFromSubchannelAddressArg(args); - grpc_uri* target_uri = - grpc_uri_parse(target_uri_str, false /* suppress errors */); - GPR_ASSERT(target_uri != nullptr); - if (target_uri->path[0] != '\0') { // "path" may be empty - const grpc_slice key = grpc_slice_from_static_string( - target_uri->path[0] == '/' ? target_uri->path + 1 : target_uri->path); - const grpc_core::UniquePtr* value = - target_authority_table->Get(key); - if (value != nullptr) authority.reset(gpr_strdup(value->get())); - grpc_slice_unref_internal(key); +class Chttp2SecureClientChannelFactory : public ClientChannelFactory { + public: + Subchannel* CreateSubchannel(const grpc_channel_args* args) override { + grpc_channel_args* new_args = GetSecureNamingChannelArgs(args); + if (new_args == nullptr) { + gpr_log(GPR_ERROR, + "Failed to create channel args during subchannel creation."); + return nullptr; } - grpc_uri_destroy(target_uri); - } - // If the authority hasn't already been set (either because no target - // authority table was present or because the target was not present - // in the table), fall back to using the original server URI. - if (authority == nullptr) { - authority = - grpc_core::ResolverRegistry::GetDefaultAuthority(server_uri_str); + grpc_connector* connector = grpc_chttp2_connector_create(); + Subchannel* s = Subchannel::Create(connector, new_args); + grpc_connector_unref(connector); + grpc_channel_args_destroy(new_args); + return s; } - grpc_arg args_to_add[2]; - size_t num_args_to_add = 0; - if (grpc_channel_args_find(args, GRPC_ARG_DEFAULT_AUTHORITY) == nullptr) { - // If the channel args don't already contain GRPC_ARG_DEFAULT_AUTHORITY, add - // the arg, setting it to the value just obtained. - args_to_add[num_args_to_add++] = grpc_channel_arg_string_create( - const_cast(GRPC_ARG_DEFAULT_AUTHORITY), authority.get()); + + grpc_channel* CreateChannel(const char* target, + const grpc_channel_args* args) override { + if (target == nullptr) { + gpr_log(GPR_ERROR, "cannot create channel with NULL target name"); + return nullptr; + } + // Add channel arg containing the server URI. + UniquePtr canonical_target = + ResolverRegistry::AddDefaultPrefixIfNeeded(target); + grpc_arg arg = grpc_channel_arg_string_create( + const_cast(GRPC_ARG_SERVER_URI), canonical_target.get()); + const char* to_remove[] = {GRPC_ARG_SERVER_URI}; + grpc_channel_args* new_args = + grpc_channel_args_copy_and_add_and_remove(args, to_remove, 1, &arg, 1); + grpc_channel* channel = + grpc_channel_create(target, new_args, GRPC_CLIENT_CHANNEL, nullptr); + grpc_channel_args_destroy(new_args); + return channel; } - grpc_channel_args* args_with_authority = - grpc_channel_args_copy_and_add(args, args_to_add, num_args_to_add); - grpc_uri_destroy(server_uri); - // Create the security connector using the credentials and target name. - grpc_channel_args* new_args_from_connector = nullptr; - grpc_core::RefCountedPtr - subchannel_security_connector = - channel_credentials->create_security_connector( - /*call_creds=*/nullptr, authority.get(), args_with_authority, - &new_args_from_connector); - if (subchannel_security_connector == nullptr) { - gpr_log(GPR_ERROR, - "Failed to create secure subchannel for secure name '%s'", - authority.get()); + + private: + static grpc_channel_args* GetSecureNamingChannelArgs( + const grpc_channel_args* args) { + grpc_channel_credentials* channel_credentials = + grpc_channel_credentials_find_in_args(args); + if (channel_credentials == nullptr) { + gpr_log(GPR_ERROR, + "Can't create subchannel: channel credentials missing for secure " + "channel."); + return nullptr; + } + // Make sure security connector does not already exist in args. + if (grpc_security_connector_find_in_args(args) != nullptr) { + gpr_log(GPR_ERROR, + "Can't create subchannel: security connector already present in " + "channel args."); + return nullptr; + } + // To which address are we connecting? By default, use the server URI. + const grpc_arg* server_uri_arg = + grpc_channel_args_find(args, GRPC_ARG_SERVER_URI); + const char* server_uri_str = grpc_channel_arg_get_string(server_uri_arg); + GPR_ASSERT(server_uri_str != nullptr); + grpc_uri* server_uri = + grpc_uri_parse(server_uri_str, true /* suppress errors */); + GPR_ASSERT(server_uri != nullptr); + const TargetAuthorityTable* target_authority_table = + FindTargetAuthorityTableInArgs(args); + UniquePtr authority; + if (target_authority_table != nullptr) { + // Find the authority for the target. + const char* target_uri_str = + Subchannel::GetUriFromSubchannelAddressArg(args); + grpc_uri* target_uri = + grpc_uri_parse(target_uri_str, false /* suppress errors */); + GPR_ASSERT(target_uri != nullptr); + if (target_uri->path[0] != '\0') { // "path" may be empty + const grpc_slice key = grpc_slice_from_static_string( + target_uri->path[0] == '/' ? target_uri->path + 1 + : target_uri->path); + const UniquePtr* value = target_authority_table->Get(key); + if (value != nullptr) authority.reset(gpr_strdup(value->get())); + grpc_slice_unref_internal(key); + } + grpc_uri_destroy(target_uri); + } + // If the authority hasn't already been set (either because no target + // authority table was present or because the target was not present + // in the table), fall back to using the original server URI. + if (authority == nullptr) { + authority = ResolverRegistry::GetDefaultAuthority(server_uri_str); + } + grpc_arg args_to_add[2]; + size_t num_args_to_add = 0; + if (grpc_channel_args_find(args, GRPC_ARG_DEFAULT_AUTHORITY) == nullptr) { + // If the channel args don't already contain GRPC_ARG_DEFAULT_AUTHORITY, + // add the arg, setting it to the value just obtained. + args_to_add[num_args_to_add++] = grpc_channel_arg_string_create( + const_cast(GRPC_ARG_DEFAULT_AUTHORITY), authority.get()); + } + grpc_channel_args* args_with_authority = + grpc_channel_args_copy_and_add(args, args_to_add, num_args_to_add); + grpc_uri_destroy(server_uri); + // Create the security connector using the credentials and target name. + grpc_channel_args* new_args_from_connector = nullptr; + RefCountedPtr + subchannel_security_connector = + channel_credentials->create_security_connector( + /*call_creds=*/nullptr, authority.get(), args_with_authority, + &new_args_from_connector); + if (subchannel_security_connector == nullptr) { + gpr_log(GPR_ERROR, + "Failed to create secure subchannel for secure name '%s'", + authority.get()); + grpc_channel_args_destroy(args_with_authority); + return nullptr; + } + grpc_arg new_security_connector_arg = + grpc_security_connector_to_arg(subchannel_security_connector.get()); + grpc_channel_args* new_args = grpc_channel_args_copy_and_add( + new_args_from_connector != nullptr ? new_args_from_connector + : args_with_authority, + &new_security_connector_arg, 1); + subchannel_security_connector.reset(DEBUG_LOCATION, "lb_channel_create"); + if (new_args_from_connector != nullptr) { + grpc_channel_args_destroy(new_args_from_connector); + } grpc_channel_args_destroy(args_with_authority); - return nullptr; + return new_args; } - grpc_arg new_security_connector_arg = - grpc_security_connector_to_arg(subchannel_security_connector.get()); +}; - grpc_channel_args* new_args = grpc_channel_args_copy_and_add( - new_args_from_connector != nullptr ? new_args_from_connector - : args_with_authority, - &new_security_connector_arg, 1); +} // namespace grpc_core - subchannel_security_connector.reset(DEBUG_LOCATION, "lb_channel_create"); - if (new_args_from_connector != nullptr) { - grpc_channel_args_destroy(new_args_from_connector); - } - grpc_channel_args_destroy(args_with_authority); - return new_args; -} +namespace { -static grpc_core::Subchannel* client_channel_factory_create_subchannel( - grpc_client_channel_factory* cc_factory, const grpc_channel_args* args) { - grpc_channel_args* new_args = get_secure_naming_channel_args(args); - if (new_args == nullptr) { - gpr_log(GPR_ERROR, - "Failed to create channel args during subchannel creation."); - return nullptr; - } - grpc_connector* connector = grpc_chttp2_connector_create(); - grpc_core::Subchannel* s = grpc_core::Subchannel::Create(connector, new_args); - grpc_connector_unref(connector); - grpc_channel_args_destroy(new_args); - return s; -} +grpc_core::Chttp2SecureClientChannelFactory* g_factory; +gpr_once g_factory_once; -static grpc_channel* client_channel_factory_create_channel( - grpc_client_channel_factory* cc_factory, const char* target, - grpc_client_channel_type type, const grpc_channel_args* args) { - if (target == nullptr) { - gpr_log(GPR_ERROR, "cannot create channel with NULL target name"); - return nullptr; - } - // Add channel arg containing the server URI. - grpc_core::UniquePtr canonical_target = - grpc_core::ResolverRegistry::AddDefaultPrefixIfNeeded(target); - grpc_arg arg = grpc_channel_arg_string_create((char*)GRPC_ARG_SERVER_URI, - canonical_target.get()); - const char* to_remove[] = {GRPC_ARG_SERVER_URI}; - grpc_channel_args* new_args = - grpc_channel_args_copy_and_add_and_remove(args, to_remove, 1, &arg, 1); - grpc_channel* channel = - grpc_channel_create(target, new_args, GRPC_CLIENT_CHANNEL, nullptr); - grpc_channel_args_destroy(new_args); - return channel; +void FactoryInit() { + g_factory = grpc_core::New(); } -static const grpc_client_channel_factory_vtable client_channel_factory_vtable = - {client_channel_factory_ref, client_channel_factory_unref, - client_channel_factory_create_subchannel, - client_channel_factory_create_channel}; - -static grpc_client_channel_factory client_channel_factory = { - &client_channel_factory_vtable}; +} // namespace // Create a secure client channel: // Asynchronously: - resolve target @@ -201,16 +201,15 @@ grpc_channel* grpc_secure_channel_create(grpc_channel_credentials* creds, if (creds != nullptr) { // Add channel args containing the client channel factory and channel // credentials. + gpr_once_init(&g_factory_once, FactoryInit); grpc_arg args_to_add[] = { - grpc_client_channel_factory_create_channel_arg(&client_channel_factory), + grpc_core::ClientChannelFactory::CreateChannelArg(g_factory), grpc_channel_credentials_to_arg(creds)}; grpc_channel_args* new_args = grpc_channel_args_copy_and_add( args, args_to_add, GPR_ARRAY_SIZE(args_to_add)); new_args = creds->update_arguments(new_args); // Create channel. - channel = client_channel_factory_create_channel( - &client_channel_factory, target, GRPC_CLIENT_CHANNEL_TYPE_REGULAR, - new_args); + channel = g_factory->CreateChannel(target, new_args); // Clean up. grpc_channel_args_destroy(new_args); } diff --git a/test/core/util/test_lb_policies.cc b/test/core/util/test_lb_policies.cc index 0a01e483f13..745162f637f 100644 --- a/test/core/util/test_lb_policies.cc +++ b/test/core/util/test_lb_policies.cc @@ -147,10 +147,8 @@ class InterceptRecvTrailingMetadataLoadBalancingPolicy } grpc_channel* CreateChannel(const char* target, - grpc_client_channel_type type, const grpc_channel_args& args) override { - return parent_->channel_control_helper()->CreateChannel(target, type, - args); + return parent_->channel_control_helper()->CreateChannel(target, args); } void UpdateState(grpc_connectivity_state state, grpc_error* state_error, diff --git a/test/cpp/microbenchmarks/bm_call_create.cc b/test/cpp/microbenchmarks/bm_call_create.cc index e57650fe5b7..c1c8651ba43 100644 --- a/test/cpp/microbenchmarks/bm_call_create.cc +++ b/test/cpp/microbenchmarks/bm_call_create.cc @@ -318,30 +318,18 @@ static void FilterDestroy(void* arg, grpc_error* error) { gpr_free(arg); } static void DoNothing(void* arg, grpc_error* error) {} -class FakeClientChannelFactory : public grpc_client_channel_factory { +class FakeClientChannelFactory : public grpc_core::ClientChannelFactory { public: - FakeClientChannelFactory() { vtable = &vtable_; } - - private: - static void NoRef(grpc_client_channel_factory* factory) {} - static void NoUnref(grpc_client_channel_factory* factory) {} - static grpc_core::Subchannel* CreateSubchannel( - grpc_client_channel_factory* factory, const grpc_channel_args* args) { + grpc_core::Subchannel* CreateSubchannel( + const grpc_channel_args* args) override { return nullptr; } - static grpc_channel* CreateClientChannel(grpc_client_channel_factory* factory, - const char* target, - grpc_client_channel_type type, - const grpc_channel_args* args) { + grpc_channel* CreateChannel(const char* target, + const grpc_channel_args* args) override { return nullptr; } - - static const grpc_client_channel_factory_vtable vtable_; }; -const grpc_client_channel_factory_vtable FakeClientChannelFactory::vtable_ = { - NoRef, NoUnref, CreateSubchannel, CreateClientChannel}; - static grpc_arg StringArg(const char* key, const char* value) { grpc_arg a; a.type = GRPC_ARG_STRING; @@ -506,13 +494,13 @@ static void BM_IsolatedFilter(benchmark::State& state) { TrackCounters track_counters; Fixture fixture; std::ostringstream label; - - std::vector args; FakeClientChannelFactory fake_client_channel_factory; - args.push_back(grpc_client_channel_factory_create_channel_arg( - &fake_client_channel_factory)); - args.push_back(StringArg(GRPC_ARG_SERVER_URI, "localhost")); + std::vector args = { + grpc_core::ClientChannelFactory::CreateChannelArg( + &fake_client_channel_factory), + StringArg(GRPC_ARG_SERVER_URI, "localhost"), + }; grpc_channel_args channel_args = {args.size(), &args[0]}; std::vector filters;