Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

grpclb stabilization #20803

Merged
merged 1 commit into from Jan 10, 2020
Merged

grpclb stabilization #20803

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 18 additions & 0 deletions BUILD
View file
Open in desktop
Expand Up @@ -1231,6 +1231,21 @@ grpc_cc_library(
],
)

grpc_cc_library(
name = "grpc_grpclb_balancer_addresses",
srcs = [
"src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc",
],
hdrs = [
"src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h",
],
language = "c++",
deps = [
"grpc_base",
"grpc_client_channel",
],
)

grpc_cc_library(
name = "grpc_lb_policy_grpclb",
srcs = [
Expand All @@ -1251,6 +1266,7 @@ grpc_cc_library(
deps = [
"grpc_base",
"grpc_client_channel",
"grpc_grpclb_balancer_addresses",
"grpc_lb_upb",
"grpc_resolver_fake",
"grpc_transport_chttp2_client_insecure",
Expand All @@ -1277,6 +1293,7 @@ grpc_cc_library(
deps = [
"grpc_base",
"grpc_client_channel",
"grpc_grpclb_balancer_addresses",
"grpc_lb_upb",
"grpc_resolver_fake",
"grpc_secure",
Expand Down Expand Up @@ -1602,6 +1619,7 @@ grpc_cc_library(
deps = [
"grpc_base",
"grpc_client_channel",
"grpc_grpclb_balancer_addresses",
"grpc_resolver_dns_selection",
],
)
Expand Down
2 changes: 2 additions & 0 deletions BUILD.gn
View file
Open in desktop
Expand Up @@ -236,6 +236,8 @@ config("grpc_config") {
"src/core/ext/filters/client_channel/lb_policy/grpclb/client_load_reporting_filter.h",
"src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc",
"src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.h",
"src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc",
"src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h",
"src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel.h",
"src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc",
"src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_client_stats.cc",
Expand Down
2 changes: 2 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Expand Up @@ -1465,6 +1465,7 @@ add_library(grpc
src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc
src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_client_stats.cc
src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.cc
src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc
src/core/ext/upb-generated/src/proto/grpc/lb/v1/load_balancer.upb.c
src/core/ext/filters/client_channel/resolver/fake/fake_resolver.cc
src/core/ext/filters/client_channel/lb_policy/xds/cds.cc
Expand Down Expand Up @@ -2959,6 +2960,7 @@ add_library(grpc_unsecure
src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper_posix.cc
src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper_windows.cc
src/core/ext/filters/client_channel/resolver/dns/dns_resolver_selection.cc
src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc
src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc
src/core/ext/filters/client_channel/resolver/sockaddr/sockaddr_resolver.cc
src/core/ext/filters/client_channel/resolver/fake/fake_resolver.cc
Expand Down
2 changes: 2 additions & 0 deletions Makefile
View file
Open in desktop
Expand Up @@ -3927,6 +3927,7 @@ LIBGRPC_SRC = \
src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc \
src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_client_stats.cc \
src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.cc \
src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc \
src/core/ext/upb-generated/src/proto/grpc/lb/v1/load_balancer.upb.c \
src/core/ext/filters/client_channel/resolver/fake/fake_resolver.cc \
src/core/ext/filters/client_channel/lb_policy/xds/cds.cc \
Expand Down Expand Up @@ -5363,6 +5364,7 @@ LIBGRPC_UNSECURE_SRC = \
src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper_posix.cc \
src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper_windows.cc \
src/core/ext/filters/client_channel/resolver/dns/dns_resolver_selection.cc \
src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc \
src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc \
src/core/ext/filters/client_channel/resolver/sockaddr/sockaddr_resolver.cc \
src/core/ext/filters/client_channel/resolver/fake/fake_resolver.cc \
Expand Down
11 changes: 11 additions & 0 deletions build.yaml
View file
Open in desktop
Expand Up @@ -1053,6 +1053,14 @@ filegroups:
plugin: grpc_deadline_filter
uses:
- grpc_base
- name: grpc_grpclb_balancer_addresses
headers:
- src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h
src:
- src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc
uses:
- grpc_base
- grpc_client_channel
- name: grpc_health_upb
headers:
- src/core/ext/upb-generated/src/proto/grpc/health/v1/health.upb.h
Expand Down Expand Up @@ -1108,6 +1116,7 @@ filegroups:
uses:
- grpc_base
- grpc_client_channel
- grpc_grpclb_balancer_addresses
- grpc_lb_upb
- grpc_resolver_fake
- name: grpc_lb_policy_grpclb_secure
Expand All @@ -1129,6 +1138,7 @@ filegroups:
uses:
- grpc_base
- grpc_client_channel
- grpc_grpclb_balancer_addresses
- grpc_lb_upb
- grpc_resolver_fake
- grpc_secure
Expand Down Expand Up @@ -1219,6 +1229,7 @@ filegroups:
- grpc_base
- grpc_client_channel
- grpc_resolver_dns_selection
- grpc_grpclb_balancer_addresses
- name: grpc_resolver_dns_native
src:
- src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc
Expand Down
1 change: 1 addition & 0 deletions config.m4
View file
Open in desktop
Expand Up @@ -53,6 +53,7 @@ if test "$PHP_GRPC" != "no"; then
src/core/ext/filters/client_channel/lb_policy.cc \
src/core/ext/filters/client_channel/lb_policy/grpclb/client_load_reporting_filter.cc \
src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc \
src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc \
src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc \
src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_client_stats.cc \
src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.cc \
Expand Down
1 change: 1 addition & 0 deletions config.w32
View file
Open in desktop
Expand Up @@ -22,6 +22,7 @@ if (PHP_GRPC != "no") {
"src\\core\\ext\\filters\\client_channel\\lb_policy.cc " +
"src\\core\\ext\\filters\\client_channel\\lb_policy\\grpclb\\client_load_reporting_filter.cc " +
"src\\core\\ext\\filters\\client_channel\\lb_policy\\grpclb\\grpclb.cc " +
"src\\core\\ext\\filters\\client_channel\\lb_policy\\grpclb\\grpclb_balancer_addresses.cc " +
"src\\core\\ext\\filters\\client_channel\\lb_policy\\grpclb\\grpclb_channel_secure.cc " +
"src\\core\\ext\\filters\\client_channel\\lb_policy\\grpclb\\grpclb_client_stats.cc " +
"src\\core\\ext\\filters\\client_channel\\lb_policy\\grpclb\\load_balancer_api.cc " +
Expand Down
2 changes: 2 additions & 0 deletions gRPC-C++.podspec
View file
Open in desktop
Expand Up @@ -232,6 +232,7 @@ Pod::Spec.new do |s|
'src/core/ext/filters/client_channel/lb_policy.h',
'src/core/ext/filters/client_channel/lb_policy/grpclb/client_load_reporting_filter.h',
'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.h',
'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h',
'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel.h',
'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_client_stats.h',
'src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.h',
Expand Down Expand Up @@ -653,6 +654,7 @@ Pod::Spec.new do |s|
'src/core/ext/filters/client_channel/lb_policy.h',
'src/core/ext/filters/client_channel/lb_policy/grpclb/client_load_reporting_filter.h',
'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.h',
'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h',
'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel.h',
'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_client_stats.h',
'src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.h',
Expand Down
3 changes: 3 additions & 0 deletions gRPC-Core.podspec
View file
Open in desktop
Expand Up @@ -205,6 +205,8 @@ Pod::Spec.new do |s|
'src/core/ext/filters/client_channel/lb_policy/grpclb/client_load_reporting_filter.h',
'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc',
'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.h',
'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc',
'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h',
'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel.h',
'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc',
'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_client_stats.cc',
Expand Down Expand Up @@ -969,6 +971,7 @@ Pod::Spec.new do |s|
'src/core/ext/filters/client_channel/lb_policy.h',
'src/core/ext/filters/client_channel/lb_policy/grpclb/client_load_reporting_filter.h',
'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.h',
'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h',
'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel.h',
'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_client_stats.h',
'src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.h',
Expand Down
2 changes: 2 additions & 0 deletions grpc.gemspec
View file
Open in desktop
Expand Up @@ -128,6 +128,8 @@ Gem::Specification.new do |s|
s.files += %w( src/core/ext/filters/client_channel/lb_policy/grpclb/client_load_reporting_filter.h )
s.files += %w( src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc )
s.files += %w( src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.h )
s.files += %w( src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc )
s.files += %w( src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h )
s.files += %w( src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel.h )
s.files += %w( src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc )
s.files += %w( src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_client_stats.cc )
Expand Down
2 changes: 2 additions & 0 deletions grpc.gyp
View file
Open in desktop
Expand Up @@ -552,6 +552,7 @@
'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc',
'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_client_stats.cc',
'src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.cc',
'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc',
'src/core/ext/upb-generated/src/proto/grpc/lb/v1/load_balancer.upb.c',
'src/core/ext/filters/client_channel/resolver/fake/fake_resolver.cc',
'src/core/ext/filters/client_channel/lb_policy/xds/cds.cc',
Expand Down Expand Up @@ -1396,6 +1397,7 @@
'src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper_posix.cc',
'src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper_windows.cc',
'src/core/ext/filters/client_channel/resolver/dns/dns_resolver_selection.cc',
'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc',
'src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc',
'src/core/ext/filters/client_channel/resolver/sockaddr/sockaddr_resolver.cc',
'src/core/ext/filters/client_channel/resolver/fake/fake_resolver.cc',
Expand Down
2 changes: 2 additions & 0 deletions package.xml
View file
Open in desktop
Expand Up @@ -111,6 +111,8 @@
<file baseinstalldir="/" name="src/core/ext/filters/client_channel/lb_policy/grpclb/client_load_reporting_filter.h" role="src" />
<file baseinstalldir="/" name="src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc" role="src" />
<file baseinstalldir="/" name="src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.h" role="src" />
<file baseinstalldir="/" name="src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc" role="src" />
<file baseinstalldir="/" name="src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h" role="src" />
<file baseinstalldir="/" name="src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel.h" role="src" />
<file baseinstalldir="/" name="src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc" role="src" />
<file baseinstalldir="/" name="src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_client_stats.cc" role="src" />
Expand Down
20 changes: 0 additions & 20 deletions src/core/ext/filters/client_channel/client_channel.cc
View file
Open in desktop
Expand Up @@ -1638,26 +1638,6 @@ void ChannelData::ProcessLbPolicy(
grpc_channel_args_find(resolver_result.args, GRPC_ARG_LB_POLICY_NAME);
local_policy_name = grpc_channel_arg_get_string(channel_arg);
}
// Special case: If at least one balancer address is present, we use
// the grpclb policy, regardless of what the resolver has returned.
bool found_balancer_address = false;
for (size_t i = 0; i < resolver_result.addresses.size(); ++i) {
const ServerAddress& address = resolver_result.addresses[i];
if (address.IsBalancer()) {
found_balancer_address = true;
break;
}
}
if (found_balancer_address) {
if (local_policy_name != nullptr &&
strcmp(local_policy_name, "grpclb") != 0) {
gpr_log(GPR_INFO,
"resolver requested LB policy %s but provided at least one "
"balancer address -- forcing use of grpclb LB policy",
local_policy_name);
}
local_policy_name = "grpclb";
}
// Use pick_first if nothing was specified and we didn't select grpclb
// above.
lb_policy_name->reset(gpr_strdup(
Expand Down
45 changes: 15 additions & 30 deletions src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc
View file
Open in desktop
Expand Up @@ -73,6 +73,7 @@
#include "src/core/ext/filters/client_channel/client_channel.h"
#include "src/core/ext/filters/client_channel/lb_policy/grpclb/client_load_reporting_filter.h"
#include "src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.h"
#include "src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h"
#include "src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel.h"
#include "src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_client_stats.h"
#include "src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.h"
Expand Down Expand Up @@ -1267,25 +1268,11 @@ void GrpcLb::BalancerCallState::OnBalancerStatusReceivedLocked(
// helper code for creating balancer channel
//

ServerAddressList ExtractBalancerAddresses(const ServerAddressList& addresses) {
ServerAddressList balancer_addresses;
for (size_t i = 0; i < addresses.size(); ++i) {
if (addresses[i].IsBalancer()) {
// Strip out the is_balancer channel arg, since we don't want to
// recursively use the grpclb policy in the channel used to talk to
// the balancers. Note that we do NOT strip out the balancer_name
// channel arg, since we need that to set the authority correctly
// to talk to the balancers.
static const char* args_to_remove[] = {
GRPC_ARG_ADDRESS_IS_BALANCER,
};
balancer_addresses.emplace_back(
addresses[i].address(),
grpc_channel_args_copy_and_remove(addresses[i].args(), args_to_remove,
GPR_ARRAY_SIZE(args_to_remove)));
}
}
return balancer_addresses;
ServerAddressList ExtractBalancerAddresses(const grpc_channel_args& args) {
const ServerAddressList* addresses =
FindGrpclbBalancerAddressesInChannelArgs(args);
if (addresses != nullptr) return *addresses;
return ServerAddressList();
}

/* Returns the channel args for the LB channel, used to create a bidirectional
Expand Down Expand Up @@ -1491,27 +1478,25 @@ void GrpcLb::UpdateLocked(UpdateArgs args) {
// helpers for UpdateLocked()
//

// Returns the backend addresses extracted from the given addresses.
ServerAddressList ExtractBackendAddresses(const ServerAddressList& addresses) {
ServerAddressList AddNullLbTokenToAddresses(
const ServerAddressList& addresses) {
static const char* lb_token = "";
grpc_arg arg = grpc_channel_arg_pointer_create(
const_cast<char*>(GRPC_ARG_GRPCLB_ADDRESS_LB_TOKEN),
const_cast<char*>(lb_token), &lb_token_arg_vtable);
ServerAddressList backend_addresses;
ServerAddressList addresses_out;
for (size_t i = 0; i < addresses.size(); ++i) {
if (!addresses[i].IsBalancer()) {
backend_addresses.emplace_back(
addresses[i].address(),
grpc_channel_args_copy_and_add(addresses[i].args(), &arg, 1));
}
addresses_out.emplace_back(
addresses[i].address(),
grpc_channel_args_copy_and_add(addresses[i].args(), &arg, 1));
}
return backend_addresses;
return addresses_out;
}

void GrpcLb::ProcessAddressesAndChannelArgsLocked(
const ServerAddressList& addresses, const grpc_channel_args& args) {
// Update fallback address list.
fallback_backend_addresses_ = ExtractBackendAddresses(addresses);
fallback_backend_addresses_ = AddNullLbTokenToAddresses(addresses);
// Make sure that GRPC_ARG_LB_POLICY_NAME is set in channel args,
// since we use this to trigger the client_load_reporting filter.
static const char* args_to_remove[] = {GRPC_ARG_LB_POLICY_NAME};
Expand All @@ -1521,7 +1506,7 @@ void GrpcLb::ProcessAddressesAndChannelArgsLocked(
args_ = grpc_channel_args_copy_and_add_and_remove(
&args, args_to_remove, GPR_ARRAY_SIZE(args_to_remove), &new_arg, 1);
// Construct args for balancer channel.
ServerAddressList balancer_addresses = ExtractBalancerAddresses(addresses);
ServerAddressList balancer_addresses = ExtractBalancerAddresses(args);
grpc_channel_args* lb_channel_args = BuildBalancerChannelArgs(
balancer_addresses, response_generator_.get(), &args);
// Create balancer channel if needed.
Expand Down