envoyproxy · mattklein123 · May 1, 2019 · Feb 11, 2019 · Feb 11, 2019 · Feb 12, 2019
diff --git a/api/envoy/admin/v2alpha/server_info.proto b/api/envoy/admin/v2alpha/server_info.proto
@@ -114,11 +114,9 @@ message CommandLineOptions {
   // See :option:`--mode` for details.
   Mode mode = 19;
 
-  // See :option:`--max-stats` for details.
-  uint64 max_stats = 20;
-
-  // See :option:`--max-obj-name-len` for details.
-  uint64 max_obj_name_len = 21;
+  // max_stats and max_obj_name_len are now unused and have no effect.
+  uint64 max_stats = 20 [deprecated = true];
+  uint64 max_obj_name_len = 21 [deprecated = true];
 
   // See :option:`--disable-hot-restart` for details.
   bool disable_hot_restart = 22;

diff --git a/api/envoy/api/v2/cds.proto b/api/envoy/api/v2/cds.proto
@@ -58,9 +58,6 @@ message Cluster {
   // :ref:`statistics <config_cluster_manager_cluster_stats>` if :ref:`alt_stat_name
   // <envoy_api_field_Cluster.alt_stat_name>` is not provided.
   // Any ``:`` in the cluster name will be converted to ``_`` when emitting statistics.
-  // By default, the maximum length of a cluster name is limited to 60
-  // characters. This limit can be increased by setting the
-  // :option:`--max-obj-name-len` command line argument to the desired value.
   string name = 1 [(validate.rules).string.min_bytes = 1];
 
   // An optional alternative to the cluster name to be used while emitting stats.

diff --git a/api/envoy/api/v2/lds.proto b/api/envoy/api/v2/lds.proto
@@ -46,9 +46,6 @@ message Listener {
   // The unique name by which this listener is known. If no name is provided,
   // Envoy will allocate an internal UUID for the listener. If the listener is to be dynamically
   // updated or removed via :ref:`LDS <config_listeners_lds>` a unique name must be provided.
-  // By default, the maximum length of a listener's name is limited to 60 characters. This limit can
-  // be increased by setting the :option:`--max-obj-name-len` command line argument to the desired
-  // value.
   string name = 1;
 
   // The address that the listener should listen on. In general, the address must be unique, though

diff --git a/bazel/README.md b/bazel/README.md
@@ -411,18 +411,6 @@ local_repository(
 ...
 ```
 
-## Stats Tunables
-
-The default maximum number of stats in shared memory, and the default
-maximum length of a cluster/route config/listener name, can be
-overridden at compile-time by defining `ENVOY_DEFAULT_MAX_STATS` and
-`ENVOY_DEFAULT_MAX_OBJ_NAME_LENGTH`, respectively, to the desired
-value. For example:
-
-```
-bazel build --copt=-DENVOY_DEFAULT_MAX_STATS=32768 --copt=-DENVOY_DEFAULT_MAX_OBJ_NAME_LENGTH=150 //source/exe:envoy-static
-```
-
 # Release builds
 
 Release builds should be built in `opt` mode, processed with `strip` and have a

diff --git a/docs/root/configuration/statistics.rst b/docs/root/configuration/statistics.rst
@@ -3,14 +3,6 @@
 Statistics
 ==========
 
-A few statistics are emitted to report statistics system behavior:
-
-.. csv-table::
-  :header: Name, Type, Description
-  :widths: 1, 1, 2
-
-  stats.overflow, Counter, Total number of times Envoy cannot allocate a statistic due to a shortage of shared memory
-
 Server
 ------
 

diff --git a/docs/root/intro/deprecated.rst b/docs/root/intro/deprecated.rst
@@ -12,6 +12,7 @@ Deprecated items below are listed in chronological order.
 
 Version 1.11.0 (Pending)
 ========================
+* The --max-stats and --max-obj-name-len flags no longer has any effect.
 * Use of :ref:`cluster <envoy_api_field_config.filter.network.redis_proxy.v2.RedisProxy.cluster>` in :ref:`redis_proxy.proto <envoy_api_file_envoy/config/filter/network/redis_proxy/v2/redis_proxy.proto>` is deprecated. Set a :ref:`catch_all_cluster <envoy_api_field_config.filter.network.redis_proxy.v2.RedisProxy.PrefixRoutes.catch_all_cluster>` instead.
 
 Version 1.10.0 (Apr 5, 2019)

diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst
@@ -9,6 +9,7 @@ Version history
 * event: added :ref:`loop duration and poll delay statistics <operations_performance>`.
 * ext_authz: added a `x-envoy-auth-partial-body` metadata header set to `false|true` indicating if there is a partial body sent in the authorization request message.
 * ext_authz: added option to `ext_authz` that allows the filter clearing route cache.
+* hot restart: stats are no longer shared between hot restart parent/child via shared memory, but rather by RPC. Hot restart version incremented to 11.
 * http: mitigated a race condition with the :ref:`delayed_close_timeout<envoy_api_field_config.filter.network.http_connection_manager.v2.HttpConnectionManager.delayed_close_timeout>` where it could trigger while actively flushing a pending write buffer for a downstream connection.
 * jwt_authn: make filter's parsing of JWT more flexible, allowing syntax like ``jwt=eyJhbGciOiJS...ZFnFIw,extra=7,realm=123``
 * redis: added :ref:`prefix routing <envoy_api_field_config.filter.network.redis_proxy.v2.RedisProxy.prefix_routes>` to enable routing commands based on their key's prefix to different upstream.
@@ -643,7 +644,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.
+  `--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,8 +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,
         "restart_epoch": 0,

diff --git a/docs/root/operations/cli.rst b/docs/root/operations/cli.rst
@@ -214,24 +214,12 @@ following are the command line options that Envoy supports.
   during a hot restart. See the :ref:`hot restart overview <arch_overview_hot_restart>` for more
   information. Defaults to 900 seconds (15 minutes).
 
-.. option:: --max-obj-name-len <uint64_t>
-
-  *(optional)* The maximum name length (in bytes) of the name field in a cluster/route_config/listener.
-  This setting is typically used in scenarios where the cluster names are auto generated, and often exceed
-  the built-in limit of 60 characters. Defaults to 60, and it's not valid to set to less than 60.
-
   .. attention::
 
     This setting affects the output of :option:`--hot-restart-version`. If you started Envoy with this
     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/docs/root/start/sandboxes/front_proxy.rst b/docs/root/start/sandboxes/front_proxy.rst
@@ -216,8 +216,6 @@ statistics. For example inside ``frontenvoy`` we can get::
       "service_node": "",
       "service_zone": "",
       "mode": "Serve",
-      "max_stats": "16384",
-      "max_obj_name_len": "60",
       "disable_hot_restart": false,
       "enable_mutex_tracing": false,
       "restart_epoch": 0,

diff --git a/include/envoy/server/BUILD b/include/envoy/server/BUILD
@@ -80,6 +80,7 @@ envoy_cc_library(
     deps = [
         "//include/envoy/event:dispatcher_interface",
         "//include/envoy/thread:thread_interface",
+        "//source/server:hot_restart_cc",
     ],
 )
 

diff --git a/include/envoy/server/hot_restart.h b/include/envoy/server/hot_restart.h
@@ -6,8 +6,11 @@
 #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"
 
+#include "source/server/hot_restart.pb.h"
+
 namespace Envoy {
 namespace Server {
 
@@ -20,13 +23,9 @@ class Instance;
  */
 class HotRestart {
 public:
-  struct GetParentStatsInfo {
-    uint64_t memory_allocated_;
-    uint64_t num_connections_;
-  };
-
-  struct ShutdownParentAdminInfo {
-    time_t original_start_time_;
+  struct ServerStatsFromParent {
+    uint64_t parent_memory_allocated_ = 0;
+    uint64_t parent_connections_ = 0;
   };
 
   virtual ~HotRestart() {}
@@ -46,32 +45,37 @@ class HotRestart {
   virtual int duplicateParentListenSocket(const std::string& address) PURE;
 
   /**
-   * 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;
-
-  /**
-   * 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;
 
   /**
    * Shutdown admin processing in the parent process if applicable. This allows admin processing
    * to start up in the new process.
-   * @param info will be filled with information from our parent if it can be retrieved.
+   * @param original_start_time will be filled with information from our parent, if retrieved.
    */
-  virtual void shutdownParentAdmin(ShutdownParentAdminInfo& info) PURE;
+  virtual void sendParentAdminShutdownRequest(time_t& original_start_time) PURE;
 
   /**
-   * Tell our parent to gracefully terminate itself.
+   * Tell our parent process to gracefully terminate itself.
    */
-  virtual void terminateParent() PURE;
+  virtual void sendParentTerminateRequest() PURE;
 
   /**
-   * Shutdown the hot restarter.
+   * Retrieve stats from our parent process and merges them into stats_store, taking into account
+   * the stats values we've already seen transferred.
+   * Skips all of the above and returns 0s if there is not currently a parent.
+   * @param stats_store the store whose stats will be updated.
+   * @param stats_proto the stats values we are updating with.
+   * @return special values relating to the "server" stats scope, whose
+   *         merging has to be handled by Server::InstanceImpl.
+   */
+  virtual ServerStatsFromParent mergeParentStatsIfAny(Stats::StoreRoot& stats_store) PURE;
+
+  /**
+   * Shutdown the half of our hot restarter that acts as a parent.
    */
   virtual void shutdown() PURE;
 
@@ -90,11 +94,6 @@ class HotRestart {
    * @return Thread::BasicLockable& a lock for access logs.
    */
   virtual Thread::BasicLockable& accessLogLock() PURE;
-
-  /**
-   * @returns an allocator for stats.
-   */
-  virtual Stats::StatDataAllocator& statsAllocator() PURE;
 };
 
 } // namespace Server

diff --git a/include/envoy/server/instance.h b/include/envoy/server/instance.h
@@ -87,12 +87,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
@@ -7,7 +7,6 @@
 #include "envoy/admin/v2alpha/server_info.pb.h"
 #include "envoy/common/pure.h"
 #include "envoy/network/address.h"
-#include "envoy/stats/stats_options.h"
 
 #include "spdlog/spdlog.h"
 
@@ -148,17 +147,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.
-   */
-  virtual const Stats::StatsOptions& statsOptions() const PURE;
-
   /**
    * @return bool indicating whether the hot restart functionality has been disabled via cli flags.
    */

diff --git a/include/envoy/stats/BUILD b/include/envoy/stats/BUILD
@@ -19,7 +19,6 @@ envoy_cc_library(
         "stat_data_allocator.h",
         "stats.h",
         "stats_matcher.h",
-        "stats_options.h",
         "store.h",
         "tag.h",
         "tag_extractor.h",

diff --git a/include/envoy/stats/scope.h b/include/envoy/stats/scope.h
@@ -5,7 +5,6 @@
 
 #include "envoy/common/pure.h"
 #include "envoy/stats/histogram.h"
-#include "envoy/stats/stats_options.h"
 #include "envoy/stats/symbol_table.h"
 
 namespace Envoy {
@@ -15,7 +14,6 @@ class Counter;
 class Gauge;
 class Histogram;
 class Scope;
-class StatsOptions;
 class NullGaugeImpl;
 
 typedef std::unique_ptr<Scope> ScopePtr;
@@ -86,12 +84,6 @@ class Scope {
    */
   virtual Histogram& histogram(const std::string& name) PURE;
 
-  /**
-   * @return a reference to the top-level StatsOptions struct, containing information about the
-   * maximum allowable object name length and stat suffix length.
-   */
-  virtual const Stats::StatsOptions& statsOptions() const PURE;
-
   /**
    * @return a reference to the symbol table.
    */

diff --git a/include/envoy/stats/stat_data_allocator.h b/include/envoy/stats/stat_data_allocator.h
@@ -23,6 +23,7 @@ namespace Stats {
  * be created utilizing a single fixed-size block suitable for
  * shared-memory, or in the heap, allowing for pointers and sharing of
  * substrings, with an opportunity for reduced memory consumption.
+ * TODO(fredlas) this interface can be deleted now that the shared memory version is gone.
  */
 class StatDataAllocator {
 public:
@@ -48,11 +49,6 @@ class StatDataAllocator {
   virtual GaugeSharedPtr makeGauge(StatName name, absl::string_view tag_extracted_name,
                                    const std::vector<Tag>& tags) PURE;
 
-  /**
-   * Determines whether this stats allocator requires bounded stat-name size.
-   */
-  virtual bool requiresBoundedStatNameSize() const PURE;
-
   virtual const SymbolTable& symbolTable() const PURE;
   virtual SymbolTable& symbolTable() PURE;
 

diff --git a/include/envoy/stats/stats.h b/include/envoy/stats/stats.h
@@ -9,6 +9,7 @@
 #include "envoy/stats/symbol_table.h"
 
 #include "absl/strings/string_view.h"
+#include "absl/types/optional.h"
 
 namespace Envoy {
 namespace Stats {
@@ -65,6 +66,20 @@ class Metric {
    */
   virtual bool used() const PURE;
 
+  /**
+   * Flags:
+   * Used: used by all stats types to figure out whether they have been used.
+   * Logic...: used by gauges to cache how they should be combined with a parent's value.
+   */
+  struct Flags {
+    static const uint8_t Used = 0x01;
+    // TODO(fredlas) these logic flags should be removed if we move to indicating combine logic in
+    // the stat declaration macros themselves. (Now that stats no longer use shared memory, it's
+    // safe to mess with what these flag bits mean whenever we want).
+    static const uint8_t LogicAccumulate = 0x02;
+    static const uint8_t LogicNeverImport = 0x04;
+    static const uint8_t LogicCached = LogicAccumulate | LogicNeverImport;
+  };
   virtual SymbolTable& symbolTable() PURE;
   virtual const SymbolTable& symbolTable() const PURE;
 };
@@ -99,6 +114,16 @@ class Gauge : public virtual Metric {
   virtual void set(uint64_t value) PURE;
   virtual void sub(uint64_t amount) PURE;
   virtual uint64_t value() const PURE;
+
+  /**
+   * Returns the stat's combine logic, if known.
+   */
+  virtual absl::optional<bool> cachedShouldImport() const PURE;
+
+  /**
+   * Sets the value to be returned by cachedCombineLogic().
+   */
+  virtual void setShouldImport(bool should_import) PURE;
 };
 
 typedef std::shared_ptr<Gauge> GaugeSharedPtr;