[LB policies] use helper GetAuthority() instead of GRPC_ARG_SERVER_URI channel arg (#33671)

pull/33747/head
Mark D. Roth 2 years ago committed by GitHub
parent 0adbe5708b
commit 6ad141ae8c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 2
      src/core/BUILD
  2. 28
      src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc
  3. 7
      src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.cc
  4. 4
      src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.h
  5. 29
      src/core/ext/filters/client_channel/lb_policy/rls/rls.cc

@ -3917,7 +3917,6 @@ grpc_cc_library(
"//:ref_counted_ptr",
"//:server_address",
"//:sockaddr_utils",
"//:uri_parser",
"//:work_serializer",
],
)
@ -4004,7 +4003,6 @@ grpc_cc_library(
"//:ref_counted_ptr",
"//:rls_upb",
"//:server_address",
"//:uri_parser",
"//:work_serializer",
],
)

@ -78,7 +78,6 @@
#include "absl/strings/str_format.h"
#include "absl/strings/str_join.h"
#include "absl/strings/string_view.h"
#include "absl/strings/strip.h"
#include "absl/types/optional.h"
#include "absl/types/variant.h"
#include "upb/upb.hpp"
@ -144,7 +143,6 @@
#include "src/core/lib/surface/channel_stack_type.h"
#include "src/core/lib/transport/connectivity_state.h"
#include "src/core/lib/transport/metadata_batch.h"
#include "src/core/lib/uri/uri_parser.h"
#define GRPC_GRPCLB_INITIAL_CONNECT_BACKOFF_SECONDS 1
#define GRPC_GRPCLB_RECONNECT_BACKOFF_MULTIPLIER 1.6
@ -523,8 +521,6 @@ class GrpcLb : public LoadBalancingPolicy {
void StartSubchannelCacheTimerLocked();
void OnSubchannelCacheTimerLocked();
// Who the client is trying to communicate with.
std::string server_name_;
// Configurations for the policy.
RefCountedPtr<GrpcLbConfig> config_;
@ -854,8 +850,6 @@ GrpcLb::BalancerCallState::BalancerCallState(
// Init the LB call. Note that the LB call will progress every time there's
// activity in grpclb_policy_->interested_parties(), which is comprised of
// the polling entities from client_channel.
GPR_ASSERT(!grpclb_policy()->server_name_.empty());
// Closure Initialization
GRPC_CLOSURE_INIT(&lb_on_initial_request_sent_, OnInitialRequestSent, this,
grpc_schedule_on_exec_ctx);
GRPC_CLOSURE_INIT(&lb_on_balancer_message_received_,
@ -877,8 +871,8 @@ GrpcLb::BalancerCallState::BalancerCallState(
upb::Arena arena;
grpc_slice request_payload_slice = GrpcLbRequestCreate(
grpclb_policy()->config_->service_name().empty()
? grpclb_policy()->server_name_.c_str()
: grpclb_policy()->config_->service_name().c_str(),
? grpclb_policy()->channel_control_helper()->GetAuthority()
: grpclb_policy()->config_->service_name(),
arena.ptr());
send_message_payload_ =
grpc_raw_byte_buffer_create(&request_payload_slice, 1);
@ -1372,10 +1366,6 @@ ChannelArgs BuildBalancerChannelArgs(
// Strip out the service config, since we don't want the LB policy
// config specified for the parent channel to affect the LB channel.
.Remove(GRPC_ARG_SERVICE_CONFIG)
// The channel arg for the server URI, since that will be different
// for the LB channel than for the parent channel. The client
// channel factory will re-add this arg with the right value.
.Remove(GRPC_ARG_SERVER_URI)
// The fake resolver response generator, because we are replacing it
// with the one from the grpclb policy, used to propagate updates to
// the LB channel.
@ -1411,16 +1401,8 @@ ChannelArgs BuildBalancerChannelArgs(
// ctor and dtor
//
std::string GetServerNameFromChannelArgs(const ChannelArgs& args) {
absl::StatusOr<URI> uri =
URI::Parse(args.GetString(GRPC_ARG_SERVER_URI).value());
GPR_ASSERT(uri.ok() && !uri->path().empty());
return std::string(absl::StripPrefix(uri->path(), "/"));
}
GrpcLb::GrpcLb(Args args)
: LoadBalancingPolicy(std::move(args)),
server_name_(GetServerNameFromChannelArgs(channel_args())),
response_generator_(MakeRefCounted<FakeResolverResponseGenerator>()),
lb_call_timeout_(std::max(
Duration::Zero(),
@ -1451,7 +1433,8 @@ GrpcLb::GrpcLb(Args args)
if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_glb_trace)) {
gpr_log(GPR_INFO,
"[grpclb %p] Will use '%s' as the server name for LB request.",
this, server_name_.c_str());
this,
std::string(channel_control_helper()->GetAuthority()).c_str());
}
}
@ -1581,7 +1564,8 @@ absl::Status GrpcLb::UpdateBalancerChannelLocked() {
BuildBalancerChannelArgs(response_generator_.get(), args_);
// Create balancer channel if needed.
if (lb_channel_ == nullptr) {
std::string uri_str = absl::StrCat("fake:///", server_name_);
std::string uri_str =
absl::StrCat("fake:///", channel_control_helper()->GetAuthority());
lb_channel_ =
grpc_channel_create(uri_str.c_str(), channel_credentials.get(),
lb_channel_args.ToC().get());

@ -59,15 +59,16 @@ grpc_slice grpc_grpclb_request_encode(
} // namespace
grpc_slice GrpcLbRequestCreate(const char* lb_service_name, upb_Arena* arena) {
grpc_slice GrpcLbRequestCreate(absl::string_view lb_service_name,
upb_Arena* arena) {
grpc_lb_v1_LoadBalanceRequest* req = grpc_lb_v1_LoadBalanceRequest_new(arena);
grpc_lb_v1_InitialLoadBalanceRequest* initial_request =
grpc_lb_v1_LoadBalanceRequest_mutable_initial_request(req, arena);
size_t name_len = std::min(strlen(lb_service_name),
size_t name_len = std::min(lb_service_name.size(),
size_t{GRPC_GRPCLB_SERVICE_NAME_MAX_LENGTH});
grpc_lb_v1_InitialLoadBalanceRequest_set_name(
initial_request,
upb_StringView_FromDataAndSize(lb_service_name, name_len));
upb_StringView_FromDataAndSize(lb_service_name.data(), name_len));
return grpc_grpclb_request_encode(req, arena);
}

@ -24,6 +24,7 @@
#include <vector>
#include "absl/strings/string_view.h"
#include "upb/mem/arena.h"
#include <grpc/slice.h>
@ -56,7 +57,8 @@ struct GrpcLbResponse {
};
// Creates a serialized grpclb request.
grpc_slice GrpcLbRequestCreate(const char* lb_service_name, upb_Arena* arena);
grpc_slice GrpcLbRequestCreate(absl::string_view lb_service_name,
upb_Arena* arena);
// Creates a serialized grpclb load report request.
grpc_slice GrpcLbLoadReportRequestCreate(

@ -48,7 +48,6 @@
#include "absl/strings/str_format.h"
#include "absl/strings/str_join.h"
#include "absl/strings/string_view.h"
#include "absl/strings/strip.h"
#include "absl/types/optional.h"
#include "upb/base/string_view.h"
#include "upb/upb.hpp"
@ -103,7 +102,6 @@
#include "src/core/lib/surface/channel.h"
#include "src/core/lib/transport/connectivity_state.h"
#include "src/core/lib/transport/error_utils.h"
#include "src/core/lib/uri/uri_parser.h"
#include "src/proto/grpc/lookup/v1/rls.upb.h"
namespace grpc_core {
@ -693,9 +691,6 @@ class RlsLb : public LoadBalancingPolicy {
// Updates the picker in the work serializer.
void UpdatePickerLocked() ABSL_LOCKS_EXCLUDED(&mu_);
// The name of the server for the channel.
std::string server_name_;
// Mutex to guard LB policy state that is accessed by the picker.
Mutex mu_;
bool is_shutdown_ ABSL_GUARDED_BY(mu_) = false;
@ -899,7 +894,7 @@ void RlsLb::ChildPolicyWrapper::ChildPolicyHelper::UpdateState(
// Builds the key to be used for a request based on path and initial_metadata.
std::map<std::string, std::string> BuildKeyMap(
const RlsLbConfig::KeyBuilderMap& key_builder_map, absl::string_view path,
const std::string& host,
absl::string_view host,
const LoadBalancingPolicy::MetadataInterface* initial_metadata) {
size_t last_slash_pos = path.npos; // May need this a few times, so cache it.
// Find key builder for this path.
@ -935,7 +930,7 @@ std::map<std::string, std::string> BuildKeyMap(
key_builder->constant_keys.end());
// Add host key.
if (!key_builder->host_key.empty()) {
key_map[key_builder->host_key] = host;
key_map[key_builder->host_key] = std::string(host);
}
// Add service key.
if (!key_builder->service_key.empty()) {
@ -970,9 +965,10 @@ RlsLb::Picker::Picker(RefCountedPtr<RlsLb> lb_policy)
LoadBalancingPolicy::PickResult RlsLb::Picker::Pick(PickArgs args) {
// Construct key for request.
RequestKey key = {BuildKeyMap(config_->key_builder_map(), args.path,
lb_policy_->server_name_,
args.initial_metadata)};
RequestKey key = {
BuildKeyMap(config_->key_builder_map(), args.path,
lb_policy_->channel_control_helper()->GetAuthority(),
args.initial_metadata)};
if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_rls_trace)) {
gpr_log(GPR_INFO, "[rlslb %p] picker=%p: request keys: %s",
lb_policy_.get(), this, key.ToString().c_str());
@ -1857,18 +1853,7 @@ RlsLb::ResponseInfo RlsLb::RlsRequest::ParseResponseProto() {
// RlsLb
//
std::string GetServerUri(const ChannelArgs& args) {
auto server_uri_str = args.GetString(GRPC_ARG_SERVER_URI);
GPR_ASSERT(server_uri_str.has_value());
absl::StatusOr<URI> uri = URI::Parse(*server_uri_str);
GPR_ASSERT(uri.ok());
return std::string(absl::StripPrefix(uri->path(), "/"));
}
RlsLb::RlsLb(Args args)
: LoadBalancingPolicy(std::move(args)),
server_name_(GetServerUri(channel_args())),
cache_(this) {
RlsLb::RlsLb(Args args) : LoadBalancingPolicy(std::move(args)), cache_(this) {
if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_rls_trace)) {
gpr_log(GPR_INFO, "[rlslb %p] policy created", this);
}

Loading…
Cancel
Save