From d397f0a37678cedccad84b524e39b3e93748e81d Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Thu, 10 Jan 2019 17:44:59 -0500 Subject: [PATCH 1/5] upstream/extension: pass degraded load to plugins, update existing plugin Changes the interface of the retry priority plugin and updates the previous_priorities plugin to account for degraded hosts. The plugin will ignore both healthy and degraded hosts for an attempted priority, to retain the idea that we're excluding the entire priority after it's been attempted. Signed-off-by: Snow Pettersen --- include/envoy/router/router.h | 8 +- include/envoy/upstream/load_balancer.h | 5 +- include/envoy/upstream/retry.h | 10 +- include/envoy/upstream/types.h | 5 + source/common/router/retry_state_impl.h | 8 +- source/common/router/router.h | 4 +- source/common/upstream/load_balancer_impl.cc | 56 +++--- source/common/upstream/load_balancer_impl.h | 19 +- .../previous_priorities.cc | 80 +++++--- .../previous_priorities/previous_priorities.h | 13 +- .../upstream/load_balancer_impl_test.cc | 45 +++-- .../previous_priorities/config_test.cc | 180 ++++++++++++++---- test/mocks/router/mocks.h | 3 +- test/mocks/upstream/load_balancer_context.h | 3 +- test/mocks/upstream/mocks.h | 15 +- 15 files changed, 301 insertions(+), 153 deletions(-) diff --git a/include/envoy/router/router.h b/include/envoy/router/router.h index 4924774f6001..efc3025331a7 100644 --- a/include/envoy/router/router.h +++ b/include/envoy/router/router.h @@ -260,12 +260,12 @@ class RetryState { /** * Returns a reference to the PriorityLoad that should be used for the next retry. * @param priority_set current priority set. - * @param healthy_priority_load original priority load. - * @return PriorityLoad that should be used to select a priority for the next retry. + * @param original_priority_load original priority load. + * @return HealthyAndDegradedLoad that should be used to select a priority for the next retry. */ - virtual const Upstream::HealthyLoad& + virtual const Upstream::HealthyAndDegradedLoad& priorityLoadForRetry(const Upstream::PrioritySet& priority_set, - const Upstream::HealthyLoad& healthy_priority_load) PURE; + const Upstream::HealthyAndDegradedLoad& original_priority_load) PURE; /** * return how many times host selection should be reattempted during host selection. */ diff --git a/include/envoy/upstream/load_balancer.h b/include/envoy/upstream/load_balancer.h index 9e59d140c1ed..6906990074e7 100644 --- a/include/envoy/upstream/load_balancer.h +++ b/include/envoy/upstream/load_balancer.h @@ -55,8 +55,9 @@ class LoadBalancerContext { * @return a reference to the priority load data that should be used to select a priority. * */ - virtual const HealthyLoad& determinePriorityLoad(const PrioritySet& priority_set, - const HealthyLoad& original_priority_load) PURE; + virtual const HealthyAndDegradedLoad& + determinePriorityLoad(const PrioritySet& priority_set, + const HealthyAndDegradedLoad& original_priority_load) PURE; /** * Called to determine whether we should reperform host selection. The load balancer diff --git a/include/envoy/upstream/retry.h b/include/envoy/upstream/retry.h index 02a980c43e99..59af0fce0a22 100644 --- a/include/envoy/upstream/retry.h +++ b/include/envoy/upstream/retry.h @@ -22,11 +22,13 @@ class RetryPriority { * * @param priority_set current priority set of cluster. * @param original_priority the unmodified PriorityLoad. - * @return a reference to the PriorityLoad to use. Return original_priority if no changes should - * be made. + * @param original_degraded_priority the unmodified PriorityLoad. + * @return a pair of PriorityLoads to use, health on left, degraded on right. Return + * a pointer to riginal_priority, original_degraded_priority if no changes should be made. */ - virtual const HealthyLoad& determinePriorityLoad(const PrioritySet& priority_set, - const HealthyLoad& original_priority) PURE; + virtual const HealthyAndDegradedLoad& + determinePriorityLoad(const PrioritySet& priority_set, + const HealthyAndDegradedLoad& original_priority_load) PURE; /** * Called after a host has been attempted but before host selection for the next attempt has diff --git a/include/envoy/upstream/types.h b/include/envoy/upstream/types.h index 150dd137579e..59bd3de65ffa 100644 --- a/include/envoy/upstream/types.h +++ b/include/envoy/upstream/types.h @@ -29,6 +29,11 @@ struct HealthyLoad : PriorityLoad { using PriorityLoad::PriorityLoad; }; +struct HealthyAndDegradedLoad { + HealthyLoad healthy_priority_load_; + DegradedLoad degraded_priority_load_; +}; + // Phantom type indicating that the type is related to host availability. struct Availability {}; diff --git a/source/common/router/retry_state_impl.h b/source/common/router/retry_state_impl.h index 92569e4e3ca4..15a6d0938c77 100644 --- a/source/common/router/retry_state_impl.h +++ b/source/common/router/retry_state_impl.h @@ -54,13 +54,13 @@ class RetryStateImpl : public RetryState { [&host](auto predicate) { return predicate->shouldSelectAnotherHost(host); }); } - const Upstream::HealthyLoad& + const Upstream::HealthyAndDegradedLoad& priorityLoadForRetry(const Upstream::PrioritySet& priority_set, - const Upstream::HealthyLoad& healthy_priority_load) override { + const Upstream::HealthyAndDegradedLoad& original_priority_load) override { if (!retry_priority_) { - return healthy_priority_load; + return original_priority_load; } - return retry_priority_->determinePriorityLoad(priority_set, healthy_priority_load); + return retry_priority_->determinePriorityLoad(priority_set, original_priority_load); } uint32_t hostSelectionMaxAttempts() const override { return host_selection_max_attempts_; } diff --git a/source/common/router/router.h b/source/common/router/router.h index da86f23affc1..291354b0c197 100644 --- a/source/common/router/router.h +++ b/source/common/router/router.h @@ -218,9 +218,9 @@ class Filter : Logger::Loggable, return retry_state_->shouldSelectAnotherHost(host); } - const Upstream::HealthyLoad& + const Upstream::HealthyAndDegradedLoad& determinePriorityLoad(const Upstream::PrioritySet& priority_set, - const Upstream::HealthyLoad& original_priority_load) override { + const Upstream::HealthyAndDegradedLoad& original_priority_load) override { // We only modify the priority load on retries. if (!is_retry_) { return original_priority_load; diff --git a/source/common/upstream/load_balancer_impl.cc b/source/common/upstream/load_balancer_impl.cc index 4b0ffb2d0115..5cb2c360b1ba 100644 --- a/source/common/upstream/load_balancer_impl.cc +++ b/source/common/upstream/load_balancer_impl.cc @@ -90,18 +90,16 @@ LoadBalancerBase::LoadBalancerBase(const PrioritySet& priority_set, ClusterStats common_config, healthy_panic_threshold, 100, 50)), priority_set_(priority_set) { for (auto& host_set : priority_set_.hostSetsPerPriority()) { - recalculatePerPriorityState(host_set->priority(), priority_set_, healthy_per_priority_load_, - degraded_per_priority_load_, per_priority_health_, - per_priority_degraded_); + recalculatePerPriorityState(host_set->priority(), priority_set_, per_priority_load_, + per_priority_health_, per_priority_degraded_); } // Reclaculate panic mode for all levels. recalculatePerPriorityPanic(); priority_set_.addMemberUpdateCb( [this](uint32_t priority, const HostVector&, const HostVector&) -> void { - recalculatePerPriorityState(priority, priority_set_, healthy_per_priority_load_, - degraded_per_priority_load_, per_priority_health_, - per_priority_degraded_); + recalculatePerPriorityState(priority, priority_set_, per_priority_load_, + per_priority_health_, per_priority_degraded_); }); priority_set_.addMemberUpdateCb( [this](uint32_t priority, const HostVector&, const HostVector&) -> void { @@ -124,12 +122,11 @@ LoadBalancerBase::LoadBalancerBase(const PrioritySet& priority_set, ClusterStats void LoadBalancerBase::recalculatePerPriorityState(uint32_t priority, const PrioritySet& priority_set, - HealthyLoad& healthy_per_priority_load, - DegradedLoad& degraded_per_priority_load, + HealthyAndDegradedLoad& per_priority_load, HealthyAvailability& per_priority_health, DegradedAvailability& per_priority_degraded) { - healthy_per_priority_load.get().resize(priority_set.hostSetsPerPriority().size()); - degraded_per_priority_load.get().resize(priority_set.hostSetsPerPriority().size()); + per_priority_load.healthy_priority_load_.get().resize(priority_set.hostSetsPerPriority().size()); + per_priority_load.degraded_priority_load_.get().resize(priority_set.hostSetsPerPriority().size()); per_priority_health.get().resize(priority_set.hostSetsPerPriority().size()); per_priority_degraded.get().resize(priority_set.hostSetsPerPriority().size()); @@ -170,22 +167,23 @@ void LoadBalancerBase::recalculatePerPriorityState(uint32_t priority, if (normalized_total_availability == 0) { // Everything is terrible. Send all load to P=0. // In this one case sumEntries(per_priority_load) != 100 since we sinkhole all traffic in P=0. - healthy_per_priority_load.get()[0] = 100; + per_priority_load.healthy_priority_load_.get()[0] = 100; return; } // We start of with a total load of 100 and distribute it between priorities based on // availability. We first attempt to distribute this load to healthy priorities based on healthy // availability. - const auto first_healthy_and_remaining = distributeLoad( - healthy_per_priority_load, per_priority_health, 100, normalized_total_availability); + const auto first_healthy_and_remaining = + distributeLoad(per_priority_load.healthy_priority_load_, per_priority_health, 100, + normalized_total_availability); // Using the remaining load after allocating load to healthy priorities, distribute it based on // degraded availability. const auto remaining_load_for_degraded = first_healthy_and_remaining.second; const auto first_degraded_and_remaining = - distributeLoad(degraded_per_priority_load, per_priority_degraded, remaining_load_for_degraded, - normalized_total_availability); + distributeLoad(per_priority_load.degraded_priority_load_, per_priority_degraded, + remaining_load_for_degraded, normalized_total_availability); // Anything that remains should just be rounding errors, so allocate that to the first available // priority, either as healthy or degraded. @@ -197,20 +195,20 @@ void LoadBalancerBase::recalculatePerPriorityState(uint32_t priority, // Attempt to allocate the remainder to the first healthy priority first. If no such priority // exist, allocate to the first degraded priority. - ASSERT(remaining_load < - healthy_per_priority_load.get().size() + per_priority_degraded.get().size()); + ASSERT(remaining_load < per_priority_load.healthy_priority_load_.get().size() + + per_priority_load.degraded_priority_load_.get().size()); if (first_healthy != -1) { - healthy_per_priority_load.get()[first_healthy] += remaining_load; + per_priority_load.healthy_priority_load_.get()[first_healthy] += remaining_load; } else { - degraded_per_priority_load.get()[first_degraded] += remaining_load; + per_priority_load.degraded_priority_load_.get()[first_degraded] += remaining_load; } } // The allocated load between healthy and degraded should be exactly 100. - ASSERT(100 == std::accumulate(healthy_per_priority_load.get().begin(), - healthy_per_priority_load.get().end(), 0) + - std::accumulate(degraded_per_priority_load.get().begin(), - degraded_per_priority_load.get().end(), 0)); + ASSERT(100 == std::accumulate(per_priority_load.healthy_priority_load_.get().begin(), + per_priority_load.healthy_priority_load_.get().end(), 0) + + std::accumulate(per_priority_load.degraded_priority_load_.get().begin(), + per_priority_load.degraded_priority_load_.get().end(), 0)); } // Method iterates through priority levels and turns on/off panic mode. @@ -226,7 +224,7 @@ void LoadBalancerBase::recalculatePerPriorityPanic() { if (normalized_total_availability == 0) { // Everything is terrible. All load should be to P=0. Turn on panic mode. - ASSERT(healthy_per_priority_load_.get()[0] == 100); + ASSERT(per_priority_load_.healthy_priority_load_.get()[0] == 100); per_priority_panic_[0] = true; return; } @@ -244,18 +242,18 @@ void LoadBalancerBase::recalculatePerPriorityPanic() { std::pair LoadBalancerBase::chooseHostSet(LoadBalancerContext* context) { if (context) { - const auto& healthy_per_priority_load = - context->determinePriorityLoad(priority_set_, healthy_per_priority_load_); + const auto priority_loads = context->determinePriorityLoad(priority_set_, per_priority_load_); - // TODO(snowp): pass degraded priority load to plugin. const auto priority_and_source = - choosePriority(random_.random(), healthy_per_priority_load, degraded_per_priority_load_); + choosePriority(random_.random(), priority_loads.healthy_priority_load_, + priority_loads.degraded_priority_load_); return {*priority_set_.hostSetsPerPriority()[priority_and_source.first], priority_and_source.second}; } const auto priority_and_source = - choosePriority(random_.random(), healthy_per_priority_load_, degraded_per_priority_load_); + choosePriority(random_.random(), per_priority_load_.healthy_priority_load_, + per_priority_load_.degraded_priority_load_); return {*priority_set_.hostSetsPerPriority()[priority_and_source.first], priority_and_source.second}; } diff --git a/source/common/upstream/load_balancer_impl.h b/source/common/upstream/load_balancer_impl.h index 26c3292d12d8..406b719727da 100644 --- a/source/common/upstream/load_balancer_impl.h +++ b/source/common/upstream/load_balancer_impl.h @@ -73,10 +73,10 @@ class LoadBalancerBase : public LoadBalancer { std::pair chooseHostSet(LoadBalancerContext* context); uint32_t percentageLoad(uint32_t priority) const { - return healthy_per_priority_load_.get()[priority]; + return per_priority_load_.healthy_priority_load_.get()[priority]; } uint32_t percentageDegradedLoad(uint32_t priority) const { - return degraded_per_priority_load_.get()[priority]; + return per_priority_load_.degraded_priority_load_.get()[priority]; } bool isInPanic(uint32_t priority) const { return per_priority_panic_[priority]; } @@ -92,8 +92,7 @@ class LoadBalancerBase : public LoadBalancer { // per_priority_health for that priority level, and may update per_priority_load for all // priority levels. void static recalculatePerPriorityState(uint32_t priority, const PrioritySet& priority_set, - HealthyLoad& healthy_priority_load, - DegradedLoad& degraded_priority_load, + HealthyAndDegradedLoad& priority_load, HealthyAvailability& per_priority_health, DegradedAvailability& per_priority_degraded); void recalculatePerPriorityPanic(); @@ -119,10 +118,9 @@ class LoadBalancerBase : public LoadBalancer { return std::min(health + degraded, 100); } - // The percentage load (0-100) for each priority level when targeting healthy hosts. - HealthyLoad healthy_per_priority_load_; - // The percentage load (0-100) for each priority level when targeting degraded hosts. - DegradedLoad degraded_per_priority_load_; + // The percentage load (0-100) for each priority level when targeting healthy hosts and + // the percentage load (0-100) for each priority level when targeting degraded hosts. + HealthyAndDegradedLoad per_priority_load_; // The health percentage (0-100) for each priority level. HealthyAvailability per_priority_health_; // The degraded percentage (0-100) for each priority level. @@ -141,8 +139,9 @@ class LoadBalancerContextBase : public LoadBalancerContext { const Http::HeaderMap* downstreamHeaders() const override { return nullptr; } - const HealthyLoad& determinePriorityLoad(const PrioritySet&, - const HealthyLoad& original_priority_load) override { + const HealthyAndDegradedLoad& + determinePriorityLoad(const PrioritySet&, + const HealthyAndDegradedLoad& original_priority_load) override { return original_priority_load; } diff --git a/source/extensions/retry/priority/previous_priorities/previous_priorities.cc b/source/extensions/retry/priority/previous_priorities/previous_priorities.cc index 81df24b32af3..80a38a22f94e 100644 --- a/source/extensions/retry/priority/previous_priorities/previous_priorities.cc +++ b/source/extensions/retry/priority/previous_priorities/previous_priorities.cc @@ -4,9 +4,10 @@ namespace Envoy { namespace Extensions { namespace Retry { namespace Priority { -const Upstream::HealthyLoad& PreviousPrioritiesRetryPriority::determinePriorityLoad( + +const Upstream::HealthyAndDegradedLoad& PreviousPrioritiesRetryPriority::determinePriorityLoad( const Upstream::PrioritySet& priority_set, - const Upstream::HealthyLoad& original_priority_load) { + const Upstream::HealthyAndDegradedLoad& original_priority_load) { // If we've not seen enough retries to modify the priority load, just // return the original. // If this retry should trigger an update, recalculate the priority load by excluding attempted @@ -27,7 +28,7 @@ const Upstream::HealthyLoad& PreviousPrioritiesRetryPriority::determinePriorityL } } - return healthy_per_priority_load_; + return per_priority_load_; } bool PreviousPrioritiesRetryPriority::adjustForAttemptedPriorities( @@ -36,41 +37,59 @@ bool PreviousPrioritiesRetryPriority::adjustForAttemptedPriorities( recalculatePerPriorityState(host_set->priority(), priority_set); } - auto adjustedHealthAndSum = adjustedHealth(); - // If there are no healthy priorities left, we reset the attempted priorities and recompute the - // adjusted health. + std::vector adjusted_per_priority_health(per_priority_health_.get().size(), 0); + std::vector adjusted_per_priority_degraded(per_priority_degraded_.get().size(), 0); + auto total_availability = + adjustedAvailability(adjusted_per_priority_health, adjusted_per_priority_degraded); + + // If there are no available priorities left, we reset the attempted priorities and recompute the + // adjusted availability. // This allows us to fall back to the unmodified priority load when we run out of priorities // instead of failing to route requests. - if (adjustedHealthAndSum.second == 0) { + if (total_availability == 0) { for (size_t i = 0; i < excluded_priorities_.size(); ++i) { excluded_priorities_[i] = false; } attempted_priorities_.clear(); - adjustedHealthAndSum = adjustedHealth(); + total_availability = + adjustedAvailability(adjusted_per_priority_health, adjusted_per_priority_degraded); } - const auto& adjusted_per_priority_health = adjustedHealthAndSum.first; - auto total_health = adjustedHealthAndSum.second; - - // If total health is still zero at this point, it must mean that all clusters are - // completely unhealthy. If so, fall back to using the original priority set. This mantains - // whatever handling the default LB uses when all priorities are unhealthy. - if (total_health == 0) { + // If total availability is still zero at this point, it must mean that all clusters are + // completely unavailable. If so, fall back to using the original priority loads. This mantains + // whatever handling the default LB uses when all priorities are unavailable. + if (total_availability == 0) { return false; } - std::fill(healthy_per_priority_load_.get().begin(), healthy_per_priority_load_.get().end(), 0); - // We then adjust the load by rebalancing priorities with the adjusted health values. + std::fill(per_priority_load_.healthy_priority_load_.get().begin(), + per_priority_load_.healthy_priority_load_.get().end(), 0); + std::fill(per_priority_load_.degraded_priority_load_.get().begin(), + per_priority_load_.degraded_priority_load_.get().end(), 0); + + // TODO(snowp): This code is basically distributeLoad from load_balancer_impl.cc, should probably + // reuse that. + + // We then adjust the load by rebalancing priorities with the adjusted availability values. size_t total_load = 100; // The outer loop is used to eliminate rounding errors: any remaining load will be assigned to the - // first healthy priority. + // first availability priority. while (total_load != 0) { for (size_t i = 0; i < adjusted_per_priority_health.size(); ++i) { // Now assign as much load as possible to the high priority levels and cease assigning load // when total_load runs out. - auto delta = - std::min(total_load, adjusted_per_priority_health[i] * 100 / total_health); - healthy_per_priority_load_.get()[i] += delta; + const auto delta = std::min(total_load, adjusted_per_priority_health[i] * 100 / + total_availability); + per_priority_load_.healthy_priority_load_.get()[i] += delta; + total_load -= delta; + } + + for (size_t i = 0; i < adjusted_per_priority_degraded.size(); ++i) { + // Now assign as much load as possible to the high priority levels and cease assigning load + // when total_load runs out. + const auto delta = std::min(total_load, adjusted_per_priority_degraded[i] * 100 / + total_availability); + per_priority_load_.degraded_priority_load_.get()[i] += delta; total_load -= delta; } } @@ -78,20 +97,29 @@ bool PreviousPrioritiesRetryPriority::adjustForAttemptedPriorities( return true; } -std::pair, uint32_t> PreviousPrioritiesRetryPriority::adjustedHealth() const { +uint32_t PreviousPrioritiesRetryPriority::adjustedAvailability( + std::vector& adjusted_per_priority_health, + std::vector& adjusted_per_priority_degraded) const { + // Create an adjusted view of the priorities, where attempted priorities are given a zero load. // Create an adjusted health view of the priorities, where attempted priorities are // given a zero weight. - uint32_t total_health = 0; - std::vector adjusted_per_priority_health(per_priority_health_.get().size(), 0); + uint32_t total_availability = 0; + + ASSERT(per_priority_health_.get().size() == per_priority_degraded_.get().size()); for (size_t i = 0; i < per_priority_health_.get().size(); ++i) { if (!excluded_priorities_[i]) { adjusted_per_priority_health[i] = per_priority_health_.get()[i]; - total_health += per_priority_health_.get()[i]; + adjusted_per_priority_degraded[i] = per_priority_degraded_.get()[i]; + total_availability += per_priority_health_.get()[i]; + total_availability += per_priority_degraded_.get()[i]; + } else { + adjusted_per_priority_health[i] = 0; + adjusted_per_priority_degraded[i] = 0; } } - return {std::move(adjusted_per_priority_health), std::min(total_health, 100u)}; + return std::min(total_availability, 100u); } } // namespace Priority diff --git a/source/extensions/retry/priority/previous_priorities/previous_priorities.h b/source/extensions/retry/priority/previous_priorities/previous_priorities.h index 14bea2875363..ddf6c171acb8 100644 --- a/source/extensions/retry/priority/previous_priorities/previous_priorities.h +++ b/source/extensions/retry/priority/previous_priorities/previous_priorities.h @@ -16,9 +16,9 @@ class PreviousPrioritiesRetryPriority : public Upstream::RetryPriority { attempted_priorities_.reserve(max_retries); } - const Upstream::HealthyLoad& + const Upstream::HealthyAndDegradedLoad& determinePriorityLoad(const Upstream::PrioritySet& priority_set, - const Upstream::HealthyLoad& original_priority) override; + const Upstream::HealthyAndDegradedLoad& original_priority_load) override; void onHostAttempted(Upstream::HostDescriptionConstSharedPtr attempted_host) override { attempted_priorities_.emplace_back(attempted_host->priority()); @@ -28,11 +28,11 @@ class PreviousPrioritiesRetryPriority : public Upstream::RetryPriority { void recalculatePerPriorityState(uint32_t priority, const Upstream::PrioritySet& priority_set) { // Recalcuate health and priority the same way the load balancer does it. Upstream::LoadBalancerBase::recalculatePerPriorityState( - priority, priority_set, healthy_per_priority_load_, degraded_per_priority_load_, - per_priority_health_, per_priority_degraded_); + priority, priority_set, per_priority_load_, per_priority_health_, per_priority_degraded_); } - std::pair, uint32_t> adjustedHealth() const; + uint32_t adjustedAvailability(std::vector& per_priority_health, + std::vector& per_priority_degraded) const; // Distributes priority load between priorities that should be considered after // excluding attempted priorities. @@ -43,8 +43,7 @@ class PreviousPrioritiesRetryPriority : public Upstream::RetryPriority { const uint32_t update_frequency_; std::vector attempted_priorities_; std::vector excluded_priorities_; - Upstream::HealthyLoad healthy_per_priority_load_; - Upstream::DegradedLoad degraded_per_priority_load_; + Upstream::HealthyAndDegradedLoad per_priority_load_; Upstream::HealthyAvailability per_priority_health_; Upstream::DegradedAvailability per_priority_degraded_; }; diff --git a/test/common/upstream/load_balancer_impl_test.cc b/test/common/upstream/load_balancer_impl_test.cc index 4d1e9ce4d1bc..5d37ae7c471d 100644 --- a/test/common/upstream/load_balancer_impl_test.cc +++ b/test/common/upstream/load_balancer_impl_test.cc @@ -118,9 +118,9 @@ TEST_P(LoadBalancerBaseTest, PrioritySelection) { updateHostSet(host_set_, 1 /* num_hosts */, 0 /* num_healthy_hosts */); updateHostSet(failover_host_set_, 1, 0); - HealthyLoad healthy_priority_load({100u, 0u}); - EXPECT_CALL(context, determinePriorityLoad(_, _)) - .WillRepeatedly(ReturnRef(healthy_priority_load)); + HealthyAndDegradedLoad priority_load{Upstream::HealthyLoad({100, 0, 0}), + Upstream::DegradedLoad({0, 0, 0})}; + EXPECT_CALL(context, determinePriorityLoad(_, _)).WillRepeatedly(ReturnRef(priority_load)); // With both the primary and failover hosts unhealthy, we should select an // unhealthy primary host. EXPECT_EQ(100, lb_.percentageLoad(0)); @@ -134,7 +134,7 @@ TEST_P(LoadBalancerBaseTest, PrioritySelection) { EXPECT_EQ(0, lb_.percentageLoad(0)); EXPECT_EQ(0, lb_.percentageLoad(1)); EXPECT_EQ(100, lb_.percentageLoad(2)); - healthy_priority_load = HealthyLoad({0u, 0u, 100u}); + priority_load.healthy_priority_load_ = HealthyLoad({0u, 0u, 100}); EXPECT_EQ(&tertiary_host_set_, &lb_.chooseHostSet(&context).first); // Now add a healthy host in P=0 and make sure it is immediately selected. @@ -143,14 +143,14 @@ TEST_P(LoadBalancerBaseTest, PrioritySelection) { host_set_.runCallbacks({}, {}); EXPECT_EQ(100, lb_.percentageLoad(0)); EXPECT_EQ(0, lb_.percentageLoad(2)); - healthy_priority_load = HealthyLoad({100u, 0u, 0u}); + priority_load.healthy_priority_load_ = HealthyLoad({100u, 0u, 0u}); EXPECT_EQ(&host_set_, &lb_.chooseHostSet(&context).first); // Remove the healthy host and ensure we fail back over to tertiary_host_set_ updateHostSet(host_set_, 1 /* num_hosts */, 0 /* num_healthy_hosts */); EXPECT_EQ(0, lb_.percentageLoad(0)); EXPECT_EQ(100, lb_.percentageLoad(2)); - healthy_priority_load = HealthyLoad({0, 0, 100}); + priority_load.healthy_priority_load_ = HealthyLoad({0u, 0u, 100}); EXPECT_EQ(&tertiary_host_set_, &lb_.chooseHostSet(&context).first); } @@ -190,9 +190,10 @@ TEST_P(LoadBalancerBaseTest, PrioritySelectionFuzz) { } EXPECT_CALL(context, determinePriorityLoad(_, _)) - .WillRepeatedly(Invoke([](const auto&, const auto& original_load) -> const HealthyLoad& { - return original_load; - })); + .WillRepeatedly( + Invoke([](const auto&, const auto& original_load) -> const HealthyAndDegradedLoad& { + return original_load; + })); for (uint64_t i = 0; i < total_hosts; ++i) { const auto hs = lb_.chooseHostSet(&context); @@ -214,15 +215,27 @@ TEST_P(LoadBalancerBaseTest, PrioritySelectionFuzz) { TEST_P(LoadBalancerBaseTest, PrioritySelectionWithFilter) { NiceMock context; - HealthyLoad healthy_priority_load({0u, 100u}); + HealthyAndDegradedLoad priority_load{Upstream::HealthyLoad({0u, 100u}), + Upstream::DegradedLoad({0, 0})}; // return a filter that excludes priority 0 - EXPECT_CALL(context, determinePriorityLoad(_, _)).WillOnce(ReturnRef(healthy_priority_load)); + EXPECT_CALL(context, determinePriorityLoad(_, _)).WillRepeatedly(ReturnRef(priority_load)); updateHostSet(host_set_, 1 /* num_hosts */, 1 /* num_healthy_hosts */); updateHostSet(failover_host_set_, 1, 1); // Since we've excluded P0, we should pick the failover host set EXPECT_EQ(failover_host_set_.priority(), lb_.chooseHostSet(&context).first.priority()); + + updateHostSet(host_set_, 1 /* num_hosts */, 0 /* num_healthy_hosts */, + 1 /* num_degraded_hosts */); + updateHostSet(failover_host_set_, 1, 0, 1); + + // exclude priority 0 for degraded hosts + priority_load.healthy_priority_load_ = Upstream::HealthyLoad({0, 0}); + priority_load.degraded_priority_load_ = Upstream::DegradedLoad({0, 100}); + + // Since we've excluded P0, we should pick the failover host set + EXPECT_EQ(failover_host_set_.priority(), lb_.chooseHostSet(&context).first.priority()); } TEST_P(LoadBalancerBaseTest, OverProvisioningFactor) { @@ -728,15 +741,15 @@ TEST_P(RoundRobinLoadBalancerTest, HostSelectionWithFilter) { .WillRepeatedly(Invoke([&](const Host& host) -> bool { return host.address()->asString() != hostSet().hosts_[0]->address()->asString(); })); - HealthyLoad healthy_priority_load; + HealthyAndDegradedLoad priority_load{Upstream::HealthyLoad({0, 0}), + Upstream::DegradedLoad({0, 0})}; if (GetParam()) { - healthy_priority_load = HealthyLoad({100u, 0u}); + priority_load.healthy_priority_load_ = HealthyLoad({100u, 0u}); } else { - healthy_priority_load = HealthyLoad({0u, 100u}); + priority_load.healthy_priority_load_ = HealthyLoad({0u, 100u}); } - EXPECT_CALL(context, determinePriorityLoad(_, _)) - .WillRepeatedly(ReturnRef(healthy_priority_load)); + EXPECT_CALL(context, determinePriorityLoad(_, _)).WillRepeatedly(ReturnRef(priority_load)); EXPECT_CALL(context, hostSelectionRetryCount()).WillRepeatedly(Return(2)); // Calling chooseHost multiple times always returns host one, since the filter will reject diff --git a/test/extensions/retry/priority/previous_priorities/config_test.cc b/test/extensions/retry/priority/previous_priorities/config_test.cc index 1ab23d3cf7fc..3846af33dc8e 100644 --- a/test/extensions/retry/priority/previous_priorities/config_test.cc +++ b/test/extensions/retry/priority/previous_priorities/config_test.cc @@ -19,7 +19,8 @@ namespace Priority { class RetryPriorityTest : public ::testing::Test { public: - void initialize() { + void initialize(const Upstream::HealthyLoad& original_healthy_priority_load, + const Upstream::DegradedLoad& original_degraded_priority_load) { auto factory = Registry::FactoryRegistry::getFactory( RetryPriorityValues::get().PreviousPrioritiesRetryPriority); @@ -30,26 +31,42 @@ class RetryPriorityTest : public ::testing::Test { auto empty = factory->createEmptyConfigProto(); empty->MergeFrom(config); retry_priority_ = factory->createRetryPriority(*empty, 3); + original_priority_load_ = Upstream::HealthyAndDegradedLoad{original_healthy_priority_load, + original_degraded_priority_load}; } - void addHosts(size_t priority, int count, int healthy_count) { + void addHosts(size_t priority, int count, int healthy_count, int degraded_count = 0) { auto host_set = priority_set_.getMockHostSet(priority); + ASSERT(count >= healthy_count + degraded_count); + host_set->hosts_.resize(count); host_set->healthy_hosts_.resize(healthy_count); + host_set->degraded_hosts_.resize(degraded_count); host_set->runCallbacks({}, {}); } + void verifyPriorityLoads(const Upstream::HealthyLoad& expected_healthy_priority_load, + const Upstream::DegradedLoad& expected_degraded_priority_load) { + const auto& priority_loads = + retry_priority_->determinePriorityLoad(priority_set_, original_priority_load_); + // Unwrapping gives a nicer gtest error. + ASSERT_EQ(priority_loads.healthy_priority_load_.get(), expected_healthy_priority_load.get()); + ASSERT_EQ(priority_loads.degraded_priority_load_.get(), expected_degraded_priority_load.get()); + } + std::vector host_sets_; uint32_t update_frequency_{1}; NiceMock priority_set_; Upstream::RetryPrioritySharedPtr retry_priority_; + Upstream::HealthyAndDegradedLoad original_priority_load_; }; TEST_F(RetryPriorityTest, DefaultFrequency) { - initialize(); + const Upstream::HealthyLoad original_priority_load({100, 0}); + const Upstream::DegradedLoad original_degraded_priority_load({0, 0}); - const Upstream::HealthyLoad original_priority_load({100u, 0u}); + initialize(original_priority_load, original_degraded_priority_load); addHosts(0, 2, 2); addHosts(1, 2, 2); @@ -60,27 +77,27 @@ TEST_F(RetryPriorityTest, DefaultFrequency) { ON_CALL(*host2, priority()).WillByDefault(Return(1)); // Before any hosts attempted, load should be unchanged. - ASSERT_EQ(original_priority_load, - retry_priority_->determinePriorityLoad(priority_set_, original_priority_load)); + verifyPriorityLoads(original_priority_load, original_degraded_priority_load); + + const Upstream::HealthyLoad expected_priority_load({0, 100}); + const Upstream::DegradedLoad expected_degraded_priority_load({0, 0}); - const Upstream::HealthyLoad expected_priority_load({0u, 100u}); // After attempting a host in P0, P1 should receive all the load. retry_priority_->onHostAttempted(host1); - ASSERT_EQ(expected_priority_load, - retry_priority_->determinePriorityLoad(priority_set_, original_priority_load)); + verifyPriorityLoads(expected_priority_load, expected_degraded_priority_load); // After we've tried host2, we've attempted all priorities and should reset back to the original // priority load. retry_priority_->onHostAttempted(host2); - ASSERT_EQ(original_priority_load, - retry_priority_->determinePriorityLoad(priority_set_, original_priority_load)); + verifyPriorityLoads(original_priority_load, original_degraded_priority_load); } // Tests that we handle all hosts being unhealthy in the orignal priority set. TEST_F(RetryPriorityTest, NoHealthyUpstreams) { - initialize(); + const Upstream::HealthyLoad original_priority_load({0, 0, 0}); + const Upstream::DegradedLoad original_degraded_priority_load({0, 0}); - const Upstream::HealthyLoad original_priority_load({0u, 0u, 0u}); + initialize(original_priority_load, original_degraded_priority_load); addHosts(0, 10, 0); addHosts(1, 10, 0); addHosts(2, 10, 0); @@ -95,23 +112,24 @@ TEST_F(RetryPriorityTest, NoHealthyUpstreams) { ON_CALL(*host3, priority()).WillByDefault(Return(2)); // Before any hosts attempted, load should be unchanged. - ASSERT_EQ(original_priority_load, - retry_priority_->determinePriorityLoad(priority_set_, original_priority_load)); + verifyPriorityLoads(original_priority_load, original_degraded_priority_load); { // After attempting a host in P0, load should remain unchanged. - const Upstream::HealthyLoad expected_priority_load({0u, 0u, 0u}); + const Upstream::HealthyLoad expected_priority_load({0, 0, 0}); + const Upstream::DegradedLoad expected_degraded_priority_load({0, 0, 0}); + retry_priority_->onHostAttempted(host1); - ASSERT_EQ(expected_priority_load, - retry_priority_->determinePriorityLoad(priority_set_, original_priority_load)); + verifyPriorityLoads(original_priority_load, original_degraded_priority_load); } } // Tests that spillover happens as we ignore attempted priorities. -TEST_F(RetryPriorityTest, DefaultFrequencyDegradedPriorities) { - initialize(); +TEST_F(RetryPriorityTest, DefaultFrequencyUnhealthyPriorities) { + const Upstream::HealthyLoad original_priority_load({42, 28, 30}); + const Upstream::DegradedLoad original_degraded_priority_load({0, 0, 0}); - const Upstream::HealthyLoad original_priority_load({42u, 28u, 30u}); + initialize(original_priority_load, original_degraded_priority_load); addHosts(0, 10, 3); addHosts(1, 10, 2); addHosts(2, 10, 10); @@ -126,36 +144,120 @@ TEST_F(RetryPriorityTest, DefaultFrequencyDegradedPriorities) { ON_CALL(*host3, priority()).WillByDefault(Return(2)); // Before any hosts attempted, load should be unchanged. - ASSERT_EQ(original_priority_load, - retry_priority_->determinePriorityLoad(priority_set_, original_priority_load)); + verifyPriorityLoads(original_priority_load, original_degraded_priority_load); { // After attempting a host in P0, load should be split between P1 and P2 since P1 is degraded. - const Upstream::HealthyLoad expected_priority_load({0u, 28u, 72u}); + const Upstream::HealthyLoad expected_priority_load({0, 28, 72}); + const Upstream::DegradedLoad expected_degraded_priority_load({0, 0, 0}); retry_priority_->onHostAttempted(host1); - ASSERT_EQ(expected_priority_load, - retry_priority_->determinePriorityLoad(priority_set_, original_priority_load)); + verifyPriorityLoads(expected_priority_load, expected_degraded_priority_load); } // After we've tried host2, everything should go to P2. - const Upstream::HealthyLoad expected_priority_load({0u, 0u, 100u}); + const Upstream::HealthyLoad expected_priority_load({0, 0, 100}); + const Upstream::DegradedLoad expected_degraded_priority_load({0, 0, 0}); retry_priority_->onHostAttempted(host2); - ASSERT_EQ(expected_priority_load, - retry_priority_->determinePriorityLoad(priority_set_, original_priority_load)); + verifyPriorityLoads(expected_priority_load, expected_degraded_priority_load); // Once we've exhausted all priorities, we should return to the originial load. retry_priority_->onHostAttempted(host3); - ASSERT_EQ(original_priority_load, - retry_priority_->determinePriorityLoad(priority_set_, original_priority_load)); + verifyPriorityLoads(original_priority_load, original_degraded_priority_load); +} + +// Tests that spillover happens as we ignore attempted priorities for degraded +// hosts. +TEST_F(RetryPriorityTest, DefaultFrequencyUnhealthyPrioritiesDegradedLoad) { + const Upstream::HealthyLoad original_priority_load({0, 0, 0}); + const Upstream::DegradedLoad original_degraded_priority_load({42, 28, 30}); + + initialize(original_priority_load, original_degraded_priority_load); + addHosts(0, 10, 0, 3); + addHosts(1, 10, 0, 2); + addHosts(2, 10, 0, 10); + + auto host1 = std::make_shared>(); + ON_CALL(*host1, priority()).WillByDefault(Return(0)); + + auto host2 = std::make_shared>(); + ON_CALL(*host2, priority()).WillByDefault(Return(1)); + + auto host3 = std::make_shared>(); + ON_CALL(*host3, priority()).WillByDefault(Return(2)); + + // Before any hosts attempted, load should be unchanged. + verifyPriorityLoads(original_priority_load, original_degraded_priority_load); + + { + // After attempting a host in P0, load should be split between P1 and P2 since P1 is degraded. + const Upstream::HealthyLoad expected_priority_load({0, 0, 0}); + const Upstream::DegradedLoad expected_degraded_priority_load({0, 28, 72}); + retry_priority_->onHostAttempted(host1); + verifyPriorityLoads(expected_priority_load, expected_degraded_priority_load); + } + + // After we've tried host2, everything should go to P2. + const Upstream::HealthyLoad expected_priority_load({0, 0, 0}); + const Upstream::DegradedLoad expected_degraded_priority_load({0, 0, 100}); + retry_priority_->onHostAttempted(host2); + verifyPriorityLoads(expected_priority_load, expected_degraded_priority_load); + + // Once we've exhausted all priorities, we should return to the originial load. + retry_priority_->onHostAttempted(host3); + verifyPriorityLoads(original_priority_load, original_degraded_priority_load); +} + +// Tests that we account for spillover between healthy and degraded priority load. +TEST_F(RetryPriorityTest, DefaultFrequencyUnhealthyPrioritiesDegradedLoadSpillover) { + const Upstream::HealthyLoad original_priority_load({0, 100, 0}); + const Upstream::DegradedLoad original_degraded_priority_load({0, 0, 0}); + + initialize(original_priority_load, original_degraded_priority_load); + addHosts(0, 10, 0, 3); + addHosts(1, 10, 9, 1); + addHosts(2, 10, 2, 0); + + auto host1 = std::make_shared>(); + ON_CALL(*host1, priority()).WillByDefault(Return(0)); + + auto host2 = std::make_shared>(); + ON_CALL(*host2, priority()).WillByDefault(Return(1)); + + auto host3 = std::make_shared>(); + ON_CALL(*host3, priority()).WillByDefault(Return(2)); + + // Before any hosts attempted, load should be unchanged. + verifyPriorityLoads(original_priority_load, original_degraded_priority_load); + + { + // After attempting a host in P1, load should be split between P2 (healthy), + // and P0, P2 (degraded). + const Upstream::HealthyLoad expected_priority_load({0, 0, 40}); + const Upstream::DegradedLoad expected_degraded_priority_load({60, 0, 0}); + retry_priority_->onHostAttempted(host2); + verifyPriorityLoads(expected_priority_load, expected_degraded_priority_load); + } + + // After we've tried host3, everything should go to P0 (degraded). + const Upstream::HealthyLoad expected_priority_load({0, 0, 0}); + const Upstream::DegradedLoad expected_degraded_priority_load({100, 0, 0}); + retry_priority_->onHostAttempted(host3); + verifyPriorityLoads(expected_priority_load, expected_degraded_priority_load); + + // Once we've exhausted all priorities, we should return to the originial load. + retry_priority_->onHostAttempted(host1); + verifyPriorityLoads(original_priority_load, original_degraded_priority_load); } // Tests that we can override the frequency at which we update the priority load with the // update_frequency parameter. TEST_F(RetryPriorityTest, OverridenFrequency) { update_frequency_ = 2; - initialize(); - const Upstream::HealthyLoad original_priority_load({100u, 0u}); + const Upstream::HealthyLoad original_priority_load({100, 0}); + const Upstream::DegradedLoad original_degraded_priority_load({0, 0}); + + initialize(original_priority_load, original_degraded_priority_load); addHosts(0, 2, 2); addHosts(1, 2, 2); @@ -166,19 +268,17 @@ TEST_F(RetryPriorityTest, OverridenFrequency) { ON_CALL(*host2, priority()).WillByDefault(Return(1)); // Before any hosts attempted, load should be unchanged. - ASSERT_EQ(original_priority_load, - retry_priority_->determinePriorityLoad(priority_set_, original_priority_load)); + verifyPriorityLoads(original_priority_load, original_degraded_priority_load); // After attempting a single host in P0, we should leave the priority load unchanged. retry_priority_->onHostAttempted(host1); - ASSERT_EQ(original_priority_load, - retry_priority_->determinePriorityLoad(priority_set_, original_priority_load)); + verifyPriorityLoads(original_priority_load, original_degraded_priority_load); // After a second attempt, the prioity load should change. - const Upstream::HealthyLoad expected_priority_load({0u, 100u}); + const Upstream::HealthyLoad expected_priority_load({0, 100}); + const Upstream::DegradedLoad expected_degraded_priority_load({0, 0}); retry_priority_->onHostAttempted(host1); - ASSERT_EQ(expected_priority_load, - retry_priority_->determinePriorityLoad(priority_set_, original_priority_load)); + verifyPriorityLoads(expected_priority_load, expected_degraded_priority_load); } } // namespace Priority diff --git a/test/mocks/router/mocks.h b/test/mocks/router/mocks.h index 0a511d41e4c5..c07459c63114 100644 --- a/test/mocks/router/mocks.h +++ b/test/mocks/router/mocks.h @@ -100,7 +100,8 @@ class MockRetryState : public RetryState { MOCK_METHOD1(onHostAttempted, void(Upstream::HostDescriptionConstSharedPtr)); MOCK_METHOD1(shouldSelectAnotherHost, bool(const Upstream::Host& host)); MOCK_METHOD2(priorityLoadForRetry, - Upstream::HealthyLoad&(const Upstream::PrioritySet&, const Upstream::HealthyLoad&)); + const Upstream::HealthyAndDegradedLoad&(const Upstream::PrioritySet&, + const Upstream::HealthyAndDegradedLoad&)); MOCK_CONST_METHOD0(hostSelectionMaxAttempts, uint32_t()); DoRetryCallback callback_; diff --git a/test/mocks/upstream/load_balancer_context.h b/test/mocks/upstream/load_balancer_context.h index 674636786a29..578813991fbe 100644 --- a/test/mocks/upstream/load_balancer_context.h +++ b/test/mocks/upstream/load_balancer_context.h @@ -14,7 +14,8 @@ class MockLoadBalancerContext : public LoadBalancerContext { MOCK_METHOD0(metadataMatchCriteria, Router::MetadataMatchCriteria*()); MOCK_CONST_METHOD0(downstreamConnection, const Network::Connection*()); MOCK_CONST_METHOD0(downstreamHeaders, const Http::HeaderMap*()); - MOCK_METHOD2(determinePriorityLoad, const HealthyLoad&(const PrioritySet&, const HealthyLoad&)); + MOCK_METHOD2(determinePriorityLoad, + const HealthyAndDegradedLoad&(const PrioritySet&, const HealthyAndDegradedLoad&)); MOCK_METHOD1(shouldSelectAnotherHost, bool(const Host&)); MOCK_CONST_METHOD0(hostSelectionRetryCount, uint32_t()); }; diff --git a/test/mocks/upstream/mocks.h b/test/mocks/upstream/mocks.h index 6ea2d4bbad1e..f50c11984f6d 100644 --- a/test/mocks/upstream/mocks.h +++ b/test/mocks/upstream/mocks.h @@ -104,20 +104,21 @@ class MockPrioritySet : public PrioritySet { class MockRetryPriority : public RetryPriority { public: - MockRetryPriority(const HealthyLoad& healthy_priority_load) - : healthy_priority_load_(healthy_priority_load) {} - MockRetryPriority(const MockRetryPriority& other) - : healthy_priority_load_(other.healthy_priority_load_) {} + MockRetryPriority(const HealthyLoad& healthy_priority_load, + const DegradedLoad& degraded_priority_load) + : priority_load_({healthy_priority_load, degraded_priority_load}) {} + MockRetryPriority(const MockRetryPriority& other) : priority_load_(other.priority_load_) {} ~MockRetryPriority(); - const HealthyLoad& determinePriorityLoad(const PrioritySet&, const HealthyLoad&) { - return healthy_priority_load_; + const HealthyAndDegradedLoad& determinePriorityLoad(const PrioritySet&, + const HealthyAndDegradedLoad&) { + return priority_load_; } MOCK_METHOD1(onHostAttempted, void(HostDescriptionConstSharedPtr)); private: - const HealthyLoad& healthy_priority_load_; + const HealthyAndDegradedLoad priority_load_; }; class MockRetryPriorityFactory : public RetryPriorityFactory { From a2cb270a1acb1cd34ded40383cf98acd55b1a48f Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Wed, 16 Jan 2019 09:12:58 -0800 Subject: [PATCH 2/5] fix integration test Signed-off-by: Snow Pettersen --- source/common/upstream/thread_aware_lb_impl.cc | 4 ++-- test/integration/http_integration.cc | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/source/common/upstream/thread_aware_lb_impl.cc b/source/common/upstream/thread_aware_lb_impl.cc index 5f8dfb422590..339a9608f8ff 100644 --- a/source/common/upstream/thread_aware_lb_impl.cc +++ b/source/common/upstream/thread_aware_lb_impl.cc @@ -21,8 +21,8 @@ void ThreadAwareLoadBalancerBase::initialize() { void ThreadAwareLoadBalancerBase::refresh() { auto per_priority_state_vector = std::make_shared>( priority_set_.hostSetsPerPriority().size()); - auto healthy_per_priority_load = std::make_shared(healthy_per_priority_load_); - auto degraded_per_priority_load = std::make_shared(degraded_per_priority_load_); + auto healthy_per_priority_load = std::make_shared(per_priority_load_.healthy_priority_load_); + auto degraded_per_priority_load = std::make_shared(per_priority_load_.degraded_priority_load_); for (const auto& host_set : priority_set_.hostSetsPerPriority()) { const uint32_t priority = host_set->priority(); diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index a2da4b69472d..3f19b3434d2e 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -836,7 +836,8 @@ void HttpIntegrationTest::testGrpcRetry() { // healthy. void HttpIntegrationTest::testRetryPriority() { const Upstream::HealthyLoad healthy_priority_load({0u, 100u}); - NiceMock retry_priority(healthy_priority_load); + const Upstream::DegradedLoad degraded_priority_load({0u, 100u}); + NiceMock retry_priority(healthy_priority_load, degraded_priority_load); Upstream::MockRetryPriorityFactory factory(retry_priority); Registry::InjectFactory inject_factory(factory); From 673992a19c5f6808acfe598cbfc060cac794e487 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Wed, 16 Jan 2019 09:14:06 -0800 Subject: [PATCH 3/5] format Signed-off-by: Snow Pettersen --- source/common/upstream/thread_aware_lb_impl.cc | 6 ++++-- test/integration/http_integration.cc | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/source/common/upstream/thread_aware_lb_impl.cc b/source/common/upstream/thread_aware_lb_impl.cc index 339a9608f8ff..9673426a3510 100644 --- a/source/common/upstream/thread_aware_lb_impl.cc +++ b/source/common/upstream/thread_aware_lb_impl.cc @@ -21,8 +21,10 @@ void ThreadAwareLoadBalancerBase::initialize() { void ThreadAwareLoadBalancerBase::refresh() { auto per_priority_state_vector = std::make_shared>( priority_set_.hostSetsPerPriority().size()); - auto healthy_per_priority_load = std::make_shared(per_priority_load_.healthy_priority_load_); - auto degraded_per_priority_load = std::make_shared(per_priority_load_.degraded_priority_load_); + auto healthy_per_priority_load = + std::make_shared(per_priority_load_.healthy_priority_load_); + auto degraded_per_priority_load = + std::make_shared(per_priority_load_.degraded_priority_load_); for (const auto& host_set : priority_set_.hostSetsPerPriority()) { const uint32_t priority = host_set->priority(); diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index 3f19b3434d2e..d647440eba43 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -837,7 +837,8 @@ void HttpIntegrationTest::testGrpcRetry() { void HttpIntegrationTest::testRetryPriority() { const Upstream::HealthyLoad healthy_priority_load({0u, 100u}); const Upstream::DegradedLoad degraded_priority_load({0u, 100u}); - NiceMock retry_priority(healthy_priority_load, degraded_priority_load); + NiceMock retry_priority(healthy_priority_load, + degraded_priority_load); Upstream::MockRetryPriorityFactory factory(retry_priority); Registry::InjectFactory inject_factory(factory); From 3915ad5d9f7a63a22bbe912b70278c8cde54c73c Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Wed, 16 Jan 2019 12:09:37 -0800 Subject: [PATCH 4/5] update comments Signed-off-by: Snow Pettersen --- .../intro/arch_overview/http_connection_management.rst | 2 +- include/envoy/upstream/retry.h | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/root/intro/arch_overview/http_connection_management.rst b/docs/root/intro/arch_overview/http_connection_management.rst index 401ab5bf348e..713594d5d964 100644 --- a/docs/root/intro/arch_overview/http_connection_management.rst +++ b/docs/root/intro/arch_overview/http_connection_management.rst @@ -67,7 +67,7 @@ this behavior retry plugins can be used, which fall into two categories: Envoy supports the following built-in priority predicates * *envoy.retry_priority.previous_priorities*: This will keep track of previously attempted priorities, - and adjust the priority load such that other priorites will be targeted in subsequent retry attempts. + and adjust the priority load such that other priorities will be targeted in subsequent retry attempts. Host selection will continue until either the configured predicates accept the host or a configurable :ref:`max attempts ` has been reached. diff --git a/include/envoy/upstream/retry.h b/include/envoy/upstream/retry.h index 59af0fce0a22..a497e0048536 100644 --- a/include/envoy/upstream/retry.h +++ b/include/envoy/upstream/retry.h @@ -21,10 +21,10 @@ class RetryPriority { * Determines what PriorityLoad to use. * * @param priority_set current priority set of cluster. - * @param original_priority the unmodified PriorityLoad. - * @param original_degraded_priority the unmodified PriorityLoad. - * @return a pair of PriorityLoads to use, health on left, degraded on right. Return - * a pointer to riginal_priority, original_degraded_priority if no changes should be made. + * @param original_priority_load the unmodified HealthAndDegradedLoad. + * @return HealthAndDegradedLoad load that should be used for the next retry. Return + * original_priority_load if the original load should be used. a pointer to riginal_priority, + * original_degraded_priority if no changes should be made. */ virtual const HealthyAndDegradedLoad& determinePriorityLoad(const PrioritySet& priority_set, From 55242d4788a0951d52316df1e0f2fbfc38cf2de4 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Wed, 16 Jan 2019 15:43:11 -0500 Subject: [PATCH 5/5] fix router test Signed-off-by: Snow Pettersen --- test/common/router/config_impl_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index 4324bed87815..bf761ce67e99 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -5296,7 +5296,7 @@ name: RetriableStatusCodes name: envoy.test_retry_priority )EOF"; - Upstream::MockRetryPriority priority({}); + Upstream::MockRetryPriority priority{{}, {}}; Upstream::MockRetryPriorityFactory priority_factory(priority); Registry::InjectFactory inject_priority_factory(priority_factory);