diff --git a/envoy/server/hot_restart.h b/envoy/server/hot_restart.h index ca373335ffc45..c038f1f871950 100644 --- a/envoy/server/hot_restart.h +++ b/envoy/server/hot_restart.h @@ -5,7 +5,6 @@ #include "envoy/common/pure.h" #include "envoy/event/dispatcher.h" -#include "envoy/stats/allocator.h" #include "envoy/stats/store.h" #include "envoy/thread/thread.h" diff --git a/envoy/stats/allocator.h b/envoy/stats/allocator.h index fc45ca8af4f14..3c22bbd028d66 100644 --- a/envoy/stats/allocator.h +++ b/envoy/stats/allocator.h @@ -1,108 +1,15 @@ #pragma once -#include -#include -#include -#include -#include -#include -#include - -#include "envoy/common/pure.h" -#include "envoy/stats/stats.h" -#include "envoy/stats/tag.h" - -#include "absl/strings/string_view.h" +// This header is a placeholder which we will carry until June 1, 2026, +// as we have deprecated the pure interface and impl pattern. +// +// Please remove references to this file and instead include +// source/common/stats/allocator.h. namespace Envoy { namespace Stats { -class Sink; -class SinkPredicates; - -/** - * Abstract interface for allocating statistics. Implementations can - * 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. - */ -class Allocator { -public: - virtual ~Allocator() = default; - - /** - * @param name the full name of the stat. - * @param tag_extracted_name the name of the stat with tag-values stripped out. - * @param tags the tag values. - * @return CounterSharedPtr a counter. - */ - virtual CounterSharedPtr makeCounter(StatName name, StatName tag_extracted_name, - const StatNameTagVector& stat_name_tags) PURE; - - /** - * @param name the full name of the stat. - * @param tag_extracted_name the name of the stat with tag-values stripped out. - * @param stat_name_tags the tag values. - * @return GaugeSharedPtr a gauge. - */ - virtual GaugeSharedPtr makeGauge(StatName name, StatName tag_extracted_name, - const StatNameTagVector& stat_name_tags, - Gauge::ImportMode import_mode) PURE; - - /** - * @param name the full name of the stat. - * @param tag_extracted_name the name of the stat with tag-values stripped out. - * @param tags the tag values. - * @return TextReadoutSharedPtr a text readout. - */ - virtual TextReadoutSharedPtr makeTextReadout(StatName name, StatName tag_extracted_name, - const StatNameTagVector& stat_name_tags) PURE; - virtual const SymbolTable& constSymbolTable() const PURE; - virtual SymbolTable& symbolTable() PURE; - - /** - * Mark rejected stats as deleted by moving them to a different vector, so they don't show up - * when iterating over stats, but prevent crashes when trying to access references to them. - * Note that allocating a stat with the same name after calling this will - * return a new stat. Hence callers should seek to avoid this situation, as is - * done in ThreadLocalStore. - */ - virtual void markCounterForDeletion(const CounterSharedPtr& counter) PURE; - virtual void markGaugeForDeletion(const GaugeSharedPtr& gauge) PURE; - virtual void markTextReadoutForDeletion(const TextReadoutSharedPtr& text_readout) PURE; - - /** - * Iterate over all stats. Note, that implementations can potentially hold on to a mutex that - * will deadlock if the passed in functors try to create or delete a stat. - * @param f_size functor that is provided the current number of all stats. Note that this is - * called only once, prior to any calls to f_stat. - * @param f_stat functor that is provided one stat at a time from the stats container. - */ - virtual void forEachCounter(SizeFn f_size, StatFn f_stat) const PURE; - virtual void forEachGauge(SizeFn f_size, StatFn f_stat) const PURE; - virtual void forEachTextReadout(SizeFn f_size, StatFn f_stat) const PURE; - - /** - * Iterate over all stats that need to be flushed to sinks. Note, that implementations can - * potentially hold on to a mutex that will deadlock if the passed in functors try to create - * or delete a stat. - * @param f_size functor that is provided the number of all stats that will be flushed to sinks. - * Note that this is called only once, prior to any calls to f_stat. - * @param f_stat functor that is provided one stat that will be flushed to sinks, at a time. - */ - virtual void forEachSinkedCounter(SizeFn f_size, StatFn f_stat) const PURE; - virtual void forEachSinkedGauge(SizeFn f_size, StatFn f_stat) const PURE; - virtual void forEachSinkedTextReadout(SizeFn f_size, StatFn f_stat) const PURE; - - /** - * Set the predicates to filter stats for sink. - */ - virtual void setSinkPredicates(std::unique_ptr&& sink_predicates) PURE; - - // TODO(jmarantz): create a parallel mechanism to instantiate histograms. At - // the moment, histograms don't fit the same pattern of counters and gauges - // as they are not actually created in the context of a stats allocator. -}; +class Allocator; } // namespace Stats } // namespace Envoy diff --git a/source/common/stats/BUILD b/source/common/stats/BUILD index bdc583dac2da1..a6a40fb51108c 100644 --- a/source/common/stats/BUILD +++ b/source/common/stats/BUILD @@ -10,8 +10,11 @@ envoy_package() envoy_cc_library( name = "allocator_lib", - srcs = ["allocator_impl.cc"], - hdrs = ["allocator_impl.h"], + srcs = ["allocator.cc"], + hdrs = [ + "allocator.h", + "allocator_impl.h", + ], deps = [ ":metric_impl_lib", ":stat_merger_lib", diff --git a/source/common/stats/allocator_impl.cc b/source/common/stats/allocator.cc similarity index 87% rename from source/common/stats/allocator_impl.cc rename to source/common/stats/allocator.cc index ad9c04b0e8099..3c6bee834681b 100644 --- a/source/common/stats/allocator_impl.cc +++ b/source/common/stats/allocator.cc @@ -1,4 +1,4 @@ -#include "source/common/stats/allocator_impl.h" +#include "source/common/stats/allocator.h" #include #include @@ -17,13 +17,14 @@ #include "source/common/stats/symbol_table.h" #include "absl/container/flat_hash_set.h" +#include "absl/strings/string_view.h" namespace Envoy { namespace Stats { -const char AllocatorImpl::DecrementToZeroSyncPoint[] = "decrement-zero"; +const char Allocator::DecrementToZeroSyncPoint[] = "decrement-zero"; -AllocatorImpl::~AllocatorImpl() { +Allocator::~Allocator() { ASSERT(counters_.empty()); ASSERT(gauges_.empty()); @@ -48,7 +49,7 @@ AllocatorImpl::~AllocatorImpl() { } #ifndef ENVOY_CONFIG_COVERAGE -void AllocatorImpl::debugPrint() { +void Allocator::debugPrint() { Thread::LockGuard lock(mutex_); for (Counter* counter : counters_) { ENVOY_LOG_MISC(info, "counter: {}", symbolTable().toString(counter->statName())); @@ -69,7 +70,7 @@ void AllocatorImpl::debugPrint() { // shared_ptr. template class StatsSharedImpl : public MetricImpl { public: - StatsSharedImpl(StatName name, AllocatorImpl& alloc, StatName tag_extracted_name, + StatsSharedImpl(StatName name, Allocator& alloc, StatName tag_extracted_name, const StatNameTagVector& stat_name_tags) : MetricImpl(name, tag_extracted_name, stat_name_tags, alloc.symbolTable()), alloc_(alloc) {} @@ -105,7 +106,7 @@ template class StatsSharedImpl : public MetricImpl Thread::LockGuard lock(alloc_.mutex_); ASSERT(ref_count_ >= 1); if (--ref_count_ == 0) { - alloc_.sync().syncPoint(AllocatorImpl::DecrementToZeroSyncPoint); + alloc_.sync().syncPoint(Allocator::DecrementToZeroSyncPoint); removeFromSetLockHeld(); return true; } @@ -121,7 +122,7 @@ template class StatsSharedImpl : public MetricImpl virtual void removeFromSetLockHeld() ABSL_EXCLUSIVE_LOCKS_REQUIRED(alloc_.mutex_) PURE; protected: - AllocatorImpl& alloc_; + Allocator& alloc_; // ref_count_ can be incremented as an atomic, without taking a new lock, as // the critical 0->1 transition occurs in makeCounter and makeGauge, which @@ -139,7 +140,7 @@ template class StatsSharedImpl : public MetricImpl class CounterImpl : public StatsSharedImpl { public: - CounterImpl(StatName name, AllocatorImpl& alloc, StatName tag_extracted_name, + CounterImpl(StatName name, Allocator& alloc, StatName tag_extracted_name, const StatNameTagVector& stat_name_tags) : StatsSharedImpl(name, alloc, tag_extracted_name, stat_name_tags) {} @@ -169,7 +170,7 @@ class CounterImpl : public StatsSharedImpl { class GaugeImpl : public StatsSharedImpl { public: - GaugeImpl(StatName name, AllocatorImpl& alloc, StatName tag_extracted_name, + GaugeImpl(StatName name, Allocator& alloc, StatName tag_extracted_name, const StatNameTagVector& stat_name_tags, ImportMode import_mode) : StatsSharedImpl(name, alloc, tag_extracted_name, stat_name_tags) { switch (import_mode) { @@ -270,7 +271,7 @@ class GaugeImpl : public StatsSharedImpl { class TextReadoutImpl : public StatsSharedImpl { public: - TextReadoutImpl(StatName name, AllocatorImpl& alloc, StatName tag_extracted_name, + TextReadoutImpl(StatName name, Allocator& alloc, StatName tag_extracted_name, const StatNameTagVector& stat_name_tags) : StatsSharedImpl(name, alloc, tag_extracted_name, stat_name_tags) {} @@ -297,8 +298,8 @@ class TextReadoutImpl : public StatsSharedImpl { std::string value_ ABSL_GUARDED_BY(mutex_); }; -CounterSharedPtr AllocatorImpl::makeCounter(StatName name, StatName tag_extracted_name, - const StatNameTagVector& stat_name_tags) { +CounterSharedPtr Allocator::makeCounter(StatName name, StatName tag_extracted_name, + const StatNameTagVector& stat_name_tags) { Thread::LockGuard lock(mutex_); ASSERT(gauges_.find(name) == gauges_.end()); ASSERT(text_readouts_.find(name) == text_readouts_.end()); @@ -316,9 +317,9 @@ CounterSharedPtr AllocatorImpl::makeCounter(StatName name, StatName tag_extracte return counter; } -GaugeSharedPtr AllocatorImpl::makeGauge(StatName name, StatName tag_extracted_name, - const StatNameTagVector& stat_name_tags, - Gauge::ImportMode import_mode) { +GaugeSharedPtr Allocator::makeGauge(StatName name, StatName tag_extracted_name, + const StatNameTagVector& stat_name_tags, + Gauge::ImportMode import_mode) { Thread::LockGuard lock(mutex_); ASSERT(counters_.find(name) == counters_.end()); ASSERT(text_readouts_.find(name) == text_readouts_.end()); @@ -337,8 +338,8 @@ GaugeSharedPtr AllocatorImpl::makeGauge(StatName name, StatName tag_extracted_na return gauge; } -TextReadoutSharedPtr AllocatorImpl::makeTextReadout(StatName name, StatName tag_extracted_name, - const StatNameTagVector& stat_name_tags) { +TextReadoutSharedPtr Allocator::makeTextReadout(StatName name, StatName tag_extracted_name, + const StatNameTagVector& stat_name_tags) { Thread::LockGuard lock(mutex_); ASSERT(counters_.find(name) == counters_.end()); ASSERT(gauges_.find(name) == gauges_.end()); @@ -357,7 +358,7 @@ TextReadoutSharedPtr AllocatorImpl::makeTextReadout(StatName name, StatName tag_ return text_readout; } -bool AllocatorImpl::isMutexLockedForTest() { +bool Allocator::isMutexLockedForTest() { bool locked = mutex_.tryLock(); if (locked) { mutex_.unlock(); @@ -365,12 +366,12 @@ bool AllocatorImpl::isMutexLockedForTest() { return !locked; } -Counter* AllocatorImpl::makeCounterInternal(StatName name, StatName tag_extracted_name, - const StatNameTagVector& stat_name_tags) { +Counter* Allocator::makeCounterInternal(StatName name, StatName tag_extracted_name, + const StatNameTagVector& stat_name_tags) { return new CounterImpl(name, *this, tag_extracted_name, stat_name_tags); } -void AllocatorImpl::forEachCounter(SizeFn f_size, StatFn f_stat) const { +void Allocator::forEachCounter(SizeFn f_size, StatFn f_stat) const { Thread::LockGuard lock(mutex_); if (f_size != nullptr) { f_size(counters_.size()); @@ -380,7 +381,7 @@ void AllocatorImpl::forEachCounter(SizeFn f_size, StatFn f_stat) const } } -void AllocatorImpl::forEachGauge(SizeFn f_size, StatFn f_stat) const { +void Allocator::forEachGauge(SizeFn f_size, StatFn f_stat) const { Thread::LockGuard lock(mutex_); if (f_size != nullptr) { f_size(gauges_.size()); @@ -390,7 +391,7 @@ void AllocatorImpl::forEachGauge(SizeFn f_size, StatFn f_stat) const { } } -void AllocatorImpl::forEachTextReadout(SizeFn f_size, StatFn f_stat) const { +void Allocator::forEachTextReadout(SizeFn f_size, StatFn f_stat) const { Thread::LockGuard lock(mutex_); if (f_size != nullptr) { f_size(text_readouts_.size()); @@ -400,7 +401,7 @@ void AllocatorImpl::forEachTextReadout(SizeFn f_size, StatFn f_stat } } -void AllocatorImpl::forEachSinkedCounter(SizeFn f_size, StatFn f_stat) const { +void Allocator::forEachSinkedCounter(SizeFn f_size, StatFn f_stat) const { if (sink_predicates_ != nullptr) { Thread::LockGuard lock(mutex_); f_size(sinked_counters_.size()); @@ -412,7 +413,7 @@ void AllocatorImpl::forEachSinkedCounter(SizeFn f_size, StatFn f_stat) } } -void AllocatorImpl::forEachSinkedGauge(SizeFn f_size, StatFn f_stat) const { +void Allocator::forEachSinkedGauge(SizeFn f_size, StatFn f_stat) const { if (sink_predicates_ != nullptr) { Thread::LockGuard lock(mutex_); f_size(sinked_gauges_.size()); @@ -428,7 +429,7 @@ void AllocatorImpl::forEachSinkedGauge(SizeFn f_size, StatFn f_stat) cons } } -void AllocatorImpl::forEachSinkedTextReadout(SizeFn f_size, StatFn f_stat) const { +void Allocator::forEachSinkedTextReadout(SizeFn f_size, StatFn f_stat) const { if (sink_predicates_ != nullptr) { Thread::LockGuard lock(mutex_); f_size(sinked_text_readouts_.size()); @@ -440,7 +441,7 @@ void AllocatorImpl::forEachSinkedTextReadout(SizeFn f_size, StatFn } } -void AllocatorImpl::setSinkPredicates(std::unique_ptr&& sink_predicates) { +void Allocator::setSinkPredicates(std::unique_ptr&& sink_predicates) { Thread::LockGuard lock(mutex_); ASSERT(sink_predicates_ == nullptr); sink_predicates_ = std::move(sink_predicates); @@ -467,7 +468,7 @@ void AllocatorImpl::setSinkPredicates(std::unique_ptr&& sink_pre } } -void AllocatorImpl::markCounterForDeletion(const CounterSharedPtr& counter) { +void Allocator::markCounterForDeletion(const CounterSharedPtr& counter) { Thread::LockGuard lock(mutex_); auto iter = counters_.find(counter->statName()); if (iter == counters_.end()) { @@ -475,13 +476,14 @@ void AllocatorImpl::markCounterForDeletion(const CounterSharedPtr& counter) { return; } ASSERT(counter.get() == *iter); - // Duplicates are ASSERTed in ~AllocatorImpl. + // Duplicates are ASSERTed in ~Allocator. These might occur if there was + // a race bug in reference counting, causing a stat to be double-deleted. deleted_counters_.emplace_back(*iter); counters_.erase(iter); sinked_counters_.erase(counter.get()); } -void AllocatorImpl::markGaugeForDeletion(const GaugeSharedPtr& gauge) { +void Allocator::markGaugeForDeletion(const GaugeSharedPtr& gauge) { Thread::LockGuard lock(mutex_); auto iter = gauges_.find(gauge->statName()); if (iter == gauges_.end()) { @@ -489,13 +491,13 @@ void AllocatorImpl::markGaugeForDeletion(const GaugeSharedPtr& gauge) { return; } ASSERT(gauge.get() == *iter); - // Duplicates are ASSERTed in ~AllocatorImpl. + // Duplicates are ASSERTed in ~Allocator. deleted_gauges_.emplace_back(*iter); gauges_.erase(iter); sinked_gauges_.erase(gauge.get()); } -void AllocatorImpl::markTextReadoutForDeletion(const TextReadoutSharedPtr& text_readout) { +void Allocator::markTextReadoutForDeletion(const TextReadoutSharedPtr& text_readout) { Thread::LockGuard lock(mutex_); auto iter = text_readouts_.find(text_readout->statName()); if (iter == text_readouts_.end()) { @@ -503,7 +505,7 @@ void AllocatorImpl::markTextReadoutForDeletion(const TextReadoutSharedPtr& text_ return; } ASSERT(text_readout.get() == *iter); - // Duplicates are ASSERTed in ~AllocatorImpl. + // Duplicates are ASSERTed in ~Allocator. deleted_text_readouts_.emplace_back(*iter); text_readouts_.erase(iter); sinked_text_readouts_.erase(text_readout.get()); diff --git a/source/common/stats/allocator.h b/source/common/stats/allocator.h new file mode 100644 index 0000000000000..37aed8de6e1ae --- /dev/null +++ b/source/common/stats/allocator.h @@ -0,0 +1,154 @@ +#pragma once + +#include + +#include "envoy/stats/sink.h" +#include "envoy/stats/stats.h" + +#include "source/common/common/thread.h" +#include "source/common/common/thread_synchronizer.h" +#include "source/common/stats/metric_impl.h" + +#include "absl/container/flat_hash_set.h" + +namespace Envoy { +namespace Stats { + +/** + * Helper class for Store to manage memory for statistics. + */ +class Allocator { +public: + static const char DecrementToZeroSyncPoint[]; + + Allocator(SymbolTable& symbol_table) : symbol_table_(symbol_table) {} + virtual ~Allocator(); + + /** + * @param name the full name of the stat. + * @param tag_extracted_name the name of the stat with tag-values stripped out. + * @param tags the tag values. + * @return CounterSharedPtr a counter. + */ + CounterSharedPtr makeCounter(StatName name, StatName tag_extracted_name, + const StatNameTagVector& stat_name_tags); + + /** + * @param name the full name of the stat. + * @param tag_extracted_name the name of the stat with tag-values stripped out. + * @param stat_name_tags the tag values. + * @return GaugeSharedPtr a gauge. + */ + GaugeSharedPtr makeGauge(StatName name, StatName tag_extracted_name, + const StatNameTagVector& stat_name_tags, Gauge::ImportMode import_mode); + + /** + * @param name the full name of the stat. + * @param tag_extracted_name the name of the stat with tag-values stripped out. + * @param tags the tag values. + * @return TextReadoutSharedPtr a text readout. + */ + TextReadoutSharedPtr makeTextReadout(StatName name, StatName tag_extracted_name, + const StatNameTagVector& stat_name_tags); + SymbolTable& symbolTable() { return symbol_table_; } + const SymbolTable& constSymbolTable() const { return symbol_table_; } + + /** + * Iterate over all stats. Note, that implementations can potentially hold on to a mutex that + * will deadlock if the passed in functors try to create or delete a stat. + * @param f_size functor that is provided the current number of all stats. Note that this is + * called only once, prior to any calls to f_stat. + * @param f_stat functor that is provided one stat at a time from the stats container. + */ + void forEachCounter(SizeFn, StatFn) const; + void forEachGauge(SizeFn, StatFn) const; + void forEachTextReadout(SizeFn, StatFn) const; + + /** + * Iterate over all stats that need to be flushed to sinks. Note, that implementations can + * potentially hold on to a mutex that will deadlock if the passed in functors try to create + * or delete a stat. + * @param f_size functor that is provided the number of all stats that will be flushed to sinks. + * Note that this is called only once, prior to any calls to f_stat. + * @param f_stat functor that is provided one stat that will be flushed to sinks, at a time. + */ + void forEachSinkedCounter(SizeFn f_size, StatFn f_stat) const; + void forEachSinkedGauge(SizeFn f_size, StatFn f_stat) const; + void forEachSinkedTextReadout(SizeFn f_size, StatFn f_stat) const; + + /** + * Set the predicates to filter stats for sink. + */ + void setSinkPredicates(std::unique_ptr&& sink_predicates); +#ifndef ENVOY_CONFIG_COVERAGE + void debugPrint(); +#endif + + /** + * @return a thread synchronizer object used for reproducing a race-condition in tests. + */ + Thread::ThreadSynchronizer& sync() { return sync_; } + + /** + * @return whether the allocator's mutex is locked, exposed for testing purposes. + */ + bool isMutexLockedForTest(); + + /** + * Mark rejected stats as deleted by moving them to a different vector, so they don't show up + * when iterating over stats, but prevent crashes when trying to access references to them. + * Note that allocating a stat with the same name after calling this will + * return a new stat. Hence callers should seek to avoid this situation, as is + * done in ThreadLocalStore. + */ + void markCounterForDeletion(const CounterSharedPtr& counter); + void markGaugeForDeletion(const GaugeSharedPtr& gauge); + void markTextReadoutForDeletion(const TextReadoutSharedPtr& text_readout); + +protected: + virtual Counter* makeCounterInternal(StatName name, StatName tag_extracted_name, + const StatNameTagVector& stat_name_tags); + +private: + template friend class StatsSharedImpl; + friend class CounterImpl; + friend class GaugeImpl; + friend class TextReadoutImpl; + + // A mutex is needed here to protect both the stats_ object from both + // alloc() and free() operations. Although alloc() operations are called under existing locking, + // free() operations are made from the destructors of the individual stat objects, which are not + // protected by locks. + mutable Thread::MutexBasicLockable mutex_; + + StatSet counters_ ABSL_GUARDED_BY(mutex_); + StatSet gauges_ ABSL_GUARDED_BY(mutex_); + StatSet text_readouts_ ABSL_GUARDED_BY(mutex_); + + template using StatPointerSet = absl::flat_hash_set; + // Stat pointers that participate in the flush to sink process. + StatPointerSet sinked_counters_ ABSL_GUARDED_BY(mutex_); + StatPointerSet sinked_gauges_ ABSL_GUARDED_BY(mutex_); + StatPointerSet sinked_text_readouts_ ABSL_GUARDED_BY(mutex_); + + // Predicates used to filter stats to be flushed. + std::unique_ptr sink_predicates_; + SymbolTable& symbol_table_; + + Thread::ThreadSynchronizer sync_; + + // Retain storage for deleted stats; these are no longer in maps because + // the matcher-pattern was established after they were created. Since the + // stats are held by reference in code that expects them to be there, we + // can't actually delete the stats. + // + // It seems like it would be better to have each client that expects a stat + // to exist to hold it as (e.g.) a CounterSharedPtr rather than a Counter& + // but that would be fairly complex to change. + std::vector deleted_counters_ ABSL_GUARDED_BY(mutex_); + std::vector deleted_gauges_ ABSL_GUARDED_BY(mutex_); + std::vector deleted_text_readouts_ ABSL_GUARDED_BY(mutex_); +}; + +} // namespace Stats +} // namespace Envoy diff --git a/source/common/stats/allocator_impl.h b/source/common/stats/allocator_impl.h index fdd25433ec2b1..3f2c4e8aa8a1f 100644 --- a/source/common/stats/allocator_impl.h +++ b/source/common/stats/allocator_impl.h @@ -1,113 +1,19 @@ #pragma once -#include +// This header is a placeholder which we will carry until June 1, 2026, +// as we have deprecated the pure interface and impl pattern. +// +// Please remove references to this file and instead include +// source/common/stats/allocator.h directly. -#include "envoy/common/optref.h" -#include "envoy/stats/allocator.h" -#include "envoy/stats/sink.h" -#include "envoy/stats/stats.h" - -#include "source/common/common/thread_synchronizer.h" -#include "source/common/stats/metric_impl.h" - -#include "absl/container/flat_hash_set.h" -#include "absl/strings/string_view.h" +#include "source/common/stats/allocator.h" namespace Envoy { namespace Stats { -class AllocatorImpl : public Allocator { -public: - static const char DecrementToZeroSyncPoint[]; - - AllocatorImpl(SymbolTable& symbol_table) : symbol_table_(symbol_table) {} - ~AllocatorImpl() override; - - // Allocator - CounterSharedPtr makeCounter(StatName name, StatName tag_extracted_name, - const StatNameTagVector& stat_name_tags) override; - GaugeSharedPtr makeGauge(StatName name, StatName tag_extracted_name, - const StatNameTagVector& stat_name_tags, - Gauge::ImportMode import_mode) override; - TextReadoutSharedPtr makeTextReadout(StatName name, StatName tag_extracted_name, - const StatNameTagVector& stat_name_tags) override; - SymbolTable& symbolTable() override { return symbol_table_; } - const SymbolTable& constSymbolTable() const override { return symbol_table_; } - - void forEachCounter(SizeFn, StatFn) const override; - - void forEachGauge(SizeFn, StatFn) const override; - - void forEachTextReadout(SizeFn, StatFn) const override; - - void forEachSinkedCounter(SizeFn f_size, StatFn f_stat) const override; - void forEachSinkedGauge(SizeFn f_size, StatFn f_stat) const override; - void forEachSinkedTextReadout(SizeFn f_size, StatFn f_stat) const override; - - void setSinkPredicates(std::unique_ptr&& sink_predicates) override; -#ifndef ENVOY_CONFIG_COVERAGE - void debugPrint(); -#endif - - /** - * @return a thread synchronizer object used for reproducing a race-condition in tests. - */ - Thread::ThreadSynchronizer& sync() { return sync_; } - - /** - * @return whether the allocator's mutex is locked, exposed for testing purposes. - */ - bool isMutexLockedForTest(); - - void markCounterForDeletion(const CounterSharedPtr& counter) override; - void markGaugeForDeletion(const GaugeSharedPtr& gauge) override; - void markTextReadoutForDeletion(const TextReadoutSharedPtr& text_readout) override; - -protected: - virtual Counter* makeCounterInternal(StatName name, StatName tag_extracted_name, - const StatNameTagVector& stat_name_tags); - -private: - template friend class StatsSharedImpl; - friend class CounterImpl; - friend class GaugeImpl; - friend class TextReadoutImpl; - friend class NotifyingAllocatorImpl; - - // A mutex is needed here to protect both the stats_ object from both - // alloc() and free() operations. Although alloc() operations are called under existing locking, - // free() operations are made from the destructors of the individual stat objects, which are not - // protected by locks. - mutable Thread::MutexBasicLockable mutex_; - - StatSet counters_ ABSL_GUARDED_BY(mutex_); - StatSet gauges_ ABSL_GUARDED_BY(mutex_); - StatSet text_readouts_ ABSL_GUARDED_BY(mutex_); - - template using StatPointerSet = absl::flat_hash_set; - // Stat pointers that participate in the flush to sink process. - StatPointerSet sinked_counters_ ABSL_GUARDED_BY(mutex_); - StatPointerSet sinked_gauges_ ABSL_GUARDED_BY(mutex_); - StatPointerSet sinked_text_readouts_ ABSL_GUARDED_BY(mutex_); - - // Predicates used to filter stats to be flushed. - std::unique_ptr sink_predicates_; - SymbolTable& symbol_table_; - - Thread::ThreadSynchronizer sync_; - - // Retain storage for deleted stats; these are no longer in maps because - // the matcher-pattern was established after they were created. Since the - // stats are held by reference in code that expects them to be there, we - // can't actually delete the stats. - // - // It seems like it would be better to have each client that expects a stat - // to exist to hold it as (e.g.) a CounterSharedPtr rather than a Counter& - // but that would be fairly complex to change. - std::vector deleted_counters_ ABSL_GUARDED_BY(mutex_); - std::vector deleted_gauges_ ABSL_GUARDED_BY(mutex_); - std::vector deleted_text_readouts_ ABSL_GUARDED_BY(mutex_); -}; +// This alias is provided for out-of-repository includes of allocator_impl.h, +// who will be expecting the concrete class to be called AllocatorImpl. +using AllocatorImpl = Allocator; } // namespace Stats } // namespace Envoy diff --git a/source/common/stats/isolated_store_impl.h b/source/common/stats/isolated_store_impl.h index 1ca80e1697236..fedaa5da41f37 100644 --- a/source/common/stats/isolated_store_impl.h +++ b/source/common/stats/isolated_store_impl.h @@ -10,7 +10,7 @@ #include "envoy/stats/store.h" #include "source/common/common/utility.h" -#include "source/common/stats/allocator_impl.h" +#include "source/common/stats/allocator.h" #include "source/common/stats/histogram_impl.h" #include "source/common/stats/null_counter.h" #include "source/common/stats/null_gauge.h" @@ -292,7 +292,7 @@ class IsolatedStoreImpl : public Store { IsolatedStoreImpl(std::unique_ptr&& symbol_table); SymbolTablePtr symbol_table_storage_; - AllocatorImpl alloc_; + Allocator alloc_; IsolatedStatsCache counters_; IsolatedStatsCache gauges_; IsolatedStatsCache histograms_; diff --git a/source/common/stats/metric_impl.h b/source/common/stats/metric_impl.h index 28d23c6aceb47..937b9f5ba4bb5 100644 --- a/source/common/stats/metric_impl.h +++ b/source/common/stats/metric_impl.h @@ -3,7 +3,6 @@ #include #include -#include "envoy/stats/allocator.h" #include "envoy/stats/stats.h" #include "envoy/stats/tag.h" @@ -56,7 +55,7 @@ class MetricHelper { // This necessitates a custom comparator and hasher, using the StatNamePtr's // own StatNamePtrHash and StatNamePtrCompare operators. // -// This is used by AllocatorImpl for counters, gauges, and text-readouts, and +// This is used by Allocator for counters, gauges, and text-readouts, and // is also used by thread_local_store.h for histograms. template using StatSet = absl::flat_hash_set; diff --git a/source/common/stats/thread_local_store.cc b/source/common/stats/thread_local_store.cc index 92ee27fb0f8fd..adc56e8f18911 100644 --- a/source/common/stats/thread_local_store.cc +++ b/source/common/stats/thread_local_store.cc @@ -6,13 +6,13 @@ #include #include -#include "envoy/stats/allocator.h" #include "envoy/stats/histogram.h" #include "envoy/stats/sink.h" #include "envoy/stats/stats.h" #include "source/common/common/lock_guard.h" #include "source/common/runtime/runtime_features.h" +#include "source/common/stats/allocator.h" #include "source/common/stats/histogram_impl.h" #include "source/common/stats/stats_matcher_impl.h" #include "source/common/stats/tag_producer_impl.h" @@ -914,7 +914,7 @@ bool ParentHistogramImpl::decRefCount() { // decrement it, and we'll wind up with a dtor/update race. To avoid this we // must hold the lock until the histogram is removed from the map. // - // See also StatsSharedImpl::decRefCount() in allocator_impl.cc, which has + // See also StatsSharedImpl::decRefCount() in allocator.cc, which has // the same issue. ret = thread_local_store_.decHistogramRefCount(*this, ref_count_); } diff --git a/source/common/stats/thread_local_store.h b/source/common/stats/thread_local_store.h index 1e5a6e89ae696..a4a04f73a834e 100644 --- a/source/common/stats/thread_local_store.h +++ b/source/common/stats/thread_local_store.h @@ -13,7 +13,7 @@ #include "source/common/common/hash.h" #include "source/common/common/thread_synchronizer.h" -#include "source/common/stats/allocator_impl.h" +#include "source/common/stats/allocator.h" #include "source/common/stats/histogram_impl.h" #include "source/common/stats/null_counter.h" #include "source/common/stats/null_gauge.h" @@ -247,7 +247,7 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo // The counters, gauges and text readouts in the TLS cache are stored by reference, // depending on the CentralCache for backing store. This avoids a potential // contention-storm when destructing a scope, as the counter/gauge ref-count - // decrement in allocator_impl.cc needs to hold the single allocator mutex. + // decrement in allocator.cc needs to hold the single allocator mutex. StatRefMap counters_; StatRefMap gauges_; StatRefMap text_readouts_; @@ -255,7 +255,7 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo // Histograms also require holding a mutex while decrementing reference // counts. The only difference from other stats is that the histogram_set_ // lives in the ThreadLocalStore object, rather than in - // AllocatorImpl. Histograms are removed from that set when all scopes + // Allocator. Histograms are removed from that set when all scopes // referencing the histogram are dropped. Each ParentHistogram has a unique // index, which is not re-used during the process lifetime. // diff --git a/source/exe/stripped_main_base.h b/source/exe/stripped_main_base.h index 2a067e88a6f54..4645cfbc0c118 100644 --- a/source/exe/stripped_main_base.h +++ b/source/exe/stripped_main_base.h @@ -78,7 +78,7 @@ class StrippedMainBase { const Envoy::Server::Options& options_; Server::ComponentFactory& component_factory_; Stats::SymbolTableImpl symbol_table_; - Stats::AllocatorImpl stats_allocator_; + Stats::Allocator stats_allocator_; ThreadLocal::InstanceImplPtr tls_; std::unique_ptr restarter_; diff --git a/source/server/hot_restart_impl.h b/source/server/hot_restart_impl.h index 72891ca2341af..4e5a27d53ef7f 100644 --- a/source/server/hot_restart_impl.h +++ b/source/server/hot_restart_impl.h @@ -12,7 +12,7 @@ #include "envoy/server/hot_restart.h" #include "source/common/common/assert.h" -#include "source/common/stats/allocator_impl.h" +#include "source/common/stats/allocator.h" #include "source/server/hot_restarting_child.h" #include "source/server/hot_restarting_parent.h" diff --git a/source/server/hot_restart_nop_impl.h b/source/server/hot_restart_nop_impl.h index c27ff0766e25e..b1756d256f7db 100644 --- a/source/server/hot_restart_nop_impl.h +++ b/source/server/hot_restart_nop_impl.h @@ -5,7 +5,7 @@ #include "envoy/server/hot_restart.h" #include "source/common/common/thread.h" -#include "source/common/stats/allocator_impl.h" +#include "source/common/stats/allocator.h" namespace Envoy { namespace Server { diff --git a/test/common/stats/BUILD b/test/common/stats/BUILD index 690c94fd01e15..cdc01554e3ae5 100644 --- a/test/common/stats/BUILD +++ b/test/common/stats/BUILD @@ -13,8 +13,8 @@ licenses(["notice"]) # Apache 2 envoy_package() envoy_cc_test( - name = "allocator_impl_test", - srcs = ["allocator_impl_test.cc"], + name = "allocator_test", + srcs = ["allocator_test.cc"], rbe_pool = "6gig", deps = [ "//source/common/stats:allocator_lib", diff --git a/test/common/stats/allocator_impl_test.cc b/test/common/stats/allocator_test.cc similarity index 95% rename from test/common/stats/allocator_impl_test.cc rename to test/common/stats/allocator_test.cc index c07d07d15768a..42ba717ab43ad 100644 --- a/test/common/stats/allocator_impl_test.cc +++ b/test/common/stats/allocator_test.cc @@ -4,7 +4,7 @@ #include "envoy/stats/sink.h" -#include "source/common/stats/allocator_impl.h" +#include "source/common/stats/allocator.h" #include "test/common/stats/stat_test_utility.h" #include "test/test_common/logging.h" @@ -18,10 +18,10 @@ namespace Envoy { namespace Stats { namespace { -class AllocatorImplTest : public testing::Test { +class AllocatorTest : public testing::Test { protected: - AllocatorImplTest() : pool_(symbol_table_), alloc_(symbol_table_) {} - ~AllocatorImplTest() override { clearStorage(); } + AllocatorTest() : pool_(symbol_table_), alloc_(symbol_table_) {} + ~AllocatorTest() override { clearStorage(); } StatNameStorage makeStatStorage(absl::string_view name) { return {name, symbol_table_}; } @@ -41,12 +41,12 @@ class AllocatorImplTest : public testing::Test { // Declare the pool before the allocator because the allocator could contain // a TestSinkPredicates object whose lifetime should be bounded by that of the pool. StatNamePool pool_; - AllocatorImpl alloc_; + Allocator alloc_; bool are_stats_marked_for_deletion_ = false; }; // Allocate 2 counters of the same name, and you'll get the same object. -TEST_F(AllocatorImplTest, CountersWithSameName) { +TEST_F(AllocatorTest, CountersWithSameName) { StatName counter_name = makeStat("counter.name"); CounterSharedPtr c1 = alloc_.makeCounter(counter_name, StatName(), {}); EXPECT_EQ(1, c1->use_count()); @@ -64,7 +64,7 @@ TEST_F(AllocatorImplTest, CountersWithSameName) { EXPECT_EQ(2, c2->value()); } -TEST_F(AllocatorImplTest, GaugesWithSameName) { +TEST_F(AllocatorTest, GaugesWithSameName) { StatName gauge_name = makeStat("gauges.name"); GaugeSharedPtr g1 = alloc_.makeGauge(gauge_name, StatName(), {}, Gauge::ImportMode::Accumulate); EXPECT_EQ(1, g1->use_count()); @@ -88,7 +88,7 @@ TEST_F(AllocatorImplTest, GaugesWithSameName) { // zero at the same time as we are allocating another instance of that // stat. This test reproduces that race organically by having a 12 threads each // iterate 10k times. -TEST_F(AllocatorImplTest, RefCountDecAllocRaceOrganic) { +TEST_F(AllocatorTest, RefCountDecAllocRaceOrganic) { StatName counter_name = makeStat("counter.name"); StatName gauge_name = makeStat("gauge.name"); Thread::ThreadFactory& thread_factory = Thread::threadFactoryForTest(); @@ -119,25 +119,25 @@ TEST_F(AllocatorImplTest, RefCountDecAllocRaceOrganic) { // makeCounter() until the first thread finishes destructing the object. Thus // the test gives thread2 5 seconds to complete before releasing thread 1 to // complete its destruction of the counter. -TEST_F(AllocatorImplTest, RefCountDecAllocRaceSynchronized) { +TEST_F(AllocatorTest, RefCountDecAllocRaceSynchronized) { StatName counter_name = makeStat("counter.name"); Thread::ThreadFactory& thread_factory = Thread::threadFactoryForTest(); alloc_.sync().enable(); - alloc_.sync().waitOn(AllocatorImpl::DecrementToZeroSyncPoint); + alloc_.sync().waitOn(Allocator::DecrementToZeroSyncPoint); Thread::ThreadPtr thread = thread_factory.createThread([&]() { CounterSharedPtr counter = alloc_.makeCounter(counter_name, StatName(), {}); counter->inc(); counter->reset(); // Blocks in thread synchronizer waiting on DecrementToZeroSyncPoint }); - alloc_.sync().barrierOn(AllocatorImpl::DecrementToZeroSyncPoint); + alloc_.sync().barrierOn(Allocator::DecrementToZeroSyncPoint); EXPECT_TRUE(alloc_.isMutexLockedForTest()); - alloc_.sync().signal(AllocatorImpl::DecrementToZeroSyncPoint); + alloc_.sync().signal(Allocator::DecrementToZeroSyncPoint); thread->join(); EXPECT_FALSE(alloc_.isMutexLockedForTest()); } -TEST_F(AllocatorImplTest, HiddenGauge) { +TEST_F(AllocatorTest, HiddenGauge) { GaugeSharedPtr hidden_gauge = alloc_.makeGauge(makeStat("hidden"), StatName(), {}, Gauge::ImportMode::HiddenAccumulate); EXPECT_EQ(hidden_gauge->importMode(), Gauge::ImportMode::HiddenAccumulate); @@ -154,7 +154,7 @@ TEST_F(AllocatorImplTest, HiddenGauge) { EXPECT_FALSE(never_import_hidden_gauge->hidden()); } -TEST_F(AllocatorImplTest, ForEachCounter) { +TEST_F(AllocatorTest, ForEachCounter) { StatNameHashSet stat_names; std::vector counters; @@ -207,7 +207,7 @@ TEST_F(AllocatorImplTest, ForEachCounter) { EXPECT_EQ(num_iterations, 0); } -TEST_F(AllocatorImplTest, ForEachGauge) { +TEST_F(AllocatorTest, ForEachGauge) { StatNameHashSet stat_names; std::vector gauges; @@ -260,7 +260,7 @@ TEST_F(AllocatorImplTest, ForEachGauge) { EXPECT_EQ(num_iterations, 0); } -TEST_F(AllocatorImplTest, ForEachTextReadout) { +TEST_F(AllocatorTest, ForEachTextReadout) { StatNameHashSet stat_names; std::vector text_readouts; @@ -316,7 +316,7 @@ TEST_F(AllocatorImplTest, ForEachTextReadout) { // Verify that we don't crash if a nullptr is passed in for the size lambda for // the for each stat methods. -TEST_F(AllocatorImplTest, ForEachWithNullSizeLambda) { +TEST_F(AllocatorTest, ForEachWithNullSizeLambda) { std::vector counters; std::vector text_readouts; std::vector gauges; @@ -363,7 +363,7 @@ TEST_F(AllocatorImplTest, ForEachWithNullSizeLambda) { // Currently, if we ask for a stat from the Allocator that has already been // marked for deletion (i.e. rejected) we get a new stat with the same name. // This test documents this behavior. -TEST_F(AllocatorImplTest, AskForDeletedStat) { +TEST_F(AllocatorTest, AskForDeletedStat) { const size_t num_stats = 10; are_stats_marked_for_deletion_ = true; @@ -431,7 +431,7 @@ TEST_F(AllocatorImplTest, AskForDeletedStat) { EXPECT_EQ(rejected_text_readout.value(), "deleted value"); } -TEST_F(AllocatorImplTest, ForEachSinkedCounter) { +TEST_F(AllocatorTest, ForEachSinkedCounter) { std::unique_ptr moved_sink_predicates = std::make_unique(); TestUtil::TestSinkPredicates* sink_predicates = moved_sink_predicates.get(); @@ -477,7 +477,7 @@ TEST_F(AllocatorImplTest, ForEachSinkedCounter) { EXPECT_EQ(num_iterations, 0); } -TEST_F(AllocatorImplTest, ForEachSinkedGauge) { +TEST_F(AllocatorTest, ForEachSinkedGauge) { std::unique_ptr moved_sink_predicates = std::make_unique(); TestUtil::TestSinkPredicates* sink_predicates = moved_sink_predicates.get(); @@ -522,7 +522,7 @@ TEST_F(AllocatorImplTest, ForEachSinkedGauge) { EXPECT_EQ(num_iterations, 0); } -TEST_F(AllocatorImplTest, ForEachSinkedGaugeHidden) { +TEST_F(AllocatorTest, ForEachSinkedGaugeHidden) { GaugeSharedPtr unhidden_gauge; GaugeSharedPtr hidden_gauge; @@ -547,7 +547,7 @@ TEST_F(AllocatorImplTest, ForEachSinkedGaugeHidden) { EXPECT_EQ(num_iterations, 1); } -TEST_F(AllocatorImplTest, ForEachSinkedGaugeHiddenPredicate) { +TEST_F(AllocatorTest, ForEachSinkedGaugeHiddenPredicate) { std::unique_ptr moved_sink_predicates = std::make_unique(); TestUtil::TestSinkPredicates* sink_predicates = moved_sink_predicates.get(); @@ -581,7 +581,7 @@ TEST_F(AllocatorImplTest, ForEachSinkedGaugeHiddenPredicate) { EXPECT_EQ(num_iterations, 2); } -TEST_F(AllocatorImplTest, ForEachSinkedTextReadout) { +TEST_F(AllocatorTest, ForEachSinkedTextReadout) { std::unique_ptr moved_sink_predicates = std::make_unique(); TestUtil::TestSinkPredicates* sink_predicates = moved_sink_predicates.get(); diff --git a/test/common/stats/deferred_creation_stats_test.cc b/test/common/stats/deferred_creation_stats_test.cc index 63fff56fa5961..f192c63cb5ac0 100644 --- a/test/common/stats/deferred_creation_stats_test.cc +++ b/test/common/stats/deferred_creation_stats_test.cc @@ -22,7 +22,7 @@ MAKE_STATS_STRUCT(AwesomeStats, AwesomeStatNames, AWESOME_STATS); class DeferredCreationStatsTest : public testing::Test { public: SymbolTableImpl symbol_table_; - AllocatorImpl allocator_{symbol_table_}; + Allocator allocator_{symbol_table_}; ThreadLocalStoreImpl store_{allocator_}; AwesomeStatNames stats_names_{symbol_table_}; }; diff --git a/test/common/stats/metric_impl_test.cc b/test/common/stats/metric_impl_test.cc index 1ca36e9bce18a..2b7268d65b210 100644 --- a/test/common/stats/metric_impl_test.cc +++ b/test/common/stats/metric_impl_test.cc @@ -1,6 +1,6 @@ #include -#include "source/common/stats/allocator_impl.h" +#include "source/common/stats/allocator.h" #include "source/common/stats/utility.h" #include "test/mocks/stats/mocks.h" @@ -25,7 +25,7 @@ class MetricImplTest : public testing::Test { } SymbolTableImpl symbol_table_; - AllocatorImpl alloc_; + Allocator alloc_; StatNamePool pool_; }; diff --git a/test/common/stats/real_thread_test_base.h b/test/common/stats/real_thread_test_base.h index c0a7d7bf48942..8450e02fc1737 100644 --- a/test/common/stats/real_thread_test_base.h +++ b/test/common/stats/real_thread_test_base.h @@ -20,7 +20,7 @@ class ThreadLocalStoreNoMocksMixin { StatName makeStatName(absl::string_view name); SymbolTableImpl symbol_table_; - AllocatorImpl alloc_; + Allocator alloc_; ThreadLocalStoreImplPtr store_; Scope& scope_; StatNamePool pool_; diff --git a/test/common/stats/stat_merger_test.cc b/test/common/stats/stat_merger_test.cc index bec86308a1a86..e15c752f54f91 100644 --- a/test/common/stats/stat_merger_test.cc +++ b/test/common/stats/stat_merger_test.cc @@ -305,7 +305,7 @@ TEST_F(StatMergerDynamicTest, DynamicsWithRealSymbolTable) { class StatMergerThreadLocalTest : public testing::Test { protected: SymbolTableImpl symbol_table_; - AllocatorImpl alloc_{symbol_table_}; + Allocator alloc_{symbol_table_}; ThreadLocalStoreImpl store_{alloc_}; }; diff --git a/test/common/stats/thread_local_store_speed_test.cc b/test/common/stats/thread_local_store_speed_test.cc index 558a44eb76ea3..75523c1ffd558 100644 --- a/test/common/stats/thread_local_store_speed_test.cc +++ b/test/common/stats/thread_local_store_speed_test.cc @@ -6,7 +6,7 @@ #include "source/common/common/logger.h" #include "source/common/common/thread.h" #include "source/common/event/dispatcher_impl.h" -#include "source/common/stats/allocator_impl.h" +#include "source/common/stats/allocator.h" #include "source/common/stats/stats_matcher_impl.h" #include "source/common/stats/symbol_table.h" #include "source/common/stats/tag_producer_impl.h" @@ -77,7 +77,7 @@ class ThreadLocalStorePerf { NiceMock context_; Stats::SymbolTableImpl symbol_table_; Event::SimulatedTimeSystem time_system_; - Stats::AllocatorImpl heap_alloc_; + Stats::Allocator heap_alloc_; Event::DispatcherPtr dispatcher_; ThreadLocal::InstanceImplPtr tls_; Stats::ThreadLocalStoreImpl store_; diff --git a/test/common/stats/thread_local_store_test.cc b/test/common/stats/thread_local_store_test.cc index f23cc2ec2fb44..ca73948af9e0b 100644 --- a/test/common/stats/thread_local_store_test.cc +++ b/test/common/stats/thread_local_store_test.cc @@ -107,7 +107,7 @@ class StatsThreadLocalStoreTest : public testing::Test { SymbolTableImpl symbol_table_; NiceMock main_thread_dispatcher_; NiceMock tls_; - AllocatorImpl alloc_; + Allocator alloc_; MockSink sink_; ThreadLocalStoreImplPtr store_; Scope& scope_; @@ -235,7 +235,7 @@ class HistogramTest : public testing::Test { NiceMock main_thread_dispatcher_; NiceMock tls_; StatNamePool pool_; - AllocatorImpl alloc_; + Allocator alloc_; MockSink sink_; ThreadLocalStoreImplPtr store_; Scope& scope_; @@ -1709,7 +1709,7 @@ class RememberStatsMatcherTest : public testing::TestWithParam { SymbolTableImpl symbol_table_; NiceMock main_thread_dispatcher_; NiceMock tls_; - AllocatorImpl heap_alloc_; + Allocator heap_alloc_; ThreadLocalStoreImpl store_; ScopeSharedPtr scope_; }; @@ -1885,7 +1885,7 @@ class StatsThreadLocalStoreTestNoFixture : public testing::Test { NiceMock tls_; MockSink sink_; SymbolTableImpl symbol_table_; - AllocatorImpl alloc_; + Allocator alloc_; ThreadLocalStoreImpl store_; Scope& scope_; NiceMock main_thread_dispatcher_; @@ -1970,7 +1970,7 @@ TEST(ThreadLocalStoreThreadTest, ConstructDestruct) { Api::ApiPtr api = Api::createApiForTest(); Event::DispatcherPtr dispatcher = api->allocateDispatcher("test_thread"); NiceMock tls; - AllocatorImpl alloc(symbol_table); + Allocator alloc(symbol_table); ThreadLocalStoreImpl store(alloc); store.initializeThreading(*dispatcher, tls); diff --git a/test/common/stats/utility_test.cc b/test/common/stats/utility_test.cc index aa2f25288c8f9..d51e7716a897c 100644 --- a/test/common/stats/utility_test.cc +++ b/test/common/stats/utility_test.cc @@ -36,7 +36,7 @@ class StatsUtilityTest : public testing::TestWithParam { {{pool_.add("tag1"), pool_.add("value1")}, {pool_.add("tag2"), pool_.add("value2")}}) { switch (GetParam()) { case StoreType::ThreadLocal: - alloc_ = std::make_unique(*symbol_table_), + alloc_ = std::make_unique(*symbol_table_), store_ = std::make_unique(*alloc_); break; case StoreType::Isolated: @@ -159,7 +159,7 @@ class StatsUtilityTest : public testing::TestWithParam { SymbolTablePtr symbol_table_; StatNamePool pool_; - std::unique_ptr alloc_; + std::unique_ptr alloc_; std::unique_ptr store_; ScopeSharedPtr scope_; absl::flat_hash_set results_; diff --git a/test/extensions/access_loggers/stats/stats_test.cc b/test/extensions/access_loggers/stats/stats_test.cc index 3c9775811f8e1..dffb67c0b576e 100644 --- a/test/extensions/access_loggers/stats/stats_test.cc +++ b/test/extensions/access_loggers/stats/stats_test.cc @@ -1,6 +1,6 @@ #include "envoy/stats/sink.h" -#include "source/common/stats/allocator_impl.h" +#include "source/common/stats/allocator.h" #include "source/common/stats/thread_local_store.h" #include "source/extensions/access_loggers/stats/stats.h" diff --git a/test/integration/server.cc b/test/integration/server.cc index 903cb15c368fd..4203d3f42d3a3 100644 --- a/test/integration/server.cc +++ b/test/integration/server.cc @@ -246,9 +246,8 @@ IntegrationTestServerImpl::IntegrationTestServerImpl( Event::TestTimeSystem& time_system, Api::Api& api, const std::string& config_path, bool use_real_stats, std::unique_ptr&& config_proto) : IntegrationTestServer(time_system, api, config_path, std::move(config_proto)) { - stats_allocator_ = - (use_real_stats ? std::make_unique(symbol_table_) - : std::make_unique(symbol_table_)); + stats_allocator_ = (use_real_stats ? std::make_unique(symbol_table_) + : std::make_unique(symbol_table_)); } void IntegrationTestServerImpl::createAndRunEnvoyServer( diff --git a/test/integration/server.h b/test/integration/server.h index 8280ef3207ead..dde90107a4225 100644 --- a/test/integration/server.h +++ b/test/integration/server.h @@ -16,7 +16,7 @@ #include "source/common/common/lock_guard.h" #include "source/common/common/logger.h" #include "source/common/common/thread.h" -#include "source/common/stats/allocator_impl.h" +#include "source/common/stats/allocator.h" #include "source/common/stats/isolated_store_impl.h" #include "source/common/stats/null_counter.h" #include "source/common/stats/null_gauge.h" @@ -214,10 +214,9 @@ class NotifyingCounter : public Stats::Counter { }; // A stats allocator which creates NotifyingCounters rather than regular CounterImpls. -class NotifyingAllocatorImpl : public Stats::AllocatorImpl { +class NotifyingAllocator : public Stats::Allocator { public: - using Stats::AllocatorImpl::AllocatorImpl; - + NotifyingAllocator(Stats::SymbolTable& symbol_table) : Stats::Allocator(symbol_table) {} void waitForCounterFromStringEq(const std::string& name, uint64_t value) { absl::MutexLock l(mutex_); ENVOY_LOG_MISC(trace, "waiting for {} to be {}", name, value); @@ -249,7 +248,7 @@ class NotifyingAllocatorImpl : public Stats::AllocatorImpl { Stats::Counter* makeCounterInternal(StatName name, StatName tag_extracted_name, const StatNameTagVector& stat_name_tags) override { Stats::Counter* counter = new NotifyingCounter( - Stats::AllocatorImpl::makeCounterInternal(name, tag_extracted_name, stat_name_tags), mutex_, + Stats::Allocator::makeCounterInternal(name, tag_extracted_name, stat_name_tags), mutex_, condvar_); { absl::MutexLock l(mutex_); @@ -569,7 +568,7 @@ class IntegrationTestServer : public Logger::Loggable, virtual Stats::Store& statStore() PURE; virtual Server::ThreadLocalOverloadState& overloadState() PURE; virtual Network::Address::InstanceConstSharedPtr adminAddress() PURE; - virtual Stats::NotifyingAllocatorImpl& notifyingStatsAllocator() PURE; + virtual Stats::NotifyingAllocator& notifyingStatsAllocator() PURE; void useAdminInterfaceToQuit(bool use) { use_admin_interface_to_quit_ = use; } bool useAdminInterfaceToQuit() { return use_admin_interface_to_quit_; } @@ -652,8 +651,8 @@ class IntegrationTestServerImpl : public IntegrationTestServer { Network::Address::InstanceConstSharedPtr adminAddress() override { return admin_address_; } - Stats::NotifyingAllocatorImpl& notifyingStatsAllocator() override { - auto* ret = dynamic_cast(stats_allocator_.get()); + Stats::NotifyingAllocator& notifyingStatsAllocator() override { + auto* ret = dynamic_cast(stats_allocator_.get()); RELEASE_ASSERT(ret != nullptr, "notifyingStatsAllocator() is not created when real_stats is true"); return *ret; @@ -676,7 +675,7 @@ class IntegrationTestServerImpl : public IntegrationTestServer { Network::Address::InstanceConstSharedPtr admin_address_; absl::Notification server_gone_; Stats::SymbolTableImpl symbol_table_; - std::unique_ptr stats_allocator_; + std::unique_ptr stats_allocator_; }; } // namespace Envoy diff --git a/test/mocks/server/hot_restart.h b/test/mocks/server/hot_restart.h index a467c4b0e613d..85d1d40051ada 100644 --- a/test/mocks/server/hot_restart.h +++ b/test/mocks/server/hot_restart.h @@ -39,7 +39,7 @@ class MockHotRestart : public HotRestart { Stats::TestUtil::TestSymbolTable symbol_table_; Thread::MutexBasicLockable log_lock_; Thread::MutexBasicLockable access_log_lock_; - Stats::AllocatorImpl stats_allocator_; + Stats::Allocator stats_allocator_; }; } // namespace Server } // namespace Envoy diff --git a/test/server/admin/prometheus_stats_test.cc b/test/server/admin/prometheus_stats_test.cc index 854ec666d2a15..a9814baf25397 100644 --- a/test/server/admin/prometheus_stats_test.cc +++ b/test/server/admin/prometheus_stats_test.cc @@ -133,7 +133,7 @@ class PrometheusStatsFormatterTest : public testing::Test { } Stats::TestUtil::TestSymbolTable symbol_table_; - Stats::AllocatorImpl alloc_; + Stats::Allocator alloc_; Stats::StatNamePool pool_; std::vector counters_; std::vector gauges_; @@ -1746,7 +1746,7 @@ class RealHistogramNativePrometheusTest : public testing::Test { } Stats::TestUtil::TestSymbolTable symbol_table_; - Stats::AllocatorImpl alloc_; + Stats::Allocator alloc_; NiceMock tls_; std::unique_ptr store_; Stats::Scope& scope_; diff --git a/test/server/admin/stats_handler_test.cc b/test/server/admin/stats_handler_test.cc index 218ed5d77c3bf..0a32f8b80332e 100644 --- a/test/server/admin/stats_handler_test.cc +++ b/test/server/admin/stats_handler_test.cc @@ -130,7 +130,7 @@ class StatsHandlerTest { NiceMock tls_; NiceMock api_; Upstream::PerEndpointMetricsTestHelper endpoints_helper_; - Stats::AllocatorImpl alloc_; + Stats::Allocator alloc_; Stats::MockSink sink_; Stats::ThreadLocalStoreImplPtr store_; Stats::CustomStatNamespacesImpl custom_namespaces_; @@ -1302,7 +1302,7 @@ class ThreadedTest : public testing::Test { Thread::RealThreadsTestHelper real_threads_; Stats::SymbolTableImpl symbol_table_; Stats::StatNamePool pool_; - Stats::AllocatorImpl alloc_; + Stats::Allocator alloc_; std::unique_ptr store_; NiceMock cm_; std::vector scopes_{NumScopes}; diff --git a/test/server/admin/stats_render_test_base.h b/test/server/admin/stats_render_test_base.h index 0c566f677be00..a4afb1aed226d 100644 --- a/test/server/admin/stats_render_test_base.h +++ b/test/server/admin/stats_render_test_base.h @@ -30,7 +30,7 @@ class StatsRenderTestBase : public testing::Test { const std::vector& vals); Stats::SymbolTableImpl symbol_table_; - Stats::AllocatorImpl alloc_; + Stats::Allocator alloc_; testing::NiceMock sink_; testing::NiceMock main_thread_dispatcher_; testing::NiceMock tls_; diff --git a/test/server/admin/stats_request_test.cc b/test/server/admin/stats_request_test.cc index 25da8e92e2104..2883de6f61451 100644 --- a/test/server/admin/stats_request_test.cc +++ b/test/server/admin/stats_request_test.cc @@ -89,7 +89,7 @@ class StatsRequestTest : public testing::Test { Stats::SymbolTableImpl symbol_table_; Stats::StatNamePool pool_; - Stats::AllocatorImpl alloc_; + Stats::Allocator alloc_; NiceMock sink_; NiceMock main_thread_dispatcher_; NiceMock tls_; diff --git a/test/server/server_stats_flush_benchmark_test.cc b/test/server/server_stats_flush_benchmark_test.cc index 45bf76a3a7b58..c95854d1f9118 100644 --- a/test/server/server_stats_flush_benchmark_test.cc +++ b/test/server/server_stats_flush_benchmark_test.cc @@ -90,7 +90,7 @@ class StatsSinkFlushSpeedTest { private: Stats::SymbolTableImpl symbol_table_; Stats::StatNamePool pool_; - Stats::AllocatorImpl stats_allocator_; + Stats::Allocator stats_allocator_; Stats::ThreadLocalStoreImpl stats_store_; Event::SimulatedTimeSystem time_system_; FastMockClusterManager cm_;