envoyproxy · mattklein123 · May 1, 2019 · Feb 11, 2019 · Feb 11, 2019 · Feb 12, 2019
diff --git a/api/envoy/admin/v2alpha/metrics.proto b/api/envoy/admin/v2alpha/metrics.proto
@@ -13,6 +13,7 @@ message SimpleMetric {
   enum Type {
     COUNTER = 0;
     GAUGE = 1;
+    BOOL_INDICATOR = 2;
   }
 
   // Type of the metric represented.

diff --git a/api/envoy/admin/v2alpha/server_info.proto b/api/envoy/admin/v2alpha/server_info.proto
@@ -114,7 +114,7 @@ message CommandLineOptions {
   // See :option:`--mode` for details.
   Mode mode = 19;
 
-  // See :option:`--max-stats` for details.
+  // Obsolete; do not use.
   uint64 max_stats = 20;
 
   // See :option:`--max-obj-name-len` for details.

diff --git a/api/envoy/api/v2/core/BUILD b/api/envoy/api/v2/core/BUILD
@@ -118,6 +118,12 @@ api_go_proto_library(
     deps = [":base_go_proto"],
 )
 
+api_proto_library_internal(
+    name = "hot_restart",
+    srcs = ["hot_restart.proto"],
+    deps = ["//envoy/admin/v2alpha:metrics"],
+)
+
 api_proto_library_internal(
     name = "protocol",
     srcs = ["protocol.proto"],

diff --git a/api/envoy/api/v2/core/hot_restart.proto b/api/envoy/api/v2/core/hot_restart.proto
@@ -0,0 +1,67 @@
+syntax = "proto3";
+
+package envoy.api.v2.core;
+
+option java_outer_classname = "HotRestartProto";
+option java_multiple_files = true;
+option java_package = "io.envoyproxy.envoy.api.v2.core";
+
+import "envoy/admin/v2alpha/metrics.proto";
+
+message HotRestartMessage {
+  // Child->parent requests
+  message Request {
+    message PassListenSocket {
+      string address = 1;
+    }
+    message ShutdownAdmin {
+    }
+    message Stats {
+    }
+    message DrainListeners {
+    }
+    message Terminate {
+    }
+    oneof request {
+      PassListenSocket pass_listen_socket = 1;
+      ShutdownAdmin shutdown_admin = 2;
+      Stats stats = 3;
+      DrainListeners drain_listeners = 4;
+      Terminate terminate = 5;
+    }
+  }
+
+  // Parent->child replies
+  message Reply {
+    message PassListenSocket {
+      int32 fd = 1;
+    }
+    message ShutdownAdmin {
+      uint64 original_start_time = 1;
+    }
+    message Stats {
+      // Values for server_stats, which don't fit with the "combination logic" approach.
+      uint64 memory_allocated = 1;
+      uint64 num_connections = 2;
+
+      // The parent's current values for various stats in its stats store.
+      repeated envoy.admin.v2alpha.SimpleMetric counters = 3;
+      repeated envoy.admin.v2alpha.SimpleMetric gauges = 4;
+    }
+    oneof reply {
+      // When this oneof is of the PassListenSocketReply type, there is a special
+      // implied meaning: the recvmsg that got this proto has control data to make
+      // the passing of the fd work, so make use of CMSG_SPACE etc.
+      PassListenSocket pass_listen_socket = 1;
+      ShutdownAdmin shutdown_admin = 2;
+      Stats stats = 3;
+    }
+  }
+
+  oneof requestreply {
+    Request request = 1;
+    Reply reply = 2;
+  }
+
+  bool didnt_recognize_your_last_message = 3;
+}
diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst
@@ -3,6 +3,11 @@ Version history
 
 1.11.0 (Pending)
 ================
+* hot restart: stats are no longer shared between hot restart parent/child via shared memory, but rather by RPC.
+* server: changed protocol that hot restart children/parents use; now protobuf based.
+* server: stats are now preserved across hot restart by being explicitly communicated through the hot
+          restart protocol, rather than living in shared memory.
+* server: hot restart version incremented to 11.
 
 1.10.0 (Apr 5, 2019)
 ====================
@@ -615,7 +620,7 @@ Version history
 * runtime: added :ref:`comment capability <config_runtime_comments>`.
 * server: change default log level (:option:`-l`) to `info`.
 * stats: maximum stat/name sizes and maximum number of stats are now variable via the
-  :option:`--max-obj-name-len` and :option:`--max-stats` options.
+  :option:`--max-obj-name-len` and `--max-stats` options.
 * tcp proxy: added :ref:`access logging <envoy_api_field_config.filter.network.tcp_proxy.v2.TcpProxy.access_log>`.
 * tcp proxy: added :ref:`configurable connect retries
   <envoy_api_field_config.filter.network.tcp_proxy.v2.TcpProxy.max_connect_attempts>`.

diff --git a/docs/root/operations/admin.rst b/docs/root/operations/admin.rst
@@ -211,7 +211,6 @@ modify different aspects of the server:
         "service_node": "",
         "service_zone": "",
         "mode": "Serve",
-        "max_stats": "16384",
         "max_obj_name_len": "60",
         "disable_hot_restart": false,
         "enable_mutex_tracing": false,

diff --git a/docs/root/operations/cli.rst b/docs/root/operations/cli.rst
@@ -226,12 +226,6 @@ following are the command line options that Envoy supports.
     option set to a non default value, you should use the same option (and same value) for subsequent hot
     restarts.
 
-.. option:: --max-stats <uint64_t>
-
-  *(optional)* The maximum number of stats that can be shared between hot-restarts. This setting
-  affects the output of :option:`--hot-restart-version`; the same value must be used to hot
-  restart. Defaults to 16384. It's not valid to set this larger than 100 million.
-
 .. option:: --disable-hot-restart
 
   *(optional)* This flag disables Envoy hot restart for builds that have it enabled. By default, hot

diff --git a/include/envoy/server/BUILD b/include/envoy/server/BUILD
@@ -108,6 +108,7 @@ envoy_cc_library(
         "//include/envoy/thread_local:thread_local_interface",
         "//include/envoy/tracing:http_tracer_interface",
         "//include/envoy/upstream:cluster_manager_interface",
+        "@envoy_api//envoy/api/v2/core:hot_restart_cc",
     ],
 )
 

diff --git a/include/envoy/server/hot_restart.h b/include/envoy/server/hot_restart.h
@@ -3,9 +3,11 @@
 #include <cstdint>
 #include <string>
 
+#include "envoy/api/v2/core/hot_restart.pb.h"
 #include "envoy/common/pure.h"
 #include "envoy/event/dispatcher.h"
 #include "envoy/stats/stat_data_allocator.h"
+#include "envoy/stats/store.h"
 #include "envoy/thread/thread.h"
 
 namespace Envoy {
@@ -20,11 +22,6 @@ class Instance;
  */
 class HotRestart {
 public:
-  struct GetParentStatsInfo {
-    uint64_t memory_allocated_;
-    uint64_t num_connections_;
-  };
-
   struct ShutdownParentAdminInfo {
     time_t original_start_time_;
   };
@@ -49,12 +46,12 @@ class HotRestart {
    * Retrieve stats from our parent process.
    * @param info will be filled with information from our parent if it can be retrieved.
    */
-  virtual void getParentStats(GetParentStatsInfo& info) PURE;
+  virtual std::unique_ptr<envoy::api::v2::core::HotRestartMessage> getParentStats() PURE;
 
   /**
-   * Initialize the restarter after primary server initialization begins. The hot restart
-   * implementation needs to be created early to deal with shared memory, logging, etc. so
-   * late initialization of needed interfaces is done here.
+   * Initialize the parent logic of our restarter. Meant to be called after initialization of a
+   * new child has begun. The hot restart implementation needs to be created early to deal with
+   * shared memory, logging, etc. so late initialization of needed interfaces is done here.
    */
   virtual void initialize(Event::Dispatcher& dispatcher, Server::Instance& server) PURE;
 
@@ -66,12 +63,22 @@ class HotRestart {
   virtual void shutdownParentAdmin(ShutdownParentAdminInfo& info) PURE;
 
   /**
-   * Tell our parent to gracefully terminate itself.
+   * Tell our parent process to gracefully terminate itself.
    */
   virtual void terminateParent() PURE;
 
   /**
-   * Shutdown the hot restarter.
+   * Merge stats_proto into stats_store, taking into account the stats values we've already
+   * seen transferred.
+   * @param stats_store the store whose stats will be updated.
+   * @param stats_proto the stats values we are updating with.
+   */
+  virtual void
+  mergeParentStats(Stats::StoreRoot& stats_store,
+                   const envoy::api::v2::core::HotRestartMessage::Reply::Stats& stats_proto) PURE;
+
+  /**
+   * Shutdown the half of our hot restarter that acts as a parent.
    */
   virtual void shutdown() PURE;
 

diff --git a/include/envoy/server/instance.h b/include/envoy/server/instance.h
@@ -6,6 +6,7 @@
 
 #include "envoy/access_log/access_log.h"
 #include "envoy/api/api.h"
+#include "envoy/api/v2/core/hot_restart.pb.h"
 #include "envoy/common/mutex_tracer.h"
 #include "envoy/event/timer.h"
 #include "envoy/http/context.h"
@@ -87,12 +88,6 @@ class Instance {
    */
   virtual void failHealthcheck(bool fail) PURE;
 
-  /**
-   * Fetch server stats specific to this process vs. global shared stats in a hot restart scenario.
-   * @param info supplies the stats structure to fill.
-   */
-  virtual void getParentStats(HotRestart::GetParentStatsInfo& info) PURE;
-
   /**
    * @return whether external healthchecks are currently failed or not.
    */

diff --git a/include/envoy/server/options.h b/include/envoy/server/options.h
@@ -148,11 +148,6 @@ class Options {
    */
   virtual const std::string& serviceZone() const PURE;
 
-  /**
-   * @return uint64_t the maximum number of stats gauges and counters.
-   */
-  virtual uint64_t maxStats() const PURE;
-
   /**
    * @return StatsOptions& the max stat name / suffix lengths for stats.
    * router/cluster/listener.

diff --git a/source/common/stats/thread_local_store.h b/source/common/stats/thread_local_store.h
@@ -132,7 +132,7 @@ class TlsScope : public Scope {
 
 /**
  * Store implementation with thread local caching. For design details see
- * https://github.com/envoyproxy/envoy/blob/master/docs/stats.md
+ * https://github.com/envoyproxy/envoy/blob/master/source/docs/stats.md
  */
 class ThreadLocalStoreImpl : Logger::Loggable<Logger::Id::stats>, public StoreRoot {
 public:

diff --git a/source/docs/stats.md b/source/docs/stats.md
@@ -9,31 +9,9 @@ binary program restarts. The metrics are tracked as:
    data accumulates. Unliked counters and gauges, histogram data is not retained across
    binary program restarts.
 
-## Hot-restart: `RawStatData` vs `HeapStatData`
-
 In order to support restarting the Envoy binary program without losing counter and gauge
-values, they are stored in a shared-memory block, including stats that are
-created dynamically at runtime in response to discovery of new clusters at
-runtime. To simplify memory management, each stat is allocated a fixed amount
-of storage, controlled via [command-line
-flags](https://www.envoyproxy.io/docs/envoy/latest/operations/cli):
-`--max-stats` and `--max-obj-name-len`, which determine the size of the pre-allocated
-shared-memory block. See
-[RawStatData](https://github.com/envoyproxy/envoy/blob/master/source/common/stats/raw_stat_data.h).
-
-Note in particular that the full stat name is retained in shared-memory, making
-it easy to correlate stats across restarts even as the dynamic cluster
-configuration changes.
-
-One challenge with this fixed memory allocation strategy is that it limits
-cluster scalability. A deployment wishing to use a single Envoy instance to
-manage tens of thousands of clusters, each with its own set of scoped stats,
-will use more memory than is ideal.
-
-A flag `--disable-hot-restart` pivots the system toward an alternate heap-based
-stat allocator that allocates stats on demand in the heap, with no preset limits
-on the number of stats or their length. See
-[HeapStatData](https://github.com/envoyproxy/envoy/blob/master/source/common/stats/heap_stat_data.h).
+values, they are passed from parent to child in an RPC protocol.
+They were previously held in shared memory, which imposed various restrictions.
 
 ## Performance and Thread Local Storage
 
@@ -67,10 +45,8 @@ This implementation is complicated so here is a rough overview of the threading
    reference the old scope which may be about to be cache flushed.
  * Since it's possible to have overlapping scopes, we de-dup stats when counters() or gauges() is
    called since these are very uncommon operations.
- * Though this implementation is designed to work with a fixed shared memory space, it will fall
-   back to heap allocated stats if needed. NOTE: In this case, overlapping scopes will not share
-   the same backing store. This is to keep things simple, it could be done in the future if
-   needed.
+ * Overlapping scopes will not share the same backing store. This is to keep things simple,
+   it could be done in the future if needed.
 
 ### Histogram threading model
 

diff --git a/source/exe/main_common.cc b/source/exe/main_common.cc
@@ -146,15 +146,13 @@ MainCommon::MainCommon(int argc, const char* const* argv)
             std::make_unique<Runtime::RandomGeneratorImpl>(), platform_impl_.threadFactory(),
             platform_impl_.fileSystem()) {}
 
-std::string MainCommon::hotRestartVersion(uint64_t max_num_stats, uint64_t max_stat_name_len,
-                                          bool hot_restart_enabled) {
+std::string MainCommon::hotRestartVersion(uint64_t max_stat_name_len, bool hot_restart_enabled) {
 #ifdef ENVOY_HOT_RESTART
   if (hot_restart_enabled) {
-    return Server::HotRestartImpl::hotRestartVersion(max_num_stats, max_stat_name_len);
+    return Server::HotRestartImpl::hotRestartVersion(max_stat_name_len);
   }
 #else
   UNREFERENCED_PARAMETER(hot_restart_enabled);
-  UNREFERENCED_PARAMETER(max_num_stats);
   UNREFERENCED_PARAMETER(max_stat_name_len);
 #endif
   return "disabled";

diff --git a/source/exe/main_common.h b/source/exe/main_common.h
@@ -103,8 +103,7 @@ class MainCommon {
     base_.adminRequest(path_and_query, method, handler);
   }
 
-  static std::string hotRestartVersion(uint64_t max_num_stats, uint64_t max_stat_name_len,
-                                       bool hot_restart_enabled);
+  static std::string hotRestartVersion(uint64_t max_stat_name_len, bool hot_restart_enabled);
 
 private:
 #ifdef ENVOY_HANDLE_SIGNALS

diff --git a/source/server/BUILD b/source/server/BUILD
@@ -108,9 +108,9 @@ envoy_cc_library(
 )
 
 envoy_cc_library(
-    name = "hot_restart_lib",
-    srcs = envoy_select_hot_restart(["hot_restart_impl.cc"]),
-    hdrs = envoy_select_hot_restart(["hot_restart_impl.h"]),
+    name = "hot_restarting_base",
+    srcs = envoy_select_hot_restart(["hot_restarting_base.cc"]),
+    hdrs = envoy_select_hot_restart(["hot_restarting_base.h"]),
     deps = [
         "//include/envoy/api:os_sys_calls_interface",
         "//include/envoy/event:dispatcher_interface",
@@ -125,6 +125,47 @@ envoy_cc_library(
         "//source/common/network:utility_lib",
         "//source/common/stats:raw_stat_data_lib",
         "//source/common/stats:stats_options_lib",
+        "@envoy_api//envoy/api/v2/core:hot_restart_cc",
+    ],
+)
+
+envoy_cc_library(
+    name = "hot_restarting_child",
+    srcs = envoy_select_hot_restart(["hot_restarting_child.cc"]),
+    hdrs = envoy_select_hot_restart(["hot_restarting_child.h"]),
+    deps = [
+        ":hot_restarting_base",
+    ],
+)
+
+envoy_cc_library(
+    name = "hot_restarting_parent",
+    srcs = envoy_select_hot_restart(["hot_restarting_parent.cc"]),
+    hdrs = envoy_select_hot_restart(["hot_restarting_parent.h"]),
+    deps = [
+        ":hot_restarting_base",
+        "//source/common/memory:stats_lib",
+    ],
+)
+
+envoy_cc_library(
+    name = "hot_restart_lib",
+    srcs = envoy_select_hot_restart(["hot_restart_impl.cc"]),
+    hdrs = envoy_select_hot_restart(["hot_restart_impl.h"]),
+    deps = [
+        ":hot_restarting_child",
+        ":hot_restarting_parent",
+        "//include/envoy/api:os_sys_calls_interface",
+        "//include/envoy/event:dispatcher_interface",
+        "//include/envoy/event:file_event_interface",
+        "//include/envoy/server:hot_restart_interface",
+        "//include/envoy/server:instance_interface",
+        "//include/envoy/server:options_interface",
+        "//source/common/api:os_sys_calls_lib",
+        "//source/common/common:assert_lib",
+        "//source/common/common:block_memory_hash_set_lib",
+        "//source/common/stats:heap_stat_data_lib",
+        "//source/common/stats:stats_options_lib",
     ],
 )
 

diff --git a/source/server/config_validation/server.h b/source/server/config_validation/server.h
@@ -75,7 +75,6 @@ class ValidationInstance : Logger::Loggable<Logger::Id::main>,
   DrainManager& drainManager() override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }
   AccessLog::AccessLogManager& accessLogManager() override { return access_log_manager_; }
   void failHealthcheck(bool) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }
-  void getParentStats(HotRestart::GetParentStatsInfo&) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }
   HotRestart& hotRestart() override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }
   Init::Manager& initManager() override { return init_manager_; }
   ServerLifecycleNotifier& lifecycleNotifier() override { return *this; }