From a1b52fd1034b8120eaad1649ece6129ce8a84722 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Kleiman?= Date: Mon, 18 May 2020 17:23:57 -0700 Subject: [PATCH 01/19] Add support for making Least Requests LB behave like Round Robin in weighted hosts case MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Gastón Kleiman --- source/common/runtime/runtime_features.cc | 1 + source/common/upstream/BUILD | 1 + source/common/upstream/load_balancer_impl.h | 22 +++++++++++++++- test/common/upstream/BUILD | 1 + .../upstream/load_balancer_impl_test.cc | 25 +++++++++++++++++++ 5 files changed, 49 insertions(+), 1 deletion(-) diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 661e58d920ac4..4d632b7b7b807 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -81,6 +81,7 @@ constexpr const char* runtime_features[] = { constexpr const char* disabled_runtime_features[] = { // Sentinel and test flag. "envoy.reloadable_features.test_feature_false", + "envoy.reloadable_features.alternative_least_request_weights", }; RuntimeFeatures::RuntimeFeatures() { diff --git a/source/common/upstream/BUILD b/source/common/upstream/BUILD index f0634fd33bc4b..152257121bc04 100644 --- a/source/common/upstream/BUILD +++ b/source/common/upstream/BUILD @@ -181,6 +181,7 @@ envoy_cc_library( "//include/envoy/upstream:upstream_interface", "//source/common/common:assert_lib", "//source/common/protobuf:utility_lib", + "//source/common/runtime:runtime_features_lib", "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto", ], ) diff --git a/source/common/upstream/load_balancer_impl.h b/source/common/upstream/load_balancer_impl.h index 9ef6f9fd8fac8..796dc1dd56fa3 100644 --- a/source/common/upstream/load_balancer_impl.h +++ b/source/common/upstream/load_balancer_impl.h @@ -11,6 +11,7 @@ #include "envoy/upstream/upstream.h" #include "common/protobuf/utility.h" +#include "common/runtime/runtime_features.h" #include "common/upstream/edf_scheduler.h" namespace Envoy { @@ -367,6 +368,8 @@ class EdfLoadBalancerBase : public ZoneAwareLoadBalancerBase { void initialize(); + virtual void refresh(uint32_t priority); + // Seed to allow us to desynchronize load balancers across a fleet. If we don't // do this, multiple Envoys that receive an update at the same time (or even // multiple load balancers on the same host) will send requests to @@ -375,7 +378,6 @@ class EdfLoadBalancerBase : public ZoneAwareLoadBalancerBase { const uint64_t seed_; private: - void refresh(uint32_t priority); virtual void refreshHostSource(const HostsSource& source) PURE; virtual double hostWeight(const Host& host) PURE; virtual HostConstSharedPtr unweightedHostPick(const HostVector& hosts_to_use, @@ -454,9 +456,25 @@ class LeastRequestLoadBalancer : public EdfLoadBalancerBase { initialize(); } + HostConstSharedPtr chooseHost(LoadBalancerContext* context) override { + alternativeWeights_ = Runtime::runtimeFeatureEnabled( + "envoy.reloadable_features.alternative_least_request_weights"); + return EdfLoadBalancerBase::chooseHost(context); + } + + void refresh(uint32_t priority) override { + alternativeWeights_ = Runtime::runtimeFeatureEnabled( + "envoy.reloadable_features.alternative_least_request_weights"); + EdfLoadBalancerBase::refresh(priority); + } + private: void refreshHostSource(const HostsSource&) override {} double hostWeight(const Host& host) override { + if (alternativeWeights_) { + return host.weight(); + } + // Here we scale host weight by the number of active requests at the time we do the pick. We // always add 1 to avoid division by 0. It might be possible to do better by picking two hosts // off of the schedule, and selecting the one with fewer active requests at the time of @@ -469,7 +487,9 @@ class LeastRequestLoadBalancer : public EdfLoadBalancerBase { } HostConstSharedPtr unweightedHostPick(const HostVector& hosts_to_use, const HostsSource& source) override; + const uint32_t choice_count_; + bool alternativeWeights_; }; /** diff --git a/test/common/upstream/BUILD b/test/common/upstream/BUILD index 19f713f672a2c..f4eda66e16c1b 100644 --- a/test/common/upstream/BUILD +++ b/test/common/upstream/BUILD @@ -189,6 +189,7 @@ envoy_cc_test( "//source/common/upstream:upstream_lib", "//test/mocks/runtime:runtime_mocks", "//test/mocks/upstream:upstream_mocks", + "//test/test_common:test_runtime_lib", "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto", ], ) diff --git a/test/common/upstream/load_balancer_impl_test.cc b/test/common/upstream/load_balancer_impl_test.cc index ced9ec06ae29b..e864117ec5f12 100644 --- a/test/common/upstream/load_balancer_impl_test.cc +++ b/test/common/upstream/load_balancer_impl_test.cc @@ -13,6 +13,7 @@ #include "test/common/upstream/utility.h" #include "test/mocks/runtime/mocks.h" #include "test/mocks/upstream/mocks.h" +#include "test/test_common/test_runtime.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -1532,6 +1533,30 @@ TEST_P(LeastRequestLoadBalancerTest, WeightImbalance) { EXPECT_EQ(hostSet().healthy_hosts_[0], lb_.chooseHost(nullptr)); } +TEST_P(LeastRequestLoadBalancerTest, WeightImbalanceWithAlternativeWeights) { + auto scoped_runtime = std::make_unique(); + Runtime::LoaderSingleton::getExisting()->mergeValues( + {{"envoy.reloadable_features.alternative_least_request_weights", "true"}}); + + hostSet().healthy_hosts_ = {makeTestHost(info_, "tcp://127.0.0.1:80", 1), + makeTestHost(info_, "tcp://127.0.0.1:81", 2)}; + stats_.max_host_weight_.set(2UL); + + hostSet().hosts_ = hostSet().healthy_hosts_; + hostSet().runCallbacks({}, {}); // Trigger callbacks. The added/removed lists are not relevant. + + EXPECT_CALL(random_, random()).WillRepeatedly(Return(0)); + + // We should see 2:1 ratio for hosts[1] to hosts[0], regardless of the active request count. + hostSet().healthy_hosts_[1]->stats().rq_active_.set(1); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[0], lb_.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[0], lb_.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_.chooseHost(nullptr)); +} + TEST_P(LeastRequestLoadBalancerTest, WeightImbalanceCallbacks) { hostSet().healthy_hosts_ = {makeTestHost(info_, "tcp://127.0.0.1:80", 1), makeTestHost(info_, "tcp://127.0.0.1:81", 2)}; From 762e61b726de4f770ac0346f2a5ae43ff4934f60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Kleiman?= Date: Tue, 19 May 2020 14:20:30 -0700 Subject: [PATCH 02/19] Address feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Gastón Kleiman --- source/common/runtime/runtime_features.cc | 1 - source/common/upstream/load_balancer_impl.h | 42 +++++++++++-------- .../upstream/load_balancer_impl_test.cc | 8 ++-- 3 files changed, 29 insertions(+), 22 deletions(-) diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 4d632b7b7b807..661e58d920ac4 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -81,7 +81,6 @@ constexpr const char* runtime_features[] = { constexpr const char* disabled_runtime_features[] = { // Sentinel and test flag. "envoy.reloadable_features.test_feature_false", - "envoy.reloadable_features.alternative_least_request_weights", }; RuntimeFeatures::RuntimeFeatures() { diff --git a/source/common/upstream/load_balancer_impl.h b/source/common/upstream/load_balancer_impl.h index 796dc1dd56fa3..29578b8cbb4f1 100644 --- a/source/common/upstream/load_balancer_impl.h +++ b/source/common/upstream/load_balancer_impl.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -11,12 +12,14 @@ #include "envoy/upstream/upstream.h" #include "common/protobuf/utility.h" -#include "common/runtime/runtime_features.h" #include "common/upstream/edf_scheduler.h" namespace Envoy { namespace Upstream { +static const std::string RuntimeLeastRequestsActiveRequestsExponent = + "upstream.least_requests.active_requests_exponent"; + // Priority levels and localities are considered overprovisioned with this factor. static constexpr uint32_t kDefaultOverProvisioningFactor = 140; @@ -366,7 +369,7 @@ class EdfLoadBalancerBase : public ZoneAwareLoadBalancerBase { std::unique_ptr> edf_; }; - void initialize(); + virtual void initialize(); virtual void refresh(uint32_t priority); @@ -439,7 +442,8 @@ class RoundRobinLoadBalancer : public EdfLoadBalancerBase { * The benefit of the Maglev table is at the expense of resolution, memory usage is capped. * Additionally, the Maglev table can be shared amongst all threads. */ -class LeastRequestLoadBalancer : public EdfLoadBalancerBase { +class LeastRequestLoadBalancer : public EdfLoadBalancerBase, + Logger::Loggable { public: LeastRequestLoadBalancer( const PrioritySet& priority_set, const PrioritySet* local_priority_set, ClusterStats& stats, @@ -456,25 +460,16 @@ class LeastRequestLoadBalancer : public EdfLoadBalancerBase { initialize(); } - HostConstSharedPtr chooseHost(LoadBalancerContext* context) override { - alternativeWeights_ = Runtime::runtimeFeatureEnabled( - "envoy.reloadable_features.alternative_least_request_weights"); - return EdfLoadBalancerBase::chooseHost(context); - } - +protected: void refresh(uint32_t priority) override { - alternativeWeights_ = Runtime::runtimeFeatureEnabled( - "envoy.reloadable_features.alternative_least_request_weights"); + active_requests_exponent_ = + runtime_.snapshot().getDouble(RuntimeLeastRequestsActiveRequestsExponent, 1.0); EdfLoadBalancerBase::refresh(priority); } private: void refreshHostSource(const HostsSource&) override {} double hostWeight(const Host& host) override { - if (alternativeWeights_) { - return host.weight(); - } - // Here we scale host weight by the number of active requests at the time we do the pick. We // always add 1 to avoid division by 0. It might be possible to do better by picking two hosts // off of the schedule, and selecting the one with fewer active requests at the time of @@ -483,13 +478,26 @@ class LeastRequestLoadBalancer : public EdfLoadBalancerBase { // be the only/best way of doing this. Essentially, it makes weight and active requests equally // important. Are they equally important in practice? There is no right answer here and we might // want to iterate on this as we gain more experience. - return static_cast(host.weight()) / (host.stats().rq_active_.value() + 1); + const double weight = static_cast(host.weight()) / + std::pow(host.stats().rq_active_.value() + 1, active_requests_exponent_); + + ENVOY_LOG(debug, "cluster={} address={} active_requests_exponent={} weight={}", + host.cluster().name(), host.address()->asString(), active_requests_exponent_, weight); + + return weight; } HostConstSharedPtr unweightedHostPick(const HostVector& hosts_to_use, const HostsSource& source) override; const uint32_t choice_count_; - bool alternativeWeights_; + + // When hosts have different weights, the host weight is calculated as: + // + // host_weight = (configured_weight / active_requests^k). k is configured via runtime and its + // value is cached to avoid having to do a runtime lookup each time a host weight is generated. + // + // The cached value is refreshed in `LeastRequestLoadBalancer::refresh(uint32_t priority)`. + double active_requests_exponent_; }; /** diff --git a/test/common/upstream/load_balancer_impl_test.cc b/test/common/upstream/load_balancer_impl_test.cc index e864117ec5f12..2ebb217d4272a 100644 --- a/test/common/upstream/load_balancer_impl_test.cc +++ b/test/common/upstream/load_balancer_impl_test.cc @@ -1533,10 +1533,10 @@ TEST_P(LeastRequestLoadBalancerTest, WeightImbalance) { EXPECT_EQ(hostSet().healthy_hosts_[0], lb_.chooseHost(nullptr)); } -TEST_P(LeastRequestLoadBalancerTest, WeightImbalanceWithAlternativeWeights) { - auto scoped_runtime = std::make_unique(); - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.alternative_least_request_weights", "true"}}); +TEST_P(LeastRequestLoadBalancerTest, WeightImbalanceWithCustomExponent) { + EXPECT_CALL(runtime_.snapshot_, + getDouble("upstream.least_requests.active_requests_exponent", 1.0)) + .WillRepeatedly(Return(0.0)); hostSet().healthy_hosts_ = {makeTestHost(info_, "tcp://127.0.0.1:80", 1), makeTestHost(info_, "tcp://127.0.0.1:81", 2)}; From 4cf5f196d6ac02ac7ab7f57d31c1f03f340febdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Kleiman?= Date: Wed, 20 May 2020 11:21:10 -0700 Subject: [PATCH 03/19] Perf/logging improvements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Gastón Kleiman --- source/common/upstream/load_balancer_impl.h | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/source/common/upstream/load_balancer_impl.h b/source/common/upstream/load_balancer_impl.h index 29578b8cbb4f1..2c3b993fbee41 100644 --- a/source/common/upstream/load_balancer_impl.h +++ b/source/common/upstream/load_balancer_impl.h @@ -478,10 +478,21 @@ class LeastRequestLoadBalancer : public EdfLoadBalancerBase, // be the only/best way of doing this. Essentially, it makes weight and active requests equally // important. Are they equally important in practice? There is no right answer here and we might // want to iterate on this as we gain more experience. - const double weight = static_cast(host.weight()) / - std::pow(host.stats().rq_active_.value() + 1, active_requests_exponent_); - ENVOY_LOG(debug, "cluster={} address={} active_requests_exponent={} weight={}", + // TODO(gkleiman): the final PR will not log anything and will return directly instead of using + // a temporary variable. + double weight; + + if (active_requests_exponent_ == 0.0) { + weight = host.weight(); + } else if (active_requests_exponent_ == 1.0) { + weight = static_cast(host.weight()) / (host.stats().rq_active_.value() + 1); + } else { + weight = static_cast(host.weight()) / + std::pow(host.stats().rq_active_.value() + 1, active_requests_exponent_); + } + + ENVOY_LOG(trace, "cluster={} address={} active_requests_exponent={} weight={}", host.cluster().name(), host.address()->asString(), active_requests_exponent_, weight); return weight; From 0a2b1fcdad5daeef1136ef7c2208f356d3fcf05f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Kleiman?= Date: Wed, 20 May 2020 16:28:54 -0700 Subject: [PATCH 04/19] Address feedback and cleanup BUILD file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Gastón Kleiman --- source/common/upstream/BUILD | 1 - source/common/upstream/load_balancer_impl.h | 4 +++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/source/common/upstream/BUILD b/source/common/upstream/BUILD index 152257121bc04..f0634fd33bc4b 100644 --- a/source/common/upstream/BUILD +++ b/source/common/upstream/BUILD @@ -181,7 +181,6 @@ envoy_cc_library( "//include/envoy/upstream:upstream_interface", "//source/common/common:assert_lib", "//source/common/protobuf:utility_lib", - "//source/common/runtime:runtime_features_lib", "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto", ], ) diff --git a/source/common/upstream/load_balancer_impl.h b/source/common/upstream/load_balancer_impl.h index 2c3b993fbee41..3c764b7312e23 100644 --- a/source/common/upstream/load_balancer_impl.h +++ b/source/common/upstream/load_balancer_impl.h @@ -14,10 +14,12 @@ #include "common/protobuf/utility.h" #include "common/upstream/edf_scheduler.h" +#include "absl/strings/string_view.h" + namespace Envoy { namespace Upstream { -static const std::string RuntimeLeastRequestsActiveRequestsExponent = +constexpr absl::string_view RuntimeLeastRequestsActiveRequestsExponent = "upstream.least_requests.active_requests_exponent"; // Priority levels and localities are considered overprovisioned with this factor. From 6864b502f5c8dc3cf03854b23f3ad87704787f76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Kleiman?= Date: Thu, 11 Jun 2020 11:33:39 -0700 Subject: [PATCH 05/19] Make active requests exponent configurable via CDS/runtime MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Gastón Kleiman --- api/envoy/config/cluster/v3/cluster.proto | 17 ++++ .../config/cluster/v4alpha/cluster.proto | 17 ++++ .../load_balancing/load_balancers.rst | 21 +++-- .../envoy/config/cluster/v3/cluster.proto | 17 ++++ .../config/cluster/v4alpha/cluster.proto | 17 ++++ source/common/upstream/BUILD | 1 + source/common/upstream/load_balancer_impl.h | 80 ++++++++++--------- .../filters/http/ext_authz/ext_authz.cc | 2 +- .../upstream/load_balancer_impl_test.cc | 23 +++--- 9 files changed, 141 insertions(+), 54 deletions(-) diff --git a/api/envoy/config/cluster/v3/cluster.proto b/api/envoy/config/cluster/v3/cluster.proto index 7eb53d84c4f81..cc8eaf3c33cbe 100644 --- a/api/envoy/config/cluster/v3/cluster.proto +++ b/api/envoy/config/cluster/v3/cluster.proto @@ -318,6 +318,23 @@ message Cluster { // The number of random healthy hosts from which the host with the fewest active requests will // be chosen. Defaults to 2 so that we perform two-choice selection if the field is not set. google.protobuf.UInt32Value choice_count = 1 [(validate.rules).uint32 = {gte: 2}]; + + // The following formula is used to calculate the dynamic weights when hosts have different + // load balancing weights: + // + // `weight = load_balancing_weight / (active_requests + 1)^active_requests_exponent` + // + // When `active_requests_exponent == 0.0` the Least Request Load Balancer doesn't consider the + // number of active requests at the time it picks a host and behaves like the Round Robin Load + // Balancer. + // + // When `active_requests_exponent > 0.0` the Least Request Load Balancer scales the load + // balancing weight by the number of active requests at the time it does a pick. + // + // The exponent is cached for performance reasons and its value is only refreshed whenever + // one of the Load Balancer's host sets changes, e.g., whenever there is a host membership + // update or a host load balancing weight change. + core.v3.RuntimeDouble active_requests_exponent = 2; } // Specific configuration for the :ref:`RingHash` diff --git a/api/envoy/config/cluster/v4alpha/cluster.proto b/api/envoy/config/cluster/v4alpha/cluster.proto index eab2f2d80fcb7..5cab7d1b7dc15 100644 --- a/api/envoy/config/cluster/v4alpha/cluster.proto +++ b/api/envoy/config/cluster/v4alpha/cluster.proto @@ -318,6 +318,23 @@ message Cluster { // The number of random healthy hosts from which the host with the fewest active requests will // be chosen. Defaults to 2 so that we perform two-choice selection if the field is not set. google.protobuf.UInt32Value choice_count = 1 [(validate.rules).uint32 = {gte: 2}]; + + // The following formula is used to calculate the dynamic weights when hosts have different + // load balancing weights: + // + // `weight = load_balancing_weight / (active_requests + 1)^active_requests_exponent` + // + // When `active_requests_exponent == 0.0` the Least Request Load Balancer doesn't consider the + // number of active requests at the time it picks a host and behaves like the Round Robin Load + // Balancer. + // + // When `active_requests_exponent > 0.0` the Least Request Load Balancer scales the load + // balancing weight by the number of active requests at the time it does a pick. + // + // The exponent is cached for performance reasons and its value is only refreshed whenever + // one of the Load Balancer's host sets changes, e.g., whenever there is a host membership + // update or a host load balancing weight change. + core.v4alpha.RuntimeDouble active_requests_exponent = 2; } // Specific configuration for the :ref:`RingHash` diff --git a/docs/root/intro/arch_overview/upstream/load_balancing/load_balancers.rst b/docs/root/intro/arch_overview/upstream/load_balancing/load_balancers.rst index 5336dccb14c3b..beae5ddc813d0 100644 --- a/docs/root/intro/arch_overview/upstream/load_balancing/load_balancers.rst +++ b/docs/root/intro/arch_overview/upstream/load_balancing/load_balancers.rst @@ -41,11 +41,22 @@ same or different weights. less than or equal to all of the other hosts. * *all weights not equal*: If two or more hosts in the cluster have different load balancing weights, the load balancer shifts into a mode where it uses a weighted round robin schedule in - which weights are dynamically adjusted based on the host's request load at the time of selection - (weight is divided by the current active request count. For example, a host with weight 2 and an - active request count of 4 will have a synthetic weight of 2 / 4 = 0.5). This algorithm provides - good balance at steady state but may not adapt to load imbalance as quickly. Additionally, unlike - P2C, a host will never truly drain, though it will receive fewer requests over time. + which weights are dynamically adjusted based on the host's request load at the time of selection. + + In this case the weights are calculated at the time a host is picked using the following formula: + + `weight = load_balancing_weight / (active_requests + 1)^active_requests_exponent`. + + :ref:`active_requests_exponent` + can be configured via runtime and defaults to 1.0. + + If `active_requests_exponent` is set to 0.0, the least request load balancer behaves like the + round robin load balancer and ignores the active request count at the time of picking. + + For example, if active_requests_exponent is 1.0, a host with weight 2 and an active request count + of 4 will have an effective weight of 2 / (4 + 1)^1 = 0.4. This algorithm provides good balance at + steady state but may not adapt to load imbalance as quickly. Additionally, unlike P2C, a host will + never truly drain, though it will receive fewer requests over time. .. _arch_overview_load_balancing_types_ring_hash: diff --git a/generated_api_shadow/envoy/config/cluster/v3/cluster.proto b/generated_api_shadow/envoy/config/cluster/v3/cluster.proto index 8140007f68afd..3e8b04a67e57f 100644 --- a/generated_api_shadow/envoy/config/cluster/v3/cluster.proto +++ b/generated_api_shadow/envoy/config/cluster/v3/cluster.proto @@ -318,6 +318,23 @@ message Cluster { // The number of random healthy hosts from which the host with the fewest active requests will // be chosen. Defaults to 2 so that we perform two-choice selection if the field is not set. google.protobuf.UInt32Value choice_count = 1 [(validate.rules).uint32 = {gte: 2}]; + + // The following formula is used to calculate the dynamic weights when hosts have different + // load balancing weights: + // + // `weight = load_balancing_weight / (active_requests + 1)^active_requests_exponent` + // + // When `active_requests_exponent == 0.0` the Least Request Load Balancer doesn't consider the + // number of active requests at the time it picks a host and behaves like the Round Robin Load + // Balancer. + // + // When `active_requests_exponent > 0.0` the Least Request Load Balancer scales the load + // balancing weight by the number of active requests at the time it does a pick. + // + // The exponent is cached for performance reasons and its value is only refreshed whenever + // one of the Load Balancer's host sets changes, e.g., whenever there is a host membership + // update or a host load balancing weight change. + core.v3.RuntimeDouble active_requests_exponent = 2; } // Specific configuration for the :ref:`RingHash` diff --git a/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto b/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto index eab2f2d80fcb7..5cab7d1b7dc15 100644 --- a/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto +++ b/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto @@ -318,6 +318,23 @@ message Cluster { // The number of random healthy hosts from which the host with the fewest active requests will // be chosen. Defaults to 2 so that we perform two-choice selection if the field is not set. google.protobuf.UInt32Value choice_count = 1 [(validate.rules).uint32 = {gte: 2}]; + + // The following formula is used to calculate the dynamic weights when hosts have different + // load balancing weights: + // + // `weight = load_balancing_weight / (active_requests + 1)^active_requests_exponent` + // + // When `active_requests_exponent == 0.0` the Least Request Load Balancer doesn't consider the + // number of active requests at the time it picks a host and behaves like the Round Robin Load + // Balancer. + // + // When `active_requests_exponent > 0.0` the Least Request Load Balancer scales the load + // balancing weight by the number of active requests at the time it does a pick. + // + // The exponent is cached for performance reasons and its value is only refreshed whenever + // one of the Load Balancer's host sets changes, e.g., whenever there is a host membership + // update or a host load balancing weight change. + core.v4alpha.RuntimeDouble active_requests_exponent = 2; } // Specific configuration for the :ref:`RingHash` diff --git a/source/common/upstream/BUILD b/source/common/upstream/BUILD index bdbe7c309f6de..6a05ae6e8d8bc 100644 --- a/source/common/upstream/BUILD +++ b/source/common/upstream/BUILD @@ -181,6 +181,7 @@ envoy_cc_library( "//include/envoy/upstream:upstream_interface", "//source/common/common:assert_lib", "//source/common/protobuf:utility_lib", + "//source/common/runtime:runtime_protos_lib", "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto", ], ) diff --git a/source/common/upstream/load_balancer_impl.h b/source/common/upstream/load_balancer_impl.h index 3c764b7312e23..a8831a214b76a 100644 --- a/source/common/upstream/load_balancer_impl.h +++ b/source/common/upstream/load_balancer_impl.h @@ -12,16 +12,12 @@ #include "envoy/upstream/upstream.h" #include "common/protobuf/utility.h" +#include "common/runtime/runtime_protos.h" #include "common/upstream/edf_scheduler.h" -#include "absl/strings/string_view.h" - namespace Envoy { namespace Upstream { -constexpr absl::string_view RuntimeLeastRequestsActiveRequestsExponent = - "upstream.least_requests.active_requests_exponent"; - // Priority levels and localities are considered overprovisioned with this factor. static constexpr uint32_t kDefaultOverProvisioningFactor = 140; @@ -371,7 +367,7 @@ class EdfLoadBalancerBase : public ZoneAwareLoadBalancerBase { std::unique_ptr> edf_; }; - virtual void initialize(); + void initialize(); virtual void refresh(uint32_t priority); @@ -444,8 +440,7 @@ class RoundRobinLoadBalancer : public EdfLoadBalancerBase { * The benefit of the Maglev table is at the expense of resolution, memory usage is capped. * Additionally, the Maglev table can be shared amongst all threads. */ -class LeastRequestLoadBalancer : public EdfLoadBalancerBase, - Logger::Loggable { +class LeastRequestLoadBalancer : public EdfLoadBalancerBase { public: LeastRequestLoadBalancer( const PrioritySet& priority_set, const PrioritySet* local_priority_set, ClusterStats& stats, @@ -458,59 +453,66 @@ class LeastRequestLoadBalancer : public EdfLoadBalancerBase, choice_count_( least_request_config.has_value() ? PROTOBUF_GET_WRAPPED_OR_DEFAULT(least_request_config.value(), choice_count, 2) - : 2) { + : 2), + active_requests_exponent_runtime_( + least_request_config.has_value() && least_request_config->has_active_requests_exponent() + ? absl::optional( + Runtime::Double(least_request_config->active_requests_exponent(), runtime)) + : absl::nullopt) { initialize(); } protected: void refresh(uint32_t priority) override { - active_requests_exponent_ = - runtime_.snapshot().getDouble(RuntimeLeastRequestsActiveRequestsExponent, 1.0); + active_requests_exponent_ = active_requests_exponent_runtime_.has_value() + ? active_requests_exponent_runtime_->value() + : 1.0; + EdfLoadBalancerBase::refresh(priority); } private: void refreshHostSource(const HostsSource&) override {} double hostWeight(const Host& host) override { - // Here we scale host weight by the number of active requests at the time we do the pick. We - // always add 1 to avoid division by 0. It might be possible to do better by picking two hosts - // off of the schedule, and selecting the one with fewer active requests at the time of - // selection. - // TODO(mattklein123): @htuch brings up the point that how we are scaling weight here might not - // be the only/best way of doing this. Essentially, it makes weight and active requests equally - // important. Are they equally important in practice? There is no right answer here and we might - // want to iterate on this as we gain more experience. - - // TODO(gkleiman): the final PR will not log anything and will return directly instead of using - // a temporary variable. - double weight; - + // This method is called to calculate the dynamic weight as following when all load balancing + // weights are not equal: + // + // `weight = load_balancing_weight / (active_requests + 1)^active_requests_exponent` + // + // `active_requests_exponent` can be configured via runtime and its value is cached in + // `active_requests_exponent_` to avoid having to do a runtime lookup each time a host weight is + // calculated. + // + // When `active_requests_exponent == 0.0` we behave like `RoundRobinLoadBalancer` and return the + // host weight without considering the number of active requests at the time we do the pick. + // + // When `active_requests_exponent > 0.0` we scale the host weight by the number of active + // requests at the time we do the pick. We always add 1 to avoid division by 0. + // + // It might be possible to do better by picking two hosts off of the schedule, and selecting the + // one with fewer active requests at the time of selection. if (active_requests_exponent_ == 0.0) { - weight = host.weight(); - } else if (active_requests_exponent_ == 1.0) { - weight = static_cast(host.weight()) / (host.stats().rq_active_.value() + 1); - } else { - weight = static_cast(host.weight()) / - std::pow(host.stats().rq_active_.value() + 1, active_requests_exponent_); + return host.weight(); } - ENVOY_LOG(trace, "cluster={} address={} active_requests_exponent={} weight={}", - host.cluster().name(), host.address()->asString(), active_requests_exponent_, weight); + if (active_requests_exponent_ == 1.0) { + return static_cast(host.weight()) / (host.stats().rq_active_.value() + 1); + } - return weight; + return static_cast(host.weight()) / + std::pow(host.stats().rq_active_.value() + 1, active_requests_exponent_); } HostConstSharedPtr unweightedHostPick(const HostVector& hosts_to_use, const HostsSource& source) override; const uint32_t choice_count_; - // When hosts have different weights, the host weight is calculated as: - // - // host_weight = (configured_weight / active_requests^k). k is configured via runtime and its - // value is cached to avoid having to do a runtime lookup each time a host weight is generated. - // - // The cached value is refreshed in `LeastRequestLoadBalancer::refresh(uint32_t priority)`. + // The exponent used to calculate host weights can be configured via runtime. We cache it for + // performance reasons and refresh it in `LeastRequestLoadBalancer::refresh(uint32_t priority)` + // whenever a `HostSet` is updated. double active_requests_exponent_; + + const absl::optional active_requests_exponent_runtime_; }; /** diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc index a5960424dc28f..38d62a85a051c 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.cc +++ b/source/extensions/filters/http/ext_authz/ext_authz.cc @@ -212,7 +212,7 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { callbacks_->sendLocalReply( response->status_code, response->body, - [& headers = response->headers_to_add, + [&headers = response->headers_to_add, &callbacks = *callbacks_](Http::HeaderMap& response_headers) -> void { ENVOY_STREAM_LOG(trace, "ext_authz filter added header(s) to the local response:", callbacks); diff --git a/test/common/upstream/load_balancer_impl_test.cc b/test/common/upstream/load_balancer_impl_test.cc index 2ebb217d4272a..cf315ae7f83ca 100644 --- a/test/common/upstream/load_balancer_impl_test.cc +++ b/test/common/upstream/load_balancer_impl_test.cc @@ -1534,9 +1534,14 @@ TEST_P(LeastRequestLoadBalancerTest, WeightImbalance) { } TEST_P(LeastRequestLoadBalancerTest, WeightImbalanceWithCustomExponent) { - EXPECT_CALL(runtime_.snapshot_, - getDouble("upstream.least_requests.active_requests_exponent", 1.0)) - .WillRepeatedly(Return(0.0)); + // Create a load balancer with a custom active requests exponent. + envoy::config::cluster::v3::Cluster::LeastRequestLbConfig lr_lb_config; + lr_lb_config.mutable_active_requests_exponent()->set_runtime_key("exponent"); + lr_lb_config.mutable_active_requests_exponent()->set_default_value(1.0); + LeastRequestLoadBalancer lb_2{priority_set_, nullptr, stats_, runtime_, + random_, common_config_, lr_lb_config}; + + EXPECT_CALL(runtime_.snapshot_, getDouble("exponent", 1.0)).WillRepeatedly(Return(0.0)); hostSet().healthy_hosts_ = {makeTestHost(info_, "tcp://127.0.0.1:80", 1), makeTestHost(info_, "tcp://127.0.0.1:81", 2)}; @@ -1549,12 +1554,12 @@ TEST_P(LeastRequestLoadBalancerTest, WeightImbalanceWithCustomExponent) { // We should see 2:1 ratio for hosts[1] to hosts[0], regardless of the active request count. hostSet().healthy_hosts_[1]->stats().rq_active_.set(1); - EXPECT_EQ(hostSet().healthy_hosts_[1], lb_.chooseHost(nullptr)); - EXPECT_EQ(hostSet().healthy_hosts_[0], lb_.chooseHost(nullptr)); - EXPECT_EQ(hostSet().healthy_hosts_[1], lb_.chooseHost(nullptr)); - EXPECT_EQ(hostSet().healthy_hosts_[1], lb_.chooseHost(nullptr)); - EXPECT_EQ(hostSet().healthy_hosts_[0], lb_.chooseHost(nullptr)); - EXPECT_EQ(hostSet().healthy_hosts_[1], lb_.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[0], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[0], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); } TEST_P(LeastRequestLoadBalancerTest, WeightImbalanceCallbacks) { From 25f7c98afb5c3451a21f71959cff167286c96a6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Kleiman?= Date: Mon, 22 Jun 2020 16:18:31 -0700 Subject: [PATCH 06/19] Address feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Gastón Kleiman --- api/envoy/config/cluster/v3/cluster.proto | 30 +++++---- .../config/cluster/v4alpha/cluster.proto | 30 +++++---- .../load_balancing/load_balancers.rst | 17 ++--- .../envoy/config/cluster/v3/cluster.proto | 30 +++++---- .../config/cluster/v4alpha/cluster.proto | 30 +++++---- source/common/runtime/runtime_protos.h | 2 + source/common/upstream/load_balancer_impl.h | 40 +++++++----- .../filters/http/ext_authz/ext_authz.cc | 2 +- .../upstream/load_balancer_impl_test.cc | 62 +++++++++++++++++-- 9 files changed, 168 insertions(+), 75 deletions(-) diff --git a/api/envoy/config/cluster/v3/cluster.proto b/api/envoy/config/cluster/v3/cluster.proto index cc8eaf3c33cbe..b1441f59965ae 100644 --- a/api/envoy/config/cluster/v3/cluster.proto +++ b/api/envoy/config/cluster/v3/cluster.proto @@ -319,22 +319,30 @@ message Cluster { // be chosen. Defaults to 2 so that we perform two-choice selection if the field is not set. google.protobuf.UInt32Value choice_count = 1 [(validate.rules).uint32 = {gte: 2}]; - // The following formula is used to calculate the dynamic weights when hosts have different - // load balancing weights: + // The following formula is used to calculate the dynamic weights when hosts have different load + // balancing weights: // - // `weight = load_balancing_weight / (active_requests + 1)^active_requests_exponent` + // `weight = load_balancing_weight / (active_requests + 1)^active_request_bias` // - // When `active_requests_exponent == 0.0` the Least Request Load Balancer doesn't consider the - // number of active requests at the time it picks a host and behaves like the Round Robin Load + // The larger the active request bias is, the more aggressively active requests will lower the + // effective weight when all host weights are not equal. + // + // `active_request_bias` must be greater than or equal to 0.0. + // + // When `active_request_bias == 0.0` the Least Request Load Balancer doesn't consider the number + // of active requests at the time it picks a host and behaves like the Round Robin Load // Balancer. // - // When `active_requests_exponent > 0.0` the Least Request Load Balancer scales the load - // balancing weight by the number of active requests at the time it does a pick. + // When `active_request_bias > 0.0` the Least Request Load Balancer scales the load balancing + // weight by the number of active requests at the time it does a pick. // - // The exponent is cached for performance reasons and its value is only refreshed whenever - // one of the Load Balancer's host sets changes, e.g., whenever there is a host membership - // update or a host load balancing weight change. - core.v3.RuntimeDouble active_requests_exponent = 2; + // The value is cached for performance reasons and refreshed whenever one of the Load Balancer's + // host sets changes, e.g., whenever there is a host membership update or a host load balancing + // weight change. + // + // .. note:: + // This setting only takes effect if all host weights are not equal. + core.v3.RuntimeDouble active_request_bias = 2; } // Specific configuration for the :ref:`RingHash` diff --git a/api/envoy/config/cluster/v4alpha/cluster.proto b/api/envoy/config/cluster/v4alpha/cluster.proto index 5cab7d1b7dc15..0799c2c7ac555 100644 --- a/api/envoy/config/cluster/v4alpha/cluster.proto +++ b/api/envoy/config/cluster/v4alpha/cluster.proto @@ -319,22 +319,30 @@ message Cluster { // be chosen. Defaults to 2 so that we perform two-choice selection if the field is not set. google.protobuf.UInt32Value choice_count = 1 [(validate.rules).uint32 = {gte: 2}]; - // The following formula is used to calculate the dynamic weights when hosts have different - // load balancing weights: + // The following formula is used to calculate the dynamic weights when hosts have different load + // balancing weights: // - // `weight = load_balancing_weight / (active_requests + 1)^active_requests_exponent` + // `weight = load_balancing_weight / (active_requests + 1)^active_request_bias` // - // When `active_requests_exponent == 0.0` the Least Request Load Balancer doesn't consider the - // number of active requests at the time it picks a host and behaves like the Round Robin Load + // The larger the active request bias is, the more aggressively active requests will lower the + // effective weight when all host weights are not equal. + // + // `active_request_bias` must be greater than or equal to 0.0. + // + // When `active_request_bias == 0.0` the Least Request Load Balancer doesn't consider the number + // of active requests at the time it picks a host and behaves like the Round Robin Load // Balancer. // - // When `active_requests_exponent > 0.0` the Least Request Load Balancer scales the load - // balancing weight by the number of active requests at the time it does a pick. + // When `active_request_bias > 0.0` the Least Request Load Balancer scales the load balancing + // weight by the number of active requests at the time it does a pick. // - // The exponent is cached for performance reasons and its value is only refreshed whenever - // one of the Load Balancer's host sets changes, e.g., whenever there is a host membership - // update or a host load balancing weight change. - core.v4alpha.RuntimeDouble active_requests_exponent = 2; + // The value is cached for performance reasons and refreshed whenever one of the Load Balancer's + // host sets changes, e.g., whenever there is a host membership update or a host load balancing + // weight change. + // + // .. note:: + // This setting only takes effect if all host weights are not equal. + core.v4alpha.RuntimeDouble active_request_bias = 2; } // Specific configuration for the :ref:`RingHash` diff --git a/docs/root/intro/arch_overview/upstream/load_balancing/load_balancers.rst b/docs/root/intro/arch_overview/upstream/load_balancing/load_balancers.rst index beae5ddc813d0..38b6c6fae88ab 100644 --- a/docs/root/intro/arch_overview/upstream/load_balancing/load_balancers.rst +++ b/docs/root/intro/arch_overview/upstream/load_balancing/load_balancers.rst @@ -45,16 +45,19 @@ same or different weights. In this case the weights are calculated at the time a host is picked using the following formula: - `weight = load_balancing_weight / (active_requests + 1)^active_requests_exponent`. + `weight = load_balancing_weight / (active_requests + 1)^active_request_bias`. - :ref:`active_requests_exponent` - can be configured via runtime and defaults to 1.0. + :ref:`active_request_bias` + can be configured via runtime and defaults to 1.0. It must be greater than or equal to 0.0. - If `active_requests_exponent` is set to 0.0, the least request load balancer behaves like the - round robin load balancer and ignores the active request count at the time of picking. + The larger the active request bias is, the more aggressively active requests will lower the + effective weight. - For example, if active_requests_exponent is 1.0, a host with weight 2 and an active request count - of 4 will have an effective weight of 2 / (4 + 1)^1 = 0.4. This algorithm provides good balance at + If `active_request_bias` is set to 0.0, the least request load balancer behaves like the round + robin load balancer and ignores the active request count at the time of picking. + + For example, if active_request_bias is 1.0, a host with weight 2 and an active request count of 4 + will have an effective weight of 2 / (4 + 1)^1 = 0.4. This algorithm provides good balance at steady state but may not adapt to load imbalance as quickly. Additionally, unlike P2C, a host will never truly drain, though it will receive fewer requests over time. diff --git a/generated_api_shadow/envoy/config/cluster/v3/cluster.proto b/generated_api_shadow/envoy/config/cluster/v3/cluster.proto index 3e8b04a67e57f..bdbab238d7829 100644 --- a/generated_api_shadow/envoy/config/cluster/v3/cluster.proto +++ b/generated_api_shadow/envoy/config/cluster/v3/cluster.proto @@ -319,22 +319,30 @@ message Cluster { // be chosen. Defaults to 2 so that we perform two-choice selection if the field is not set. google.protobuf.UInt32Value choice_count = 1 [(validate.rules).uint32 = {gte: 2}]; - // The following formula is used to calculate the dynamic weights when hosts have different - // load balancing weights: + // The following formula is used to calculate the dynamic weights when hosts have different load + // balancing weights: // - // `weight = load_balancing_weight / (active_requests + 1)^active_requests_exponent` + // `weight = load_balancing_weight / (active_requests + 1)^active_request_bias` // - // When `active_requests_exponent == 0.0` the Least Request Load Balancer doesn't consider the - // number of active requests at the time it picks a host and behaves like the Round Robin Load + // The larger the active request bias is, the more aggressively active requests will lower the + // effective weight when all host weights are not equal. + // + // `active_request_bias` must be greater than or equal to 0.0. + // + // When `active_request_bias == 0.0` the Least Request Load Balancer doesn't consider the number + // of active requests at the time it picks a host and behaves like the Round Robin Load // Balancer. // - // When `active_requests_exponent > 0.0` the Least Request Load Balancer scales the load - // balancing weight by the number of active requests at the time it does a pick. + // When `active_request_bias > 0.0` the Least Request Load Balancer scales the load balancing + // weight by the number of active requests at the time it does a pick. // - // The exponent is cached for performance reasons and its value is only refreshed whenever - // one of the Load Balancer's host sets changes, e.g., whenever there is a host membership - // update or a host load balancing weight change. - core.v3.RuntimeDouble active_requests_exponent = 2; + // The value is cached for performance reasons and refreshed whenever one of the Load Balancer's + // host sets changes, e.g., whenever there is a host membership update or a host load balancing + // weight change. + // + // .. note:: + // This setting only takes effect if all host weights are not equal. + core.v3.RuntimeDouble active_request_bias = 2; } // Specific configuration for the :ref:`RingHash` diff --git a/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto b/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto index 5cab7d1b7dc15..0799c2c7ac555 100644 --- a/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto +++ b/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto @@ -319,22 +319,30 @@ message Cluster { // be chosen. Defaults to 2 so that we perform two-choice selection if the field is not set. google.protobuf.UInt32Value choice_count = 1 [(validate.rules).uint32 = {gte: 2}]; - // The following formula is used to calculate the dynamic weights when hosts have different - // load balancing weights: + // The following formula is used to calculate the dynamic weights when hosts have different load + // balancing weights: // - // `weight = load_balancing_weight / (active_requests + 1)^active_requests_exponent` + // `weight = load_balancing_weight / (active_requests + 1)^active_request_bias` // - // When `active_requests_exponent == 0.0` the Least Request Load Balancer doesn't consider the - // number of active requests at the time it picks a host and behaves like the Round Robin Load + // The larger the active request bias is, the more aggressively active requests will lower the + // effective weight when all host weights are not equal. + // + // `active_request_bias` must be greater than or equal to 0.0. + // + // When `active_request_bias == 0.0` the Least Request Load Balancer doesn't consider the number + // of active requests at the time it picks a host and behaves like the Round Robin Load // Balancer. // - // When `active_requests_exponent > 0.0` the Least Request Load Balancer scales the load - // balancing weight by the number of active requests at the time it does a pick. + // When `active_request_bias > 0.0` the Least Request Load Balancer scales the load balancing + // weight by the number of active requests at the time it does a pick. // - // The exponent is cached for performance reasons and its value is only refreshed whenever - // one of the Load Balancer's host sets changes, e.g., whenever there is a host membership - // update or a host load balancing weight change. - core.v4alpha.RuntimeDouble active_requests_exponent = 2; + // The value is cached for performance reasons and refreshed whenever one of the Load Balancer's + // host sets changes, e.g., whenever there is a host membership update or a host load balancing + // weight change. + // + // .. note:: + // This setting only takes effect if all host weights are not equal. + core.v4alpha.RuntimeDouble active_request_bias = 2; } // Specific configuration for the :ref:`RingHash` diff --git a/source/common/runtime/runtime_protos.h b/source/common/runtime/runtime_protos.h index 06b0e5816d5a2..dd2780701d1bc 100644 --- a/source/common/runtime/runtime_protos.h +++ b/source/common/runtime/runtime_protos.h @@ -35,6 +35,8 @@ class Double { : runtime_key_(double_proto.runtime_key()), default_value_(double_proto.default_value()), runtime_(runtime) {} + const std::string& runtime_key() const { return runtime_key_; } + double value() const { return runtime_.snapshot().getDouble(runtime_key_, default_value_); } private: diff --git a/source/common/upstream/load_balancer_impl.h b/source/common/upstream/load_balancer_impl.h index a8831a214b76a..a6a5f7c61081f 100644 --- a/source/common/upstream/load_balancer_impl.h +++ b/source/common/upstream/load_balancer_impl.h @@ -440,7 +440,8 @@ class RoundRobinLoadBalancer : public EdfLoadBalancerBase { * The benefit of the Maglev table is at the expense of resolution, memory usage is capped. * Additionally, the Maglev table can be shared amongst all threads. */ -class LeastRequestLoadBalancer : public EdfLoadBalancerBase { +class LeastRequestLoadBalancer : public EdfLoadBalancerBase, + Logger::Loggable { public: LeastRequestLoadBalancer( const PrioritySet& priority_set, const PrioritySet* local_priority_set, ClusterStats& stats, @@ -454,19 +455,24 @@ class LeastRequestLoadBalancer : public EdfLoadBalancerBase { least_request_config.has_value() ? PROTOBUF_GET_WRAPPED_OR_DEFAULT(least_request_config.value(), choice_count, 2) : 2), - active_requests_exponent_runtime_( - least_request_config.has_value() && least_request_config->has_active_requests_exponent() + active_request_bias_runtime_( + least_request_config.has_value() && least_request_config->has_active_request_bias() ? absl::optional( - Runtime::Double(least_request_config->active_requests_exponent(), runtime)) + Runtime::Double(least_request_config->active_request_bias(), runtime)) : absl::nullopt) { initialize(); } protected: void refresh(uint32_t priority) override { - active_requests_exponent_ = active_requests_exponent_runtime_.has_value() - ? active_requests_exponent_runtime_->value() - : 1.0; + active_request_bias_ = + active_request_bias_runtime_.has_value() ? active_request_bias_runtime_->value() : 1.0; + + if (active_request_bias_ < 0.0) { + ENVOY_LOG(warn, "upstream: invalid active request bias supplied (runtime key {}), using 1.0", + active_request_bias_runtime_->runtime_key()); + active_request_bias_ = 1.0; + } EdfLoadBalancerBase::refresh(priority); } @@ -477,30 +483,30 @@ class LeastRequestLoadBalancer : public EdfLoadBalancerBase { // This method is called to calculate the dynamic weight as following when all load balancing // weights are not equal: // - // `weight = load_balancing_weight / (active_requests + 1)^active_requests_exponent` + // `weight = load_balancing_weight / (active_requests + 1)^active_request_bias` // - // `active_requests_exponent` can be configured via runtime and its value is cached in - // `active_requests_exponent_` to avoid having to do a runtime lookup each time a host weight is + // `active_request_bias` can be configured via runtime and its value is cached in + // `active_request_bias_` to avoid having to do a runtime lookup each time a host weight is // calculated. // - // When `active_requests_exponent == 0.0` we behave like `RoundRobinLoadBalancer` and return the + // When `active_request_bias == 0.0` we behave like `RoundRobinLoadBalancer` and return the // host weight without considering the number of active requests at the time we do the pick. // - // When `active_requests_exponent > 0.0` we scale the host weight by the number of active + // When `active_request_bias > 0.0` we scale the host weight by the number of active // requests at the time we do the pick. We always add 1 to avoid division by 0. // // It might be possible to do better by picking two hosts off of the schedule, and selecting the // one with fewer active requests at the time of selection. - if (active_requests_exponent_ == 0.0) { + if (active_request_bias_ == 0.0) { return host.weight(); } - if (active_requests_exponent_ == 1.0) { + if (active_request_bias_ == 1.0) { return static_cast(host.weight()) / (host.stats().rq_active_.value() + 1); } return static_cast(host.weight()) / - std::pow(host.stats().rq_active_.value() + 1, active_requests_exponent_); + std::pow(host.stats().rq_active_.value() + 1, active_request_bias_); } HostConstSharedPtr unweightedHostPick(const HostVector& hosts_to_use, const HostsSource& source) override; @@ -510,9 +516,9 @@ class LeastRequestLoadBalancer : public EdfLoadBalancerBase { // The exponent used to calculate host weights can be configured via runtime. We cache it for // performance reasons and refresh it in `LeastRequestLoadBalancer::refresh(uint32_t priority)` // whenever a `HostSet` is updated. - double active_requests_exponent_; + double active_request_bias_; - const absl::optional active_requests_exponent_runtime_; + const absl::optional active_request_bias_runtime_; }; /** diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc index 38d62a85a051c..a5960424dc28f 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.cc +++ b/source/extensions/filters/http/ext_authz/ext_authz.cc @@ -212,7 +212,7 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { callbacks_->sendLocalReply( response->status_code, response->body, - [&headers = response->headers_to_add, + [& headers = response->headers_to_add, &callbacks = *callbacks_](Http::HeaderMap& response_headers) -> void { ENVOY_STREAM_LOG(trace, "ext_authz filter added header(s) to the local response:", callbacks); diff --git a/test/common/upstream/load_balancer_impl_test.cc b/test/common/upstream/load_balancer_impl_test.cc index cf315ae7f83ca..0f989a13068aa 100644 --- a/test/common/upstream/load_balancer_impl_test.cc +++ b/test/common/upstream/load_balancer_impl_test.cc @@ -1533,19 +1533,69 @@ TEST_P(LeastRequestLoadBalancerTest, WeightImbalance) { EXPECT_EQ(hostSet().healthy_hosts_[0], lb_.chooseHost(nullptr)); } -TEST_P(LeastRequestLoadBalancerTest, WeightImbalanceWithCustomExponent) { - // Create a load balancer with a custom active requests exponent. +// Validate that the load balancer defaults to an active request bias value of 1.0 if the runtime +// value is invalid (less than 0.0). +TEST_P(LeastRequestLoadBalancerTest, WeightImbalanceWithInvalidActiveRequestBias) { envoy::config::cluster::v3::Cluster::LeastRequestLbConfig lr_lb_config; - lr_lb_config.mutable_active_requests_exponent()->set_runtime_key("exponent"); - lr_lb_config.mutable_active_requests_exponent()->set_default_value(1.0); + lr_lb_config.mutable_active_request_bias()->set_runtime_key("ar_bias"); + lr_lb_config.mutable_active_request_bias()->set_default_value(1.0); LeastRequestLoadBalancer lb_2{priority_set_, nullptr, stats_, runtime_, random_, common_config_, lr_lb_config}; - EXPECT_CALL(runtime_.snapshot_, getDouble("exponent", 1.0)).WillRepeatedly(Return(0.0)); + EXPECT_CALL(runtime_.snapshot_, getDouble("ar_bias", 1.0)).WillRepeatedly(Return(-1.0)); + + hostSet().healthy_hosts_ = {makeTestHost(info_, "tcp://127.0.0.1:80", 1), + makeTestHost(info_, "tcp://127.0.0.1:81", 2)}; + + hostSet().hosts_ = hostSet().healthy_hosts_; + hostSet().runCallbacks({}, {}); // Trigger callbacks. The added/removed lists are not relevant. + + EXPECT_CALL(random_, random()).WillRepeatedly(Return(0)); + + // We should see 2:1 ratio for hosts[1] to hosts[0]. + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[0], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[0], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + + // Bringing hosts[1] to an active request should yield a 1:1 ratio. + hostSet().healthy_hosts_[1]->stats().rq_active_.set(1); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[0], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[0], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + + // Settings hosts[0] to an active request and hosts[1] to no active requests should yield a 4:1 + // ratio. + hostSet().healthy_hosts_[0]->stats().rq_active_.set(1); + hostSet().healthy_hosts_[1]->stats().rq_active_.set(0); + EXPECT_EQ(hostSet().healthy_hosts_[0], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[0], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[0], lb_2.chooseHost(nullptr)); +} + +TEST_P(LeastRequestLoadBalancerTest, WeightImbalanceWithCustomActiveRequestBias) { + // Create a load balancer with a custom active request bias. + envoy::config::cluster::v3::Cluster::LeastRequestLbConfig lr_lb_config; + lr_lb_config.mutable_active_request_bias()->set_runtime_key("ar_bias"); + lr_lb_config.mutable_active_request_bias()->set_default_value(1.0); + LeastRequestLoadBalancer lb_2{priority_set_, nullptr, stats_, runtime_, + random_, common_config_, lr_lb_config}; + + EXPECT_CALL(runtime_.snapshot_, getDouble("ar_bias", 1.0)).WillRepeatedly(Return(0.0)); hostSet().healthy_hosts_ = {makeTestHost(info_, "tcp://127.0.0.1:80", 1), makeTestHost(info_, "tcp://127.0.0.1:81", 2)}; - stats_.max_host_weight_.set(2UL); hostSet().hosts_ = hostSet().healthy_hosts_; hostSet().runCallbacks({}, {}); // Trigger callbacks. The added/removed lists are not relevant. From f4b2da33b3d6b2be7a30a0d1e24db1d81d3d47a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Kleiman?= Date: Tue, 23 Jun 2020 12:43:41 -0700 Subject: [PATCH 07/19] Validate log message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Gastón Kleiman --- test/common/upstream/BUILD | 1 + test/common/upstream/load_balancer_impl_test.cc | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/test/common/upstream/BUILD b/test/common/upstream/BUILD index 9f28017e87c37..48d2abba5d995 100644 --- a/test/common/upstream/BUILD +++ b/test/common/upstream/BUILD @@ -189,6 +189,7 @@ envoy_cc_test( "//source/common/upstream:upstream_lib", "//test/mocks/runtime:runtime_mocks", "//test/mocks/upstream:upstream_mocks", + "//test/test_common:logging_lib", "//test/test_common:test_runtime_lib", "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto", ], diff --git a/test/common/upstream/load_balancer_impl_test.cc b/test/common/upstream/load_balancer_impl_test.cc index 0f989a13068aa..16f18a0ca192e 100644 --- a/test/common/upstream/load_balancer_impl_test.cc +++ b/test/common/upstream/load_balancer_impl_test.cc @@ -13,6 +13,7 @@ #include "test/common/upstream/utility.h" #include "test/mocks/runtime/mocks.h" #include "test/mocks/upstream/mocks.h" +#include "test/test_common/logging.h" #include "test/test_common/test_runtime.h" #include "gmock/gmock.h" @@ -1548,7 +1549,11 @@ TEST_P(LeastRequestLoadBalancerTest, WeightImbalanceWithInvalidActiveRequestBias makeTestHost(info_, "tcp://127.0.0.1:81", 2)}; hostSet().hosts_ = hostSet().healthy_hosts_; - hostSet().runCallbacks({}, {}); // Trigger callbacks. The added/removed lists are not relevant. + + // Trigger callbacks. The added/removed lists are not relevant. + EXPECT_LOG_CONTAINS( + "warn", "upstream: invalid active request bias supplied (runtime key ar_bias), using 1.0", + hostSet().runCallbacks({}, {})); EXPECT_CALL(random_, random()).WillRepeatedly(Return(0)); From e96d52defcfc8eb37773ac35e0abad7b6cd398af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Kleiman?= Date: Tue, 23 Jun 2020 16:09:51 -0700 Subject: [PATCH 08/19] Update cluster memory test golden values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Gastón Kleiman --- test/integration/stats_integration_test.cc | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/integration/stats_integration_test.cc b/test/integration/stats_integration_test.cc index 43985c20fead3..218a130d6151f 100644 --- a/test/integration/stats_integration_test.cc +++ b/test/integration/stats_integration_test.cc @@ -277,6 +277,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithFakeSymbolTable) { // 2020/05/05 10908 44233 44600 router: add InternalRedirectPolicy and predicate // 2020/05/13 10531 44425 44600 Refactor resource manager // 2020/05/20 11223 44491 44600 Add primary clusters tracking to cluster manager. + // 2020/06/23 11252 44747 44800 Introduce Least Request LB active request bias config // Note: when adjusting this value: EXPECT_MEMORY_EQ is active only in CI // 'release' builds, where we control the platform and tool-chain. So you @@ -290,8 +291,8 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithFakeSymbolTable) { // If you encounter a failure here, please see // https://github.com/envoyproxy/envoy/blob/master/source/docs/stats.md#stats-memory-tests // for details on how to fix. - EXPECT_MEMORY_EQ(m_per_cluster, 44491); - EXPECT_MEMORY_LE(m_per_cluster, 44600); + EXPECT_MEMORY_EQ(m_per_cluster, 44747); + EXPECT_MEMORY_LE(m_per_cluster, 44800); } TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithRealSymbolTable) { @@ -339,6 +340,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithRealSymbolTable) { // 2020/05/05 10908 36345 36800 router: add InternalRedirectPolicy and predicate // 2020/05/13 10531 36537 36800 Refactor resource manager // 2020/05/20 11223 36603 36800 Add primary clusters tracking to cluster manager. + // 2020/06/23 11252 44747 44800 Introduce Least Request LB active request bias config // Note: when adjusting this value: EXPECT_MEMORY_EQ is active only in CI // 'release' builds, where we control the platform and tool-chain. So you @@ -352,8 +354,8 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithRealSymbolTable) { // If you encounter a failure here, please see // https://github.com/envoyproxy/envoy/blob/master/source/docs/stats.md#stats-memory-tests // for details on how to fix. - EXPECT_MEMORY_EQ(m_per_cluster, 36603); - EXPECT_MEMORY_LE(m_per_cluster, 36800); + EXPECT_MEMORY_EQ(m_per_cluster, 36859); + EXPECT_MEMORY_LE(m_per_cluster, 36900); } TEST_P(ClusterMemoryTestRunner, MemoryLargeHostSizeWithStats) { From cfca8f7ec7505654e991d5d11a5ca9df58934f3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Kleiman?= Date: Thu, 25 Jun 2020 17:11:44 -0700 Subject: [PATCH 09/19] Fix method name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Gastón Kleiman --- source/common/runtime/runtime_protos.h | 2 +- source/common/upstream/load_balancer_impl.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/source/common/runtime/runtime_protos.h b/source/common/runtime/runtime_protos.h index dd2780701d1bc..855b145121db1 100644 --- a/source/common/runtime/runtime_protos.h +++ b/source/common/runtime/runtime_protos.h @@ -35,7 +35,7 @@ class Double { : runtime_key_(double_proto.runtime_key()), default_value_(double_proto.default_value()), runtime_(runtime) {} - const std::string& runtime_key() const { return runtime_key_; } + const std::string& runtimeKey() const { return runtime_key_; } double value() const { return runtime_.snapshot().getDouble(runtime_key_, default_value_); } diff --git a/source/common/upstream/load_balancer_impl.h b/source/common/upstream/load_balancer_impl.h index a6a5f7c61081f..fbf5e37463b78 100644 --- a/source/common/upstream/load_balancer_impl.h +++ b/source/common/upstream/load_balancer_impl.h @@ -470,7 +470,7 @@ class LeastRequestLoadBalancer : public EdfLoadBalancerBase, if (active_request_bias_ < 0.0) { ENVOY_LOG(warn, "upstream: invalid active request bias supplied (runtime key {}), using 1.0", - active_request_bias_runtime_->runtime_key()); + active_request_bias_runtime_->runtimeKey()); active_request_bias_ = 1.0; } From b487a5e544048ac8b35d833a0dd6ee8fcc405fba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Kleiman?= Date: Mon, 29 Jun 2020 12:05:44 -0700 Subject: [PATCH 10/19] Explicitly initialize active_request_bias_ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Gastón Kleiman --- source/common/upstream/load_balancer_impl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/common/upstream/load_balancer_impl.h b/source/common/upstream/load_balancer_impl.h index fbf5e37463b78..3d81cda145500 100644 --- a/source/common/upstream/load_balancer_impl.h +++ b/source/common/upstream/load_balancer_impl.h @@ -516,7 +516,7 @@ class LeastRequestLoadBalancer : public EdfLoadBalancerBase, // The exponent used to calculate host weights can be configured via runtime. We cache it for // performance reasons and refresh it in `LeastRequestLoadBalancer::refresh(uint32_t priority)` // whenever a `HostSet` is updated. - double active_request_bias_; + double active_request_bias_{}; const absl::optional active_request_bias_runtime_; }; From 3fc99ea3150cc143beb2a6d195199d65b4b7b292 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Kleiman?= Date: Mon, 29 Jun 2020 14:33:34 -0700 Subject: [PATCH 11/19] Try to make clang-tidy happy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Gastón Kleiman --- source/common/runtime/BUILD | 1 + 1 file changed, 1 insertion(+) diff --git a/source/common/runtime/BUILD b/source/common/runtime/BUILD index 4e76ab3157608..47ee7ef962574 100644 --- a/source/common/runtime/BUILD +++ b/source/common/runtime/BUILD @@ -34,6 +34,7 @@ envoy_cc_library( ], deps = [ "//include/envoy/runtime:runtime_interface", + "//source/common/protobuf:utility_lib", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", "@envoy_api//envoy/type/v3:pkg_cc_proto", ], From 9be22f0ac5304879f4c11c0380cdd9f025de318a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Kleiman?= Date: Mon, 29 Jun 2020 17:24:52 -0700 Subject: [PATCH 12/19] Use unique_ptr instead of optional MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Gastón Kleiman --- source/common/upstream/load_balancer_impl.h | 11 ++++++----- test/integration/stats_integration_test.cc | 6 ++---- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/source/common/upstream/load_balancer_impl.h b/source/common/upstream/load_balancer_impl.h index 3d81cda145500..41a8229845fbc 100644 --- a/source/common/upstream/load_balancer_impl.h +++ b/source/common/upstream/load_balancer_impl.h @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -457,16 +458,16 @@ class LeastRequestLoadBalancer : public EdfLoadBalancerBase, : 2), active_request_bias_runtime_( least_request_config.has_value() && least_request_config->has_active_request_bias() - ? absl::optional( - Runtime::Double(least_request_config->active_request_bias(), runtime)) - : absl::nullopt) { + ? std::make_unique(least_request_config->active_request_bias(), + runtime) + : nullptr) { initialize(); } protected: void refresh(uint32_t priority) override { active_request_bias_ = - active_request_bias_runtime_.has_value() ? active_request_bias_runtime_->value() : 1.0; + active_request_bias_runtime_ != nullptr ? active_request_bias_runtime_->value() : 1.0; if (active_request_bias_ < 0.0) { ENVOY_LOG(warn, "upstream: invalid active request bias supplied (runtime key {}), using 1.0", @@ -518,7 +519,7 @@ class LeastRequestLoadBalancer : public EdfLoadBalancerBase, // whenever a `HostSet` is updated. double active_request_bias_{}; - const absl::optional active_request_bias_runtime_; + const std::unique_ptr active_request_bias_runtime_; }; /** diff --git a/test/integration/stats_integration_test.cc b/test/integration/stats_integration_test.cc index 681c023fec078..dd4fb2bfc7225 100644 --- a/test/integration/stats_integration_test.cc +++ b/test/integration/stats_integration_test.cc @@ -273,7 +273,6 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithFakeSymbolTable) { // 2020/05/13 10531 44425 44600 Refactor resource manager // 2020/05/20 11223 44491 44600 Add primary clusters tracking to cluster manager. // 2020/06/10 11561 44491 44811 Make upstreams pluggable - // 2020/06/23 11252 44747 46000 Introduce Least Request LB active request bias config // Note: when adjusting this value: EXPECT_MEMORY_EQ is active only in CI // 'release' builds, where we control the platform and tool-chain. So you @@ -287,7 +286,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithFakeSymbolTable) { // If you encounter a failure here, please see // https://github.com/envoyproxy/envoy/blob/master/source/docs/stats.md#stats-memory-tests // for details on how to fix. - EXPECT_MEMORY_EQ(m_per_cluster, 44747); + EXPECT_MEMORY_EQ(m_per_cluster, 44491); EXPECT_MEMORY_LE(m_per_cluster, 46000); // Round up to allow platform variations. } @@ -337,7 +336,6 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithRealSymbolTable) { // 2020/05/13 10531 36537 36800 Refactor resource manager // 2020/05/20 11223 36603 36800 Add primary clusters tracking to cluster manager. // 2020/06/10 11561 36603 36923 Make upstreams pluggable - // 2020/06/23 11252 36859 38000 Introduce Least Request LB active request bias config // Note: when adjusting this value: EXPECT_MEMORY_EQ is active only in CI // 'release' builds, where we control the platform and tool-chain. So you @@ -351,7 +349,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithRealSymbolTable) { // If you encounter a failure here, please see // https://github.com/envoyproxy/envoy/blob/master/source/docs/stats.md#stats-memory-tests // for details on how to fix. - EXPECT_MEMORY_EQ(m_per_cluster, 36859); + EXPECT_MEMORY_EQ(m_per_cluster, 36603); EXPECT_MEMORY_LE(m_per_cluster, 38000); // Round up to allow platform variations. } From 71fcad862c2a12123f5225da6dbb44cb91dbff47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Kleiman?= Date: Tue, 30 Jun 2020 17:17:51 -0700 Subject: [PATCH 13/19] Update stats integration test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Gastón Kleiman --- test/integration/stats_integration_test.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/integration/stats_integration_test.cc b/test/integration/stats_integration_test.cc index 8cf76dc866083..008650a236add 100644 --- a/test/integration/stats_integration_test.cc +++ b/test/integration/stats_integration_test.cc @@ -274,6 +274,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithFakeSymbolTable) { // 2020/05/20 11223 44491 44600 Add primary clusters tracking to cluster manager. // 2020/06/10 11561 44491 44811 Make upstreams pluggable // 2020/04/23 10661 44425 46000 per-listener connection limits + // 2020/06/30 11252 44747 46000 Introduce Least Request LB active request bias config // Note: when adjusting this value: EXPECT_MEMORY_EQ is active only in CI // 'release' builds, where we control the platform and tool-chain. So you @@ -287,7 +288,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithFakeSymbolTable) { // If you encounter a failure here, please see // https://github.com/envoyproxy/envoy/blob/master/source/docs/stats.md#stats-memory-tests // for details on how to fix. - EXPECT_MEMORY_EQ(m_per_cluster, 44491); + EXPECT_MEMORY_EQ(m_per_cluster, 44747); EXPECT_MEMORY_LE(m_per_cluster, 46000); // Round up to allow platform variations. } @@ -338,6 +339,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithRealSymbolTable) { // 2020/05/20 11223 36603 36800 Add primary clusters tracking to cluster manager. // 2020/06/10 11561 36603 36923 Make upstreams pluggable // 2020/04/23 10661 36537 37000 per-listener connection limits + // 2020/06/30 11252 36859 37500 Introduce Least Request LB active request bias config // Note: when adjusting this value: EXPECT_MEMORY_EQ is active only in CI // 'release' builds, where we control the platform and tool-chain. So you @@ -351,8 +353,8 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithRealSymbolTable) { // If you encounter a failure here, please see // https://github.com/envoyproxy/envoy/blob/master/source/docs/stats.md#stats-memory-tests // for details on how to fix. - EXPECT_MEMORY_EQ(m_per_cluster, 36603); - EXPECT_MEMORY_LE(m_per_cluster, 37000); + EXPECT_MEMORY_EQ(m_per_cluster, 36859); + EXPECT_MEMORY_LE(m_per_cluster, 37500); // Round up to allow platform variations. } TEST_P(ClusterMemoryTestRunner, MemoryLargeHostSizeWithStats) { From 2690778d1d144436ada24e2caff726265b344b42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Kleiman?= Date: Wed, 1 Jul 2020 16:02:27 -0700 Subject: [PATCH 14/19] Check whether memory footprint is reduced without LB changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Gastón Kleiman --- source/common/upstream/load_balancer_impl.h | 74 +++------------- .../upstream/load_balancer_impl_test.cc | 85 ------------------- test/integration/stats_integration_test.cc | 8 +- 3 files changed, 15 insertions(+), 152 deletions(-) diff --git a/source/common/upstream/load_balancer_impl.h b/source/common/upstream/load_balancer_impl.h index 41a8229845fbc..9ef6f9fd8fac8 100644 --- a/source/common/upstream/load_balancer_impl.h +++ b/source/common/upstream/load_balancer_impl.h @@ -1,8 +1,6 @@ #pragma once -#include #include -#include #include #include #include @@ -13,7 +11,6 @@ #include "envoy/upstream/upstream.h" #include "common/protobuf/utility.h" -#include "common/runtime/runtime_protos.h" #include "common/upstream/edf_scheduler.h" namespace Envoy { @@ -370,8 +367,6 @@ class EdfLoadBalancerBase : public ZoneAwareLoadBalancerBase { void initialize(); - virtual void refresh(uint32_t priority); - // Seed to allow us to desynchronize load balancers across a fleet. If we don't // do this, multiple Envoys that receive an update at the same time (or even // multiple load balancers on the same host) will send requests to @@ -380,6 +375,7 @@ class EdfLoadBalancerBase : public ZoneAwareLoadBalancerBase { const uint64_t seed_; private: + void refresh(uint32_t priority); virtual void refreshHostSource(const HostsSource& source) PURE; virtual double hostWeight(const Host& host) PURE; virtual HostConstSharedPtr unweightedHostPick(const HostVector& hosts_to_use, @@ -441,8 +437,7 @@ class RoundRobinLoadBalancer : public EdfLoadBalancerBase { * The benefit of the Maglev table is at the expense of resolution, memory usage is capped. * Additionally, the Maglev table can be shared amongst all threads. */ -class LeastRequestLoadBalancer : public EdfLoadBalancerBase, - Logger::Loggable { +class LeastRequestLoadBalancer : public EdfLoadBalancerBase { public: LeastRequestLoadBalancer( const PrioritySet& priority_set, const PrioritySet* local_priority_set, ClusterStats& stats, @@ -455,71 +450,26 @@ class LeastRequestLoadBalancer : public EdfLoadBalancerBase, choice_count_( least_request_config.has_value() ? PROTOBUF_GET_WRAPPED_OR_DEFAULT(least_request_config.value(), choice_count, 2) - : 2), - active_request_bias_runtime_( - least_request_config.has_value() && least_request_config->has_active_request_bias() - ? std::make_unique(least_request_config->active_request_bias(), - runtime) - : nullptr) { + : 2) { initialize(); } -protected: - void refresh(uint32_t priority) override { - active_request_bias_ = - active_request_bias_runtime_ != nullptr ? active_request_bias_runtime_->value() : 1.0; - - if (active_request_bias_ < 0.0) { - ENVOY_LOG(warn, "upstream: invalid active request bias supplied (runtime key {}), using 1.0", - active_request_bias_runtime_->runtimeKey()); - active_request_bias_ = 1.0; - } - - EdfLoadBalancerBase::refresh(priority); - } - private: void refreshHostSource(const HostsSource&) override {} double hostWeight(const Host& host) override { - // This method is called to calculate the dynamic weight as following when all load balancing - // weights are not equal: - // - // `weight = load_balancing_weight / (active_requests + 1)^active_request_bias` - // - // `active_request_bias` can be configured via runtime and its value is cached in - // `active_request_bias_` to avoid having to do a runtime lookup each time a host weight is - // calculated. - // - // When `active_request_bias == 0.0` we behave like `RoundRobinLoadBalancer` and return the - // host weight without considering the number of active requests at the time we do the pick. - // - // When `active_request_bias > 0.0` we scale the host weight by the number of active - // requests at the time we do the pick. We always add 1 to avoid division by 0. - // - // It might be possible to do better by picking two hosts off of the schedule, and selecting the - // one with fewer active requests at the time of selection. - if (active_request_bias_ == 0.0) { - return host.weight(); - } - - if (active_request_bias_ == 1.0) { - return static_cast(host.weight()) / (host.stats().rq_active_.value() + 1); - } - - return static_cast(host.weight()) / - std::pow(host.stats().rq_active_.value() + 1, active_request_bias_); + // Here we scale host weight by the number of active requests at the time we do the pick. We + // always add 1 to avoid division by 0. It might be possible to do better by picking two hosts + // off of the schedule, and selecting the one with fewer active requests at the time of + // selection. + // TODO(mattklein123): @htuch brings up the point that how we are scaling weight here might not + // be the only/best way of doing this. Essentially, it makes weight and active requests equally + // important. Are they equally important in practice? There is no right answer here and we might + // want to iterate on this as we gain more experience. + return static_cast(host.weight()) / (host.stats().rq_active_.value() + 1); } HostConstSharedPtr unweightedHostPick(const HostVector& hosts_to_use, const HostsSource& source) override; - const uint32_t choice_count_; - - // The exponent used to calculate host weights can be configured via runtime. We cache it for - // performance reasons and refresh it in `LeastRequestLoadBalancer::refresh(uint32_t priority)` - // whenever a `HostSet` is updated. - double active_request_bias_{}; - - const std::unique_ptr active_request_bias_runtime_; }; /** diff --git a/test/common/upstream/load_balancer_impl_test.cc b/test/common/upstream/load_balancer_impl_test.cc index 16f18a0ca192e..ced9ec06ae29b 100644 --- a/test/common/upstream/load_balancer_impl_test.cc +++ b/test/common/upstream/load_balancer_impl_test.cc @@ -13,8 +13,6 @@ #include "test/common/upstream/utility.h" #include "test/mocks/runtime/mocks.h" #include "test/mocks/upstream/mocks.h" -#include "test/test_common/logging.h" -#include "test/test_common/test_runtime.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -1534,89 +1532,6 @@ TEST_P(LeastRequestLoadBalancerTest, WeightImbalance) { EXPECT_EQ(hostSet().healthy_hosts_[0], lb_.chooseHost(nullptr)); } -// Validate that the load balancer defaults to an active request bias value of 1.0 if the runtime -// value is invalid (less than 0.0). -TEST_P(LeastRequestLoadBalancerTest, WeightImbalanceWithInvalidActiveRequestBias) { - envoy::config::cluster::v3::Cluster::LeastRequestLbConfig lr_lb_config; - lr_lb_config.mutable_active_request_bias()->set_runtime_key("ar_bias"); - lr_lb_config.mutable_active_request_bias()->set_default_value(1.0); - LeastRequestLoadBalancer lb_2{priority_set_, nullptr, stats_, runtime_, - random_, common_config_, lr_lb_config}; - - EXPECT_CALL(runtime_.snapshot_, getDouble("ar_bias", 1.0)).WillRepeatedly(Return(-1.0)); - - hostSet().healthy_hosts_ = {makeTestHost(info_, "tcp://127.0.0.1:80", 1), - makeTestHost(info_, "tcp://127.0.0.1:81", 2)}; - - hostSet().hosts_ = hostSet().healthy_hosts_; - - // Trigger callbacks. The added/removed lists are not relevant. - EXPECT_LOG_CONTAINS( - "warn", "upstream: invalid active request bias supplied (runtime key ar_bias), using 1.0", - hostSet().runCallbacks({}, {})); - - EXPECT_CALL(random_, random()).WillRepeatedly(Return(0)); - - // We should see 2:1 ratio for hosts[1] to hosts[0]. - EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); - EXPECT_EQ(hostSet().healthy_hosts_[0], lb_2.chooseHost(nullptr)); - EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); - EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); - EXPECT_EQ(hostSet().healthy_hosts_[0], lb_2.chooseHost(nullptr)); - EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); - - // Bringing hosts[1] to an active request should yield a 1:1 ratio. - hostSet().healthy_hosts_[1]->stats().rq_active_.set(1); - EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); - EXPECT_EQ(hostSet().healthy_hosts_[0], lb_2.chooseHost(nullptr)); - EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); - EXPECT_EQ(hostSet().healthy_hosts_[0], lb_2.chooseHost(nullptr)); - EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); - - // Settings hosts[0] to an active request and hosts[1] to no active requests should yield a 4:1 - // ratio. - hostSet().healthy_hosts_[0]->stats().rq_active_.set(1); - hostSet().healthy_hosts_[1]->stats().rq_active_.set(0); - EXPECT_EQ(hostSet().healthy_hosts_[0], lb_2.chooseHost(nullptr)); - EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); - EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); - EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); - EXPECT_EQ(hostSet().healthy_hosts_[0], lb_2.chooseHost(nullptr)); - EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); - EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); - EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); - EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); - EXPECT_EQ(hostSet().healthy_hosts_[0], lb_2.chooseHost(nullptr)); -} - -TEST_P(LeastRequestLoadBalancerTest, WeightImbalanceWithCustomActiveRequestBias) { - // Create a load balancer with a custom active request bias. - envoy::config::cluster::v3::Cluster::LeastRequestLbConfig lr_lb_config; - lr_lb_config.mutable_active_request_bias()->set_runtime_key("ar_bias"); - lr_lb_config.mutable_active_request_bias()->set_default_value(1.0); - LeastRequestLoadBalancer lb_2{priority_set_, nullptr, stats_, runtime_, - random_, common_config_, lr_lb_config}; - - EXPECT_CALL(runtime_.snapshot_, getDouble("ar_bias", 1.0)).WillRepeatedly(Return(0.0)); - - hostSet().healthy_hosts_ = {makeTestHost(info_, "tcp://127.0.0.1:80", 1), - makeTestHost(info_, "tcp://127.0.0.1:81", 2)}; - - hostSet().hosts_ = hostSet().healthy_hosts_; - hostSet().runCallbacks({}, {}); // Trigger callbacks. The added/removed lists are not relevant. - - EXPECT_CALL(random_, random()).WillRepeatedly(Return(0)); - - // We should see 2:1 ratio for hosts[1] to hosts[0], regardless of the active request count. - hostSet().healthy_hosts_[1]->stats().rq_active_.set(1); - EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); - EXPECT_EQ(hostSet().healthy_hosts_[0], lb_2.chooseHost(nullptr)); - EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); - EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); - EXPECT_EQ(hostSet().healthy_hosts_[0], lb_2.chooseHost(nullptr)); - EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); -} - TEST_P(LeastRequestLoadBalancerTest, WeightImbalanceCallbacks) { hostSet().healthy_hosts_ = {makeTestHost(info_, "tcp://127.0.0.1:80", 1), makeTestHost(info_, "tcp://127.0.0.1:81", 2)}; diff --git a/test/integration/stats_integration_test.cc b/test/integration/stats_integration_test.cc index 008650a236add..8cf76dc866083 100644 --- a/test/integration/stats_integration_test.cc +++ b/test/integration/stats_integration_test.cc @@ -274,7 +274,6 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithFakeSymbolTable) { // 2020/05/20 11223 44491 44600 Add primary clusters tracking to cluster manager. // 2020/06/10 11561 44491 44811 Make upstreams pluggable // 2020/04/23 10661 44425 46000 per-listener connection limits - // 2020/06/30 11252 44747 46000 Introduce Least Request LB active request bias config // Note: when adjusting this value: EXPECT_MEMORY_EQ is active only in CI // 'release' builds, where we control the platform and tool-chain. So you @@ -288,7 +287,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithFakeSymbolTable) { // If you encounter a failure here, please see // https://github.com/envoyproxy/envoy/blob/master/source/docs/stats.md#stats-memory-tests // for details on how to fix. - EXPECT_MEMORY_EQ(m_per_cluster, 44747); + EXPECT_MEMORY_EQ(m_per_cluster, 44491); EXPECT_MEMORY_LE(m_per_cluster, 46000); // Round up to allow platform variations. } @@ -339,7 +338,6 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithRealSymbolTable) { // 2020/05/20 11223 36603 36800 Add primary clusters tracking to cluster manager. // 2020/06/10 11561 36603 36923 Make upstreams pluggable // 2020/04/23 10661 36537 37000 per-listener connection limits - // 2020/06/30 11252 36859 37500 Introduce Least Request LB active request bias config // Note: when adjusting this value: EXPECT_MEMORY_EQ is active only in CI // 'release' builds, where we control the platform and tool-chain. So you @@ -353,8 +351,8 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithRealSymbolTable) { // If you encounter a failure here, please see // https://github.com/envoyproxy/envoy/blob/master/source/docs/stats.md#stats-memory-tests // for details on how to fix. - EXPECT_MEMORY_EQ(m_per_cluster, 36859); - EXPECT_MEMORY_LE(m_per_cluster, 37500); // Round up to allow platform variations. + EXPECT_MEMORY_EQ(m_per_cluster, 36603); + EXPECT_MEMORY_LE(m_per_cluster, 37000); } TEST_P(ClusterMemoryTestRunner, MemoryLargeHostSizeWithStats) { From be7c000329a85ce5603a287b245379ab5fb9de4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Kleiman?= Date: Mon, 6 Jul 2020 13:12:25 -0700 Subject: [PATCH 15/19] Use plain double for active request bias MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use a plain double instead of a runtime double to store the per-cluster active request bias. Note: The goal of this commit is to evaluate the memory overhead of this approach. A commit with te Least Requests LB changes might follow if we deem the memory overhead of this approach acceptable. Signed-off-by: Gastón Kleiman --- api/envoy/config/cluster/v3/cluster.proto | 2 +- api/envoy/config/cluster/v4alpha/cluster.proto | 2 +- generated_api_shadow/envoy/config/cluster/v3/cluster.proto | 2 +- generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/api/envoy/config/cluster/v3/cluster.proto b/api/envoy/config/cluster/v3/cluster.proto index b8cd4d6844a5e..4aec03ef53fa4 100644 --- a/api/envoy/config/cluster/v3/cluster.proto +++ b/api/envoy/config/cluster/v3/cluster.proto @@ -343,7 +343,7 @@ message Cluster { // // .. note:: // This setting only takes effect if all host weights are not equal. - core.v3.RuntimeDouble active_request_bias = 2; + double active_request_bias = 2; } // Specific configuration for the :ref:`RingHash` diff --git a/api/envoy/config/cluster/v4alpha/cluster.proto b/api/envoy/config/cluster/v4alpha/cluster.proto index be00ca338eac7..b3d08e6faaf76 100644 --- a/api/envoy/config/cluster/v4alpha/cluster.proto +++ b/api/envoy/config/cluster/v4alpha/cluster.proto @@ -343,7 +343,7 @@ message Cluster { // // .. note:: // This setting only takes effect if all host weights are not equal. - core.v4alpha.RuntimeDouble active_request_bias = 2; + double active_request_bias = 2; } // Specific configuration for the :ref:`RingHash` diff --git a/generated_api_shadow/envoy/config/cluster/v3/cluster.proto b/generated_api_shadow/envoy/config/cluster/v3/cluster.proto index 2009fb98b71e6..14ca21ba81d0a 100644 --- a/generated_api_shadow/envoy/config/cluster/v3/cluster.proto +++ b/generated_api_shadow/envoy/config/cluster/v3/cluster.proto @@ -343,7 +343,7 @@ message Cluster { // // .. note:: // This setting only takes effect if all host weights are not equal. - core.v3.RuntimeDouble active_request_bias = 2; + double active_request_bias = 2; } // Specific configuration for the :ref:`RingHash` diff --git a/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto b/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto index be00ca338eac7..b3d08e6faaf76 100644 --- a/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto +++ b/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto @@ -343,7 +343,7 @@ message Cluster { // // .. note:: // This setting only takes effect if all host weights are not equal. - core.v4alpha.RuntimeDouble active_request_bias = 2; + double active_request_bias = 2; } // Specific configuration for the :ref:`RingHash` From a4e7b39b3e6142ff5533c4f8eeeb70af96695a1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Kleiman?= Date: Mon, 6 Jul 2020 18:05:12 -0700 Subject: [PATCH 16/19] Revert back to approved implementation using RuntimeDouble MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Gastón Kleiman --- api/envoy/config/cluster/v3/cluster.proto | 2 +- .../config/cluster/v4alpha/cluster.proto | 2 +- .../envoy/config/cluster/v3/cluster.proto | 2 +- .../config/cluster/v4alpha/cluster.proto | 2 +- source/common/upstream/load_balancer_impl.h | 74 +++++++++++++--- .../upstream/load_balancer_impl_test.cc | 85 +++++++++++++++++++ test/integration/stats_integration_test.cc | 8 +- 7 files changed, 156 insertions(+), 19 deletions(-) diff --git a/api/envoy/config/cluster/v3/cluster.proto b/api/envoy/config/cluster/v3/cluster.proto index 4aec03ef53fa4..b8cd4d6844a5e 100644 --- a/api/envoy/config/cluster/v3/cluster.proto +++ b/api/envoy/config/cluster/v3/cluster.proto @@ -343,7 +343,7 @@ message Cluster { // // .. note:: // This setting only takes effect if all host weights are not equal. - double active_request_bias = 2; + core.v3.RuntimeDouble active_request_bias = 2; } // Specific configuration for the :ref:`RingHash` diff --git a/api/envoy/config/cluster/v4alpha/cluster.proto b/api/envoy/config/cluster/v4alpha/cluster.proto index b3d08e6faaf76..be00ca338eac7 100644 --- a/api/envoy/config/cluster/v4alpha/cluster.proto +++ b/api/envoy/config/cluster/v4alpha/cluster.proto @@ -343,7 +343,7 @@ message Cluster { // // .. note:: // This setting only takes effect if all host weights are not equal. - double active_request_bias = 2; + core.v4alpha.RuntimeDouble active_request_bias = 2; } // Specific configuration for the :ref:`RingHash` diff --git a/generated_api_shadow/envoy/config/cluster/v3/cluster.proto b/generated_api_shadow/envoy/config/cluster/v3/cluster.proto index 14ca21ba81d0a..2009fb98b71e6 100644 --- a/generated_api_shadow/envoy/config/cluster/v3/cluster.proto +++ b/generated_api_shadow/envoy/config/cluster/v3/cluster.proto @@ -343,7 +343,7 @@ message Cluster { // // .. note:: // This setting only takes effect if all host weights are not equal. - double active_request_bias = 2; + core.v3.RuntimeDouble active_request_bias = 2; } // Specific configuration for the :ref:`RingHash` diff --git a/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto b/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto index b3d08e6faaf76..be00ca338eac7 100644 --- a/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto +++ b/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto @@ -343,7 +343,7 @@ message Cluster { // // .. note:: // This setting only takes effect if all host weights are not equal. - double active_request_bias = 2; + core.v4alpha.RuntimeDouble active_request_bias = 2; } // Specific configuration for the :ref:`RingHash` diff --git a/source/common/upstream/load_balancer_impl.h b/source/common/upstream/load_balancer_impl.h index 9ef6f9fd8fac8..41a8229845fbc 100644 --- a/source/common/upstream/load_balancer_impl.h +++ b/source/common/upstream/load_balancer_impl.h @@ -1,6 +1,8 @@ #pragma once +#include #include +#include #include #include #include @@ -11,6 +13,7 @@ #include "envoy/upstream/upstream.h" #include "common/protobuf/utility.h" +#include "common/runtime/runtime_protos.h" #include "common/upstream/edf_scheduler.h" namespace Envoy { @@ -367,6 +370,8 @@ class EdfLoadBalancerBase : public ZoneAwareLoadBalancerBase { void initialize(); + virtual void refresh(uint32_t priority); + // Seed to allow us to desynchronize load balancers across a fleet. If we don't // do this, multiple Envoys that receive an update at the same time (or even // multiple load balancers on the same host) will send requests to @@ -375,7 +380,6 @@ class EdfLoadBalancerBase : public ZoneAwareLoadBalancerBase { const uint64_t seed_; private: - void refresh(uint32_t priority); virtual void refreshHostSource(const HostsSource& source) PURE; virtual double hostWeight(const Host& host) PURE; virtual HostConstSharedPtr unweightedHostPick(const HostVector& hosts_to_use, @@ -437,7 +441,8 @@ class RoundRobinLoadBalancer : public EdfLoadBalancerBase { * The benefit of the Maglev table is at the expense of resolution, memory usage is capped. * Additionally, the Maglev table can be shared amongst all threads. */ -class LeastRequestLoadBalancer : public EdfLoadBalancerBase { +class LeastRequestLoadBalancer : public EdfLoadBalancerBase, + Logger::Loggable { public: LeastRequestLoadBalancer( const PrioritySet& priority_set, const PrioritySet* local_priority_set, ClusterStats& stats, @@ -450,26 +455,71 @@ class LeastRequestLoadBalancer : public EdfLoadBalancerBase { choice_count_( least_request_config.has_value() ? PROTOBUF_GET_WRAPPED_OR_DEFAULT(least_request_config.value(), choice_count, 2) - : 2) { + : 2), + active_request_bias_runtime_( + least_request_config.has_value() && least_request_config->has_active_request_bias() + ? std::make_unique(least_request_config->active_request_bias(), + runtime) + : nullptr) { initialize(); } +protected: + void refresh(uint32_t priority) override { + active_request_bias_ = + active_request_bias_runtime_ != nullptr ? active_request_bias_runtime_->value() : 1.0; + + if (active_request_bias_ < 0.0) { + ENVOY_LOG(warn, "upstream: invalid active request bias supplied (runtime key {}), using 1.0", + active_request_bias_runtime_->runtimeKey()); + active_request_bias_ = 1.0; + } + + EdfLoadBalancerBase::refresh(priority); + } + private: void refreshHostSource(const HostsSource&) override {} double hostWeight(const Host& host) override { - // Here we scale host weight by the number of active requests at the time we do the pick. We - // always add 1 to avoid division by 0. It might be possible to do better by picking two hosts - // off of the schedule, and selecting the one with fewer active requests at the time of - // selection. - // TODO(mattklein123): @htuch brings up the point that how we are scaling weight here might not - // be the only/best way of doing this. Essentially, it makes weight and active requests equally - // important. Are they equally important in practice? There is no right answer here and we might - // want to iterate on this as we gain more experience. - return static_cast(host.weight()) / (host.stats().rq_active_.value() + 1); + // This method is called to calculate the dynamic weight as following when all load balancing + // weights are not equal: + // + // `weight = load_balancing_weight / (active_requests + 1)^active_request_bias` + // + // `active_request_bias` can be configured via runtime and its value is cached in + // `active_request_bias_` to avoid having to do a runtime lookup each time a host weight is + // calculated. + // + // When `active_request_bias == 0.0` we behave like `RoundRobinLoadBalancer` and return the + // host weight without considering the number of active requests at the time we do the pick. + // + // When `active_request_bias > 0.0` we scale the host weight by the number of active + // requests at the time we do the pick. We always add 1 to avoid division by 0. + // + // It might be possible to do better by picking two hosts off of the schedule, and selecting the + // one with fewer active requests at the time of selection. + if (active_request_bias_ == 0.0) { + return host.weight(); + } + + if (active_request_bias_ == 1.0) { + return static_cast(host.weight()) / (host.stats().rq_active_.value() + 1); + } + + return static_cast(host.weight()) / + std::pow(host.stats().rq_active_.value() + 1, active_request_bias_); } HostConstSharedPtr unweightedHostPick(const HostVector& hosts_to_use, const HostsSource& source) override; + const uint32_t choice_count_; + + // The exponent used to calculate host weights can be configured via runtime. We cache it for + // performance reasons and refresh it in `LeastRequestLoadBalancer::refresh(uint32_t priority)` + // whenever a `HostSet` is updated. + double active_request_bias_{}; + + const std::unique_ptr active_request_bias_runtime_; }; /** diff --git a/test/common/upstream/load_balancer_impl_test.cc b/test/common/upstream/load_balancer_impl_test.cc index ced9ec06ae29b..16f18a0ca192e 100644 --- a/test/common/upstream/load_balancer_impl_test.cc +++ b/test/common/upstream/load_balancer_impl_test.cc @@ -13,6 +13,8 @@ #include "test/common/upstream/utility.h" #include "test/mocks/runtime/mocks.h" #include "test/mocks/upstream/mocks.h" +#include "test/test_common/logging.h" +#include "test/test_common/test_runtime.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -1532,6 +1534,89 @@ TEST_P(LeastRequestLoadBalancerTest, WeightImbalance) { EXPECT_EQ(hostSet().healthy_hosts_[0], lb_.chooseHost(nullptr)); } +// Validate that the load balancer defaults to an active request bias value of 1.0 if the runtime +// value is invalid (less than 0.0). +TEST_P(LeastRequestLoadBalancerTest, WeightImbalanceWithInvalidActiveRequestBias) { + envoy::config::cluster::v3::Cluster::LeastRequestLbConfig lr_lb_config; + lr_lb_config.mutable_active_request_bias()->set_runtime_key("ar_bias"); + lr_lb_config.mutable_active_request_bias()->set_default_value(1.0); + LeastRequestLoadBalancer lb_2{priority_set_, nullptr, stats_, runtime_, + random_, common_config_, lr_lb_config}; + + EXPECT_CALL(runtime_.snapshot_, getDouble("ar_bias", 1.0)).WillRepeatedly(Return(-1.0)); + + hostSet().healthy_hosts_ = {makeTestHost(info_, "tcp://127.0.0.1:80", 1), + makeTestHost(info_, "tcp://127.0.0.1:81", 2)}; + + hostSet().hosts_ = hostSet().healthy_hosts_; + + // Trigger callbacks. The added/removed lists are not relevant. + EXPECT_LOG_CONTAINS( + "warn", "upstream: invalid active request bias supplied (runtime key ar_bias), using 1.0", + hostSet().runCallbacks({}, {})); + + EXPECT_CALL(random_, random()).WillRepeatedly(Return(0)); + + // We should see 2:1 ratio for hosts[1] to hosts[0]. + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[0], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[0], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + + // Bringing hosts[1] to an active request should yield a 1:1 ratio. + hostSet().healthy_hosts_[1]->stats().rq_active_.set(1); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[0], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[0], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + + // Settings hosts[0] to an active request and hosts[1] to no active requests should yield a 4:1 + // ratio. + hostSet().healthy_hosts_[0]->stats().rq_active_.set(1); + hostSet().healthy_hosts_[1]->stats().rq_active_.set(0); + EXPECT_EQ(hostSet().healthy_hosts_[0], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[0], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[0], lb_2.chooseHost(nullptr)); +} + +TEST_P(LeastRequestLoadBalancerTest, WeightImbalanceWithCustomActiveRequestBias) { + // Create a load balancer with a custom active request bias. + envoy::config::cluster::v3::Cluster::LeastRequestLbConfig lr_lb_config; + lr_lb_config.mutable_active_request_bias()->set_runtime_key("ar_bias"); + lr_lb_config.mutable_active_request_bias()->set_default_value(1.0); + LeastRequestLoadBalancer lb_2{priority_set_, nullptr, stats_, runtime_, + random_, common_config_, lr_lb_config}; + + EXPECT_CALL(runtime_.snapshot_, getDouble("ar_bias", 1.0)).WillRepeatedly(Return(0.0)); + + hostSet().healthy_hosts_ = {makeTestHost(info_, "tcp://127.0.0.1:80", 1), + makeTestHost(info_, "tcp://127.0.0.1:81", 2)}; + + hostSet().hosts_ = hostSet().healthy_hosts_; + hostSet().runCallbacks({}, {}); // Trigger callbacks. The added/removed lists are not relevant. + + EXPECT_CALL(random_, random()).WillRepeatedly(Return(0)); + + // We should see 2:1 ratio for hosts[1] to hosts[0], regardless of the active request count. + hostSet().healthy_hosts_[1]->stats().rq_active_.set(1); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[0], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[0], lb_2.chooseHost(nullptr)); + EXPECT_EQ(hostSet().healthy_hosts_[1], lb_2.chooseHost(nullptr)); +} + TEST_P(LeastRequestLoadBalancerTest, WeightImbalanceCallbacks) { hostSet().healthy_hosts_ = {makeTestHost(info_, "tcp://127.0.0.1:80", 1), makeTestHost(info_, "tcp://127.0.0.1:81", 2)}; diff --git a/test/integration/stats_integration_test.cc b/test/integration/stats_integration_test.cc index 8cf76dc866083..008650a236add 100644 --- a/test/integration/stats_integration_test.cc +++ b/test/integration/stats_integration_test.cc @@ -274,6 +274,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithFakeSymbolTable) { // 2020/05/20 11223 44491 44600 Add primary clusters tracking to cluster manager. // 2020/06/10 11561 44491 44811 Make upstreams pluggable // 2020/04/23 10661 44425 46000 per-listener connection limits + // 2020/06/30 11252 44747 46000 Introduce Least Request LB active request bias config // Note: when adjusting this value: EXPECT_MEMORY_EQ is active only in CI // 'release' builds, where we control the platform and tool-chain. So you @@ -287,7 +288,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithFakeSymbolTable) { // If you encounter a failure here, please see // https://github.com/envoyproxy/envoy/blob/master/source/docs/stats.md#stats-memory-tests // for details on how to fix. - EXPECT_MEMORY_EQ(m_per_cluster, 44491); + EXPECT_MEMORY_EQ(m_per_cluster, 44747); EXPECT_MEMORY_LE(m_per_cluster, 46000); // Round up to allow platform variations. } @@ -338,6 +339,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithRealSymbolTable) { // 2020/05/20 11223 36603 36800 Add primary clusters tracking to cluster manager. // 2020/06/10 11561 36603 36923 Make upstreams pluggable // 2020/04/23 10661 36537 37000 per-listener connection limits + // 2020/06/30 11252 36859 37500 Introduce Least Request LB active request bias config // Note: when adjusting this value: EXPECT_MEMORY_EQ is active only in CI // 'release' builds, where we control the platform and tool-chain. So you @@ -351,8 +353,8 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithRealSymbolTable) { // If you encounter a failure here, please see // https://github.com/envoyproxy/envoy/blob/master/source/docs/stats.md#stats-memory-tests // for details on how to fix. - EXPECT_MEMORY_EQ(m_per_cluster, 36603); - EXPECT_MEMORY_LE(m_per_cluster, 37000); + EXPECT_MEMORY_EQ(m_per_cluster, 36859); + EXPECT_MEMORY_LE(m_per_cluster, 37500); // Round up to allow platform variations. } TEST_P(ClusterMemoryTestRunner, MemoryLargeHostSizeWithStats) { From a6a285dcee9e0fe618286d3dfbfab98c957dd9c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Kleiman?= Date: Wed, 8 Jul 2020 12:32:32 -0700 Subject: [PATCH 17/19] Add extra fields to CDS cluster proto to check memory usage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Gastón Kleiman --- api/envoy/config/cluster/v3/cluster.proto | 10 ++++++++++ api/envoy/config/cluster/v4alpha/cluster.proto | 10 ++++++++++ .../envoy/config/cluster/v3/cluster.proto | 10 ++++++++++ .../envoy/config/cluster/v4alpha/cluster.proto | 10 ++++++++++ 4 files changed, 40 insertions(+) diff --git a/api/envoy/config/cluster/v3/cluster.proto b/api/envoy/config/cluster/v3/cluster.proto index 6123bd59c14e4..44dfd8e16292d 100644 --- a/api/envoy/config/cluster/v3/cluster.proto +++ b/api/envoy/config/cluster/v3/cluster.proto @@ -327,6 +327,7 @@ message Cluster { } // Specific configuration for the LeastRequest load balancing policy. + // [#next-free-field: 7] message LeastRequestLbConfig { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.Cluster.LeastRequestLbConfig"; @@ -359,6 +360,15 @@ message Cluster { // .. note:: // This setting only takes effect if all host weights are not equal. core.v3.RuntimeDouble active_request_bias = 2; + + // Add new fields to see whether the per-cluster memory footprint increases + core.v3.RuntimeDouble foo = 3; + + bool foo1 = 4; + + double foo2 = 5; + + double foo3 = 6; } // Specific configuration for the :ref:`RingHash` diff --git a/api/envoy/config/cluster/v4alpha/cluster.proto b/api/envoy/config/cluster/v4alpha/cluster.proto index 6c1302d28941c..c11f24f83f2ae 100644 --- a/api/envoy/config/cluster/v4alpha/cluster.proto +++ b/api/envoy/config/cluster/v4alpha/cluster.proto @@ -330,6 +330,7 @@ message Cluster { } // Specific configuration for the LeastRequest load balancing policy. + // [#next-free-field: 7] message LeastRequestLbConfig { option (udpa.annotations.versioning).previous_message_type = "envoy.config.cluster.v3.Cluster.LeastRequestLbConfig"; @@ -362,6 +363,15 @@ message Cluster { // .. note:: // This setting only takes effect if all host weights are not equal. core.v4alpha.RuntimeDouble active_request_bias = 2; + + // Add new fields to see whether the per-cluster memory footprint increases + core.v4alpha.RuntimeDouble foo = 3; + + bool foo1 = 4; + + double foo2 = 5; + + double foo3 = 6; } // Specific configuration for the :ref:`RingHash` diff --git a/generated_api_shadow/envoy/config/cluster/v3/cluster.proto b/generated_api_shadow/envoy/config/cluster/v3/cluster.proto index cf6b9cb652b35..4a849f19ea0a6 100644 --- a/generated_api_shadow/envoy/config/cluster/v3/cluster.proto +++ b/generated_api_shadow/envoy/config/cluster/v3/cluster.proto @@ -327,6 +327,7 @@ message Cluster { } // Specific configuration for the LeastRequest load balancing policy. + // [#next-free-field: 7] message LeastRequestLbConfig { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.Cluster.LeastRequestLbConfig"; @@ -359,6 +360,15 @@ message Cluster { // .. note:: // This setting only takes effect if all host weights are not equal. core.v3.RuntimeDouble active_request_bias = 2; + + // Add new fields to see whether the per-cluster memory footprint increases + core.v3.RuntimeDouble foo = 3; + + bool foo1 = 4; + + double foo2 = 5; + + double foo3 = 6; } // Specific configuration for the :ref:`RingHash` diff --git a/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto b/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto index 6c1302d28941c..c11f24f83f2ae 100644 --- a/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto +++ b/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto @@ -330,6 +330,7 @@ message Cluster { } // Specific configuration for the LeastRequest load balancing policy. + // [#next-free-field: 7] message LeastRequestLbConfig { option (udpa.annotations.versioning).previous_message_type = "envoy.config.cluster.v3.Cluster.LeastRequestLbConfig"; @@ -362,6 +363,15 @@ message Cluster { // .. note:: // This setting only takes effect if all host weights are not equal. core.v4alpha.RuntimeDouble active_request_bias = 2; + + // Add new fields to see whether the per-cluster memory footprint increases + core.v4alpha.RuntimeDouble foo = 3; + + bool foo1 = 4; + + double foo2 = 5; + + double foo3 = 6; } // Specific configuration for the :ref:`RingHash` From 2676928d97913351c9ce916a919e759570906ad0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Kleiman?= Date: Mon, 13 Jul 2020 11:28:04 -0700 Subject: [PATCH 18/19] Revert "Add extra fields to CDS cluster proto to check memory usage" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit a6a285dcee9e0fe618286d3dfbfab98c957dd9c7. Signed-off-by: Gastón Kleiman --- api/envoy/config/cluster/v3/cluster.proto | 10 ---------- api/envoy/config/cluster/v4alpha/cluster.proto | 10 ---------- .../envoy/config/cluster/v3/cluster.proto | 10 ---------- .../envoy/config/cluster/v4alpha/cluster.proto | 10 ---------- 4 files changed, 40 deletions(-) diff --git a/api/envoy/config/cluster/v3/cluster.proto b/api/envoy/config/cluster/v3/cluster.proto index 44dfd8e16292d..6123bd59c14e4 100644 --- a/api/envoy/config/cluster/v3/cluster.proto +++ b/api/envoy/config/cluster/v3/cluster.proto @@ -327,7 +327,6 @@ message Cluster { } // Specific configuration for the LeastRequest load balancing policy. - // [#next-free-field: 7] message LeastRequestLbConfig { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.Cluster.LeastRequestLbConfig"; @@ -360,15 +359,6 @@ message Cluster { // .. note:: // This setting only takes effect if all host weights are not equal. core.v3.RuntimeDouble active_request_bias = 2; - - // Add new fields to see whether the per-cluster memory footprint increases - core.v3.RuntimeDouble foo = 3; - - bool foo1 = 4; - - double foo2 = 5; - - double foo3 = 6; } // Specific configuration for the :ref:`RingHash` diff --git a/api/envoy/config/cluster/v4alpha/cluster.proto b/api/envoy/config/cluster/v4alpha/cluster.proto index c11f24f83f2ae..6c1302d28941c 100644 --- a/api/envoy/config/cluster/v4alpha/cluster.proto +++ b/api/envoy/config/cluster/v4alpha/cluster.proto @@ -330,7 +330,6 @@ message Cluster { } // Specific configuration for the LeastRequest load balancing policy. - // [#next-free-field: 7] message LeastRequestLbConfig { option (udpa.annotations.versioning).previous_message_type = "envoy.config.cluster.v3.Cluster.LeastRequestLbConfig"; @@ -363,15 +362,6 @@ message Cluster { // .. note:: // This setting only takes effect if all host weights are not equal. core.v4alpha.RuntimeDouble active_request_bias = 2; - - // Add new fields to see whether the per-cluster memory footprint increases - core.v4alpha.RuntimeDouble foo = 3; - - bool foo1 = 4; - - double foo2 = 5; - - double foo3 = 6; } // Specific configuration for the :ref:`RingHash` diff --git a/generated_api_shadow/envoy/config/cluster/v3/cluster.proto b/generated_api_shadow/envoy/config/cluster/v3/cluster.proto index 4a849f19ea0a6..cf6b9cb652b35 100644 --- a/generated_api_shadow/envoy/config/cluster/v3/cluster.proto +++ b/generated_api_shadow/envoy/config/cluster/v3/cluster.proto @@ -327,7 +327,6 @@ message Cluster { } // Specific configuration for the LeastRequest load balancing policy. - // [#next-free-field: 7] message LeastRequestLbConfig { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.Cluster.LeastRequestLbConfig"; @@ -360,15 +359,6 @@ message Cluster { // .. note:: // This setting only takes effect if all host weights are not equal. core.v3.RuntimeDouble active_request_bias = 2; - - // Add new fields to see whether the per-cluster memory footprint increases - core.v3.RuntimeDouble foo = 3; - - bool foo1 = 4; - - double foo2 = 5; - - double foo3 = 6; } // Specific configuration for the :ref:`RingHash` diff --git a/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto b/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto index c11f24f83f2ae..6c1302d28941c 100644 --- a/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto +++ b/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto @@ -330,7 +330,6 @@ message Cluster { } // Specific configuration for the LeastRequest load balancing policy. - // [#next-free-field: 7] message LeastRequestLbConfig { option (udpa.annotations.versioning).previous_message_type = "envoy.config.cluster.v3.Cluster.LeastRequestLbConfig"; @@ -363,15 +362,6 @@ message Cluster { // .. note:: // This setting only takes effect if all host weights are not equal. core.v4alpha.RuntimeDouble active_request_bias = 2; - - // Add new fields to see whether the per-cluster memory footprint increases - core.v4alpha.RuntimeDouble foo = 3; - - bool foo1 = 4; - - double foo2 = 5; - - double foo3 = 6; } // Specific configuration for the :ref:`RingHash` From eb1b8574d8e465342725611bc94de077c6eaadd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Kleiman?= Date: Tue, 14 Jul 2020 22:15:38 -0700 Subject: [PATCH 19/19] Add changelog entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Gastón Kleiman --- docs/root/version_history/current.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 91b93b797ffa0..4494f39d38226 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -30,6 +30,7 @@ Removed Config or Runtime New Features ------------ * grpc-json: support specifying `response_body` field in for `google.api.HttpBody` message. +* load balancer: added a :ref:`configuration` option to specify the active request bias used by the least request load balancer. * tap: added :ref:`generic body matcher` to scan http requests and responses for text or hex patterns. Deprecated