Skip to content

Commit 7711181

Browse files
barrocabarroca
committed
Fix Tests for load_balancer_impl_test
Signed-off-by: Leonardo da Mata <ldamata@spotify.com>
1 parent d9f6c1a commit 7711181

4 files changed

Lines changed: 94 additions & 35 deletions

File tree

‎api/envoy/extensions/load_balancing_policies/least_request/v3/least_request.proto‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
2222
// This configuration allows the built-in LEAST_REQUEST LB policy to be configured via the LB policy
2323
// extension point. See the :ref:`load balancing architecture overview
2424
// <arch_overview_load_balancing_types>` for more information.
25+
// [#next-free-field: 6]
2526
message LeastRequest {
2627
// The number of random healthy hosts from which the host with the fewest active requests will
2728
// be chosen. Defaults to 2 so that we perform two-choice selection if the field is not set.

‎source/common/upstream/load_balancer_impl.cc‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1300,39 +1300,39 @@ HostConstSharedPtr LeastRequestLoadBalancer::unweightedHostPick(const HostVector
13001300
HostSharedPtr candidate_host = nullptr;
13011301

13021302
// We do a full scan if the number of choices is equal to the size.
1303-
if ((hosts_to_use.size() <= choice_count_ )|| full_scan_hosts_) {
1303+
if ((hosts_to_use.size() <= choice_count_) || full_scan_hosts_) {
13041304
for (auto& sampled_host : hosts_to_use) {
1305-
if (candidate_host == nullptr) {
1306-
// Make a first choice to start the comparisons.
1307-
candidate_host = sampled_host;
1308-
continue;
1305+
if (candidate_host == nullptr) {
1306+
// Make a first choice to start the comparisons.
1307+
candidate_host = sampled_host;
1308+
continue;
13091309
}
13101310

13111311
const auto candidate_active_rq = candidate_host->stats().rq_active_.value();
13121312
const auto sampled_active_rq = sampled_host->stats().rq_active_.value();
13131313
if (sampled_active_rq < candidate_active_rq) {
1314-
candidate_host = sampled_host;
1315-
}
1314+
candidate_host = sampled_host;
1315+
}
13161316
}
13171317
return candidate_host;
13181318
}
1319-
1319+
13201320
uint32_t hosts_to_use_current_size = hosts_to_use.size();
1321-
HostVectorSharedPtr hosts( new HostVector(hosts_to_use));
1321+
HostVectorSharedPtr hosts(new HostVector(hosts_to_use));
13221322

13231323
for (uint32_t choice_idx = 0; choice_idx < choice_count_; ++choice_idx) {
13241324
const int rand_idx = random_.random() % hosts_to_use_current_size;
13251325
const HostSharedPtr& sampled_host = hosts->at(rand_idx);
13261326

1327-
// Swap selected host with latest one and skip latest one on next iteration
1327+
// Swap selected host with latest one and skip latest one on next iteration
13281328
// so we don't repeat the selection when there are enough hosts.
13291329
// This will prevent use to choose the same host on our selection since there
13301330
// is a higher chance of selecting the same host when the number of hosts is
13311331
// too big.
1332-
uint32_t last_host_idx = hosts_to_use_current_size-1;
1333-
if ( hosts_to_use.size() > choice_count_ ) {
1332+
uint32_t last_host_idx = hosts_to_use_current_size - 1;
1333+
if (hosts_to_use.size() > choice_count_) {
13341334
--hosts_to_use_current_size;
1335-
1335+
13361336
hosts->at(rand_idx) = hosts->at(last_host_idx);
13371337
}
13381338

‎source/common/upstream/load_balancer_impl.h‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -690,16 +690,15 @@ class LeastRequestLoadBalancer : public EdfLoadBalancerBase {
690690
least_request_config.has_value()
691691
? PROTOBUF_GET_WRAPPED_OR_DEFAULT(least_request_config.ref(), choice_count, 2)
692692
: 2),
693-
693+
694694
active_request_bias_runtime_(
695695
least_request_config.has_value() && least_request_config->has_active_request_bias()
696696
? absl::optional<Runtime::Double>(
697697
{least_request_config->active_request_bias(), runtime})
698698
: absl::nullopt),
699-
full_scan_hosts_(
700-
least_request_config.has_value()
701-
? least_request_config->full_scan_hosts().value()
702-
: false) {
699+
full_scan_hosts_(least_request_config.has_value()
700+
? least_request_config->full_scan_hosts().value()
701+
: false) {
703702
initialize();
704703
}
705704

@@ -720,9 +719,9 @@ class LeastRequestLoadBalancer : public EdfLoadBalancerBase {
720719
? absl::optional<Runtime::Double>(
721720
{least_request_config.active_request_bias(), runtime})
722721
: absl::nullopt),
723-
full_scan_hosts_(
724-
least_request_config.has_full_scan_hosts() ? least_request_config.full_scan_hosts().value()
725-
: false) {
722+
full_scan_hosts_(least_request_config.has_full_scan_hosts()
723+
? least_request_config.full_scan_hosts().value()
724+
: false) {
726725
initialize();
727726
}
728727

‎test/common/upstream/load_balancer_impl_test.cc‎

Lines changed: 73 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2787,20 +2787,20 @@ TEST_P(LeastRequestLoadBalancerTest, SingleHost) {
27872787

27882788
// Host weight is 1.
27892789
{
2790-
EXPECT_CALL(random_, random()).WillOnce(Return(0)).WillOnce(Return(2)).WillOnce(Return(3));
2790+
EXPECT_CALL(random_, random()).WillOnce(Return(9999));
27912791
EXPECT_EQ(hostSet().healthy_hosts_[0], lb_.chooseHost(nullptr));
27922792
}
27932793

27942794
// Host weight is 100.
27952795
{
2796-
EXPECT_CALL(random_, random()).WillOnce(Return(0)).WillOnce(Return(2)).WillOnce(Return(3));
2796+
EXPECT_CALL(random_, random()).WillOnce(Return(9999));
27972797
EXPECT_EQ(hostSet().healthy_hosts_[0], lb_.chooseHost(nullptr));
27982798
}
27992799

28002800
HostVector empty;
28012801
{
28022802
hostSet().runCallbacks(empty, empty);
2803-
EXPECT_CALL(random_, random()).WillOnce(Return(0)).WillOnce(Return(2)).WillOnce(Return(3));
2803+
EXPECT_CALL(random_, random()).WillOnce(Return(9999));
28042804
EXPECT_EQ(hostSet().healthy_hosts_[0], lb_.chooseHost(nullptr));
28052805
}
28062806

@@ -2810,7 +2810,7 @@ TEST_P(LeastRequestLoadBalancerTest, SingleHost) {
28102810
hostSet().healthy_hosts_.clear();
28112811
hostSet().hosts_.clear();
28122812
hostSet().runCallbacks(empty, remove_hosts);
2813-
EXPECT_CALL(random_, random()).WillOnce(Return(0));
2813+
EXPECT_CALL(random_, random()).WillOnce(Return(9999));
28142814
EXPECT_EQ(nullptr, lb_.chooseHost(nullptr));
28152815
}
28162816
}
@@ -2823,27 +2823,29 @@ TEST_P(LeastRequestLoadBalancerTest, Normal) {
28232823

28242824
hostSet().healthy_hosts_[0]->stats().rq_active_.set(1);
28252825
hostSet().healthy_hosts_[1]->stats().rq_active_.set(2);
2826-
EXPECT_CALL(random_, random()).WillOnce(Return(0)).WillOnce(Return(2)).WillOnce(Return(3));
2826+
EXPECT_CALL(random_, random()).WillOnce(Return(0));
28272827
EXPECT_EQ(hostSet().healthy_hosts_[0], lb_.chooseHost(nullptr));
28282828

28292829
hostSet().healthy_hosts_[0]->stats().rq_active_.set(2);
28302830
hostSet().healthy_hosts_[1]->stats().rq_active_.set(1);
2831-
EXPECT_CALL(random_, random()).WillOnce(Return(0)).WillOnce(Return(2)).WillOnce(Return(3));
2831+
EXPECT_CALL(random_, random()).WillOnce(Return(0));
28322832
EXPECT_EQ(hostSet().healthy_hosts_[1], lb_.chooseHost(nullptr));
28332833
}
28342834

28352835
TEST_P(LeastRequestLoadBalancerTest, PNC) {
28362836
hostSet().healthy_hosts_ = {makeTestHost(info_, "tcp://127.0.0.1:80", simTime()),
28372837
makeTestHost(info_, "tcp://127.0.0.1:81", simTime()),
28382838
makeTestHost(info_, "tcp://127.0.0.1:82", simTime()),
2839-
makeTestHost(info_, "tcp://127.0.0.1:83", simTime())};
2839+
makeTestHost(info_, "tcp://127.0.0.1:83", simTime()),
2840+
makeTestHost(info_, "tcp://127.0.0.1:84", simTime())};
28402841
hostSet().hosts_ = hostSet().healthy_hosts_;
28412842
hostSet().runCallbacks({}, {}); // Trigger callbacks. The added/removed lists are not relevant.
28422843

28432844
hostSet().healthy_hosts_[0]->stats().rq_active_.set(4);
28442845
hostSet().healthy_hosts_[1]->stats().rq_active_.set(3);
28452846
hostSet().healthy_hosts_[2]->stats().rq_active_.set(2);
28462847
hostSet().healthy_hosts_[3]->stats().rq_active_.set(1);
2848+
hostSet().healthy_hosts_[4]->stats().rq_active_.set(5);
28472849

28482850
// Creating various load balancer objects with different choice configs.
28492851
envoy::config::cluster::v3::Cluster::LeastRequestLbConfig lr_lb_config;
@@ -2856,7 +2858,9 @@ TEST_P(LeastRequestLoadBalancerTest, PNC) {
28562858
lr_lb_config.mutable_choice_count()->set_value(4);
28572859
LeastRequestLoadBalancer lb_4{priority_set_, nullptr, stats_, runtime_,
28582860
random_, common_config_, lr_lb_config, simTime()};
2859-
2861+
lr_lb_config.mutable_choice_count()->set_value(6);
2862+
LeastRequestLoadBalancer lb_6{priority_set_, nullptr, stats_, runtime_,
2863+
random_, common_config_, lr_lb_config, simTime()};
28602864
// Verify correct number of choices.
28612865

28622866
// 0 choices configured should default to P2C.
@@ -2867,21 +2871,76 @@ TEST_P(LeastRequestLoadBalancerTest, PNC) {
28672871
EXPECT_CALL(random_, random()).Times(3).WillRepeatedly(Return(0));
28682872
EXPECT_EQ(hostSet().healthy_hosts_[3], lb_2.chooseHost(nullptr));
28692873

2870-
28712874
// Verify correct host chosen in P3C scenario.
28722875
EXPECT_CALL(random_, random())
2873-
.Times(3)
2876+
.Times(4)
28742877
.WillOnce(Return(0))
28752878
.WillOnce(Return(3))
2876-
.WillOnce(Return(1));
2877-
EXPECT_EQ(hostSet().healthy_hosts_[3], lb_3.chooseHost(nullptr));
2879+
.WillOnce(Return(1))
2880+
.WillOnce(Return(9999));
2881+
EXPECT_EQ(hostSet().healthy_hosts_[2], lb_3.chooseHost(nullptr));
28782882

2883+
// Verify correct host chosen in P4C scenario.
2884+
EXPECT_CALL(random_, random())
2885+
.Times(5)
2886+
.WillOnce(Return(0))
2887+
.WillOnce(Return(3))
2888+
.WillOnce(Return(1))
2889+
.WillOnce(Return(1))
2890+
.WillOnce(Return(9999));
2891+
EXPECT_EQ(hostSet().healthy_hosts_[2], lb_4.chooseHost(nullptr));
28792892

28802893
// When the number of hosts is smaller or equal to the number of choices we don't call
28812894
// random() since we do a full table scan.
2882-
EXPECT_CALL(random_, random()).Times(0);
2883-
EXPECT_EQ(hostSet().healthy_hosts_[0], lb_4.chooseHost(nullptr));
2895+
EXPECT_CALL(random_, random()).WillOnce(Return(9999));
2896+
EXPECT_EQ(hostSet().healthy_hosts_[3], lb_6.chooseHost(nullptr));
2897+
}
2898+
2899+
TEST_P(LeastRequestLoadBalancerTest, FullScan) {
2900+
hostSet().healthy_hosts_ = {makeTestHost(info_, "tcp://127.0.0.1:80", simTime()),
2901+
makeTestHost(info_, "tcp://127.0.0.1:81", simTime()),
2902+
makeTestHost(info_, "tcp://127.0.0.1:82", simTime()),
2903+
makeTestHost(info_, "tcp://127.0.0.1:83", simTime()),
2904+
makeTestHost(info_, "tcp://127.0.0.1:84", simTime())};
2905+
hostSet().hosts_ = hostSet().healthy_hosts_;
2906+
hostSet().runCallbacks({}, {}); // Trigger callbacks. The added/removed lists are not relevant.
2907+
2908+
hostSet().healthy_hosts_[0]->stats().rq_active_.set(4);
2909+
hostSet().healthy_hosts_[1]->stats().rq_active_.set(3);
2910+
hostSet().healthy_hosts_[2]->stats().rq_active_.set(2);
2911+
hostSet().healthy_hosts_[3]->stats().rq_active_.set(1);
2912+
hostSet().healthy_hosts_[4]->stats().rq_active_.set(5);
2913+
2914+
// Creating various load balancer objects with different choice configs.
2915+
envoy::config::cluster::v3::Cluster::LeastRequestLbConfig lr_lb_config;
2916+
lr_lb_config.mutable_choice_count()->set_value(2);
2917+
// Enable full table scan on hosts
2918+
lr_lb_config.mutable_full_scan_hosts()->set_value(true);
2919+
LeastRequestLoadBalancer lb_2{priority_set_, nullptr, stats_, runtime_,
2920+
random_, common_config_, lr_lb_config, simTime()};
2921+
lr_lb_config.mutable_choice_count()->set_value(3);
2922+
LeastRequestLoadBalancer lb_3{priority_set_, nullptr, stats_, runtime_,
2923+
random_, common_config_, lr_lb_config, simTime()};
2924+
lr_lb_config.mutable_choice_count()->set_value(4);
2925+
LeastRequestLoadBalancer lb_4{priority_set_, nullptr, stats_, runtime_,
2926+
random_, common_config_, lr_lb_config, simTime()};
2927+
lr_lb_config.mutable_choice_count()->set_value(6);
2928+
LeastRequestLoadBalancer lb_6{priority_set_, nullptr, stats_, runtime_,
2929+
random_, common_config_, lr_lb_config, simTime()};
2930+
2931+
// random is called only once everytime and is not to select the host.
2932+
2933+
EXPECT_CALL(random_, random()).WillOnce(Return(9999));
2934+
EXPECT_EQ(hostSet().healthy_hosts_[3], lb_2.chooseHost(nullptr));
2935+
2936+
EXPECT_CALL(random_, random()).WillOnce(Return(9999));
2937+
EXPECT_EQ(hostSet().healthy_hosts_[3], lb_3.chooseHost(nullptr));
2938+
2939+
EXPECT_CALL(random_, random()).WillOnce(Return(9999));
2940+
EXPECT_EQ(hostSet().healthy_hosts_[3], lb_4.chooseHost(nullptr));
28842941

2942+
EXPECT_CALL(random_, random()).WillOnce(Return(9999));
2943+
EXPECT_EQ(hostSet().healthy_hosts_[3], lb_6.chooseHost(nullptr));
28852944
}
28862945

28872946
TEST_P(LeastRequestLoadBalancerTest, WeightImbalance) {

0 commit comments

Comments
 (0)