drain_manager: Unclean Revert of "HTTP2 Proactive GOAWAY on Drain - P…

…reamble (#17026)" (#37465)

This reverts commit 96dd735.


96dd735
was a precursor to
#16201
which never landed.

Unwinding the change
envoyproxy/envoy-mobile#176

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

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",
     ],
 )
 

envoy/network/drain_decision.h

@@ -1,38 +1,19 @@
 #pragma once
 
-#include <chrono>
-#include <functional>
-
-#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<absl::Status(std::chrono::milliseconds)>;
-
   virtual ~DrainDecision() = default;
 
   /**
    * @return TRUE if a connection should be drained and closed. It is up to individual network
    *         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

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(

envoy/server/drain_manager.h

@@ -3,43 +3,17 @@
 #include <functional>
 #include <memory>
 
-#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<DrainManager>;
-
 /**
  * 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

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",

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",
     ],
 )
 

source/common/common/callback_impl.h

@@ -2,24 +2,17 @@
 
 #include <functional>
 #include <list>
-#include <memory>
-#include <tuple>
 
 #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 <typename... CallbackArgs> class CallbackManager {
 public:
@@ -54,22 +47,6 @@ template <typename... CallbackArgs> 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<std::tuple<CallbackArgs...>(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 <typename... CallbackArgs> class CallbackManager {
   const std::shared_ptr<bool> still_alive_{std::make_shared<bool>(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<ThreadSafeCallbackManager> {
-  struct CallbackHolder;
-  using CallbackListEntry = std::tuple<CallbackHolder*, Event::Dispatcher&, std::weak_ptr<bool>>;
-
-public:
-  using Callback = std::function<void()>;
-
-  /**
-   * @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<ThreadSafeCallbackManager> create() {
-    return std::shared_ptr<ThreadSafeCallbackManager>(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<ThreadSafeCallbackManager> parent, Callback cb,
-                   Event::Dispatcher& cb_dispatcher);
-
-    ~CallbackHolder() override;
-
-    std::shared_ptr<ThreadSafeCallbackManager> parent_;
-    Callback cb_;
-    Event::Dispatcher& callback_dispatcher_;
-    std::shared_ptr<bool> still_alive_{std::make_shared<bool>(true)};
-
-    typename std::list<CallbackListEntry>::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<CallbackListEntry>::iterator& it);
-
-  // This must be held on all read/writes of callbacks_
-  mutable Thread::MutexBasicLockable lock_{};
-
-  std::list<CallbackListEntry> callbacks_ ABSL_GUARDED_BY(lock_);
-};
-
 } // namespace Common
 } // namespace Envoy

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;

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;
 

source/common/listener_manager/listener_manager_impl.cc

@@ -342,7 +342,7 @@ absl::StatusOr<Network::SocketSharedPtr> 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,

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",

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::DrainManagerImpl>(
-      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,

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",

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",

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",
     ],

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,