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 aa310d2c40c2..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" @@ -30,6 +31,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..91d13f4d113c 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_timer", ":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", @@ -42,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", ], @@ -53,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 394a6dbb41bb..36e34f78bd1a 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_timer.h" #include "envoy/event/schedulable_cb.h" #include "envoy/event/signal.h" #include "envoy/event/timer.h" @@ -77,6 +78,20 @@ 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::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(Event::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 9cf477b692c9..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" @@ -30,6 +30,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(ScaledTimerType 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 @@ -43,5 +49,8 @@ class ScaledRangeTimerManager { using ScaledRangeTimerManagerPtr = std::unique_ptr; +class Dispatcher; +using ScaledRangeTimerManagerFactory = std::function; + } // namespace Event } // namespace Envoy diff --git a/include/envoy/event/scaled_timer.h b/include/envoy/event/scaled_timer.h new file mode 100644 index 000000000000..63ac8a5f421e --- /dev/null +++ b/include/envoy/event/scaled_timer.h @@ -0,0 +1,84 @@ +#pragma once + +#include + +#include "common/common/interval_value.h" + +#include "absl/container/flat_hash_map.h" +#include "absl/types/variant.h" + +namespace Envoy { +namespace Event { + +/** + * Describes a minimum timer value that is equal to a scale factor applied to the maximum. + */ +struct ScaledMinimum { + 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_; +}; + +/** + * Describes a minimum timer value that is an absolute duration. + */ +struct AbsoluteMinimum { + 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_; +}; + +/** + * 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 { +public: + // 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: + // the return value is the scale factor applied to the provided maximum. + // - AbsoluteMinimum: + // the return value is that minimum, and the provided maximum is ignored. + std::chrono::milliseconds computeMinimum(std::chrono::milliseconds maximum) const { + struct Visitor { + explicit Visitor(std::chrono::milliseconds value) : value_(value) {} + std::chrono::milliseconds operator()(ScaledMinimum scale_factor) { + return std::chrono::duration_cast( + scale_factor.scale_factor_.value() * value_); + } + std::chrono::milliseconds operator()(AbsoluteMinimum absolute_value) { + return absolute_value.value_; + } + const std::chrono::milliseconds value_; + }; + return absl::visit(Visitor(maximum), impl_); + } + + inline bool operator==(const ScaledTimerMinimum& other) const { return impl_ == other.impl_; } + +private: + 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/event/scaled_timer_minimum.h b/include/envoy/event/scaled_timer_minimum.h deleted file mode 100644 index edf3b1777b7f..000000000000 --- a/include/envoy/event/scaled_timer_minimum.h +++ /dev/null @@ -1,60 +0,0 @@ -#pragma once - -#include - -#include "common/common/interval_value.h" - -#include "absl/types/variant.h" - -namespace Envoy { -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) {} - const UnitFloat scale_factor_; -}; - -/** - * Describes a minimum timer value that is an absolute duration. - */ -struct AbsoluteMinimum { - explicit AbsoluteMinimum(std::chrono::milliseconds value) : value_(value) {} - const std::chrono::milliseconds value_; -}; - -/** - * 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 { -public: - // Use base class constructor. - using absl::variant::variant; - - // Computes the minimum value for a given maximum timeout. If this object was constructed with a - // - ScaledMinimum value: - // the return value is the scale factor applied to the provided maximum. - // - AbsoluteMinimum: - // the return value is that minimum, and the provided maximum is ignored. - std::chrono::milliseconds computeMinimum(std::chrono::milliseconds maximum) const { - struct Visitor { - explicit Visitor(std::chrono::milliseconds value) : value_(value) {} - std::chrono::milliseconds operator()(ScaledMinimum scale_factor) { - return std::chrono::duration_cast( - scale_factor.scale_factor_.value() * value_); - } - std::chrono::milliseconds operator()(AbsoluteMinimum absolute_value) { - return absolute_value.value_; - } - const std::chrono::milliseconds value_; - }; - return absl::visit&>( - Visitor(maximum), *this); - } -}; - -} // namespace Event -} // namespace Envoy diff --git a/include/envoy/server/overload/BUILD b/include/envoy/server/overload/BUILD index f6f4a90933e9..a6e3f789c444 100644 --- a/include/envoy/server/overload/BUILD +++ b/include/envoy/server/overload/BUILD @@ -23,7 +23,7 @@ envoy_cc_library( name = "thread_local_overload_state", hdrs = ["thread_local_overload_state.h"], deps = [ - "//include/envoy/event:scaled_timer_minimum", + "//include/envoy/event:scaled_range_timer_manager_interface", "//include/envoy/event:timer_interface", "//include/envoy/thread_local:thread_local_object", "//source/common/common:interval_value", 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 e7531253d31a..23c5e7add043 100644 --- a/include/envoy/server/overload/thread_local_overload_state.h +++ b/include/envoy/server/overload/thread_local_overload_state.h @@ -4,7 +4,7 @@ #include #include "envoy/common/pure.h" -#include "envoy/event/scaled_timer_minimum.h" +#include "envoy/event/scaled_range_timer_manager.h" #include "envoy/event/timer.h" #include "envoy/thread_local/thread_local_object.h" @@ -40,17 +40,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. */ @@ -58,14 +47,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(OverloadTimerType 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..f91e5c6a1c6f 100644 --- a/source/common/api/api_impl.cc +++ b/source/common/api/api_impl.cc @@ -21,6 +21,13 @@ Event::DispatcherPtr Impl::allocateDispatcher(const std::string& name) { return std::make_unique(name, *this, time_system_, 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)); 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 09d640b6b817..333755eeac35 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 008fb99ff952..f67d7787abc9 100644 --- a/source/common/event/dispatcher_impl.cc +++ b/source/common/event/dispatcher_impl.cc @@ -17,6 +17,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" @@ -39,17 +40,33 @@ namespace Envoy { namespace Event { +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 Buffer::WatermarkFactorySharedPtr& factory) + const ScaledRangeTimerManagerFactory& 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(*this)) { ASSERT(!name_.empty()); FatalErrorHandler::registerFatalErrorHandler(*this); updateApproximateMonotonicTimeInternal(); @@ -192,6 +209,16 @@ TimerPtr DispatcherImpl::createTimer(TimerCb cb) { return createTimerInternal(cb); } +TimerPtr DispatcherImpl::createScaledTimer(ScaledTimerType 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 ecbfffebf39d..f94107ec79bb 100644 --- a/source/common/event/dispatcher_impl.h +++ b/source/common/event/dispatcher_impl.h @@ -37,8 +37,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 Buffer::WatermarkFactorySharedPtr& factory = nullptr); + const ScaledRangeTimerManagerFactory& scaled_timer_factory, + const Buffer::WatermarkFactorySharedPtr& watermark_factory); ~DispatcherImpl() override; /** @@ -74,6 +78,9 @@ class DispatcherImpl : Logger::Loggable, Network::UdpListenerPtr createUdpListener(Network::SocketSharedPtr socket, Network::UdpListenerCallbacks& cb) override; TimerPtr createTimer(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; void deferredDelete(DeferredDeletablePtr&& to_delete) override; void exit() override; @@ -150,6 +157,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/event/scaled_range_timer_manager_impl.cc b/source/common/event/scaled_range_timer_manager_impl.cc index 5e9e1a8b8f32..baea9de390bf 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 ScaledTimerTypeMapConstSharedPtr& 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(ScaledTimerType 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 2f0a78662873..4c36b50f5de0 100644 --- a/source/common/event/scaled_range_timer_manager_impl.h +++ b/source/common/event/scaled_range_timer_manager_impl.h @@ -23,11 +23,14 @@ namespace Event { */ class ScaledRangeTimerManagerImpl : public ScaledRangeTimerManager { public: - explicit ScaledRangeTimerManagerImpl(Dispatcher& dispatcher); + // Takes a Dispatcher and a map from timer type to scaled minimum value. + ScaledRangeTimerManagerImpl(Dispatcher& dispatcher, + const ScaledTimerTypeMapConstSharedPtr& timer_minimums = nullptr); ~ScaledRangeTimerManagerImpl() override; // ScaledRangeTimerManager impl TimerPtr createTimer(ScaledTimerMinimum minimum, TimerCb callback) override; + TimerPtr createTimer(ScaledTimerType timer_type, TimerCb callback) override; void setScaleFactor(UnitFloat scale_factor) override; private: @@ -116,9 +119,10 @@ class ScaledRangeTimerManagerImpl : public ScaledRangeTimerManager { void onQueueTimerFired(Queue& queue); Dispatcher& dispatcher_; + 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 d6f501985486..e2ceee4f1c2a 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" @@ -121,8 +122,8 @@ void ConnectionManagerImpl::initializeReadFilterCallbacks(Network::ReadFilterCal read_callbacks_->connection().addConnectionCallbacks(*this); if (config_.idleTimeout()) { - connection_idle_timer_ = overload_state_.createScaledTimer( - Server::OverloadTimerType::HttpDownstreamIdleConnectionTimeout, + connection_idle_timer_ = read_callbacks_->connection().dispatcher().createScaledTimer( + Event::ScaledTimerType::HttpDownstreamIdleConnectionTimeout, [this]() -> void { onIdleTimeout(); }); connection_idle_timer_->enableTimer(config_.idleTimeout().value()); } @@ -627,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( - Server::OverloadTimerType::HttpDownstreamIdleStreamTimeout, - [this]() -> void { onIdleTimeout(); }); + stream_idle_timer_ = + connection_manager_.read_callbacks_->connection().dispatcher().createScaledTimer( + Event::ScaledTimerType::HttpDownstreamIdleStreamTimeout, + [this]() -> void { onIdleTimeout(); }); resetIdleTimer(); } @@ -1051,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( - Server::OverloadTimerType::HttpDownstreamIdleStreamTimeout, - [this]() -> void { onIdleTimeout(); }); + stream_idle_timer_ = + connection_manager_.read_callbacks_->connection().dispatcher().createScaledTimer( + Event::ScaledTimerType::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 c766978c9638..3ca7be09f1af 100644 --- a/source/server/admin/admin.h +++ b/source/server/admin/admin.h @@ -256,14 +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(OverloadTimerType, 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(); }; @@ -281,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/config_validation/api.cc b/source/server/config_validation/api.cc index fb6152b9053d..bcf28c69172b 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&, + const Event::ScaledRangeTimerManagerFactory&) { + NOT_REACHED_GCOVR_EXCL_LINE; +} + 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; diff --git a/source/server/overload_manager_impl.cc b/source/server/overload_manager_impl.cc index ebd07896f8ba..5c9b7266c453 100644 --- a/source/server/overload_manager_impl.cc +++ b/source/server/overload_manager_impl.cc @@ -26,14 +26,9 @@ namespace Server { */ 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), - 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)) {} + explicit ThreadLocalOverloadStateImpl(const NamedOverloadActionSymbolTable& action_symbol_table) + : action_symbol_table_(action_symbol_table), + 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) { @@ -42,39 +37,14 @@ class ThreadLocalOverloadStateImpl : public ThreadLocalOverloadState { return always_inactive_; } - Event::TimerPtr createScaledTimer(OverloadTimerType 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)); - } - - 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( - // The action state is 0 for no overload up to 1 for maximal overload, - // but the scale factor for timers is 1 for no scaling and 0 for - // maximal scaling, so invert the value to pass in (1-value). - state.value().invert()); - } } 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_; }; const OverloadActionState ThreadLocalOverloadStateImpl::always_inactive_{UnitFloat::min()}; @@ -150,31 +120,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::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 OverloadTimerType::HttpDownstreamIdleConnectionTimeout; + return Event::ScaledTimerType::HttpDownstreamIdleConnectionTimeout; case Config::HTTP_DOWNSTREAM_STREAM_IDLE: - return OverloadTimerType::HttpDownstreamIdleStreamTimeout; + return Event::ScaledTimerType::HttpDownstreamIdleStreamTimeout; default: throw EnvoyException(fmt::format("Unknown timer type {}", config_timer_type)); } } -absl::flat_hash_map +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); - absl::flat_hash_map timer_map; + Event::ScaledTimerTypeMap timer_map; for (const auto& scale_timer : action_config.timer_scale_factors()) { - const OverloadTimerType timer_type = parseTimerType(scale_timer.timer()); + const Event::ScaledTimerType timer_type = parseTimerType(scale_timer.timer()); const Event::ScaledTimerMinimum minimum = scale_timer.has_min_timeout() @@ -324,7 +294,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)); @@ -347,9 +318,8 @@ void OverloadManagerImpl::start() { ASSERT(!started_); started_ = true; - tls_.set([this](Event::Dispatcher& dispatcher) { - return std::make_shared(createScaledRangeTimerManager(dispatcher), - action_symbol_table_, timer_minimums_); + tls_.set([this](Event::Dispatcher&) { + return std::make_shared(action_symbol_table_); }); if (resources_.empty()) { @@ -401,10 +371,25 @@ 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, timer_minimums_); + registerForAction(OverloadActionNames::get().ReduceTimeouts, dispatcher, + [manager = manager.get()](OverloadActionState scale_state) { + manager->setScaleFactor( + // The action state is 0 for no overload up to 1 for maximal overload, + // but the scale factor for timers is 1 for no scaling and 0 for maximal + // scaling, so invert the value to pass in (1-value). + scale_state.value().invert()); + }); + return manager; + }; +} -Event::ScaledRangeTimerManagerPtr -OverloadManagerImpl::createScaledRangeTimerManager(Event::Dispatcher& dispatcher) const { - return std::make_unique(dispatcher); +Event::ScaledRangeTimerManagerPtr OverloadManagerImpl::createScaledRangeTimerManager( + Event::Dispatcher& dispatcher, + const Event::ScaledTimerTypeMapConstSharedPtr& 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 b86162fb0224..6dcfff6d032b 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" @@ -114,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 @@ -122,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::ScaledTimerTypeMapConstSharedPtr& timer_minimums) const; private: using FlushEpochId = uint64_t; @@ -170,7 +173,7 @@ class OverloadManagerImpl : Logger::Loggable, public OverloadM absl::node_hash_map resources_; absl::node_hash_map actions_; - absl::flat_hash_map timer_minimums_; + Event::ScaledTimerTypeMapConstSharedPtr timer_minimums_; absl::flat_hash_map state_updates_to_flush_; diff --git a/source/server/worker_impl.cc b/source/server/worker_impl.cc index 47a5d59e9881..95c24175321c 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/dispatcher_impl_test.cc b/test/common/event/dispatcher_impl_test.cc index 72ff1096d146..198a0ef4fd8b 100644 --- a/test/common/event/dispatcher_impl_test.cc +++ b/test/common/event/dispatcher_impl_test.cc @@ -24,8 +24,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 { @@ -1335,6 +1338,48 @@ 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_(ScaledTimerType::UnscaledRealTimerForTest, _)); + dispatcher->createScaledTimer(ScaledTimerType::UnscaledRealTimerForTest, []() {}); +} + class DispatcherWithWatchdogTest : public testing::Test { protected: DispatcherWithWatchdogTest() 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 5d98d3ac8020..14d240ecc704 100644 --- a/test/common/event/scaled_range_timer_manager_impl_test.cc +++ b/test/common/event/scaled_range_timer_manager_impl_test.cc @@ -623,6 +623,34 @@ 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::createTimer; + using ScaledRangeTimerManagerImpl::ScaledRangeTimerManagerImpl; + TimerPtr createTimer(ScaledTimerMinimum minimum, TimerCb callback) override { + return createScaledTimer(minimum, callback); + } + MOCK_METHOD(TimerPtr, createScaledTimer, (ScaledTimerMinimum, TimerCb)); + }; + + const ScaledTimerTypeMap timer_types{ + {ScaledTimerType::UnscaledRealTimerForTest, ScaledMinimum(UnitFloat::max())}, + {ScaledTimerType::HttpDownstreamIdleConnectionTimeout, ScaledMinimum(UnitFloat(0.3))}, + {ScaledTimerType::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/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..c402005a6dde 100644 --- a/test/mocks/api/mocks.cc +++ b/test/mocks/api/mocks.cc @@ -23,10 +23,16 @@ 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, {}, 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, {}, 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.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/mocks/event/mocks.h b/test/mocks/event/mocks.h index f103b55346f8..066eb68d6747 100644 --- a/test/mocks/event/mocks.h +++ b/test/mocks/event/mocks.h @@ -84,6 +84,20 @@ 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(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); + 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. @@ -125,6 +139,8 @@ 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_, (ScaledTimerType timer_type, Event::TimerCb cb)); MOCK_METHOD(SchedulableCallback*, createSchedulableCallback_, (std::function cb)); MOCK_METHOD(void, deferredDelete_, (DeferredDeletable * to_delete)); MOCK_METHOD(void, exit, ()); @@ -186,7 +202,11 @@ class MockScaledRangeTimerManager : public ScaledRangeTimerManager { TimerPtr createTimer(ScaledTimerMinimum minimum, TimerCb callback) override { return TimerPtr{createTimer_(minimum, std::move(callback))}; } + 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_, (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 c3e61726ba16..a6dc7be22716 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(ScaledTimerType 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 461b0b27daa3..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(OverloadTimerType 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 c694ab7b1254..e5ab03992836 100644 --- a/test/mocks/server/overload_manager.h +++ b/test/mocks/server/overload_manager.h @@ -14,12 +14,6 @@ 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::ScaledTimerMinimum minimum, - Event::TimerCb callback) override; - MOCK_METHOD(Event::Timer*, createScaledTypedTimer_, (OverloadTimerType, Event::TimerCb)); - MOCK_METHOD(Event::Timer*, createScaledMinimumTimer_, - (Event::ScaledTimerMinimum, Event::TimerCb)); private: const OverloadActionState disabled_state_; @@ -35,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/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}" 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 e73f91b3834d..cc0e8b29e287 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,17 +27,23 @@ using testing::_; using testing::AllOf; using testing::AnyNumber; using testing::ByMove; +using testing::DoAll; using testing::FloatNear; using testing::Invoke; -using testing::MockFunction; +using testing::InvokeArgument; 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::ScaledTimerType; + class FakeResourceMonitor : public ResourceMonitor { public: FakeResourceMonitor(Event::Dispatcher& dispatcher) : dispatcher_(dispatcher), response_(0.0) {} @@ -120,12 +128,14 @@ class TestOverloadManager : public OverloadManagerImpl { } MOCK_METHOD(Event::ScaledRangeTimerManagerPtr, createScaledRangeTimerManager, - (Event::Dispatcher&), (const, override)); + (Event::Dispatcher&, const Event::ScaledTimerTypeMapConstSharedPtr&), + (const, override)); private: - Event::ScaledRangeTimerManagerPtr - createDefaultScaledRangeTimerManager(Event::Dispatcher& dispatcher) const { - return OverloadManagerImpl::createScaledRangeTimerManager(dispatcher); + Event::ScaledRangeTimerManagerPtr createDefaultScaledRangeTimerManager( + Event::Dispatcher& dispatcher, + const Event::ScaledTimerTypeMapConstSharedPtr& timer_minimums) const { + return OverloadManagerImpl::createScaledRangeTimerManager(dispatcher, timer_minimums); } }; @@ -492,97 +502,51 @@ constexpr char kReducedTimeoutsConfig[] = R"YAML( saturation_threshold: 1.0 )YAML"; -TEST_F(OverloadManagerImplTest, AdjustScaleFactor) { - setDispatcherExpectation(); +// 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* scaled_timer_manager = new Event::MockScaledRangeTimerManager(); - EXPECT_CALL(*manager, createScaledRangeTimerManager) - .WillOnce(Return(ByMove(Event::ScaledRangeTimerManagerPtr{scaled_timer_manager}))); + auto* mock_scaled_timer_manager = new Event::MockScaledRangeTimerManager(); - manager->start(); + Event::ScaledTimerTypeMapConstSharedPtr timer_minimums; + EXPECT_CALL(*manager, createScaledRangeTimerManager) + .WillOnce( + DoAll(SaveArg<1>(&timer_minimums), + 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(Property(&UnitFloat::value, FloatNear(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); + EXPECT_THAT(timer_minimums, Pointee(UnorderedElementsAreArray(kReducedTimeoutsMinimums))); } -TEST_F(OverloadManagerImplTest, CreateUnscaledScaledTimer) { +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, 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; - }); - - auto timer = manager->getThreadLocalOverloadState().createScaledTimer( - OverloadTimerType::UnscaledRealTimerForTest, mock_callback.AsStdFunction()); - - EXPECT_CALL(*mock_scaled_timer, enableTimer(std::chrono::milliseconds(5 * 1000), _)); - timer->enableTimer(std::chrono::seconds(5)); -} + Event::MockDispatcher mock_dispatcher; + auto scaled_timer_manager = manager->scaledTimerFactory()(mock_dispatcher); -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(); + 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(Property(&UnitFloat::value, FloatNear(0.8, 0.00001)))); + factory1_.monitor_->setPressure(0.6); - 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); + timer_cb_(); } TEST_F(OverloadManagerImplTest, DuplicateResourceMonitor) {