From 549b93909fb81a038cbe3f6579396ee1ccca3e71 Mon Sep 17 00:00:00 2001 From: wbpcode Date: Wed, 8 Nov 2023 05:17:42 +0000 Subject: [PATCH] runtime: add new runtime guard for previous least request lb patch Signed-off-by: wbpcode --- changelogs/current.yaml | 5 +++++ source/common/runtime/runtime_features.cc | 1 + source/common/upstream/load_balancer_impl.cc | 3 ++- source/common/upstream/load_balancer_impl.h | 10 +++++++-- .../upstream/load_balancer_impl_test.cc | 21 +++++++++++++++++++ 5 files changed, 37 insertions(+), 3 deletions(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 2e952dea71eb6..a018de09a947e 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -41,6 +41,11 @@ minor_behavior_changes: Flip the runtime guard ``envoy.reloadable_features.defer_processing_backedup_streams`` to be on by default. This feature improves flow control within the proxy by deferring work on the receiving end if the other end is backed up. +- area: upstream + change: | + Do full scan on the list of hosts for the least requested load balancer if the number of hosts is smaller + than the choice_count by default. This behavior can be reverted temporarily by setting the runtime flag + ``envoy.reloadable_features.full_scan_if_host_num_less_than_choice_num`` to false. bug_fixes: # *Changes expected to improve the state of the world and are unlikely to have negative effects* diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index ea7a92e65f548..47f9b10a1b54c 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -45,6 +45,7 @@ RUNTIME_GUARD(envoy_reloadable_features_enable_connect_udp_support); RUNTIME_GUARD(envoy_reloadable_features_enable_intermediate_ca); RUNTIME_GUARD(envoy_reloadable_features_enable_zone_routing_different_zone_counts); RUNTIME_GUARD(envoy_reloadable_features_ext_authz_http_send_original_xff); +RUNTIME_GUARD(envoy_reloadable_features_full_scan_if_host_num_less_than_choice_num); RUNTIME_GUARD(envoy_reloadable_features_handle_uppercase_scheme); RUNTIME_GUARD(envoy_reloadable_features_hmac_base64_encoding_only); RUNTIME_GUARD(envoy_reloadable_features_http1_allow_codec_error_response_after_1xx_headers); diff --git a/source/common/upstream/load_balancer_impl.cc b/source/common/upstream/load_balancer_impl.cc index 434c87244888a..7773c2195351d 100644 --- a/source/common/upstream/load_balancer_impl.cc +++ b/source/common/upstream/load_balancer_impl.cc @@ -1301,7 +1301,8 @@ HostConstSharedPtr LeastRequestLoadBalancer::unweightedHostPick(const HostVector // Do full scan if it's required explicitly or the number of choices is equal to or larger than // the hosts size. - if ((hosts_to_use.size() <= choice_count_) || enable_full_scan_) { + if ((full_scan_if_host_num_less_than_choice_num_ && hosts_to_use.size() <= choice_count_) || + enable_full_scan_) { for (const auto& sampled_host : hosts_to_use) { if (candidate_host == nullptr) { // Make a first choice to start the comparisons. diff --git a/source/common/upstream/load_balancer_impl.h b/source/common/upstream/load_balancer_impl.h index c5eeed3916db8..ae0cd9adedbf6 100644 --- a/source/common/upstream/load_balancer_impl.h +++ b/source/common/upstream/load_balancer_impl.h @@ -20,6 +20,7 @@ #include "envoy/upstream/upstream.h" #include "source/common/protobuf/utility.h" +#include "source/common/runtime/runtime_features.h" #include "source/common/runtime/runtime_protos.h" #include "source/common/upstream/edf_scheduler.h" #include "source/common/upstream/subset_lb_config.h" @@ -691,7 +692,9 @@ class LeastRequestLoadBalancer : public EdfLoadBalancerBase { least_request_config.has_value() && least_request_config->has_active_request_bias() ? absl::optional( {least_request_config->active_request_bias(), runtime}) - : absl::nullopt) { + : absl::nullopt), + full_scan_if_host_num_less_than_choice_num_(Runtime::runtimeFeatureEnabled( + "envoy.reloadable_features.full_scan_if_host_num_less_than_choice_num")) { initialize(); } @@ -712,7 +715,9 @@ class LeastRequestLoadBalancer : public EdfLoadBalancerBase { {least_request_config.active_request_bias(), runtime}) : absl::nullopt), enable_full_scan_( - PROTOBUF_GET_WRAPPED_OR_DEFAULT(least_request_config, enable_full_scan, false)) { + PROTOBUF_GET_WRAPPED_OR_DEFAULT(least_request_config, enable_full_scan, false)), + full_scan_if_host_num_less_than_choice_num_(Runtime::runtimeFeatureEnabled( + "envoy.reloadable_features.full_scan_if_host_num_less_than_choice_num")) { initialize(); } @@ -749,6 +754,7 @@ class LeastRequestLoadBalancer : public EdfLoadBalancerBase { const absl::optional active_request_bias_runtime_; const bool enable_full_scan_{}; + const bool full_scan_if_host_num_less_than_choice_num_{}; }; /** diff --git a/test/common/upstream/load_balancer_impl_test.cc b/test/common/upstream/load_balancer_impl_test.cc index c9b572f9b3b30..16dbe63502e52 100644 --- a/test/common/upstream/load_balancer_impl_test.cc +++ b/test/common/upstream/load_balancer_impl_test.cc @@ -2894,6 +2894,27 @@ TEST_P(LeastRequestLoadBalancerTest, PNC) { // random() since we do a full table scan. EXPECT_CALL(random_, random()).WillOnce(Return(9999)); EXPECT_EQ(hostSet().healthy_hosts_[3], lb_6.chooseHost(nullptr)); + + TestScopedRuntime scoped_runtime; + scoped_runtime.mergeValues( + {{"envoy.reloadable_features.full_scan_if_host_num_less_than_choice_num", "false"}}); + lr_lb_config.mutable_choice_count()->set_value(7); + LeastRequestLoadBalancer lb_7{priority_set_, nullptr, stats_, runtime_, + random_, common_config_, lr_lb_config, simTime()}; + + // Host number is smaller than choice number but full scan is disabled, we should still call + // random(). + EXPECT_CALL(random_, random()) + .Times(8) // One more time than the number of choices. + .WillOnce(Return(0)) + .WillOnce(Return(1)) + .WillOnce(Return(2)) + .WillOnce(Return(0)) + .WillOnce(Return(1)) + .WillOnce(Return(2)) + .WillOnce(Return(0)) + .WillOnce(Return(1)); + EXPECT_EQ(hostSet().healthy_hosts_[2], lb_7.chooseHost(nullptr)); } TEST_P(LeastRequestLoadBalancerTest, FullScan) {