diff --git a/WORKSPACE b/WORKSPACE index b04ca558d99..ab08c519b09 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -33,10 +33,10 @@ bind( # 1. Determine SHA256 `wget https://github.com/envoyproxy/envoy/archive/$COMMIT.tar.gz && sha256sum $COMMIT.tar.gz` # 2. Update .bazelversion, envoy.bazelrc and .bazelrc if needed. # -# Commit date: 2023-11-08 -ENVOY_SHA = "c124a78aaa65ffb983ccf34b24c8f1cdf500f137" +# Commit date: 2023-11-10 +ENVOY_SHA = "d3464bfd6d247530e669688c79e99c7245299df7" -ENVOY_SHA256 = "dfb9e67be1d27f7a213fd79aae7c1facdc9683319e8d735a2e50908f632085d1" +ENVOY_SHA256 = "4db83e8af51deec61333686bda5342c3b42c9406c92cb524d1f647e3422dd508" ENVOY_ORG = "envoyproxy" @@ -46,8 +46,6 @@ ENVOY_REPO = "envoy" # persist the option in `user.bazelrc`. http_archive( name = "envoy", - patch_args = ["-p1"], - patches = ["patch.diff"], sha256 = ENVOY_SHA256, strip_prefix = ENVOY_REPO + "-" + ENVOY_SHA, url = "https://github.com/" + ENVOY_ORG + "/" + ENVOY_REPO + "/archive/" + ENVOY_SHA + ".tar.gz", diff --git a/external/patch.diff b/external/patch.diff deleted file mode 100644 index 8bd5f47dbc1..00000000000 --- a/external/patch.diff +++ /dev/null @@ -1,316 +0,0 @@ -commit e58ac8d7dc152d61e2609f8a8e2b6691ec0d765b -Author: Kuat Yessenov -Date: Wed Nov 8 01:45:47 2023 +0000 - - Revert "Fix least request lb not fair (#29873)" - - This reverts commit 3ea2bc40590c1a48f26e8297ae55d7a6d08083e9. - -diff --git a/api/envoy/extensions/load_balancing_policies/least_request/v3/least_request.proto b/api/envoy/extensions/load_balancing_policies/least_request/v3/least_request.proto -index 7be284a4c6..e54ad70d24 100644 ---- a/api/envoy/extensions/load_balancing_policies/least_request/v3/least_request.proto -+++ b/api/envoy/extensions/load_balancing_policies/least_request/v3/least_request.proto -@@ -22,7 +22,6 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; - // This configuration allows the built-in LEAST_REQUEST LB policy to be configured via the LB policy - // extension point. See the :ref:`load balancing architecture overview - // ` for more information. --// [#next-free-field: 6] - message LeastRequest { - // 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. -@@ -59,9 +58,4 @@ message LeastRequest { - - // Configuration for local zone aware load balancing or locality weighted load balancing. - common.v3.LocalityLbConfig locality_lb_config = 4; -- -- // Configuration for performing full scan on the list of hosts. -- // If this configuration is set, when selecting the host a full scan on the list hosts will be -- // used to select the one with least requests instead of using random choices. -- google.protobuf.BoolValue enable_full_scan = 5; - } -diff --git a/changelogs/current.yaml b/changelogs/current.yaml -index 2e952dea71..85bee5be62 100644 ---- a/changelogs/current.yaml -+++ b/changelogs/current.yaml -@@ -21,6 +21,7 @@ behavior_changes: - - minor_behavior_changes: - # *Changes that may cause incompatibilities for some users, but should not for most* -+<<<<<<< HEAD - - area: aws - change: | - uses http async client to fetch the credentials from EC2 instance metadata and ECS task metadata providers instead of libcurl -@@ -31,6 +32,8 @@ minor_behavior_changes: - Request load balancer policy to be unfair when the number of hosts are very small, when the number - of hosts is smaller than the choice_count, instead of randomly selection hosts from the list, we - perform a full scan on it to choose the host with least requests. -+======= -+>>>>>>> parent of 3ea2bc4059 (Fix least request lb not fair (#29873)) - - area: local_rate_limit - change: | - Added new configuration field :ref:`rate_limited_as_resource_exhausted -@@ -103,13 +106,6 @@ new_features: - change: | - Added :ref:`the Basic Auth filter `, which can be used to - authenticate user credentials in the HTTP Authentication heaer defined in `RFC7617 `_. --- area: upstream -- change: | -- Added :ref:`enable_full_scan ` -- option to the least requested load balancer. If set to true, Envoy will perform a full scan on the list of hosts -- instead of using :ref:`choice_count -- ` -- to select the hosts. - - area: stats - change: | - added :ref:`per_endpoint_stats ` to get some metrics -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 f6deaa4968..e99fe65b23 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 -@@ -38,9 +38,7 @@ same or different weights. - approach is nearly as good as an O(N) full scan). This is also known as P2C (power of two - choices). The P2C load balancer has the property that a host with the highest number of active - requests in the cluster will never receive new requests. It will be allowed to drain until it is -- less than or equal to all of the other hosts. The number of hosts chosen can be changed by setting -- ``choice_count``. -- -+ 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. -diff --git a/source/common/upstream/load_balancer_impl.cc b/source/common/upstream/load_balancer_impl.cc -index 434c872448..c85565bfc6 100644 ---- a/source/common/upstream/load_balancer_impl.cc -+++ b/source/common/upstream/load_balancer_impl.cc -@@ -1299,25 +1299,6 @@ HostConstSharedPtr LeastRequestLoadBalancer::unweightedHostPick(const HostVector - const HostsSource&) { - HostSharedPtr candidate_host = nullptr; - -- // 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_) { -- for (const auto& sampled_host : hosts_to_use) { -- if (candidate_host == nullptr) { -- // Make a first choice to start the comparisons. -- candidate_host = sampled_host; -- continue; -- } -- -- const auto candidate_active_rq = candidate_host->stats().rq_active_.value(); -- const auto sampled_active_rq = sampled_host->stats().rq_active_.value(); -- if (sampled_active_rq < candidate_active_rq) { -- candidate_host = sampled_host; -- } -- } -- return candidate_host; -- } -- - for (uint32_t choice_idx = 0; choice_idx < choice_count_; ++choice_idx) { - const int rand_idx = random_.random() % hosts_to_use.size(); - const HostSharedPtr& sampled_host = hosts_to_use[rand_idx]; -diff --git a/source/common/upstream/load_balancer_impl.h b/source/common/upstream/load_balancer_impl.h -index c5eeed3916..6145410577 100644 ---- a/source/common/upstream/load_balancer_impl.h -+++ b/source/common/upstream/load_balancer_impl.h -@@ -710,9 +710,7 @@ public: - least_request_config.has_active_request_bias() - ? absl::optional( - {least_request_config.active_request_bias(), runtime}) -- : absl::nullopt), -- enable_full_scan_( -- PROTOBUF_GET_WRAPPED_OR_DEFAULT(least_request_config, enable_full_scan, false)) { -+ : absl::nullopt) { - initialize(); - } - -@@ -748,7 +746,6 @@ private: - double active_request_bias_{}; - - const absl::optional active_request_bias_runtime_; -- const bool enable_full_scan_{}; - }; - - /** -diff --git a/test/common/upstream/load_balancer_impl_test.cc b/test/common/upstream/load_balancer_impl_test.cc -index c9b572f9b3..518f2a4de1 100644 ---- a/test/common/upstream/load_balancer_impl_test.cc -+++ b/test/common/upstream/load_balancer_impl_test.cc -@@ -2787,20 +2787,20 @@ TEST_P(LeastRequestLoadBalancerTest, SingleHost) { - - // Host weight is 1. - { -- EXPECT_CALL(random_, random()).WillOnce(Return(0)); -+ EXPECT_CALL(random_, random()).WillOnce(Return(0)).WillOnce(Return(2)).WillOnce(Return(3)); - EXPECT_EQ(hostSet().healthy_hosts_[0], lb_.chooseHost(nullptr)); - } - - // Host weight is 100. - { -- EXPECT_CALL(random_, random()).WillOnce(Return(0)); -+ EXPECT_CALL(random_, random()).WillOnce(Return(0)).WillOnce(Return(2)).WillOnce(Return(3)); - EXPECT_EQ(hostSet().healthy_hosts_[0], lb_.chooseHost(nullptr)); - } - - HostVector empty; - { - hostSet().runCallbacks(empty, empty); -- EXPECT_CALL(random_, random()).WillOnce(Return(0)); -+ EXPECT_CALL(random_, random()).WillOnce(Return(0)).WillOnce(Return(2)).WillOnce(Return(3)); - EXPECT_EQ(hostSet().healthy_hosts_[0], lb_.chooseHost(nullptr)); - } - -@@ -2823,12 +2823,12 @@ TEST_P(LeastRequestLoadBalancerTest, Normal) { - - hostSet().healthy_hosts_[0]->stats().rq_active_.set(1); - hostSet().healthy_hosts_[1]->stats().rq_active_.set(2); -- EXPECT_CALL(random_, random()).WillOnce(Return(0)); -+ EXPECT_CALL(random_, random()).WillOnce(Return(0)).WillOnce(Return(2)).WillOnce(Return(3)); - EXPECT_EQ(hostSet().healthy_hosts_[0], lb_.chooseHost(nullptr)); - - hostSet().healthy_hosts_[0]->stats().rq_active_.set(2); - hostSet().healthy_hosts_[1]->stats().rq_active_.set(1); -- EXPECT_CALL(random_, random()).WillOnce(Return(0)); -+ EXPECT_CALL(random_, random()).WillOnce(Return(0)).WillOnce(Return(2)).WillOnce(Return(3)); - EXPECT_EQ(hostSet().healthy_hosts_[1], lb_.chooseHost(nullptr)); - } - -@@ -2836,8 +2836,7 @@ TEST_P(LeastRequestLoadBalancerTest, PNC) { - hostSet().healthy_hosts_ = {makeTestHost(info_, "tcp://127.0.0.1:80", simTime()), - makeTestHost(info_, "tcp://127.0.0.1:81", simTime()), - makeTestHost(info_, "tcp://127.0.0.1:82", simTime()), -- makeTestHost(info_, "tcp://127.0.0.1:83", simTime()), -- makeTestHost(info_, "tcp://127.0.0.1:84", simTime())}; -+ makeTestHost(info_, "tcp://127.0.0.1:83", simTime())}; - hostSet().hosts_ = hostSet().healthy_hosts_; - hostSet().runCallbacks({}, {}); // Trigger callbacks. The added/removed lists are not relevant. - -@@ -2845,22 +2844,16 @@ TEST_P(LeastRequestLoadBalancerTest, PNC) { - hostSet().healthy_hosts_[1]->stats().rq_active_.set(3); - hostSet().healthy_hosts_[2]->stats().rq_active_.set(2); - hostSet().healthy_hosts_[3]->stats().rq_active_.set(1); -- hostSet().healthy_hosts_[4]->stats().rq_active_.set(5); - - // Creating various load balancer objects with different choice configs. - envoy::config::cluster::v3::Cluster::LeastRequestLbConfig lr_lb_config; - lr_lb_config.mutable_choice_count()->set_value(2); - LeastRequestLoadBalancer lb_2{priority_set_, nullptr, stats_, runtime_, - random_, common_config_, lr_lb_config, simTime()}; -- lr_lb_config.mutable_choice_count()->set_value(3); -- LeastRequestLoadBalancer lb_3{priority_set_, nullptr, stats_, runtime_, -- random_, common_config_, lr_lb_config, simTime()}; -- lr_lb_config.mutable_choice_count()->set_value(4); -- LeastRequestLoadBalancer lb_4{priority_set_, nullptr, stats_, runtime_, -- random_, common_config_, lr_lb_config, simTime()}; -- lr_lb_config.mutable_choice_count()->set_value(6); -- LeastRequestLoadBalancer lb_6{priority_set_, nullptr, stats_, runtime_, -+ lr_lb_config.mutable_choice_count()->set_value(5); -+ LeastRequestLoadBalancer lb_5{priority_set_, nullptr, stats_, runtime_, - random_, common_config_, lr_lb_config, simTime()}; -+ - // Verify correct number of choices. - - // 0 choices configured should default to P2C. -@@ -2871,78 +2864,20 @@ TEST_P(LeastRequestLoadBalancerTest, PNC) { - EXPECT_CALL(random_, random()).Times(3).WillRepeatedly(Return(0)); - EXPECT_EQ(hostSet().healthy_hosts_[0], lb_2.chooseHost(nullptr)); - -- // Verify correct host chosen in P3C scenario. -+ // 5 choices configured results in P5C. -+ EXPECT_CALL(random_, random()).Times(6).WillRepeatedly(Return(0)); -+ EXPECT_EQ(hostSet().healthy_hosts_[0], lb_5.chooseHost(nullptr)); -+ -+ // Verify correct host chosen in P5C scenario. - EXPECT_CALL(random_, random()) -- .Times(4) -+ .Times(6) - .WillOnce(Return(0)) - .WillOnce(Return(3)) -- .WillOnce(Return(1)) -- .WillOnce(Return(2)); -- EXPECT_EQ(hostSet().healthy_hosts_[3], lb_3.chooseHost(nullptr)); -- -- // Verify correct host chosen in P4C scenario. -- EXPECT_CALL(random_, random()) -- .Times(5) - .WillOnce(Return(0)) - .WillOnce(Return(3)) -- .WillOnce(Return(1)) -- .WillOnce(Return(1)) -- .WillOnce(Return(2)); -- EXPECT_EQ(hostSet().healthy_hosts_[3], lb_4.chooseHost(nullptr)); -- -- // When the number of hosts is smaller or equal to the number of choices we don't call -- // 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)); --} -- --TEST_P(LeastRequestLoadBalancerTest, FullScan) { -- hostSet().healthy_hosts_ = {makeTestHost(info_, "tcp://127.0.0.1:80", simTime()), -- makeTestHost(info_, "tcp://127.0.0.1:81", simTime()), -- makeTestHost(info_, "tcp://127.0.0.1:82", simTime()), -- makeTestHost(info_, "tcp://127.0.0.1:83", simTime()), -- makeTestHost(info_, "tcp://127.0.0.1:84", simTime())}; -- hostSet().hosts_ = hostSet().healthy_hosts_; -- hostSet().runCallbacks({}, {}); // Trigger callbacks. The added/removed lists are not relevant. -- -- hostSet().healthy_hosts_[0]->stats().rq_active_.set(4); -- hostSet().healthy_hosts_[1]->stats().rq_active_.set(3); -- hostSet().healthy_hosts_[2]->stats().rq_active_.set(2); -- hostSet().healthy_hosts_[3]->stats().rq_active_.set(1); -- hostSet().healthy_hosts_[4]->stats().rq_active_.set(5); -- -- // Creating various load balancer objects with different choice configs. -- envoy::extensions::load_balancing_policies::least_request::v3::LeastRequest lr_lb_config; -- lr_lb_config.mutable_choice_count()->set_value(2); -- // Enable full table scan on hosts -- lr_lb_config.mutable_enable_full_scan()->set_value(true); -- common_config_.mutable_healthy_panic_threshold()->set_value(0); -- -- LeastRequestLoadBalancer lb_2{priority_set_, nullptr, stats_, runtime_, -- random_, 1, lr_lb_config, simTime()}; -- lr_lb_config.mutable_choice_count()->set_value(3); -- LeastRequestLoadBalancer lb_3{priority_set_, nullptr, stats_, runtime_, -- random_, 1, lr_lb_config, simTime()}; -- lr_lb_config.mutable_choice_count()->set_value(4); -- LeastRequestLoadBalancer lb_4{priority_set_, nullptr, stats_, runtime_, -- random_, 1, lr_lb_config, simTime()}; -- lr_lb_config.mutable_choice_count()->set_value(6); -- LeastRequestLoadBalancer lb_6{priority_set_, nullptr, stats_, runtime_, -- random_, 1, lr_lb_config, simTime()}; -- -- // random is called only once every time and is not to select the host. -- -- EXPECT_CALL(random_, random()).WillOnce(Return(9999)); -- EXPECT_EQ(hostSet().healthy_hosts_[3], lb_2.chooseHost(nullptr)); -- -- EXPECT_CALL(random_, random()).WillOnce(Return(9999)); -- EXPECT_EQ(hostSet().healthy_hosts_[3], lb_3.chooseHost(nullptr)); -- -- EXPECT_CALL(random_, random()).WillOnce(Return(9999)); -- EXPECT_EQ(hostSet().healthy_hosts_[3], lb_4.chooseHost(nullptr)); -- -- EXPECT_CALL(random_, random()).WillOnce(Return(9999)); -- EXPECT_EQ(hostSet().healthy_hosts_[3], lb_6.chooseHost(nullptr)); -+ .WillOnce(Return(2)) -+ .WillOnce(Return(1)); -+ EXPECT_EQ(hostSet().healthy_hosts_[3], lb_5.chooseHost(nullptr)); - } - - TEST_P(LeastRequestLoadBalancerTest, WeightImbalance) { -diff --git a/test/integration/http_subset_lb_integration_test.cc b/test/integration/http_subset_lb_integration_test.cc -index bf2969e35b..11707c6248 100644 ---- a/test/integration/http_subset_lb_integration_test.cc -+++ b/test/integration/http_subset_lb_integration_test.cc -@@ -176,10 +176,7 @@ public: - } - } - -- // The default number of choices for the LEAST_REQUEST policy is 2 hosts, if the number of hosts -- // is equal to the number of choices, a full scan happens instead, this means that the same host -- // will be chosen. -- if (is_hash_lb_ || (GetParam() == envoy::config::cluster::v3::Cluster::LEAST_REQUEST)) { -+ if (is_hash_lb_) { - EXPECT_EQ(hosts.size(), 1) << "Expected a single unique host to be selected for " - << envoy::config::cluster::v3::Cluster::LbPolicy_Name(GetParam()); - } else { diff --git a/source/extensions/filters/http/istio_stats/istio_stats.cc b/source/extensions/filters/http/istio_stats/istio_stats.cc index 4e95b25d9af..e59e46bf925 100644 --- a/source/extensions/filters/http/istio_stats/istio_stats.cc +++ b/source/extensions/filters/http/istio_stats/istio_stats.cc @@ -783,10 +783,12 @@ class IstioStatsFilter : public Http::PassThroughFilter, } // AccessLog::Instance - void log(const Http::RequestHeaderMap* request_headers, - const Http::ResponseHeaderMap* response_headers, - const Http::ResponseTrailerMap* response_trailers, const StreamInfo::StreamInfo& info, - AccessLog::AccessLogType) override { + void log(const Formatter::HttpFormatterContext& log_context, + const StreamInfo::StreamInfo& info) override { + const Http::RequestHeaderMap* request_headers = &log_context.requestHeaders(); + const Http::ResponseHeaderMap* response_headers = &log_context.responseHeaders(); + const Http::ResponseTrailerMap* response_trailers = &log_context.responseTrailers(); + reportHelper(true); if (is_grpc_) { tags_.push_back({context_.request_protocol_, context_.grpc_});