stats: bump hot restart version given the data structure change.

source/common/common/block_memory_hash_set.h

@@ -216,8 +216,8 @@ template <class Value> class BlockMemoryHashSet : public Logger::Loggable<Logger
    * Computes a version signature based on the options and the hash function.
    */
   std::string version() {
-    return fmt::format("options={} hash={}", control_->options.toString(),
-                       control_->hash_signature);
+    return fmt::format("options={} hash={} size={}", control_->options.toString(),
+                       control_->hash_signature, numBytes());
   }
 
 private:

source/server/hot_restart_impl.cc

@@ -26,7 +26,7 @@ namespace Server {
 
 // Increment this whenever there is a shared memory / RPC change that will prevent a hot restart
 // from working. Operations code can then cope with this and do a full restart.
-const uint64_t SharedMemory::VERSION = 9;
+const uint64_t SharedMemory::VERSION = 10;
 
 static BlockMemoryHashSetOptions blockMemHashOptions(uint64_t max_stats) {
   BlockMemoryHashSetOptions hash_set_options;

source/server/options_impl.cc

@@ -139,13 +139,6 @@ OptionsImpl::OptionsImpl(int argc, const char* const* argv,
   // TODO(jmarantz): should we also multiply these to bound the total amount of memory?
 
   hot_restart_disabled_ = disable_hot_restart.getValue();
-  if (hot_restart_version_option.getValue()) {
-    std::cerr << hot_restart_version_cb(max_stats.getValue(),
-                                        max_obj_name_len.getValue() +
-                                            Stats::RawStatData::maxStatSuffixLength(),
-                                        !hot_restart_disabled_);
-    throw NoServingException();
-  }
 
   log_level_ = default_log_level;
   for (size_t i = 0; i < ARRAY_SIZE(spdlog::level::level_names); i++) {
@@ -196,5 +189,14 @@ OptionsImpl::OptionsImpl(int argc, const char* const* argv,
   parent_shutdown_time_ = std::chrono::seconds(parent_shutdown_time_s.getValue());
   max_stats_ = max_stats.getValue();
   max_obj_name_length_ = max_obj_name_len.getValue();
+
+  if (hot_restart_version_option.getValue()) {
+    Stats::RawStatData::configure(*this);
+    std::cerr << hot_restart_version_cb(max_stats.getValue(),
+                                        max_obj_name_len.getValue() +
+                                            Stats::RawStatData::maxStatSuffixLength(),
+                                        !hot_restart_disabled_);
+    throw NoServingException();
+  }
 }
 } // namespace Envoy

test/server/BUILD

@@ -103,6 +103,7 @@ envoy_cc_test(
     srcs = ["options_impl_test.cc"],
     deps = [
         "//source/common/common:utility_lib",
+        "//source/common/stats:stats_lib",
         "//source/server:options_lib",
         "//test/test_common:utility_lib",
     ],

test/server/hot_restart_impl_test.cc

@@ -59,7 +59,7 @@ TEST_F(HotRestartImplTest, versionString) {
   // Tests that the version-string will be consistent and SharedMemory::VERSION,
   // between multiple instantiations.
   std::string version;
-  uint64_t max_stats;
+  uint64_t max_stats, max_obj_name_length;
 
   // The mocking infrastructure requires a test setup and teardown every time we
   // want to re-instantiate HotRestartImpl.
@@ -68,6 +68,7 @@ TEST_F(HotRestartImplTest, versionString) {
     version = hot_restart_->version();
     EXPECT_TRUE(absl::StartsWith(version, fmt::format("{}.", SharedMemory::VERSION))) << version;
     max_stats = options_.maxStats(); // Save this so we can double it below.
+    max_obj_name_length = options_.maxObjNameLength();
     TearDown();
   }
 
@@ -80,7 +81,15 @@ TEST_F(HotRestartImplTest, versionString) {
   {
     ON_CALL(options_, maxStats()).WillByDefault(Return(2 * max_stats));
     setup();
-    EXPECT_NE(version, hot_restart_->version()) << "Version changes when options change";
+    EXPECT_NE(version, hot_restart_->version()) << "Version changes when max-stats change";
+    TearDown();
+  }
+
+  {
+    ON_CALL(options_, maxObjNameLength()).WillByDefault(Return(2 * max_obj_name_length));
+    setup();
+    EXPECT_NE(version, hot_restart_->version())
+        << "Version changes when max-obj-name-length changes";
     // TearDown is called automatically at end of test.
   }
 }

test/server/options_impl_test.cc

@@ -6,6 +6,7 @@
 #include "envoy/common/exception.h"
 
 #include "common/common/utility.h"
+#include "common/stats/stats_impl.h"
 
 #include "server/options_impl.h"
 
@@ -19,6 +20,8 @@ using testing::HasSubstr;
 
 namespace Envoy {
 
+namespace {
+
 // Do the ugly work of turning a std::string into a char** and create an OptionsImpl. Args are
 // separated by a single space: no fancy quoting or escaping.
 std::unique_ptr<OptionsImpl> createOptionsImpl(const std::string& args) {
@@ -27,12 +30,23 @@ std::unique_ptr<OptionsImpl> createOptionsImpl(const std::string& args) {
   for (const std::string& s : words) {
     argv.push_back(s.c_str());
   }
-  return std::unique_ptr<OptionsImpl>(new OptionsImpl(argv.size(), const_cast<char**>(&argv[0]),
-                                                      [](uint64_t, uint64_t, bool) { return "1"; },
-                                                      spdlog::level::warn));
+  return std::make_unique<OptionsImpl>(
+      argv.size(), argv.data(), [](uint64_t, uint64_t, bool) { return "1"; }, spdlog::level::warn);
 }
 
+} // namespace
+
 TEST(OptionsImplTest, HotRestartVersion) {
+  // There's an evil static local in
+  // Stats::RawStatsData::initializeAndGetMutableMaxObjNameLength, which causes
+  // problems when all test.cc files are linked together for coverage-testing.
+  // This resets the static to the default options-value of 60. Note; this is only
+  // needed in coverage tests.
+  {
+    auto options = createOptionsImpl("envoy");
+    Stats::RawStatData::configureForTestsOnly(*options);
+  }
+
   EXPECT_THROW_WITH_REGEX(createOptionsImpl("envoy --hot-restart-version"), NoServingException,
                           "NoServingException");
 }