From 3c2354f62a1f1f0163a415a40d1fbbdab1940979 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Fri, 13 Mar 2020 14:40:32 -0700 Subject: [PATCH 1/5] Update grpclb with optional field "name" --- .../client_channel/lb_policy/grpclb/grpclb.cc | 42 +++++++++++++++---- 1 file changed, 35 insertions(+), 7 deletions(-) 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 c39bf8e28be..9ee850dcea2 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 @@ -130,16 +130,20 @@ constexpr char kGrpclb[] = "grpclb"; class GrpcLbConfig : public LoadBalancingPolicy::Config { public: - explicit GrpcLbConfig(RefCountedPtr child_policy) - : child_policy_(std::move(child_policy)) {} + GrpcLbConfig(RefCountedPtr child_policy, + const std::string& target_name) + : child_policy_(std::move(child_policy)), target_name_(target_name) {} const char* name() const override { return kGrpclb; } RefCountedPtr child_policy() const { return child_policy_; } + const std::string& target_name() const { return target_name_; } + private: RefCountedPtr child_policy_; + std::string target_name_; }; class GrpcLb : public LoadBalancingPolicy { @@ -369,6 +373,9 @@ class GrpcLb : public LoadBalancingPolicy { // Who the client is trying to communicate with. const char* server_name_ = nullptr; + // The target name from configuration; if set, it overrides server_name_ in + // the balancer requests. + const char* target_name_ = nullptr; // Current channel args from the resolver. grpc_channel_args* args_ = nullptr; @@ -761,8 +768,9 @@ GrpcLb::BalancerCallState::BalancerCallState( nullptr, deadline, nullptr); // Init the LB call request payload. upb::Arena arena; - grpc_slice request_payload_slice = - GrpcLbRequestCreate(grpclb_policy()->server_name_, arena.ptr()); + grpc_slice request_payload_slice = GrpcLbRequestCreate( + grpclb_policy()->target_name_ ?: grpclb_policy()->server_name_, + arena.ptr()); send_message_payload_ = grpc_raw_byte_buffer_create(&request_payload_slice, 1); grpc_slice_unref_internal(request_payload_slice); @@ -1344,6 +1352,7 @@ GrpcLb::GrpcLb(Args args) GrpcLb::~GrpcLb() { gpr_free((void*)server_name_); + if (target_name_ != nullptr) gpr_free((void*)target_name_); grpc_channel_args_destroy(args_); } @@ -1391,8 +1400,14 @@ void GrpcLb::UpdateLocked(UpdateArgs args) { auto* grpclb_config = static_cast(args.config.get()); if (grpclb_config != nullptr) { child_policy_config_ = grpclb_config->child_policy(); + if (grpclb_config->target_name().length() > 0) { + target_name_ = gpr_strdup(grpclb_config->target_name().c_str()); + } else { + target_name_ = nullptr; + } } else { child_policy_config_ = nullptr; + target_name_ = nullptr; } ProcessAddressesAndChannelArgsLocked(args.addresses, *args.args); // Update the existing child policy. @@ -1678,12 +1693,23 @@ class GrpcLbFactory : public LoadBalancingPolicyFactory { const Json& json, grpc_error** error) const override { GPR_DEBUG_ASSERT(error != nullptr && *error == GRPC_ERROR_NONE); if (json.type() == Json::Type::JSON_NULL) { - return MakeRefCounted(nullptr); + return MakeRefCounted(nullptr, nullptr); } std::vector error_list; Json child_policy_config_json_tmp; const Json* child_policy_config_json; - auto it = json.object_value().find("childPolicy"); + const std::string* target_name_ptr = nullptr; + auto it = json.object_value().find("targetName"); + if (it != json.object_value().end()) { + const Json& target_name_json = it->second; + if (target_name_json.type() != Json::Type::STRING) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "targetname filed is not string")); + } else { + target_name_ptr = &target_name_json.string_value(); + } + } + it = json.object_value().find("childPolicy"); if (it == json.object_value().end()) { child_policy_config_json_tmp = Json::Array{Json::Object{ {"round_robin", Json::Object()}, @@ -1703,7 +1729,9 @@ class GrpcLbFactory : public LoadBalancingPolicyFactory { GRPC_ERROR_CREATE_FROM_VECTOR("field:childPolicy", &child_errors)); } if (error_list.empty()) { - return MakeRefCounted(std::move(child_policy_config)); + return MakeRefCounted( + std::move(child_policy_config), + target_name_ptr == nullptr ? std::string() : *target_name_ptr); } else { *error = GRPC_ERROR_CREATE_FROM_VECTOR("GrpcLb Parser", &error_list); return nullptr; From 3f684093d75fc620ea0008d5adeccf91b65269cb Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Wed, 6 May 2020 17:33:27 -0700 Subject: [PATCH 2/5] Add test for the new config field --- test/cpp/end2end/grpclb_end2end_test.cc | 61 +++++++++++++++++++------ 1 file changed, 48 insertions(+), 13 deletions(-) diff --git a/test/cpp/end2end/grpclb_end2end_test.cc b/test/cpp/end2end/grpclb_end2end_test.cc index a6a7d3697b6..0ed90e85673 100644 --- a/test/cpp/end2end/grpclb_end2end_test.cc +++ b/test/cpp/end2end/grpclb_end2end_test.cc @@ -212,7 +212,11 @@ struct ClientStats { class BalancerServiceImpl : public BalancerService { public: using Stream = ServerReaderWriter; - using ResponseDelayPair = std::pair; + struct ResponseConfig { + LoadBalanceResponse response; + int delay_ms; + std::string for_target; + }; explicit BalancerServiceImpl(int client_load_reporting_interval_seconds) : client_load_reporting_interval_seconds_( @@ -229,10 +233,15 @@ class BalancerServiceImpl : public BalancerService { EXPECT_EQ(context->client_metadata().find(g_kCallCredsMdKey), context->client_metadata().end()); LoadBalanceRequest request; - std::vector responses_and_delays; + std::string target; + std::vector response_configs; if (!stream->Read(&request)) { goto done; + } else { + if (request.has_initial_request()) { + target = request.initial_request().name(); + } } IncreaseRequestCount(); gpr_log(GPR_INFO, "LB[%p]: received initial message '%s'", this, @@ -249,11 +258,14 @@ class BalancerServiceImpl : public BalancerService { { grpc::internal::MutexLock lock(&mu_); - responses_and_delays = responses_and_delays_; + response_configs = response_configs_; } - for (const auto& response_and_delay : responses_and_delays) { - SendResponse(stream, response_and_delay.first, - response_and_delay.second); + for (const auto& response_config : response_configs) { + if (response_config.for_target.empty() || + response_config.for_target == target) { + SendResponse(stream, response_config.response, + response_config.delay_ms); + } } { grpc::internal::MutexLock lock(&mu_); @@ -295,16 +307,16 @@ class BalancerServiceImpl : public BalancerService { return Status::OK; } - void add_response(const LoadBalanceResponse& response, int send_after_ms) { + void add_response(const LoadBalanceResponse& response, int send_after_ms, + std::string for_target = "") { grpc::internal::MutexLock lock(&mu_); - responses_and_delays_.push_back(std::make_pair(response, send_after_ms)); + response_configs_.push_back({response, send_after_ms, for_target}); } void Start() { grpc::internal::MutexLock lock(&mu_); serverlist_done_ = false; - responses_and_delays_.clear(); - load_report_queue_.clear(); + response_configs_.clear(); } void Shutdown() { @@ -372,7 +384,7 @@ class BalancerServiceImpl : public BalancerService { } const int client_load_reporting_interval_seconds_; - std::vector responses_and_delays_; + std::vector response_configs_; grpc::internal::Mutex mu_; grpc::internal::CondVar serverlist_cond_; @@ -613,8 +625,8 @@ class GrpclbEnd2endTest : public ::testing::Test { void ScheduleResponseForBalancer(size_t i, const LoadBalanceResponse& response, - int delay_ms) { - balancers_[i]->service_.add_response(response, delay_ms); + int delay_ms, std::string target = "") { + balancers_[i]->service_.add_response(response, delay_ms, target); } Status SendRpc(EchoResponse* response = nullptr, int timeout_ms = 1000, @@ -1383,6 +1395,29 @@ TEST_F(SingleBalancerTest, BackendsRestart) { EXPECT_EQ(1U, balancers_[0]->service_.response_count()); } +TEST_F(SingleBalancerTest, TargetFromLbPolicyConfig) { + constexpr char kServiceConfigWithTarget[] = + "{\n" + " \"loadBalancingConfig\":[\n" + " { \"grpclb\":{\n" + " \"targetName\":\"test_target\"\n" + " }}\n" + " ]\n" + "}"; + + SetNextResolutionAllBalancers(kServiceConfigWithTarget); + const size_t kNumRpcsPerAddress = 1; + ScheduleResponseForBalancer( + 0, BalancerServiceImpl::BuildResponseForBackends(GetBackendPorts(), {}), + 0, "test_target"); + // Make sure that trying to connect works without a call. + channel_->GetState(true /* try_to_connect */); + // We need to wait for all backends to come online. + WaitForAllBackends(); + // Send kNumRpcsPerAddress RPCs per server. + CheckRpcSendOk(kNumRpcsPerAddress * num_backends_); +} + class UpdatesTest : public GrpclbEnd2endTest { public: UpdatesTest() : GrpclbEnd2endTest(4, 3, 0) {} From ec2cd96426725c8682a08485435ac31bbfa61db9 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Thu, 7 May 2020 10:22:49 -0700 Subject: [PATCH 3/5] Address comments --- .../client_channel/lb_policy/grpclb/grpclb.cc | 43 ++++++++----------- test/cpp/end2end/grpclb_end2end_test.cc | 2 +- 2 files changed, 19 insertions(+), 26 deletions(-) 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 9ee850dcea2..66ad37db134 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 @@ -131,8 +131,9 @@ constexpr char kGrpclb[] = "grpclb"; class GrpcLbConfig : public LoadBalancingPolicy::Config { public: GrpcLbConfig(RefCountedPtr child_policy, - const std::string& target_name) - : child_policy_(std::move(child_policy)), target_name_(target_name) {} + std::string target_name) + : child_policy_(std::move(child_policy)), + target_name_(std::move(target_name)) {} const char* name() const override { return kGrpclb; } RefCountedPtr child_policy() const { @@ -375,7 +376,7 @@ class GrpcLb : public LoadBalancingPolicy { const char* server_name_ = nullptr; // The target name from configuration; if set, it overrides server_name_ in // the balancer requests. - const char* target_name_ = nullptr; + std::string target_name_; // Current channel args from the resolver. grpc_channel_args* args_ = nullptr; @@ -768,9 +769,11 @@ GrpcLb::BalancerCallState::BalancerCallState( nullptr, deadline, nullptr); // Init the LB call request payload. upb::Arena arena; - grpc_slice request_payload_slice = GrpcLbRequestCreate( - grpclb_policy()->target_name_ ?: grpclb_policy()->server_name_, - arena.ptr()); + grpc_slice request_payload_slice = + GrpcLbRequestCreate(grpclb_policy()->target_name_.empty() + ? grpclb_policy()->server_name_ + : grpclb_policy()->target_name_.c_str(), + arena.ptr()); send_message_payload_ = grpc_raw_byte_buffer_create(&request_payload_slice, 1); grpc_slice_unref_internal(request_payload_slice); @@ -1352,7 +1355,6 @@ GrpcLb::GrpcLb(Args args) GrpcLb::~GrpcLb() { gpr_free((void*)server_name_); - if (target_name_ != nullptr) gpr_free((void*)target_name_); grpc_channel_args_destroy(args_); } @@ -1398,17 +1400,9 @@ void GrpcLb::ResetBackoffLocked() { void GrpcLb::UpdateLocked(UpdateArgs args) { const bool is_initial_update = lb_channel_ == nullptr; auto* grpclb_config = static_cast(args.config.get()); - if (grpclb_config != nullptr) { - child_policy_config_ = grpclb_config->child_policy(); - if (grpclb_config->target_name().length() > 0) { - target_name_ = gpr_strdup(grpclb_config->target_name().c_str()); - } else { - target_name_ = nullptr; - } - } else { - child_policy_config_ = nullptr; - target_name_ = nullptr; - } + GPR_ASSERT(grpclb_config != nullptr); + child_policy_config_ = grpclb_config->child_policy(); + target_name_ = grpclb_config->target_name(); ProcessAddressesAndChannelArgsLocked(args.addresses, *args.args); // Update the existing child policy. if (child_policy_ != nullptr) CreateOrUpdateChildPolicyLocked(); @@ -1698,15 +1692,15 @@ class GrpcLbFactory : public LoadBalancingPolicyFactory { std::vector error_list; Json child_policy_config_json_tmp; const Json* child_policy_config_json; - const std::string* target_name_ptr = nullptr; - auto it = json.object_value().find("targetName"); + std::string target_name; + auto it = json.object_value().find("serviceName"); if (it != json.object_value().end()) { const Json& target_name_json = it->second; if (target_name_json.type() != Json::Type::STRING) { error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "targetname filed is not string")); + "field:serviceName error:type should be string")); } else { - target_name_ptr = &target_name_json.string_value(); + target_name = target_name_json.string_value(); } } it = json.object_value().find("childPolicy"); @@ -1729,9 +1723,8 @@ class GrpcLbFactory : public LoadBalancingPolicyFactory { GRPC_ERROR_CREATE_FROM_VECTOR("field:childPolicy", &child_errors)); } if (error_list.empty()) { - return MakeRefCounted( - std::move(child_policy_config), - target_name_ptr == nullptr ? std::string() : *target_name_ptr); + return MakeRefCounted(std::move(child_policy_config), + target_name); } else { *error = GRPC_ERROR_CREATE_FROM_VECTOR("GrpcLb Parser", &error_list); return nullptr; diff --git a/test/cpp/end2end/grpclb_end2end_test.cc b/test/cpp/end2end/grpclb_end2end_test.cc index 0ed90e85673..0572edf1f4c 100644 --- a/test/cpp/end2end/grpclb_end2end_test.cc +++ b/test/cpp/end2end/grpclb_end2end_test.cc @@ -1400,7 +1400,7 @@ TEST_F(SingleBalancerTest, TargetFromLbPolicyConfig) { "{\n" " \"loadBalancingConfig\":[\n" " { \"grpclb\":{\n" - " \"targetName\":\"test_target\"\n" + " \"serviceName\":\"test_target\"\n" " }}\n" " ]\n" "}"; From fa28bab4562f2dafaf0e92ebc87580ee2dce2f23 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Wed, 8 Jul 2020 16:56:21 -0700 Subject: [PATCH 4/5] Address comments --- .../client_channel/lb_policy/grpclb/grpclb.cc | 45 +++++++--------- test/cpp/end2end/grpclb_end2end_test.cc | 54 +++++++++---------- 2 files changed, 47 insertions(+), 52 deletions(-) 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 66ad37db134..da3b29e5826 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 @@ -131,20 +131,20 @@ constexpr char kGrpclb[] = "grpclb"; class GrpcLbConfig : public LoadBalancingPolicy::Config { public: GrpcLbConfig(RefCountedPtr child_policy, - std::string target_name) + std::string service_name) : child_policy_(std::move(child_policy)), - target_name_(std::move(target_name)) {} + service_name_(std::move(service_name)) {} const char* name() const override { return kGrpclb; } RefCountedPtr child_policy() const { return child_policy_; } - const std::string& target_name() const { return target_name_; } + const std::string& service_name() const { return service_name_; } private: RefCountedPtr child_policy_; - std::string target_name_; + std::string service_name_; }; class GrpcLb : public LoadBalancingPolicy { @@ -374,9 +374,8 @@ class GrpcLb : public LoadBalancingPolicy { // Who the client is trying to communicate with. const char* server_name_ = nullptr; - // The target name from configuration; if set, it overrides server_name_ in - // the balancer requests. - std::string target_name_; + // Configurations for the policy. + RefCountedPtr config_; // Current channel args from the resolver. grpc_channel_args* args_ = nullptr; @@ -422,8 +421,6 @@ class GrpcLb : public LoadBalancingPolicy { // The child policy to use for the backends. OrphanablePtr child_policy_; - // The child policy config. - RefCountedPtr child_policy_config_; // Child policy in state READY. bool child_policy_ready_ = false; }; @@ -769,11 +766,11 @@ GrpcLb::BalancerCallState::BalancerCallState( nullptr, deadline, nullptr); // Init the LB call request payload. upb::Arena arena; - grpc_slice request_payload_slice = - GrpcLbRequestCreate(grpclb_policy()->target_name_.empty() - ? grpclb_policy()->server_name_ - : grpclb_policy()->target_name_.c_str(), - arena.ptr()); + grpc_slice request_payload_slice = GrpcLbRequestCreate( + grpclb_policy()->config_->service_name().empty() + ? grpclb_policy()->server_name_ + : grpclb_policy()->config_->service_name().c_str(), + arena.ptr()); send_message_payload_ = grpc_raw_byte_buffer_create(&request_payload_slice, 1); grpc_slice_unref_internal(request_payload_slice); @@ -1399,10 +1396,8 @@ void GrpcLb::ResetBackoffLocked() { void GrpcLb::UpdateLocked(UpdateArgs args) { const bool is_initial_update = lb_channel_ == nullptr; - auto* grpclb_config = static_cast(args.config.get()); - GPR_ASSERT(grpclb_config != nullptr); - child_policy_config_ = grpclb_config->child_policy(); - target_name_ = grpclb_config->target_name(); + config_ = args.config; + GPR_ASSERT(config_ != nullptr); ProcessAddressesAndChannelArgsLocked(args.addresses, *args.args); // Update the existing child policy. if (child_policy_ != nullptr) CreateOrUpdateChildPolicyLocked(); @@ -1657,7 +1652,7 @@ void GrpcLb::CreateOrUpdateChildPolicyLocked() { update_args.args = CreateChildPolicyArgsLocked(is_backend_from_grpclb_load_balancer); GPR_ASSERT(update_args.args != nullptr); - update_args.config = child_policy_config_; + update_args.config = config_->child_policy(); // Create child policy if needed. if (child_policy_ == nullptr) { child_policy_ = CreateChildPolicyLocked(update_args.args); @@ -1687,20 +1682,20 @@ class GrpcLbFactory : public LoadBalancingPolicyFactory { const Json& json, grpc_error** error) const override { GPR_DEBUG_ASSERT(error != nullptr && *error == GRPC_ERROR_NONE); if (json.type() == Json::Type::JSON_NULL) { - return MakeRefCounted(nullptr, nullptr); + return MakeRefCounted(nullptr, ""); } std::vector error_list; Json child_policy_config_json_tmp; const Json* child_policy_config_json; - std::string target_name; + std::string service_name; auto it = json.object_value().find("serviceName"); if (it != json.object_value().end()) { - const Json& target_name_json = it->second; - if (target_name_json.type() != Json::Type::STRING) { + const Json& service_name_json = it->second; + if (service_name_json.type() != Json::Type::STRING) { error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( "field:serviceName error:type should be string")); } else { - target_name = target_name_json.string_value(); + service_name = service_name_json.string_value(); } } it = json.object_value().find("childPolicy"); @@ -1724,7 +1719,7 @@ class GrpcLbFactory : public LoadBalancingPolicyFactory { } if (error_list.empty()) { return MakeRefCounted(std::move(child_policy_config), - target_name); + std::move(service_name)); } else { *error = GRPC_ERROR_CREATE_FROM_VECTOR("GrpcLb Parser", &error_list); return nullptr; diff --git a/test/cpp/end2end/grpclb_end2end_test.cc b/test/cpp/end2end/grpclb_end2end_test.cc index 0572edf1f4c..9b109c89dda 100644 --- a/test/cpp/end2end/grpclb_end2end_test.cc +++ b/test/cpp/end2end/grpclb_end2end_test.cc @@ -212,11 +212,7 @@ struct ClientStats { class BalancerServiceImpl : public BalancerService { public: using Stream = ServerReaderWriter; - struct ResponseConfig { - LoadBalanceResponse response; - int delay_ms; - std::string for_target; - }; + using ResponseDelayPair = std::pair; explicit BalancerServiceImpl(int client_load_reporting_interval_seconds) : client_load_reporting_interval_seconds_( @@ -233,14 +229,14 @@ class BalancerServiceImpl : public BalancerService { EXPECT_EQ(context->client_metadata().find(g_kCallCredsMdKey), context->client_metadata().end()); LoadBalanceRequest request; - std::string target; - std::vector response_configs; + std::vector responses_and_delays; if (!stream->Read(&request)) { goto done; } else { if (request.has_initial_request()) { - target = request.initial_request().name(); + grpc::internal::MutexLock lock(&mu_); + service_names_.push_back(request.initial_request().name()); } } IncreaseRequestCount(); @@ -258,14 +254,11 @@ class BalancerServiceImpl : public BalancerService { { grpc::internal::MutexLock lock(&mu_); - response_configs = response_configs_; + responses_and_delays = responses_and_delays_; } - for (const auto& response_config : response_configs) { - if (response_config.for_target.empty() || - response_config.for_target == target) { - SendResponse(stream, response_config.response, - response_config.delay_ms); - } + for (const auto& response_and_delay : responses_and_delays) { + SendResponse(stream, response_and_delay.first, + response_and_delay.second); } { grpc::internal::MutexLock lock(&mu_); @@ -307,16 +300,16 @@ class BalancerServiceImpl : public BalancerService { return Status::OK; } - void add_response(const LoadBalanceResponse& response, int send_after_ms, - std::string for_target = "") { + void add_response(const LoadBalanceResponse& response, int send_after_ms) { grpc::internal::MutexLock lock(&mu_); - response_configs_.push_back({response, send_after_ms, for_target}); + responses_and_delays_.push_back(std::make_pair(response, send_after_ms)); } void Start() { grpc::internal::MutexLock lock(&mu_); serverlist_done_ = false; - response_configs_.clear(); + responses_and_delays_.clear(); + load_report_queue_.clear(); } void Shutdown() { @@ -370,6 +363,11 @@ class BalancerServiceImpl : public BalancerService { } } + std::vector service_names() { + grpc::internal::MutexLock lock(&mu_); + return service_names_; + } + private: void SendResponse(Stream* stream, const LoadBalanceResponse& response, int delay_ms) { @@ -384,7 +382,8 @@ class BalancerServiceImpl : public BalancerService { } const int client_load_reporting_interval_seconds_; - std::vector response_configs_; + std::vector responses_and_delays_; + std::vector service_names_; grpc::internal::Mutex mu_; grpc::internal::CondVar serverlist_cond_; @@ -625,8 +624,8 @@ class GrpclbEnd2endTest : public ::testing::Test { void ScheduleResponseForBalancer(size_t i, const LoadBalanceResponse& response, - int delay_ms, std::string target = "") { - balancers_[i]->service_.add_response(response, delay_ms, target); + int delay_ms) { + balancers_[i]->service_.add_response(response, delay_ms); } Status SendRpc(EchoResponse* response = nullptr, int timeout_ms = 1000, @@ -1395,12 +1394,12 @@ TEST_F(SingleBalancerTest, BackendsRestart) { EXPECT_EQ(1U, balancers_[0]->service_.response_count()); } -TEST_F(SingleBalancerTest, TargetFromLbPolicyConfig) { +TEST_F(SingleBalancerTest, ServiceNameFromLbPolicyConfig) { constexpr char kServiceConfigWithTarget[] = "{\n" " \"loadBalancingConfig\":[\n" " { \"grpclb\":{\n" - " \"serviceName\":\"test_target\"\n" + " \"serviceName\":\"test_service\"\n" " }}\n" " ]\n" "}"; @@ -1409,13 +1408,14 @@ TEST_F(SingleBalancerTest, TargetFromLbPolicyConfig) { const size_t kNumRpcsPerAddress = 1; ScheduleResponseForBalancer( 0, BalancerServiceImpl::BuildResponseForBackends(GetBackendPorts(), {}), - 0, "test_target"); + 0); // Make sure that trying to connect works without a call. channel_->GetState(true /* try_to_connect */); // We need to wait for all backends to come online. WaitForAllBackends(); - // Send kNumRpcsPerAddress RPCs per server. - CheckRpcSendOk(kNumRpcsPerAddress * num_backends_); + // Send an RPC to trigger load balancing. + CheckRpcSendOk(); + EXPECT_EQ(balancers_[0]->service_.service_names().back(), "test_service"); } class UpdatesTest : public GrpclbEnd2endTest { From 232756e7bf112489c771253c4788b8ed4e660331 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Thu, 9 Jul 2020 09:37:05 -0700 Subject: [PATCH 5/5] Remove unnecessary RPC --- test/cpp/end2end/grpclb_end2end_test.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/cpp/end2end/grpclb_end2end_test.cc b/test/cpp/end2end/grpclb_end2end_test.cc index 9b109c89dda..49cbcf00d3e 100644 --- a/test/cpp/end2end/grpclb_end2end_test.cc +++ b/test/cpp/end2end/grpclb_end2end_test.cc @@ -1413,8 +1413,6 @@ TEST_F(SingleBalancerTest, ServiceNameFromLbPolicyConfig) { channel_->GetState(true /* try_to_connect */); // We need to wait for all backends to come online. WaitForAllBackends(); - // Send an RPC to trigger load balancing. - CheckRpcSendOk(); EXPECT_EQ(balancers_[0]->service_.service_names().back(), "test_service"); }