Skip to content

edf-lb: internal refactor - convert hostWeight() to const #28464

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
KBaichoo merged 4 commits into from
Jul 27, 2023
Merged

edf-lb: internal refactor - convert hostWeight() to const #28464

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d46d08f
edf-lb: internal refactor - make hostWeight() const
adisuissa Jul 17, 2023
f057abf
Merge remote-tracking branch 'upstream/main' into const_host_weight
adisuissa Jul 19, 2023
4230360
Merge remote-tracking branch 'upstream/main' into const_host_weight
adisuissa Jul 21, 2023
d420dd1
Merge remote-tracking branch 'upstream/main' into const_host_weight
adisuissa Jul 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 15 additions & 13 deletions source/common/upstream/load_balancer_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -922,11 +922,11 @@ void EdfLoadBalancerBase::refresh(uint32_t priority) {
}
}

bool EdfLoadBalancerBase::isSlowStartEnabled() {
bool EdfLoadBalancerBase::isSlowStartEnabled() const {
return slow_start_window_ > std::chrono::milliseconds(0);
}

bool EdfLoadBalancerBase::noHostsAreInSlowStart() {
bool EdfLoadBalancerBase::noHostsAreInSlowStart() const {
if (!isSlowStartEnabled()) {
return true;
}
Expand Down Expand Up @@ -996,35 +996,37 @@ HostConstSharedPtr EdfLoadBalancerBase::chooseHostOnce(LoadBalancerContext* cont
}
}

double EdfLoadBalancerBase::applyAggressionFactor(double time_factor) {
if (aggression_ == 1.0 || time_factor == 1.0) {
namespace {
double applyAggressionFactor(double time_factor, double aggression) {
if (aggression == 1.0 || time_factor == 1.0) {
return time_factor;
} else {
return std::pow(time_factor, 1.0 / aggression_);
return std::pow(time_factor, 1.0 / aggression);
}
}
} // namespace

double EdfLoadBalancerBase::applySlowStartFactor(double host_weight, const Host& host) {
double EdfLoadBalancerBase::applySlowStartFactor(double host_weight, const Host& host) const {
// We can reliably apply slow start weight only if `last_hc_pass_time` in host has been populated
// either by active HC or by `member_update_cb_` in `EdfLoadBalancerBase`.
if (host.lastHcPassTime() && host.coarseHealth() == Upstream::Host::Health::Healthy) {
auto in_healthy_state_duration = std::chrono::duration_cast<std::chrono::milliseconds>(
time_source_.monotonicTime() - host.lastHcPassTime().value());
if (in_healthy_state_duration < slow_start_window_) {
aggression_ =
double aggression =
aggression_runtime_ != absl::nullopt ? aggression_runtime_.value().value() : 1.0;
if (aggression_ <= 0.0 || std::isnan(aggression_)) {
if (aggression <= 0.0 || std::isnan(aggression)) {
ENVOY_LOG_EVERY_POW_2(error, "Invalid runtime value provided for aggression parameter, "
"aggression cannot be less than 0.0");
aggression_ = 1.0;
aggression = 1.0;
}

ASSERT(aggression_ > 0.0);
ASSERT(aggression > 0.0);
auto time_factor = static_cast<double>(std::max(std::chrono::milliseconds(1).count(),
in_healthy_state_duration.count())) /
slow_start_window_.count();
return host_weight *
std::max(applyAggressionFactor(time_factor), slow_start_min_weight_percent_);
return host_weight * std::max(applyAggressionFactor(time_factor, aggression),
slow_start_min_weight_percent_);
} else {
return host_weight;
}
Expand All @@ -1033,7 +1035,7 @@ double EdfLoadBalancerBase::applySlowStartFactor(double host_weight, const Host&
}
}

double LeastRequestLoadBalancer::hostWeight(const Host& host) {
double LeastRequestLoadBalancer::hostWeight(const Host& host) const {
// This method is called to calculate the dynamic weight as following when all load balancing
// weights are not equal:
//
Expand Down
14 changes: 6 additions & 8 deletions source/common/upstream/load_balancer_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -494,8 +494,8 @@ class EdfLoadBalancerBase : public ZoneAwareLoadBalancerBase {

virtual void refresh(uint32_t priority);

bool isSlowStartEnabled();
bool noHostsAreInSlowStart();
bool isSlowStartEnabled() const;
bool noHostsAreInSlowStart() const;

virtual void recalculateHostsInSlowStart(const HostVector& hosts_added);

Expand All @@ -506,13 +506,12 @@ class EdfLoadBalancerBase : public ZoneAwareLoadBalancerBase {
// overload.
const uint64_t seed_;

double applyAggressionFactor(double time_factor);
double applySlowStartFactor(double host_weight, const Host& host);
double applySlowStartFactor(double host_weight, const Host& host) const;

private:
friend class EdfLoadBalancerBasePeer;
virtual void refreshHostSource(const HostsSource& source) PURE;
virtual double hostWeight(const Host& host) PURE;
virtual double hostWeight(const Host& host) const PURE;
virtual HostConstSharedPtr unweightedHostPeek(const HostVector& hosts_to_use,
const HostsSource& source) PURE;
virtual HostConstSharedPtr unweightedHostPick(const HostVector& hosts_to_use,
Expand All @@ -526,7 +525,6 @@ class EdfLoadBalancerBase : public ZoneAwareLoadBalancerBase {
protected:
// Slow start related config
const std::chrono::milliseconds slow_start_window_;
double aggression_{1.0};
const absl::optional<Runtime::Double> aggression_runtime_;
TimeSource& time_source_;
MonotonicTime latest_host_added_time_;
Expand Down Expand Up @@ -580,7 +578,7 @@ class RoundRobinLoadBalancer : public EdfLoadBalancerBase {
// index.
peekahead_index_ = 0;
}
double hostWeight(const Host& host) override {
double hostWeight(const Host& host) const override {
if (!noHostsAreInSlowStart()) {
return applySlowStartFactor(host.weight(), host);
}
Expand Down Expand Up @@ -696,7 +694,7 @@ class LeastRequestLoadBalancer : public EdfLoadBalancerBase {

private:
void refreshHostSource(const HostsSource&) override {}
double hostWeight(const Host& host) override;
double hostWeight(const Host& host) const override;
HostConstSharedPtr unweightedHostPeek(const HostVector& hosts_to_use,
const HostsSource& source) override;
HostConstSharedPtr unweightedHostPick(const HostVector& hosts_to_use,
Expand Down
10 changes: 0 additions & 10 deletions test/common/upstream/load_balancer_impl_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ class EdfLoadBalancerBasePeer {
static const std::chrono::milliseconds& slowStartWindow(EdfLoadBalancerBase& edf_lb) {
return edf_lb.slow_start_window_;
}
static double aggression(EdfLoadBalancerBase& edf_lb) { return edf_lb.aggression_; }
static const std::chrono::milliseconds latestHostAddedTime(EdfLoadBalancerBase& edf_lb) {
return std::chrono::time_point_cast<std::chrono::milliseconds>(edf_lb.latest_host_added_time_)
.time_since_epoch();
Expand Down Expand Up @@ -1617,9 +1616,6 @@ TEST_P(RoundRobinLoadBalancerTest, SlowStartWithDefaultParams) {
const auto slow_start_window =
EdfLoadBalancerBasePeer::slowStartWindow(static_cast<EdfLoadBalancerBase&>(*lb_));
EXPECT_EQ(std::chrono::milliseconds(0), slow_start_window);
const auto aggression =
EdfLoadBalancerBasePeer::aggression(static_cast<EdfLoadBalancerBase&>(*lb_));
EXPECT_EQ(1.0, aggression);
const auto latest_host_added_time =
EdfLoadBalancerBasePeer::latestHostAddedTime(static_cast<EdfLoadBalancerBase&>(*lb_));
EXPECT_EQ(std::chrono::milliseconds(0), latest_host_added_time);
Expand All @@ -1634,9 +1630,6 @@ TEST_P(RoundRobinLoadBalancerTest, SlowStartWithMinWeightPercent) {
const auto slow_start_window =
EdfLoadBalancerBasePeer::slowStartWindow(static_cast<EdfLoadBalancerBase&>(*lb_));
EXPECT_EQ(std::chrono::milliseconds(0), slow_start_window);
const auto aggression =
EdfLoadBalancerBasePeer::aggression(static_cast<EdfLoadBalancerBase&>(*lb_));
EXPECT_EQ(1.0, aggression);
const auto latest_host_added_time =
EdfLoadBalancerBasePeer::latestHostAddedTime(static_cast<EdfLoadBalancerBase&>(*lb_));
EXPECT_EQ(std::chrono::milliseconds(0), latest_host_added_time);
Expand Down Expand Up @@ -2292,9 +2285,6 @@ TEST_P(LeastRequestLoadBalancerTest, SlowStartWithDefaultParams) {
const auto slow_start_window =
EdfLoadBalancerBasePeer::slowStartWindow(static_cast<EdfLoadBalancerBase&>(lb_2));
EXPECT_EQ(std::chrono::milliseconds(0), slow_start_window);
const auto aggression =
EdfLoadBalancerBasePeer::aggression(static_cast<EdfLoadBalancerBase&>(lb_2));
EXPECT_EQ(1.0, aggression);
const auto latest_host_added_time =
EdfLoadBalancerBasePeer::latestHostAddedTime(static_cast<EdfLoadBalancerBase&>(lb_2));
EXPECT_EQ(std::chrono::milliseconds(0), latest_host_added_time);
Expand Down