diff --git a/envoy/network/BUILD b/envoy/network/BUILD index a3b1cb173319..9304abf8d8d2 100644 --- a/envoy/network/BUILD +++ b/envoy/network/BUILD @@ -104,9 +104,7 @@ envoy_cc_library( name = "drain_decision_interface", hdrs = ["drain_decision.h"], deps = [ - "//envoy/common:callback", - "@com_google_absl//absl/base", - "@com_google_absl//absl/status", + "//envoy/common:pure_lib", ], ) diff --git a/envoy/network/drain_decision.h b/envoy/network/drain_decision.h index 993e19f4502e..e071dfdc8480 100644 --- a/envoy/network/drain_decision.h +++ b/envoy/network/drain_decision.h @@ -1,21 +1,12 @@ #pragma once -#include -#include - -#include "envoy/common/callback.h" #include "envoy/common/pure.h" -#include "absl/base/attributes.h" -#include "absl/status/status.h" - namespace Envoy { namespace Network { class DrainDecision { public: - using DrainCloseCb = std::function; - virtual ~DrainDecision() = default; /** @@ -23,16 +14,6 @@ class DrainDecision { * filters to determine when this should be called for the least impact possible. */ virtual bool drainClose() const PURE; - - /** - * @brief Register a callback to be called proactively when a drain decision enters into a - * 'close' state. - * - * @param cb Callback to be called once drain decision enters close state - * @return handle to remove callback - */ - ABSL_MUST_USE_RESULT - virtual Common::CallbackHandlePtr addOnDrainCloseCb(DrainCloseCb cb) const PURE; }; } // namespace Network diff --git a/envoy/server/BUILD b/envoy/server/BUILD index 947a286b1c3b..ebd6ffb9bb6f 100644 --- a/envoy/server/BUILD +++ b/envoy/server/BUILD @@ -52,12 +52,7 @@ envoy_cc_library( envoy_cc_library( name = "drain_manager_interface", hdrs = ["drain_manager.h"], - deps = [ - "//envoy/event:dispatcher_interface", - "//envoy/network:drain_decision_interface", - "//envoy/thread_local:thread_local_object", - "@envoy_api//envoy/config/listener/v3:pkg_cc_proto", - ], + deps = ["//envoy/network:drain_decision_interface"], ) envoy_cc_library( diff --git a/envoy/server/drain_manager.h b/envoy/server/drain_manager.h index 0e37091fa5dd..49ecc194166a 100644 --- a/envoy/server/drain_manager.h +++ b/envoy/server/drain_manager.h @@ -3,43 +3,17 @@ #include #include -#include "envoy/config/listener/v3/listener.pb.h" -#include "envoy/event/dispatcher.h" #include "envoy/network/drain_decision.h" -#include "envoy/thread_local/thread_local_object.h" namespace Envoy { namespace Server { -class DrainManager; -using DrainManagerPtr = std::unique_ptr; - /** * Handles connection draining. This concept is used globally during hot restart / server draining - * as well as on individual listeners and filter-chains when they are being dynamically removed. + * as well as on individual listeners when they are being dynamically removed. */ -class DrainManager : public Network::DrainDecision, public ThreadLocal::ThreadLocalObject { +class DrainManager : public Network::DrainDecision { public: - /** - * @brief Create a child drain-manager. Will proxy the drain status from the parent, but can also - * be used to enact local draining. - * - * Child managers can be used to construct "drain trees" where each node in the tree can drain - * independently of it's parent node, but the drain status cascades to child nodes. - * - * A notable difference to drain callbacks is that child managers are notified immediately and - * without a delay timing. Additionally, notifications from parent to child is a thread-safe - * operation whereas callback registration and triggering is not. - * - * @param dispatcher Dispatcher for the current thread in which the new child drain-manager will - * exist. - * @param drain_type The drain-type for the manager. May be different from the parent manager. - */ - virtual DrainManagerPtr - createChildManager(Event::Dispatcher& dispatcher, - envoy::config::listener::v3::Listener::DrainType drain_type) PURE; - virtual DrainManagerPtr createChildManager(Event::Dispatcher& dispatcher) PURE; - /** * Invoked to begin the drain procedure. (Making drain close operations more likely). * @param drain_complete_cb will be invoked once the drain sequence is finished. The parameter is diff --git a/mobile/envoy_build_config/BUILD b/mobile/envoy_build_config/BUILD index 53ca845b120b..28e153e7c745 100644 --- a/mobile/envoy_build_config/BUILD +++ b/mobile/envoy_build_config/BUILD @@ -19,6 +19,7 @@ envoy_cc_library( deps = [ "extension_registry_platform_additions", "@envoy//source/common/http/matching:inputs_lib", + "@envoy//source/common/network:default_client_connection_factory", "@envoy//source/common/network:socket_lib", "@envoy//source/common/quic:quic_transport_socket_factory_lib", "@envoy//source/common/router:upstream_codec_filter_lib", diff --git a/source/common/common/BUILD b/source/common/common/BUILD index c80ef430d63f..4b168b30601a 100644 --- a/source/common/common/BUILD +++ b/source/common/common/BUILD @@ -507,14 +507,10 @@ envoy_cc_library( envoy_cc_library( name = "callback_impl_lib", - srcs = ["callback_impl.cc"], hdrs = ["callback_impl.h"], deps = [ ":assert_lib", - ":lock_guard_lib", - ":thread_lib", "//envoy/common:callback", - "//source/common/event:dispatcher_lib", ], ) diff --git a/source/common/common/callback_impl.cc b/source/common/common/callback_impl.cc deleted file mode 100644 index 00e434857557..000000000000 --- a/source/common/common/callback_impl.cc +++ /dev/null @@ -1,56 +0,0 @@ -#include "source/common/common/callback_impl.h" - -namespace Envoy { -namespace Common { - -CallbackHandlePtr ThreadSafeCallbackManager::add(Event::Dispatcher& dispatcher, Callback callback) { - Thread::LockGuard lock(lock_); - auto new_callback = std::make_unique(shared_from_this(), callback, dispatcher); - callbacks_.push_back(CallbackListEntry(new_callback.get(), dispatcher, - std::weak_ptr(new_callback->still_alive_))); - // Get the list iterator of added callback handle, which will be used to remove itself from - // callbacks_ list. - new_callback->it_ = (--callbacks_.end()); - return new_callback; -} - -void ThreadSafeCallbackManager::runCallbacks() { - Thread::LockGuard lock(lock_); - for (auto it = callbacks_.cbegin(); it != callbacks_.cend();) { - auto& [cb, cb_dispatcher, still_alive] = *(it++); - - cb_dispatcher.post([cb = cb, still_alive = still_alive] { - // Once we're running on the thread that scheduled the callback, validate the - // callback is still valid and execute. Even though 'expired()' is racy, because - // we are on the scheduling thread, this should not race with destruction. - if (!still_alive.expired()) { - cb->cb_(); - } - }); - } -} - -size_t ThreadSafeCallbackManager::size() const noexcept { - Thread::LockGuard lock(lock_); - return callbacks_.size(); -} - -void ThreadSafeCallbackManager::remove(typename std::list::iterator& it) { - Thread::LockGuard lock(lock_); - callbacks_.erase(it); -} - -ThreadSafeCallbackManager::CallbackHolder::CallbackHolder( - std::shared_ptr parent, Callback cb, - Event::Dispatcher& cb_dispatcher) - : parent_(parent), cb_(cb), callback_dispatcher_(cb_dispatcher) {} - -ThreadSafeCallbackManager::CallbackHolder::~CallbackHolder() { - // Validate that destruction of the callback is happening on the same thread in which it was - // intended to be executed. - ASSERT(callback_dispatcher_.isThreadSafe()); - parent_->remove(it_); -} - -} // namespace Common -} // namespace Envoy diff --git a/source/common/common/callback_impl.h b/source/common/common/callback_impl.h index 628043e11bdf..6ceeca39b55a 100644 --- a/source/common/common/callback_impl.h +++ b/source/common/common/callback_impl.h @@ -2,24 +2,17 @@ #include #include -#include -#include #include "envoy/common/callback.h" -#include "envoy/event/dispatcher.h" -#include "envoy/thread/thread.h" +#include "envoy/common/exception.h" #include "source/common/common/assert.h" -#include "source/common/common/lock_guard.h" -#include "source/common/common/thread.h" namespace Envoy { namespace Common { /** * Utility class for managing callbacks. - * - * @see ThreadSafeCallbackManager for dealing with callbacks across multiple threads */ template class CallbackManager { public: @@ -54,22 +47,6 @@ template class CallbackManager { return absl::OkStatus(); } - /** - * @brief Run all callbacks with a function that returns the input arguments - * - * NOTE: This code is currently safe if a callback deletes ITSELF from within a callback. It is - * not safe if a callback deletes other callbacks. - * @param run_with function that is responsible for generating inputs to callbacks. This will be - * executed once for each callback. - */ - absl::Status runCallbacksWith(std::function(void)> run_with) { - for (auto it = callbacks_.cbegin(); it != callbacks_.cend();) { - auto cb = *(it++); - RETURN_IF_NOT_OK(std::apply(cb->cb_, run_with())); - } - return absl::OkStatus(); - } - size_t size() const noexcept { return callbacks_.size(); } private: @@ -108,72 +85,5 @@ template class CallbackManager { const std::shared_ptr still_alive_{std::make_shared(true)}; }; -/** - * @brief Utility class for managing callbacks across multiple threads. - * - * @see CallbackManager for a non-thread-safe version - */ -class ThreadSafeCallbackManager : public std::enable_shared_from_this { - struct CallbackHolder; - using CallbackListEntry = std::tuple>; - -public: - using Callback = std::function; - - /** - * @brief Create a ThreadSafeCallbackManager - * - * @note The ThreadSafeCallbackManager must always be represented as a std::shared_ptr in - * order to satisfy internal conditions to how callbacks are managed. - */ - static std::shared_ptr create() { - return std::shared_ptr(new ThreadSafeCallbackManager()); - } - - /** - * @brief Add a callback. - * @param dispatcher Dispatcher from the same thread as the registered callback. This will be used - * to schedule the execution of the callback. - * @param callback callback to add - * @return Handle that can be used to remove the callback. - */ - ABSL_MUST_USE_RESULT CallbackHandlePtr add(Event::Dispatcher& dispatcher, Callback callback); - - /** - * @brief Run all callbacks - */ - void runCallbacks(); - - size_t size() const noexcept; - -private: - struct CallbackHolder : public CallbackHandle { - CallbackHolder(std::shared_ptr parent, Callback cb, - Event::Dispatcher& cb_dispatcher); - - ~CallbackHolder() override; - - std::shared_ptr parent_; - Callback cb_; - Event::Dispatcher& callback_dispatcher_; - std::shared_ptr still_alive_{std::make_shared(true)}; - - typename std::list::iterator it_; - }; - - ThreadSafeCallbackManager() = default; - - /** - * Remove a member update callback added via add(). - * @param handle supplies the callback handle to remove. - */ - void remove(typename std::list::iterator& it); - - // This must be held on all read/writes of callbacks_ - mutable Thread::MutexBasicLockable lock_{}; - - std::list callbacks_ ABSL_GUARDED_BY(lock_); -}; - } // namespace Common } // namespace Envoy diff --git a/source/common/listener_manager/filter_chain_manager_impl.h b/source/common/listener_manager/filter_chain_manager_impl.h index 9b6f77428537..9ca6d893eae7 100644 --- a/source/common/listener_manager/filter_chain_manager_impl.h +++ b/source/common/listener_manager/filter_chain_manager_impl.h @@ -52,10 +52,6 @@ class PerFilterChainFactoryContextImpl : public Configuration::FilterChainFactor // DrainDecision bool drainClose() const override; - Common::CallbackHandlePtr addOnDrainCloseCb(DrainCloseCb) const override { - IS_ENVOY_BUG("Unexpected function call"); - return nullptr; - } // Configuration::FactoryContext Network::DrainDecision& drainDecision() override; diff --git a/source/common/listener_manager/listener_impl.h b/source/common/listener_manager/listener_impl.h index 2f37b82b1dbb..cc674f400ad6 100644 --- a/source/common/listener_manager/listener_impl.h +++ b/source/common/listener_manager/listener_impl.h @@ -147,10 +147,6 @@ class ListenerFactoryContextBaseImpl final : public Server::FactoryContextImplBa bool drainClose() const override { return drain_manager_->drainClose() || server_.drainManager().drainClose(); } - Common::CallbackHandlePtr addOnDrainCloseCb(DrainCloseCb) const override { - IS_ENVOY_BUG("Unexpected function call"); - return nullptr; - } Server::DrainManager& drainManager(); friend class ListenerImpl; diff --git a/source/common/listener_manager/listener_manager_impl.cc b/source/common/listener_manager/listener_manager_impl.cc index 8de7a7da3360..3412d1051c19 100644 --- a/source/common/listener_manager/listener_manager_impl.cc +++ b/source/common/listener_manager/listener_manager_impl.cc @@ -342,7 +342,7 @@ absl::StatusOr ProdListenerComponentFactory::createLis DrainManagerPtr ProdListenerComponentFactory::createDrainManager( envoy::config::listener::v3::Listener::DrainType drain_type) { - return DrainManagerPtr{new DrainManagerImpl(server_, drain_type, server_.dispatcher())}; + return DrainManagerPtr{new DrainManagerImpl(server_, drain_type)}; } DrainingFilterChainsManager::DrainingFilterChainsManager(ListenerImplPtr&& draining_listener, diff --git a/source/common/secret/BUILD b/source/common/secret/BUILD index d6e61c39531f..0deb1b16519a 100644 --- a/source/common/secret/BUILD +++ b/source/common/secret/BUILD @@ -61,6 +61,7 @@ envoy_cc_library( "//source/common/config:subscription_base_interface", "//source/common/config:utility_lib", "//source/common/config:watched_directory_lib", + "//source/common/grpc:common_lib", "//source/common/init:target_lib", "//source/common/protobuf:utility_lib", "//source/common/ssl:certificate_validation_context_config_impl_lib", diff --git a/source/exe/stripped_main_base.cc b/source/exe/stripped_main_base.cc index 3a84503c12c0..52519ee768cf 100644 --- a/source/exe/stripped_main_base.cc +++ b/source/exe/stripped_main_base.cc @@ -33,7 +33,7 @@ Server::DrainManagerPtr ProdComponentFactory::createDrainManager(Server::Instanc // hot restart at the global level. The per-listener drain managers decide whether to // to include /healthcheck/fail status. return std::make_unique( - server, envoy::config::listener::v3::Listener::MODIFY_ONLY, server.dispatcher()); + server, envoy::config::listener::v3::Listener::MODIFY_ONLY); } Runtime::LoaderPtr ProdComponentFactory::createRuntime(Server::Instance& server, diff --git a/source/extensions/filters/network/http_connection_manager/BUILD b/source/extensions/filters/network/http_connection_manager/BUILD index fdb8b2b23abf..eb590c3cb195 100644 --- a/source/extensions/filters/network/http_connection_manager/BUILD +++ b/source/extensions/filters/network/http_connection_manager/BUILD @@ -51,6 +51,7 @@ envoy_cc_extension( "//source/common/network:cidr_range_lib", "//source/common/protobuf:utility_lib", "//source/common/router:route_provider_manager_lib", + "//source/common/runtime:runtime_lib", "//source/common/tracing:custom_tag_lib", "//source/common/tracing:http_tracer_lib", "//source/common/tracing:tracer_config_lib", diff --git a/source/extensions/filters/network/redis_proxy/BUILD b/source/extensions/filters/network/redis_proxy/BUILD index 39d05785bfde..1881d9099734 100644 --- a/source/extensions/filters/network/redis_proxy/BUILD +++ b/source/extensions/filters/network/redis_proxy/BUILD @@ -131,6 +131,7 @@ envoy_cc_library( "//source/common/common:assert_lib", "//source/common/config:datasource_lib", "//source/common/config:utility_lib", + "//source/common/event:real_time_system_lib", "//source/extensions/common/dynamic_forward_proxy:dns_cache_interface", "//source/extensions/filters/network/common/redis:codec_interface", "@envoy_api//envoy/extensions/filters/network/redis_proxy/v3:pkg_cc_proto", diff --git a/source/server/BUILD b/source/server/BUILD index f37fa4146c0d..95e886beb8f7 100644 --- a/source/server/BUILD +++ b/source/server/BUILD @@ -116,7 +116,6 @@ envoy_cc_library( "//envoy/server:drain_manager_interface", "//envoy/server:instance_interface", "//source/common/common:assert_lib", - "//source/common/common:callback_impl_lib", "//source/common/common:minimal_logger_lib", "@envoy_api//envoy/config/listener/v3:pkg_cc_proto", ], diff --git a/source/server/api_listener_impl.h b/source/server/api_listener_impl.h index 4d862498f8f9..74b98e8f0c12 100644 --- a/source/server/api_listener_impl.h +++ b/source/server/api_listener_impl.h @@ -43,10 +43,6 @@ class ApiListenerImplBase : public ApiListener, // Network::DrainDecision // TODO(junr03): hook up draining to listener state management. bool drainClose() const override { return false; } - Common::CallbackHandlePtr addOnDrainCloseCb(DrainCloseCb) const override { - IS_ENVOY_BUG("Unexpected call to addOnDrainCloseCb"); - return nullptr; - } protected: ApiListenerImplBase(Network::Address::InstanceConstSharedPtr&& address, diff --git a/source/server/drain_manager_impl.cc b/source/server/drain_manager_impl.cc index 31704f476973..ef4820ef9588 100644 --- a/source/server/drain_manager_impl.cc +++ b/source/server/drain_manager_impl.cc @@ -3,10 +3,8 @@ #include #include #include -#include #include "envoy/config/listener/v3/listener.pb.h" -#include "envoy/event/dispatcher.h" #include "envoy/event/timer.h" #include "source/common/common/assert.h" @@ -15,30 +13,8 @@ namespace Envoy { namespace Server { DrainManagerImpl::DrainManagerImpl(Instance& server, - envoy::config::listener::v3::Listener::DrainType drain_type, - Event::Dispatcher& dispatcher) - : server_(server), dispatcher_(dispatcher), drain_type_(drain_type), - children_(Common::ThreadSafeCallbackManager::create()) {} - -DrainManagerPtr -DrainManagerImpl::createChildManager(Event::Dispatcher& dispatcher, - envoy::config::listener::v3::Listener::DrainType drain_type) { - auto child = std::make_unique(server_, drain_type, dispatcher); - - // Wire up the child so that when the parent starts draining, the child also sees the - // state-change - auto child_cb = children_->add(dispatcher, [child = child.get()] { - if (!child->draining_) { - child->startDrainSequence([] {}); - } - }); - child->parent_callback_handle_ = std::move(child_cb); - return child; -} - -DrainManagerPtr DrainManagerImpl::createChildManager(Event::Dispatcher& dispatcher) { - return createChildManager(dispatcher, drain_type_); -} + envoy::config::listener::v3::Listener::DrainType drain_type) + : server_(server), drain_type_(drain_type) {} bool DrainManagerImpl::drainClose() const { // If we are actively health check failed and the drain type is default, always drain close. @@ -63,7 +39,7 @@ bool DrainManagerImpl::drainClose() const { // P(return true) = elapsed time / drain timeout // If the drain deadline is exceeded, skip the probability calculation. - const MonotonicTime current_time = dispatcher_.timeSource().monotonicTime(); + const MonotonicTime current_time = server_.dispatcher().timeSource().monotonicTime(); if (current_time >= drain_deadline_) { return true; } @@ -85,63 +61,12 @@ bool DrainManagerImpl::drainClose() const { (server_.api().randomGenerator().random() % drain_time_count); } -Common::CallbackHandlePtr DrainManagerImpl::addOnDrainCloseCb(DrainCloseCb cb) const { - ASSERT(dispatcher_.isThreadSafe()); - - if (draining_) { - const MonotonicTime current_time = dispatcher_.timeSource().monotonicTime(); - - // Calculate the delay. If using an immediate drain-strategy or past our deadline, use - // a zero millisecond delay. Otherwise, pick a random value within the remaining time-span. - std::chrono::milliseconds drain_delay{0}; - if (server_.options().drainStrategy() != Server::DrainStrategy::Immediate) { - if (current_time < drain_deadline_) { - const auto delta = drain_deadline_ - current_time; - const auto ms = std::chrono::duration_cast(delta).count(); - - // Note; current_time may be less than drain_deadline_ by only a - // microsecond (delta will be 1000 nanoseconds), in which case when we - // convert to milliseconds that will be 0, which will throw a SIGFPE - // if used as a modulus unguarded. - if (ms > 0) { - drain_delay = std::chrono::milliseconds(server_.api().randomGenerator().random() % ms); - } - } - } - THROW_IF_NOT_OK(cb(drain_delay)); - return nullptr; - } - - return cbs_.add(cb); -} - -void DrainManagerImpl::addDrainCompleteCallback(std::function cb) { - ASSERT(dispatcher_.isThreadSafe()); - ASSERT(draining_); - - // If the drain-tick-timer is active, add the callback to the queue. If not defined - // then it must have already expired, invoke the callback immediately. - if (drain_tick_timer_) { - drain_complete_cbs_.push_back(cb); - } else { - cb(); - } -} - void DrainManagerImpl::startDrainSequence(std::function drain_complete_cb) { - ASSERT(dispatcher_.isThreadSafe()); - ASSERT(drain_complete_cb); - - // If we've already started draining (either through direct invocation or through - // parent-initiated draining), enqueue the drain_complete_cb and return - if (draining_) { - addDrainCompleteCallback(drain_complete_cb); - return; - } - ASSERT(!drain_tick_timer_); - const std::chrono::seconds drain_delay(server_.options().drainTime()); + drain_tick_timer_ = server_.dispatcher().createTimer(drain_complete_cb); + const std::chrono::seconds drain_delay(server_.options().drainTime()); + drain_tick_timer_->enableTimer(drain_delay); // Note https://github.com/envoyproxy/envoy/issues/31457, previous to which, // drain_deadline_ was set *after* draining_ resulting in a read/write race between // the main thread running this function from admin, and the worker thread calling @@ -149,52 +74,13 @@ void DrainManagerImpl::startDrainSequence(std::function drain_complete_c // to set the time-since epoch to a count of 0 // (https://en.cppreference.com/w/cpp/chrono/time_point/time_point). ASSERT(drain_deadline_.time_since_epoch().count() == 0, "drain_deadline_ cannot be set twice."); - // Since draining_ is atomic, it is safe to set drain_deadline_ without a mutex // as drain_close() only reads from drain_deadline_ if draining_ is true, and // C++ will not re-order an assign to an atomic. See // https://stackoverflow.com/questions/40320254/reordering-atomic-operations-in-c . - drain_deadline_ = dispatcher_.timeSource().monotonicTime() + drain_delay; - + drain_deadline_ = server_.dispatcher().timeSource().monotonicTime() + drain_delay; // Atomic assign must come after the assign to drain_deadline_. draining_.store(true, std::memory_order_seq_cst); - - // Signal to child drain-managers to start their drain sequence - children_->runCallbacks(); - - // Schedule callback to run at end of drain time - drain_tick_timer_ = dispatcher_.createTimer([this]() { - for (auto& cb : drain_complete_cbs_) { - cb(); - } - drain_complete_cbs_.clear(); - drain_tick_timer_.reset(); - }); - addDrainCompleteCallback(drain_complete_cb); - drain_tick_timer_->enableTimer(drain_delay); - - // Call registered on-drain callbacks - with gradual delays - // Note: This will distribute drain events in the first 1/4th of the drain window - // to ensure that we initiate draining with enough time for graceful shutdowns. - const MonotonicTime current_time = dispatcher_.timeSource().monotonicTime(); - std::chrono::seconds remaining_time{0}; - if (server_.options().drainStrategy() != Server::DrainStrategy::Immediate && - current_time < drain_deadline_) { - remaining_time = - std::chrono::duration_cast(drain_deadline_ - current_time); - ASSERT(server_.options().drainTime() >= remaining_time); - } - - uint32_t step_count = 0; - size_t num_cbs = cbs_.size(); - THROW_IF_NOT_OK(cbs_.runCallbacksWith([&]() { - // switch to floating-point math to avoid issues with integer division - std::chrono::milliseconds delay{static_cast( - static_cast(step_count) / 4 / num_cbs * - std::chrono::duration_cast(remaining_time).count())}; - step_count++; - return delay; - })); } void DrainManagerImpl::startParentShutdownSequence() { diff --git a/source/server/drain_manager_impl.h b/source/server/drain_manager_impl.h index 6c20ec8b04c7..352d0669b38a 100644 --- a/source/server/drain_manager_impl.h +++ b/source/server/drain_manager_impl.h @@ -1,17 +1,13 @@ #pragma once -#include #include -#include #include "envoy/common/time.h" #include "envoy/config/listener/v3/listener.pb.h" -#include "envoy/event/dispatcher.h" #include "envoy/event/timer.h" #include "envoy/server/drain_manager.h" #include "envoy/server/instance.h" -#include "source/common/common/callback_impl.h" #include "source/common/common/logger.h" namespace Envoy { @@ -25,41 +21,23 @@ namespace Server { */ class DrainManagerImpl : Logger::Loggable, public DrainManager { public: - DrainManagerImpl(Instance& server, envoy::config::listener::v3::Listener::DrainType drain_type, - Event::Dispatcher& dispatcher); + DrainManagerImpl(Instance& server, envoy::config::listener::v3::Listener::DrainType drain_type); // Network::DrainDecision bool drainClose() const override; - Common::CallbackHandlePtr addOnDrainCloseCb(DrainCloseCb cb) const override; // Server::DrainManager void startDrainSequence(std::function drain_complete_cb) override; bool draining() const override { return draining_; } void startParentShutdownSequence() override; - DrainManagerPtr - createChildManager(Event::Dispatcher& dispatcher, - envoy::config::listener::v3::Listener::DrainType drain_type) override; - DrainManagerPtr createChildManager(Event::Dispatcher& dispatcher) override; private: - void addDrainCompleteCallback(std::function cb); - Instance& server_; - Event::Dispatcher& dispatcher_; const envoy::config::listener::v3::Listener::DrainType drain_type_; std::atomic draining_{false}; Event::TimerPtr drain_tick_timer_; MonotonicTime drain_deadline_; - mutable Common::CallbackManager cbs_{}; - std::vector> drain_complete_cbs_{}; - - // Callbacks called by startDrainSequence to cascade/proxy to children - std::shared_ptr children_; - - // Callback handle parent will invoke to initiate drain-sequence. Created and set - // by the parent drain-manager. - Common::CallbackHandlePtr parent_callback_handle_; Event::TimerPtr parent_shutdown_timer_; }; diff --git a/test/common/common/callback_impl_test.cc b/test/common/common/callback_impl_test.cc index bf1410fa495d..965fc540182e 100644 --- a/test/common/common/callback_impl_test.cc +++ b/test/common/common/callback_impl_test.cc @@ -1,10 +1,5 @@ -#include -#include - #include "source/common/common/callback_impl.h" -#include "test/mocks/event/mocks.h" - #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -68,104 +63,5 @@ TEST_F(CallbackManagerTest, DestroyManagerBeforeHandle) { handle.reset(); } -class ThreadSafeCallbackManagerTest : public testing::Test { -public: - MOCK_METHOD(void, called, (int arg)); -}; - -// Test basic behaviors of the thread-safe callback-manager with respect to callback registration, -// de-registration, and execution. -TEST_F(ThreadSafeCallbackManagerTest, All) { - InSequence s; - - testing::NiceMock cb_dispatcher; - ON_CALL(cb_dispatcher, post(_)).WillByDefault(Invoke([](Event::PostCb cb) { cb(); })); - - auto manager = ThreadSafeCallbackManager::create(); - - auto handle1 = manager->add(cb_dispatcher, [this]() { - called(5); - return absl::OkStatus(); - }); - auto handle2 = manager->add(cb_dispatcher, [this]() { - called(10); - return absl::OkStatus(); - }); - - EXPECT_CALL(*this, called(5)); - EXPECT_CALL(*this, called(10)); - manager->runCallbacks(); - - handle1.reset(); - EXPECT_CALL(*this, called(10)); - manager->runCallbacks(); - - EXPECT_CALL(*this, called(10)); - EXPECT_CALL(*this, called(20)); - auto handle3 = manager->add(cb_dispatcher, [this]() { - called(20); - return absl::OkStatus(); - }); - manager->runCallbacks(); - handle3.reset(); - - EXPECT_CALL(*this, called(10)); - manager->runCallbacks(); -} - -// Validate that the handles returned from callback-registration can outlive the manager -// and can be destructed without error. -TEST_F(ThreadSafeCallbackManagerTest, DestroyManagerBeforeHandle) { - testing::NiceMock cb_dispatcher; - ON_CALL(cb_dispatcher, post(_)).WillByDefault(Invoke([](Event::PostCb cb) { cb(); })); - - CallbackHandlePtr handle; - { - auto manager = ThreadSafeCallbackManager::create(); - handle = manager->add(cb_dispatcher, [this]() { - called(5); - return absl::OkStatus(); - }); - EXPECT_CALL(*this, called(5)); - manager->runCallbacks(); - } - EXPECT_NE(nullptr, handle); - // It should be safe to destruct the handle after the manager. - handle.reset(); -} - -// Validate that a callback added and removed from a thread (and thus dispatcher) that -// no longer exist is a safe operation. -TEST_F(ThreadSafeCallbackManagerTest, RegisterAndRemoveOnExpiredThread) { - auto manager = ThreadSafeCallbackManager::create(); - - testing::NiceMock cb_dispatcher; - ON_CALL(cb_dispatcher, post(_)).WillByDefault(Invoke([](Event::PostCb cb) { cb(); })); - - // Register a callback in a new thread and then remove it - auto t = std::thread([this, manager = manager.get()] { - testing::NiceMock cb_dispatcher; - ON_CALL(cb_dispatcher, post(_)).WillByDefault(Invoke([](Event::PostCb cb) { cb(); })); - - auto handle = manager->add(cb_dispatcher, [this]() { - called(20); - return absl::OkStatus(); - }); - handle.reset(); - }); - - // Add another callback on the main thread - auto handle = manager->add(cb_dispatcher, [this]() { - called(10); - return absl::OkStatus(); - }); - - // Validate that we can wait for the above thread to terminate (and de-register the - // callback), then run the remaining callbacks. - t.join(); - EXPECT_CALL(*this, called(10)); - manager->runCallbacks(); -} - } // namespace Common } // namespace Envoy diff --git a/test/integration/server.h b/test/integration/server.h index d62e81b7389c..e0a411056fbf 100644 --- a/test/integration/server.h +++ b/test/integration/server.h @@ -55,8 +55,8 @@ OptionsImplBase createTestOptionsImpl( class TestComponentFactory : public ComponentFactory { public: Server::DrainManagerPtr createDrainManager(Server::Instance& server) override { - return Server::DrainManagerPtr{new Server::DrainManagerImpl( - server, envoy::config::listener::v3::Listener::MODIFY_ONLY, server.dispatcher())}; + return Server::DrainManagerPtr{ + new Server::DrainManagerImpl(server, envoy::config::listener::v3::Listener::MODIFY_ONLY)}; } Runtime::LoaderPtr createRuntime(Server::Instance& server, Server::Configuration::Initial& config) override { @@ -537,8 +537,8 @@ class IntegrationTestServer : public Logger::Loggable, // Server::ComponentFactory Server::DrainManagerPtr createDrainManager(Server::Instance& server) override { - drain_manager_ = new Server::DrainManagerImpl( - server, envoy::config::listener::v3::Listener::MODIFY_ONLY, server.dispatcher()); + drain_manager_ = + new Server::DrainManagerImpl(server, envoy::config::listener::v3::Listener::MODIFY_ONLY); return Server::DrainManagerPtr{drain_manager_}; } Runtime::LoaderPtr createRuntime(Server::Instance& server, diff --git a/test/mocks/network/mocks.h b/test/mocks/network/mocks.h index a2b90f753a2c..43d77b6345b3 100644 --- a/test/mocks/network/mocks.h +++ b/test/mocks/network/mocks.h @@ -199,7 +199,6 @@ class MockDrainDecision : public DrainDecision { ~MockDrainDecision() override; MOCK_METHOD(bool, drainClose, (), (const)); - MOCK_METHOD(Common::CallbackHandlePtr, addOnDrainCloseCb, (DrainCloseCb cb), (const, override)); }; class MockListenerFilter : public ListenerFilter { diff --git a/test/mocks/server/drain_manager.h b/test/mocks/server/drain_manager.h index 883930091b1e..dc0331b05876 100644 --- a/test/mocks/server/drain_manager.h +++ b/test/mocks/server/drain_manager.h @@ -3,11 +3,8 @@ #include #include #include -#include #include -#include -#include "envoy/event/dispatcher.h" #include "envoy/server/drain_manager.h" #include "gmock/gmock.h" @@ -19,17 +16,11 @@ class MockDrainManager : public DrainManager { MockDrainManager(); ~MockDrainManager() override; - // Network::DrainManager - MOCK_METHOD(DrainManagerPtr, createChildManager, - (Event::Dispatcher&, envoy::config::listener::v3::Listener::DrainType), (override)); - MOCK_METHOD(DrainManagerPtr, createChildManager, (Event::Dispatcher&), (override)); + // Server::DrainManager + MOCK_METHOD(bool, drainClose, (), (const)); MOCK_METHOD(bool, draining, (), (const)); - MOCK_METHOD(void, startParentShutdownSequence, ()); MOCK_METHOD(void, startDrainSequence, (std::function completion)); - - // Network::DrainDecision - MOCK_METHOD(bool, drainClose, (), (const)); - MOCK_METHOD(Common::CallbackHandlePtr, addOnDrainCloseCb, (DrainCloseCb cb), (const, override)); + MOCK_METHOD(void, startParentShutdownSequence, ()); std::function drain_sequence_completion_; }; diff --git a/test/server/BUILD b/test/server/BUILD index fac1ee71961b..233d9276777a 100644 --- a/test/server/BUILD +++ b/test/server/BUILD @@ -142,7 +142,6 @@ envoy_cc_test( rbe_pool = "6gig", deps = [ "//source/server:drain_manager_lib", - "//test/mocks/event:event_mocks", "//test/mocks/server:instance_mocks", "@envoy_api//envoy/config/listener/v3:pkg_cc_proto", ], diff --git a/test/server/drain_manager_impl_test.cc b/test/server/drain_manager_impl_test.cc index cec26edbb870..652b87ba1c2e 100644 --- a/test/server/drain_manager_impl_test.cc +++ b/test/server/drain_manager_impl_test.cc @@ -1,6 +1,5 @@ #include -#include "envoy/common/callback.h" #include "envoy/config/listener/v3/listener.pb.h" #include "source/server/drain_manager_impl.h" @@ -11,7 +10,6 @@ #include "gtest/gtest.h" using testing::_; -using testing::AllOf; using testing::Ge; using testing::InSequence; using testing::Le; @@ -33,53 +31,12 @@ class DrainManagerImplTest : public Event::TestUsingSimulatedTime, .WillByDefault(Return(std::chrono::seconds(900))); } - template - void testRegisterCallbackAfterDrainBeginGradualStrategy(Duration delay) { - ON_CALL(server_.options_, drainStrategy()) - .WillByDefault(Return(Server::DrainStrategy::Gradual)); - ON_CALL(server_.options_, drainTime()).WillByDefault(Return(std::chrono::seconds(1))); - - DrainManagerImpl drain_manager(server_, envoy::config::listener::v3::Listener::DEFAULT, - server_.dispatcher()); - - testing::MockFunction cb_before_drain; - testing::MockFunction cb_after_drain1; - testing::MockFunction cb_after_drain2; - - EXPECT_CALL(cb_before_drain, Call(_)); - // Validate that callbacks after the drain sequence has started (or after the drain deadline - // has been reached) are called with a random value between 0 (immediate) and the max - // drain window (minus time that has passed). - EXPECT_CALL(cb_after_drain1, Call(_)).WillOnce(Invoke([](std::chrono::milliseconds delay) { - EXPECT_THAT(delay.count(), Ge(0)); - EXPECT_THAT(delay.count(), Le(990)); - return absl::OkStatus(); - })); - EXPECT_CALL(cb_after_drain2, Call(_)).WillOnce(Invoke([](std::chrono::milliseconds delay) { - EXPECT_EQ(delay.count(), 0); - return absl::OkStatus(); - })); - - auto before_handle = drain_manager.addOnDrainCloseCb(cb_before_drain.AsStdFunction()); - drain_manager.startDrainSequence([] {}); - - server_.api_.time_system_.advanceTimeWait(std::chrono::milliseconds(10)); - auto after_handle1 = drain_manager.addOnDrainCloseCb(cb_after_drain1.AsStdFunction()); - - server_.api_.time_system_.advanceTimeWait(delay); - auto after_handle2 = drain_manager.addOnDrainCloseCb(cb_after_drain2.AsStdFunction()); - - EXPECT_EQ(after_handle1, nullptr); - EXPECT_EQ(after_handle2, nullptr); - } - NiceMock server_; }; TEST_F(DrainManagerImplTest, Default) { InSequence s; - DrainManagerImpl drain_manager(server_, envoy::config::listener::v3::Listener::DEFAULT, - server_.dispatcher()); + DrainManagerImpl drain_manager(server_, envoy::config::listener::v3::Listener::DEFAULT); // Test parent shutdown. Event::MockTimer* shutdown_timer = new Event::MockTimer(&server_.dispatcher_); @@ -107,8 +64,7 @@ TEST_F(DrainManagerImplTest, Default) { TEST_F(DrainManagerImplTest, ModifyOnly) { InSequence s; - DrainManagerImpl drain_manager(server_, envoy::config::listener::v3::Listener::MODIFY_ONLY, - server_.dispatcher()); + DrainManagerImpl drain_manager(server_, envoy::config::listener::v3::Listener::MODIFY_ONLY); EXPECT_CALL(server_, healthCheckFailed()).Times(0); // Listener check will short-circuit EXPECT_FALSE(drain_manager.drainClose()); @@ -121,8 +77,7 @@ TEST_P(DrainManagerImplTest, DrainDeadline) { : Server::DrainStrategy::Immediate)); // TODO(auni53): Add integration tests for this once TestDrainManager is // removed. - DrainManagerImpl drain_manager(server_, envoy::config::listener::v3::Listener::DEFAULT, - server_.dispatcher()); + DrainManagerImpl drain_manager(server_, envoy::config::listener::v3::Listener::DEFAULT); // Ensure drainClose() behaviour is determined by the deadline. drain_manager.startDrainSequence([] {}); @@ -167,8 +122,7 @@ TEST_P(DrainManagerImplTest, DrainDeadlineProbability) { ON_CALL(server_.api_.random_, random()).WillByDefault(Return(4)); ON_CALL(server_.options_, drainTime()).WillByDefault(Return(std::chrono::seconds(3))); - DrainManagerImpl drain_manager(server_, envoy::config::listener::v3::Listener::DEFAULT, - server_.dispatcher()); + DrainManagerImpl drain_manager(server_, envoy::config::listener::v3::Listener::DEFAULT); EXPECT_CALL(server_, healthCheckFailed()).WillOnce(Return(true)); EXPECT_TRUE(drain_manager.drainClose()); @@ -199,399 +153,8 @@ TEST_P(DrainManagerImplTest, DrainDeadlineProbability) { } } -TEST_P(DrainManagerImplTest, OnDrainCallbacks) { - constexpr int num_cbs = 20; - const bool drain_gradually = GetParam(); - ON_CALL(server_.options_, drainStrategy()) - .WillByDefault(Return(drain_gradually ? Server::DrainStrategy::Gradual - : Server::DrainStrategy::Immediate)); - ON_CALL(server_.options_, drainTime()).WillByDefault(Return(std::chrono::seconds(4))); - - DrainManagerImpl drain_manager(server_, envoy::config::listener::v3::Listener::DEFAULT, - server_.dispatcher()); - - { - // Register callbacks (store in array to keep in scope for test) - std::array, num_cbs> cbs; - std::array cb_handles; - for (auto i = 0; i < num_cbs; i++) { - auto& cb = cbs[i]; - if (drain_gradually) { - auto step = 1000 / num_cbs; - EXPECT_CALL(cb, Call(_)).WillRepeatedly(Invoke([i, step](std::chrono::milliseconds delay) { - // Everything should happen within the first 1/4 of the drain time - EXPECT_LT(delay.count(), 1001); - - // Validate that our wait times are spread out (within some small error) - EXPECT_THAT(delay.count(), AllOf(Ge(i * step - 1), Le(i * step + 1))); - return absl::OkStatus(); - })); - } else { - EXPECT_CALL(cb, Call(std::chrono::milliseconds{0})); - } - - cb_handles[i] = drain_manager.addOnDrainCloseCb(cb.AsStdFunction()); - } - drain_manager.startDrainSequence([] {}); - } - - EXPECT_TRUE(drain_manager.draining()); -} - INSTANTIATE_TEST_SUITE_P(DrainStrategies, DrainManagerImplTest, testing::Bool()); -// Test gradual draining when there are more callbacks than milliseconds in the drain time, -// which should cause some drains to happen within roughly the same window. -TEST_F(DrainManagerImplTest, OnDrainCallbacksManyGradualSteps) { - constexpr int num_cbs = 3000; - ON_CALL(server_.options_, drainStrategy()).WillByDefault(Return(Server::DrainStrategy::Gradual)); - ON_CALL(server_.options_, drainTime()).WillByDefault(Return(std::chrono::seconds(4))); - - DrainManagerImpl drain_manager(server_, envoy::config::listener::v3::Listener::DEFAULT, - server_.dispatcher()); - - { - // Register callbacks (store in array to keep in scope for test) - std::array, num_cbs> cbs; - std::array cb_handles; - for (auto i = 0; i < num_cbs; i++) { - auto& cb = cbs[i]; - auto step = 1000.0 / num_cbs; - EXPECT_CALL(cb, Call(_)).WillRepeatedly(Invoke([i, step](std::chrono::milliseconds delay) { - // Everything should happen within the first 1/4 of the drain time - EXPECT_LT(delay.count(), 1001); - - // Validate that our wait times are spread out (within some small error) - EXPECT_THAT(delay.count(), AllOf(Ge(i * step - 1), Le(i * step + 1))); - return absl::OkStatus(); - })); - - cb_handles[i] = drain_manager.addOnDrainCloseCb(cb.AsStdFunction()); - } - drain_manager.startDrainSequence([] {}); - } - - EXPECT_TRUE(drain_manager.draining()); -} - -// Test gradual draining when the number of callbacks does not evenly divide into -// the drain time. -TEST_F(DrainManagerImplTest, OnDrainCallbacksNonEvenlyDividedSteps) { - constexpr int num_cbs = 30; - ON_CALL(server_.options_, drainStrategy()).WillByDefault(Return(Server::DrainStrategy::Gradual)); - ON_CALL(server_.options_, drainTime()).WillByDefault(Return(std::chrono::seconds(1))); - - DrainManagerImpl drain_manager(server_, envoy::config::listener::v3::Listener::DEFAULT, - server_.dispatcher()); - - { - // Register callbacks (store in array to keep in scope for test) - std::array, num_cbs> cbs; - std::array cb_handles; - for (auto i = 0; i < num_cbs; i++) { - auto& cb = cbs[i]; - auto step = 250.0 / num_cbs; - EXPECT_CALL(cb, Call(_)).WillRepeatedly(Invoke([i, step](std::chrono::milliseconds delay) { - // Everything should happen within the first 1/4 of the drain time - EXPECT_LT(delay.count(), 251); - - // Validate that our wait times are spread out (within some small error) - EXPECT_THAT(delay.count(), AllOf(Ge(i * step - 1), Le(i * step + 1))); - return absl::OkStatus(); - })); - - cb_handles[i] = drain_manager.addOnDrainCloseCb(cb.AsStdFunction()); - } - - drain_manager.startDrainSequence([] {}); - } - - EXPECT_TRUE(drain_manager.draining()); -} - -// Validate the expected behavior when a drain-close callback is registered -// after draining has begun with a Gradual drain strategy (should be called with -// delay between 0 and maximum) -TEST_F(DrainManagerImplTest, RegisterCallbackAfterDrainBeginGradualStrategy) { - testRegisterCallbackAfterDrainBeginGradualStrategy(std::chrono::milliseconds(1000)); -} - -// Repeat above test, but add simulated delay that falls 1 microsecond short of -// the deadline, thus triggering a corner case where the current time is less -// than the deadline by 1 microsecond, which rounds to 0 milliseconds. -TEST_F(DrainManagerImplTest, RegisterCallbackAfterDrainBeginGradualStrategyMicroDelay) { - testRegisterCallbackAfterDrainBeginGradualStrategy(std::chrono::microseconds(990 * 1000 - 1)); -} - -// Validate the expected behavior when a drain-close callback is registered after draining has begun -// with an Immediate drain strategy (should be called with 0 delay) -TEST_F(DrainManagerImplTest, RegisterCallbackAfterDrainBeginImmediateStrategy) { - ON_CALL(server_.options_, drainStrategy()).WillByDefault(Return(Server::DrainStrategy::Gradual)); - ON_CALL(server_.options_, drainTime()).WillByDefault(Return(std::chrono::seconds(1))); - - DrainManagerImpl drain_manager(server_, envoy::config::listener::v3::Listener::DEFAULT, - server_.dispatcher()); - - testing::MockFunction cb_before_drain; - testing::MockFunction cb_after_drain; - - EXPECT_CALL(cb_before_drain, Call(_)); - EXPECT_CALL(cb_after_drain, Call(_)).WillOnce(Invoke([](std::chrono::milliseconds delay) { - EXPECT_EQ(delay.count(), 0); - return absl::OkStatus(); - })); - - auto before_handle = drain_manager.addOnDrainCloseCb(cb_before_drain.AsStdFunction()); - drain_manager.startDrainSequence([] {}); - auto after_handle = drain_manager.addOnDrainCloseCb(cb_after_drain.AsStdFunction()); - EXPECT_EQ(after_handle, nullptr); -} - -// Destruction doesn't trigger draining, so it should be for the parent to be cleaned up -// before the child. -TEST_F(DrainManagerImplTest, ParentDestructedBeforeChildren) { - ON_CALL(server_.options_, drainStrategy()).WillByDefault(Return(Server::DrainStrategy::Gradual)); - ON_CALL(server_.options_, drainTime()).WillByDefault(Return(std::chrono::seconds(1))); - - auto parent = std::make_unique( - server_, envoy::config::listener::v3::Listener::DEFAULT, server_.dispatcher()); - auto child_a = parent->createChildManager(server_.dispatcher()); - auto child_b = parent->createChildManager(server_.dispatcher()); - - EXPECT_FALSE(parent->draining()); - EXPECT_FALSE(child_a->draining()); - EXPECT_FALSE(child_b->draining()); - - parent.reset(); - - // parent destruction should not effect drain state - EXPECT_FALSE(child_a->draining()); - EXPECT_FALSE(child_b->draining()); - - // Further children creation (from existing children) is still possible - auto child_a1 = child_a->createChildManager(server_.dispatcher()); - auto child_b1 = child_b->createChildManager(server_.dispatcher()); - EXPECT_TRUE(child_a1 != nullptr); - EXPECT_TRUE(child_b1 != nullptr); - - // draining cascades as expected - int called = 0; - testing::MockFunction cb_a1; - testing::MockFunction cb_b1; - EXPECT_CALL(cb_a1, Call(_)).WillRepeatedly(Invoke([&called](std::chrono::milliseconds) { - called += 1; - return absl::OkStatus(); - })); - EXPECT_CALL(cb_b1, Call(_)).WillRepeatedly(Invoke([&called](std::chrono::milliseconds) { - called += 1; - return absl::OkStatus(); - })); - auto handle_a1 = child_a1->addOnDrainCloseCb(cb_a1.AsStdFunction()); - auto handle_b1 = child_b1->addOnDrainCloseCb(cb_b1.AsStdFunction()); - child_a->startDrainSequence([] {}); - child_b->startDrainSequence([] {}); - EXPECT_EQ(called, 2); - - // It is safe to clean up children - child_a.reset(); - child_b.reset(); -} - -// Validate that draining will cascade through all nodes in the tree. This test uses the following -// tree structure: -// a -// │ -// ┌──────┴────────┐ -// ▼ ▼ -// b c -// │ │ -// ┌───┴────┐ ┌────┴───┐ -// ▼ ▼ ▼ ▼ -// d e f g -TEST_F(DrainManagerImplTest, DrainingCascadesThroughAllNodesInTree) { - ON_CALL(server_.options_, drainStrategy()).WillByDefault(Return(Server::DrainStrategy::Gradual)); - ON_CALL(server_.options_, drainTime()).WillByDefault(Return(std::chrono::seconds(1))); - - auto a = DrainManagerImpl(server_, envoy::config::listener::v3::Listener::DEFAULT, - server_.dispatcher()); - - auto b = a.createChildManager(server_.dispatcher()); - auto d = b->createChildManager(server_.dispatcher()); - auto e = b->createChildManager(server_.dispatcher()); - - auto c = a.createChildManager(server_.dispatcher()); - auto f = c->createChildManager(server_.dispatcher()); - auto g = c->createChildManager(server_.dispatcher()); - - // wire up callbacks at all levels - int call_count = 0; - std::array, 7> cbs; - - for (auto& cb : cbs) { - EXPECT_CALL(cb, Call(_)).WillOnce(Invoke([&call_count](std::chrono::milliseconds) { - call_count++; - return absl::OkStatus(); - })); - } - auto handle_a = a.addOnDrainCloseCb(cbs[0].AsStdFunction()); - auto handle_b = b->addOnDrainCloseCb(cbs[1].AsStdFunction()); - auto handle_c = c->addOnDrainCloseCb(cbs[2].AsStdFunction()); - auto handle_d = d->addOnDrainCloseCb(cbs[3].AsStdFunction()); - auto handle_e = e->addOnDrainCloseCb(cbs[4].AsStdFunction()); - auto handle_f = f->addOnDrainCloseCb(cbs[5].AsStdFunction()); - auto handle_g = g->addOnDrainCloseCb(cbs[6].AsStdFunction()); - - a.startDrainSequence([] {}); - EXPECT_EQ(call_count, 7); -} - -// Validate that sub-trees are independent of each other (a tree's drain-state is not affected by -// its neighbors). This test uses the following tree structure: -// a -// │ -// ┌──────┴────────┐ -// ▼ ▼ -// b c -// │ │ -// ┌───┴────┐ ┌────┴───┐ -// ▼ ▼ ▼ ▼ -// d e f g -// -// Draining will happen on B and validate that no impact is seen on C. -TEST_F(DrainManagerImplTest, DrainingIsIndependentToNeighbors) { - ON_CALL(server_.options_, drainStrategy()).WillByDefault(Return(Server::DrainStrategy::Gradual)); - ON_CALL(server_.options_, drainTime()).WillByDefault(Return(std::chrono::seconds(1))); - - auto a = DrainManagerImpl(server_, envoy::config::listener::v3::Listener::DEFAULT, - server_.dispatcher()); - - auto b = a.createChildManager(server_.dispatcher()); - auto d = b->createChildManager(server_.dispatcher()); - auto e = b->createChildManager(server_.dispatcher()); - - auto c = a.createChildManager(server_.dispatcher()); - auto f = c->createChildManager(server_.dispatcher()); - auto g = c->createChildManager(server_.dispatcher()); - - int call_count = 0; - testing::MockFunction cb_d; - testing::MockFunction cb_e; - testing::MockFunction cb_f; - testing::MockFunction cb_g; - - EXPECT_CALL(cb_d, Call(_)).WillOnce(Invoke([&call_count](std::chrono::milliseconds) { - call_count++; - return absl::OkStatus(); - })); - EXPECT_CALL(cb_e, Call(_)).WillOnce(Invoke([&call_count](std::chrono::milliseconds) { - call_count++; - return absl::OkStatus(); - })); - // validate neighbor remains uneffected - EXPECT_CALL(cb_f, Call(_)).Times(0); - EXPECT_CALL(cb_g, Call(_)).Times(0); - - auto handle_d = d->addOnDrainCloseCb(cb_d.AsStdFunction()); - auto handle_e = e->addOnDrainCloseCb(cb_e.AsStdFunction()); - auto handle_f = f->addOnDrainCloseCb(cb_f.AsStdFunction()); - auto handle_g = g->addOnDrainCloseCb(cb_g.AsStdFunction()); - - b->startDrainSequence([] {}); - EXPECT_EQ(call_count, 2); -} - -// Validate that draining of a child does not impact the drain-state of the parent -TEST_F(DrainManagerImplTest, DrainOnlyCascadesDownwards) { - ON_CALL(server_.options_, drainStrategy()).WillByDefault(Return(Server::DrainStrategy::Gradual)); - ON_CALL(server_.options_, drainTime()).WillByDefault(Return(std::chrono::seconds(1))); - - auto a = DrainManagerImpl(server_, envoy::config::listener::v3::Listener::DEFAULT, - server_.dispatcher()); - auto b = a.createChildManager(server_.dispatcher()); - auto c = b->createChildManager(server_.dispatcher()); - - int call_count = 0; - testing::MockFunction cb_a; - testing::MockFunction cb_b; - testing::MockFunction cb_c; - - // validate top-level callback is never fired - EXPECT_CALL(cb_a, Call(_)).Times(0); - EXPECT_CALL(cb_b, Call(_)).WillOnce(Invoke([&call_count](std::chrono::milliseconds) { - call_count++; - return absl::OkStatus(); - })); - EXPECT_CALL(cb_c, Call(_)).WillOnce(Invoke([&call_count](std::chrono::milliseconds) { - call_count++; - return absl::OkStatus(); - })); - auto handle_a = a.addOnDrainCloseCb(cb_a.AsStdFunction()); - auto handle_b = b->addOnDrainCloseCb(cb_b.AsStdFunction()); - auto handle_c = c->addOnDrainCloseCb(cb_c.AsStdFunction()); - - // drain the middle of the tree - b->startDrainSequence([] {}); - EXPECT_EQ(call_count, 2); -} - -// Validate that we can initiate draining on a child (to no effect) after the parent -// has already started draining -TEST_F(DrainManagerImplTest, DrainChildExplicitlyAfterParent) { - ON_CALL(server_.options_, drainStrategy()).WillByDefault(Return(Server::DrainStrategy::Gradual)); - ON_CALL(server_.options_, drainTime()).WillByDefault(Return(std::chrono::seconds(1))); - - auto a = DrainManagerImpl(server_, envoy::config::listener::v3::Listener::DEFAULT, - server_.dispatcher()); - auto b = a.createChildManager(server_.dispatcher()); - auto c = b->createChildManager(server_.dispatcher()); - - int call_count = 0; - testing::MockFunction cb; - - // validate top-level callback is never fired - EXPECT_CALL(cb, Call(_)).WillRepeatedly(Invoke([&call_count](std::chrono::milliseconds) { - call_count++; - return absl::OkStatus(); - })); - auto handle_a = a.addOnDrainCloseCb(cb.AsStdFunction()); - auto handle_b = b->addOnDrainCloseCb(cb.AsStdFunction()); - auto handle_c = c->addOnDrainCloseCb(cb.AsStdFunction()); - - // Drain the parent, then the child - a.startDrainSequence([&] {}); - b->startDrainSequence([&] {}); - EXPECT_EQ(call_count, 3); -} - -// Validate that we can initiate draining on a parent safely after a child has -// already started draining -TEST_F(DrainManagerImplTest, DrainParentAfterChild) { - ON_CALL(server_.options_, drainStrategy()).WillByDefault(Return(Server::DrainStrategy::Gradual)); - ON_CALL(server_.options_, drainTime()).WillByDefault(Return(std::chrono::seconds(1))); - - auto a = DrainManagerImpl(server_, envoy::config::listener::v3::Listener::DEFAULT, - server_.dispatcher()); - auto b = a.createChildManager(server_.dispatcher()); - auto c = b->createChildManager(server_.dispatcher()); - - int call_count = 0; - testing::MockFunction cb; - - // validate top-level callback is never fired - EXPECT_CALL(cb, Call(_)).WillRepeatedly(Invoke([&call_count](std::chrono::milliseconds) { - call_count++; - return absl::OkStatus(); - })); - auto handle_a = a.addOnDrainCloseCb(cb.AsStdFunction()); - auto handle_b = b->addOnDrainCloseCb(cb.AsStdFunction()); - auto handle_c = c->addOnDrainCloseCb(cb.AsStdFunction()); - - // Drain the child, then the parent - b->startDrainSequence([] {}); - a.startDrainSequence([] {}); - EXPECT_EQ(call_count, 3); -} - } // namespace } // namespace Server } // namespace Envoy diff --git a/tools/code_format/config.yaml b/tools/code_format/config.yaml index 3990fc67971f..fe97c500701a 100644 --- a/tools/code_format/config.yaml +++ b/tools/code_format/config.yaml @@ -140,7 +140,6 @@ paths: - source/common/grpc/async_client_impl.cc - source/common/grpc/google_grpc_creds_impl.cc - source/common/local_reply/local_reply.cc - - source/server/drain_manager_impl.cc - source/common/router/rds_impl.cc - source/server/hot_restarting_parent.cc - source/common/io/io_uring_worker_impl.cc