diff --git a/api/envoy/api/v2/eds.proto b/api/envoy/api/v2/eds.proto index 44a1fe0a9749..d4fcfe0152c7 100644 --- a/api/envoy/api/v2/eds.proto +++ b/api/envoy/api/v2/eds.proto @@ -12,6 +12,7 @@ import "google/api/annotations.proto"; import "validate/validate.proto"; import "gogoproto/gogo.proto"; +import "google/protobuf/wrappers.proto"; option (gogoproto.equal_all) = true; @@ -80,6 +81,17 @@ message ClusterLoadAssignment { // "lb"_drop = 20% // 50% of the remaining 'actual' load, which is 40%. // actual_outgoing_load = 20% // remaining after applying all categories. repeated DropOverload drop_overloads = 2; + + // Priority levels and localities are considered overprovisioned with this + // factor (in percentage). This means that we don't consider a priority + // level or locality unhealthy until the percentage of healthy hosts + // multiplied by the overprovisioning factor drops below 100. + // With the default value 140(1.4), Envoy doesn't consider a priority level + // or a locality unhealthy until their percentage of healthy hosts drops + // below 72%. + // Read more at :ref:`priority levels ` and + // :ref:`localities `. + google.protobuf.UInt32Value overprovisioning_factor = 3 [(validate.rules).uint32.gt = 0]; } // Load balancing policy settings. diff --git a/docs/root/intro/arch_overview/load_balancing.rst b/docs/root/intro/arch_overview/load_balancing.rst index e23cc1862ada..2052e79691ad 100644 --- a/docs/root/intro/arch_overview/load_balancing.rst +++ b/docs/root/intro/arch_overview/load_balancing.rst @@ -132,6 +132,17 @@ Please note that fully resolved IP address should be passed in this header. For routed to a host with IP address 10.195.16.237 at port 8888, the request header value should be set as ``10.195.16.237:8888``. +.. _arch_overview_load_balancing_overprovisioning_factor: + +Overprovisioning Factor +----------------------- +Priority levels and localities are considered overprovisioned with +:ref:`this percentage `. +Envoy doesn't consider a priority level or locality unhealthy until the +percentage of healthy hosts multiplied by the overprovisioning factor drops +below 100. The default value is 1.4, so a priority level or locality will not be +considered unhealthy until the percentage of healthy endpoints goes below 72%. + .. _arch_overview_load_balancing_panic_threshold: Panic threshold @@ -163,9 +174,10 @@ priority may also be specified. When endpoints at the highest priority level (P= traffic will land on endpoints in that priority level. As endpoints for the highest priority level become unhealthy, traffic will begin to trickle to lower priority levels. -Currently, it is assumed that each priority level is over-provisioned by a (hard-coded) factor of -1.4. So if 80% of the endpoints are healthy, the priority level is still considered healthy because -80*1.4 > 100. As the number of healthy endpoints dips below 72%, the health of the priority level +Currently, it is assumed that each priority level is over-provisioned by the +:ref:`overprovisioning factor `. +With default factor value 1.4, if 80% of the endpoints are healthy, the priority level is still considered +healthy because 80*1.4 > 100. As the number of healthy endpoints dips below 72%, the health of the priority level goes below 100. At that point the percent of traffic equivalent to the health of P=0 will go to P=0 and remaining traffic will flow to P=1. @@ -303,12 +315,14 @@ When all endpoints are healthy, the locality is picked using a weighted round-robin schedule, where the locality weight is used for weighting. When some endpoints in a locality are unhealthy, we adjust the locality weight to reflect this. As with :ref:`priority levels -`, we assume an over-provision -factor (currently hardcoded at 1.4), which means we do not perform any weight +`, we assume an +:ref:`over-provision factor ` +(default value 1.4), which means we do not perform any weight adjustment when only a small number of endpoints in a locality are unhealthy. Assume a simple set-up with 2 localities X and Y, where X has a locality weight -of 1 and Y has a locality weight of 2, L=Y 100% healthy. +of 1 and Y has a locality weight of 2, L=Y 100% healthy, +with default overprovisioning factor 1.4. +----------------------------+---------------------------+----------------------------+ | L=X healthy endpoints | Percent of traffic to L=X | Percent of traffic to L=Y | diff --git a/include/envoy/upstream/upstream.h b/include/envoy/upstream/upstream.h index d89ae682c541..655bd2de4570 100644 --- a/include/envoy/upstream/upstream.h +++ b/include/envoy/upstream/upstream.h @@ -237,17 +237,24 @@ class HostSet { * @param locality_weights supplies a map from locality to associated weight. * @param hosts_added supplies the hosts added since the last update. * @param hosts_removed supplies the hosts removed since the last update. + * @param overprovisioning_factor if presents, overwrites the current overprovisioning_factor. */ virtual void updateHosts(HostVectorConstSharedPtr hosts, HostVectorConstSharedPtr healthy_hosts, HostsPerLocalityConstSharedPtr hosts_per_locality, HostsPerLocalityConstSharedPtr healthy_hosts_per_locality, LocalityWeightsConstSharedPtr locality_weights, - const HostVector& hosts_added, const HostVector& hosts_removed) PURE; + const HostVector& hosts_added, const HostVector& hosts_removed, + absl::optional overprovisioning_factor) PURE; /** * @return uint32_t the priority of this host set. */ virtual uint32_t priority() const PURE; + + /** + * @return uint32_t the overprovisioning factor of this host set. + */ + virtual uint32_t overprovisioning_factor() const PURE; }; typedef std::unique_ptr HostSetPtr; diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index b77d7007567e..f17f8d605387 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -958,7 +958,7 @@ void ClusterManagerImpl::ThreadLocalClusterManagerImpl::updateClusterMembership( cluster_entry->priority_set_.getOrCreateHostSet(priority).updateHosts( std::move(hosts), std::move(healthy_hosts), std::move(hosts_per_locality), std::move(healthy_hosts_per_locality), std::move(locality_weights), hosts_added, - hosts_removed); + hosts_removed, absl::nullopt); // If an LB is thread aware, create a new worker local LB on membership changes. if (cluster_entry->lb_factory_ != nullptr) { diff --git a/source/common/upstream/eds.cc b/source/common/upstream/eds.cc index d2c2e26fcc68..306602a3b854 100644 --- a/source/common/upstream/eds.cc +++ b/source/common/upstream/eds.cc @@ -13,6 +13,7 @@ #include "common/network/resolver_impl.h" #include "common/network/utility.h" #include "common/protobuf/utility.h" +#include "common/upstream/load_balancer_impl.h" #include "common/upstream/sds_subscription.h" namespace Envoy { @@ -69,6 +70,7 @@ void EdsClusterImpl::onConfigUpdate(const ResourceVector& resources, const std:: PriorityStateManager priority_state_manager(*this, local_info_); for (const auto& locality_lb_endpoint : cluster_load_assignment.endpoints()) { const uint32_t priority = locality_lb_endpoint.priority(); + if (priority > 0 && !cluster_name_.empty() && cluster_name_ == cm_.localClusterName()) { throw EnvoyException( fmt::format("Unexpected non-zero priority for local cluster '{}'.", cluster_name_)); @@ -85,6 +87,9 @@ void EdsClusterImpl::onConfigUpdate(const ResourceVector& resources, const std:: // Track whether we rebuilt any LB structures. bool cluster_rebuilt = false; + const uint32_t overprovisioning_factor = PROTOBUF_GET_WRAPPED_OR_DEFAULT( + cluster_load_assignment.policy(), overprovisioning_factor, kDefaultOverProvisioningFactor); + // Loop over existing priorities not present in the config. This will empty out any priorities // the config update did not refer to auto& priority_state = priority_state_manager.priorityState(); @@ -93,9 +98,9 @@ void EdsClusterImpl::onConfigUpdate(const ResourceVector& resources, const std:: if (locality_weights_map_.size() <= i) { locality_weights_map_.resize(i + 1); } - cluster_rebuilt |= - updateHostsPerLocality(i, *priority_state[i].first, locality_weights_map_[i], - priority_state[i].second, priority_state_manager, updated_hosts); + cluster_rebuilt |= updateHostsPerLocality( + i, overprovisioning_factor, *priority_state[i].first, locality_weights_map_[i], + priority_state[i].second, priority_state_manager, updated_hosts); } } @@ -109,8 +114,8 @@ void EdsClusterImpl::onConfigUpdate(const ResourceVector& resources, const std:: locality_weights_map_.resize(i + 1); } cluster_rebuilt |= - updateHostsPerLocality(i, empty_hosts, locality_weights_map_[i], empty_locality_map, - priority_state_manager, updated_hosts); + updateHostsPerLocality(i, overprovisioning_factor, empty_hosts, locality_weights_map_[i], + empty_locality_map, priority_state_manager, updated_hosts); } updateHostMap(std::move(updated_hosts)); @@ -125,10 +130,11 @@ void EdsClusterImpl::onConfigUpdate(const ResourceVector& resources, const std:: } bool EdsClusterImpl::updateHostsPerLocality( - const uint32_t priority, const HostVector& new_hosts, LocalityWeightsMap& locality_weights_map, - LocalityWeightsMap& new_locality_weights_map, PriorityStateManager& priority_state_manager, + const uint32_t priority, const uint32_t overprovisioning_factor, const HostVector& new_hosts, + LocalityWeightsMap& locality_weights_map, LocalityWeightsMap& new_locality_weights_map, + PriorityStateManager& priority_state_manager, std::unordered_map& updated_hosts) { - const auto& host_set = priority_set_.getOrCreateHostSet(priority); + const auto& host_set = priority_set_.getOrCreateHostSet(priority, overprovisioning_factor); HostVectorSharedPtr current_hosts_copy(new HostVector(host_set.hosts())); HostVector hosts_added; @@ -141,7 +147,8 @@ bool EdsClusterImpl::updateHostsPerLocality( // out of the locality scheduler, we discover their new weights. We don't currently have a shared // object for locality weights that we can update here, we should add something like this to // improve performance and scalability of locality weight updates. - if (updateDynamicHostList(new_hosts, *current_hosts_copy, hosts_added, hosts_removed, + if (host_set.overprovisioning_factor() != overprovisioning_factor || + updateDynamicHostList(new_hosts, *current_hosts_copy, hosts_added, hosts_removed, updated_hosts) || locality_weights_map != new_locality_weights_map) { locality_weights_map = new_locality_weights_map; @@ -149,8 +156,8 @@ bool EdsClusterImpl::updateHostsPerLocality( info_->name(), host_set.hosts().size(), host_set.priority()); priority_state_manager.updateClusterPrioritySet(priority, std::move(current_hosts_copy), - hosts_added, hosts_removed, absl::nullopt); - + hosts_added, hosts_removed, absl::nullopt, + overprovisioning_factor); return true; } return false; diff --git a/source/common/upstream/eds.h b/source/common/upstream/eds.h index 468854c3c5c2..dbd40ac98f3f 100644 --- a/source/common/upstream/eds.h +++ b/source/common/upstream/eds.h @@ -36,8 +36,8 @@ class EdsClusterImpl : public BaseDynamicClusterImpl, private: using LocalityWeightsMap = std::unordered_map; - bool updateHostsPerLocality(const uint32_t priority, const HostVector& new_hosts, - LocalityWeightsMap& locality_weights_map, + bool updateHostsPerLocality(const uint32_t priority, const uint32_t overprovisioning_factor, + const HostVector& new_hosts, LocalityWeightsMap& locality_weights_map, LocalityWeightsMap& new_locality_weights_map, PriorityStateManager& priority_state_manager, std::unordered_map& updated_hosts); diff --git a/source/common/upstream/health_discovery_service.cc b/source/common/upstream/health_discovery_service.cc index 02bcce89a499..35573b69df4e 100644 --- a/source/common/upstream/health_discovery_service.cc +++ b/source/common/upstream/health_discovery_service.cc @@ -248,7 +248,7 @@ void HdsCluster::initialize(std::function callback) { auto healthy = createHealthyHostList(*initial_hosts_); first_host_set.updateHosts(initial_hosts_, healthy, HostsPerLocalityImpl::empty(), - HostsPerLocalityImpl::empty(), {}, *initial_hosts_, {}); + HostsPerLocalityImpl::empty(), {}, *initial_hosts_, {}, absl::nullopt); } void HdsCluster::setOutlierDetector(const Outlier::DetectorSharedPtr&) { diff --git a/source/common/upstream/load_balancer_impl.cc b/source/common/upstream/load_balancer_impl.cc index ce8ebb94ef6b..98dc98bd13e8 100644 --- a/source/common/upstream/load_balancer_impl.cc +++ b/source/common/upstream/load_balancer_impl.cc @@ -58,13 +58,13 @@ void LoadBalancerBase::recalculatePerPriorityState(uint32_t priority) { // Determine the health of the newly modified priority level. // Health ranges from 0-100, and is the ratio of healthy hosts to total hosts, modified by the - // somewhat arbitrary overprovision factor of kOverProvisioningFactor. - // Eventually the overprovision factor will likely be made configurable. + // overprovisioning factor. HostSet& host_set = *priority_set_.hostSetsPerPriority()[priority]; per_priority_health_[priority] = 0; if (host_set.hosts().size() > 0) { - per_priority_health_[priority] = std::min( - 100, kOverProvisioningFactor * host_set.healthyHosts().size() / host_set.hosts().size()); + per_priority_health_[priority] = + std::min(100, (host_set.overprovisioning_factor() * + host_set.healthyHosts().size() / host_set.hosts().size())); } // Now that we've updated health for the changed priority level, we need to caculate percentage diff --git a/source/common/upstream/load_balancer_impl.h b/source/common/upstream/load_balancer_impl.h index 82946aa091cf..ad52e0e5d55b 100644 --- a/source/common/upstream/load_balancer_impl.h +++ b/source/common/upstream/load_balancer_impl.h @@ -15,10 +15,8 @@ namespace Envoy { namespace Upstream { -// Priority levels and localities are considered overprovisioned with this factor. This means that -// we don't consider a priority level or locality unhealthy until the percentage of healthy hosts -// multiplied by kOverProvisioningFactor drops below 100. -static constexpr uint32_t kOverProvisioningFactor = 140; +// Priority levels and localities are considered overprovisioned with this factor. +static constexpr uint32_t kDefaultOverProvisioningFactor = 140; /** * Base class for all LB implementations. diff --git a/source/common/upstream/original_dst_cluster.cc b/source/common/upstream/original_dst_cluster.cc index e52f11003904..70578143d4fd 100644 --- a/source/common/upstream/original_dst_cluster.cc +++ b/source/common/upstream/original_dst_cluster.cc @@ -150,7 +150,7 @@ void OriginalDstCluster::addHost(HostSharedPtr& host) { new_hosts->emplace_back(host); first_host_set.updateHosts(new_hosts, createHealthyHostList(*new_hosts), HostsPerLocalityImpl::empty(), HostsPerLocalityImpl::empty(), {}, - {std::move(host)}, {}); + {std::move(host)}, {}, absl::nullopt); } void OriginalDstCluster::cleanup() { @@ -175,7 +175,7 @@ void OriginalDstCluster::cleanup() { if (to_be_removed.size() > 0) { host_set.updateHosts(new_hosts, createHealthyHostList(*new_hosts), HostsPerLocalityImpl::empty(), HostsPerLocalityImpl::empty(), {}, {}, - to_be_removed); + to_be_removed, absl::nullopt); } cleanup_timer_->enableTimer(cleanup_interval_ms_); diff --git a/source/common/upstream/ring_hash_lb.cc b/source/common/upstream/ring_hash_lb.cc index e5b5c164e6e8..e7221937a70b 100644 --- a/source/common/upstream/ring_hash_lb.cc +++ b/source/common/upstream/ring_hash_lb.cc @@ -1,6 +1,7 @@ #include "common/upstream/ring_hash_lb.h" #include +#include #include #include @@ -125,9 +126,8 @@ RingHashLoadBalancer::Ring::Ring( std::sort(ring_.begin(), ring_.end(), [](const RingEntry& lhs, const RingEntry& rhs) -> bool { return lhs.hash_ < rhs.hash_; }); - if (ENVOY_LOG_CHECK_LEVEL(trace)) { - for (auto entry : ring_) { + for (const auto& entry : ring_) { ENVOY_LOG(trace, "ring hash: host={} hash={}", entry.host_->address()->asString(), entry.hash_); } diff --git a/source/common/upstream/subset_lb.cc b/source/common/upstream/subset_lb.cc index e15f6250ac3f..4bb3ec37bcec 100644 --- a/source/common/upstream/subset_lb.cc +++ b/source/common/upstream/subset_lb.cc @@ -544,10 +544,16 @@ void SubsetLoadBalancer::HostSubsetImpl::update(const HostVector& hosts_added, } } -HostSetImplPtr SubsetLoadBalancer::PrioritySubsetImpl::createHostSet(uint32_t priority) { +HostSetImplPtr SubsetLoadBalancer::PrioritySubsetImpl::createHostSet( + uint32_t priority, absl::optional overprovisioning_factor) { + // Use original hostset's overprovisioning_factor. RELEASE_ASSERT(priority < original_priority_set_.hostSetsPerPriority().size(), ""); - return HostSetImplPtr{new HostSubsetImpl(*original_priority_set_.hostSetsPerPriority()[priority], - locality_weight_aware_)}; + + const HostSetPtr& host_set = original_priority_set_.hostSetsPerPriority()[priority]; + + ASSERT(!overprovisioning_factor.has_value() || + overprovisioning_factor.value() == host_set->overprovisioning_factor()); + return HostSetImplPtr{new HostSubsetImpl(*host_set, locality_weight_aware_)}; } void SubsetLoadBalancer::PrioritySubsetImpl::update(uint32_t priority, diff --git a/source/common/upstream/subset_lb.h b/source/common/upstream/subset_lb.h index 3eef95806f5a..0f7e5c3d5c26 100644 --- a/source/common/upstream/subset_lb.h +++ b/source/common/upstream/subset_lb.h @@ -39,8 +39,8 @@ class SubsetLoadBalancer : public LoadBalancer, Logger::Loggable overprovisioning_factor) override; private: const PrioritySet& original_priority_set_; diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index cfae8526daff..4446e6ea36c5 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -198,7 +198,12 @@ void HostSetImpl::updateHosts(HostVectorConstSharedPtr hosts, HostsPerLocalityConstSharedPtr hosts_per_locality, HostsPerLocalityConstSharedPtr healthy_hosts_per_locality, LocalityWeightsConstSharedPtr locality_weights, - const HostVector& hosts_added, const HostVector& hosts_removed) { + const HostVector& hosts_added, const HostVector& hosts_removed, + absl::optional overprovisioning_factor) { + if (overprovisioning_factor.has_value()) { + ASSERT(overprovisioning_factor.value() > 0); + overprovisioning_factor_ = overprovisioning_factor.value(); + } hosts_ = std::move(hosts); healthy_hosts_ = std::move(healthy_hosts); hosts_per_locality_ = std::move(hosts_per_locality); @@ -261,17 +266,17 @@ double HostSetImpl::effectiveLocalityWeight(uint32_t index) const { const double locality_healthy_ratio = 1.0 * locality_healthy_hosts.size() / locality_hosts.size(); const uint32_t weight = (*locality_weights_)[index]; // Health ranges from 0-1.0, and is the ratio of healthy hosts to total hosts, modified by the - // somewhat arbitrary overprovision factor of kOverProvisioningFactor. - // Eventually the overprovision factor will likely be made configurable. + // overprovisioning factor. const double effective_locality_health_ratio = - std::min(1.0, (kOverProvisioningFactor / 100.0) * locality_healthy_ratio); + std::min(1.0, (overprovisioning_factor() / 100.0) * locality_healthy_ratio); return weight * effective_locality_health_ratio; } -HostSet& PrioritySetImpl::getOrCreateHostSet(uint32_t priority) { +HostSet& PrioritySetImpl::getOrCreateHostSet(uint32_t priority, + absl::optional overprovisioning_factor) { if (host_sets_.size() < priority + 1) { for (size_t i = host_sets_.size(); i <= priority; ++i) { - HostSetImplPtr host_set = createHostSet(i); + HostSetImplPtr host_set = createHostSet(i, overprovisioning_factor); host_set->addMemberUpdateCb([this](uint32_t priority, const HostVector& hosts_added, const HostVector& hosts_removed) { runUpdateCallbacks(priority, hosts_added, hosts_removed); @@ -628,9 +633,10 @@ void ClusterImplBase::reloadHealthyHosts() { // TODO(htuch): Can we skip these copies by exporting out const shared_ptr from HostSet? HostVectorConstSharedPtr hosts_copy(new HostVector(host_set->hosts())); HostsPerLocalityConstSharedPtr hosts_per_locality_copy = host_set->hostsPerLocality().clone(); - host_set->updateHosts( - hosts_copy, createHealthyHostList(host_set->hosts()), hosts_per_locality_copy, - createHealthyHostLists(host_set->hostsPerLocality()), host_set->localityWeights(), {}, {}); + host_set->updateHosts(hosts_copy, createHealthyHostList(host_set->hosts()), + hosts_per_locality_copy, + createHealthyHostLists(host_set->hostsPerLocality()), + host_set->localityWeights(), {}, {}, absl::nullopt); } } @@ -752,7 +758,8 @@ void PriorityStateManager::registerHostForPriority( void PriorityStateManager::updateClusterPrioritySet( const uint32_t priority, HostVectorSharedPtr&& current_hosts, const absl::optional& hosts_added, const absl::optional& hosts_removed, - const absl::optional health_checker_flag) { + const absl::optional health_checker_flag, + absl::optional overprovisioning_factor) { // If local locality is not defined then skip populating per locality hosts. const auto& local_locality = local_info_node_.locality(); ENVOY_LOG(trace, "Local locality: {}", local_locality.DebugString()); @@ -813,12 +820,12 @@ void PriorityStateManager::updateClusterPrioritySet( auto per_locality_shared = std::make_shared(std::move(per_locality), non_empty_local_locality); - auto& host_set = - static_cast(parent_.prioritySet()).getOrCreateHostSet(priority); + auto& host_set = static_cast(parent_.prioritySet()) + .getOrCreateHostSet(priority, overprovisioning_factor); host_set.updateHosts(hosts, ClusterImplBase::createHealthyHostList(*hosts), per_locality_shared, ClusterImplBase::createHealthyHostLists(*per_locality_shared), std::move(locality_weights), hosts_added.value_or(*hosts), - hosts_removed.value_or({})); + hosts_removed.value_or({}), overprovisioning_factor); } StaticClusterImpl::StaticClusterImpl( diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h index 036e06853cc2..9d34dba7ce53 100644 --- a/source/common/upstream/upstream_impl.h +++ b/source/common/upstream/upstream_impl.h @@ -225,14 +225,18 @@ class HostsPerLocalityImpl : public HostsPerLocality { */ class HostSetImpl : public HostSet { public: - HostSetImpl(uint32_t priority) - : priority_(priority), hosts_(new HostVector()), healthy_hosts_(new HostVector()) {} + HostSetImpl(uint32_t priority, absl::optional overprovisioning_factor) + : priority_(priority), overprovisioning_factor_(overprovisioning_factor.has_value() + ? overprovisioning_factor.value() + : kDefaultOverProvisioningFactor), + hosts_(new HostVector()), healthy_hosts_(new HostVector()) {} void updateHosts(HostVectorConstSharedPtr hosts, HostVectorConstSharedPtr healthy_hosts, HostsPerLocalityConstSharedPtr hosts_per_locality, HostsPerLocalityConstSharedPtr healthy_hosts_per_locality, LocalityWeightsConstSharedPtr locality_weights, const HostVector& hosts_added, - const HostVector& hosts_removed) override; + const HostVector& hosts_removed, + absl::optional overprovisioning_factor = absl::nullopt) override; /** * Install a callback that will be invoked when the host set membership changes. @@ -256,6 +260,7 @@ class HostSetImpl : public HostSet { LocalityWeightsConstSharedPtr localityWeights() const override { return locality_weights_; } absl::optional chooseLocality() override; uint32_t priority() const override { return priority_; } + uint32_t overprovisioning_factor() const override { return overprovisioning_factor_; } protected: virtual void runUpdateCallbacks(const HostVector& hosts_added, const HostVector& hosts_removed) { @@ -267,6 +272,7 @@ class HostSetImpl : public HostSet { double effectiveLocalityWeight(uint32_t index) const; uint32_t priority_; + uint32_t overprovisioning_factor_; HostVectorConstSharedPtr hosts_; HostVectorConstSharedPtr healthy_hosts_; HostsPerLocalityConstSharedPtr hosts_per_locality_{HostsPerLocalityImpl::empty()}; @@ -304,12 +310,14 @@ class PrioritySetImpl : public PrioritySet { } std::vector>& hostSetsPerPriority() override { return host_sets_; } // Get the host set for this priority level, creating it if necessary. - HostSet& getOrCreateHostSet(uint32_t priority); + HostSet& getOrCreateHostSet(uint32_t priority, + absl::optional overprovisioning_factor = absl::nullopt); protected: // Allows subclasses of PrioritySetImpl to create their own type of HostSetImpl. - virtual HostSetImplPtr createHostSet(uint32_t priority) { - return HostSetImplPtr{new HostSetImpl(priority)}; + virtual HostSetImplPtr createHostSet(uint32_t priority, + absl::optional overprovisioning_factor) { + return HostSetImplPtr{new HostSetImpl(priority, overprovisioning_factor)}; } private: @@ -562,7 +570,8 @@ class PriorityStateManager : protected Logger::Loggable { updateClusterPrioritySet(const uint32_t priority, HostVectorSharedPtr&& current_hosts, const absl::optional& hosts_added, const absl::optional& hosts_removed, - const absl::optional health_checker_flag); + const absl::optional health_checker_flag, + absl::optional overprovisioning_factor = absl::nullopt); // Returns the size of the current cluster priority state. size_t size() const { return priority_state_.size(); } diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc index 510e74e423d5..a3700d53a10e 100644 --- a/test/common/upstream/cluster_manager_impl_test.cc +++ b/test/common/upstream/cluster_manager_impl_test.cc @@ -1769,18 +1769,21 @@ TEST_F(ClusterManagerImplTest, MergedUpdates) { // The first update should be applied immediately, since it's not mergeable. hosts_removed.push_back((*hosts)[0]); - cluster.prioritySet().hostSetsPerPriority()[0]->updateHosts( - hosts, hosts, hosts_per_locality, hosts_per_locality, {}, hosts_added, hosts_removed); + cluster.prioritySet().hostSetsPerPriority()[0]->updateHosts(hosts, hosts, hosts_per_locality, + hosts_per_locality, {}, hosts_added, + hosts_removed, absl::nullopt); EXPECT_EQ(1, factory_.stats_.counter("cluster_manager.cluster_updated").value()); EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.cluster_updated_via_merge").value()); EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.update_merge_cancelled").value()); // These calls should be merged, since there are no added/removed hosts. hosts_removed.clear(); - cluster.prioritySet().hostSetsPerPriority()[0]->updateHosts( - hosts, hosts, hosts_per_locality, hosts_per_locality, {}, hosts_added, hosts_removed); - cluster.prioritySet().hostSetsPerPriority()[0]->updateHosts( - hosts, hosts, hosts_per_locality, hosts_per_locality, {}, hosts_added, hosts_removed); + cluster.prioritySet().hostSetsPerPriority()[0]->updateHosts(hosts, hosts, hosts_per_locality, + hosts_per_locality, {}, hosts_added, + hosts_removed, absl::nullopt); + cluster.prioritySet().hostSetsPerPriority()[0]->updateHosts(hosts, hosts, hosts_per_locality, + hosts_per_locality, {}, hosts_added, + hosts_removed, absl::nullopt); EXPECT_EQ(1, factory_.stats_.counter("cluster_manager.cluster_updated").value()); EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.cluster_updated_via_merge").value()); EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.update_merge_cancelled").value()); @@ -1794,8 +1797,9 @@ TEST_F(ClusterManagerImplTest, MergedUpdates) { // Add the host back, the update should be immediately applied. hosts_removed.clear(); hosts_added.push_back((*hosts)[0]); - cluster.prioritySet().hostSetsPerPriority()[0]->updateHosts( - hosts, hosts, hosts_per_locality, hosts_per_locality, {}, hosts_added, hosts_removed); + cluster.prioritySet().hostSetsPerPriority()[0]->updateHosts(hosts, hosts, hosts_per_locality, + hosts_per_locality, {}, hosts_added, + hosts_removed, absl::nullopt); EXPECT_EQ(2, factory_.stats_.counter("cluster_manager.cluster_updated").value()); EXPECT_EQ(1, factory_.stats_.counter("cluster_manager.cluster_updated_via_merge").value()); EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.update_merge_cancelled").value()); @@ -1804,16 +1808,19 @@ TEST_F(ClusterManagerImplTest, MergedUpdates) { hosts_added.clear(); (*hosts)[0]->metadata(buildMetadata("v1")); - cluster.prioritySet().hostSetsPerPriority()[0]->updateHosts( - hosts, hosts, hosts_per_locality, hosts_per_locality, {}, hosts_added, hosts_removed); + cluster.prioritySet().hostSetsPerPriority()[0]->updateHosts(hosts, hosts, hosts_per_locality, + hosts_per_locality, {}, hosts_added, + hosts_removed, absl::nullopt); (*hosts)[0]->healthFlagSet(Host::HealthFlag::FAILED_EDS_HEALTH); - cluster.prioritySet().hostSetsPerPriority()[0]->updateHosts( - hosts, hosts, hosts_per_locality, hosts_per_locality, {}, hosts_added, hosts_removed); + cluster.prioritySet().hostSetsPerPriority()[0]->updateHosts(hosts, hosts, hosts_per_locality, + hosts_per_locality, {}, hosts_added, + hosts_removed, absl::nullopt); (*hosts)[0]->weight(100); - cluster.prioritySet().hostSetsPerPriority()[0]->updateHosts( - hosts, hosts, hosts_per_locality, hosts_per_locality, {}, hosts_added, hosts_removed); + cluster.prioritySet().hostSetsPerPriority()[0]->updateHosts(hosts, hosts, hosts_per_locality, + hosts_per_locality, {}, hosts_added, + hosts_removed, absl::nullopt); // Updates not delivered yet. EXPECT_EQ(2, factory_.stats_.counter("cluster_manager.cluster_updated").value()); @@ -1822,8 +1829,9 @@ TEST_F(ClusterManagerImplTest, MergedUpdates) { // Remove the host again, should cancel the scheduled update and be delivered immediately. hosts_removed.push_back((*hosts)[0]); - cluster.prioritySet().hostSetsPerPriority()[0]->updateHosts( - hosts, hosts, hosts_per_locality, hosts_per_locality, {}, hosts_added, hosts_removed); + cluster.prioritySet().hostSetsPerPriority()[0]->updateHosts(hosts, hosts, hosts_per_locality, + hosts_per_locality, {}, hosts_added, + hosts_removed, absl::nullopt); EXPECT_EQ(3, factory_.stats_.counter("cluster_manager.cluster_updated").value()); EXPECT_EQ(1, factory_.stats_.counter("cluster_manager.cluster_updated_via_merge").value()); @@ -1853,8 +1861,9 @@ TEST_F(ClusterManagerImplTest, MergedUpdatesOutOfWindow) { // The first update should be applied immediately, because even though it's mergeable // it's outside a merge window. - cluster.prioritySet().hostSetsPerPriority()[0]->updateHosts( - hosts, hosts, hosts_per_locality, hosts_per_locality, {}, hosts_added, hosts_removed); + cluster.prioritySet().hostSetsPerPriority()[0]->updateHosts(hosts, hosts, hosts_per_locality, + hosts_per_locality, {}, hosts_added, + hosts_removed, absl::nullopt); EXPECT_EQ(1, factory_.stats_.counter("cluster_manager.cluster_updated").value()); EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.cluster_updated_via_merge").value()); EXPECT_EQ(1, factory_.stats_.counter("cluster_manager.update_out_of_merge_window").value()); @@ -1885,8 +1894,9 @@ TEST_F(ClusterManagerImplTest, MergedUpdatesOutOfWindowDisabled) { // The first update should be applied immediately, because even though it's mergeable // and outside a merge window, merging is disabled. - cluster.prioritySet().hostSetsPerPriority()[0]->updateHosts( - hosts, hosts, hosts_per_locality, hosts_per_locality, {}, hosts_added, hosts_removed); + cluster.prioritySet().hostSetsPerPriority()[0]->updateHosts(hosts, hosts, hosts_per_locality, + hosts_per_locality, {}, hosts_added, + hosts_removed, absl::nullopt); EXPECT_EQ(1, factory_.stats_.counter("cluster_manager.cluster_updated").value()); EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.cluster_updated_via_merge").value()); EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.update_out_of_merge_window").value()); @@ -1942,18 +1952,21 @@ TEST_F(ClusterManagerImplTest, MergedUpdatesDestroyedOnUpdate) { // The first update should be applied immediately, since it's not mergeable. hosts_removed.push_back((*hosts)[0]); - cluster.prioritySet().hostSetsPerPriority()[0]->updateHosts( - hosts, hosts, hosts_per_locality, hosts_per_locality, {}, hosts_added, hosts_removed); + cluster.prioritySet().hostSetsPerPriority()[0]->updateHosts(hosts, hosts, hosts_per_locality, + hosts_per_locality, {}, hosts_added, + hosts_removed, absl::nullopt); EXPECT_EQ(1, factory_.stats_.counter("cluster_manager.cluster_updated").value()); EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.cluster_updated_via_merge").value()); EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.update_merge_cancelled").value()); // These calls should be merged, since there are no added/removed hosts. hosts_removed.clear(); - cluster.prioritySet().hostSetsPerPriority()[0]->updateHosts( - hosts, hosts, hosts_per_locality, hosts_per_locality, {}, hosts_added, hosts_removed); - cluster.prioritySet().hostSetsPerPriority()[0]->updateHosts( - hosts, hosts, hosts_per_locality, hosts_per_locality, {}, hosts_added, hosts_removed); + cluster.prioritySet().hostSetsPerPriority()[0]->updateHosts(hosts, hosts, hosts_per_locality, + hosts_per_locality, {}, hosts_added, + hosts_removed, absl::nullopt); + cluster.prioritySet().hostSetsPerPriority()[0]->updateHosts(hosts, hosts, hosts_per_locality, + hosts_per_locality, {}, hosts_added, + hosts_removed, absl::nullopt); EXPECT_EQ(1, factory_.stats_.counter("cluster_manager.cluster_updated").value()); EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.cluster_updated_via_merge").value()); EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.update_merge_cancelled").value()); diff --git a/test/common/upstream/load_balancer_benchmark.cc b/test/common/upstream/load_balancer_benchmark.cc index 337a3e5dc761..a0d0e148fdb2 100644 --- a/test/common/upstream/load_balancer_benchmark.cc +++ b/test/common/upstream/load_balancer_benchmark.cc @@ -30,7 +30,8 @@ class BaseTester { should_weight ? weight : 1)); } HostVectorConstSharedPtr updated_hosts{new HostVector(hosts)}; - host_set.updateHosts(updated_hosts, updated_hosts, nullptr, nullptr, {}, hosts, {}); + host_set.updateHosts(updated_hosts, updated_hosts, nullptr, nullptr, {}, hosts, {}, + absl::nullopt); } PrioritySetImpl priority_set_; diff --git a/test/common/upstream/load_balancer_impl_test.cc b/test/common/upstream/load_balancer_impl_test.cc index 44b2a919b6d1..cf447c2f7a8b 100644 --- a/test/common/upstream/load_balancer_impl_test.cc +++ b/test/common/upstream/load_balancer_impl_test.cc @@ -115,6 +115,20 @@ TEST_P(LoadBalancerBaseTest, PrioritySelection) { EXPECT_EQ(&tertiary_host_set_, &lb_.chooseHostSet()); } +TEST_P(LoadBalancerBaseTest, OverProvisioningFactor) { + // Default overprovisioning factor 1.4 makes P0 receives 70% load. + updateHostSet(host_set_, 4, 2); + updateHostSet(failover_host_set_, 4, 2); + ASSERT_THAT(getLoadPercentage(), ElementsAre(70, 30)); + + // Set overprovisioning factor to 1, now it should be proportioned to healthy ratio. + host_set_.set_overprovisioning_factor(100); + updateHostSet(host_set_, 4, 2); + failover_host_set_.set_overprovisioning_factor(100); + updateHostSet(failover_host_set_, 4, 2); + ASSERT_THAT(getLoadPercentage(), ElementsAre(50, 50)); +} + TEST_P(LoadBalancerBaseTest, GentleFailover) { // With 100% of P=0 hosts healthy, P=0 gets all the load. updateHostSet(host_set_, 1, 1); @@ -303,8 +317,9 @@ TEST_P(FailoverTest, ExtendPrioritiesWithLocalPrioritySet) { // Update the local hosts. We're not doing locality based routing in this // test, but it should at least do no harm. HostVectorSharedPtr hosts(new HostVector({makeTestHost(info_, "tcp://127.0.0.1:82")})); - local_priority_set_->getOrCreateHostSet(0).updateHosts( - hosts, hosts, empty_locality_, empty_locality_, {}, empty_host_vector_, empty_host_vector_); + local_priority_set_->getOrCreateHostSet(0).updateHosts(hosts, hosts, empty_locality_, + empty_locality_, {}, empty_host_vector_, + empty_host_vector_, absl::nullopt); EXPECT_EQ(tertiary_host_set_.hosts_[0], lb_->chooseHost(nullptr)); } @@ -498,7 +513,7 @@ TEST_P(RoundRobinLoadBalancerTest, ZoneAwareSmallCluster) { common_config_.mutable_zone_aware_lb_config()->mutable_min_cluster_size()->set_value(7); init(true); local_host_set_->updateHosts(hosts, hosts, hosts_per_locality, hosts_per_locality, {}, - empty_host_vector_, empty_host_vector_); + empty_host_vector_, empty_host_vector_, absl::nullopt); EXPECT_CALL(runtime_.snapshot_, getInteger("upstream.healthy_panic_threshold", 0)) .WillRepeatedly(Return(50)); @@ -522,7 +537,7 @@ TEST_P(RoundRobinLoadBalancerTest, ZoneAwareSmallCluster) { .WillRepeatedly(Return(1)); // Trigger reload. local_host_set_->updateHosts(hosts, hosts, hosts_per_locality, hosts_per_locality, {}, - empty_host_vector_, empty_host_vector_); + empty_host_vector_, empty_host_vector_, absl::nullopt); EXPECT_EQ(hostSet().healthy_hosts_per_locality_->get()[0][0], lb_->chooseHost(nullptr)); } @@ -548,7 +563,7 @@ TEST_P(RoundRobinLoadBalancerTest, NoZoneAwareDifferentZoneSize) { common_config_.mutable_zone_aware_lb_config()->mutable_min_cluster_size()->set_value(7); init(true); local_host_set_->updateHosts(hosts, hosts, local_hosts_per_locality, local_hosts_per_locality, {}, - empty_host_vector_, empty_host_vector_); + empty_host_vector_, empty_host_vector_, absl::nullopt); EXPECT_CALL(runtime_.snapshot_, getInteger("upstream.healthy_panic_threshold", 100)) .WillRepeatedly(Return(50)); @@ -585,7 +600,7 @@ TEST_P(RoundRobinLoadBalancerTest, ZoneAwareRoutingLargeZoneSwitchOnOff) { hostSet().healthy_hosts_per_locality_ = hosts_per_locality; init(true); local_host_set_->updateHosts(hosts, hosts, hosts_per_locality, hosts_per_locality, {}, - empty_host_vector_, empty_host_vector_); + empty_host_vector_, empty_host_vector_, absl::nullopt); // There is only one host in the given zone for zone aware routing. EXPECT_EQ(hostSet().healthy_hosts_per_locality_->get()[0][0], lb_->chooseHost(nullptr)); @@ -633,8 +648,8 @@ TEST_P(RoundRobinLoadBalancerTest, ZoneAwareRoutingSmallZone) { hostSet().healthy_hosts_per_locality_ = upstream_hosts_per_locality; init(true); local_host_set_->updateHosts(local_hosts, local_hosts, local_hosts_per_locality, - local_hosts_per_locality, {}, empty_host_vector_, - empty_host_vector_); + local_hosts_per_locality, {}, empty_host_vector_, empty_host_vector_, + absl::nullopt); // There is only one host in the given zone for zone aware routing. EXPECT_CALL(random_, random()).WillOnce(Return(0)).WillOnce(Return(100)); @@ -705,7 +720,7 @@ TEST_P(RoundRobinLoadBalancerTest, LowPrecisionForDistribution) { auto local_hosts_per_locality_shared = makeHostsPerLocality(std::move(local_hosts_per_locality)); local_host_set_->updateHosts(local_hosts, local_hosts, local_hosts_per_locality_shared, local_hosts_per_locality_shared, {}, empty_host_vector_, - empty_host_vector_); + empty_host_vector_, absl::nullopt); // Force request out of small zone and to randomly select zone. EXPECT_CALL(random_, random()).WillOnce(Return(0)).WillOnce(Return(9999)).WillOnce(Return(2)); @@ -726,7 +741,7 @@ TEST_P(RoundRobinLoadBalancerTest, NoZoneAwareRoutingOneZone) { hostSet().healthy_hosts_per_locality_ = hosts_per_locality; init(true); local_host_set_->updateHosts(hosts, hosts, hosts_per_locality, hosts_per_locality, {}, - empty_host_vector_, empty_host_vector_); + empty_host_vector_, empty_host_vector_, absl::nullopt); EXPECT_EQ(hostSet().healthy_hosts_[0], lb_->chooseHost(nullptr)); } @@ -741,7 +756,7 @@ TEST_P(RoundRobinLoadBalancerTest, NoZoneAwareRoutingNotHealthy) { hostSet().healthy_hosts_per_locality_ = hosts_per_locality; init(true); local_host_set_->updateHosts(hosts, hosts, hosts_per_locality, hosts_per_locality, {}, - empty_host_vector_, empty_host_vector_); + empty_host_vector_, empty_host_vector_, absl::nullopt); // local zone has no healthy hosts, take from the all healthy hosts. EXPECT_EQ(hostSet().healthy_hosts_[0], lb_->chooseHost(nullptr)); @@ -773,8 +788,8 @@ TEST_P(RoundRobinLoadBalancerTest, NoZoneAwareRoutingLocalEmpty) { hostSet().healthy_hosts_per_locality_ = upstream_hosts_per_locality; init(true); local_host_set_->updateHosts(local_hosts, local_hosts, local_hosts_per_locality, - local_hosts_per_locality, {}, empty_host_vector_, - empty_host_vector_); + local_hosts_per_locality, {}, empty_host_vector_, empty_host_vector_, + absl::nullopt); // Local cluster is not OK, we'll do regular routing. EXPECT_EQ(hostSet().healthy_hosts_[0], lb_->chooseHost(nullptr)); @@ -802,8 +817,8 @@ TEST_P(RoundRobinLoadBalancerTest, NoZoneAwareRoutingNoLocalLocality) { hostSet().healthy_hosts_per_locality_ = upstream_hosts_per_locality; init(true); local_host_set_->updateHosts(local_hosts, local_hosts, local_hosts_per_locality, - local_hosts_per_locality, {}, empty_host_vector_, - empty_host_vector_); + local_hosts_per_locality, {}, empty_host_vector_, empty_host_vector_, + absl::nullopt); // Local cluster is not OK, we'll do regular routing. EXPECT_EQ(hostSet().healthy_hosts_[0], lb_->chooseHost(nullptr)); diff --git a/test/common/upstream/load_balancer_simulation_test.cc b/test/common/upstream/load_balancer_simulation_test.cc index 568e75ba3fb7..344c72a6578a 100644 --- a/test/common/upstream/load_balancer_simulation_test.cc +++ b/test/common/upstream/load_balancer_simulation_test.cc @@ -54,7 +54,7 @@ TEST(DISABLED_LeastRequestLoadBalancerWeightTest, Weight) { HostVectorConstSharedPtr updated_hosts{new HostVector(hosts)}; HostsPerLocalitySharedPtr updated_locality_hosts{new HostsPerLocalityImpl(hosts)}; host_set.updateHosts(updated_hosts, updated_hosts, updated_locality_hosts, updated_locality_hosts, - {}, hosts, {}); + {}, hosts, {}, absl::nullopt); Stats::IsolatedStoreImpl stats_store; ClusterStats stats{ClusterInfoImpl::generateStats(stats_store)}; @@ -155,7 +155,7 @@ class DISABLED_SimulationTest : public testing::Test { auto per_zone_local_shared = makeHostsPerLocality(std::move(per_zone_local)); local_priority_set_->getOrCreateHostSet(0).updateHosts( originating_hosts, originating_hosts, per_zone_local_shared, per_zone_local_shared, {}, - empty_vector_, empty_vector_); + empty_vector_, empty_vector_, absl::nullopt); HostConstSharedPtr selected = lb.chooseHost(nullptr); hits[selected->address()->asString()]++; diff --git a/test/common/upstream/original_dst_cluster_test.cc b/test/common/upstream/original_dst_cluster_test.cc index 2d9bfa201731..d3816097ea59 100644 --- a/test/common/upstream/original_dst_cluster_test.cc +++ b/test/common/upstream/original_dst_cluster_test.cc @@ -456,7 +456,8 @@ TEST_F(OriginalDstClusterTest, MultipleClusters) { const HostsPerLocalityConstSharedPtr empty_hosts_per_locality{new HostsPerLocalityImpl()}; second.getOrCreateHostSet(0).updateHosts(new_hosts, healthy_hosts, empty_hosts_per_locality, - empty_hosts_per_locality, {}, added, removed); + empty_hosts_per_locality, {}, added, removed, + absl::nullopt); }); EXPECT_CALL(membership_updated_, ready()); diff --git a/test/common/upstream/subset_lb_test.cc b/test/common/upstream/subset_lb_test.cc index 4da2cfbb0811..cdcf31bea3c2 100644 --- a/test/common/upstream/subset_lb_test.cc +++ b/test/common/upstream/subset_lb_test.cc @@ -212,9 +212,9 @@ class SubsetLoadBalancerTest : public testing::TestWithParam { } local_hosts_per_locality_ = makeHostsPerLocality(std::move(local_hosts_per_locality_vector)); - local_priority_set_.getOrCreateHostSet(0).updateHosts(local_hosts_, local_hosts_, - local_hosts_per_locality_, - local_hosts_per_locality_, {}, {}, {}); + local_priority_set_.getOrCreateHostSet(0).updateHosts( + local_hosts_, local_hosts_, local_hosts_per_locality_, local_hosts_per_locality_, {}, {}, + {}, absl::nullopt); lb_.reset(new SubsetLoadBalancer(lb_type_, priority_set_, &local_priority_set_, stats_, runtime_, random_, subset_info_, ring_hash_lb_config_, @@ -310,7 +310,7 @@ class SubsetLoadBalancerTest : public testing::TestWithParam { if (GetParam() == REMOVES_FIRST && !remove.empty()) { local_priority_set_.getOrCreateHostSet(0).updateHosts( local_hosts_, local_hosts_, local_hosts_per_locality_, local_hosts_per_locality_, {}, {}, - remove); + remove, absl::nullopt); } for (const auto& host : add) { @@ -324,12 +324,12 @@ class SubsetLoadBalancerTest : public testing::TestWithParam { if (!add.empty()) { local_priority_set_.getOrCreateHostSet(0).updateHosts( local_hosts_, local_hosts_, local_hosts_per_locality_, local_hosts_per_locality_, {}, - add, {}); + add, {}, absl::nullopt); } } else if (!add.empty() || !remove.empty()) { local_priority_set_.getOrCreateHostSet(0).updateHosts( local_hosts_, local_hosts_, local_hosts_per_locality_, local_hosts_per_locality_, {}, add, - remove); + remove, absl::nullopt); } } diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc index d5f84f9ff3ae..cbf04dbed544 100644 --- a/test/common/upstream/upstream_impl_test.cc +++ b/test/common/upstream/upstream_impl_test.cc @@ -1466,8 +1466,9 @@ TEST(PrioritySet, Extend) { HostVector hosts_added{hosts->front()}; HostVector hosts_removed{}; - priority_set.hostSetsPerPriority()[1]->updateHosts( - hosts, hosts, hosts_per_locality, hosts_per_locality, {}, hosts_added, hosts_removed); + priority_set.hostSetsPerPriority()[1]->updateHosts(hosts, hosts, hosts_per_locality, + hosts_per_locality, {}, hosts_added, + hosts_removed, absl::nullopt); EXPECT_EQ(1, changes); EXPECT_EQ(last_priority, 1); EXPECT_EQ(1, priority_set.hostSetsPerPriority()[1]->hosts().size()); @@ -1701,7 +1702,7 @@ TEST(HostsPerLocalityImpl, Filter) { class HostSetImplLocalityTest : public ::testing::Test { public: LocalityWeightsConstSharedPtr locality_weights_; - HostSetImpl host_set_{0}; + HostSetImpl host_set_{0, kDefaultOverProvisioningFactor}; std::shared_ptr info_{new NiceMock()}; HostVector hosts_{ makeTestHost(info_, "tcp://127.0.0.1:80"), makeTestHost(info_, "tcp://127.0.0.1:81"), @@ -1722,7 +1723,7 @@ TEST_F(HostSetImplLocalityTest, AllUnhealthy) { LocalityWeightsConstSharedPtr locality_weights{new LocalityWeights{1, 1, 1}}; auto hosts = makeHostsFromHostsPerLocality(hosts_per_locality); host_set_.updateHosts(hosts, std::make_shared(), hosts_per_locality, - hosts_per_locality, locality_weights, {}, {}); + hosts_per_locality, locality_weights, {}, {}, absl::nullopt); EXPECT_FALSE(host_set_.chooseLocality().has_value()); } @@ -1743,7 +1744,7 @@ TEST_F(HostSetImplLocalityTest, Unweighted) { LocalityWeightsConstSharedPtr locality_weights{new LocalityWeights{1, 1, 1}}; auto hosts = makeHostsFromHostsPerLocality(hosts_per_locality); host_set_.updateHosts(hosts, hosts, hosts_per_locality, hosts_per_locality, locality_weights, {}, - {}); + {}, absl::nullopt); EXPECT_EQ(0, host_set_.chooseLocality().value()); EXPECT_EQ(1, host_set_.chooseLocality().value()); EXPECT_EQ(2, host_set_.chooseLocality().value()); @@ -1758,7 +1759,7 @@ TEST_F(HostSetImplLocalityTest, Weighted) { LocalityWeightsConstSharedPtr locality_weights{new LocalityWeights{1, 2}}; auto hosts = makeHostsFromHostsPerLocality(hosts_per_locality); host_set_.updateHosts(hosts, hosts, hosts_per_locality, hosts_per_locality, locality_weights, {}, - {}); + {}, absl::nullopt); EXPECT_EQ(1, host_set_.chooseLocality().value()); EXPECT_EQ(0, host_set_.chooseLocality().value()); EXPECT_EQ(1, host_set_.chooseLocality().value()); @@ -1774,7 +1775,7 @@ TEST_F(HostSetImplLocalityTest, MissingWeight) { LocalityWeightsConstSharedPtr locality_weights{new LocalityWeights{1, 0, 1}}; auto hosts = makeHostsFromHostsPerLocality(hosts_per_locality); host_set_.updateHosts(hosts, hosts, hosts_per_locality, hosts_per_locality, locality_weights, {}, - {}); + {}, absl::nullopt); EXPECT_EQ(0, host_set_.chooseLocality().value()); EXPECT_EQ(2, host_set_.chooseLocality().value()); EXPECT_EQ(0, host_set_.chooseLocality().value()); @@ -1799,7 +1800,8 @@ TEST_F(HostSetImplLocalityTest, UnhealthyFailover) { auto hosts = makeHostsFromHostsPerLocality(hosts_per_locality); host_set_.updateHosts(makeHostsFromHostsPerLocality(hosts_per_locality), makeHostsFromHostsPerLocality(healthy_hosts_per_locality), - hosts_per_locality, healthy_hosts_per_locality, locality_weights, {}, {}); + hosts_per_locality, healthy_hosts_per_locality, locality_weights, {}, {}, + absl::nullopt); }; const auto expectPicks = [this](uint32_t locality_0_picks, uint32_t locality_1_picks) { @@ -1828,6 +1830,47 @@ TEST_F(HostSetImplLocalityTest, UnhealthyFailover) { expectPicks(0, 100); } +TEST(OverProvisioningFactorTest, LocalityPickChanges) { + auto setUpHostSetWithOPFAndTestPicks = [](const uint32_t overprovisioning_factor, + const uint32_t pick_0, const uint32_t pick_1) { + HostSetImpl host_set(0, overprovisioning_factor); + std::shared_ptr cluster_info{new NiceMock()}; + HostVector hosts{makeTestHost(cluster_info, "tcp://127.0.0.1:80"), + makeTestHost(cluster_info, "tcp://127.0.0.1:81"), + makeTestHost(cluster_info, "tcp://127.0.0.1:82")}; + LocalityWeightsConstSharedPtr locality_weights{new LocalityWeights{1, 1}}; + HostsPerLocalitySharedPtr hosts_per_locality = + makeHostsPerLocality({{hosts[0], hosts[1]}, {hosts[2]}}); + // Healthy ratio: (1/2, 1). + HostsPerLocalitySharedPtr healthy_hosts_per_locality = + makeHostsPerLocality({{hosts[0]}, {hosts[2]}}); + host_set.updateHosts(makeHostsFromHostsPerLocality(hosts_per_locality), + makeHostsFromHostsPerLocality(healthy_hosts_per_locality), + hosts_per_locality, healthy_hosts_per_locality, locality_weights, {}, {}, + absl::nullopt); + uint32_t cnts[] = {0, 0}; + for (uint32_t i = 0; i < 100; ++i) { + absl::optional locality_index = host_set.chooseLocality(); + if (!locality_index.has_value()) { + // It's possible locality scheduler is nullptr (when factor is 0). + continue; + } + ASSERT_LT(locality_index.value(), 2); + ++cnts[locality_index.value()]; + } + EXPECT_EQ(pick_0, cnts[0]); + EXPECT_EQ(pick_1, cnts[1]); + }; + + // NOTE: effective locality weight: weight * min(1, factor * healthy-ratio). + + // Picks in localities match to weight(1) * healthy-ratio when + // overprovisioning factor is 1. + setUpHostSetWithOPFAndTestPicks(100, 33, 67); + // Picks in localities match to weights as factor * healthy-ratio > 1. + setUpHostSetWithOPFAndTestPicks(200, 50, 50); +}; + } // namespace } // namespace Upstream } // namespace Envoy diff --git a/test/integration/BUILD b/test/integration/BUILD index 002c019fbb01..25534d61eec2 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -68,6 +68,7 @@ envoy_cc_test( srcs = ["eds_integration_test.cc"], deps = [ ":http_integration_lib", + "//source/common/upstream:load_balancer_lib", "//test/config:utility_lib", "//test/test_common:network_utility_lib", "@envoy_api//envoy/api/v2:eds_cc", diff --git a/test/integration/eds_integration_test.cc b/test/integration/eds_integration_test.cc index 0e63806f3edb..ca4af097bb76 100644 --- a/test/integration/eds_integration_test.cc +++ b/test/integration/eds_integration_test.cc @@ -1,5 +1,7 @@ #include "envoy/api/v2/eds.pb.h" +#include "common/upstream/load_balancer_impl.h" + #include "test/config/utility.h" #include "test/integration/http_integration.h" #include "test/test_common/network_utility.h" @@ -18,9 +20,14 @@ class EdsIntegrationTest : public HttpIntegrationTest, // We need to supply the endpoints via EDS to provide health status. Use a // filesystem delivery to simplify test mechanics. - void setEndpoints(uint32_t total_endpoints, uint32_t healthy_endpoints) { + void setEndpoints(uint32_t total_endpoints, uint32_t healthy_endpoints, + absl::optional overprovisioning_factor = absl::nullopt) { envoy::api::v2::ClusterLoadAssignment cluster_load_assignment; cluster_load_assignment.set_cluster_name("cluster_0"); + if (overprovisioning_factor.has_value()) { + cluster_load_assignment.mutable_policy()->mutable_overprovisioning_factor()->set_value( + overprovisioning_factor.value()); + } auto* locality_lb_endpoints = cluster_load_assignment.add_endpoints(); for (uint32_t i = 0; i < total_endpoints; ++i) { @@ -34,7 +41,7 @@ class EdsIntegrationTest : public HttpIntegrationTest, } void initialize() override { - setUpstreamCount(3); + setUpstreamCount(4); config_helper_.addConfigModifier([this](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { // Switch predefined cluster_0 to EDS filesystem sourcing. auto* cluster_0 = bootstrap.mutable_static_resources()->mutable_clusters(0); @@ -82,5 +89,27 @@ TEST_P(EdsIntegrationTest, HealthUpdate) { EXPECT_EQ(2, test_server_->gauge("cluster.cluster_0.membership_healthy")->value()); } +// Validate that overprovisioning_factor update are picked up by Envoy. +TEST_P(EdsIntegrationTest, OverprovisioningFactorUpdate) { + initialize(); + // Default overprovisioning factor. + setEndpoints(4, 4); + auto get_and_compare = [this](const uint32_t expected_factor) { + const auto& cluster_map = test_server_->server().clusterManager().clusters(); + EXPECT_EQ(1, cluster_map.size()); + EXPECT_EQ(1, cluster_map.count("cluster_0")); + const auto& cluster_ref = cluster_map.find("cluster_0")->second; + const auto& hostset_per_priority = cluster_ref.get().prioritySet().hostSetsPerPriority(); + EXPECT_EQ(1, hostset_per_priority.size()); + const Envoy::Upstream::HostSetPtr& host_set = hostset_per_priority[0]; + EXPECT_EQ(expected_factor, host_set->overprovisioning_factor()); + }; + get_and_compare(Envoy::Upstream::kDefaultOverProvisioningFactor); + + // Use new overprovisioning factor 200. + setEndpoints(4, 4, 200); + get_and_compare(200); +} + } // namespace } // namespace Envoy diff --git a/test/mocks/upstream/mocks.cc b/test/mocks/upstream/mocks.cc index f4f1bbc1a8e2..b487c0b16fc1 100644 --- a/test/mocks/upstream/mocks.cc +++ b/test/mocks/upstream/mocks.cc @@ -18,7 +18,8 @@ using testing::_; namespace Envoy { namespace Upstream { -MockHostSet::MockHostSet(uint32_t priority) : priority_(priority) { +MockHostSet::MockHostSet(uint32_t priority, uint32_t overprovisioning_factor) + : priority_(priority), overprovisioning_factor_(overprovisioning_factor) { ON_CALL(*this, priority()).WillByDefault(Return(priority_)); ON_CALL(*this, hosts()).WillByDefault(ReturnRef(hosts_)); ON_CALL(*this, healthyHosts()).WillByDefault(ReturnRef(healthy_hosts_)); diff --git a/test/mocks/upstream/mocks.h b/test/mocks/upstream/mocks.h index 5ad05945a582..a19de4087863 100644 --- a/test/mocks/upstream/mocks.h +++ b/test/mocks/upstream/mocks.h @@ -13,6 +13,7 @@ #include "common/common/callback_impl.h" #include "common/upstream/health_discovery_service.h" +#include "common/upstream/load_balancer_impl.h" #include "common/upstream/upstream_impl.h" #include "test/mocks/config/mocks.h" @@ -34,7 +35,8 @@ namespace Upstream { class MockHostSet : public HostSet { public: - MockHostSet(uint32_t priority = 0); + MockHostSet(uint32_t priority = 0, + uint32_t overprovisioning_factor = kDefaultOverProvisioningFactor); void runCallbacks(const HostVector added, const HostVector removed) { member_update_cb_helper_.runCallbacks(priority(), added, removed); @@ -51,13 +53,18 @@ class MockHostSet : public HostSet { MOCK_CONST_METHOD0(healthyHostsPerLocality, const HostsPerLocality&()); MOCK_CONST_METHOD0(localityWeights, LocalityWeightsConstSharedPtr()); MOCK_METHOD0(chooseLocality, absl::optional()); - MOCK_METHOD7(updateHosts, void(std::shared_ptr hosts, + MOCK_METHOD8(updateHosts, void(std::shared_ptr hosts, std::shared_ptr healthy_hosts, HostsPerLocalityConstSharedPtr hosts_per_locality, HostsPerLocalityConstSharedPtr healthy_hosts_per_locality, LocalityWeightsConstSharedPtr locality_weights, - const HostVector& hosts_added, const HostVector& hosts_removed)); + const HostVector& hosts_added, const HostVector& hosts_removed, + absl::optional overprovisioning_factor)); MOCK_CONST_METHOD0(priority, uint32_t()); + uint32_t overprovisioning_factor() const override { return overprovisioning_factor_; } + void set_overprovisioning_factor(const uint32_t overprovisioning_factor) { + overprovisioning_factor_ = overprovisioning_factor; + } HostVector hosts_; HostVector healthy_hosts_; @@ -66,6 +73,7 @@ class MockHostSet : public HostSet { LocalityWeightsConstSharedPtr locality_weights_{{}}; Common::CallbackManager member_update_cb_helper_; uint32_t priority_{}; + uint32_t overprovisioning_factor_{}; }; class MockPrioritySet : public PrioritySet {