From 842d5fa2b0934e01f1db17af1eeab0ad5c08605c Mon Sep 17 00:00:00 2001 From: Alex Konradi Date: Mon, 11 Jan 2021 17:37:39 -0500 Subject: [PATCH 1/9] Move timer type map into ScaledRangeTimerManager Make ScaledRangeTimerManager the ultimate arbiter for determining the timer minimum for a given timer type. Signed-off-by: Alex Konradi --- .../envoy/event/scaled_range_timer_manager.h | 17 +++++ include/envoy/event/scaled_timer_minimum.h | 23 +++++-- include/envoy/server/overload/BUILD | 1 + .../overload/thread_local_overload_state.h | 14 +--- .../event/scaled_range_timer_manager_impl.cc | 17 ++++- .../event/scaled_range_timer_manager_impl.h | 9 ++- source/common/http/conn_manager_impl.cc | 7 +- source/server/admin/admin.h | 3 +- source/server/overload_manager_impl.cc | 44 ++++++------ source/server/overload_manager_impl.h | 3 +- .../scaled_range_timer_manager_impl_test.cc | 31 +++++++++ test/mocks/event/mocks.h | 4 ++ test/mocks/server/overload_manager.cc | 4 +- test/mocks/server/overload_manager.h | 6 +- test/server/overload_manager_impl_test.cc | 67 +++---------------- 15 files changed, 135 insertions(+), 115 deletions(-) diff --git a/include/envoy/event/scaled_range_timer_manager.h b/include/envoy/event/scaled_range_timer_manager.h index 7defc1b45be7..22efa6d4a299 100644 --- a/include/envoy/event/scaled_range_timer_manager.h +++ b/include/envoy/event/scaled_range_timer_manager.h @@ -18,6 +18,17 @@ namespace Event { */ class ScaledRangeTimerManager { public: + enum class TimerType { + // Timers created with this type will never be scaled. This should only be used for testing. + UnscaledRealTimerForTest, + // The amount of time an HTTP connection to a downstream client can remain idle (no streams). + // This corresponds to the HTTP_DOWNSTREAM_CONNECTION_IDLE TimerType in overload.proto. + HttpDownstreamIdleConnectionTimeout, + // The amount of time an HTTP stream from a downstream client can remain idle. This corresponds + // to the HTTP_DOWNSTREAM_STREAM_IDLE TimerType in overload.proto. + HttpDownstreamIdleStreamTimeout, + }; + virtual ~ScaledRangeTimerManager() = default; /** @@ -28,6 +39,12 @@ class ScaledRangeTimerManager { */ virtual TimerPtr createTimer(ScaledTimerMinimum minimum, TimerCb callback) PURE; + /** + * Creates a new timer backed by the manager using the provided timer type to + * determine the minimum. + */ + virtual TimerPtr createTimer(TimerType timer_type, TimerCb callback) PURE; + /** * Sets the scale factor for all timers created through this manager. The value should be between * 0 and 1, inclusive. The scale factor affects the amount of time timers spend in their target diff --git a/include/envoy/event/scaled_timer_minimum.h b/include/envoy/event/scaled_timer_minimum.h index edf3b1777b7f..bf8cfd3715b8 100644 --- a/include/envoy/event/scaled_timer_minimum.h +++ b/include/envoy/event/scaled_timer_minimum.h @@ -13,7 +13,10 @@ namespace Event { * Describes a minimum timer value that is equal to a scale factor applied to the maximum. */ struct ScaledMinimum { - explicit ScaledMinimum(UnitFloat scale_factor) : scale_factor_(scale_factor) {} + explicit constexpr ScaledMinimum(UnitFloat scale_factor) : scale_factor_(scale_factor) {} + inline bool operator==(const ScaledMinimum& other) const { + return other.scale_factor_.value() == scale_factor_.value(); + } const UnitFloat scale_factor_; }; @@ -21,7 +24,8 @@ struct ScaledMinimum { * Describes a minimum timer value that is an absolute duration. */ struct AbsoluteMinimum { - explicit AbsoluteMinimum(std::chrono::milliseconds value) : value_(value) {} + explicit constexpr AbsoluteMinimum(std::chrono::milliseconds value) : value_(value) {} + inline bool operator==(const AbsoluteMinimum& other) const { return other.value_ == value_; } const std::chrono::milliseconds value_; }; @@ -29,10 +33,11 @@ struct AbsoluteMinimum { * Class that describes how to compute a minimum timeout given a maximum timeout value. It wraps * ScaledMinimum and AbsoluteMinimum and provides a single computeMinimum() method. */ -class ScaledTimerMinimum : private absl::variant { +class ScaledTimerMinimum { public: - // Use base class constructor. - using absl::variant::variant; + // Forward arguments to impl_'s constructor. + constexpr ScaledTimerMinimum(AbsoluteMinimum arg) : impl_(arg) {} + constexpr ScaledTimerMinimum(ScaledMinimum arg) : impl_(arg) {} // Computes the minimum value for a given maximum timeout. If this object was constructed with a // - ScaledMinimum value: @@ -51,9 +56,13 @@ class ScaledTimerMinimum : private absl::variant } const std::chrono::milliseconds value_; }; - return absl::visit&>( - Visitor(maximum), *this); + return absl::visit(Visitor(maximum), impl_); } + + inline bool operator==(const ScaledTimerMinimum& other) const { return impl_ == other.impl_; } + +private: + absl::variant impl_; }; } // namespace Event diff --git a/include/envoy/server/overload/BUILD b/include/envoy/server/overload/BUILD index f6f4a90933e9..a0a631653882 100644 --- a/include/envoy/server/overload/BUILD +++ b/include/envoy/server/overload/BUILD @@ -23,6 +23,7 @@ envoy_cc_library( name = "thread_local_overload_state", hdrs = ["thread_local_overload_state.h"], deps = [ + "//include/envoy/event:scaled_range_timer_manager_interface", "//include/envoy/event:scaled_timer_minimum", "//include/envoy/event:timer_interface", "//include/envoy/thread_local:thread_local_object", diff --git a/include/envoy/server/overload/thread_local_overload_state.h b/include/envoy/server/overload/thread_local_overload_state.h index ef7a701618d9..d215af683040 100644 --- a/include/envoy/server/overload/thread_local_overload_state.h +++ b/include/envoy/server/overload/thread_local_overload_state.h @@ -4,6 +4,7 @@ #include #include "envoy/common/pure.h" +#include "envoy/event/scaled_range_timer_manager.h" #include "envoy/event/scaled_timer_minimum.h" #include "envoy/event/timer.h" #include "envoy/thread_local/thread_local_object.h" @@ -40,17 +41,6 @@ class OverloadActionState { */ using OverloadActionCb = std::function; -enum class OverloadTimerType { - // Timers created with this type will never be scaled. This should only be used for testing. - UnscaledRealTimerForTest, - // The amount of time an HTTP connection to a downstream client can remain idle (no streams). This - // corresponds to the HTTP_DOWNSTREAM_CONNECTION_IDLE TimerType in overload.proto. - HttpDownstreamIdleConnectionTimeout, - // The amount of time an HTTP stream from a downstream client can remain idle. This corresponds to - // the HTTP_DOWNSTREAM_STREAM_IDLE TimerType in overload.proto. - HttpDownstreamIdleStreamTimeout, -}; - /** * Thread-local copy of the state of each configured overload action. */ @@ -60,7 +50,7 @@ class ThreadLocalOverloadState : public ThreadLocal::ThreadLocalObject { virtual const OverloadActionState& getState(const std::string& action) PURE; // Get a scaled timer whose minimum corresponds to the configured value for the given timer type. - virtual Event::TimerPtr createScaledTimer(OverloadTimerType timer_type, + virtual Event::TimerPtr createScaledTimer(Event::ScaledRangeTimerManager::TimerType timer_type, Event::TimerCb callback) PURE; // Get a scaled timer whose minimum is determined by the given scaling rule. diff --git a/source/common/event/scaled_range_timer_manager_impl.cc b/source/common/event/scaled_range_timer_manager_impl.cc index 6aab81486929..5922a4fb9f23 100644 --- a/source/common/event/scaled_range_timer_manager_impl.cc +++ b/source/common/event/scaled_range_timer_manager_impl.cc @@ -137,8 +137,12 @@ class ScaledRangeTimerManagerImpl::RangeTimerImpl final : public Timer { const ScopeTrackedObject* scope_; }; -ScaledRangeTimerManagerImpl::ScaledRangeTimerManagerImpl(Dispatcher& dispatcher) - : dispatcher_(dispatcher), scale_factor_(1.0) {} +ScaledRangeTimerManagerImpl::ScaledRangeTimerManagerImpl( + Dispatcher& dispatcher, const TimerTypeMapConstSharedPtr& timer_minimums) + : dispatcher_(dispatcher), + timer_minimums_(timer_minimums != nullptr ? timer_minimums + : std::make_shared()), + scale_factor_(1.0) {} ScaledRangeTimerManagerImpl::~ScaledRangeTimerManagerImpl() { // Scaled timers created by the manager shouldn't outlive it. This is @@ -146,6 +150,15 @@ ScaledRangeTimerManagerImpl::~ScaledRangeTimerManagerImpl() { ASSERT(queues_.empty()); } +TimerPtr ScaledRangeTimerManagerImpl::createTimer(TimerType timer_type, TimerCb callback) { + const auto minimum_it = timer_minimums_->find(timer_type); + const Event::ScaledTimerMinimum minimum = + minimum_it != timer_minimums_->end() + ? minimum_it->second + : Event::ScaledTimerMinimum(Event::ScaledMinimum(UnitFloat::max())); + return createTimer(minimum, std::move(callback)); +} + TimerPtr ScaledRangeTimerManagerImpl::createTimer(ScaledTimerMinimum minimum, TimerCb callback) { return std::make_unique(minimum, callback, *this); } diff --git a/source/common/event/scaled_range_timer_manager_impl.h b/source/common/event/scaled_range_timer_manager_impl.h index f9bcc7242b7d..ddeaf93992a0 100644 --- a/source/common/event/scaled_range_timer_manager_impl.h +++ b/source/common/event/scaled_range_timer_manager_impl.h @@ -21,11 +21,17 @@ namespace Event { */ class ScaledRangeTimerManagerImpl : public ScaledRangeTimerManager { public: - explicit ScaledRangeTimerManagerImpl(Dispatcher& dispatcher); + using TimerTypeMap = absl::flat_hash_map; + using TimerTypeMapConstSharedPtr = std::shared_ptr; + + // Takes a Dispatcher and a map from timer type to scaled minimum value. + ScaledRangeTimerManagerImpl(Dispatcher& dispatcher, + const TimerTypeMapConstSharedPtr& timer_minimums = nullptr); ~ScaledRangeTimerManagerImpl() override; // ScaledRangeTimerManager impl TimerPtr createTimer(ScaledTimerMinimum minimum, TimerCb callback) override; + TimerPtr createTimer(TimerType timer_type, TimerCb callback) override; void setScaleFactor(double scale_factor) override; private: @@ -114,6 +120,7 @@ class ScaledRangeTimerManagerImpl : public ScaledRangeTimerManager { void onQueueTimerFired(Queue& queue); Dispatcher& dispatcher_; + const TimerTypeMapConstSharedPtr timer_minimums_; UnitFloat scale_factor_; absl::flat_hash_set, Hash, Eq> queues_; }; diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index d6f501985486..12f89a76bef2 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -10,6 +10,7 @@ #include "envoy/buffer/buffer.h" #include "envoy/common/time.h" #include "envoy/event/dispatcher.h" +#include "envoy/event/scaled_range_timer_manager.h" #include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.h" #include "envoy/http/header_map.h" #include "envoy/network/drain_decision.h" @@ -122,7 +123,7 @@ void ConnectionManagerImpl::initializeReadFilterCallbacks(Network::ReadFilterCal if (config_.idleTimeout()) { connection_idle_timer_ = overload_state_.createScaledTimer( - Server::OverloadTimerType::HttpDownstreamIdleConnectionTimeout, + Event::ScaledRangeTimerManager::TimerType::HttpDownstreamIdleConnectionTimeout, [this]() -> void { onIdleTimeout(); }); connection_idle_timer_->enableTimer(config_.idleTimeout().value()); } @@ -628,7 +629,7 @@ ConnectionManagerImpl::ActiveStream::ActiveStream(ConnectionManagerImpl& connect if (connection_manager_.config_.streamIdleTimeout().count()) { idle_timeout_ms_ = connection_manager_.config_.streamIdleTimeout(); stream_idle_timer_ = connection_manager_.overload_state_.createScaledTimer( - Server::OverloadTimerType::HttpDownstreamIdleStreamTimeout, + Event::ScaledRangeTimerManager::TimerType::HttpDownstreamIdleStreamTimeout, [this]() -> void { onIdleTimeout(); }); resetIdleTimer(); } @@ -1052,7 +1053,7 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(RequestHeaderMapPtr&& he // If we have a route-level idle timeout but no global stream idle timeout, create a timer. if (stream_idle_timer_ == nullptr) { stream_idle_timer_ = connection_manager_.overload_state_.createScaledTimer( - Server::OverloadTimerType::HttpDownstreamIdleStreamTimeout, + Event::ScaledRangeTimerManager::TimerType::HttpDownstreamIdleStreamTimeout, [this]() -> void { onIdleTimeout(); }); } } else if (stream_idle_timer_ != nullptr) { diff --git a/source/server/admin/admin.h b/source/server/admin/admin.h index c766978c9638..26cd982be634 100644 --- a/source/server/admin/admin.h +++ b/source/server/admin/admin.h @@ -256,7 +256,8 @@ class AdminImpl : public Admin, struct NullThreadLocalOverloadState : public ThreadLocalOverloadState { NullThreadLocalOverloadState(Event::Dispatcher& dispatcher) : dispatcher_(dispatcher) {} const OverloadActionState& getState(const std::string&) override { return inactive_; } - Event::TimerPtr createScaledTimer(OverloadTimerType, Event::TimerCb callback) override { + Event::TimerPtr createScaledTimer(Event::ScaledRangeTimerManager::TimerType, + Event::TimerCb callback) override { return dispatcher_.createTimer(callback); } Event::TimerPtr createScaledTimer(Event::ScaledTimerMinimum, diff --git a/source/server/overload_manager_impl.cc b/source/server/overload_manager_impl.cc index 07872b98442b..7f0022e665e7 100644 --- a/source/server/overload_manager_impl.cc +++ b/source/server/overload_manager_impl.cc @@ -9,7 +9,6 @@ #include "common/common/fmt.h" #include "common/config/utility.h" -#include "common/event/scaled_range_timer_manager_impl.h" #include "common/protobuf/utility.h" #include "common/stats/symbol_table_impl.h" @@ -21,16 +20,16 @@ namespace Envoy { namespace Server { +using TimerTypeMap = Event::ScaledRangeTimerManagerImpl::TimerTypeMap; + /** * Thread-local copy of the state of each configured overload action. */ class ThreadLocalOverloadStateImpl : public ThreadLocalOverloadState { public: - ThreadLocalOverloadStateImpl( - Event::ScaledRangeTimerManagerPtr scaled_timer_manager, - const NamedOverloadActionSymbolTable& action_symbol_table, - const absl::flat_hash_map& timer_minimums) - : action_symbol_table_(action_symbol_table), timer_minimums_(timer_minimums), + ThreadLocalOverloadStateImpl(Event::ScaledRangeTimerManagerPtr scaled_timer_manager, + const NamedOverloadActionSymbolTable& action_symbol_table) + : action_symbol_table_(action_symbol_table), actions_(action_symbol_table.size(), OverloadActionState(UnitFloat::min())), scaled_timer_action_(action_symbol_table.lookup(OverloadActionNames::get().ReduceTimeouts)), scaled_timer_manager_(std::move(scaled_timer_manager)) {} @@ -42,14 +41,9 @@ class ThreadLocalOverloadStateImpl : public ThreadLocalOverloadState { return always_inactive_; } - Event::TimerPtr createScaledTimer(OverloadTimerType timer_type, + Event::TimerPtr createScaledTimer(Event::ScaledRangeTimerManager::TimerType timer_type, Event::TimerCb callback) override { - auto minimum_it = timer_minimums_.find(timer_type); - const Event::ScaledTimerMinimum minimum = - minimum_it != timer_minimums_.end() - ? minimum_it->second - : Event::ScaledTimerMinimum(Event::ScaledMinimum(UnitFloat::max())); - return scaled_timer_manager_->createTimer(minimum, std::move(callback)); + return scaled_timer_manager_->createTimer(timer_type, std::move(callback)); } Event::TimerPtr createScaledTimer(Event::ScaledTimerMinimum minimum, @@ -67,7 +61,6 @@ class ThreadLocalOverloadStateImpl : public ThreadLocalOverloadState { private: static const OverloadActionState always_inactive_; const NamedOverloadActionSymbolTable& action_symbol_table_; - const absl::flat_hash_map& timer_minimums_; std::vector actions_; absl::optional scaled_timer_action_; const Event::ScaledRangeTimerManagerPtr scaled_timer_manager_; @@ -141,31 +134,31 @@ Stats::Gauge& makeGauge(Stats::Scope& scope, absl::string_view a, absl::string_v return scope.gaugeFromStatName(stat_name.statName(), import_mode); } -OverloadTimerType parseTimerType( +Event::ScaledRangeTimerManager::TimerType parseTimerType( envoy::config::overload::v3::ScaleTimersOverloadActionConfig::TimerType config_timer_type) { using Config = envoy::config::overload::v3::ScaleTimersOverloadActionConfig; switch (config_timer_type) { case Config::HTTP_DOWNSTREAM_CONNECTION_IDLE: - return OverloadTimerType::HttpDownstreamIdleConnectionTimeout; + return Event::ScaledRangeTimerManager::TimerType::HttpDownstreamIdleConnectionTimeout; case Config::HTTP_DOWNSTREAM_STREAM_IDLE: - return OverloadTimerType::HttpDownstreamIdleStreamTimeout; + return Event::ScaledRangeTimerManager::TimerType::HttpDownstreamIdleStreamTimeout; default: throw EnvoyException(fmt::format("Unknown timer type {}", config_timer_type)); } } -absl::flat_hash_map -parseTimerMinimums(const ProtobufWkt::Any& typed_config, - ProtobufMessage::ValidationVisitor& validation_visitor) { +TimerTypeMap parseTimerMinimums(const ProtobufWkt::Any& typed_config, + ProtobufMessage::ValidationVisitor& validation_visitor) { using Config = envoy::config::overload::v3::ScaleTimersOverloadActionConfig; const Config action_config = MessageUtil::anyConvertAndValidate(typed_config, validation_visitor); - absl::flat_hash_map timer_map; + TimerTypeMap timer_map; for (const auto& scale_timer : action_config.timer_scale_factors()) { - const OverloadTimerType timer_type = parseTimerType(scale_timer.timer()); + const Event::ScaledRangeTimerManager::TimerType timer_type = + parseTimerType(scale_timer.timer()); const Event::ScaledTimerMinimum minimum = scale_timer.has_min_timeout() @@ -315,7 +308,8 @@ OverloadManagerImpl::OverloadManagerImpl(Event::Dispatcher& dispatcher, Stats::S } if (name == OverloadActionNames::get().ReduceTimeouts) { - timer_minimums_ = parseTimerMinimums(action.typed_config(), validation_visitor); + timer_minimums_ = std::make_shared( + parseTimerMinimums(action.typed_config(), validation_visitor)); } else if (action.has_typed_config()) { throw EnvoyException(fmt::format( "Overload action \"{}\" has an unexpected value for the typed_config field", name)); @@ -340,7 +334,7 @@ void OverloadManagerImpl::start() { tls_.set([this](Event::Dispatcher& dispatcher) { return std::make_shared(createScaledRangeTimerManager(dispatcher), - action_symbol_table_, timer_minimums_); + action_symbol_table_); }); if (resources_.empty()) { @@ -395,7 +389,7 @@ ThreadLocalOverloadState& OverloadManagerImpl::getThreadLocalOverloadState() { r Event::ScaledRangeTimerManagerPtr OverloadManagerImpl::createScaledRangeTimerManager(Event::Dispatcher& dispatcher) const { - return std::make_unique(dispatcher); + return std::make_unique(dispatcher, timer_minimums_); } void OverloadManagerImpl::updateResourcePressure(const std::string& resource, double pressure, diff --git a/source/server/overload_manager_impl.h b/source/server/overload_manager_impl.h index b86162fb0224..97cc399e31b7 100644 --- a/source/server/overload_manager_impl.h +++ b/source/server/overload_manager_impl.h @@ -15,6 +15,7 @@ #include "envoy/thread_local/thread_local.h" #include "common/common/logger.h" +#include "common/event/scaled_range_timer_manager_impl.h" #include "absl/container/node_hash_map.h" #include "absl/container/node_hash_set.h" @@ -170,7 +171,7 @@ class OverloadManagerImpl : Logger::Loggable, public OverloadM absl::node_hash_map resources_; absl::node_hash_map actions_; - absl::flat_hash_map timer_minimums_; + Event::ScaledRangeTimerManagerImpl::TimerTypeMapConstSharedPtr timer_minimums_; absl::flat_hash_map state_updates_to_flush_; diff --git a/test/common/event/scaled_range_timer_manager_impl_test.cc b/test/common/event/scaled_range_timer_manager_impl_test.cc index 824a285ee4f6..cfc61a32d212 100644 --- a/test/common/event/scaled_range_timer_manager_impl_test.cc +++ b/test/common/event/scaled_range_timer_manager_impl_test.cc @@ -617,6 +617,37 @@ TEST_F(ScaledRangeTimerManagerTest, MultipleTimersWithChangeInScalingFactor) { ElementsAre(start + std::chrono::seconds(9), start + std::chrono::seconds(16))); } +TEST_F(ScaledRangeTimerManagerTest, LooksUpConfiguredMinimums) { + // Test-only class that overrides one of the createScaledTimer overloads to show that the other + // one calls into this one after looking up the minimum. + class TestScaledRangeTimerManager : public ScaledRangeTimerManagerImpl { + public: + using ScaledRangeTimerManagerImpl::ScaledRangeTimerManagerImpl; + using ScaledRangeTimerManagerImpl::createTimer; + TimerPtr createTimer(ScaledTimerMinimum minimum, TimerCb callback) override { + return createScaledTimer(minimum, callback); + } + MOCK_METHOD(TimerPtr, createScaledTimer, (ScaledTimerMinimum, TimerCb)); + }; + + const ScaledRangeTimerManagerImpl::TimerTypeMap timer_types{ + {ScaledRangeTimerManager::TimerType::UnscaledRealTimerForTest, + ScaledMinimum(UnitFloat::max())}, + {ScaledRangeTimerManager::TimerType::HttpDownstreamIdleConnectionTimeout, + ScaledMinimum(UnitFloat(0.3))}, + {ScaledRangeTimerManager::TimerType::HttpDownstreamIdleStreamTimeout, + ScaledMinimum(UnitFloat(0.6))}, + }; + + TestScaledRangeTimerManager manager(dispatcher_, + std::make_unique(timer_types)); + for (const auto& [timer_type, minimum] : timer_types) { + SCOPED_TRACE(static_cast(timer_type)); + EXPECT_CALL(manager, createScaledTimer(minimum, _)); + manager.createTimer(timer_type, []() {}); + } +} + } // namespace } // namespace Event } // namespace Envoy diff --git a/test/mocks/event/mocks.h b/test/mocks/event/mocks.h index 8e29e84c3b32..850fe7b6447c 100644 --- a/test/mocks/event/mocks.h +++ b/test/mocks/event/mocks.h @@ -184,7 +184,11 @@ class MockScaledRangeTimerManager : public ScaledRangeTimerManager { TimerPtr createTimer(ScaledTimerMinimum minimum, TimerCb callback) override { return TimerPtr{createTimer_(minimum, std::move(callback))}; } + TimerPtr createTimer(ScaledRangeTimerManager::TimerType timer_type, TimerCb callback) override { + return TimerPtr{createTypedTimer_(timer_type, std::move(callback))}; + } MOCK_METHOD(Timer*, createTimer_, (ScaledTimerMinimum, TimerCb)); + MOCK_METHOD(Timer*, createTypedTimer_, (ScaledRangeTimerManager::TimerType, TimerCb)); MOCK_METHOD(void, setScaleFactor, (double), (override)); }; diff --git a/test/mocks/server/overload_manager.cc b/test/mocks/server/overload_manager.cc index 461b0b27daa3..ef20eca27582 100644 --- a/test/mocks/server/overload_manager.cc +++ b/test/mocks/server/overload_manager.cc @@ -21,8 +21,8 @@ MockThreadLocalOverloadState::MockThreadLocalOverloadState() ON_CALL(*this, createScaledMinimumTimer_).WillByDefault(ReturnNew>()); } -Event::TimerPtr MockThreadLocalOverloadState::createScaledTimer(OverloadTimerType timer_type, - Event::TimerCb callback) { +Event::TimerPtr MockThreadLocalOverloadState::createScaledTimer( + Event::ScaledRangeTimerManager::TimerType timer_type, Event::TimerCb callback) { return Event::TimerPtr{createScaledTypedTimer_(timer_type, std::move(callback))}; } diff --git a/test/mocks/server/overload_manager.h b/test/mocks/server/overload_manager.h index c694ab7b1254..7865ac1ffae8 100644 --- a/test/mocks/server/overload_manager.h +++ b/test/mocks/server/overload_manager.h @@ -14,10 +14,12 @@ class MockThreadLocalOverloadState : public ThreadLocalOverloadState { public: MockThreadLocalOverloadState(); MOCK_METHOD(const OverloadActionState&, getState, (const std::string&), (override)); - Event::TimerPtr createScaledTimer(OverloadTimerType timer_type, Event::TimerCb callback) override; + Event::TimerPtr createScaledTimer(Event::ScaledRangeTimerManager::TimerType timer_type, + Event::TimerCb callback) override; Event::TimerPtr createScaledTimer(Event::ScaledTimerMinimum minimum, Event::TimerCb callback) override; - MOCK_METHOD(Event::Timer*, createScaledTypedTimer_, (OverloadTimerType, Event::TimerCb)); + MOCK_METHOD(Event::Timer*, createScaledTypedTimer_, + (Event::ScaledRangeTimerManager::TimerType, Event::TimerCb)); MOCK_METHOD(Event::Timer*, createScaledMinimumTimer_, (Event::ScaledTimerMinimum, Event::TimerCb)); diff --git a/test/server/overload_manager_impl_test.cc b/test/server/overload_manager_impl_test.cc index d160b42ceae8..da3b156cea70 100644 --- a/test/server/overload_manager_impl_test.cc +++ b/test/server/overload_manager_impl_test.cc @@ -510,7 +510,7 @@ TEST_F(OverloadManagerImplTest, AdjustScaleFactor) { timer_cb_(); } -TEST_F(OverloadManagerImplTest, CreateUnscaledScaledTimer) { +TEST_F(OverloadManagerImplTest, CreateTypedTimer) { setDispatcherExpectation(); auto manager(createOverloadManager(kReducedTimeoutsConfig)); @@ -521,67 +521,16 @@ TEST_F(OverloadManagerImplTest, CreateUnscaledScaledTimer) { auto* mock_scaled_timer = new Event::MockTimer(); MockFunction mock_callback; - EXPECT_CALL(*scaled_timer_manager, createTimer_) - .WillOnce([&](Event::ScaledTimerMinimum minimum, auto) { - // Since this timer was created with the timer type "unscaled", it should use the same value - // for the min and max. Test that by checking an arbitrary value. - EXPECT_EQ(minimum.computeMinimum(std::chrono::seconds(55)), std::chrono::seconds(55)); - return mock_scaled_timer; - }); + EXPECT_CALL( + *scaled_timer_manager, + createTypedTimer_(Event::ScaledRangeTimerManager::TimerType::UnscaledRealTimerForTest, _)) + .WillOnce(Return(mock_scaled_timer)); auto timer = manager->getThreadLocalOverloadState().createScaledTimer( - OverloadTimerType::UnscaledRealTimerForTest, mock_callback.AsStdFunction()); + Event::ScaledRangeTimerManager::TimerType::UnscaledRealTimerForTest, + mock_callback.AsStdFunction()); - EXPECT_CALL(*mock_scaled_timer, enableTimer(std::chrono::milliseconds(5 * 1000), _)); - timer->enableTimer(std::chrono::seconds(5)); -} - -TEST_F(OverloadManagerImplTest, CreateScaledTimerWithAbsoluteMinimum) { - setDispatcherExpectation(); - auto manager(createOverloadManager(kReducedTimeoutsConfig)); - - auto* scaled_timer_manager = new Event::MockScaledRangeTimerManager(); - EXPECT_CALL(*manager, createScaledRangeTimerManager) - .WillOnce(Return(ByMove(Event::ScaledRangeTimerManagerPtr{scaled_timer_manager}))); - manager->start(); - - auto* mock_scaled_timer = new Event::MockTimer(); - MockFunction mock_callback; - EXPECT_CALL(*scaled_timer_manager, createTimer_) - .WillOnce([&](Event::ScaledTimerMinimum minimum, auto) { - // This timer was created with an absolute minimum. Test that by checking an arbitrary - // value. - EXPECT_EQ(minimum.computeMinimum(std::chrono::seconds(55)), std::chrono::seconds(2)); - return mock_scaled_timer; - }); - - auto timer = manager->getThreadLocalOverloadState().createScaledTimer( - OverloadTimerType::HttpDownstreamIdleConnectionTimeout, mock_callback.AsStdFunction()); - EXPECT_EQ(timer.get(), mock_scaled_timer); -} - -TEST_F(OverloadManagerImplTest, CreateScaledTimerWithProvidedMinimum) { - setDispatcherExpectation(); - auto manager(createOverloadManager(kReducedTimeoutsConfig)); - - auto* scaled_timer_manager = new Event::MockScaledRangeTimerManager(); - EXPECT_CALL(*manager, createScaledRangeTimerManager) - .WillOnce(Return(ByMove(Event::ScaledRangeTimerManagerPtr{scaled_timer_manager}))); - manager->start(); - - auto* mock_scaled_timer = new Event::MockTimer(); - MockFunction mock_callback; - EXPECT_CALL(*scaled_timer_manager, createTimer_) - .WillOnce([&](Event::ScaledTimerMinimum minimum, auto) { - // This timer was created with an absolute minimum. Test that by checking an arbitrary - // value. - EXPECT_EQ(minimum.computeMinimum(std::chrono::seconds(55)), std::chrono::seconds(3)); - return mock_scaled_timer; - }); - - auto timer = manager->getThreadLocalOverloadState().createScaledTimer( - Event::AbsoluteMinimum(std::chrono::seconds(3)), mock_callback.AsStdFunction()); - EXPECT_EQ(timer.get(), mock_scaled_timer); + EXPECT_EQ(mock_scaled_timer, timer.get()); } TEST_F(OverloadManagerImplTest, DuplicateResourceMonitor) { From 9d167b1744612d5222529f2df13659675f80eb3f Mon Sep 17 00:00:00 2001 From: Alex Konradi Date: Tue, 12 Jan 2021 12:36:42 -0500 Subject: [PATCH 2/9] Create scaled timers through the Dispatcher When creating the Dispatcher, use a factory returned by the OverloadManager to construct the ScaledRangeTimerManager. Signed-off-by: Alex Konradi --- include/envoy/api/api.h | 12 +++++ include/envoy/event/BUILD | 3 +- include/envoy/event/dispatcher.h | 16 +++++++ .../envoy/event/scaled_range_timer_manager.h | 3 ++ .../envoy/server/overload/overload_manager.h | 6 +++ .../overload/thread_local_overload_state.h | 8 ---- source/common/api/api_impl.cc | 12 ++++- source/common/api/api_impl.h | 3 ++ source/common/event/BUILD | 1 + source/common/event/dispatcher_impl.cc | 29 +++++++++--- source/common/event/dispatcher_impl.h | 7 ++- source/common/http/conn_manager_impl.cc | 16 ++++--- source/server/admin/admin.h | 11 +---- source/server/overload_manager_impl.cc | 37 ++++++--------- source/server/overload_manager_impl.h | 1 + source/server/worker_impl.cc | 3 +- .../scaled_range_timer_manager_impl_test.cc | 2 +- .../http/conn_manager_impl_test_base.cc | 2 +- test/mocks/api/mocks.cc | 9 +++- test/mocks/api/mocks.h | 8 +++- test/mocks/event/mocks.h | 18 ++++++++ test/mocks/event/wrapped_dispatcher.h | 7 +++ test/mocks/server/overload_manager.cc | 12 ----- test/mocks/server/overload_manager.h | 9 +--- test/server/api_listener_test.cc | 7 ++- test/server/overload_manager_impl_test.cc | 45 +++++++++---------- 26 files changed, 178 insertions(+), 109 deletions(-) diff --git a/include/envoy/api/api.h b/include/envoy/api/api.h index aa310d2c40c2..1bb0ebb5abc6 100644 --- a/include/envoy/api/api.h +++ b/include/envoy/api/api.h @@ -30,6 +30,18 @@ class Api { */ virtual Event::DispatcherPtr allocateDispatcher(const std::string& name) PURE; + /** + * Allocate a dispatcher. + * @param name the identity name for a dispatcher, e.g. "worker_2" or "main_thread". + * This name will appear in per-handler/worker statistics, such as + * "server.worker_2.watchdog_miss". + * @param scaled_timer_factory the factory to use when creating the scaled timer manager. + * @return Event::DispatcherPtr which is owned by the caller. + */ + virtual Event::DispatcherPtr + allocateDispatcher(const std::string& name, + const Event::ScaledRangeTimerManagerFactory& scaled_timer_factory) PURE; + /** * Allocate a dispatcher. * @param name the identity name for a dispatcher, e.g. "worker_2" or "main_thread". diff --git a/include/envoy/event/BUILD b/include/envoy/event/BUILD index a4c865a2d225..8b357d194a79 100644 --- a/include/envoy/event/BUILD +++ b/include/envoy/event/BUILD @@ -19,11 +19,12 @@ envoy_cc_library( deps = [ ":deferred_deletable", ":file_event_interface", + ":scaled_range_timer_manager_interface", ":schedulable_cb_interface", ":signal_interface", + ":timer_interface", "//include/envoy/common:scope_tracker_interface", "//include/envoy/common:time_interface", - "//include/envoy/event:timer_interface", "//include/envoy/filesystem:watcher_interface", "//include/envoy/network:connection_handler_interface", "//include/envoy/network:connection_interface", diff --git a/include/envoy/event/dispatcher.h b/include/envoy/event/dispatcher.h index 599617e87d28..737d5be8b293 100644 --- a/include/envoy/event/dispatcher.h +++ b/include/envoy/event/dispatcher.h @@ -9,6 +9,7 @@ #include "envoy/common/scope_tracker.h" #include "envoy/common/time.h" #include "envoy/event/file_event.h" +#include "envoy/event/scaled_range_timer_manager.h" #include "envoy/event/schedulable_cb.h" #include "envoy/event/signal.h" #include "envoy/event/timer.h" @@ -77,6 +78,21 @@ class DispatcherBase { */ virtual Event::TimerPtr createTimer(TimerCb cb) PURE; + /** + * Allocates a scaled timer. @see Timer for docs on how to use the timer. + * @param timer_type the type of timer to create. + * @param cb supplies the callback to invoke when the timer fires. + */ + virtual Event::TimerPtr createScaledTimer(Event::ScaledRangeTimerManager::TimerType timer_type, + TimerCb cb) PURE; + + /** + * Allocates a scaled timer. @see Timer for docs on how to use the timer. + * @param minimum the rule for computing the minimum value of the timer. + * @param cb supplies the callback to invoke when the timer fires. + */ + virtual Event::TimerPtr createScaledTimer(ScaledTimerMinimum minimum, TimerCb cb) PURE; + /** * Allocates a schedulable callback. @see SchedulableCallback for docs on how to use the wrapped * callback. diff --git a/include/envoy/event/scaled_range_timer_manager.h b/include/envoy/event/scaled_range_timer_manager.h index 22efa6d4a299..27ab7952f87b 100644 --- a/include/envoy/event/scaled_range_timer_manager.h +++ b/include/envoy/event/scaled_range_timer_manager.h @@ -58,5 +58,8 @@ class ScaledRangeTimerManager { using ScaledRangeTimerManagerPtr = std::unique_ptr; +class Dispatcher; +using ScaledRangeTimerManagerFactory = std::function; + } // namespace Event } // namespace Envoy diff --git a/include/envoy/server/overload/overload_manager.h b/include/envoy/server/overload/overload_manager.h index a4b42a7c9fad..a9092b64341b 100644 --- a/include/envoy/server/overload/overload_manager.h +++ b/include/envoy/server/overload/overload_manager.h @@ -4,6 +4,7 @@ #include "envoy/common/pure.h" #include "envoy/event/dispatcher.h" +#include "envoy/event/scaled_range_timer_manager.h" #include "envoy/server/overload/thread_local_overload_state.h" #include "common/singleton/const_singleton.h" @@ -69,6 +70,11 @@ class OverloadManager { * an alternative to registering a callback for overload action state changes. */ virtual ThreadLocalOverloadState& getThreadLocalOverloadState() PURE; + + /** + * Get a factory for constructing scaled timer managers that respond to overload state. + */ + virtual Event::ScaledRangeTimerManagerFactory scaledTimerFactory() PURE; }; } // namespace Server diff --git a/include/envoy/server/overload/thread_local_overload_state.h b/include/envoy/server/overload/thread_local_overload_state.h index d215af683040..c442f5c4fafc 100644 --- a/include/envoy/server/overload/thread_local_overload_state.h +++ b/include/envoy/server/overload/thread_local_overload_state.h @@ -48,14 +48,6 @@ class ThreadLocalOverloadState : public ThreadLocal::ThreadLocalObject { public: // Get a thread-local reference to the value for the given action key. virtual const OverloadActionState& getState(const std::string& action) PURE; - - // Get a scaled timer whose minimum corresponds to the configured value for the given timer type. - virtual Event::TimerPtr createScaledTimer(Event::ScaledRangeTimerManager::TimerType timer_type, - Event::TimerCb callback) PURE; - - // Get a scaled timer whose minimum is determined by the given scaling rule. - virtual Event::TimerPtr createScaledTimer(Event::ScaledTimerMinimum minimum, - Event::TimerCb callback) PURE; }; } // namespace Server diff --git a/source/common/api/api_impl.cc b/source/common/api/api_impl.cc index 2ac6bd73ae05..9982fe2ac346 100644 --- a/source/common/api/api_impl.cc +++ b/source/common/api/api_impl.cc @@ -18,12 +18,20 @@ Impl::Impl(Thread::ThreadFactory& thread_factory, Stats::Store& store, process_context_(process_context), watermark_factory_(std::move(watermark_factory)) {} Event::DispatcherPtr Impl::allocateDispatcher(const std::string& name) { - return std::make_unique(name, *this, time_system_, watermark_factory_); + return std::make_unique(name, *this, time_system_, nullptr, + watermark_factory_); +} +Event::DispatcherPtr +Impl::allocateDispatcher(const std::string& name, + const Event::ScaledRangeTimerManagerFactory& scaled_timer_factory) { + return std::make_unique(name, *this, time_system_, scaled_timer_factory, + watermark_factory_); } Event::DispatcherPtr Impl::allocateDispatcher(const std::string& name, Buffer::WatermarkFactoryPtr&& factory) { - return std::make_unique(name, *this, time_system_, std::move(factory)); + return std::make_unique(name, *this, time_system_, nullptr, + std::move(factory)); } } // namespace Api diff --git a/source/common/api/api_impl.h b/source/common/api/api_impl.h index c7bea1a73f24..0bec3b866562 100644 --- a/source/common/api/api_impl.h +++ b/source/common/api/api_impl.h @@ -24,6 +24,9 @@ class Impl : public Api { // Api::Api Event::DispatcherPtr allocateDispatcher(const std::string& name) override; + Event::DispatcherPtr + allocateDispatcher(const std::string& name, + const Event::ScaledRangeTimerManagerFactory& scaled_timer_factory) override; Event::DispatcherPtr allocateDispatcher(const std::string& name, Buffer::WatermarkFactoryPtr&& watermark_factory) override; Thread::ThreadFactory& threadFactory() override { return thread_factory_; } diff --git a/source/common/event/BUILD b/source/common/event/BUILD index c14a6ee08e99..ba6e5eec132c 100644 --- a/source/common/event/BUILD +++ b/source/common/event/BUILD @@ -34,6 +34,7 @@ envoy_cc_library( ":libevent_scheduler_lib", ":real_time_system_lib", ":signal_lib", + ":scaled_range_timer_manager_lib", "//include/envoy/common:scope_tracker_interface", "//include/envoy/common:time_interface", "//include/envoy/event:signal_interface", diff --git a/source/common/event/dispatcher_impl.cc b/source/common/event/dispatcher_impl.cc index 558d82b9230d..eea47145fd43 100644 --- a/source/common/event/dispatcher_impl.cc +++ b/source/common/event/dispatcher_impl.cc @@ -15,6 +15,7 @@ #include "common/common/thread.h" #include "common/event/file_event_impl.h" #include "common/event/libevent_scheduler.h" +#include "common/event/scaled_range_timer_manager_impl.h" #include "common/event/signal_impl.h" #include "common/event/timer_impl.h" #include "common/filesystem/watcher_impl.h" @@ -37,17 +38,22 @@ namespace Envoy { namespace Event { -DispatcherImpl::DispatcherImpl(const std::string& name, Api::Api& api, - Event::TimeSystem& time_system, - const Buffer::WatermarkFactorySharedPtr& factory) +DispatcherImpl::DispatcherImpl( + const std::string& name, Api::Api& api, Event::TimeSystem& time_system, + const std::function& scaled_timer_factory, + const Buffer::WatermarkFactorySharedPtr& watermark_factory) : name_(name), api_(api), - buffer_factory_(factory != nullptr ? factory - : std::make_shared()), + buffer_factory_(watermark_factory != nullptr + ? watermark_factory + : std::make_shared()), scheduler_(time_system.createScheduler(base_scheduler_, base_scheduler_)), deferred_delete_cb_(base_scheduler_.createSchedulableCallback( [this]() -> void { clearDeferredDeleteList(); })), post_cb_(base_scheduler_.createSchedulableCallback([this]() -> void { runPostCallbacks(); })), - current_to_delete_(&to_delete_1_) { + current_to_delete_(&to_delete_1_), + scaled_timer_manager_(scaled_timer_factory != nullptr + ? scaled_timer_factory(*this) + : std::make_unique(*this)) { ASSERT(!name_.empty()); FatalErrorHandler::registerFatalErrorHandler(*this); updateApproximateMonotonicTimeInternal(); @@ -190,6 +196,17 @@ TimerPtr DispatcherImpl::createTimer(TimerCb cb) { return createTimerInternal(cb); } +TimerPtr DispatcherImpl::createScaledTimer(ScaledRangeTimerManager::TimerType timer_type, + TimerCb cb) { + ASSERT(isThreadSafe()); + return scaled_timer_manager_->createTimer(timer_type, std::move(cb)); +} + +TimerPtr DispatcherImpl::createScaledTimer(ScaledTimerMinimum minimum, TimerCb cb) { + ASSERT(isThreadSafe()); + return scaled_timer_manager_->createTimer(minimum, std::move(cb)); +} + Event::SchedulableCallbackPtr DispatcherImpl::createSchedulableCallback(std::function cb) { ASSERT(isThreadSafe()); return base_scheduler_.createSchedulableCallback([this, cb]() { diff --git a/source/common/event/dispatcher_impl.h b/source/common/event/dispatcher_impl.h index bd3b698af11f..0692001045b8 100644 --- a/source/common/event/dispatcher_impl.h +++ b/source/common/event/dispatcher_impl.h @@ -31,7 +31,8 @@ class DispatcherImpl : Logger::Loggable, public FatalErrorHandlerInterface { public: DispatcherImpl(const std::string& name, Api::Api& api, Event::TimeSystem& time_system, - const Buffer::WatermarkFactorySharedPtr& factory = nullptr); + const ScaledRangeTimerManagerFactory& scaled_timer_factory = nullptr, + const Buffer::WatermarkFactorySharedPtr& watermark_factory = nullptr); ~DispatcherImpl() override; /** @@ -67,6 +68,9 @@ class DispatcherImpl : Logger::Loggable, Network::UdpListenerPtr createUdpListener(Network::SocketSharedPtr socket, Network::UdpListenerCallbacks& cb) override; TimerPtr createTimer(TimerCb cb) override; + TimerPtr createScaledTimer(ScaledRangeTimerManager::TimerType timer_type, TimerCb cb) override; + TimerPtr createScaledTimer(ScaledTimerMinimum minimum, TimerCb cb) override; + Event::SchedulableCallbackPtr createSchedulableCallback(std::function cb) override; void deferredDelete(DeferredDeletablePtr&& to_delete) override; void exit() override; @@ -154,6 +158,7 @@ class DispatcherImpl : Logger::Loggable, bool deferred_deleting_{}; MonotonicTime approximate_monotonic_time_; WatchdogRegistrationPtr watchdog_registration_; + const ScaledRangeTimerManagerPtr scaled_timer_manager_; }; } // namespace Event diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index 12f89a76bef2..4cc466f32e0a 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -122,7 +122,7 @@ void ConnectionManagerImpl::initializeReadFilterCallbacks(Network::ReadFilterCal read_callbacks_->connection().addConnectionCallbacks(*this); if (config_.idleTimeout()) { - connection_idle_timer_ = overload_state_.createScaledTimer( + connection_idle_timer_ = read_callbacks_->connection().dispatcher().createScaledTimer( Event::ScaledRangeTimerManager::TimerType::HttpDownstreamIdleConnectionTimeout, [this]() -> void { onIdleTimeout(); }); connection_idle_timer_->enableTimer(config_.idleTimeout().value()); @@ -628,9 +628,10 @@ ConnectionManagerImpl::ActiveStream::ActiveStream(ConnectionManagerImpl& connect if (connection_manager_.config_.streamIdleTimeout().count()) { idle_timeout_ms_ = connection_manager_.config_.streamIdleTimeout(); - stream_idle_timer_ = connection_manager_.overload_state_.createScaledTimer( - Event::ScaledRangeTimerManager::TimerType::HttpDownstreamIdleStreamTimeout, - [this]() -> void { onIdleTimeout(); }); + stream_idle_timer_ = + connection_manager_.read_callbacks_->connection().dispatcher().createScaledTimer( + Event::ScaledRangeTimerManager::TimerType::HttpDownstreamIdleStreamTimeout, + [this]() -> void { onIdleTimeout(); }); resetIdleTimer(); } @@ -1052,9 +1053,10 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(RequestHeaderMapPtr&& he if (idle_timeout_ms_.count()) { // If we have a route-level idle timeout but no global stream idle timeout, create a timer. if (stream_idle_timer_ == nullptr) { - stream_idle_timer_ = connection_manager_.overload_state_.createScaledTimer( - Event::ScaledRangeTimerManager::TimerType::HttpDownstreamIdleStreamTimeout, - [this]() -> void { onIdleTimeout(); }); + stream_idle_timer_ = + connection_manager_.read_callbacks_->connection().dispatcher().createScaledTimer( + Event::ScaledRangeTimerManager::TimerType::HttpDownstreamIdleStreamTimeout, + [this]() -> void { onIdleTimeout(); }); } } else if (stream_idle_timer_ != nullptr) { // If we had a global stream idle timeout but the route-level idle timeout is set to zero diff --git a/source/server/admin/admin.h b/source/server/admin/admin.h index 26cd982be634..3ca7be09f1af 100644 --- a/source/server/admin/admin.h +++ b/source/server/admin/admin.h @@ -256,15 +256,6 @@ class AdminImpl : public Admin, struct NullThreadLocalOverloadState : public ThreadLocalOverloadState { NullThreadLocalOverloadState(Event::Dispatcher& dispatcher) : dispatcher_(dispatcher) {} const OverloadActionState& getState(const std::string&) override { return inactive_; } - Event::TimerPtr createScaledTimer(Event::ScaledRangeTimerManager::TimerType, - Event::TimerCb callback) override { - return dispatcher_.createTimer(callback); - } - Event::TimerPtr createScaledTimer(Event::ScaledTimerMinimum, - Event::TimerCb callback) override { - return dispatcher_.createTimer(callback); - } - Event::Dispatcher& dispatcher_; const OverloadActionState inactive_ = OverloadActionState::inactive(); }; @@ -282,6 +273,8 @@ class AdminImpl : public Admin, return tls_->getTyped(); } + Event::ScaledRangeTimerManagerFactory scaledTimerFactory() override { return nullptr; } + bool registerForAction(const std::string&, Event::Dispatcher&, OverloadActionCb) override { // This method shouldn't be called by the admin listener NOT_REACHED_GCOVR_EXCL_LINE; diff --git a/source/server/overload_manager_impl.cc b/source/server/overload_manager_impl.cc index 7f0022e665e7..7a442bf00e52 100644 --- a/source/server/overload_manager_impl.cc +++ b/source/server/overload_manager_impl.cc @@ -27,12 +27,9 @@ using TimerTypeMap = Event::ScaledRangeTimerManagerImpl::TimerTypeMap; */ class ThreadLocalOverloadStateImpl : public ThreadLocalOverloadState { public: - ThreadLocalOverloadStateImpl(Event::ScaledRangeTimerManagerPtr scaled_timer_manager, - const NamedOverloadActionSymbolTable& action_symbol_table) + explicit ThreadLocalOverloadStateImpl(const NamedOverloadActionSymbolTable& action_symbol_table) : action_symbol_table_(action_symbol_table), - actions_(action_symbol_table.size(), OverloadActionState(UnitFloat::min())), - scaled_timer_action_(action_symbol_table.lookup(OverloadActionNames::get().ReduceTimeouts)), - scaled_timer_manager_(std::move(scaled_timer_manager)) {} + actions_(action_symbol_table.size(), OverloadActionState(UnitFloat::min())) {} const OverloadActionState& getState(const std::string& action) override { if (const auto symbol = action_symbol_table_.lookup(action); symbol != absl::nullopt) { @@ -41,29 +38,14 @@ class ThreadLocalOverloadStateImpl : public ThreadLocalOverloadState { return always_inactive_; } - Event::TimerPtr createScaledTimer(Event::ScaledRangeTimerManager::TimerType timer_type, - Event::TimerCb callback) override { - return scaled_timer_manager_->createTimer(timer_type, std::move(callback)); - } - - Event::TimerPtr createScaledTimer(Event::ScaledTimerMinimum minimum, - Event::TimerCb callback) override { - return scaled_timer_manager_->createTimer(minimum, std::move(callback)); - } - void setState(NamedOverloadActionSymbolTable::Symbol action, OverloadActionState state) { actions_[action.index()] = state; - if (scaled_timer_action_.has_value() && scaled_timer_action_.value() == action) { - scaled_timer_manager_->setScaleFactor(1 - state.value()); - } } private: static const OverloadActionState always_inactive_; const NamedOverloadActionSymbolTable& action_symbol_table_; std::vector actions_; - absl::optional scaled_timer_action_; - const Event::ScaledRangeTimerManagerPtr scaled_timer_manager_; }; const OverloadActionState ThreadLocalOverloadStateImpl::always_inactive_{UnitFloat::min()}; @@ -332,9 +314,8 @@ void OverloadManagerImpl::start() { ASSERT(!started_); started_ = true; - tls_.set([this](Event::Dispatcher& dispatcher) { - return std::make_shared(createScaledRangeTimerManager(dispatcher), - action_symbol_table_); + tls_.set([this](Event::Dispatcher&) { + return std::make_shared(action_symbol_table_); }); if (resources_.empty()) { @@ -386,6 +367,16 @@ bool OverloadManagerImpl::registerForAction(const std::string& action, } ThreadLocalOverloadState& OverloadManagerImpl::getThreadLocalOverloadState() { return *tls_; } +Event::ScaledRangeTimerManagerFactory OverloadManagerImpl::scaledTimerFactory() { + return [this](Event::Dispatcher& dispatcher) { + auto manager = createScaledRangeTimerManager(dispatcher); + registerForAction(OverloadActionNames::get().ReduceTimeouts, dispatcher, + [manager = manager.get()](OverloadActionState scale_state) { + manager->setScaleFactor(1 - scale_state.value()); + }); + return manager; + }; +} Event::ScaledRangeTimerManagerPtr OverloadManagerImpl::createScaledRangeTimerManager(Event::Dispatcher& dispatcher) const { diff --git a/source/server/overload_manager_impl.h b/source/server/overload_manager_impl.h index 97cc399e31b7..50e9cb4fdd87 100644 --- a/source/server/overload_manager_impl.h +++ b/source/server/overload_manager_impl.h @@ -115,6 +115,7 @@ class OverloadManagerImpl : Logger::Loggable, public OverloadM bool registerForAction(const std::string& action, Event::Dispatcher& dispatcher, OverloadActionCb callback) override; ThreadLocalOverloadState& getThreadLocalOverloadState() override; + Event::ScaledRangeTimerManagerFactory scaledTimerFactory() override; // Stop the overload manager timer and wait for any pending resource updates to complete. // After this returns, overload manager clients should not receive any more callbacks diff --git a/source/server/worker_impl.cc b/source/server/worker_impl.cc index 760b7ca630bc..aff043c63df9 100644 --- a/source/server/worker_impl.cc +++ b/source/server/worker_impl.cc @@ -16,7 +16,8 @@ namespace Server { WorkerPtr ProdWorkerFactory::createWorker(uint32_t index, OverloadManager& overload_manager, const std::string& worker_name) { - Event::DispatcherPtr dispatcher(api_.allocateDispatcher(worker_name)); + Event::DispatcherPtr dispatcher( + api_.allocateDispatcher(worker_name, overload_manager.scaledTimerFactory())); return std::make_unique(tls_, hooks_, std::move(dispatcher), std::make_unique(*dispatcher, index), overload_manager, api_); diff --git a/test/common/event/scaled_range_timer_manager_impl_test.cc b/test/common/event/scaled_range_timer_manager_impl_test.cc index cfc61a32d212..0cbbb610ee53 100644 --- a/test/common/event/scaled_range_timer_manager_impl_test.cc +++ b/test/common/event/scaled_range_timer_manager_impl_test.cc @@ -622,8 +622,8 @@ TEST_F(ScaledRangeTimerManagerTest, LooksUpConfiguredMinimums) { // one calls into this one after looking up the minimum. class TestScaledRangeTimerManager : public ScaledRangeTimerManagerImpl { public: - using ScaledRangeTimerManagerImpl::ScaledRangeTimerManagerImpl; using ScaledRangeTimerManagerImpl::createTimer; + using ScaledRangeTimerManagerImpl::ScaledRangeTimerManagerImpl; TimerPtr createTimer(ScaledTimerMinimum minimum, TimerCb callback) override { return createScaledTimer(minimum, callback); } diff --git a/test/common/http/conn_manager_impl_test_base.cc b/test/common/http/conn_manager_impl_test_base.cc index 973e67bf5949..3407d3c6faed 100644 --- a/test/common/http/conn_manager_impl_test_base.cc +++ b/test/common/http/conn_manager_impl_test_base.cc @@ -53,7 +53,7 @@ void HttpConnectionManagerImplTest::setup(bool ssl, const std::string& server_na server_name_ = server_name; ON_CALL(filter_callbacks_.connection_, ssl()).WillByDefault(Return(ssl_connection_)); ON_CALL(Const(filter_callbacks_.connection_), ssl()).WillByDefault(Return(ssl_connection_)); - ON_CALL(overload_manager_.overload_state_, createScaledTypedTimer_) + ON_CALL(filter_callbacks_.connection_.dispatcher_, createScaledTypedTimer_) .WillByDefault([&](auto, auto callback) { return filter_callbacks_.connection_.dispatcher_.createTimer(callback).release(); }); diff --git a/test/mocks/api/mocks.cc b/test/mocks/api/mocks.cc index 6678bf4b15ba..5cf717fcfc91 100644 --- a/test/mocks/api/mocks.cc +++ b/test/mocks/api/mocks.cc @@ -23,10 +23,17 @@ MockApi::~MockApi() = default; Event::DispatcherPtr MockApi::allocateDispatcher(const std::string& name) { return Event::DispatcherPtr{allocateDispatcher_(name, time_system_)}; } + +Event::DispatcherPtr +MockApi::allocateDispatcher(const std::string& name, + const Event::ScaledRangeTimerManagerFactory& scaled_timer_factory) { + return Event::DispatcherPtr{ + allocateDispatcher_(name, scaled_timer_factory, nullptr, time_system_)}; +} Event::DispatcherPtr MockApi::allocateDispatcher(const std::string& name, Buffer::WatermarkFactoryPtr&& watermark_factory) { return Event::DispatcherPtr{ - allocateDispatcher_(name, std::move(watermark_factory), time_system_)}; + allocateDispatcher_(name, nullptr, std::move(watermark_factory), time_system_)}; } MockOsSysCalls::MockOsSysCalls() { diff --git a/test/mocks/api/mocks.h b/test/mocks/api/mocks.h index c6a8222c4298..3658717afce1 100644 --- a/test/mocks/api/mocks.h +++ b/test/mocks/api/mocks.h @@ -31,14 +31,18 @@ class MockApi : public Api { // Api::Api Event::DispatcherPtr allocateDispatcher(const std::string& name) override; + Event::DispatcherPtr + allocateDispatcher(const std::string& name, + const Event::ScaledRangeTimerManagerFactory& scaled_timer_factory) override; Event::DispatcherPtr allocateDispatcher(const std::string& name, Buffer::WatermarkFactoryPtr&& watermark_factory) override; TimeSource& timeSource() override { return time_system_; } MOCK_METHOD(Event::Dispatcher*, allocateDispatcher_, (const std::string&, Event::TimeSystem&)); MOCK_METHOD(Event::Dispatcher*, allocateDispatcher_, - (const std::string&, Buffer::WatermarkFactoryPtr&& watermark_factory, - Event::TimeSystem&)); + (const std::string&, + const Event::ScaledRangeTimerManagerFactory& scaled_timer_factory, + Buffer::WatermarkFactoryPtr&& watermark_factory, Event::TimeSystem&)); MOCK_METHOD(Filesystem::Instance&, fileSystem, ()); MOCK_METHOD(Thread::ThreadFactory&, threadFactory, ()); MOCK_METHOD(Stats::Scope&, rootScope, ()); diff --git a/test/mocks/event/mocks.h b/test/mocks/event/mocks.h index 850fe7b6447c..70880c495a82 100644 --- a/test/mocks/event/mocks.h +++ b/test/mocks/event/mocks.h @@ -83,6 +83,21 @@ class MockDispatcher : public Dispatcher { return timer; } + Event::TimerPtr createScaledTimer(ScaledTimerMinimum minimum, Event::TimerCb cb) override { + auto timer = Event::TimerPtr{createScaledTimer_(minimum, cb)}; + // Assert that the timer is not null to avoid confusing test failures down the line. + ASSERT(timer != nullptr); + return timer; + } + + Event::TimerPtr createScaledTimer(ScaledRangeTimerManager::TimerType timer_type, + Event::TimerCb cb) override { + auto timer = Event::TimerPtr{createScaledTypedTimer_(timer_type, cb)}; + // Assert that the timer is not null to avoid confusing test failures down the line. + ASSERT(timer != nullptr); + return timer; + } + Event::SchedulableCallbackPtr createSchedulableCallback(std::function cb) override { auto schedulable_cb = Event::SchedulableCallbackPtr{createSchedulableCallback_(cb)}; // Assert that schedulable_cb is not null to avoid confusing test failures down the line. @@ -124,6 +139,9 @@ class MockDispatcher : public Dispatcher { MOCK_METHOD(Network::UdpListener*, createUdpListener_, (Network::SocketSharedPtr socket, Network::UdpListenerCallbacks& cb)); MOCK_METHOD(Timer*, createTimer_, (Event::TimerCb cb)); + MOCK_METHOD(Timer*, createScaledTimer_, (ScaledTimerMinimum minimum, Event::TimerCb cb)); + MOCK_METHOD(Timer*, createScaledTypedTimer_, + (ScaledRangeTimerManager::TimerType timer_type, Event::TimerCb cb)); MOCK_METHOD(SchedulableCallback*, createSchedulableCallback_, (std::function cb)); MOCK_METHOD(void, deferredDelete_, (DeferredDeletable * to_delete)); MOCK_METHOD(void, exit, ()); diff --git a/test/mocks/event/wrapped_dispatcher.h b/test/mocks/event/wrapped_dispatcher.h index 974d61b39be2..26a6fe3c33fb 100644 --- a/test/mocks/event/wrapped_dispatcher.h +++ b/test/mocks/event/wrapped_dispatcher.h @@ -77,6 +77,13 @@ class WrappedDispatcher : public Dispatcher { } TimerPtr createTimer(TimerCb cb) override { return impl_.createTimer(std::move(cb)); } + TimerPtr createScaledTimer(ScaledTimerMinimum minimum, TimerCb cb) override { + return impl_.createScaledTimer(minimum, std::move(cb)); + } + + TimerPtr createScaledTimer(ScaledRangeTimerManager::TimerType timer_type, TimerCb cb) override { + return impl_.createScaledTimer(timer_type, std::move(cb)); + } Event::SchedulableCallbackPtr createSchedulableCallback(std::function cb) override { return impl_.createSchedulableCallback(std::move(cb)); diff --git a/test/mocks/server/overload_manager.cc b/test/mocks/server/overload_manager.cc index ef20eca27582..3cb951ea7f8e 100644 --- a/test/mocks/server/overload_manager.cc +++ b/test/mocks/server/overload_manager.cc @@ -17,18 +17,6 @@ using ::testing::ReturnRef; MockThreadLocalOverloadState::MockThreadLocalOverloadState() : disabled_state_(OverloadActionState::inactive()) { ON_CALL(*this, getState).WillByDefault(ReturnRef(disabled_state_)); - ON_CALL(*this, createScaledTypedTimer_).WillByDefault(ReturnNew>()); - ON_CALL(*this, createScaledMinimumTimer_).WillByDefault(ReturnNew>()); -} - -Event::TimerPtr MockThreadLocalOverloadState::createScaledTimer( - Event::ScaledRangeTimerManager::TimerType timer_type, Event::TimerCb callback) { - return Event::TimerPtr{createScaledTypedTimer_(timer_type, std::move(callback))}; -} - -Event::TimerPtr MockThreadLocalOverloadState::createScaledTimer(Event::ScaledTimerMinimum minimum, - Event::TimerCb callback) { - return Event::TimerPtr{createScaledMinimumTimer_(minimum, std::move(callback))}; } MockOverloadManager::MockOverloadManager() { diff --git a/test/mocks/server/overload_manager.h b/test/mocks/server/overload_manager.h index 7865ac1ffae8..e5ab03992836 100644 --- a/test/mocks/server/overload_manager.h +++ b/test/mocks/server/overload_manager.h @@ -14,14 +14,6 @@ class MockThreadLocalOverloadState : public ThreadLocalOverloadState { public: MockThreadLocalOverloadState(); MOCK_METHOD(const OverloadActionState&, getState, (const std::string&), (override)); - Event::TimerPtr createScaledTimer(Event::ScaledRangeTimerManager::TimerType timer_type, - Event::TimerCb callback) override; - Event::TimerPtr createScaledTimer(Event::ScaledTimerMinimum minimum, - Event::TimerCb callback) override; - MOCK_METHOD(Event::Timer*, createScaledTypedTimer_, - (Event::ScaledRangeTimerManager::TimerType, Event::TimerCb)); - MOCK_METHOD(Event::Timer*, createScaledMinimumTimer_, - (Event::ScaledTimerMinimum, Event::TimerCb)); private: const OverloadActionState disabled_state_; @@ -37,6 +29,7 @@ class MockOverloadManager : public OverloadManager { MOCK_METHOD(bool, registerForAction, (const std::string& action, Event::Dispatcher& dispatcher, OverloadActionCb callback)); + MOCK_METHOD(Event::ScaledRangeTimerManagerFactory, scaledTimerFactory, (), (override)); MOCK_METHOD(ThreadLocalOverloadState&, getThreadLocalOverloadState, ()); testing::NiceMock overload_state_; diff --git a/test/server/api_listener_test.cc b/test/server/api_listener_test.cc index 477b36dc9069..92267adb37b3 100644 --- a/test/server/api_listener_test.cc +++ b/test/server/api_listener_test.cc @@ -23,7 +23,12 @@ class ApiListenerTest : public testing::Test { protected: ApiListenerTest() : listener_manager_(std::make_unique(server_, listener_factory_, - worker_factory_, false)) {} + worker_factory_, false)) { + ON_CALL(server_.dispatcher_, createScaledTypedTimer_) + .WillByDefault([&](auto, Event::TimerCb callback) { + return server_.dispatcher_.createTimer_(std::move(callback)); + }); + } NiceMock server_; NiceMock listener_factory_; diff --git a/test/server/overload_manager_impl_test.cc b/test/server/overload_manager_impl_test.cc index da3b156cea70..7c971774d059 100644 --- a/test/server/overload_manager_impl_test.cc +++ b/test/server/overload_manager_impl_test.cc @@ -27,6 +27,7 @@ using testing::AnyNumber; using testing::ByMove; using testing::DoubleNear; using testing::Invoke; +using testing::InvokeArgument; using testing::MockFunction; using testing::NiceMock; using testing::Property; @@ -492,45 +493,39 @@ constexpr char kReducedTimeoutsConfig[] = R"YAML( saturation_threshold: 1.0 )YAML"; -TEST_F(OverloadManagerImplTest, AdjustScaleFactor) { - setDispatcherExpectation(); +TEST_F(OverloadManagerImplTest, CreateScaledTimerManager) { auto manager(createOverloadManager(kReducedTimeoutsConfig)); - auto* scaled_timer_manager = new Event::MockScaledRangeTimerManager(); + auto* mock_scaled_timer_manager = new Event::MockScaledRangeTimerManager(); EXPECT_CALL(*manager, createScaledRangeTimerManager) - .WillOnce(Return(ByMove(Event::ScaledRangeTimerManagerPtr{scaled_timer_manager}))); - - manager->start(); + .WillOnce(Return(ByMove(Event::ScaledRangeTimerManagerPtr{mock_scaled_timer_manager}))); - // The scaled trigger has range [0.5, 1.0] so 0.6 should map to a scale value of 0.2, which means - // a timer scale factor of 0.8 (1 - 0.2). - EXPECT_CALL(*scaled_timer_manager, setScaleFactor(DoubleNear(0.8, 0.00001))); - factory1_.monitor_->setPressure(0.6); + Event::MockDispatcher mock_dispatcher; + auto scaled_timer_manager = manager->scaledTimerFactory()(mock_dispatcher); - timer_cb_(); + EXPECT_EQ(scaled_timer_manager.get(), mock_scaled_timer_manager); } -TEST_F(OverloadManagerImplTest, CreateTypedTimer) { +TEST_F(OverloadManagerImplTest, AdjustScaleFactor) { setDispatcherExpectation(); auto manager(createOverloadManager(kReducedTimeoutsConfig)); - auto* scaled_timer_manager = new Event::MockScaledRangeTimerManager(); + auto* mock_scaled_timer_manager = new Event::MockScaledRangeTimerManager(); EXPECT_CALL(*manager, createScaledRangeTimerManager) - .WillOnce(Return(ByMove(Event::ScaledRangeTimerManagerPtr{scaled_timer_manager}))); - manager->start(); + .WillOnce(Return(ByMove(Event::ScaledRangeTimerManagerPtr{mock_scaled_timer_manager}))); - auto* mock_scaled_timer = new Event::MockTimer(); - MockFunction mock_callback; - EXPECT_CALL( - *scaled_timer_manager, - createTypedTimer_(Event::ScaledRangeTimerManager::TimerType::UnscaledRealTimerForTest, _)) - .WillOnce(Return(mock_scaled_timer)); + Event::MockDispatcher mock_dispatcher; + auto scaled_timer_manager = manager->scaledTimerFactory()(mock_dispatcher); + + manager->start(); - auto timer = manager->getThreadLocalOverloadState().createScaledTimer( - Event::ScaledRangeTimerManager::TimerType::UnscaledRealTimerForTest, - mock_callback.AsStdFunction()); + EXPECT_CALL(mock_dispatcher, post).WillOnce(InvokeArgument<0>()); + // The scaled trigger has range [0.5, 1.0] so 0.6 should map to a scale value of 0.2, which means + // a timer scale factor of 0.8 (1 - 0.2). + EXPECT_CALL(*mock_scaled_timer_manager, setScaleFactor(DoubleNear(0.8, 0.00001))); + factory1_.monitor_->setPressure(0.6); - EXPECT_EQ(mock_scaled_timer, timer.get()); + timer_cb_(); } TEST_F(OverloadManagerImplTest, DuplicateResourceMonitor) { From e39717a9a30d5263d16774e0c2e2f84a195ced4d Mon Sep 17 00:00:00 2001 From: Alex Konradi Date: Wed, 13 Jan 2021 13:52:44 -0500 Subject: [PATCH 3/9] Address review feedback Signed-off-by: Alex Konradi --- source/common/api/api_impl.cc | 7 ++-- source/common/event/dispatcher_impl.cc | 27 +++++++++---- source/common/event/dispatcher_impl.h | 7 +++- test/common/event/dispatcher_impl_test.cc | 47 +++++++++++++++++++++++ test/mocks/api/mocks.cc | 5 +-- test/mocks/event/mocks.cc | 3 ++ test/server/api_listener_test.cc | 7 +--- 7 files changed, 80 insertions(+), 23 deletions(-) diff --git a/source/common/api/api_impl.cc b/source/common/api/api_impl.cc index 9982fe2ac346..f91e5c6a1c6f 100644 --- a/source/common/api/api_impl.cc +++ b/source/common/api/api_impl.cc @@ -18,9 +18,9 @@ Impl::Impl(Thread::ThreadFactory& thread_factory, Stats::Store& store, process_context_(process_context), watermark_factory_(std::move(watermark_factory)) {} Event::DispatcherPtr Impl::allocateDispatcher(const std::string& name) { - return std::make_unique(name, *this, time_system_, nullptr, - watermark_factory_); + return std::make_unique(name, *this, time_system_, watermark_factory_); } + Event::DispatcherPtr Impl::allocateDispatcher(const std::string& name, const Event::ScaledRangeTimerManagerFactory& scaled_timer_factory) { @@ -30,8 +30,7 @@ Impl::allocateDispatcher(const std::string& name, Event::DispatcherPtr Impl::allocateDispatcher(const std::string& name, Buffer::WatermarkFactoryPtr&& factory) { - return std::make_unique(name, *this, time_system_, nullptr, - std::move(factory)); + return std::make_unique(name, *this, time_system_, std::move(factory)); } } // namespace Api diff --git a/source/common/event/dispatcher_impl.cc b/source/common/event/dispatcher_impl.cc index eea47145fd43..2d6ef4c9d89c 100644 --- a/source/common/event/dispatcher_impl.cc +++ b/source/common/event/dispatcher_impl.cc @@ -38,10 +38,24 @@ namespace Envoy { namespace Event { -DispatcherImpl::DispatcherImpl( - const std::string& name, Api::Api& api, Event::TimeSystem& time_system, - const std::function& scaled_timer_factory, - const Buffer::WatermarkFactorySharedPtr& watermark_factory) +DispatcherImpl::DispatcherImpl(const std::string& name, Api::Api& api, + Event::TimeSystem& time_system) + : DispatcherImpl(name, api, time_system, {}) {} + +DispatcherImpl::DispatcherImpl(const std::string& name, Api::Api& api, + Event::TimeSystem& time_system, + const Buffer::WatermarkFactorySharedPtr& watermark_factory) + : DispatcherImpl( + name, api, time_system, + [](Dispatcher& dispatcher) { + return std::make_unique(dispatcher); + }, + watermark_factory) {} + +DispatcherImpl::DispatcherImpl(const std::string& name, Api::Api& api, + Event::TimeSystem& time_system, + const ScaledRangeTimerManagerFactory& scaled_timer_factory, + const Buffer::WatermarkFactorySharedPtr& watermark_factory) : name_(name), api_(api), buffer_factory_(watermark_factory != nullptr ? watermark_factory @@ -50,10 +64,7 @@ DispatcherImpl::DispatcherImpl( deferred_delete_cb_(base_scheduler_.createSchedulableCallback( [this]() -> void { clearDeferredDeleteList(); })), post_cb_(base_scheduler_.createSchedulableCallback([this]() -> void { runPostCallbacks(); })), - current_to_delete_(&to_delete_1_), - scaled_timer_manager_(scaled_timer_factory != nullptr - ? scaled_timer_factory(*this) - : std::make_unique(*this)) { + current_to_delete_(&to_delete_1_), scaled_timer_manager_(scaled_timer_factory(*this)) { ASSERT(!name_.empty()); FatalErrorHandler::registerFatalErrorHandler(*this); updateApproximateMonotonicTimeInternal(); diff --git a/source/common/event/dispatcher_impl.h b/source/common/event/dispatcher_impl.h index 0692001045b8..609055f31f37 100644 --- a/source/common/event/dispatcher_impl.h +++ b/source/common/event/dispatcher_impl.h @@ -30,9 +30,12 @@ class DispatcherImpl : Logger::Loggable, public Dispatcher, public FatalErrorHandlerInterface { public: + DispatcherImpl(const std::string& name, Api::Api& api, Event::TimeSystem& time_system); + DispatcherImpl(const std::string& name, Api::Api& api, Event::TimeSystem& time_systems, + const Buffer::WatermarkFactorySharedPtr& watermark_factory); DispatcherImpl(const std::string& name, Api::Api& api, Event::TimeSystem& time_system, - const ScaledRangeTimerManagerFactory& scaled_timer_factory = nullptr, - const Buffer::WatermarkFactorySharedPtr& watermark_factory = nullptr); + const ScaledRangeTimerManagerFactory& scaled_timer_factory, + const Buffer::WatermarkFactorySharedPtr& watermark_factory); ~DispatcherImpl() override; /** diff --git a/test/common/event/dispatcher_impl_test.cc b/test/common/event/dispatcher_impl_test.cc index 8c612144db50..0cca917163e6 100644 --- a/test/common/event/dispatcher_impl_test.cc +++ b/test/common/event/dispatcher_impl_test.cc @@ -21,8 +21,11 @@ #include "gtest/gtest.h" using testing::_; +using testing::ByMove; using testing::InSequence; +using testing::MockFunction; using testing::NiceMock; +using testing::Return; namespace Envoy { namespace Event { @@ -1270,6 +1273,50 @@ TEST_F(TimerUtilsTest, TimerValueConversion) { checkConversion(std::chrono::milliseconds(600014), 600, 14000); } +TEST(DispatcherWithScaledTimerFactoryTest, CreatesScaledTimerManager) { + Api::ApiPtr api = Api::createApiForTest(); + MockFunction scaled_timer_manager_factory; + + MockScaledRangeTimerManager* manager = new MockScaledRangeTimerManager(); + EXPECT_CALL(scaled_timer_manager_factory, Call) + .WillOnce(Return(ByMove(ScaledRangeTimerManagerPtr(manager)))); + + DispatcherPtr dispatcher = + api->allocateDispatcher("test_thread", scaled_timer_manager_factory.AsStdFunction()); +} + +TEST(DispatcherWithScaledTimerFactoryTest, CreateScaledTimerWithMinimum) { + Api::ApiPtr api = Api::createApiForTest(); + MockFunction scaled_timer_manager_factory; + + MockScaledRangeTimerManager* manager = new MockScaledRangeTimerManager(); + EXPECT_CALL(scaled_timer_manager_factory, Call) + .WillOnce(Return(ByMove(ScaledRangeTimerManagerPtr(manager)))); + + DispatcherPtr dispatcher = + api->allocateDispatcher("test_thread", scaled_timer_manager_factory.AsStdFunction()); + + EXPECT_CALL(*manager, createTimer_(ScaledTimerMinimum(ScaledMinimum(UnitFloat(0.8f))), _)); + dispatcher->createScaledTimer(ScaledTimerMinimum(ScaledMinimum(UnitFloat(0.8f))), []() {}); +} + +TEST(DispatcherWithScaledTimerFactoryTest, CreateScaledTimerWithTimerType) { + Api::ApiPtr api = Api::createApiForTest(); + MockFunction scaled_timer_manager_factory; + + MockScaledRangeTimerManager* manager = new MockScaledRangeTimerManager(); + EXPECT_CALL(scaled_timer_manager_factory, Call) + .WillOnce(Return(ByMove(ScaledRangeTimerManagerPtr(manager)))); + + DispatcherPtr dispatcher = + api->allocateDispatcher("test_thread", scaled_timer_manager_factory.AsStdFunction()); + + EXPECT_CALL(*manager, + createTypedTimer_(ScaledRangeTimerManager::TimerType::UnscaledRealTimerForTest, _)); + dispatcher->createScaledTimer(ScaledRangeTimerManager::TimerType::UnscaledRealTimerForTest, + []() {}); +} + class DispatcherWithWatchdogTest : public testing::Test { protected: DispatcherWithWatchdogTest() diff --git a/test/mocks/api/mocks.cc b/test/mocks/api/mocks.cc index 5cf717fcfc91..c402005a6dde 100644 --- a/test/mocks/api/mocks.cc +++ b/test/mocks/api/mocks.cc @@ -27,13 +27,12 @@ Event::DispatcherPtr MockApi::allocateDispatcher(const std::string& name) { Event::DispatcherPtr MockApi::allocateDispatcher(const std::string& name, const Event::ScaledRangeTimerManagerFactory& scaled_timer_factory) { - return Event::DispatcherPtr{ - allocateDispatcher_(name, scaled_timer_factory, nullptr, time_system_)}; + return Event::DispatcherPtr{allocateDispatcher_(name, scaled_timer_factory, {}, time_system_)}; } Event::DispatcherPtr MockApi::allocateDispatcher(const std::string& name, Buffer::WatermarkFactoryPtr&& watermark_factory) { return Event::DispatcherPtr{ - allocateDispatcher_(name, nullptr, std::move(watermark_factory), time_system_)}; + allocateDispatcher_(name, {}, std::move(watermark_factory), time_system_)}; } MockOsSysCalls::MockOsSysCalls() { diff --git a/test/mocks/event/mocks.cc b/test/mocks/event/mocks.cc index a8db4995abb3..75b60c1cbccb 100644 --- a/test/mocks/event/mocks.cc +++ b/test/mocks/event/mocks.cc @@ -24,6 +24,9 @@ MockDispatcher::MockDispatcher(const std::string& name) : name_(name) { to_delete_.clear(); })); ON_CALL(*this, createTimer_(_)).WillByDefault(ReturnNew>()); + ON_CALL(*this, createScaledTimer_(_, _)).WillByDefault(ReturnNew>()); + ON_CALL(*this, createScaledTypedTimer_(_, _)) + .WillByDefault(ReturnNew>()); ON_CALL(*this, post(_)).WillByDefault(Invoke([](PostCb cb) -> void { cb(); })); ON_CALL(buffer_factory_, create_(_, _, _)) diff --git a/test/server/api_listener_test.cc b/test/server/api_listener_test.cc index 92267adb37b3..477b36dc9069 100644 --- a/test/server/api_listener_test.cc +++ b/test/server/api_listener_test.cc @@ -23,12 +23,7 @@ class ApiListenerTest : public testing::Test { protected: ApiListenerTest() : listener_manager_(std::make_unique(server_, listener_factory_, - worker_factory_, false)) { - ON_CALL(server_.dispatcher_, createScaledTypedTimer_) - .WillByDefault([&](auto, Event::TimerCb callback) { - return server_.dispatcher_.createTimer_(std::move(callback)); - }); - } + worker_factory_, false)) {} NiceMock server_; NiceMock listener_factory_; From 485693417f943f6c20901c878c115ee4860f4b05 Mon Sep 17 00:00:00 2001 From: Alex Konradi Date: Thu, 14 Jan 2021 10:28:26 -0500 Subject: [PATCH 4/9] Test timer map correctness Signed-off-by: Alex Konradi --- source/server/overload_manager_impl.cc | 10 ++++--- source/server/overload_manager_impl.h | 5 ++-- test/server/overload_manager_impl_test.cc | 33 +++++++++++++++++++---- 3 files changed, 37 insertions(+), 11 deletions(-) diff --git a/source/server/overload_manager_impl.cc b/source/server/overload_manager_impl.cc index 137d90dc88f6..0917b3ddf420 100644 --- a/source/server/overload_manager_impl.cc +++ b/source/server/overload_manager_impl.cc @@ -2,6 +2,7 @@ #include +#include "common/event/scaled_range_timer_manager_impl.h" #include "envoy/common/exception.h" #include "envoy/config/overload/v3/overload.pb.h" #include "envoy/config/overload/v3/overload.pb.validate.h" @@ -374,7 +375,7 @@ bool OverloadManagerImpl::registerForAction(const std::string& action, ThreadLocalOverloadState& OverloadManagerImpl::getThreadLocalOverloadState() { return *tls_; } Event::ScaledRangeTimerManagerFactory OverloadManagerImpl::scaledTimerFactory() { return [this](Event::Dispatcher& dispatcher) { - auto manager = createScaledRangeTimerManager(dispatcher); + auto manager = createScaledRangeTimerManager(dispatcher, timer_minimums_); registerForAction(OverloadActionNames::get().ReduceTimeouts, dispatcher, [manager = manager.get()](OverloadActionState scale_state) { manager->setScaleFactor( @@ -387,9 +388,10 @@ Event::ScaledRangeTimerManagerFactory OverloadManagerImpl::scaledTimerFactory() }; } -Event::ScaledRangeTimerManagerPtr -OverloadManagerImpl::createScaledRangeTimerManager(Event::Dispatcher& dispatcher) const { - return std::make_unique(dispatcher, timer_minimums_); +Event::ScaledRangeTimerManagerPtr OverloadManagerImpl::createScaledRangeTimerManager( + Event::Dispatcher& dispatcher, + const Event::ScaledRangeTimerManagerImpl::TimerTypeMapConstSharedPtr& timer_minimums) const { + return std::make_unique(dispatcher, timer_minimums); } void OverloadManagerImpl::updateResourcePressure(const std::string& resource, double pressure, diff --git a/source/server/overload_manager_impl.h b/source/server/overload_manager_impl.h index 50e9cb4fdd87..f55f4afb68f7 100644 --- a/source/server/overload_manager_impl.h +++ b/source/server/overload_manager_impl.h @@ -124,8 +124,9 @@ class OverloadManagerImpl : Logger::Loggable, public OverloadM protected: // Factory for timer managers. This allows test-only subclasses to inject a mock implementation. - virtual Event::ScaledRangeTimerManagerPtr - createScaledRangeTimerManager(Event::Dispatcher& dispatcher) const; + virtual Event::ScaledRangeTimerManagerPtr createScaledRangeTimerManager( + Event::Dispatcher& dispatcher, + const Event::ScaledRangeTimerManagerImpl::TimerTypeMapConstSharedPtr& timer_minimums) const; private: using FlushEpochId = uint64_t; diff --git a/test/server/overload_manager_impl_test.cc b/test/server/overload_manager_impl_test.cc index 4610eb677e65..e478021eedc8 100644 --- a/test/server/overload_manager_impl_test.cc +++ b/test/server/overload_manager_impl_test.cc @@ -1,6 +1,8 @@ #include +#include #include "envoy/config/overload/v3/overload.pb.h" +#include "envoy/event/scaled_range_timer_manager.h" #include "envoy/server/overload/overload_manager.h" #include "envoy/server/resource_monitor.h" #include "envoy/server/resource_monitor_config.h" @@ -25,18 +27,24 @@ using testing::_; using testing::AllOf; using testing::AnyNumber; using testing::ByMove; +using testing::DoAll; using testing::FloatNear; using testing::Invoke; using testing::InvokeArgument; using testing::MockFunction; using testing::NiceMock; +using testing::Pointee; using testing::Property; using testing::Return; +using testing::SaveArg; +using testing::UnorderedElementsAreArray; namespace Envoy { namespace Server { namespace { +using TimerType = Event::ScaledRangeTimerManager::TimerType; + class FakeResourceMonitor : public ResourceMonitor { public: FakeResourceMonitor(Event::Dispatcher& dispatcher) : dispatcher_(dispatcher), response_(0.0) {} @@ -121,12 +129,15 @@ class TestOverloadManager : public OverloadManagerImpl { } MOCK_METHOD(Event::ScaledRangeTimerManagerPtr, createScaledRangeTimerManager, - (Event::Dispatcher&), (const, override)); + (Event::Dispatcher&, + const Event::ScaledRangeTimerManagerImpl::TimerTypeMapConstSharedPtr&), + (const, override)); private: - Event::ScaledRangeTimerManagerPtr - createDefaultScaledRangeTimerManager(Event::Dispatcher& dispatcher) const { - return OverloadManagerImpl::createScaledRangeTimerManager(dispatcher); + Event::ScaledRangeTimerManagerPtr createDefaultScaledRangeTimerManager( + Event::Dispatcher& dispatcher, + const Event::ScaledRangeTimerManagerImpl::TimerTypeMapConstSharedPtr& timer_minimums) const { + return OverloadManagerImpl::createScaledRangeTimerManager(dispatcher, timer_minimums); } }; @@ -493,17 +504,29 @@ constexpr char kReducedTimeoutsConfig[] = R"YAML( saturation_threshold: 1.0 )YAML"; +// These are the timer types according to the reduced timeouts config above. +constexpr std::pair kReducedTimeoutsMinimums[]{ + {TimerType::HttpDownstreamIdleConnectionTimeout, + Event::AbsoluteMinimum(std::chrono::seconds(2))}, + {TimerType::HttpDownstreamIdleStreamTimeout, + Event::ScaledMinimum(UnitFloat(0.1))}, +}; TEST_F(OverloadManagerImplTest, CreateScaledTimerManager) { auto manager(createOverloadManager(kReducedTimeoutsConfig)); auto* mock_scaled_timer_manager = new Event::MockScaledRangeTimerManager(); + + Event::ScaledRangeTimerManagerImpl::TimerTypeMapConstSharedPtr timer_minimums; EXPECT_CALL(*manager, createScaledRangeTimerManager) - .WillOnce(Return(ByMove(Event::ScaledRangeTimerManagerPtr{mock_scaled_timer_manager}))); + .WillOnce( + DoAll(SaveArg<1>(&timer_minimums), + Return(ByMove(Event::ScaledRangeTimerManagerPtr{mock_scaled_timer_manager})))); Event::MockDispatcher mock_dispatcher; auto scaled_timer_manager = manager->scaledTimerFactory()(mock_dispatcher); EXPECT_EQ(scaled_timer_manager.get(), mock_scaled_timer_manager); + EXPECT_THAT(timer_minimums, Pointee(UnorderedElementsAreArray(kReducedTimeoutsMinimums))); } TEST_F(OverloadManagerImplTest, AdjustScaleFactor) { From db50f12761fdeff998cc86c2a0d1078c6610c607 Mon Sep 17 00:00:00 2001 From: Alex Konradi Date: Thu, 14 Jan 2021 13:50:52 -0500 Subject: [PATCH 5/9] Fix formatting Signed-off-by: Alex Konradi --- source/server/overload_manager_impl.cc | 2 +- test/server/overload_manager_impl_test.cc | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/source/server/overload_manager_impl.cc b/source/server/overload_manager_impl.cc index 0917b3ddf420..1f3e5c03c4ac 100644 --- a/source/server/overload_manager_impl.cc +++ b/source/server/overload_manager_impl.cc @@ -2,7 +2,6 @@ #include -#include "common/event/scaled_range_timer_manager_impl.h" #include "envoy/common/exception.h" #include "envoy/config/overload/v3/overload.pb.h" #include "envoy/config/overload/v3/overload.pb.validate.h" @@ -10,6 +9,7 @@ #include "common/common/fmt.h" #include "common/config/utility.h" +#include "common/event/scaled_range_timer_manager_impl.h" #include "common/protobuf/utility.h" #include "common/stats/symbol_table_impl.h" diff --git a/test/server/overload_manager_impl_test.cc b/test/server/overload_manager_impl_test.cc index e478021eedc8..08d50a3a29fc 100644 --- a/test/server/overload_manager_impl_test.cc +++ b/test/server/overload_manager_impl_test.cc @@ -508,8 +508,7 @@ constexpr char kReducedTimeoutsConfig[] = R"YAML( constexpr std::pair kReducedTimeoutsMinimums[]{ {TimerType::HttpDownstreamIdleConnectionTimeout, Event::AbsoluteMinimum(std::chrono::seconds(2))}, - {TimerType::HttpDownstreamIdleStreamTimeout, - Event::ScaledMinimum(UnitFloat(0.1))}, + {TimerType::HttpDownstreamIdleStreamTimeout, Event::ScaledMinimum(UnitFloat(0.1))}, }; TEST_F(OverloadManagerImplTest, CreateScaledTimerManager) { auto manager(createOverloadManager(kReducedTimeoutsConfig)); From c2e548acc164b34b0ea6b6b5d828a5fbe9660826 Mon Sep 17 00:00:00 2001 From: Alex Konradi Date: Thu, 14 Jan 2021 14:20:54 -0500 Subject: [PATCH 6/9] Move TimerType enum Signed-off-by: Alex Konradi --- include/envoy/api/BUILD | 1 + include/envoy/api/api.h | 1 + include/envoy/event/BUILD | 8 +++---- include/envoy/event/dispatcher.h | 7 +++--- .../envoy/event/scaled_range_timer_manager.h | 15 ++----------- ...{scaled_timer_minimum.h => scaled_timer.h} | 15 +++++++++++++ include/envoy/server/overload/BUILD | 1 - .../overload/thread_local_overload_state.h | 1 - source/common/event/dispatcher_impl.cc | 3 +-- source/common/event/dispatcher_impl.h | 2 +- .../event/scaled_range_timer_manager_impl.cc | 6 ++--- .../event/scaled_range_timer_manager_impl.h | 11 ++++------ source/common/http/conn_manager_impl.cc | 6 ++--- source/server/overload_manager_impl.cc | 22 +++++++++---------- source/server/overload_manager_impl.h | 4 ++-- test/common/event/dispatcher_impl_test.cc | 6 ++--- .../scaled_range_timer_manager_impl_test.cc | 11 ++++------ test/mocks/event/mocks.h | 10 ++++----- test/mocks/event/wrapped_dispatcher.h | 2 +- test/server/overload_manager_impl_test.cc | 9 ++++---- 20 files changed, 65 insertions(+), 76 deletions(-) rename include/envoy/event/{scaled_timer_minimum.h => scaled_timer.h} (75%) diff --git a/include/envoy/api/BUILD b/include/envoy/api/BUILD index 2d9c47ddc0b6..36736838c6d8 100644 --- a/include/envoy/api/BUILD +++ b/include/envoy/api/BUILD @@ -14,6 +14,7 @@ envoy_cc_library( deps = [ "//include/envoy/common:random_generator_interface", "//include/envoy/event:dispatcher_interface", + "//include/envoy/event:scaled_range_timer_manager_interface", "//include/envoy/filesystem:filesystem_interface", "//include/envoy/server:process_context_interface", "//include/envoy/thread:thread_interface", diff --git a/include/envoy/api/api.h b/include/envoy/api/api.h index 1bb0ebb5abc6..89336aaf5734 100644 --- a/include/envoy/api/api.h +++ b/include/envoy/api/api.h @@ -6,6 +6,7 @@ #include "envoy/common/random_generator.h" #include "envoy/common/time.h" #include "envoy/event/dispatcher.h" +#include "envoy/event/scaled_range_timer_manager.h" #include "envoy/filesystem/filesystem.h" #include "envoy/server/process_context.h" #include "envoy/stats/store.h" diff --git a/include/envoy/event/BUILD b/include/envoy/event/BUILD index 8b357d194a79..91d13f4d113c 100644 --- a/include/envoy/event/BUILD +++ b/include/envoy/event/BUILD @@ -19,7 +19,7 @@ envoy_cc_library( deps = [ ":deferred_deletable", ":file_event_interface", - ":scaled_range_timer_manager_interface", + ":scaled_timer", ":schedulable_cb_interface", ":signal_interface", ":timer_interface", @@ -43,8 +43,8 @@ envoy_cc_library( ) envoy_cc_library( - name = "scaled_timer_minimum", - hdrs = ["scaled_timer_minimum.h"], + name = "scaled_timer", + hdrs = ["scaled_timer.h"], deps = [ "//source/common/common:interval_value", ], @@ -54,7 +54,7 @@ envoy_cc_library( name = "scaled_range_timer_manager_interface", hdrs = ["scaled_range_timer_manager.h"], deps = [ - ":scaled_timer_minimum", + ":scaled_timer", ":timer_interface", ], ) diff --git a/include/envoy/event/dispatcher.h b/include/envoy/event/dispatcher.h index 737d5be8b293..1af37f7825ed 100644 --- a/include/envoy/event/dispatcher.h +++ b/include/envoy/event/dispatcher.h @@ -9,7 +9,7 @@ #include "envoy/common/scope_tracker.h" #include "envoy/common/time.h" #include "envoy/event/file_event.h" -#include "envoy/event/scaled_range_timer_manager.h" +#include "envoy/event/scaled_timer.h" #include "envoy/event/schedulable_cb.h" #include "envoy/event/signal.h" #include "envoy/event/timer.h" @@ -83,15 +83,14 @@ class DispatcherBase { * @param timer_type the type of timer to create. * @param cb supplies the callback to invoke when the timer fires. */ - virtual Event::TimerPtr createScaledTimer(Event::ScaledRangeTimerManager::TimerType timer_type, - TimerCb cb) PURE; + virtual Event::TimerPtr createScaledTimer(Event::ScaledTimerType timer_type, TimerCb cb) PURE; /** * Allocates a scaled timer. @see Timer for docs on how to use the timer. * @param minimum the rule for computing the minimum value of the timer. * @param cb supplies the callback to invoke when the timer fires. */ - virtual Event::TimerPtr createScaledTimer(ScaledTimerMinimum minimum, TimerCb cb) PURE; + virtual Event::TimerPtr createScaledTimer(Event::ScaledTimerMinimum minimum, TimerCb cb) PURE; /** * Allocates a schedulable callback. @see SchedulableCallback for docs on how to use the wrapped diff --git a/include/envoy/event/scaled_range_timer_manager.h b/include/envoy/event/scaled_range_timer_manager.h index 02a9ddeea1e3..8b91ec5758fc 100644 --- a/include/envoy/event/scaled_range_timer_manager.h +++ b/include/envoy/event/scaled_range_timer_manager.h @@ -1,7 +1,7 @@ #pragma once #include "envoy/common/pure.h" -#include "envoy/event/scaled_timer_minimum.h" +#include "envoy/event/scaled_timer.h" #include "envoy/event/timer.h" #include "common/common/interval_value.h" @@ -20,17 +20,6 @@ namespace Event { */ class ScaledRangeTimerManager { public: - enum class TimerType { - // Timers created with this type will never be scaled. This should only be used for testing. - UnscaledRealTimerForTest, - // The amount of time an HTTP connection to a downstream client can remain idle (no streams). - // This corresponds to the HTTP_DOWNSTREAM_CONNECTION_IDLE TimerType in overload.proto. - HttpDownstreamIdleConnectionTimeout, - // The amount of time an HTTP stream from a downstream client can remain idle. This corresponds - // to the HTTP_DOWNSTREAM_STREAM_IDLE TimerType in overload.proto. - HttpDownstreamIdleStreamTimeout, - }; - virtual ~ScaledRangeTimerManager() = default; /** @@ -45,7 +34,7 @@ class ScaledRangeTimerManager { * Creates a new timer backed by the manager using the provided timer type to * determine the minimum. */ - virtual TimerPtr createTimer(TimerType timer_type, TimerCb callback) PURE; + virtual TimerPtr createTimer(ScaledTimerType timer_type, TimerCb callback) PURE; /** * Sets the scale factor for all timers created through this manager. The value should be between diff --git a/include/envoy/event/scaled_timer_minimum.h b/include/envoy/event/scaled_timer.h similarity index 75% rename from include/envoy/event/scaled_timer_minimum.h rename to include/envoy/event/scaled_timer.h index bf8cfd3715b8..63ac8a5f421e 100644 --- a/include/envoy/event/scaled_timer_minimum.h +++ b/include/envoy/event/scaled_timer.h @@ -4,6 +4,7 @@ #include "common/common/interval_value.h" +#include "absl/container/flat_hash_map.h" #include "absl/types/variant.h" namespace Envoy { @@ -65,5 +66,19 @@ class ScaledTimerMinimum { absl::variant impl_; }; +enum class ScaledTimerType { + // Timers created with this type will never be scaled. This should only be used for testing. + UnscaledRealTimerForTest, + // The amount of time an HTTP connection to a downstream client can remain idle (no streams). This + // corresponds to the HTTP_DOWNSTREAM_CONNECTION_IDLE TimerType in overload.proto. + HttpDownstreamIdleConnectionTimeout, + // The amount of time an HTTP stream from a downstream client can remain idle. This corresponds to + // the HTTP_DOWNSTREAM_STREAM_IDLE TimerType in overload.proto. + HttpDownstreamIdleStreamTimeout, +}; + +using ScaledTimerTypeMap = absl::flat_hash_map; +using ScaledTimerTypeMapConstSharedPtr = std::shared_ptr; + } // namespace Event } // namespace Envoy diff --git a/include/envoy/server/overload/BUILD b/include/envoy/server/overload/BUILD index a0a631653882..a6e3f789c444 100644 --- a/include/envoy/server/overload/BUILD +++ b/include/envoy/server/overload/BUILD @@ -24,7 +24,6 @@ envoy_cc_library( hdrs = ["thread_local_overload_state.h"], deps = [ "//include/envoy/event:scaled_range_timer_manager_interface", - "//include/envoy/event:scaled_timer_minimum", "//include/envoy/event:timer_interface", "//include/envoy/thread_local:thread_local_object", "//source/common/common:interval_value", diff --git a/include/envoy/server/overload/thread_local_overload_state.h b/include/envoy/server/overload/thread_local_overload_state.h index ba5ceb1cdf1c..23c5e7add043 100644 --- a/include/envoy/server/overload/thread_local_overload_state.h +++ b/include/envoy/server/overload/thread_local_overload_state.h @@ -5,7 +5,6 @@ #include "envoy/common/pure.h" #include "envoy/event/scaled_range_timer_manager.h" -#include "envoy/event/scaled_timer_minimum.h" #include "envoy/event/timer.h" #include "envoy/thread_local/thread_local_object.h" diff --git a/source/common/event/dispatcher_impl.cc b/source/common/event/dispatcher_impl.cc index 2d6ef4c9d89c..9537fb5a3490 100644 --- a/source/common/event/dispatcher_impl.cc +++ b/source/common/event/dispatcher_impl.cc @@ -207,8 +207,7 @@ TimerPtr DispatcherImpl::createTimer(TimerCb cb) { return createTimerInternal(cb); } -TimerPtr DispatcherImpl::createScaledTimer(ScaledRangeTimerManager::TimerType timer_type, - TimerCb cb) { +TimerPtr DispatcherImpl::createScaledTimer(ScaledTimerType timer_type, TimerCb cb) { ASSERT(isThreadSafe()); return scaled_timer_manager_->createTimer(timer_type, std::move(cb)); } diff --git a/source/common/event/dispatcher_impl.h b/source/common/event/dispatcher_impl.h index 609055f31f37..da3da167107f 100644 --- a/source/common/event/dispatcher_impl.h +++ b/source/common/event/dispatcher_impl.h @@ -71,7 +71,7 @@ class DispatcherImpl : Logger::Loggable, Network::UdpListenerPtr createUdpListener(Network::SocketSharedPtr socket, Network::UdpListenerCallbacks& cb) override; TimerPtr createTimer(TimerCb cb) override; - TimerPtr createScaledTimer(ScaledRangeTimerManager::TimerType timer_type, TimerCb cb) override; + TimerPtr createScaledTimer(ScaledTimerType timer_type, TimerCb cb) override; TimerPtr createScaledTimer(ScaledTimerMinimum minimum, TimerCb cb) override; Event::SchedulableCallbackPtr createSchedulableCallback(std::function cb) override; diff --git a/source/common/event/scaled_range_timer_manager_impl.cc b/source/common/event/scaled_range_timer_manager_impl.cc index 2778d13505f8..baea9de390bf 100644 --- a/source/common/event/scaled_range_timer_manager_impl.cc +++ b/source/common/event/scaled_range_timer_manager_impl.cc @@ -138,10 +138,10 @@ class ScaledRangeTimerManagerImpl::RangeTimerImpl final : public Timer { }; ScaledRangeTimerManagerImpl::ScaledRangeTimerManagerImpl( - Dispatcher& dispatcher, const TimerTypeMapConstSharedPtr& timer_minimums) + Dispatcher& dispatcher, const ScaledTimerTypeMapConstSharedPtr& timer_minimums) : dispatcher_(dispatcher), timer_minimums_(timer_minimums != nullptr ? timer_minimums - : std::make_shared()), + : std::make_shared()), scale_factor_(1.0) {} ScaledRangeTimerManagerImpl::~ScaledRangeTimerManagerImpl() { @@ -150,7 +150,7 @@ ScaledRangeTimerManagerImpl::~ScaledRangeTimerManagerImpl() { ASSERT(queues_.empty()); } -TimerPtr ScaledRangeTimerManagerImpl::createTimer(TimerType timer_type, TimerCb callback) { +TimerPtr ScaledRangeTimerManagerImpl::createTimer(ScaledTimerType timer_type, TimerCb callback) { const auto minimum_it = timer_minimums_->find(timer_type); const Event::ScaledTimerMinimum minimum = minimum_it != timer_minimums_->end() diff --git a/source/common/event/scaled_range_timer_manager_impl.h b/source/common/event/scaled_range_timer_manager_impl.h index eff92c19bf34..4c36b50f5de0 100644 --- a/source/common/event/scaled_range_timer_manager_impl.h +++ b/source/common/event/scaled_range_timer_manager_impl.h @@ -23,17 +23,14 @@ namespace Event { */ class ScaledRangeTimerManagerImpl : public ScaledRangeTimerManager { public: - using TimerTypeMap = absl::flat_hash_map; - using TimerTypeMapConstSharedPtr = std::shared_ptr; - // Takes a Dispatcher and a map from timer type to scaled minimum value. ScaledRangeTimerManagerImpl(Dispatcher& dispatcher, - const TimerTypeMapConstSharedPtr& timer_minimums = nullptr); + const ScaledTimerTypeMapConstSharedPtr& timer_minimums = nullptr); ~ScaledRangeTimerManagerImpl() override; // ScaledRangeTimerManager impl TimerPtr createTimer(ScaledTimerMinimum minimum, TimerCb callback) override; - TimerPtr createTimer(TimerType timer_type, TimerCb callback) override; + TimerPtr createTimer(ScaledTimerType timer_type, TimerCb callback) override; void setScaleFactor(UnitFloat scale_factor) override; private: @@ -122,10 +119,10 @@ class ScaledRangeTimerManagerImpl : public ScaledRangeTimerManager { void onQueueTimerFired(Queue& queue); Dispatcher& dispatcher_; - const TimerTypeMapConstSharedPtr timer_minimums_; + const ScaledTimerTypeMapConstSharedPtr timer_minimums_; UnitFloat scale_factor_; absl::flat_hash_set, Hash, Eq> queues_; }; } // namespace Event -} // namespace Envoy \ No newline at end of file +} // namespace Envoy diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index 4cc466f32e0a..e2ceee4f1c2a 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -123,7 +123,7 @@ void ConnectionManagerImpl::initializeReadFilterCallbacks(Network::ReadFilterCal if (config_.idleTimeout()) { connection_idle_timer_ = read_callbacks_->connection().dispatcher().createScaledTimer( - Event::ScaledRangeTimerManager::TimerType::HttpDownstreamIdleConnectionTimeout, + Event::ScaledTimerType::HttpDownstreamIdleConnectionTimeout, [this]() -> void { onIdleTimeout(); }); connection_idle_timer_->enableTimer(config_.idleTimeout().value()); } @@ -630,7 +630,7 @@ ConnectionManagerImpl::ActiveStream::ActiveStream(ConnectionManagerImpl& connect idle_timeout_ms_ = connection_manager_.config_.streamIdleTimeout(); stream_idle_timer_ = connection_manager_.read_callbacks_->connection().dispatcher().createScaledTimer( - Event::ScaledRangeTimerManager::TimerType::HttpDownstreamIdleStreamTimeout, + Event::ScaledTimerType::HttpDownstreamIdleStreamTimeout, [this]() -> void { onIdleTimeout(); }); resetIdleTimer(); } @@ -1055,7 +1055,7 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(RequestHeaderMapPtr&& he if (stream_idle_timer_ == nullptr) { stream_idle_timer_ = connection_manager_.read_callbacks_->connection().dispatcher().createScaledTimer( - Event::ScaledRangeTimerManager::TimerType::HttpDownstreamIdleStreamTimeout, + Event::ScaledTimerType::HttpDownstreamIdleStreamTimeout, [this]() -> void { onIdleTimeout(); }); } } else if (stream_idle_timer_ != nullptr) { diff --git a/source/server/overload_manager_impl.cc b/source/server/overload_manager_impl.cc index 1f3e5c03c4ac..5c9b7266c453 100644 --- a/source/server/overload_manager_impl.cc +++ b/source/server/overload_manager_impl.cc @@ -21,8 +21,6 @@ namespace Envoy { namespace Server { -using TimerTypeMap = Event::ScaledRangeTimerManagerImpl::TimerTypeMap; - /** * Thread-local copy of the state of each configured overload action. */ @@ -122,31 +120,31 @@ Stats::Gauge& makeGauge(Stats::Scope& scope, absl::string_view a, absl::string_v return scope.gaugeFromStatName(stat_name.statName(), import_mode); } -Event::ScaledRangeTimerManager::TimerType parseTimerType( +Event::ScaledTimerType parseTimerType( envoy::config::overload::v3::ScaleTimersOverloadActionConfig::TimerType config_timer_type) { using Config = envoy::config::overload::v3::ScaleTimersOverloadActionConfig; switch (config_timer_type) { case Config::HTTP_DOWNSTREAM_CONNECTION_IDLE: - return Event::ScaledRangeTimerManager::TimerType::HttpDownstreamIdleConnectionTimeout; + return Event::ScaledTimerType::HttpDownstreamIdleConnectionTimeout; case Config::HTTP_DOWNSTREAM_STREAM_IDLE: - return Event::ScaledRangeTimerManager::TimerType::HttpDownstreamIdleStreamTimeout; + return Event::ScaledTimerType::HttpDownstreamIdleStreamTimeout; default: throw EnvoyException(fmt::format("Unknown timer type {}", config_timer_type)); } } -TimerTypeMap parseTimerMinimums(const ProtobufWkt::Any& typed_config, - ProtobufMessage::ValidationVisitor& validation_visitor) { +Event::ScaledTimerTypeMap +parseTimerMinimums(const ProtobufWkt::Any& typed_config, + ProtobufMessage::ValidationVisitor& validation_visitor) { using Config = envoy::config::overload::v3::ScaleTimersOverloadActionConfig; const Config action_config = MessageUtil::anyConvertAndValidate(typed_config, validation_visitor); - TimerTypeMap timer_map; + Event::ScaledTimerTypeMap timer_map; for (const auto& scale_timer : action_config.timer_scale_factors()) { - const Event::ScaledRangeTimerManager::TimerType timer_type = - parseTimerType(scale_timer.timer()); + const Event::ScaledTimerType timer_type = parseTimerType(scale_timer.timer()); const Event::ScaledTimerMinimum minimum = scale_timer.has_min_timeout() @@ -296,7 +294,7 @@ OverloadManagerImpl::OverloadManagerImpl(Event::Dispatcher& dispatcher, Stats::S } if (name == OverloadActionNames::get().ReduceTimeouts) { - timer_minimums_ = std::make_shared( + timer_minimums_ = std::make_shared( parseTimerMinimums(action.typed_config(), validation_visitor)); } else if (action.has_typed_config()) { throw EnvoyException(fmt::format( @@ -390,7 +388,7 @@ Event::ScaledRangeTimerManagerFactory OverloadManagerImpl::scaledTimerFactory() Event::ScaledRangeTimerManagerPtr OverloadManagerImpl::createScaledRangeTimerManager( Event::Dispatcher& dispatcher, - const Event::ScaledRangeTimerManagerImpl::TimerTypeMapConstSharedPtr& timer_minimums) const { + const Event::ScaledTimerTypeMapConstSharedPtr& timer_minimums) const { return std::make_unique(dispatcher, timer_minimums); } diff --git a/source/server/overload_manager_impl.h b/source/server/overload_manager_impl.h index f55f4afb68f7..6dcfff6d032b 100644 --- a/source/server/overload_manager_impl.h +++ b/source/server/overload_manager_impl.h @@ -126,7 +126,7 @@ class OverloadManagerImpl : Logger::Loggable, public OverloadM // Factory for timer managers. This allows test-only subclasses to inject a mock implementation. virtual Event::ScaledRangeTimerManagerPtr createScaledRangeTimerManager( Event::Dispatcher& dispatcher, - const Event::ScaledRangeTimerManagerImpl::TimerTypeMapConstSharedPtr& timer_minimums) const; + const Event::ScaledTimerTypeMapConstSharedPtr& timer_minimums) const; private: using FlushEpochId = uint64_t; @@ -173,7 +173,7 @@ class OverloadManagerImpl : Logger::Loggable, public OverloadM absl::node_hash_map resources_; absl::node_hash_map actions_; - Event::ScaledRangeTimerManagerImpl::TimerTypeMapConstSharedPtr timer_minimums_; + Event::ScaledTimerTypeMapConstSharedPtr timer_minimums_; absl::flat_hash_map state_updates_to_flush_; diff --git a/test/common/event/dispatcher_impl_test.cc b/test/common/event/dispatcher_impl_test.cc index 0cca917163e6..f08340468610 100644 --- a/test/common/event/dispatcher_impl_test.cc +++ b/test/common/event/dispatcher_impl_test.cc @@ -1311,10 +1311,8 @@ TEST(DispatcherWithScaledTimerFactoryTest, CreateScaledTimerWithTimerType) { DispatcherPtr dispatcher = api->allocateDispatcher("test_thread", scaled_timer_manager_factory.AsStdFunction()); - EXPECT_CALL(*manager, - createTypedTimer_(ScaledRangeTimerManager::TimerType::UnscaledRealTimerForTest, _)); - dispatcher->createScaledTimer(ScaledRangeTimerManager::TimerType::UnscaledRealTimerForTest, - []() {}); + EXPECT_CALL(*manager, createTypedTimer_(ScaledTimerType::UnscaledRealTimerForTest, _)); + dispatcher->createScaledTimer(ScaledTimerType::UnscaledRealTimerForTest, []() {}); } class DispatcherWithWatchdogTest : public testing::Test { diff --git a/test/common/event/scaled_range_timer_manager_impl_test.cc b/test/common/event/scaled_range_timer_manager_impl_test.cc index dd5da77d19d9..8900c5d96d4c 100644 --- a/test/common/event/scaled_range_timer_manager_impl_test.cc +++ b/test/common/event/scaled_range_timer_manager_impl_test.cc @@ -630,13 +630,10 @@ TEST_F(ScaledRangeTimerManagerTest, LooksUpConfiguredMinimums) { MOCK_METHOD(TimerPtr, createScaledTimer, (ScaledTimerMinimum, TimerCb)); }; - const ScaledRangeTimerManagerImpl::TimerTypeMap timer_types{ - {ScaledRangeTimerManager::TimerType::UnscaledRealTimerForTest, - ScaledMinimum(UnitFloat::max())}, - {ScaledRangeTimerManager::TimerType::HttpDownstreamIdleConnectionTimeout, - ScaledMinimum(UnitFloat(0.3))}, - {ScaledRangeTimerManager::TimerType::HttpDownstreamIdleStreamTimeout, - ScaledMinimum(UnitFloat(0.6))}, + const ScaledTimerTypeMap timer_types{ + {ScaledTimerType::UnscaledRealTimerForTest, ScaledMinimum(UnitFloat::max())}, + {ScaledTimerType::HttpDownstreamIdleConnectionTimeout, ScaledMinimum(UnitFloat(0.3))}, + {ScaledTimerType::HttpDownstreamIdleStreamTimeout, ScaledMinimum(UnitFloat(0.6))}, }; TestScaledRangeTimerManager manager(dispatcher_, diff --git a/test/mocks/event/mocks.h b/test/mocks/event/mocks.h index 5818061d4357..acbc1f1d89f9 100644 --- a/test/mocks/event/mocks.h +++ b/test/mocks/event/mocks.h @@ -90,8 +90,7 @@ class MockDispatcher : public Dispatcher { return timer; } - Event::TimerPtr createScaledTimer(ScaledRangeTimerManager::TimerType timer_type, - Event::TimerCb cb) override { + Event::TimerPtr createScaledTimer(ScaledTimerType timer_type, Event::TimerCb cb) override { auto timer = Event::TimerPtr{createScaledTypedTimer_(timer_type, cb)}; // Assert that the timer is not null to avoid confusing test failures down the line. ASSERT(timer != nullptr); @@ -140,8 +139,7 @@ class MockDispatcher : public Dispatcher { (Network::SocketSharedPtr socket, Network::UdpListenerCallbacks& cb)); MOCK_METHOD(Timer*, createTimer_, (Event::TimerCb cb)); MOCK_METHOD(Timer*, createScaledTimer_, (ScaledTimerMinimum minimum, Event::TimerCb cb)); - MOCK_METHOD(Timer*, createScaledTypedTimer_, - (ScaledRangeTimerManager::TimerType timer_type, Event::TimerCb cb)); + MOCK_METHOD(Timer*, createScaledTypedTimer_, (ScaledTimerType timer_type, Event::TimerCb cb)); MOCK_METHOD(SchedulableCallback*, createSchedulableCallback_, (std::function cb)); MOCK_METHOD(void, deferredDelete_, (DeferredDeletable * to_delete)); MOCK_METHOD(void, exit, ()); @@ -202,11 +200,11 @@ class MockScaledRangeTimerManager : public ScaledRangeTimerManager { TimerPtr createTimer(ScaledTimerMinimum minimum, TimerCb callback) override { return TimerPtr{createTimer_(minimum, std::move(callback))}; } - TimerPtr createTimer(ScaledRangeTimerManager::TimerType timer_type, TimerCb callback) override { + TimerPtr createTimer(ScaledTimerType timer_type, TimerCb callback) override { return TimerPtr{createTypedTimer_(timer_type, std::move(callback))}; } MOCK_METHOD(Timer*, createTimer_, (ScaledTimerMinimum, TimerCb)); - MOCK_METHOD(Timer*, createTypedTimer_, (ScaledRangeTimerManager::TimerType, TimerCb)); + MOCK_METHOD(Timer*, createTypedTimer_, (ScaledTimerType, TimerCb)); MOCK_METHOD(void, setScaleFactor, (UnitFloat), (override)); }; diff --git a/test/mocks/event/wrapped_dispatcher.h b/test/mocks/event/wrapped_dispatcher.h index 26a6fe3c33fb..1f9b912bd31b 100644 --- a/test/mocks/event/wrapped_dispatcher.h +++ b/test/mocks/event/wrapped_dispatcher.h @@ -81,7 +81,7 @@ class WrappedDispatcher : public Dispatcher { return impl_.createScaledTimer(minimum, std::move(cb)); } - TimerPtr createScaledTimer(ScaledRangeTimerManager::TimerType timer_type, TimerCb cb) override { + TimerPtr createScaledTimer(ScaledTimerType timer_type, TimerCb cb) override { return impl_.createScaledTimer(timer_type, std::move(cb)); } diff --git a/test/server/overload_manager_impl_test.cc b/test/server/overload_manager_impl_test.cc index 08d50a3a29fc..5952a094980c 100644 --- a/test/server/overload_manager_impl_test.cc +++ b/test/server/overload_manager_impl_test.cc @@ -43,7 +43,7 @@ namespace Envoy { namespace Server { namespace { -using TimerType = Event::ScaledRangeTimerManager::TimerType; +using TimerType = Event::ScaledTimerType; class FakeResourceMonitor : public ResourceMonitor { public: @@ -129,14 +129,13 @@ class TestOverloadManager : public OverloadManagerImpl { } MOCK_METHOD(Event::ScaledRangeTimerManagerPtr, createScaledRangeTimerManager, - (Event::Dispatcher&, - const Event::ScaledRangeTimerManagerImpl::TimerTypeMapConstSharedPtr&), + (Event::Dispatcher&, const Event::ScaledTimerTypeMapConstSharedPtr&), (const, override)); private: Event::ScaledRangeTimerManagerPtr createDefaultScaledRangeTimerManager( Event::Dispatcher& dispatcher, - const Event::ScaledRangeTimerManagerImpl::TimerTypeMapConstSharedPtr& timer_minimums) const { + const Event::ScaledTimerTypeMapConstSharedPtr& timer_minimums) const { return OverloadManagerImpl::createScaledRangeTimerManager(dispatcher, timer_minimums); } }; @@ -515,7 +514,7 @@ TEST_F(OverloadManagerImplTest, CreateScaledTimerManager) { auto* mock_scaled_timer_manager = new Event::MockScaledRangeTimerManager(); - Event::ScaledRangeTimerManagerImpl::TimerTypeMapConstSharedPtr timer_minimums; + Event::ScaledTimerTypeMapConstSharedPtr timer_minimums; EXPECT_CALL(*manager, createScaledRangeTimerManager) .WillOnce( DoAll(SaveArg<1>(&timer_minimums), From 58cc3c525130e3d1bd84eb5b5c7a3da8695c944c Mon Sep 17 00:00:00 2001 From: Alex Konradi Date: Thu, 14 Jan 2021 17:16:28 -0500 Subject: [PATCH 7/9] Add missing override in Api::ValidationImpl Signed-off-by: Alex Konradi --- source/server/config_validation/api.cc | 6 ++++++ source/server/config_validation/api.h | 2 ++ 2 files changed, 8 insertions(+) diff --git a/source/server/config_validation/api.cc b/source/server/config_validation/api.cc index fb6152b9053d..f63eaf1895f4 100644 --- a/source/server/config_validation/api.cc +++ b/source/server/config_validation/api.cc @@ -17,6 +17,12 @@ Event::DispatcherPtr ValidationImpl::allocateDispatcher(const std::string& name) return Event::DispatcherPtr{new Event::ValidationDispatcher(name, *this, time_system_)}; } +Event::DispatcherPtr +ValidationImpl::allocateDispatcher(const std::string& name, + const Event::ScaledRangeTimerManagerFactory&) { + return Event::DispatcherPtr{new Event::ValidationDispatcher(name, *this, time_system_)}; +} + Event::DispatcherPtr ValidationImpl::allocateDispatcher(const std::string&, Buffer::WatermarkFactoryPtr&&) { NOT_REACHED_GCOVR_EXCL_LINE; diff --git a/source/server/config_validation/api.h b/source/server/config_validation/api.h index 21c0dca05820..2d44022c191a 100644 --- a/source/server/config_validation/api.h +++ b/source/server/config_validation/api.h @@ -20,6 +20,8 @@ class ValidationImpl : public Impl { Random::RandomGenerator& random_generator); Event::DispatcherPtr allocateDispatcher(const std::string& name) override; + Event::DispatcherPtr allocateDispatcher(const std::string& name, + const Event::ScaledRangeTimerManagerFactory&) override; Event::DispatcherPtr allocateDispatcher(const std::string& name, Buffer::WatermarkFactoryPtr&& watermark_factory) override; From ec55d5f8fe40ffe4dc2d167bafbfcb302e8d5e4e Mon Sep 17 00:00:00 2001 From: Alex Konradi Date: Fri, 15 Jan 2021 13:46:58 -0500 Subject: [PATCH 8/9] Fix test issues Signed-off-by: Alex Konradi --- test/server/config_validation/dispatcher_test.cc | 9 +++++++++ test/server/overload_manager_impl_test.cc | 1 - 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/test/server/config_validation/dispatcher_test.cc b/test/server/config_validation/dispatcher_test.cc index a4eb71df8d8a..b8a60c1da7a1 100644 --- a/test/server/config_validation/dispatcher_test.cc +++ b/test/server/config_validation/dispatcher_test.cc @@ -49,6 +49,15 @@ TEST_P(ConfigValidation, CreateConnection) { SUCCEED(); } +TEST_P(ConfigValidation, CreateScaledTimer) { + EXPECT_NE(dispatcher_->createScaledTimer(Event::ScaledTimerType::UnscaledRealTimerForTest, [] {}), + nullptr); + EXPECT_NE(dispatcher_->createScaledTimer( + Event::ScaledTimerMinimum(Event::ScaledMinimum(UnitFloat(0.5f))), [] {}), + nullptr); + SUCCEED(); +} + // Make sure that creating DnsResolver does not cause crash and each call to create // DNS resolver returns the same shared_ptr. TEST_F(ConfigValidation, SharedDnsResolver) { diff --git a/test/server/overload_manager_impl_test.cc b/test/server/overload_manager_impl_test.cc index 5952a094980c..cc0e8b29e287 100644 --- a/test/server/overload_manager_impl_test.cc +++ b/test/server/overload_manager_impl_test.cc @@ -31,7 +31,6 @@ using testing::DoAll; using testing::FloatNear; using testing::Invoke; using testing::InvokeArgument; -using testing::MockFunction; using testing::NiceMock; using testing::Pointee; using testing::Property; From 053a31562559c94ff879502bc8369727cef91cae Mon Sep 17 00:00:00 2001 From: Alex Konradi Date: Fri, 15 Jan 2021 17:28:51 -0500 Subject: [PATCH 9/9] Lower coverage limit Signed-off-by: Alex Konradi --- source/server/config_validation/api.cc | 4 ++-- test/per_file_coverage.sh | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/source/server/config_validation/api.cc b/source/server/config_validation/api.cc index f63eaf1895f4..bcf28c69172b 100644 --- a/source/server/config_validation/api.cc +++ b/source/server/config_validation/api.cc @@ -18,9 +18,9 @@ Event::DispatcherPtr ValidationImpl::allocateDispatcher(const std::string& name) } Event::DispatcherPtr -ValidationImpl::allocateDispatcher(const std::string& name, +ValidationImpl::allocateDispatcher(const std::string&, const Event::ScaledRangeTimerManagerFactory&) { - return Event::DispatcherPtr{new Event::ValidationDispatcher(name, *this, time_system_)}; + NOT_REACHED_GCOVR_EXCL_LINE; } Event::DispatcherPtr ValidationImpl::allocateDispatcher(const std::string&, diff --git a/test/per_file_coverage.sh b/test/per_file_coverage.sh index 32f71e40fadf..077ba276f79d 100755 --- a/test/per_file_coverage.sh +++ b/test/per_file_coverage.sh @@ -65,7 +65,7 @@ declare -a KNOWN_LOW_COVERAGE=( "source/extensions/watchdog/profile_action:85.7" "source/server:94.5" "source/server/admin:95.1" -"source/server/config_validation:76.6" +"source/server/config_validation:75.6" ) [[ -z "${SRCDIR}" ]] && SRCDIR="${PWD}"