diff --git a/source/common/upstream/load_balancer_impl.cc b/source/common/upstream/load_balancer_impl.cc index 434c87244888..bfcc451981d9 100644 --- a/source/common/upstream/load_balancer_impl.cc +++ b/source/common/upstream/load_balancer_impl.cc @@ -1299,9 +1299,8 @@ 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_) { + // Do full scan if it's required explicitly. + if (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/test/common/upstream/load_balancer_impl_test.cc b/test/common/upstream/load_balancer_impl_test.cc index c9b572f9b3b3..379f6b082b18 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,29 +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)); + .WillOnce(Return(2)) + .WillOnce(Return(1)); + EXPECT_EQ(hostSet().healthy_hosts_[3], lb_5.chooseHost(nullptr)); } TEST_P(LeastRequestLoadBalancerTest, FullScan) { diff --git a/test/integration/http_subset_lb_integration_test.cc b/test/integration/http_subset_lb_integration_test.cc index bf2969e35bd2..11707c624851 100644 --- a/test/integration/http_subset_lb_integration_test.cc +++ b/test/integration/http_subset_lb_integration_test.cc @@ -176,10 +176,7 @@ class HttpSubsetLbIntegrationTest } } - // 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 {