From 477ebef532b0163d87ba8c068c84d2adb67a3477 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Tue, 28 May 2019 14:18:15 -0700 Subject: [PATCH] Remove CreateChannel() method from LB helper API. --- .../filters/client_channel/client_channel.cc | 5 -- .../client_channel/client_channel_factory.h | 4 -- .../ext/filters/client_channel/lb_policy.h | 6 --- .../client_channel/lb_policy/grpclb/grpclb.cc | 18 ++----- .../lb_policy/grpclb/grpclb_channel.cc | 15 +++++- .../lb_policy/grpclb/grpclb_channel.h | 11 ++++- .../lb_policy/grpclb/grpclb_channel_secure.cc | 48 ++++++++++++------- .../client_channel/lb_policy/xds/xds.cc | 31 ++---------- .../lb_policy/xds/xds_channel.cc | 14 +++++- .../lb_policy/xds/xds_channel.h | 10 +++- .../lb_policy/xds/xds_channel_secure.cc | 41 +++++++++++----- .../client_channel/resolving_lb_policy.cc | 7 --- .../chttp2/client/insecure/channel_create.cc | 43 +++++++++-------- .../client/secure/secure_channel_create.cc | 45 +++++++++-------- test/core/util/test_lb_policies.cc | 5 -- test/cpp/microbenchmarks/bm_call_create.cc | 4 -- 16 files changed, 156 insertions(+), 151 deletions(-) diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index c0586d459b2..acbf1238113 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -960,11 +960,6 @@ class ChannelData::ClientChannelControlHelper return subchannel; } - grpc_channel* CreateChannel(const char* target, - const grpc_channel_args& args) override { - return chand_->client_channel_factory_->CreateChannel(target, &args); - } - void UpdateState( grpc_connectivity_state state, UniquePtr picker) override { 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 21f78a833df..42bc7cb505f 100644 --- a/src/core/ext/filters/client_channel/client_channel_factory.h +++ b/src/core/ext/filters/client_channel/client_channel_factory.h @@ -36,10 +36,6 @@ class ClientChannelFactory { virtual Subchannel* CreateSubchannel(const grpc_channel_args* args) GRPC_ABSTRACT; - // 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; - // Returns a channel arg containing the specified factory. static grpc_arg CreateChannelArg(ClientChannelFactory* factory); diff --git a/src/core/ext/filters/client_channel/lb_policy.h b/src/core/ext/filters/client_channel/lb_policy.h index 2ac7df63b7d..237b5930d4a 100644 --- a/src/core/ext/filters/client_channel/lb_policy.h +++ b/src/core/ext/filters/client_channel/lb_policy.h @@ -191,12 +191,6 @@ class LoadBalancingPolicy : public InternallyRefCounted { virtual Subchannel* CreateSubchannel(const grpc_channel_args& args) GRPC_ABSTRACT; - /// Creates a channel with the specified target and channel args. - /// This can be used in cases where the LB policy needs to create a - /// channel for its own use (e.g., to talk to an external load balancer). - virtual grpc_channel* CreateChannel( - 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. virtual void UpdateState(grpc_connectivity_state state, 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 ed6e8de3f21..26c9fdfc9c1 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 @@ -294,8 +294,6 @@ class GrpcLb : public LoadBalancingPolicy { : parent_(std::move(parent)) {} Subchannel* CreateSubchannel(const grpc_channel_args& args) override; - grpc_channel* CreateChannel(const char* target, - const grpc_channel_args& args) override; void UpdateState(grpc_connectivity_state state, UniquePtr picker) override; void RequestReresolution() override; @@ -625,15 +623,6 @@ Subchannel* GrpcLb::Helper::CreateSubchannel(const grpc_channel_args& args) { return parent_->channel_control_helper()->CreateSubchannel(args); } -grpc_channel* GrpcLb::Helper::CreateChannel(const char* target, - const grpc_channel_args& args) { - if (parent_->shutting_down_ || - (!CalledByPendingChild() && !CalledByCurrentChild())) { - return nullptr; - } - return parent_->channel_control_helper()->CreateChannel(target, args); -} - void GrpcLb::Helper::UpdateState(grpc_connectivity_state state, UniquePtr picker) { if (parent_->shutting_down_) return; @@ -1232,7 +1221,7 @@ grpc_channel_args* BuildBalancerChannelArgs( // the LB channel. GRPC_ARG_FAKE_RESOLVER_RESPONSE_GENERATOR, // The LB channel should use the authority indicated by the target - // authority table (see \a grpc_lb_policy_grpclb_modify_lb_channel_args), + // authority table (see \a ModifyGrpclbBalancerChannelArgs), // as opposed to the authority from the parent channel. GRPC_ARG_DEFAULT_AUTHORITY, // Just as for \a GRPC_ARG_DEFAULT_AUTHORITY, the LB channel should be @@ -1259,7 +1248,7 @@ grpc_channel_args* BuildBalancerChannelArgs( args, args_to_remove, GPR_ARRAY_SIZE(args_to_remove), args_to_add, GPR_ARRAY_SIZE(args_to_add)); // Make any necessary modifications for security. - return grpc_lb_policy_grpclb_modify_lb_channel_args(addresses, new_args); + return ModifyGrpclbBalancerChannelArgs(addresses, new_args); } // @@ -1464,8 +1453,7 @@ void GrpcLb::ProcessAddressesAndChannelArgsLocked( if (lb_channel_ == nullptr) { char* uri_str; gpr_asprintf(&uri_str, "fake:///%s", server_name_); - lb_channel_ = - channel_control_helper()->CreateChannel(uri_str, *lb_channel_args); + lb_channel_ = CreateGrpclbBalancerChannel(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/grpclb/grpclb_channel.cc b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel.cc index b713e26713f..c7237632c0a 100644 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel.cc +++ b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel.cc @@ -18,9 +18,20 @@ #include +#include + #include "src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel.h" -grpc_channel_args* grpc_lb_policy_grpclb_modify_lb_channel_args( - const grpc_core::ServerAddressList& addresses, grpc_channel_args* args) { +namespace grpc_core { + +grpc_channel_args* ModifyGrpclbBalancerChannelArgs( + const ServerAddressList& addresses, grpc_channel_args* args) { return args; } + +grpc_channel* CreateGrpclbBalancerChannel(const char* target_uri, + const grpc_channel_args& args) { + return grpc_insecure_channel_create(target_uri, &args, nullptr); +} + +} // namespace grpc_core diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel.h b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel.h index c78ba36cf1d..1458233022f 100644 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel.h +++ b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel.h @@ -25,14 +25,21 @@ #include "src/core/ext/filters/client_channel/server_address.h" +namespace grpc_core { + /// Makes any necessary modifications to \a args for use in the grpclb /// balancer channel. /// /// Takes ownership of \a args. /// /// Caller takes ownership of the returned args. -grpc_channel_args* grpc_lb_policy_grpclb_modify_lb_channel_args( - const grpc_core::ServerAddressList& addresses, grpc_channel_args* args); +grpc_channel_args* ModifyGrpclbBalancerChannelArgs( + const ServerAddressList& addresses, grpc_channel_args* args); + +grpc_channel* CreateGrpclbBalancerChannel(const char* target_uri, + const grpc_channel_args& args); + +} // namespace grpc_core #endif /* GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_LB_POLICY_GRPCLB_GRPCLB_CHANNEL_H \ */ diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc index 892cdeb27b7..376209fa7e2 100644 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc +++ b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc @@ -22,6 +22,7 @@ #include +#include #include #include @@ -35,6 +36,7 @@ #include "src/core/lib/slice/slice_internal.h" namespace grpc_core { + namespace { int BalancerNameCmp(const grpc_core::UniquePtr& a, @@ -65,37 +67,49 @@ RefCountedPtr CreateTargetAuthorityTable( } } // namespace -} // namespace grpc_core -grpc_channel_args* grpc_lb_policy_grpclb_modify_lb_channel_args( - const grpc_core::ServerAddressList& addresses, grpc_channel_args* args) { - const char* args_to_remove[1]; - size_t num_args_to_remove = 0; - grpc_arg args_to_add[2]; - size_t num_args_to_add = 0; +grpc_channel_args* ModifyGrpclbBalancerChannelArgs( + const ServerAddressList& addresses, grpc_channel_args* args) { + InlinedVector args_to_remove; + InlinedVector args_to_add; // Add arg for targets info table. - grpc_core::RefCountedPtr - target_authority_table = grpc_core::CreateTargetAuthorityTable(addresses); - args_to_add[num_args_to_add++] = - grpc_core::CreateTargetAuthorityTableChannelArg( - target_authority_table.get()); + RefCountedPtr target_authority_table = + CreateTargetAuthorityTable(addresses); + args_to_add.emplace_back( + CreateTargetAuthorityTableChannelArg(target_authority_table.get())); // Substitute the channel credentials with a version without call // credentials: the load balancer is not necessarily trusted to handle // bearer token credentials. grpc_channel_credentials* channel_credentials = grpc_channel_credentials_find_in_args(args); - grpc_core::RefCountedPtr creds_sans_call_creds; + RefCountedPtr creds_sans_call_creds; if (channel_credentials != nullptr) { creds_sans_call_creds = channel_credentials->duplicate_without_call_credentials(); GPR_ASSERT(creds_sans_call_creds != nullptr); - args_to_remove[num_args_to_remove++] = GRPC_ARG_CHANNEL_CREDENTIALS; - args_to_add[num_args_to_add++] = - grpc_channel_credentials_to_arg(creds_sans_call_creds.get()); + args_to_remove.emplace_back(GRPC_ARG_CHANNEL_CREDENTIALS); + args_to_add.emplace_back( + grpc_channel_credentials_to_arg(creds_sans_call_creds.get())); } grpc_channel_args* result = grpc_channel_args_copy_and_add_and_remove( - args, args_to_remove, num_args_to_remove, args_to_add, num_args_to_add); + args, args_to_remove.data(), args_to_remove.size(), args_to_add.data(), + args_to_add.size()); // Clean up. grpc_channel_args_destroy(args); return result; } + +grpc_channel* CreateGrpclbBalancerChannel(const char* target_uri, + const grpc_channel_args& args) { + grpc_channel_credentials* creds = + grpc_channel_credentials_find_in_args(&args); + const char* arg_to_remove = GRPC_ARG_CHANNEL_CREDENTIALS; + grpc_channel_args* new_args = + grpc_channel_args_copy_and_remove(&args, &arg_to_remove, 1); + grpc_channel* channel = + grpc_secure_channel_create(creds, target_uri, new_args, nullptr); + grpc_channel_args_destroy(new_args); + return channel; +} + +} // namespace grpc_core 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 819bad6c00d..08ecec5c8ba 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 @@ -338,8 +338,6 @@ class XdsLb : public LoadBalancingPolicy { : parent_(std::move(parent)) {} Subchannel* CreateSubchannel(const grpc_channel_args& args) override; - grpc_channel* CreateChannel(const char* target, - const grpc_channel_args& args) override; void UpdateState(grpc_connectivity_state state, UniquePtr picker) override; void RequestReresolution() override; @@ -378,8 +376,6 @@ class XdsLb : public LoadBalancingPolicy { : entry_(std::move(entry)) {} Subchannel* CreateSubchannel(const grpc_channel_args& args) override; - grpc_channel* CreateChannel(const char* target, - const grpc_channel_args& args) override; void UpdateState(grpc_connectivity_state state, UniquePtr picker) override; void RequestReresolution() override; @@ -592,15 +588,6 @@ Subchannel* XdsLb::FallbackHelper::CreateSubchannel( return parent_->channel_control_helper()->CreateSubchannel(args); } -grpc_channel* XdsLb::FallbackHelper::CreateChannel( - const char* target, const grpc_channel_args& args) { - if (parent_->shutting_down_ || - (!CalledByPendingFallback() && !CalledByCurrentFallback())) { - return nullptr; - } - return parent_->channel_control_helper()->CreateChannel(target, args); -} - void XdsLb::FallbackHelper::UpdateState(grpc_connectivity_state state, UniquePtr picker) { if (parent_->shutting_down_) return; @@ -722,7 +709,7 @@ ServerAddressList ProcessServerlist(const xds_grpclb_serverlist* serverlist) { XdsLb::BalancerChannelState::BalancerChannelState( const char* balancer_name, const grpc_channel_args& args, - grpc_core::RefCountedPtr parent_xdslb_policy) + RefCountedPtr parent_xdslb_policy) : InternallyRefCounted(&grpc_lb_xds_trace), xdslb_policy_(std::move(parent_xdslb_policy)), lb_call_backoff_( @@ -735,8 +722,7 @@ XdsLb::BalancerChannelState::BalancerChannelState( GRPC_CLOSURE_INIT(&on_connectivity_changed_, &XdsLb::BalancerChannelState::OnConnectivityChangedLocked, this, grpc_combiner_scheduler(xdslb_policy_->combiner())); - channel_ = xdslb_policy_->channel_control_helper()->CreateChannel( - balancer_name, args); + channel_ = CreateXdsBalancerChannel(balancer_name, args); GPR_ASSERT(channel_ != nullptr); StartCallLocked(); } @@ -1325,7 +1311,7 @@ grpc_channel_args* BuildBalancerChannelArgs(const grpc_channel_args* args) { // factory will re-add this arg with the right value. GRPC_ARG_SERVER_URI, // The LB channel should use the authority indicated by the target - // authority table (see \a grpc_lb_policy_xds_modify_lb_channel_args), + // authority table (see \a ModifyXdsBalancerChannelArgs), // as opposed to the authority from the parent channel. GRPC_ARG_DEFAULT_AUTHORITY, // Just as for \a GRPC_ARG_DEFAULT_AUTHORITY, the LB channel should be @@ -1348,7 +1334,7 @@ grpc_channel_args* BuildBalancerChannelArgs(const grpc_channel_args* args) { args, args_to_remove, GPR_ARRAY_SIZE(args_to_remove), args_to_add, GPR_ARRAY_SIZE(args_to_add)); // Make any necessary modifications for security. - return grpc_lb_policy_xds_modify_lb_channel_args(new_args); + return ModifyXdsBalancerChannelArgs(new_args); } // @@ -2010,15 +1996,6 @@ Subchannel* XdsLb::LocalityMap::LocalityEntry::Helper::CreateSubchannel( return entry_->parent_->channel_control_helper()->CreateSubchannel(args); } -grpc_channel* XdsLb::LocalityMap::LocalityEntry::Helper::CreateChannel( - const char* target, const grpc_channel_args& args) { - if (entry_->parent_->shutting_down_ || - (!CalledByPendingChild() && !CalledByCurrentChild())) { - return nullptr; - } - return entry_->parent_->channel_control_helper()->CreateChannel(target, args); -} - void XdsLb::LocalityMap::LocalityEntry::Helper::UpdateState( grpc_connectivity_state state, UniquePtr picker) { if (entry_->parent_->shutting_down_) return; diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/xds_channel.cc b/src/core/ext/filters/client_channel/lb_policy/xds/xds_channel.cc index 0aa145a24e5..386517d6427 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/xds_channel.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/xds_channel.cc @@ -18,9 +18,19 @@ #include +#include + #include "src/core/ext/filters/client_channel/lb_policy/xds/xds_channel.h" -grpc_channel_args* grpc_lb_policy_xds_modify_lb_channel_args( - grpc_channel_args* args) { +namespace grpc_core { + +grpc_channel_args* ModifyXdsBalancerChannelArgs(grpc_channel_args* args) { return args; } + +grpc_channel* CreateXdsBalancerChannel(const char* target_uri, + const grpc_channel_args& args) { + return grpc_insecure_channel_create(target_uri, &args, nullptr); +} + +} // namespace grpc_core diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/xds_channel.h b/src/core/ext/filters/client_channel/lb_policy/xds/xds_channel.h index f713b7f563d..516bac1df25 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/xds_channel.h +++ b/src/core/ext/filters/client_channel/lb_policy/xds/xds_channel.h @@ -23,14 +23,20 @@ #include +namespace grpc_core { + /// Makes any necessary modifications to \a args for use in the xds /// balancer channel. /// /// Takes ownership of \a args. /// /// Caller takes ownership of the returned args. -grpc_channel_args* grpc_lb_policy_xds_modify_lb_channel_args( - grpc_channel_args* args); +grpc_channel_args* ModifyXdsBalancerChannelArgs(grpc_channel_args* args); + +grpc_channel* CreateXdsBalancerChannel(const char* target_uri, + const grpc_channel_args& args); + +} // namespace grpc_core #endif /* GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_LB_POLICY_XDS_XDS_CHANNEL_H \ */ diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_secure.cc b/src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_secure.cc index 7f8c232d6d0..ad5e7cbbc2e 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_secure.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_secure.cc @@ -20,9 +20,11 @@ #include "src/core/ext/filters/client_channel/lb_policy/xds/xds_channel.h" +#include + +#include #include #include -#include #include "src/core/ext/filters/client_channel/client_channel.h" #include "src/core/ext/filters/client_channel/server_address.h" @@ -33,29 +35,44 @@ #include "src/core/lib/security/transport/target_authority_table.h" #include "src/core/lib/slice/slice_internal.h" -grpc_channel_args* grpc_lb_policy_xds_modify_lb_channel_args( - grpc_channel_args* args) { - const char* args_to_remove[1]; - size_t num_args_to_remove = 0; - grpc_arg args_to_add[2]; - size_t num_args_to_add = 0; +namespace grpc_core { + +grpc_channel_args* ModifyXdsBalancerChannelArgs(grpc_channel_args* args) { + InlinedVector args_to_remove; + InlinedVector args_to_add; // Substitute the channel credentials with a version without call // credentials: the load balancer is not necessarily trusted to handle // bearer token credentials. grpc_channel_credentials* channel_credentials = grpc_channel_credentials_find_in_args(args); - grpc_core::RefCountedPtr creds_sans_call_creds; + RefCountedPtr creds_sans_call_creds; if (channel_credentials != nullptr) { creds_sans_call_creds = channel_credentials->duplicate_without_call_credentials(); GPR_ASSERT(creds_sans_call_creds != nullptr); - args_to_remove[num_args_to_remove++] = GRPC_ARG_CHANNEL_CREDENTIALS; - args_to_add[num_args_to_add++] = - grpc_channel_credentials_to_arg(creds_sans_call_creds.get()); + args_to_remove.emplace_back(GRPC_ARG_CHANNEL_CREDENTIALS); + args_to_add.emplace_back( + grpc_channel_credentials_to_arg(creds_sans_call_creds.get())); } grpc_channel_args* result = grpc_channel_args_copy_and_add_and_remove( - args, args_to_remove, num_args_to_remove, args_to_add, num_args_to_add); + args, args_to_remove.data(), args_to_remove.size(), args_to_add.data(), + args_to_add.size()); // Clean up. grpc_channel_args_destroy(args); return result; } + +grpc_channel* CreateXdsBalancerChannel(const char* target_uri, + const grpc_channel_args& args) { + grpc_channel_credentials* creds = + grpc_channel_credentials_find_in_args(&args); + const char* arg_to_remove = GRPC_ARG_CHANNEL_CREDENTIALS; + grpc_channel_args* new_args = + grpc_channel_args_copy_and_remove(&args, &arg_to_remove, 1); + grpc_channel* channel = + grpc_secure_channel_create(creds, target_uri, new_args, nullptr); + grpc_channel_args_destroy(new_args); + return channel; +} + +} // namespace grpc_core 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 4e383f65dd1..6f3cfe813e8 100644 --- a/src/core/ext/filters/client_channel/resolving_lb_policy.cc +++ b/src/core/ext/filters/client_channel/resolving_lb_policy.cc @@ -112,13 +112,6 @@ class ResolvingLoadBalancingPolicy::ResolvingControlHelper return parent_->channel_control_helper()->CreateSubchannel(args); } - grpc_channel* CreateChannel(const char* target, - const grpc_channel_args& args) override { - if (parent_->resolver_ == nullptr) return nullptr; // Shutting down. - if (!CalledByCurrentChild() && !CalledByPendingChild()) return nullptr; - return parent_->channel_control_helper()->CreateChannel(target, args); - } - void UpdateState(grpc_connectivity_state state, UniquePtr picker) override { if (parent_->resolver_ == nullptr) return; // Shutting down. 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 0d61abd2a01..cbd522d6e50 100644 --- a/src/core/ext/transport/chttp2/client/insecure/channel_create.cc +++ b/src/core/ext/transport/chttp2/client/insecure/channel_create.cc @@ -46,27 +46,30 @@ class Chttp2InsecureClientChannelFactory : public ClientChannelFactory { grpc_channel_args_destroy(new_args); return s; } +}; - 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; +namespace { + +grpc_channel* CreateChannel(const char* target, 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. + 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; +} + +} // namespace } // namespace grpc_core @@ -98,7 +101,7 @@ grpc_channel* grpc_insecure_channel_create(const char* target, 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 = g_factory->CreateChannel(target, new_args); + grpc_channel* channel = grpc_core::CreateChannel(target, new_args); // Clean up. grpc_channel_args_destroy(new_args); return channel != nullptr ? channel 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 bc38ff25c79..b18d742ed12 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 @@ -58,26 +58,6 @@ class Chttp2SecureClientChannelFactory : public ClientChannelFactory { return s; } - 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; - } - private: static grpc_channel_args* GetSecureNamingChannelArgs( const grpc_channel_args* args) { @@ -170,6 +150,29 @@ class Chttp2SecureClientChannelFactory : public ClientChannelFactory { } }; +namespace { + +grpc_channel* CreateChannel(const char* target, 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. + 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; +} + +} // namespace + } // namespace grpc_core namespace { @@ -209,7 +212,7 @@ grpc_channel* grpc_secure_channel_create(grpc_channel_credentials* creds, args, args_to_add, GPR_ARRAY_SIZE(args_to_add)); new_args = creds->update_arguments(new_args); // Create channel. - channel = g_factory->CreateChannel(target, new_args); + channel = grpc_core::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 b871f04bc9e..684e7495d36 100644 --- a/test/core/util/test_lb_policies.cc +++ b/test/core/util/test_lb_policies.cc @@ -145,11 +145,6 @@ class InterceptRecvTrailingMetadataLoadBalancingPolicy return parent_->channel_control_helper()->CreateSubchannel(args); } - grpc_channel* CreateChannel(const char* target, - const grpc_channel_args& args) override { - return parent_->channel_control_helper()->CreateChannel(target, args); - } - void UpdateState(grpc_connectivity_state state, UniquePtr picker) override { parent_->channel_control_helper()->UpdateState( diff --git a/test/cpp/microbenchmarks/bm_call_create.cc b/test/cpp/microbenchmarks/bm_call_create.cc index 3bd1464b2aa..e687c644516 100644 --- a/test/cpp/microbenchmarks/bm_call_create.cc +++ b/test/cpp/microbenchmarks/bm_call_create.cc @@ -322,10 +322,6 @@ class FakeClientChannelFactory : public grpc_core::ClientChannelFactory { const grpc_channel_args* args) override { return nullptr; } - grpc_channel* CreateChannel(const char* target, - const grpc_channel_args* args) override { - return nullptr; - } }; static grpc_arg StringArg(const char* key, const char* value) {