Skip to content

Commit

Permalink
Start Full Scan from a random index for Least Request LB.
Browse files Browse the repository at this point in the history 
Fixed a bug (envoyproxy#11006) that caused the Least Request load balancer policy to choose
the first host of the list when the number of requests are the same during a full scan. Start the selection from a random
index instead of 0.

Signed-off-by: Leonardo da Mata <[email protected]>
  • Loading branch information
@barroca @jkirschner-hashicorp
barroca authored and jkirschner-hashicorp committed Dec 21, 2023
1 parent f21c64e commit 25d41ba
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 17 deletions.
7 changes: 4 additions & 3 deletions api/envoy/extensions/load_balancing_policies/least_request/v3/least_request.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,9 @@ message LeastRequest {
common.v3.LocalityLbConfig locality_lb_config = 4;

// Configuration for performing full scan on the list of hosts.
// If this configuration is set, or if choice_count >= the number of hosts,
// Envoy will use a select the host with the least requests from a full scan rather than
// choice_count random choices.
// 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.
// It uses a random index to start scan to avoid selecting the same host when the number of
// requests are the same.
google.protobuf.BoolValue enable_full_scan = 5;
}
14 changes: 10 additions & 4 deletions source/common/upstream/load_balancer_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1299,10 +1299,15 @@ 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) {
// Use a full scan if:
// - it's explicitly configured, or
// - the number of choices is equal to or larger than the number of hosts.
if (enable_full_scan_ || (hosts_to_use.size() <= choice_count_)) {
// Choose a random index to start from preventing always picking the first host in the list.
const int rand_idx = random_.random() % hosts_to_use.size();
for (unsigned long i = 0; i < hosts_to_use.size(); i++) {
const HostSharedPtr& sampled_host = hosts_to_use[(rand_idx + i) % hosts_to_use.size()];

if (candidate_host == nullptr) {
// Make a first choice to start the comparisons.
candidate_host = sampled_host;
Expand All @@ -1318,6 +1323,7 @@ HostConstSharedPtr LeastRequestLoadBalancer::unweightedHostPick(const HostVector
return candidate_host;
}

// Use random selection (not full scan)
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];
Expand Down
60 changes: 50 additions & 10 deletions test/common/upstream/load_balancer_impl_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2914,7 +2914,7 @@ TEST_P(LeastRequestLoadBalancerTest, FullScan) {
// 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
// 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);

Expand All @@ -2930,21 +2930,61 @@ TEST_P(LeastRequestLoadBalancerTest, FullScan) {
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));
// With full scan we will always choose the host with least number of active requests.
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));
}

TEST_P(LeastRequestLoadBalancerTest, FullScanReturnDifferentHostWhenRequestsAreEqual) {
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(3);
hostSet().healthy_hosts_[1]->stats().rq_active_.set(3);
hostSet().healthy_hosts_[2]->stats().rq_active_.set(3);
hostSet().healthy_hosts_[3]->stats().rq_active_.set(3);
hostSet().healthy_hosts_[4]->stats().rq_active_.set(3);

// 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{priority_set_, nullptr, stats_, runtime_,
random_, 1, lr_lb_config, simTime()};

// random is called every time to choose the first host.
EXPECT_CALL(random_, random())
.WillOnce(Return(0))
.WillOnce(Return(0))
.WillOnce(Return(1))
.WillOnce(Return(1))
.WillOnce(Return(2))
.WillOnce(Return(2))
.WillOnce(Return(3))
.WillOnce(Return(3))
.WillOnce(Return(4))
.WillOnce(Return(4))
.WillOnce(Return(5))
.WillOnce(Return(5));

EXPECT_EQ(hostSet().healthy_hosts_[0], lb.chooseHost(nullptr));
EXPECT_EQ(hostSet().healthy_hosts_[1], lb.chooseHost(nullptr));
EXPECT_EQ(hostSet().healthy_hosts_[2], lb.chooseHost(nullptr));
EXPECT_EQ(hostSet().healthy_hosts_[3], lb.chooseHost(nullptr));
EXPECT_EQ(hostSet().healthy_hosts_[4], lb.chooseHost(nullptr));
EXPECT_EQ(hostSet().healthy_hosts_[0], lb.chooseHost(nullptr));
}

TEST_P(LeastRequestLoadBalancerTest, WeightImbalance) {
hostSet().healthy_hosts_ = {makeTestHost(info_, "tcp://127.0.0.1:80", simTime(), 1),
makeTestHost(info_, "tcp://127.0.0.1:81", simTime(), 2)};
Expand Down

0 comments on commit 25d41ba

Please sign in to comment.