From e1881aa62fdffd60cb804baa7a790d44247e937f Mon Sep 17 00:00:00 2001
From: Fred Douglas <fredlas@google.com>
Date: Tue, 7 May 2019 17:42:52 -0400
Subject: [PATCH 01/12] make hot restart counters temporary by using a scope

Signed-off-by: Fred Douglas <fredlas@google.com>
---
 source/common/stats/stat_merger.cc    | 11 +++--------
 source/common/stats/stat_merger.h     | 21 ++++++++++++++++++++-
 source/server/hot_restarting_child.cc |  4 ++++
 3 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/source/common/stats/stat_merger.cc b/source/common/stats/stat_merger.cc
index 55dbb1822dc0c..159ca8db37149 100644
--- a/source/common/stats/stat_merger.cc
+++ b/source/common/stats/stat_merger.cc
@@ -5,7 +5,8 @@
 namespace Envoy {
 namespace Stats {
 
-StatMerger::StatMerger(Stats::Store& target_store) : target_store_(target_store) {}
+StatMerger::StatMerger(Stats::Store& target_store)
+    : target_store_(target_store), temp_counter_scope_(target_store_.createScope("")) {}
 
 bool StatMerger::shouldImport(Gauge& gauge, const std::string& gauge_name) {
   absl::optional<bool> should_import = gauge.cachedShouldImport();
@@ -56,7 +57,7 @@ bool StatMerger::shouldImport(Gauge& gauge, const std::string& gauge_name) {
 
 void StatMerger::mergeCounters(const Protobuf::Map<std::string, uint64_t>& counter_deltas) {
   for (const auto& counter : counter_deltas) {
-    target_store_.counter(counter.first).add(counter.second);
+    temp_counter_scope_->counter(counter.first).add(counter.second);
   }
 }
 
@@ -79,12 +80,6 @@ void StatMerger::mergeGauges(const Protobuf::Map<std::string, uint64_t>& gauges)
   }
 }
 
-// TODO(fredlas) the current implementation can "leak" obsolete parent stats into the child.
-// That is, the parent had stat "foo", the child doesn't care about "foo" and back in the
-// shared memory implementation would have dropped it, but the import causes it to be made into
-// a real stat that stays around forever. The initial mini-consensus approach will be to
-// track which stats are actually getting used by the child, and drop those that aren't when
-// the hot restart completes.
 void StatMerger::mergeStats(const Protobuf::Map<std::string, uint64_t>& counter_deltas,
                             const Protobuf::Map<std::string, uint64_t>& gauges) {
   mergeCounters(counter_deltas);
diff --git a/source/common/stats/stat_merger.h b/source/common/stats/stat_merger.h
index d0d25874b4626..a082857004355 100644
--- a/source/common/stats/stat_merger.h
+++ b/source/common/stats/stat_merger.h
@@ -36,7 +36,26 @@ class StatMerger {
   void mergeCounters(const Protobuf::Map<std::string, uint64_t>& counter_deltas);
   void mergeGauges(const Protobuf::Map<std::string, uint64_t>& gauges);
   StatNameHashMap<uint64_t> parent_gauge_values_;
-  Stats::Store& target_store_;
+  Store& target_store_;
+  // A stats Scope for our in-the-merging-process counters to live in. Scopes conceptually hold
+  // shared_ptrs to the stats that live in them, with the question of which stats are living in a
+  // given scope determined by which stat names have been accessed via that scope. E.g., if you
+  // access a stat named "some.statricia" directly through the ordinary store, and then access a
+  // stat named "statricia" in a scope configured with the prefix "some.", there is now a single
+  // stat named some.statricia pointed to by both. As another example, if you access the stat
+  // "statrick" in the "some" scope, there will be a stat named "some.statrick" pointed to by just
+  // that scope. Now, if you delete the scope, some.statricia will stick around, but some.statrick
+  // will be destroyed.
+  //
+  // All of that is relevant here because it is used to get a certain desired behavior for counters.
+  // Specifically, counters must be kept up to date with values from the parent throughout hot
+  // restart, but once the restart completes, they must be dropped without a trace if the child has
+  // not taken action (independent of the hot restart stat merging) that would lead to them getting
+  // created in the store. By storing these counters in a scope (with an empty prefix), we can
+  // preserve all counters throughout the hot restart. Then, when the restart completes, dropping
+  // the scope will drop exactly those stats whose names have not already been accessed through
+  // another store/scope.
+  ScopePtr temp_counter_scope_;
 };
 
 } // namespace Stats
diff --git a/source/server/hot_restarting_child.cc b/source/server/hot_restarting_child.cc
index 3ff34ef0a844f..1807f53903c5c 100644
--- a/source/server/hot_restarting_child.cc
+++ b/source/server/hot_restarting_child.cc
@@ -82,6 +82,10 @@ void HotRestartingChild::sendParentTerminateRequest() {
   parent_terminated_ = true;
   // Once setting parent_terminated_ == true, we can send no more hot restart RPCs, and therefore
   // receive no more responses, including stats. So, now safe to forget our stat transferral state.
+  //
+  // This destruction is actually important far beyond memory efficiency. The scope-based temporary
+  // counter logic relies on the StatMerger getting destroyed once hot restart's stat merging is
+  // all done. (See stat_merger.h for details).
   stat_merger_.reset();
 }
 

From a0eff7f99047aac44eb7ba77353aa567e1e94163 Mon Sep 17 00:00:00 2001
From: Fred Douglas <fredlas@google.com>
Date: Tue, 7 May 2019 18:17:37 -0400
Subject: [PATCH 02/12] update test

Signed-off-by: Fred Douglas <fredlas@google.com>
---
 include/envoy/stats/store.h               |  5 ++
 source/common/stats/isolated_store_impl.h |  7 +++
 source/common/stats/thread_local_store.cc | 13 +++++
 source/common/stats/thread_local_store.h  |  1 +
 test/common/stats/BUILD                   |  4 ++
 test/common/stats/stat_merger_test.cc     | 60 ++++++++++++++++-------
 test/mocks/stats/mocks.h                  |  1 +
 7 files changed, 74 insertions(+), 17 deletions(-)

diff --git a/include/envoy/stats/store.h b/include/envoy/stats/store.h
index cd017f9ad8843..8f48202146ada 100644
--- a/include/envoy/stats/store.h
+++ b/include/envoy/stats/store.h
@@ -43,6 +43,11 @@ class Store : public Scope {
    * @return a list of all known histograms.
    */
   virtual std::vector<ParentHistogramSharedPtr> histograms() const PURE;
+
+  /**
+   * @return whether any known counter exists with this name.
+   */
+  virtual bool counterExists(const std::string& counter_name) PURE;
 };
 
 typedef std::unique_ptr<Store> StorePtr;
diff --git a/source/common/stats/isolated_store_impl.h b/source/common/stats/isolated_store_impl.h
index 8e720a6ad8e3e..656b54da03570 100644
--- a/source/common/stats/isolated_store_impl.h
+++ b/source/common/stats/isolated_store_impl.h
@@ -49,6 +49,8 @@ template <class Base> class IsolatedStatsCache {
     return vec;
   }
 
+  bool statExists(StatName name) { return stats_.find(name) != stats_.end(); }
+
 private:
   StatNameHashMap<std::shared_ptr<Base>> stats_;
   Allocator alloc_;
@@ -77,6 +79,11 @@ class IsolatedStoreImpl : public StoreImpl {
     return std::vector<ParentHistogramSharedPtr>{};
   }
 
+  bool counterExists(const std::string& counter_name) override {
+    StatNameManagedStorage storage(counter_name, symbolTable());
+    return counters_.statExists(storage.statName());
+  }
+
   Counter& counter(const std::string& name) override {
     StatNameManagedStorage storage(name, symbolTable());
     return counterFromStatName(storage.statName());
diff --git a/source/common/stats/thread_local_store.cc b/source/common/stats/thread_local_store.cc
index e9d1892bd2c2a..7ffea433c53bc 100644
--- a/source/common/stats/thread_local_store.cc
+++ b/source/common/stats/thread_local_store.cc
@@ -146,6 +146,19 @@ std::vector<ParentHistogramSharedPtr> ThreadLocalStoreImpl::histograms() const {
   return ret;
 }
 
+bool ThreadLocalStoreImpl::counterExists(const std::string& counter_name) {
+  Thread::LockGuard lock(lock_);
+  for (ScopeImpl* scope : scopes_) {
+
+    if (scope->central_cache_.counters_.find(
+            StatNameManagedStorage(counter_name, symbolTable()).statName()) !=
+        scope->central_cache_.counters_.end()) {
+      return true;
+    }
+  }
+  return false;
+}
+
 void ThreadLocalStoreImpl::initializeThreading(Event::Dispatcher& main_thread_dispatcher,
                                                ThreadLocal::Instance& tls) {
   main_thread_dispatcher_ = &main_thread_dispatcher;
diff --git a/source/common/stats/thread_local_store.h b/source/common/stats/thread_local_store.h
index 489b5b9604f07..c31cdc8a70f7f 100644
--- a/source/common/stats/thread_local_store.h
+++ b/source/common/stats/thread_local_store.h
@@ -168,6 +168,7 @@ class ThreadLocalStoreImpl : Logger::Loggable<Logger::Id::stats>, public StoreRo
   std::vector<CounterSharedPtr> counters() const override;
   std::vector<GaugeSharedPtr> gauges() const override;
   std::vector<ParentHistogramSharedPtr> histograms() const override;
+  bool counterExists(const std::string& counter_name) override;
 
   // Stats::StoreRoot
   void addSink(Sink& sink) override { timer_sinks_.push_back(sink); }
diff --git a/test/common/stats/BUILD b/test/common/stats/BUILD
index 1e1e7626c863f..971f1fe3a8213 100644
--- a/test/common/stats/BUILD
+++ b/test/common/stats/BUILD
@@ -51,8 +51,12 @@ envoy_cc_test(
     name = "stat_merger_test",
     srcs = ["stat_merger_test.cc"],
     deps = [
+        "//source/common/stats:fake_symbol_table_lib",
         "//source/common/stats:isolated_store_lib",
         "//source/common/stats:stat_merger_lib",
+        "//source/common/stats:thread_local_store_lib",
+        "//test/mocks/event:event_mocks",
+        "//test/mocks/thread_local:thread_local_mocks",
     ],
 )
 
diff --git a/test/common/stats/stat_merger_test.cc b/test/common/stats/stat_merger_test.cc
index 5d6d70b5073df..3af3c3d959d35 100644
--- a/test/common/stats/stat_merger_test.cc
+++ b/test/common/stats/stat_merger_test.cc
@@ -1,10 +1,17 @@
 #include <memory>
 
+#include "common/stats/fake_symbol_table_impl.h"
 #include "common/stats/isolated_store_impl.h"
 #include "common/stats/stat_merger.h"
+#include "common/stats/thread_local_store.h"
+
+#include "test/mocks/event/mocks.h"
+#include "test/mocks/thread_local/mocks.h"
 
 #include "gtest/gtest.h"
 
+using testing::NiceMock;
+
 namespace Envoy {
 namespace Stats {
 namespace {
@@ -52,23 +59,6 @@ TEST_F(StatMergerTest, counterMerge) {
   EXPECT_EQ(4, store_.counter("draculaer").latch());
 }
 
-// It should be fine for the parent to send us stats we haven't ourselves instantiated.
-// TODO(6756) This is how things currently work, but this is actually what 6756 is looking to avoid.
-TEST_F(StatMergerTest, newStatFromParent) {
-  Protobuf::Map<std::string, uint64_t> counter_values;
-  Protobuf::Map<std::string, uint64_t> counter_deltas;
-  Protobuf::Map<std::string, uint64_t> gauges;
-  counter_deltas["newcounter0"] = 0;
-  counter_deltas["newcounter1"] = 1;
-  gauges["newgauge"] = 5;
-  stat_merger_.mergeStats(counter_deltas, gauges);
-  EXPECT_EQ(0, store_.counter("newcounter0").value());
-  EXPECT_EQ(0, store_.counter("newcounter0").latch());
-  EXPECT_EQ(1, store_.counter("newcounter1").value());
-  EXPECT_EQ(1, store_.counter("newcounter1").latch());
-  EXPECT_EQ(5, store_.gauge("newgauge").value());
-}
-
 TEST_F(StatMergerTest, basicDefaultAccumulationImport) {
   Protobuf::Map<std::string, uint64_t> gauges;
   gauges["whywassixafraidofseven"] = 111;
@@ -164,6 +154,42 @@ TEST_F(StatMergerTest, exclusionsNotImported) {
   EXPECT_FALSE(store_.gauge("overload.something.pressure").used());
 }
 
+// When the parent sends us counters we haven't ourselves instantiated, they should be stored
+// temporarily, but then uninstantiated if hot restart ends without the child accessing them.
+TEST(StatMergerNonFixtureTest, newStatFromParent) {
+  FakeSymbolTableImpl symbol_table;
+  HeapStatDataAllocator alloc(symbol_table);
+  ThreadLocalStoreImpl store(alloc);
+  {
+    StatMerger stat_merger(store);
+
+    Protobuf::Map<std::string, uint64_t> counter_values;
+    Protobuf::Map<std::string, uint64_t> counter_deltas;
+    Protobuf::Map<std::string, uint64_t> gauges;
+    counter_deltas["newcounter0"] = 0;
+    counter_deltas["newcounter1"] = 1;
+    counter_deltas["newcounter2"] = 2;
+    gauges["newgauge"] = 5;
+    stat_merger.mergeStats(counter_deltas, gauges);
+    EXPECT_EQ(0, store.counter("newcounter0").value());
+    EXPECT_EQ(0, store.counter("newcounter0").latch());
+    EXPECT_EQ(1, store.counter("newcounter1").value());
+    EXPECT_EQ(1, store.counter("newcounter1").latch());
+    EXPECT_EQ(5, store.gauge("newgauge").value());
+  }
+  // We accessed 0 and 1 above, but not 2. Now that StatMerger has been destroyed,
+  // 2 should be gone.
+  EXPECT_TRUE(store.counterExists("newcounter0"));
+  EXPECT_TRUE(store.counterExists("newcounter1"));
+  EXPECT_FALSE(store.counterExists("newcounter2"));
+  // HACK: doesn't like shutting down without threading having started.
+  NiceMock<Event::MockDispatcher> main_thread_dispatcher;
+  NiceMock<ThreadLocal::MockInstance> tls;
+  store.initializeThreading(main_thread_dispatcher, tls);
+  store.shutdownThreading();
+  tls.shutdownThread();
+}
+
 } // namespace
 } // namespace Stats
 } // namespace Envoy
diff --git a/test/mocks/stats/mocks.h b/test/mocks/stats/mocks.h
index 48c76e01424aa..4167a9d6d6405 100644
--- a/test/mocks/stats/mocks.h
+++ b/test/mocks/stats/mocks.h
@@ -188,6 +188,7 @@ class MockStore : public SymbolTableProvider, public StoreImpl {
   MOCK_CONST_METHOD0(gauges, std::vector<GaugeSharedPtr>());
   MOCK_METHOD1(histogram, Histogram&(const std::string& name));
   MOCK_CONST_METHOD0(histograms, std::vector<ParentHistogramSharedPtr>());
+  MOCK_METHOD1(counterExists, bool(const std::string& counter_name));
 
   Counter& counterFromStatName(StatName name) override {
     return counter(symbol_table_->toString(name));

From 0dae84fe1da367ae101ed7ef98778cecacbca944 Mon Sep 17 00:00:00 2001
From: Fred Douglas <fredlas@google.com>
Date: Tue, 7 May 2019 18:26:24 -0400
Subject: [PATCH 03/12] added statricia and statrick to spellcheck, although
 statrick somehow passed anyways

Signed-off-by: Fred Douglas <fredlas@google.com>
---
 tools/spelling_dictionary.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/spelling_dictionary.txt b/tools/spelling_dictionary.txt
index 3a9f385f42355..9686a8c09b19a 100644
--- a/tools/spelling_dictionary.txt
+++ b/tools/spelling_dictionary.txt
@@ -236,6 +236,8 @@ SRDS
 SRV
 SS
 SSL
+STATRICIA
+STATRICK
 STL
 SVG
 TBD

From 9f862f86ebaafa7a8742d95b751f0930f5518ac8 Mon Sep 17 00:00:00 2001
From: Fred Douglas <fredlas@google.com>
Date: Tue, 7 May 2019 18:42:27 -0400
Subject: [PATCH 04/12] fill in missing counterExists

Signed-off-by: Fred Douglas <fredlas@google.com>
---
 test/integration/server.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/test/integration/server.h b/test/integration/server.h
index 096cc2dfac0a5..6b6b26767c5d2 100644
--- a/test/integration/server.h
+++ b/test/integration/server.h
@@ -170,6 +170,10 @@ class TestIsolatedStoreImpl : public StoreRoot {
     return store_.histograms();
   }
 
+  bool counterExists(const std::string& counter_name) override {
+    return store_.counterExists(counter_name);
+  }
+
   // Stats::StoreRoot
   void addSink(Sink&) override {}
   void setTagProducer(TagProducerPtr&&) override {}

From 05d456f4c0065238d853168c3637571abd4eb64f Mon Sep 17 00:00:00 2001
From: Fred Douglas <fredlas@google.com>
Date: Wed, 8 May 2019 12:59:14 -0400
Subject: [PATCH 05/12] snapshot

Signed-off-by: Fred Douglas <fredlas@google.com>
---
 include/envoy/stats/store.h               |  5 +++++
 source/common/stats/isolated_store_impl.h |  4 ++++
 source/common/stats/stat_merger.cc        |  7 +++----
 source/common/stats/stat_merger.h         | 21 ++++++++++-----------
 source/common/stats/thread_local_store.cc | 13 ++++++++++++-
 source/common/stats/thread_local_store.h  |  1 +
 test/common/stats/stat_merger_test.cc     |  8 ++++++--
 test/mocks/stats/mocks.h                  |  1 +
 tools/spelling_dictionary.txt             |  2 --
 9 files changed, 42 insertions(+), 20 deletions(-)

diff --git a/include/envoy/stats/store.h b/include/envoy/stats/store.h
index 8f48202146ada..d982cce41c70a 100644
--- a/include/envoy/stats/store.h
+++ b/include/envoy/stats/store.h
@@ -48,6 +48,11 @@ class Store : public Scope {
    * @return whether any known counter exists with this name.
    */
   virtual bool counterExists(const std::string& counter_name) PURE;
+
+  /**
+   * @return whether any known gauge exists with this name.
+   */
+  virtual bool gaugeExists(const std::string& gauge_name) PURE;
 };
 
 typedef std::unique_ptr<Store> StorePtr;
diff --git a/source/common/stats/isolated_store_impl.h b/source/common/stats/isolated_store_impl.h
index 656b54da03570..e923b6e09d359 100644
--- a/source/common/stats/isolated_store_impl.h
+++ b/source/common/stats/isolated_store_impl.h
@@ -83,6 +83,10 @@ class IsolatedStoreImpl : public StoreImpl {
     StatNameManagedStorage storage(counter_name, symbolTable());
     return counters_.statExists(storage.statName());
   }
+  bool gaugeExists(const std::string& gauge_name) override {
+    StatNameManagedStorage storage(gauge_name, symbolTable());
+    return gauges_.statExists(storage.statName());
+  }
 
   Counter& counter(const std::string& name) override {
     StatNameManagedStorage storage(name, symbolTable());
diff --git a/source/common/stats/stat_merger.cc b/source/common/stats/stat_merger.cc
index 159ca8db37149..b111fed68d0ba 100644
--- a/source/common/stats/stat_merger.cc
+++ b/source/common/stats/stat_merger.cc
@@ -5,8 +5,7 @@
 namespace Envoy {
 namespace Stats {
 
-StatMerger::StatMerger(Stats::Store& target_store)
-    : target_store_(target_store), temp_counter_scope_(target_store_.createScope("")) {}
+StatMerger::StatMerger(Stats::Store& target_store) : temp_scope_(target_store.createScope("")) {}
 
 bool StatMerger::shouldImport(Gauge& gauge, const std::string& gauge_name) {
   absl::optional<bool> should_import = gauge.cachedShouldImport();
@@ -57,13 +56,13 @@ bool StatMerger::shouldImport(Gauge& gauge, const std::string& gauge_name) {
 
 void StatMerger::mergeCounters(const Protobuf::Map<std::string, uint64_t>& counter_deltas) {
   for (const auto& counter : counter_deltas) {
-    temp_counter_scope_->counter(counter.first).add(counter.second);
+    temp_scope_->counter(counter.first).add(counter.second);
   }
 }
 
 void StatMerger::mergeGauges(const Protobuf::Map<std::string, uint64_t>& gauges) {
   for (const auto& gauge : gauges) {
-    auto& gauge_ref = target_store_.gauge(gauge.first);
+    auto& gauge_ref = temp_scope_->gauge(gauge.first);
     uint64_t& parent_value_ref = parent_gauge_values_[gauge_ref.statName()];
     uint64_t old_parent_value = parent_value_ref;
     uint64_t new_parent_value = gauge.second;
diff --git a/source/common/stats/stat_merger.h b/source/common/stats/stat_merger.h
index a082857004355..3558e312fd5ac 100644
--- a/source/common/stats/stat_merger.h
+++ b/source/common/stats/stat_merger.h
@@ -36,26 +36,25 @@ class StatMerger {
   void mergeCounters(const Protobuf::Map<std::string, uint64_t>& counter_deltas);
   void mergeGauges(const Protobuf::Map<std::string, uint64_t>& gauges);
   StatNameHashMap<uint64_t> parent_gauge_values_;
-  Store& target_store_;
   // A stats Scope for our in-the-merging-process counters to live in. Scopes conceptually hold
   // shared_ptrs to the stats that live in them, with the question of which stats are living in a
   // given scope determined by which stat names have been accessed via that scope. E.g., if you
-  // access a stat named "some.statricia" directly through the ordinary store, and then access a
-  // stat named "statricia" in a scope configured with the prefix "some.", there is now a single
-  // stat named some.statricia pointed to by both. As another example, if you access the stat
-  // "statrick" in the "some" scope, there will be a stat named "some.statrick" pointed to by just
-  // that scope. Now, if you delete the scope, some.statricia will stick around, but some.statrick
+  // access a stat named "some.shared" directly through the ordinary store, and then access a
+  // stat named "shared" in a scope configured with the prefix "some.", there is now a single
+  // stat named some.shared pointed to by both. As another example, if you access the stat
+  // "single" in the "some" scope, there will be a stat named "some.single" pointed to by just
+  // that scope. Now, if you delete the scope, some.shared will stick around, but some.single
   // will be destroyed.
   //
-  // All of that is relevant here because it is used to get a certain desired behavior for counters.
-  // Specifically, counters must be kept up to date with values from the parent throughout hot
+  // All of that is relevant here because it is used to get a certain desired behavior.
+  // Specifically, stats must be kept up to date with values from the parent throughout hot
   // restart, but once the restart completes, they must be dropped without a trace if the child has
   // not taken action (independent of the hot restart stat merging) that would lead to them getting
-  // created in the store. By storing these counters in a scope (with an empty prefix), we can
-  // preserve all counters throughout the hot restart. Then, when the restart completes, dropping
+  // created in the store. By storing these stats in a scope (with an empty prefix), we can
+  // preserve all stats throughout the hot restart. Then, when the restart completes, dropping
   // the scope will drop exactly those stats whose names have not already been accessed through
   // another store/scope.
-  ScopePtr temp_counter_scope_;
+  ScopePtr temp_scope_;
 };
 
 } // namespace Stats
diff --git a/source/common/stats/thread_local_store.cc b/source/common/stats/thread_local_store.cc
index 7ffea433c53bc..ec49378741003 100644
--- a/source/common/stats/thread_local_store.cc
+++ b/source/common/stats/thread_local_store.cc
@@ -149,7 +149,6 @@ std::vector<ParentHistogramSharedPtr> ThreadLocalStoreImpl::histograms() const {
 bool ThreadLocalStoreImpl::counterExists(const std::string& counter_name) {
   Thread::LockGuard lock(lock_);
   for (ScopeImpl* scope : scopes_) {
-
     if (scope->central_cache_.counters_.find(
             StatNameManagedStorage(counter_name, symbolTable()).statName()) !=
         scope->central_cache_.counters_.end()) {
@@ -159,6 +158,18 @@ bool ThreadLocalStoreImpl::counterExists(const std::string& counter_name) {
   return false;
 }
 
+bool ThreadLocalStoreImpl::gaugeExists(const std::string& gauge_name) {
+  Thread::LockGuard lock(lock_);
+  for (ScopeImpl* scope : scopes_) {
+    if (scope->central_cache_.gauges_.find(
+            StatNameManagedStorage(gauge_name, symbolTable()).statName()) !=
+        scope->central_cache_.gauges_.end()) {
+      return true;
+    }
+  }
+  return false;
+}
+
 void ThreadLocalStoreImpl::initializeThreading(Event::Dispatcher& main_thread_dispatcher,
                                                ThreadLocal::Instance& tls) {
   main_thread_dispatcher_ = &main_thread_dispatcher;
diff --git a/source/common/stats/thread_local_store.h b/source/common/stats/thread_local_store.h
index c31cdc8a70f7f..8453c6eb59597 100644
--- a/source/common/stats/thread_local_store.h
+++ b/source/common/stats/thread_local_store.h
@@ -169,6 +169,7 @@ class ThreadLocalStoreImpl : Logger::Loggable<Logger::Id::stats>, public StoreRo
   std::vector<GaugeSharedPtr> gauges() const override;
   std::vector<ParentHistogramSharedPtr> histograms() const override;
   bool counterExists(const std::string& counter_name) override;
+  bool gaugeExists(const std::string& gauge_name) override;
 
   // Stats::StoreRoot
   void addSink(Sink& sink) override { timer_sinks_.push_back(sink); }
diff --git a/test/common/stats/stat_merger_test.cc b/test/common/stats/stat_merger_test.cc
index 3af3c3d959d35..11919f6ca2cf7 100644
--- a/test/common/stats/stat_merger_test.cc
+++ b/test/common/stats/stat_merger_test.cc
@@ -169,19 +169,23 @@ TEST(StatMergerNonFixtureTest, newStatFromParent) {
     counter_deltas["newcounter0"] = 0;
     counter_deltas["newcounter1"] = 1;
     counter_deltas["newcounter2"] = 2;
-    gauges["newgauge"] = 5;
+    gauges["newgauge1"] = 1;
+    gauges["newgauge2"] = 2;
     stat_merger.mergeStats(counter_deltas, gauges);
     EXPECT_EQ(0, store.counter("newcounter0").value());
     EXPECT_EQ(0, store.counter("newcounter0").latch());
     EXPECT_EQ(1, store.counter("newcounter1").value());
     EXPECT_EQ(1, store.counter("newcounter1").latch());
-    EXPECT_EQ(5, store.gauge("newgauge").value());
+    EXPECT_EQ(1, store.gauge("newgauge1").value());
   }
   // We accessed 0 and 1 above, but not 2. Now that StatMerger has been destroyed,
   // 2 should be gone.
+  // TODO(fredlas) replace counterExists with Scope::getCounter once #6712 is merged.
   EXPECT_TRUE(store.counterExists("newcounter0"));
   EXPECT_TRUE(store.counterExists("newcounter1"));
   EXPECT_FALSE(store.counterExists("newcounter2"));
+  EXPECT_TRUE(store.gaugeExists("newgauge1"));
+  EXPECT_FALSE(store.gaugeExists("newgauge2"));
   // HACK: doesn't like shutting down without threading having started.
   NiceMock<Event::MockDispatcher> main_thread_dispatcher;
   NiceMock<ThreadLocal::MockInstance> tls;
diff --git a/test/mocks/stats/mocks.h b/test/mocks/stats/mocks.h
index 4167a9d6d6405..7eef86938004f 100644
--- a/test/mocks/stats/mocks.h
+++ b/test/mocks/stats/mocks.h
@@ -189,6 +189,7 @@ class MockStore : public SymbolTableProvider, public StoreImpl {
   MOCK_METHOD1(histogram, Histogram&(const std::string& name));
   MOCK_CONST_METHOD0(histograms, std::vector<ParentHistogramSharedPtr>());
   MOCK_METHOD1(counterExists, bool(const std::string& counter_name));
+  MOCK_METHOD1(gaugeExists, bool(const std::string& gauge_name));
 
   Counter& counterFromStatName(StatName name) override {
     return counter(symbol_table_->toString(name));
diff --git a/tools/spelling_dictionary.txt b/tools/spelling_dictionary.txt
index 9686a8c09b19a..3a9f385f42355 100644
--- a/tools/spelling_dictionary.txt
+++ b/tools/spelling_dictionary.txt
@@ -236,8 +236,6 @@ SRDS
 SRV
 SS
 SSL
-STATRICIA
-STATRICK
 STL
 SVG
 TBD

From c3c888ac23871640659fdcd90fa38279d6829946 Mon Sep 17 00:00:00 2001
From: Fred Douglas <fredlas@google.com>
Date: Wed, 8 May 2019 13:01:35 -0400
Subject: [PATCH 06/12] more TODOs

Signed-off-by: Fred Douglas <fredlas@google.com>
---
 ci/prebuilt/thirdparty       | 1 +
 ci/prebuilt/thirdparty_build | 1 +
 include/envoy/stats/store.h  | 2 ++
 3 files changed, 4 insertions(+)
 create mode 120000 ci/prebuilt/thirdparty
 create mode 120000 ci/prebuilt/thirdparty_build

diff --git a/ci/prebuilt/thirdparty b/ci/prebuilt/thirdparty
new file mode 120000
index 0000000000000..45c1c64a5b97a
--- /dev/null
+++ b/ci/prebuilt/thirdparty
@@ -0,0 +1 @@
+/thirdparty
\ No newline at end of file
diff --git a/ci/prebuilt/thirdparty_build b/ci/prebuilt/thirdparty_build
new file mode 120000
index 0000000000000..715acb9bad2d8
--- /dev/null
+++ b/ci/prebuilt/thirdparty_build
@@ -0,0 +1 @@
+/thirdparty_build
\ No newline at end of file
diff --git a/include/envoy/stats/store.h b/include/envoy/stats/store.h
index d982cce41c70a..287c51df0bdd1 100644
--- a/include/envoy/stats/store.h
+++ b/include/envoy/stats/store.h
@@ -44,11 +44,13 @@ class Store : public Scope {
    */
   virtual std::vector<ParentHistogramSharedPtr> histograms() const PURE;
 
+  // TODO(fredlas) can be removed once #6712 is merged.
   /**
    * @return whether any known counter exists with this name.
    */
   virtual bool counterExists(const std::string& counter_name) PURE;
 
+  // TODO(fredlas) can be removed once #6712 is merged.
   /**
    * @return whether any known gauge exists with this name.
    */

From 7b377c781cc5ff42233c7b540152c5b071342508 Mon Sep 17 00:00:00 2001
From: Fred Douglas <fredlas@google.com>
Date: Wed, 8 May 2019 15:33:13 -0400
Subject: [PATCH 07/12] add gaugeExsits to test store thing

Signed-off-by: Fred Douglas <fredlas@google.com>
---
 test/integration/server.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/test/integration/server.h b/test/integration/server.h
index 6b6b26767c5d2..d0cf34783a464 100644
--- a/test/integration/server.h
+++ b/test/integration/server.h
@@ -173,6 +173,9 @@ class TestIsolatedStoreImpl : public StoreRoot {
   bool counterExists(const std::string& counter_name) override {
     return store_.counterExists(counter_name);
   }
+  bool gaugeExists(const std::string& gauge_name) override {
+    return store_.gaugeExists(gauge_name);
+  }
 
   // Stats::StoreRoot
   void addSink(Sink&) override {}

From 2d572f770943b06638c8b60e5bc3eeba2f84b627 Mon Sep 17 00:00:00 2001
From: Fred Douglas <fredlas@google.com>
Date: Wed, 8 May 2019 15:35:20 -0400
Subject: [PATCH 08/12] remove weird autogen files that snuck in

Signed-off-by: Fred Douglas <fredlas@google.com>
---
 ci/prebuilt/thirdparty       | 1 -
 ci/prebuilt/thirdparty_build | 1 -
 2 files changed, 2 deletions(-)
 delete mode 120000 ci/prebuilt/thirdparty
 delete mode 120000 ci/prebuilt/thirdparty_build

diff --git a/ci/prebuilt/thirdparty b/ci/prebuilt/thirdparty
deleted file mode 120000
index 45c1c64a5b97a..0000000000000
--- a/ci/prebuilt/thirdparty
+++ /dev/null
@@ -1 +0,0 @@
-/thirdparty
\ No newline at end of file
diff --git a/ci/prebuilt/thirdparty_build b/ci/prebuilt/thirdparty_build
deleted file mode 120000
index 715acb9bad2d8..0000000000000
--- a/ci/prebuilt/thirdparty_build
+++ /dev/null
@@ -1 +0,0 @@
-/thirdparty_build
\ No newline at end of file

From 21082d2d02e5a6674f24c04c9639c140f6ed3b2e Mon Sep 17 00:00:00 2001
From: Fred Douglas <fredlas@google.com>
Date: Wed, 8 May 2019 16:27:04 -0400
Subject: [PATCH 09/12] work around tls thread shutdown

Signed-off-by: Fred Douglas <fredlas@google.com>
---
 test/common/stats/stat_merger_test.cc | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/test/common/stats/stat_merger_test.cc b/test/common/stats/stat_merger_test.cc
index 11919f6ca2cf7..ce494d25a15b3 100644
--- a/test/common/stats/stat_merger_test.cc
+++ b/test/common/stats/stat_merger_test.cc
@@ -186,12 +186,8 @@ TEST(StatMergerNonFixtureTest, newStatFromParent) {
   EXPECT_FALSE(store.counterExists("newcounter2"));
   EXPECT_TRUE(store.gaugeExists("newgauge1"));
   EXPECT_FALSE(store.gaugeExists("newgauge2"));
-  // HACK: doesn't like shutting down without threading having started.
-  NiceMock<Event::MockDispatcher> main_thread_dispatcher;
-  NiceMock<ThreadLocal::MockInstance> tls;
-  store.initializeThreading(main_thread_dispatcher, tls);
+
   store.shutdownThreading();
-  tls.shutdownThread();
 }
 
 } // namespace

From 95b22ede458b1dae07f886de4995302e417a340c Mon Sep 17 00:00:00 2001
From: Fred Douglas <fredlas@google.com>
Date: Wed, 8 May 2019 16:51:09 -0400
Subject: [PATCH 10/12] fix TLS never started up threading

Signed-off-by: Fred Douglas <fredlas@google.com>
---
 source/common/stats/thread_local_store.cc    | 3 ++-
 source/common/stats/thread_local_store.h     | 1 +
 test/common/stats/stat_merger_test.cc        | 2 --
 test/common/stats/thread_local_store_test.cc | 7 -------
 4 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/source/common/stats/thread_local_store.cc b/source/common/stats/thread_local_store.cc
index ec49378741003..9f2c2f9030355 100644
--- a/source/common/stats/thread_local_store.cc
+++ b/source/common/stats/thread_local_store.cc
@@ -29,7 +29,7 @@ ThreadLocalStoreImpl::ThreadLocalStoreImpl(StatDataAllocator& alloc)
       null_histogram_(alloc.symbolTable()) {}
 
 ThreadLocalStoreImpl::~ThreadLocalStoreImpl() {
-  ASSERT(shutting_down_);
+  ASSERT(shutting_down_ || !threading_ever_initialized_);
   default_scope_.reset();
   ASSERT(scopes_.empty());
   for (StatNameStorageSet* rejected_stats : rejected_stats_purgatory_) {
@@ -172,6 +172,7 @@ bool ThreadLocalStoreImpl::gaugeExists(const std::string& gauge_name) {
 
 void ThreadLocalStoreImpl::initializeThreading(Event::Dispatcher& main_thread_dispatcher,
                                                ThreadLocal::Instance& tls) {
+  threading_ever_initialized_ = true;
   main_thread_dispatcher_ = &main_thread_dispatcher;
   tls_ = tls.allocateSlot();
   tls_->set([](Event::Dispatcher&) -> ThreadLocal::ThreadLocalObjectSharedPtr {
diff --git a/source/common/stats/thread_local_store.h b/source/common/stats/thread_local_store.h
index 8453c6eb59597..e3eacf96e5938 100644
--- a/source/common/stats/thread_local_store.h
+++ b/source/common/stats/thread_local_store.h
@@ -307,6 +307,7 @@ class ThreadLocalStoreImpl : Logger::Loggable<Logger::Id::stats>, public StoreRo
   std::list<std::reference_wrapper<Sink>> timer_sinks_;
   TagProducerPtr tag_producer_;
   StatsMatcherPtr stats_matcher_;
+  std::atomic<bool> threading_ever_initialized_{};
   std::atomic<bool> shutting_down_{};
   std::atomic<bool> merge_in_progress_{};
   HeapStatDataAllocator heap_allocator_;
diff --git a/test/common/stats/stat_merger_test.cc b/test/common/stats/stat_merger_test.cc
index ce494d25a15b3..9a8119b72a65b 100644
--- a/test/common/stats/stat_merger_test.cc
+++ b/test/common/stats/stat_merger_test.cc
@@ -186,8 +186,6 @@ TEST(StatMergerNonFixtureTest, newStatFromParent) {
   EXPECT_FALSE(store.counterExists("newcounter2"));
   EXPECT_TRUE(store.gaugeExists("newgauge1"));
   EXPECT_FALSE(store.gaugeExists("newgauge2"));
-
-  store.shutdownThreading();
 }
 
 } // namespace
diff --git a/test/common/stats/thread_local_store_test.cc b/test/common/stats/thread_local_store_test.cc
index f41c2c7f5d446..e00ac79745cb8 100644
--- a/test/common/stats/thread_local_store_test.cc
+++ b/test/common/stats/thread_local_store_test.cc
@@ -788,13 +788,6 @@ TEST(StatsThreadLocalStoreTestNoFixture, MemoryWithoutTls) {
   EXPECT_LT(start_mem, end_mem);
   const size_t million = 1000 * 1000;
   EXPECT_LT(end_mem - start_mem, 20 * million); // actual value: 19601552 as of March 14, 2019
-
-  // HACK: doesn't like shutting down without threading having started.
-  NiceMock<Event::MockDispatcher> main_thread_dispatcher;
-  NiceMock<ThreadLocal::MockInstance> tls;
-  store->initializeThreading(main_thread_dispatcher, tls);
-  store->shutdownThreading();
-  tls.shutdownThread();
 }
 
 TEST(StatsThreadLocalStoreTestNoFixture, MemoryWithTls) {

From a935618d58738dfcc3e18beb44b3f36f732ba578 Mon Sep 17 00:00:00 2001
From: Fred Douglas <fredlas@google.com>
Date: Thu, 9 May 2019 13:57:12 -0400
Subject: [PATCH 11/12] switch to TestUtility::findCounter

Signed-off-by: Fred Douglas <fredlas@google.com>
---
 include/envoy/stats/store.h               | 12 ------------
 source/common/stats/isolated_store_impl.h | 11 -----------
 source/common/stats/thread_local_store.cc | 24 -----------------------
 source/common/stats/thread_local_store.h  |  2 --
 test/common/stats/stat_merger_test.cc     | 11 +++++------
 test/integration/server.h                 |  7 -------
 test/mocks/stats/mocks.h                  |  2 --
 7 files changed, 5 insertions(+), 64 deletions(-)

diff --git a/include/envoy/stats/store.h b/include/envoy/stats/store.h
index 287c51df0bdd1..cd017f9ad8843 100644
--- a/include/envoy/stats/store.h
+++ b/include/envoy/stats/store.h
@@ -43,18 +43,6 @@ class Store : public Scope {
    * @return a list of all known histograms.
    */
   virtual std::vector<ParentHistogramSharedPtr> histograms() const PURE;
-
-  // TODO(fredlas) can be removed once #6712 is merged.
-  /**
-   * @return whether any known counter exists with this name.
-   */
-  virtual bool counterExists(const std::string& counter_name) PURE;
-
-  // TODO(fredlas) can be removed once #6712 is merged.
-  /**
-   * @return whether any known gauge exists with this name.
-   */
-  virtual bool gaugeExists(const std::string& gauge_name) PURE;
 };
 
 typedef std::unique_ptr<Store> StorePtr;
diff --git a/source/common/stats/isolated_store_impl.h b/source/common/stats/isolated_store_impl.h
index e923b6e09d359..8e720a6ad8e3e 100644
--- a/source/common/stats/isolated_store_impl.h
+++ b/source/common/stats/isolated_store_impl.h
@@ -49,8 +49,6 @@ template <class Base> class IsolatedStatsCache {
     return vec;
   }
 
-  bool statExists(StatName name) { return stats_.find(name) != stats_.end(); }
-
 private:
   StatNameHashMap<std::shared_ptr<Base>> stats_;
   Allocator alloc_;
@@ -79,15 +77,6 @@ class IsolatedStoreImpl : public StoreImpl {
     return std::vector<ParentHistogramSharedPtr>{};
   }
 
-  bool counterExists(const std::string& counter_name) override {
-    StatNameManagedStorage storage(counter_name, symbolTable());
-    return counters_.statExists(storage.statName());
-  }
-  bool gaugeExists(const std::string& gauge_name) override {
-    StatNameManagedStorage storage(gauge_name, symbolTable());
-    return gauges_.statExists(storage.statName());
-  }
-
   Counter& counter(const std::string& name) override {
     StatNameManagedStorage storage(name, symbolTable());
     return counterFromStatName(storage.statName());
diff --git a/source/common/stats/thread_local_store.cc b/source/common/stats/thread_local_store.cc
index 9f2c2f9030355..03d799509c756 100644
--- a/source/common/stats/thread_local_store.cc
+++ b/source/common/stats/thread_local_store.cc
@@ -146,30 +146,6 @@ std::vector<ParentHistogramSharedPtr> ThreadLocalStoreImpl::histograms() const {
   return ret;
 }
 
-bool ThreadLocalStoreImpl::counterExists(const std::string& counter_name) {
-  Thread::LockGuard lock(lock_);
-  for (ScopeImpl* scope : scopes_) {
-    if (scope->central_cache_.counters_.find(
-            StatNameManagedStorage(counter_name, symbolTable()).statName()) !=
-        scope->central_cache_.counters_.end()) {
-      return true;
-    }
-  }
-  return false;
-}
-
-bool ThreadLocalStoreImpl::gaugeExists(const std::string& gauge_name) {
-  Thread::LockGuard lock(lock_);
-  for (ScopeImpl* scope : scopes_) {
-    if (scope->central_cache_.gauges_.find(
-            StatNameManagedStorage(gauge_name, symbolTable()).statName()) !=
-        scope->central_cache_.gauges_.end()) {
-      return true;
-    }
-  }
-  return false;
-}
-
 void ThreadLocalStoreImpl::initializeThreading(Event::Dispatcher& main_thread_dispatcher,
                                                ThreadLocal::Instance& tls) {
   threading_ever_initialized_ = true;
diff --git a/source/common/stats/thread_local_store.h b/source/common/stats/thread_local_store.h
index e3eacf96e5938..4e318bb949fb0 100644
--- a/source/common/stats/thread_local_store.h
+++ b/source/common/stats/thread_local_store.h
@@ -168,8 +168,6 @@ class ThreadLocalStoreImpl : Logger::Loggable<Logger::Id::stats>, public StoreRo
   std::vector<CounterSharedPtr> counters() const override;
   std::vector<GaugeSharedPtr> gauges() const override;
   std::vector<ParentHistogramSharedPtr> histograms() const override;
-  bool counterExists(const std::string& counter_name) override;
-  bool gaugeExists(const std::string& gauge_name) override;
 
   // Stats::StoreRoot
   void addSink(Sink& sink) override { timer_sinks_.push_back(sink); }
diff --git a/test/common/stats/stat_merger_test.cc b/test/common/stats/stat_merger_test.cc
index 9a8119b72a65b..58a8377145a89 100644
--- a/test/common/stats/stat_merger_test.cc
+++ b/test/common/stats/stat_merger_test.cc
@@ -180,12 +180,11 @@ TEST(StatMergerNonFixtureTest, newStatFromParent) {
   }
   // We accessed 0 and 1 above, but not 2. Now that StatMerger has been destroyed,
   // 2 should be gone.
-  // TODO(fredlas) replace counterExists with Scope::getCounter once #6712 is merged.
-  EXPECT_TRUE(store.counterExists("newcounter0"));
-  EXPECT_TRUE(store.counterExists("newcounter1"));
-  EXPECT_FALSE(store.counterExists("newcounter2"));
-  EXPECT_TRUE(store.gaugeExists("newgauge1"));
-  EXPECT_FALSE(store.gaugeExists("newgauge2"));
+  EXPECT_TRUE(TestUtility::findCounter(store, "newcounter0"));
+  EXPECT_TRUE(TestUtility::findCounter(store, "newcounter1"));
+  EXPECT_FALSE(TestUtility::findCounter(store, "newcounter2"));
+  EXPECT_TRUE(TestUtility::findGauge(store, "newgauge1"));
+  EXPECT_FALSE(TestUtility::findGauge(store, "newgauge2"));
 }
 
 } // namespace
diff --git a/test/integration/server.h b/test/integration/server.h
index d0cf34783a464..096cc2dfac0a5 100644
--- a/test/integration/server.h
+++ b/test/integration/server.h
@@ -170,13 +170,6 @@ class TestIsolatedStoreImpl : public StoreRoot {
     return store_.histograms();
   }
 
-  bool counterExists(const std::string& counter_name) override {
-    return store_.counterExists(counter_name);
-  }
-  bool gaugeExists(const std::string& gauge_name) override {
-    return store_.gaugeExists(gauge_name);
-  }
-
   // Stats::StoreRoot
   void addSink(Sink&) override {}
   void setTagProducer(TagProducerPtr&&) override {}
diff --git a/test/mocks/stats/mocks.h b/test/mocks/stats/mocks.h
index 7eef86938004f..48c76e01424aa 100644
--- a/test/mocks/stats/mocks.h
+++ b/test/mocks/stats/mocks.h
@@ -188,8 +188,6 @@ class MockStore : public SymbolTableProvider, public StoreImpl {
   MOCK_CONST_METHOD0(gauges, std::vector<GaugeSharedPtr>());
   MOCK_METHOD1(histogram, Histogram&(const std::string& name));
   MOCK_CONST_METHOD0(histograms, std::vector<ParentHistogramSharedPtr>());
-  MOCK_METHOD1(counterExists, bool(const std::string& counter_name));
-  MOCK_METHOD1(gaugeExists, bool(const std::string& gauge_name));
 
   Counter& counterFromStatName(StatName name) override {
     return counter(symbol_table_->toString(name));

From 1958feeaa1c97ba071e5fd63dea5cbc404794979 Mon Sep 17 00:00:00 2001
From: Fred Douglas <fredlas@google.com>
Date: Thu, 9 May 2019 14:03:13 -0400
Subject: [PATCH 12/12] dependency cleanup

Signed-off-by: Fred Douglas <fredlas@google.com>
---
 test/common/stats/BUILD               | 3 +--
 test/common/stats/stat_merger_test.cc | 5 +----
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/test/common/stats/BUILD b/test/common/stats/BUILD
index 971f1fe3a8213..c8a45d7100f61 100644
--- a/test/common/stats/BUILD
+++ b/test/common/stats/BUILD
@@ -55,8 +55,7 @@ envoy_cc_test(
         "//source/common/stats:isolated_store_lib",
         "//source/common/stats:stat_merger_lib",
         "//source/common/stats:thread_local_store_lib",
-        "//test/mocks/event:event_mocks",
-        "//test/mocks/thread_local:thread_local_mocks",
+        "//test/test_common:utility_lib",
     ],
 )
 
diff --git a/test/common/stats/stat_merger_test.cc b/test/common/stats/stat_merger_test.cc
index 58a8377145a89..0e02c0f910bba 100644
--- a/test/common/stats/stat_merger_test.cc
+++ b/test/common/stats/stat_merger_test.cc
@@ -5,13 +5,10 @@
 #include "common/stats/stat_merger.h"
 #include "common/stats/thread_local_store.h"
 
-#include "test/mocks/event/mocks.h"
-#include "test/mocks/thread_local/mocks.h"
+#include "test/test_common/utility.h"
 
 #include "gtest/gtest.h"
 
-using testing::NiceMock;
-
 namespace Envoy {
 namespace Stats {
 namespace {