diff --git a/src/core/ext/filters/client_channel/lb_policy/weighted_target/weighted_target.cc b/src/core/ext/filters/client_channel/lb_policy/weighted_target/weighted_target.cc index a0b0fef04a3..acbbcf0f957 100644 --- a/src/core/ext/filters/client_channel/lb_policy/weighted_target/weighted_target.cc +++ b/src/core/ext/filters/client_channel/lb_policy/weighted_target/weighted_target.cc @@ -278,19 +278,29 @@ void WeightedTargetLb::UpdateLocked(UpdateArgs args) { child->DeactivateLocked(); } } - // Add or update the targets in the new config. - HierarchicalAddressMap address_map = - MakeHierarchicalAddressMap(args.addresses); + // Create any children that don't already exist. + // Note that we add all children before updating any of them, because + // an update may trigger a child to immediately update its + // connectivity state (e.g., reporting TRANSIENT_FAILURE immediately when + // receiving an empty address list), and we don't want to return an + // overall state with incomplete data. for (const auto& p : config_->target_map()) { const std::string& name = p.first; - const WeightedTargetLbConfig::ChildConfig& config = p.second; auto it = targets_.find(name); if (it == targets_.end()) { it = targets_.emplace(std::make_pair(name, nullptr)).first; it->second = MakeOrphanable( Ref(DEBUG_LOCATION, "WeightedChild"), it->first); } - it->second->UpdateLocked(config, std::move(address_map[name]), args.args); + } + // Update all children. + HierarchicalAddressMap address_map = + MakeHierarchicalAddressMap(args.addresses); + for (const auto& p : config_->target_map()) { + const std::string& name = p.first; + const WeightedTargetLbConfig::ChildConfig& config = p.second; + targets_[name]->UpdateLocked(config, std::move(address_map[name]), + args.args); } } diff --git a/test/cpp/end2end/xds_end2end_test.cc b/test/cpp/end2end/xds_end2end_test.cc index f993bb88e78..0b4d671fa84 100644 --- a/test/cpp/end2end/xds_end2end_test.cc +++ b/test/cpp/end2end/xds_end2end_test.cc @@ -3196,6 +3196,22 @@ TEST_P(FailoverTest, DoesNotUsePriorityWithNoEndpoints) { } } +// Does not choose locality with no endpoints. +TEST_P(FailoverTest, DoesNotUseLocalityWithNoEndpoints) { + SetNextResolution({}); + SetNextResolutionForLbChannelAllBalancers(); + AdsServiceImpl::EdsResourceArgs args({ + {"locality0", {}, kDefaultLocalityWeight, 0}, + {"locality1", GetBackendPorts(), kDefaultLocalityWeight, 0}, + }); + balancers_[0]->ads_service()->SetEdsResource( + AdsServiceImpl::BuildEdsResource(args), kDefaultResourceName); + // Wait for all backends to be used. + std::tuple counts = WaitForAllBackends(); + // Make sure no RPCs failed in the transition. + EXPECT_EQ(0, std::get<1>(counts)); +} + // If the higher priority localities are not reachable, failover to the highest // priority among the rest. TEST_P(FailoverTest, Failover) {