diff --git a/include/envoy/event/scaled_timer_minimum.h b/include/envoy/event/scaled_timer_minimum.h index edf3b1777b7f0..34ad8ec9c3fda 100644 --- a/include/envoy/event/scaled_timer_minimum.h +++ b/include/envoy/event/scaled_timer_minimum.h @@ -13,7 +13,11 @@ 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 +25,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 +34,10 @@ 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; + constexpr ScaledTimerMinimum(ScaledMinimum arg) : impl_(arg) {} + constexpr ScaledTimerMinimum(AbsoluteMinimum 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/overload_manager.h b/include/envoy/server/overload/overload_manager.h index a4b42a7c9fadc..e3c66e8016e9d 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_timer_minimum.h" #include "envoy/server/overload/thread_local_overload_state.h" #include "common/singleton/const_singleton.h" @@ -37,6 +38,17 @@ class OverloadActionNameValues { using OverloadActionNames = ConstSingleton; +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, +}; + /** * The OverloadManager protects the Envoy instance from being overwhelmed by client * requests. It monitors a set of resources and notifies registered listeners if @@ -69,6 +81,12 @@ class OverloadManager { * an alternative to registering a callback for overload action state changes. */ virtual ThreadLocalOverloadState& getThreadLocalOverloadState() PURE; + + /** + * Get the configured minimum rule for the given timer type. + */ + virtual Event::ScaledTimerMinimum + getConfiguredTimerMinimum(OverloadTimerType timer_type) const 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 ef7a701618d92..271329d0e32d9 100644 --- a/include/envoy/server/overload/thread_local_overload_state.h +++ b/include/envoy/server/overload/thread_local_overload_state.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. */ @@ -59,10 +48,6 @@ class ThreadLocalOverloadState : public ThreadLocal::ThreadLocalObject { // 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; diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index fe435c3977e81..8f1a59087e4c9 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -101,6 +101,10 @@ ConnectionManagerImpl::ConnectionManagerImpl(ConnectionManagerConfig& config, overload_state_.getState(Server::OverloadActionNames::get().StopAcceptingRequests)), overload_disable_keepalive_ref_( overload_state_.getState(Server::OverloadActionNames::get().DisableHttpKeepAlive)), + downstream_idle_connection_scaled_timeout_(overload_manager.getConfiguredTimerMinimum( + Server::OverloadTimerType::HttpDownstreamIdleConnectionTimeout)), + downstream_idle_stream_scaled_timeout_(overload_manager.getConfiguredTimerMinimum( + Server::OverloadTimerType::HttpDownstreamIdleStreamTimeout)), time_source_(time_source) {} const ResponseHeaderMap& ConnectionManagerImpl::continueHeader() { @@ -122,8 +126,7 @@ void ConnectionManagerImpl::initializeReadFilterCallbacks(Network::ReadFilterCal if (config_.idleTimeout()) { connection_idle_timer_ = overload_state_.createScaledTimer( - Server::OverloadTimerType::HttpDownstreamIdleConnectionTimeout, - [this]() -> void { onIdleTimeout(); }); + downstream_idle_connection_scaled_timeout_, [this]() -> void { onIdleTimeout(); }); connection_idle_timer_->enableTimer(config_.idleTimeout().value()); } @@ -638,7 +641,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, + connection_manager_.downstream_idle_stream_scaled_timeout_, [this]() -> void { onIdleTimeout(); }); resetIdleTimer(); } @@ -1063,7 +1066,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, + connection_manager_.downstream_idle_stream_scaled_timeout_, [this]() -> void { onIdleTimeout(); }); } } else if (stream_idle_timer_ != nullptr) { diff --git a/source/common/http/conn_manager_impl.h b/source/common/http/conn_manager_impl.h index 4872cd1a75515..fef0853570f78 100644 --- a/source/common/http/conn_manager_impl.h +++ b/source/common/http/conn_manager_impl.h @@ -440,6 +440,9 @@ class ConnectionManagerImpl : Logger::Loggable, // map lookup in the hot path of processing each request. const Server::OverloadActionState& overload_stop_accepting_requests_ref_; const Server::OverloadActionState& overload_disable_keepalive_ref_; + // Timer scaling factors. + const Event::ScaledTimerMinimum downstream_idle_connection_scaled_timeout_; + const Event::ScaledTimerMinimum downstream_idle_stream_scaled_timeout_; TimeSource& time_source_; bool remote_close_{}; }; diff --git a/source/server/admin/admin.h b/source/server/admin/admin.h index 6e78cea526c63..a2c17b8f2ad50 100644 --- a/source/server/admin/admin.h +++ b/source/server/admin/admin.h @@ -255,9 +255,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); @@ -286,6 +283,10 @@ class AdminImpl : public Admin, return false; } + Event::ScaledTimerMinimum getConfiguredTimerMinimum(OverloadTimerType) const override { + return Event::ScaledMinimum(UnitFloat::max()); + } + ThreadLocal::SlotPtr tls_; }; diff --git a/source/server/overload_manager_impl.cc b/source/server/overload_manager_impl.cc index 07872b98442b3..f5a178fe09e0e 100644 --- a/source/server/overload_manager_impl.cc +++ b/source/server/overload_manager_impl.cc @@ -26,11 +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), + 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,16 +40,6 @@ 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)); @@ -67,7 +55,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_; @@ -340,7 +327,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()) { @@ -393,6 +380,14 @@ bool OverloadManagerImpl::registerForAction(const std::string& action, ThreadLocalOverloadState& OverloadManagerImpl::getThreadLocalOverloadState() { return *tls_; } +Event::ScaledTimerMinimum +OverloadManagerImpl::getConfiguredTimerMinimum(OverloadTimerType timer_type) const { + if (auto it = timer_minimums_.find(timer_type); it != timer_minimums_.end()) { + return it->second; + } + return Event::ScaledMinimum(UnitFloat::max()); +} + Event::ScaledRangeTimerManagerPtr OverloadManagerImpl::createScaledRangeTimerManager(Event::Dispatcher& dispatcher) const { return std::make_unique(dispatcher); diff --git a/source/server/overload_manager_impl.h b/source/server/overload_manager_impl.h index b86162fb02249..c1b8ae227cf8d 100644 --- a/source/server/overload_manager_impl.h +++ b/source/server/overload_manager_impl.h @@ -114,6 +114,7 @@ class OverloadManagerImpl : Logger::Loggable, public OverloadM bool registerForAction(const std::string& action, Event::Dispatcher& dispatcher, OverloadActionCb callback) override; ThreadLocalOverloadState& getThreadLocalOverloadState() override; + Event::ScaledTimerMinimum getConfiguredTimerMinimum(OverloadTimerType timer_type) const 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/test/common/http/conn_manager_impl_test_base.cc b/test/common/http/conn_manager_impl_test_base.cc index f32f39b70258d..e4a3d81108754 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(overload_manager_.overload_state_, createScaledMinimumTimer_) .WillByDefault([&](auto, auto callback) { return filter_callbacks_.connection_.dispatcher_.createTimer(callback).release(); }); diff --git a/test/mocks/server/overload_manager.cc b/test/mocks/server/overload_manager.cc index 461b0b27daa3d..b246f054a52a5 100644 --- a/test/mocks/server/overload_manager.cc +++ b/test/mocks/server/overload_manager.cc @@ -11,21 +11,16 @@ namespace Envoy { namespace Server { using ::testing::NiceMock; +using ::testing::Return; using ::testing::ReturnNew; 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))}; @@ -33,6 +28,8 @@ Event::TimerPtr MockThreadLocalOverloadState::createScaledTimer(Event::ScaledTim MockOverloadManager::MockOverloadManager() { ON_CALL(*this, getThreadLocalOverloadState()).WillByDefault(ReturnRef(overload_state_)); + ON_CALL(*this, getConfiguredTimerMinimum) + .WillByDefault(Return(Event::ScaledMinimum(UnitFloat::max()))); } MockOverloadManager::~MockOverloadManager() = default; diff --git a/test/mocks/server/overload_manager.h b/test/mocks/server/overload_manager.h index c694ab7b12548..7fddbdd84ccfb 100644 --- a/test/mocks/server/overload_manager.h +++ b/test/mocks/server/overload_manager.h @@ -14,10 +14,8 @@ 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)); @@ -36,6 +34,8 @@ class MockOverloadManager : public OverloadManager { (const std::string& action, Event::Dispatcher& dispatcher, OverloadActionCb callback)); MOCK_METHOD(ThreadLocalOverloadState&, getThreadLocalOverloadState, ()); + MOCK_METHOD(Event::ScaledTimerMinimum, getConfiguredTimerMinimum, (OverloadTimerType timer_type), + (const)); testing::NiceMock overload_state_; }; diff --git a/test/server/overload_manager_impl_test.cc b/test/server/overload_manager_impl_test.cc index d160b42ceae84..65a275b8cd9ea 100644 --- a/test/server/overload_manager_impl_test.cc +++ b/test/server/overload_manager_impl_test.cc @@ -492,25 +492,14 @@ constexpr char kReducedTimeoutsConfig[] = R"YAML( saturation_threshold: 1.0 )YAML"; -TEST_F(OverloadManagerImplTest, AdjustScaleFactor) { - 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(); - - // 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); - - timer_cb_(); -} +// These are the timer types according to the reduced timeouts config above. +constexpr std::pair kReducedTimeoutsMinimums[]{ + {OverloadTimerType::UnscaledRealTimerForTest, Event::ScaledMinimum(UnitFloat(1.0))}, + {OverloadTimerType::HttpDownstreamIdleConnectionTimeout, + Event::AbsoluteMinimum(std::chrono::seconds(2))}, +}; -TEST_F(OverloadManagerImplTest, CreateUnscaledScaledTimer) { +TEST_F(OverloadManagerImplTest, TimerTypesProduceCorrectMinimums) { setDispatcherExpectation(); auto manager(createOverloadManager(kReducedTimeoutsConfig)); @@ -519,45 +508,28 @@ TEST_F(OverloadManagerImplTest, CreateUnscaledScaledTimer) { .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) { - // 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)); + for (const auto& [timer_type, expected_minimum] : kReducedTimeoutsMinimums) { + SCOPED_TRACE(static_cast(timer_type)); + EXPECT_EQ(manager->getConfiguredTimerMinimum(timer_type), expected_minimum); + } } -TEST_F(OverloadManagerImplTest, CreateScaledTimerWithAbsoluteMinimum) { +TEST_F(OverloadManagerImplTest, AdjustScaleFactor) { 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; - }); + // 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); - auto timer = manager->getThreadLocalOverloadState().createScaledTimer( - OverloadTimerType::HttpDownstreamIdleConnectionTimeout, mock_callback.AsStdFunction()); - EXPECT_EQ(timer.get(), mock_scaled_timer); + timer_cb_(); } TEST_F(OverloadManagerImplTest, CreateScaledTimerWithProvidedMinimum) {