diff --git a/include/envoy/upstream/upstream.h b/include/envoy/upstream/upstream.h index 2334233cd9c9..33251a0f0c18 100644 --- a/include/envoy/upstream/upstream.h +++ b/include/envoy/upstream/upstream.h @@ -127,13 +127,20 @@ class HostSet { * set on the command line and to use a cluster type that supports population such as * the SDS cluster type. */ - virtual const std::vector& localZoneHosts() const PURE; /** - * @return all healthy hosts that are in the zone local to this node. See healthyHosts() and - * localZoneHosts() for more information. + * @return hosts per zone, index 0 is dedicated to local zone hosts. + * If there are no hosts in local zone for upstream cluster hostPerZone() will @return + * empty vector. + * + * Note, that we sort zones in alphabetical order starting from index 1. */ - virtual const std::vector& localZoneHealthyHosts() const PURE; + virtual const std::vector>& hostsPerZone() const PURE; + + /** + * @return same as hostsPerZone but only contains healthy hosts. + */ + virtual const std::vector>& healthyHostsPerZone() const PURE; }; /** @@ -188,8 +195,7 @@ class HostSet { COUNTER(zone_over_percentage) \ COUNTER(zone_routing_sampled) \ COUNTER(zone_routing_no_sampled) \ - GAUGE (max_host_weight) \ - GAUGE (upstream_zone_count) + GAUGE (max_host_weight) // clang-format on /** diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index 0c7ff418977b..36a63cb39b78 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -174,16 +174,17 @@ void ClusterManagerImpl::postThreadLocalClusterUpdate(const ClusterImplBase& pri const std::string& name = primary_cluster.name(); ConstHostVectorPtr hosts_copy = primary_cluster.rawHosts(); ConstHostVectorPtr healthy_hosts_copy = primary_cluster.rawHealthyHosts(); - ConstHostVectorPtr local_zone_hosts_copy = primary_cluster.rawLocalZoneHosts(); - ConstHostVectorPtr local_zone_healthy_hosts_copy = primary_cluster.rawLocalZoneHealthyHosts(); + ConstHostListsPtr hosts_per_zone_copy = primary_cluster.rawHostsPerZone(); + ConstHostListsPtr healthy_hosts_per_zone_copy = primary_cluster.rawHealthyHostsPerZone(); ThreadLocal::Instance& tls = tls_; uint32_t thead_local_slot = thread_local_slot_; + tls_.runOnAllThreads( - [name, hosts_copy, healthy_hosts_copy, local_zone_hosts_copy, local_zone_healthy_hosts_copy, + [name, hosts_copy, healthy_hosts_copy, hosts_per_zone_copy, healthy_hosts_per_zone_copy, hosts_added, hosts_removed, &tls, thead_local_slot]() mutable -> void { ThreadLocalClusterManagerImpl::updateClusterMembership( - name, hosts_copy, healthy_hosts_copy, local_zone_hosts_copy, - local_zone_healthy_hosts_copy, hosts_added, hosts_removed, tls, thead_local_slot); + name, hosts_copy, healthy_hosts_copy, hosts_per_zone_copy, healthy_hosts_per_zone_copy, + hosts_added, hosts_removed, tls, thead_local_slot); }); } @@ -289,7 +290,7 @@ void ClusterManagerImpl::ThreadLocalClusterManagerImpl::drainConnPools( void ClusterManagerImpl::ThreadLocalClusterManagerImpl::updateClusterMembership( const std::string& name, ConstHostVectorPtr hosts, ConstHostVectorPtr healthy_hosts, - ConstHostVectorPtr local_zone_hosts, ConstHostVectorPtr local_zone_healthy_hosts, + ConstHostListsPtr hosts_per_zone, ConstHostListsPtr healthy_hosts_per_zone, const std::vector& hosts_added, const std::vector& hosts_removed, ThreadLocal::Instance& tls, uint32_t thead_local_slot) { @@ -298,7 +299,7 @@ void ClusterManagerImpl::ThreadLocalClusterManagerImpl::updateClusterMembership( ASSERT(config.thread_local_clusters_.find(name) != config.thread_local_clusters_.end()); config.thread_local_clusters_[name]->host_set_.updateHosts( - hosts, healthy_hosts, local_zone_hosts, local_zone_healthy_hosts, hosts_added, hosts_removed); + hosts, healthy_hosts, hosts_per_zone, healthy_hosts_per_zone, hosts_added, hosts_removed); } void ClusterManagerImpl::ThreadLocalClusterManagerImpl::shutdown() { diff --git a/source/common/upstream/cluster_manager_impl.h b/source/common/upstream/cluster_manager_impl.h index 9a472c38222f..b4f9869879c1 100644 --- a/source/common/upstream/cluster_manager_impl.h +++ b/source/common/upstream/cluster_manager_impl.h @@ -96,8 +96,8 @@ class ClusterManagerImpl : public ClusterManager { void drainConnPools(HostPtr old_host, ConnPoolsContainer& container); static void updateClusterMembership(const std::string& name, ConstHostVectorPtr hosts, ConstHostVectorPtr healthy_hosts, - ConstHostVectorPtr local_zone_hosts, - ConstHostVectorPtr local_zone_healthy_hosts, + ConstHostListsPtr hosts_per_zone, + ConstHostListsPtr healthy_hosts_per_zone, const std::vector& hosts_added, const std::vector& hosts_removed, ThreadLocal::Instance& tls, uint32_t thread_local_slot); diff --git a/source/common/upstream/load_balancer_impl.cc b/source/common/upstream/load_balancer_impl.cc index 4eca6d431e71..b27125b425c3 100644 --- a/source/common/upstream/load_balancer_impl.cc +++ b/source/common/upstream/load_balancer_impl.cc @@ -8,12 +8,31 @@ namespace Upstream { -const std::vector& LoadBalancerBase::hostsToUse() { - ASSERT(host_set_.healthyHosts().size() <= host_set_.hosts().size()); - if (host_set_.hosts().empty()) { - return host_set_.hosts(); +bool LoadBalancerBase::earlyExitNonZoneRouting() { + uint32_t number_of_zones = host_set_.healthyHostsPerZone().size(); + if (number_of_zones < 2 || + !runtime_.snapshot().featureEnabled("upstream.zone_routing.enabled", 100)) { + return true; + } + + const std::vector& local_zone_healthy_hosts = host_set_.healthyHostsPerZone()[0]; + if (local_zone_healthy_hosts.empty()) { + return true; + } + + // Do not perform zone routing for small clusters. + uint64_t min_cluster_size = + runtime_.snapshot().getInteger("upstream.zone_routing.min_cluster_size", 6U); + + if (host_set_.healthyHosts().size() < min_cluster_size) { + stats_.zone_cluster_too_small_.inc(); + return true; } + return false; +} + +bool LoadBalancerBase::isGlobalPanic() { uint64_t global_panic_threshold = std::min(100UL, runtime_.snapshot().getInteger("upstream.healthy_panic_threshold", 50)); double healthy_percent = 100.0 * host_set_.healthyHosts().size() / host_set_.hosts().size(); @@ -21,36 +40,37 @@ const std::vector& LoadBalancerBase::hostsToUse() { // If the % of healthy hosts in the cluster is less than our panic threshold, we use all hosts. if (healthy_percent < global_panic_threshold) { stats_.upstream_rq_lb_healthy_panic_.inc(); - return host_set_.hosts(); + return true; } - uint32_t number_of_zones = stats_.upstream_zone_count_.value(); - // Early exit if we cannot perform zone aware routing. - if (number_of_zones < 2 || host_set_.localZoneHealthyHosts().empty() || - !runtime_.snapshot().featureEnabled("upstream.zone_routing.enabled", 100)) { - return host_set_.healthyHosts(); - } + return false; +} - // Do not perform zone routing for small clusters. - uint64_t min_cluster_size = - runtime_.snapshot().getInteger("upstream.zone_routing.min_cluster_size", 6U); +const std::vector& LoadBalancerBase::hostsToUse() { + ASSERT(host_set_.healthyHosts().size() <= host_set_.hosts().size()); - if (host_set_.healthyHosts().size() < min_cluster_size) { - stats_.zone_cluster_too_small_.inc(); + if (host_set_.hosts().empty() || isGlobalPanic()) { + return host_set_.hosts(); + } + + if (earlyExitNonZoneRouting()) { return host_set_.healthyHosts(); } - // If number of hosts in a local zone big enough route all requests to the same zone. - if (host_set_.localZoneHealthyHosts().size() * number_of_zones >= - host_set_.healthyHosts().size()) { + // At this point it's guaranteed to be at least 2 zones. + uint32_t number_of_zones = host_set_.healthyHostsPerZone().size(); + ASSERT(number_of_zones >= 2U); + const std::vector& local_zone_healthy_hosts = host_set_.healthyHostsPerZone()[0]; + + // If number of hosts in a local zone big enough then route all requests to the same zone. + if (local_zone_healthy_hosts.size() * number_of_zones >= host_set_.healthyHosts().size()) { stats_.zone_over_percentage_.inc(); - return host_set_.localZoneHealthyHosts(); + return local_zone_healthy_hosts; } // If local zone ratio is lower than expected we should only partially route requests from the // same zone. - double zone_host_ratio = - 1.0 * host_set_.localZoneHealthyHosts().size() / host_set_.healthyHosts().size(); + double zone_host_ratio = 1.0 * local_zone_healthy_hosts.size() / host_set_.healthyHosts().size(); double ratio_to_route = zone_host_ratio * number_of_zones; // Not zone routed requests will be distributed between all hosts and hence @@ -63,7 +83,7 @@ const std::vector& LoadBalancerBase::hostsToUse() { if (random_.random() % 10000 < zone_routing_threshold) { stats_.zone_routing_sampled_.inc(); - return host_set_.localZoneHealthyHosts(); + return local_zone_healthy_hosts; } else { stats_.zone_routing_no_sampled_.inc(); return host_set_.healthyHosts(); diff --git a/source/common/upstream/load_balancer_impl.h b/source/common/upstream/load_balancer_impl.h index 0e02286109ae..b2bc09987641 100644 --- a/source/common/upstream/load_balancer_impl.h +++ b/source/common/upstream/load_balancer_impl.h @@ -26,6 +26,9 @@ class LoadBalancerBase { Runtime::RandomGenerator& random_; private: + bool earlyExitNonZoneRouting(); + bool isGlobalPanic(); + const HostSet& host_set_; const HostSet* local_host_set_; }; diff --git a/source/common/upstream/logical_dns_cluster.cc b/source/common/upstream/logical_dns_cluster.cc index bd6d134d8b71..d88a04844ff2 100644 --- a/source/common/upstream/logical_dns_cluster.cc +++ b/source/common/upstream/logical_dns_cluster.cc @@ -53,8 +53,8 @@ void LogicalDnsCluster::startResolve() { logical_host_.reset(new LogicalHost(*this, dns_url_, *this)); HostVectorPtr new_hosts(new std::vector()); new_hosts->emplace_back(logical_host_); - updateHosts(new_hosts, createHealthyHostList(*new_hosts), empty_host_list_, - empty_host_list_, *new_hosts, {}); + updateHosts(new_hosts, createHealthyHostList(*new_hosts), empty_host_lists_, + empty_host_lists_, *new_hosts, {}); } } diff --git a/source/common/upstream/sds.cc b/source/common/upstream/sds.cc index 44f3baa57e5d..4b6fa172db81 100644 --- a/source/common/upstream/sds.cc +++ b/source/common/upstream/sds.cc @@ -73,17 +73,30 @@ void SdsClusterImpl::parseSdsResponse(Http::Message& response) { if (updateDynamicHostList(new_hosts, *current_hosts_copy, hosts_added, hosts_removed, health_checker_ != nullptr)) { log_debug("sds hosts changed for cluster: {} ({})", name_, hosts().size()); - HostVectorPtr local_zone_hosts(new std::vector()); + HostListsPtr per_zone(new std::vector>()); + + // If local zone name is not defined then skip populating per zone hosts. if (!sds_config_.local_zone_name_.empty()) { + std::map> hosts_per_zone; + for (HostPtr host : *current_hosts_copy) { - if (host->zone() == sds_config_.local_zone_name_) { - local_zone_hosts->push_back(host); + hosts_per_zone[host->zone()].push_back(host); + } + + // Populate per_zone hosts only if upstream cluster has hosts in the same zone. + if (hosts_per_zone.find(sds_config_.local_zone_name_) != hosts_per_zone.end()) { + per_zone->push_back(hosts_per_zone[sds_config_.local_zone_name_]); + + for (auto& entry : hosts_per_zone) { + if (sds_config_.local_zone_name_ != entry.first) { + per_zone->push_back(entry.second); + } } } } - updateHosts(current_hosts_copy, createHealthyHostList(*current_hosts_copy), local_zone_hosts, - createHealthyHostList(*local_zone_hosts), hosts_added, hosts_removed); + updateHosts(current_hosts_copy, createHealthyHostList(*current_hosts_copy), per_zone, + createHealthyHostLists(*per_zone), hosts_added, hosts_removed); if (initialize_callback_ && health_checker_ && pending_health_checks_ == 0) { pending_health_checks_ = hosts().size(); diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index f33c841c6057..3db680387156 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -52,7 +52,7 @@ void HostSetImpl::runUpdateCallbacks(const std::vector& hosts_added, } } -const ConstHostVectorPtr ClusterImplBase::empty_host_list_{new std::vector{}}; +const ConstHostListsPtr ClusterImplBase::empty_host_lists_{new std::vector>()}; ClusterImplBase::ClusterImplBase(const Json::Object& config, Runtime::Loader& runtime, Stats::Store& stats, Ssl::ContextManager& ssl_context_manager) @@ -85,7 +85,7 @@ ClusterImplBase::ClusterImplBase(const Json::Object& config, Runtime::Loader& ru ConstHostVectorPtr ClusterImplBase::createHealthyHostList(const std::vector& hosts) { HostVectorPtr healthy_list(new std::vector()); - for (auto host : hosts) { + for (const auto& host : hosts) { if (host->healthy()) { healthy_list->emplace_back(host); } @@ -94,6 +94,23 @@ ConstHostVectorPtr ClusterImplBase::createHealthyHostList(const std::vector>& hosts) { + HostListsPtr healthy_list(new std::vector>()); + + for (const auto& hosts_zone : hosts) { + std::vector current_zone_hosts; + for (const auto& host : hosts_zone) { + if (host->healthy()) { + current_zone_hosts.emplace_back(host); + } + } + healthy_list->push_back(std::move(current_zone_hosts)); + } + + return healthy_list; +} + uint64_t ClusterImplBase::parseFeatures(const Json::Object& config) { uint64_t features = 0; for (const std::string& feature : StringUtil::split(config.getString("features", ""), ',')) { @@ -130,8 +147,8 @@ void ClusterImplBase::setHealthChecker(HealthCheckerPtr&& health_checker) { // If we get a health check completion that resulted in a state change, signal to // update the host sets on all threads. if (changed_state) { - updateHosts(rawHosts(), createHealthyHostList(*rawHosts()), rawLocalZoneHosts(), - createHealthyHostList(*rawLocalZoneHosts()), {}, {}); + updateHosts(rawHosts(), createHealthyHostList(*rawHosts()), rawHostsPerZone(), + createHealthyHostLists(*rawHostsPerZone()), {}, {}); } }); } @@ -143,8 +160,8 @@ void ClusterImplBase::setOutlierDetector(OutlierDetectorPtr&& outlier_detector) outlier_detector_ = std::move(outlier_detector); outlier_detector_->addChangedStateCb([this](HostPtr) -> void { - updateHosts(rawHosts(), createHealthyHostList(*rawHosts()), rawLocalZoneHosts(), - createHealthyHostList(*rawLocalZoneHosts()), {}, {}); + updateHosts(rawHosts(), createHealthyHostList(*rawHosts()), rawHostsPerZone(), + createHealthyHostLists(*rawHostsPerZone()), {}, {}); }); } @@ -187,8 +204,8 @@ StaticClusterImpl::StaticClusterImpl(const Json::Object& config, Runtime::Loader new_hosts->emplace_back(HostPtr{new HostImpl(*this, url, false, 1, "")}); } - updateHosts(new_hosts, createHealthyHostList(*new_hosts), empty_host_list_, empty_host_list_, {}, - {}); + updateHosts(new_hosts, createHealthyHostList(*new_hosts), empty_host_lists_, empty_host_lists_, + {}, {}); } bool BaseDynamicClusterImpl::updateDynamicHostList(const std::vector& new_hosts, @@ -197,7 +214,6 @@ bool BaseDynamicClusterImpl::updateDynamicHostList(const std::vector& n std::vector& hosts_removed, bool depend_on_hc) { uint64_t max_host_weight = 1; - std::unordered_set zones; // Go through and see if the list we have is different from what we just got. If it is, we // make a new host list and raise a change notification. This uses an N^2 search given that @@ -209,7 +225,6 @@ bool BaseDynamicClusterImpl::updateDynamicHostList(const std::vector& n // If we find a host matched based on URL, we keep it. However we do change weight inline so // do that here. if ((*i)->url() == host->url()) { - zones.insert((*i)->zone()); if (host->weight() > max_host_weight) { max_host_weight = host->weight(); } @@ -227,7 +242,6 @@ bool BaseDynamicClusterImpl::updateDynamicHostList(const std::vector& n if (host->weight() > max_host_weight) { max_host_weight = host->weight(); } - zones.insert(host->zone()); final_hosts.push_back(host); hosts_added.push_back(host); @@ -246,7 +260,6 @@ bool BaseDynamicClusterImpl::updateDynamicHostList(const std::vector& n if ((*i)->weight() > max_host_weight) { max_host_weight = (*i)->weight(); } - zones.insert((*i)->zone()); final_hosts.push_back(*i); i = current_hosts.erase(i); @@ -257,7 +270,6 @@ bool BaseDynamicClusterImpl::updateDynamicHostList(const std::vector& n } stats_.max_host_weight_.set(max_host_weight); - stats_.upstream_zone_count_.set(zones.size()); if (!hosts_added.empty() || !current_hosts.empty()) { hosts_removed = std::move(current_hosts); @@ -295,7 +307,7 @@ void StrictDnsClusterImpl::updateAllHosts(const std::vector& hosts_adde } } - updateHosts(new_hosts, createHealthyHostList(*new_hosts), empty_host_list_, empty_host_list_, + updateHosts(new_hosts, createHealthyHostList(*new_hosts), empty_host_lists_, empty_host_lists_, hosts_added, hosts_removed); } diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h index 5cccee519bca..8b69af6fd89f 100644 --- a/source/common/upstream/upstream_impl.h +++ b/source/common/upstream/upstream_impl.h @@ -101,6 +101,8 @@ class HostImpl : public HostDescriptionImpl, typedef std::shared_ptr> HostVectorPtr; typedef std::shared_ptr> ConstHostVectorPtr; +typedef std::shared_ptr>> HostListsPtr; +typedef std::shared_ptr>> ConstHostListsPtr; /** * Base clase for all clusters as well as thread local host sets. @@ -111,25 +113,27 @@ class HostSetImpl : public virtual HostSet { ConstHostVectorPtr rawHosts() const { return hosts_; } ConstHostVectorPtr rawHealthyHosts() const { return healthy_hosts_; } - ConstHostVectorPtr rawLocalZoneHosts() const { return local_zone_hosts_; } - ConstHostVectorPtr rawLocalZoneHealthyHosts() const { return local_zone_healthy_hosts_; } + ConstHostListsPtr rawHostsPerZone() const { return hosts_per_zone_; } + ConstHostListsPtr rawHealthyHostsPerZone() const { return healthy_hosts_per_zone_; } void updateHosts(ConstHostVectorPtr hosts, ConstHostVectorPtr healthy_hosts, - ConstHostVectorPtr local_zone_hosts, ConstHostVectorPtr local_zone_healthy_hosts, + ConstHostListsPtr hosts_per_zone, ConstHostListsPtr healthy_hosts_per_zone, const std::vector& hosts_added, const std::vector& hosts_removed) { hosts_ = hosts; healthy_hosts_ = healthy_hosts; - local_zone_hosts_ = local_zone_hosts; - local_zone_healthy_hosts_ = local_zone_healthy_hosts; + hosts_per_zone_ = hosts_per_zone; + healthy_hosts_per_zone_ = healthy_hosts_per_zone; runUpdateCallbacks(hosts_added, hosts_removed); } // Upstream::HostSet const std::vector& hosts() const override { return *hosts_; } const std::vector& healthyHosts() const override { return *healthy_hosts_; } - const std::vector& localZoneHosts() const override { return *local_zone_hosts_; } - const std::vector& localZoneHealthyHosts() const override { - return *local_zone_healthy_hosts_; + const std::vector>& hostsPerZone() const override { + return *hosts_per_zone_; + } + const std::vector>& healthyHostsPerZone() const override { + return *healthy_hosts_per_zone_; } void addMemberUpdateCb(MemberUpdateCb callback) const override; @@ -140,8 +144,8 @@ class HostSetImpl : public virtual HostSet { private: ConstHostVectorPtr hosts_; ConstHostVectorPtr healthy_hosts_; - ConstHostVectorPtr local_zone_hosts_; - ConstHostVectorPtr local_zone_healthy_hosts_; + ConstHostListsPtr hosts_per_zone_; + ConstHostListsPtr healthy_hosts_per_zone_; mutable std::list callbacks_; }; @@ -187,10 +191,11 @@ class ClusterImplBase : public Cluster, Ssl::ContextManager& ssl_context_manager); static ConstHostVectorPtr createHealthyHostList(const std::vector& hosts); + static ConstHostListsPtr createHealthyHostLists(const std::vector>& hosts); void runUpdateCallbacks(const std::vector& hosts_added, const std::vector& hosts_removed) override; - static const ConstHostVectorPtr empty_host_list_; + static const ConstHostListsPtr empty_host_lists_; Ssl::ClientContext* ssl_ctx_; std::string name_; diff --git a/test/common/upstream/load_balancer_impl_test.cc b/test/common/upstream/load_balancer_impl_test.cc index a6b564ef116d..36576a32e91e 100644 --- a/test/common/upstream/load_balancer_impl_test.cc +++ b/test/common/upstream/load_balancer_impl_test.cc @@ -74,9 +74,9 @@ TEST_F(RoundRobinLoadBalancerTest, ZoneAwareSmallCluster) { cluster_.hosts_ = {newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:80"), newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:81"), newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:82")}; - cluster_.local_zone_hosts_ = {newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:81")}; - cluster_.local_zone_healthy_hosts_ = {newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:81")}; - stats_.upstream_zone_count_.set(3UL); + cluster_.healthy_hosts_per_zone_ = {{newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:81")}, + {newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:80")}, + {newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:82")}}; EXPECT_CALL(runtime_.snapshot_, featureEnabled("upstream.zone_routing.enabled", 100)) .WillRepeatedly(Return(true)); @@ -96,9 +96,9 @@ TEST_F(RoundRobinLoadBalancerTest, ZoneAwareRoutingLargeZone) { cluster_.hosts_ = {newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:80"), newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:81"), newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:82")}; - cluster_.local_zone_hosts_ = {newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:81")}; - cluster_.local_zone_healthy_hosts_ = {newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:81")}; - stats_.upstream_zone_count_.set(3UL); + cluster_.healthy_hosts_per_zone_ = {{newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:81")}, + {newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:80")}, + {newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:82")}}; EXPECT_CALL(runtime_.snapshot_, getInteger("upstream.healthy_panic_threshold", 50)) .WillRepeatedly(Return(50)); @@ -109,9 +109,9 @@ TEST_F(RoundRobinLoadBalancerTest, ZoneAwareRoutingLargeZone) { .WillRepeatedly(Return(3)); // There is only one host in the given zone for zone aware routing. - EXPECT_EQ(cluster_.local_zone_healthy_hosts_[0], lb_.chooseHost()); + EXPECT_EQ(cluster_.healthy_hosts_per_zone_[0][0], lb_.chooseHost()); EXPECT_EQ(1U, stats_.zone_over_percentage_.value()); - EXPECT_EQ(cluster_.local_zone_healthy_hosts_[0], lb_.chooseHost()); + EXPECT_EQ(cluster_.healthy_hosts_per_zone_[0][0], lb_.chooseHost()); EXPECT_EQ(2U, stats_.zone_over_percentage_.value()); // Disable runtime global zone routing. @@ -131,9 +131,11 @@ TEST_F(RoundRobinLoadBalancerTest, ZoneAwareRoutingSmallZone) { newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:82"), newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:83"), newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:84")}; - cluster_.local_zone_hosts_ = {newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:81")}; - cluster_.local_zone_healthy_hosts_ = {newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:81")}; - stats_.upstream_zone_count_.set(3UL); + cluster_.healthy_hosts_per_zone_ = {{newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:81")}, + {newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:80"), + newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:82")}, + {newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:83"), + newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:84")}}; EXPECT_CALL(runtime_.snapshot_, getInteger("upstream.healthy_panic_threshold", 50)) .WillRepeatedly(Return(50)); @@ -145,7 +147,7 @@ TEST_F(RoundRobinLoadBalancerTest, ZoneAwareRoutingSmallZone) { // There is only one host in the given zone for zone aware routing. EXPECT_CALL(random_, random()).WillOnce(Return(1000)); - EXPECT_EQ(cluster_.local_zone_healthy_hosts_[0], lb_.chooseHost()); + EXPECT_EQ(cluster_.healthy_hosts_per_zone_[0][0], lb_.chooseHost()); EXPECT_EQ(1U, stats_.zone_routing_sampled_.value()); EXPECT_CALL(random_, random()).WillOnce(Return(6500)); EXPECT_EQ(cluster_.healthy_hosts_[1], lb_.chooseHost()); @@ -155,9 +157,7 @@ TEST_F(RoundRobinLoadBalancerTest, ZoneAwareRoutingSmallZone) { TEST_F(RoundRobinLoadBalancerTest, NoZoneAwareRoutingOneZone) { cluster_.healthy_hosts_ = {newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:80")}; cluster_.hosts_ = {newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:80")}; - cluster_.local_zone_hosts_ = {newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:80")}; - cluster_.local_zone_healthy_hosts_ = {newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:80")}; - stats_.upstream_zone_count_.set(1UL); + cluster_.healthy_hosts_per_zone_ = {{newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:80")}}; EXPECT_CALL(runtime_.snapshot_, featureEnabled("upstream.zone_routing.enabled", 100)).Times(0); EXPECT_CALL(runtime_.snapshot_, getInteger("upstream.healthy_panic_threshold", 50)) @@ -173,9 +173,7 @@ TEST_F(RoundRobinLoadBalancerTest, ZoneAwareRoutingNotHealthy) { cluster_.hosts_ = {newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:80"), newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:81"), newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:82")}; - cluster_.local_zone_hosts_ = {newTestHost(Upstream::MockCluster{}, "tcp://127.0.0.1:81")}; - cluster_.local_zone_healthy_hosts_ = {}; - stats_.upstream_zone_count_.set(3UL); + cluster_.healthy_hosts_per_zone_ = {{}, {}, {}}; EXPECT_CALL(runtime_.snapshot_, featureEnabled("upstream.zone_routing.enabled", 100)) .WillRepeatedly(Return(true)); diff --git a/test/common/upstream/load_balancer_simulation_test.cc b/test/common/upstream/load_balancer_simulation_test.cc index bca5896a3472..bb04b638cc92 100644 --- a/test/common/upstream/load_balancer_simulation_test.cc +++ b/test/common/upstream/load_balancer_simulation_test.cc @@ -39,12 +39,9 @@ class DISABLED_SimulationTest : public testing::Test { */ void run(std::vector originating_cluster, std::vector all_destination_cluster, std::vector healthy_destination_cluster) { - stats_.upstream_zone_count_.set(all_destination_cluster.size()); - std::unordered_map> healthy_map = + std::vector> per_zone_hosts = generateHostsPerZone(healthy_destination_cluster); - std::unordered_map> all_map = - generateHostsPerZone(all_destination_cluster); std::vector originating_hosts = generateHostList(originating_cluster); cluster_.healthy_hosts_ = generateHostList(healthy_destination_cluster); @@ -53,9 +50,19 @@ class DISABLED_SimulationTest : public testing::Test { std::map hits; for (uint32_t i = 0; i < total_number_of_requests; ++i) { HostPtr from_host = selectOriginatingHost(originating_hosts); + uint32_t from_zone = atoi(from_host->zone().c_str()); - cluster_.local_zone_hosts_ = all_map[from_host->zone()]; - cluster_.local_zone_healthy_hosts_ = healthy_map[from_host->zone()]; + std::vector> per_zone_upstream; + per_zone_upstream.push_back(per_zone_hosts[from_zone]); + for (size_t pos = 0; pos < per_zone_hosts.size(); ++pos) { + if (pos == from_zone) { + continue; + } + + per_zone_upstream.push_back(per_zone_hosts[pos]); + } + + cluster_.healthy_hosts_per_zone_ = std::move(per_zone_upstream); ConstHostPtr selected = lb_.chooseHost(); hits[selected->url()]++; @@ -93,9 +100,8 @@ class DISABLED_SimulationTest : public testing::Test { * Generate hosts by zone. * @param hosts number of hosts per zone. */ - std::unordered_map> - generateHostsPerZone(const std::vector& hosts) { - std::unordered_map> ret; + std::vector> generateHostsPerZone(const std::vector& hosts) { + std::vector> ret; for (size_t i = 0; i < hosts.size(); ++i) { const std::string zone = std::to_string(i); std::vector zone_hosts; @@ -105,7 +111,7 @@ class DISABLED_SimulationTest : public testing::Test { zone_hosts.push_back(newTestHost(cluster_, url, 1, zone)); } - ret.insert({zone, std::move(zone_hosts)}); + ret.push_back(std::move(zone_hosts)); } return ret; diff --git a/test/common/upstream/logical_dns_cluster_test.cc b/test/common/upstream/logical_dns_cluster_test.cc index bc36c2cf6cd7..cdc5dd162d2a 100644 --- a/test/common/upstream/logical_dns_cluster_test.cc +++ b/test/common/upstream/logical_dns_cluster_test.cc @@ -78,8 +78,8 @@ TEST_F(LogicalDnsClusterTest, Basic) { EXPECT_EQ(1UL, cluster_->hosts().size()); EXPECT_EQ(1UL, cluster_->healthyHosts().size()); - EXPECT_EQ(0UL, cluster_->localZoneHosts().size()); - EXPECT_EQ(0UL, cluster_->localZoneHealthyHosts().size()); + EXPECT_EQ(0UL, cluster_->hostsPerZone().size()); + EXPECT_EQ(0UL, cluster_->healthyHostsPerZone().size()); EXPECT_EQ(cluster_->hosts()[0], cluster_->healthyHosts()[0]); HostPtr logical_host = cluster_->hosts()[0]; diff --git a/test/common/upstream/sds_test.cc b/test/common/upstream/sds_test.cc index 7dbb46767d5e..3aec2a236b9e 100644 --- a/test/common/upstream/sds_test.cc +++ b/test/common/upstream/sds_test.cc @@ -124,14 +124,16 @@ TEST_F(SdsTest, NoHealthChecker) { callbacks_->onSuccess(std::move(message)); EXPECT_EQ(13UL, cluster_->hosts().size()); EXPECT_EQ(13UL, cluster_->healthyHosts().size()); - EXPECT_EQ(4UL, cluster_->localZoneHosts().size()); - EXPECT_EQ(4UL, cluster_->localZoneHealthyHosts().size()); + EXPECT_EQ(3UL, cluster_->healthyHostsPerZone().size()); + EXPECT_EQ(4UL, cluster_->healthyHostsPerZone()[0].size()); + EXPECT_EQ(5UL, cluster_->healthyHostsPerZone()[1].size()); + EXPECT_EQ(4UL, cluster_->healthyHostsPerZone()[2].size()); + HostPtr canary_host = findHost("10.0.16.43"); EXPECT_TRUE(canary_host->canary()); EXPECT_EQ("us-east-1d", canary_host->zone()); EXPECT_EQ(1U, canary_host->weight()); EXPECT_EQ(1UL, cluster_->stats().max_host_weight_.value()); - EXPECT_EQ(3UL, cluster_->stats().upstream_zone_count_.value()); // Test response with weight change. We should still have the same host. setupRequest(); @@ -149,7 +151,10 @@ TEST_F(SdsTest, NoHealthChecker) { EXPECT_EQ("us-east-1d", canary_host->zone()); EXPECT_EQ(50U, canary_host->weight()); EXPECT_EQ(50UL, cluster_->stats().max_host_weight_.value()); - EXPECT_EQ(3UL, cluster_->stats().upstream_zone_count_.value()); + EXPECT_EQ(3UL, cluster_->healthyHostsPerZone().size()); + EXPECT_EQ(4UL, cluster_->healthyHostsPerZone()[0].size()); + EXPECT_EQ(5UL, cluster_->healthyHostsPerZone()[1].size()); + EXPECT_EQ(4UL, cluster_->healthyHostsPerZone()[2].size()); // Now test the failure case, our cluster size should not change. setupRequest(); @@ -160,7 +165,10 @@ TEST_F(SdsTest, NoHealthChecker) { EXPECT_EQ(13UL, cluster_->hosts().size()); EXPECT_EQ(50U, canary_host->weight()); EXPECT_EQ(50UL, cluster_->stats().max_host_weight_.value()); - EXPECT_EQ(3UL, cluster_->stats().upstream_zone_count_.value()); + EXPECT_EQ(3UL, cluster_->healthyHostsPerZone().size()); + EXPECT_EQ(4UL, cluster_->healthyHostsPerZone()[0].size()); + EXPECT_EQ(5UL, cluster_->healthyHostsPerZone()[1].size()); + EXPECT_EQ(4UL, cluster_->healthyHostsPerZone()[2].size()); // 503 response. setupRequest(); @@ -173,7 +181,10 @@ TEST_F(SdsTest, NoHealthChecker) { EXPECT_EQ(13UL, cluster_->hosts().size()); EXPECT_EQ(50U, canary_host->weight()); EXPECT_EQ(50UL, cluster_->stats().max_host_weight_.value()); - EXPECT_EQ(3UL, cluster_->stats().upstream_zone_count_.value()); + EXPECT_EQ(3UL, cluster_->healthyHostsPerZone().size()); + EXPECT_EQ(4UL, cluster_->healthyHostsPerZone()[0].size()); + EXPECT_EQ(5UL, cluster_->healthyHostsPerZone()[1].size()); + EXPECT_EQ(4UL, cluster_->healthyHostsPerZone()[2].size()); } TEST_F(SdsTest, HealthChecker) { @@ -198,8 +209,11 @@ TEST_F(SdsTest, HealthChecker) { EXPECT_EQ(13UL, cluster_->hosts().size()); EXPECT_EQ(0UL, cluster_->healthyHosts().size()); EXPECT_EQ(0UL, numHealthy()); - EXPECT_EQ(4UL, cluster_->localZoneHosts().size()); - EXPECT_EQ(0UL, cluster_->localZoneHealthyHosts().size()); + EXPECT_EQ(3UL, cluster_->healthyHostsPerZone().size()); + EXPECT_EQ(3UL, cluster_->hostsPerZone().size()); + EXPECT_EQ(0UL, cluster_->healthyHostsPerZone()[0].size()); + EXPECT_EQ(0UL, cluster_->healthyHostsPerZone()[1].size()); + EXPECT_EQ(0UL, cluster_->healthyHostsPerZone()[2].size()); // Now run through and make all the hosts healthy except for the first one. for (size_t i = 1; i < cluster_->hosts().size(); i++) { @@ -208,16 +222,20 @@ TEST_F(SdsTest, HealthChecker) { } EXPECT_EQ(12UL, cluster_->healthyHosts().size()); - EXPECT_EQ(4UL, cluster_->localZoneHosts().size()); - EXPECT_EQ(3UL, cluster_->localZoneHealthyHosts().size()); + EXPECT_EQ(3UL, cluster_->healthyHostsPerZone().size()); + EXPECT_EQ(3UL, cluster_->healthyHostsPerZone()[0].size()); + EXPECT_EQ(5UL, cluster_->healthyHostsPerZone()[1].size()); + EXPECT_EQ(4UL, cluster_->healthyHostsPerZone()[2].size()); // Do the last one now which should fire the initialized event. EXPECT_CALL(membership_updated_, ready()); cluster_->hosts()[0]->healthFlagClear(Host::HealthFlag::FAILED_ACTIVE_HC); health_checker->runCallbacks(cluster_->hosts()[0], true); EXPECT_EQ(13UL, cluster_->healthyHosts().size()); - EXPECT_EQ(4UL, cluster_->localZoneHosts().size()); - EXPECT_EQ(4UL, cluster_->localZoneHealthyHosts().size()); + EXPECT_EQ(3UL, cluster_->healthyHostsPerZone().size()); + EXPECT_EQ(4UL, cluster_->healthyHostsPerZone()[0].size()); + EXPECT_EQ(5UL, cluster_->healthyHostsPerZone()[1].size()); + EXPECT_EQ(4UL, cluster_->healthyHostsPerZone()[2].size()); // Now we will remove some hosts, but since they are all healthy, they shouldn't actually be gone. setupRequest(); @@ -232,8 +250,10 @@ TEST_F(SdsTest, HealthChecker) { EXPECT_EQ(13UL, cluster_->hosts().size()); EXPECT_EQ(13UL, cluster_->healthyHosts().size()); EXPECT_EQ(13UL, numHealthy()); - EXPECT_EQ(4UL, cluster_->localZoneHosts().size()); - EXPECT_EQ(4UL, cluster_->localZoneHealthyHosts().size()); + EXPECT_EQ(3UL, cluster_->healthyHostsPerZone().size()); + EXPECT_EQ(4UL, cluster_->healthyHostsPerZone()[0].size()); + EXPECT_EQ(5UL, cluster_->healthyHostsPerZone()[1].size()); + EXPECT_EQ(4UL, cluster_->healthyHostsPerZone()[2].size()); // Now set one of the removed hosts to unhealthy, and return the same query again, this should // remove it. @@ -249,8 +269,10 @@ TEST_F(SdsTest, HealthChecker) { EXPECT_EQ(12UL, cluster_->hosts().size()); EXPECT_EQ(12UL, cluster_->healthyHosts().size()); EXPECT_EQ(12UL, numHealthy()); - EXPECT_EQ(3UL, cluster_->localZoneHosts().size()); - EXPECT_EQ(3UL, cluster_->localZoneHealthyHosts().size()); + EXPECT_EQ(3UL, cluster_->healthyHostsPerZone().size()); + EXPECT_EQ(3UL, cluster_->healthyHostsPerZone()[0].size()); + EXPECT_EQ(5UL, cluster_->healthyHostsPerZone()[1].size()); + EXPECT_EQ(4UL, cluster_->healthyHostsPerZone()[2].size()); // Now add back one of the hosts that was previously missing but we still have and make sure // nothing changes. @@ -265,8 +287,10 @@ TEST_F(SdsTest, HealthChecker) { EXPECT_EQ(12UL, cluster_->hosts().size()); EXPECT_EQ(12UL, cluster_->healthyHosts().size()); EXPECT_EQ(12UL, numHealthy()); - EXPECT_EQ(3UL, cluster_->localZoneHosts().size()); - EXPECT_EQ(3UL, cluster_->localZoneHealthyHosts().size()); + EXPECT_EQ(3UL, cluster_->healthyHostsPerZone().size()); + EXPECT_EQ(3UL, cluster_->healthyHostsPerZone()[0].size()); + EXPECT_EQ(5UL, cluster_->healthyHostsPerZone()[1].size()); + EXPECT_EQ(4UL, cluster_->healthyHostsPerZone()[2].size()); } } // Upstream diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc index bd4dc0637733..c96d08758a84 100644 --- a/test/common/upstream/upstream_impl_test.cc +++ b/test/common/upstream/upstream_impl_test.cc @@ -132,8 +132,8 @@ TEST(StrictDnsClusterImplTest, Basic) { ContainerEq(hostListToURLs(cluster.hosts()))); EXPECT_EQ(2UL, cluster.healthyHosts().size()); - EXPECT_EQ(0UL, cluster.localZoneHosts().size()); - EXPECT_EQ(0UL, cluster.localZoneHealthyHosts().size()); + EXPECT_EQ(0UL, cluster.hostsPerZone().size()); + EXPECT_EQ(0UL, cluster.healthyHostsPerZone().size()); for (const HostPtr& host : cluster.hosts()) { EXPECT_EQ(&cluster, &host->cluster()); @@ -255,8 +255,8 @@ TEST(StaticClusterImplTest, UrlConfig) { EXPECT_THAT(std::list({"tcp://10.0.0.1:11001", "tcp://10.0.0.2:11002"}), ContainerEq(hostListToURLs(cluster.hosts()))); EXPECT_EQ(2UL, cluster.healthyHosts().size()); - EXPECT_EQ(0UL, cluster.localZoneHosts().size()); - EXPECT_EQ(0UL, cluster.localZoneHealthyHosts().size()); + EXPECT_EQ(0UL, cluster.hostsPerZone().size()); + EXPECT_EQ(0UL, cluster.healthyHostsPerZone().size()); } TEST(StaticClusterImplTest, UnsupportedLBType) { diff --git a/test/mocks/upstream/mocks.cc b/test/mocks/upstream/mocks.cc index 66f9fcdaea74..d8f775c4e731 100644 --- a/test/mocks/upstream/mocks.cc +++ b/test/mocks/upstream/mocks.cc @@ -38,8 +38,8 @@ MockCluster::MockCluster() ON_CALL(*this, connectTimeout()).WillByDefault(Return(std::chrono::milliseconds(1))); ON_CALL(*this, hosts()).WillByDefault(ReturnRef(hosts_)); ON_CALL(*this, healthyHosts()).WillByDefault(ReturnRef(healthy_hosts_)); - ON_CALL(*this, localZoneHosts()).WillByDefault(ReturnRef(local_zone_hosts_)); - ON_CALL(*this, localZoneHealthyHosts()).WillByDefault(ReturnRef(local_zone_healthy_hosts_)); + ON_CALL(*this, hostsPerZone()).WillByDefault(ReturnRef(hosts_per_zone_)); + ON_CALL(*this, healthyHostsPerZone()).WillByDefault(ReturnRef(healthy_hosts_per_zone_)); ON_CALL(*this, name()).WillByDefault(ReturnRef(name_)); ON_CALL(*this, altStatName()).WillByDefault(ReturnRef(alt_stat_name_)); ON_CALL(*this, lbType()).WillByDefault(Return(Upstream::LoadBalancerType::RoundRobin)); diff --git a/test/mocks/upstream/mocks.h b/test/mocks/upstream/mocks.h index 99de3d5ace57..03d9c368dc4e 100644 --- a/test/mocks/upstream/mocks.h +++ b/test/mocks/upstream/mocks.h @@ -29,8 +29,8 @@ class MockCluster : public Cluster { MOCK_CONST_METHOD1(addMemberUpdateCb, void(MemberUpdateCb callback)); MOCK_CONST_METHOD0(hosts, const std::vector&()); MOCK_CONST_METHOD0(healthyHosts, const std::vector&()); - MOCK_CONST_METHOD0(localZoneHosts, const std::vector&()); - MOCK_CONST_METHOD0(localZoneHealthyHosts, const std::vector&()); + MOCK_CONST_METHOD0(hostsPerZone, const std::vector>&()); + MOCK_CONST_METHOD0(healthyHostsPerZone, const std::vector>&()); // Upstream::Cluster MOCK_CONST_METHOD0(altStatName, const std::string&()); @@ -48,8 +48,8 @@ class MockCluster : public Cluster { std::vector hosts_; std::vector healthy_hosts_; - std::vector local_zone_hosts_; - std::vector local_zone_healthy_hosts_; + std::vector> hosts_per_zone_; + std::vector> healthy_hosts_per_zone_; std::string name_{"fake_cluster"}; std::string alt_stat_name_{"fake_alt_cluster"}; std::list callbacks_;