From 70fdab2d8a7beba7eb817eb900a21e3ffcc994ba Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Tue, 13 Nov 2018 22:58:46 -0500 Subject: [PATCH 1/8] Make an object for the FIlesystem (Filesystem::Instance) and hold the stats counters in there. Signed-off-by: Joshua Marantz --- include/envoy/api/api.h | 3 +- .../access_log/access_log_manager_impl.cc | 2 +- .../access_log/access_log_manager_impl.h | 6 +- source/common/api/api_impl.cc | 10 +-- source/common/api/api_impl.h | 9 ++- source/common/filesystem/filesystem_impl.cc | 17 +++- source/common/filesystem/filesystem_impl.h | 21 ++++- source/common/stats/thread_local_store.cc | 42 ++++++++++ source/common/stats/thread_local_store.h | 19 ++++- source/server/config_validation/api.cc | 5 +- source/server/config_validation/api.h | 2 +- source/server/config_validation/server.cc | 4 +- source/server/server.cc | 4 +- .../access_log_manager_impl_test.cc | 6 +- test/common/api/BUILD | 1 + test/common/api/api_impl_test.cc | 21 +++-- .../common/filesystem/filesystem_impl_test.cc | 78 ++++++++++--------- test/integration/echo_integration_test.cc | 7 +- test/integration/fake_upstream.cc | 5 +- test/integration/http2_integration_test.cc | 8 +- test/integration/integration.cc | 8 +- test/integration/integration.h | 4 +- test/integration/integration_admin_test.cc | 3 +- test/integration/utility.cc | 9 ++- test/integration/utility.h | 2 +- test/mocks/api/mocks.cc | 2 +- test/mocks/api/mocks.h | 4 +- test/server/config_validation/BUILD | 1 + .../config_validation/dispatcher_test.cc | 5 +- 29 files changed, 205 insertions(+), 103 deletions(-) diff --git a/include/envoy/api/api.h b/include/envoy/api/api.h index eece627e00ab5..44da8b525a0cc 100644 --- a/include/envoy/api/api.h +++ b/include/envoy/api/api.h @@ -35,8 +35,7 @@ class Api { */ virtual Filesystem::FileSharedPtr createFile(const std::string& path, Event::Dispatcher& dispatcher, - Thread::BasicLockable& lock, - Stats::Store& stats_store) PURE; + Thread::BasicLockable& lock) PURE; /** * @return bool whether a file exists and can be opened for read on disk. diff --git a/source/common/access_log/access_log_manager_impl.cc b/source/common/access_log/access_log_manager_impl.cc index 56933387b348b..8993bda70a908 100644 --- a/source/common/access_log/access_log_manager_impl.cc +++ b/source/common/access_log/access_log_manager_impl.cc @@ -16,7 +16,7 @@ Filesystem::FileSharedPtr AccessLogManagerImpl::createAccessLog(const std::strin return access_logs_[file_name]; } - access_logs_[file_name] = api_.createFile(file_name, dispatcher_, lock_, stats_store_); + access_logs_[file_name] = api_.createFile(file_name, dispatcher_, lock_); return access_logs_[file_name]; } diff --git a/source/common/access_log/access_log_manager_impl.h b/source/common/access_log/access_log_manager_impl.h index 3c6c0208a2cdf..502ca50c74e86 100644 --- a/source/common/access_log/access_log_manager_impl.h +++ b/source/common/access_log/access_log_manager_impl.h @@ -11,9 +11,8 @@ namespace AccessLog { class AccessLogManagerImpl : public AccessLogManager { public: - AccessLogManagerImpl(Api::Api& api, Event::Dispatcher& dispatcher, Thread::BasicLockable& lock, - Stats::Store& stats_store) - : api_(api), dispatcher_(dispatcher), lock_(lock), stats_store_(stats_store) {} + AccessLogManagerImpl(Api::Api& api, Event::Dispatcher& dispatcher, Thread::BasicLockable& lock) + : api_(api), dispatcher_(dispatcher), lock_(lock) {} // AccessLog::AccessLogManager void reopen() override; @@ -23,7 +22,6 @@ class AccessLogManagerImpl : public AccessLogManager { Api::Api& api_; Event::Dispatcher& dispatcher_; Thread::BasicLockable& lock_; - Stats::Store& stats_store_; std::unordered_map access_logs_; }; diff --git a/source/common/api/api_impl.cc b/source/common/api/api_impl.cc index 7e96e0f1a1a55..a15feb6ea39f9 100644 --- a/source/common/api/api_impl.cc +++ b/source/common/api/api_impl.cc @@ -13,13 +13,13 @@ Event::DispatcherPtr Impl::allocateDispatcher(Event::TimeSystem& time_system) { return Event::DispatcherPtr{new Event::DispatcherImpl(time_system)}; } -Impl::Impl(std::chrono::milliseconds file_flush_interval_msec) - : file_flush_interval_msec_(file_flush_interval_msec) {} +Impl::Impl(std::chrono::milliseconds file_flush_interval_msec, Stats::Store& stats_store) + : file_system_(file_flush_interval_msec, stats_store) {} Filesystem::FileSharedPtr Impl::createFile(const std::string& path, Event::Dispatcher& dispatcher, - Thread::BasicLockable& lock, Stats::Store& stats_store) { - return std::make_shared(path, dispatcher, lock, stats_store, - file_flush_interval_msec_); + Thread::BasicLockable& lock) { + + return file_system_.createFile(path, dispatcher, lock); } bool Impl::fileExists(const std::string& path) { return Filesystem::fileExists(path); } diff --git a/source/common/api/api_impl.h b/source/common/api/api_impl.h index ef4e84f4c9a8c..7d9260a545637 100644 --- a/source/common/api/api_impl.h +++ b/source/common/api/api_impl.h @@ -7,6 +7,8 @@ #include "envoy/event/timer.h" #include "envoy/filesystem/filesystem.h" +#include "common/filesystem/filesystem_impl.h" + namespace Envoy { namespace Api { @@ -15,18 +17,17 @@ namespace Api { */ class Impl : public Api::Api { public: - Impl(std::chrono::milliseconds file_flush_interval_msec); + Impl(std::chrono::milliseconds file_flush_interval_msec, Stats::Store& stats_store); // Api::Api Event::DispatcherPtr allocateDispatcher(Event::TimeSystem& time_system) override; Filesystem::FileSharedPtr createFile(const std::string& path, Event::Dispatcher& dispatcher, - Thread::BasicLockable& lock, - Stats::Store& stats_store) override; + Thread::BasicLockable& lock) override; bool fileExists(const std::string& path) override; std::string fileReadToEnd(const std::string& path) override; private: - std::chrono::milliseconds file_flush_interval_msec_; + Filesystem::Instance file_system_; }; } // namespace Api diff --git a/source/common/filesystem/filesystem_impl.cc b/source/common/filesystem/filesystem_impl.cc index 90d44982026e5..7d464e7fb0f74 100644 --- a/source/common/filesystem/filesystem_impl.cc +++ b/source/common/filesystem/filesystem_impl.cc @@ -94,8 +94,20 @@ bool illegalPath(const std::string& path) { } } +Instance::Instance(std::chrono::milliseconds file_flush_interval_msec, Stats::Store& stats_store) + : file_flush_interval_msec_(file_flush_interval_msec), + file_stats_{FILESYSTEM_STATS(POOL_COUNTER_PREFIX(stats_store, "filesystem."), + POOL_GAUGE_PREFIX(stats_store, "filesystem."))} {} + +FileSharedPtr Instance::createFile(const std::string& path, Event::Dispatcher& dispatcher, + Thread::BasicLockable& lock, + std::chrono::milliseconds file_flush_interval_msec) { + return std::make_shared(path, dispatcher, lock, file_stats_, + file_flush_interval_msec); +}; + FileImpl::FileImpl(const std::string& path, Event::Dispatcher& dispatcher, - Thread::BasicLockable& lock, Stats::Store& stats_store, + Thread::BasicLockable& lock, FileSystemStats& stats, std::chrono::milliseconds flush_interval_msec) : path_(path), file_lock_(lock), flush_timer_(dispatcher.createTimer([this]() -> void { stats_.flushed_by_timer_.inc(); @@ -103,8 +115,7 @@ FileImpl::FileImpl(const std::string& path, Event::Dispatcher& dispatcher, flush_timer_->enableTimer(flush_interval_msec_); })), os_sys_calls_(Api::OsSysCallsSingleton::get()), flush_interval_msec_(flush_interval_msec), - stats_{FILESYSTEM_STATS(POOL_COUNTER_PREFIX(stats_store, "filesystem."), - POOL_GAUGE_PREFIX(stats_store, "filesystem."))} { + stats_(stats) { open(); } diff --git a/source/common/filesystem/filesystem_impl.h b/source/common/filesystem/filesystem_impl.h index 0d1bb7a4116f4..2b43b4a263baf 100644 --- a/source/common/filesystem/filesystem_impl.h +++ b/source/common/filesystem/filesystem_impl.h @@ -31,6 +31,23 @@ struct FileSystemStats { namespace Filesystem { +class Instance { +public: + explicit Instance(std::chrono::milliseconds file_flush_interval_msec, Stats::Store& store); + + FileSharedPtr createFile(const std::string& path, Event::Dispatcher& dispatcher, + Thread::BasicLockable& lock, + std::chrono::milliseconds file_flush_interval_msec); + FileSharedPtr createFile(const std::string& path, Event::Dispatcher& dispatcher, + Thread::BasicLockable& lock) { + return createFile(path, dispatcher, lock, file_flush_interval_msec_); + } + +private: + std::chrono::milliseconds file_flush_interval_msec_; + FileSystemStats file_stats_; +}; + /** * @return bool whether a file exists on disk and can be opened for read. */ @@ -82,7 +99,7 @@ bool illegalPath(const std::string& path); class FileImpl : public File { public: FileImpl(const std::string& path, Event::Dispatcher& dispatcher, Thread::BasicLockable& lock, - Stats::Store& stats_store, std::chrono::milliseconds flush_interval_msec); + FileSystemStats& stats_, std::chrono::milliseconds flush_interval_msec); ~FileImpl(); // Filesystem::File @@ -149,7 +166,7 @@ class FileImpl : public File { const std::chrono::milliseconds flush_interval_msec_; // Time interval buffer gets flushed no // matter if it reached the MIN_FLUSH_SIZE // or not. - FileSystemStats stats_; + FileSystemStats& stats_; }; } // namespace Filesystem diff --git a/source/common/stats/thread_local_store.cc b/source/common/stats/thread_local_store.cc index 6726622616898..0327387412884 100644 --- a/source/common/stats/thread_local_store.cc +++ b/source/common/stats/thread_local_store.cc @@ -34,6 +34,48 @@ ThreadLocalStoreImpl::~ThreadLocalStoreImpl() { ASSERT(scopes_.empty()); } +void ThreadLocalStoreImpl::setStatsMatcher(StatsMatcherPtr&& stats_matcher) { + stats_matcher_ = std::move(stats_matcher); + + // The Filesystem and potentially other stat-registering objects are + // constructed prior to the stat-matcher, and those add stats + // in the default_scope. There should be no requests, so there will + // be no copies in TLS caches. + Thread::LockGuard lock(lock_); + for (ScopeImpl* scope : scopes_) { + removeRejectedStats(scope->central_cache_.counters_, deleted_counters_); + removeRejectedStats(scope->central_cache_.gauges_, deleted_gauges_); + removeRejectedStats(scope->central_cache_.histograms_, deleted_histograms_); + } +} + +template +void ThreadLocalStoreImpl::removeRejectedStats(StatMapClass& map, StatListClass& list) { + std::vector remove_list; + for (auto& stat : map) { + if (rejects(stat.first)) { + remove_list.push_back(stat.first); + } + } + for (const char* stat_name : remove_list) { + auto p = map.find(stat_name); + ASSERT(p != map.end()); + list.push_back(p->second); // Save SharedPtr to the list to avoid invalidating refs to stat. + map.erase(p); + } +} + +bool ThreadLocalStoreImpl::rejects(const char* name) const { + // TODO(ambuc): If stats_matcher_ depends on regexes, this operation (on the + // hot path) could become prohibitively expensive. Revisit this usage in the + // future. + // + // Also note that the elaboration of the stat-name into a string is expensive, + // so I think it might be better to move the matcher test until after caching, + // unless its acceptsAll/rejectsAll. + return stats_matcher_->rejects(name); +} + std::vector ThreadLocalStoreImpl::counters() const { // Handle de-dup due to overlapping scopes. std::vector ret; diff --git a/source/common/stats/thread_local_store.h b/source/common/stats/thread_local_store.h index 74f70c51f8f24..99f85cb9455e2 100644 --- a/source/common/stats/thread_local_store.h +++ b/source/common/stats/thread_local_store.h @@ -158,9 +158,7 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo void setTagProducer(TagProducerPtr&& tag_producer) override { tag_producer_ = std::move(tag_producer); } - void setStatsMatcher(StatsMatcherPtr&& stats_matcher) override { - stats_matcher_ = std::move(stats_matcher); - } + void setStatsMatcher(StatsMatcherPtr&& stats_matcher) override; void initializeThreading(Event::Dispatcher& main_thread_dispatcher, ThreadLocal::Instance& tls) override; void shutdownThreading() override; @@ -254,6 +252,9 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo void releaseScopeCrossThread(ScopeImpl* scope); void mergeInternal(PostMergeCb mergeCb); absl::string_view truncateStatNameIfNeeded(absl::string_view name); + bool rejects(const char* name) const; + template + void removeRejectedStats(StatMapClass& map, StatListClass& list); const Stats::StatsOptions& stats_options_; StatDataAllocator& alloc_; @@ -270,6 +271,18 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo Counter& num_last_resort_stats_; HeapStatDataAllocator heap_allocator_; SourceImpl source_; + + // 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_; + std::vector deleted_gauges_; + std::vector deleted_histograms_; }; } // namespace Stats diff --git a/source/server/config_validation/api.cc b/source/server/config_validation/api.cc index 227ed06f4aeb6..7b65d5e3f8fde 100644 --- a/source/server/config_validation/api.cc +++ b/source/server/config_validation/api.cc @@ -5,8 +5,9 @@ namespace Envoy { namespace Api { -ValidationImpl::ValidationImpl(std::chrono::milliseconds file_flush_interval_msec) - : Impl(file_flush_interval_msec) {} +ValidationImpl::ValidationImpl(std::chrono::milliseconds file_flush_interval_msec, + Stats::Store& stats_store) + : Impl(file_flush_interval_msec, stats_store) {} Event::DispatcherPtr ValidationImpl::allocateDispatcher(Event::TimeSystem& time_system) { return Event::DispatcherPtr{new Event::ValidationDispatcher(time_system)}; diff --git a/source/server/config_validation/api.h b/source/server/config_validation/api.h index ef2f71dcbd85d..c09cad87141c0 100644 --- a/source/server/config_validation/api.h +++ b/source/server/config_validation/api.h @@ -15,7 +15,7 @@ namespace Api { */ class ValidationImpl : public Impl { public: - ValidationImpl(std::chrono::milliseconds file_flush_interval_msec); + ValidationImpl(std::chrono::milliseconds file_flush_interval_msec, Stats::Store& stats_store); Event::DispatcherPtr allocateDispatcher(Event::TimeSystem&) override; }; diff --git a/source/server/config_validation/server.cc b/source/server/config_validation/server.cc index 30a483de3eb19..d5184f6d41294 100644 --- a/source/server/config_validation/server.cc +++ b/source/server/config_validation/server.cc @@ -42,10 +42,10 @@ ValidationInstance::ValidationInstance(Options& options, Event::TimeSystem& time Thread::BasicLockable& access_log_lock, ComponentFactory& component_factory) : options_(options), time_system_(time_system), stats_store_(store), - api_(new Api::ValidationImpl(options.fileFlushIntervalMsec())), + api_(new Api::ValidationImpl(options.fileFlushIntervalMsec(), store)), dispatcher_(api_->allocateDispatcher(time_system)), singleton_manager_(new Singleton::ManagerImpl()), - access_log_manager_(*api_, *dispatcher_, access_log_lock, store), mutex_tracer_(nullptr) { + access_log_manager_(*api_, *dispatcher_, access_log_lock), mutex_tracer_(nullptr) { try { initialize(options, local_address, component_factory); } catch (const EnvoyException& e) { diff --git a/source/server/server.cc b/source/server/server.cc index d1170b4125f41..6345d71fb1b26 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -53,7 +53,7 @@ InstanceImpl::InstanceImpl(Options& options, Event::TimeSystem& time_system, ThreadLocal::Instance& tls) : shutdown_(false), options_(options), time_system_(time_system), restarter_(restarter), start_time_(time(nullptr)), original_start_time_(start_time_), stats_store_(store), - thread_local_(tls), api_(new Api::Impl(options.fileFlushIntervalMsec())), + thread_local_(tls), api_(new Api::Impl(options.fileFlushIntervalMsec(), store)), secret_manager_(std::make_unique()), dispatcher_(api_->allocateDispatcher(time_system)), singleton_manager_(new Singleton::ManagerImpl()), @@ -61,7 +61,7 @@ InstanceImpl::InstanceImpl(Options& options, Event::TimeSystem& time_system, random_generator_(std::move(random_generator)), listener_component_factory_(*this), worker_factory_(thread_local_, *api_, hooks, time_system), dns_resolver_(dispatcher_->createDnsResolver({})), - access_log_manager_(*api_, *dispatcher_, access_log_lock, store), terminated_(false), + access_log_manager_(*api_, *dispatcher_, access_log_lock), terminated_(false), mutex_tracer_(options.mutexTracingEnabled() ? &Envoy::MutexTracerImpl::getOrCreateTracer() : nullptr) { diff --git a/test/common/access_log/access_log_manager_impl_test.cc b/test/common/access_log/access_log_manager_impl_test.cc index d4b644ad6db7e..cd39e86db0249 100644 --- a/test/common/access_log/access_log_manager_impl_test.cc +++ b/test/common/access_log/access_log_manager_impl_test.cc @@ -25,10 +25,10 @@ TEST(AccessLogManagerImpl, reopenAllFiles) { std::shared_ptr log1(new Filesystem::MockFile()); std::shared_ptr log2(new Filesystem::MockFile()); - AccessLogManagerImpl access_log_manager(api, dispatcher, lock, stats_store); - EXPECT_CALL(api, createFile("foo", _, _, _)).WillOnce(Return(log1)); + AccessLogManagerImpl access_log_manager(api, dispatcher, lock); + EXPECT_CALL(api, createFile("foo", _, _)).WillOnce(Return(log1)); access_log_manager.createAccessLog("foo"); - EXPECT_CALL(api, createFile("bar", _, _, _)).WillOnce(Return(log2)); + EXPECT_CALL(api, createFile("bar", _, _)).WillOnce(Return(log2)); access_log_manager.createAccessLog("bar"); // Make sure that getting the access log with the same name returns the same underlying file. diff --git a/test/common/api/BUILD b/test/common/api/BUILD index 0144845cbf774..bb81e6900c84d 100644 --- a/test/common/api/BUILD +++ b/test/common/api/BUILD @@ -13,6 +13,7 @@ envoy_cc_test( srcs = ["api_impl_test.cc"], deps = [ "//source/common/api:api_lib", + "//source/common/stats:isolated_store_lib", "//test/test_common:environment_lib", ], ) diff --git a/test/common/api/api_impl_test.cc b/test/common/api/api_impl_test.cc index f06330d2bd1dc..0fe29955b7aae 100644 --- a/test/common/api/api_impl_test.cc +++ b/test/common/api/api_impl_test.cc @@ -2,6 +2,7 @@ #include #include "common/api/api_impl.h" +#include "common/stats/isolated_store_impl.h" #include "test/test_common/environment.h" @@ -10,20 +11,24 @@ namespace Envoy { namespace Api { -TEST(ApiImplTest, readFileToEnd) { - Impl api(std::chrono::milliseconds(10000)); +class ApiImplTest : public testing::Test { +protected: + ApiImplTest() : api_(std::chrono::milliseconds(10000), store_) {} + Stats::IsolatedStoreImpl store_; + Impl api_; +}; + +TEST_F(ApiImplTest, readFileToEnd) { const std::string data = "test read To End\nWith new lines."; const std::string file_path = TestEnvironment::writeStringToFileForTest("test_api_envoy", data); - EXPECT_EQ(data, api.fileReadToEnd(file_path)); + EXPECT_EQ(data, api_.fileReadToEnd(file_path)); } -TEST(ApiImplTest, fileExists) { - Impl api(std::chrono::milliseconds(10000)); - - EXPECT_TRUE(api.fileExists("/dev/null")); - EXPECT_FALSE(api.fileExists("/dev/blahblahblah")); +TEST_F(ApiImplTest, fileExists) { + EXPECT_TRUE(api_.fileExists("/dev/null")); + EXPECT_FALSE(api_.fileExists("/dev/blahblahblah")); } } // namespace Api diff --git a/test/common/filesystem/filesystem_impl_test.cc b/test/common/filesystem/filesystem_impl_test.cc index 2a1c706dce473..6d5f08eefade8 100644 --- a/test/common/filesystem/filesystem_impl_test.cc +++ b/test/common/filesystem/filesystem_impl_test.cc @@ -28,13 +28,20 @@ using testing::Throw; namespace Envoy { -TEST(FileSystemImpl, BadFile) { +class FileSystemImplTest : public testing::Test { +protected: + FileSystemImplTest() : file_system_(std::chrono::milliseconds(10000), stats_store_) {} + + const std::chrono::milliseconds timeout_40ms_{40}; + Stats::IsolatedStoreImpl stats_store_; + Filesystem::Instance file_system_; +}; + +TEST_F(FileSystemImplTest, BadFile) { Event::MockDispatcher dispatcher; Thread::MutexBasicLockable lock; - Stats::IsolatedStoreImpl store; EXPECT_CALL(dispatcher, createTimer_(_)); - EXPECT_THROW(Filesystem::FileImpl("", dispatcher, lock, store, std::chrono::milliseconds(10000)), - EnvoyException); + EXPECT_THROW(file_system_.createFile("", dispatcher, lock), EnvoyException); } TEST(FileSystemImpl, fileExists) { @@ -104,19 +111,18 @@ TEST(FilesystemImpl, IllegalPath) { EXPECT_TRUE(Filesystem::illegalPath("/_some_non_existent_file")); } -TEST(FileSystemImpl, flushToLogFilePeriodically) { +TEST_F(FileSystemImplTest, flushToLogFilePeriodically) { NiceMock dispatcher; NiceMock* timer = new NiceMock(&dispatcher); Thread::MutexBasicLockable mutex; - Stats::IsolatedStoreImpl stats_store; NiceMock os_sys_calls; TestThreadsafeSingletonInjector os_calls(&os_sys_calls); EXPECT_CALL(os_sys_calls, open_(_, _, _)).WillOnce(Return(5)); - Filesystem::FileImpl file("", dispatcher, mutex, stats_store, std::chrono::milliseconds(40)); + Filesystem::FileSharedPtr file = file_system_.createFile("", dispatcher, mutex, timeout_40ms_); - EXPECT_CALL(*timer, enableTimer(std::chrono::milliseconds(40))); + EXPECT_CALL(*timer, enableTimer(timeout_40ms_)); EXPECT_CALL(os_sys_calls, write_(_, _, _)) .WillOnce(Invoke([](int fd, const void* buffer, size_t num_bytes) -> ssize_t { std::string written = std::string(reinterpret_cast(buffer), num_bytes); @@ -126,7 +132,7 @@ TEST(FileSystemImpl, flushToLogFilePeriodically) { return num_bytes; })); - file.write("test"); + file->write("test"); { Thread::LockGuard lock(os_sys_calls.write_mutex_); @@ -145,8 +151,8 @@ TEST(FileSystemImpl, flushToLogFilePeriodically) { })); // make sure timer is re-enabled on callback call - file.write("test2"); - EXPECT_CALL(*timer, enableTimer(std::chrono::milliseconds(40))); + file->write("test2"); + EXPECT_CALL(*timer, enableTimer(timeout_40ms_)); timer->callback_(); { @@ -157,27 +163,26 @@ TEST(FileSystemImpl, flushToLogFilePeriodically) { } } -TEST(FileSystemImpl, flushToLogFileOnDemand) { +TEST_F(FileSystemImplTest, flushToLogFileOnDemand) { NiceMock dispatcher; NiceMock* timer = new NiceMock(&dispatcher); Thread::MutexBasicLockable mutex; - Stats::IsolatedStoreImpl stats_store; NiceMock os_sys_calls; TestThreadsafeSingletonInjector os_calls(&os_sys_calls); EXPECT_CALL(os_sys_calls, open_(_, _, _)).WillOnce(Return(5)); - Filesystem::FileImpl file("", dispatcher, mutex, stats_store, std::chrono::milliseconds(40)); + Filesystem::FileSharedPtr file = file_system_.createFile("", dispatcher, mutex, timeout_40ms_); - EXPECT_CALL(*timer, enableTimer(std::chrono::milliseconds(40))); + EXPECT_CALL(*timer, enableTimer(timeout_40ms_)); // The first write to a given file will start the flush thread, which can flush // immediately (race on whether it will or not). So do a write and flush to // get that state out of the way, then test that small writes don't trigger a flush. EXPECT_CALL(os_sys_calls, write_(_, _, _)) .WillOnce(Invoke([](int, const void*, size_t num_bytes) -> ssize_t { return num_bytes; })); - file.write("prime-it"); - file.flush(); + file->write("prime-it"); + file->flush(); uint32_t expected_writes = 1; { Thread::LockGuard lock(os_sys_calls.write_mutex_); @@ -193,14 +198,14 @@ TEST(FileSystemImpl, flushToLogFileOnDemand) { return num_bytes; })); - file.write("test"); + file->write("test"); { Thread::LockGuard lock(os_sys_calls.write_mutex_); EXPECT_EQ(expected_writes, os_sys_calls.num_writes_); } - file.flush(); + file->flush(); expected_writes++; { Thread::LockGuard lock(os_sys_calls.write_mutex_); @@ -217,8 +222,8 @@ TEST(FileSystemImpl, flushToLogFileOnDemand) { })); // make sure timer is re-enabled on callback call - file.write("test2"); - EXPECT_CALL(*timer, enableTimer(std::chrono::milliseconds(40))); + file->write("test2"); + EXPECT_CALL(*timer, enableTimer(timeout_40ms_)); timer->callback_(); expected_writes++; @@ -230,18 +235,17 @@ TEST(FileSystemImpl, flushToLogFileOnDemand) { } } -TEST(FileSystemImpl, reopenFile) { +TEST_F(FileSystemImplTest, reopenFile) { NiceMock dispatcher; NiceMock* timer = new NiceMock(&dispatcher); Thread::MutexBasicLockable mutex; - Stats::IsolatedStoreImpl stats_store; NiceMock os_sys_calls; TestThreadsafeSingletonInjector os_calls(&os_sys_calls); Sequence sq; EXPECT_CALL(os_sys_calls, open_(_, _, _)).InSequence(sq).WillOnce(Return(5)); - Filesystem::FileImpl file("", dispatcher, mutex, stats_store, std::chrono::milliseconds(40)); + Filesystem::FileSharedPtr file = file_system_.createFile("", dispatcher, mutex, timeout_40ms_); EXPECT_CALL(os_sys_calls, write_(_, _, _)) .InSequence(sq) @@ -253,7 +257,7 @@ TEST(FileSystemImpl, reopenFile) { return num_bytes; })); - file.write("before"); + file->write("before"); timer->callback_(); { @@ -278,8 +282,8 @@ TEST(FileSystemImpl, reopenFile) { EXPECT_CALL(os_sys_calls, close(10)).InSequence(sq); - file.reopen(); - file.write("reopened"); + file->reopen(); + file->write("reopened"); timer->callback_(); { @@ -290,7 +294,7 @@ TEST(FileSystemImpl, reopenFile) { } } -TEST(FilesystemImpl, reopenThrows) { +TEST_F(FileSystemImplTest, reopenThrows) { NiceMock dispatcher; NiceMock* timer = new NiceMock(&dispatcher); @@ -310,11 +314,11 @@ TEST(FilesystemImpl, reopenThrows) { Sequence sq; EXPECT_CALL(os_sys_calls, open_(_, _, _)).InSequence(sq).WillOnce(Return(5)); - Filesystem::FileImpl file("", dispatcher, mutex, stats_store, std::chrono::milliseconds(40)); + Filesystem::FileSharedPtr file = file_system_.createFile("", dispatcher, mutex, timeout_40ms_); EXPECT_CALL(os_sys_calls, close(5)).InSequence(sq); EXPECT_CALL(os_sys_calls, open_(_, _, _)).InSequence(sq).WillOnce(Return(-1)); - file.write("test write"); + file->write("test write"); timer->callback_(); { Thread::LockGuard lock(os_sys_calls.write_mutex_); @@ -322,9 +326,9 @@ TEST(FilesystemImpl, reopenThrows) { os_sys_calls.write_event_.wait(os_sys_calls.write_mutex_); } } - file.reopen(); + file->reopen(); - file.write("this is to force reopen"); + file->write("this is to force reopen"); timer->callback_(); { @@ -335,18 +339,18 @@ TEST(FilesystemImpl, reopenThrows) { } // write call should not cause any exceptions - file.write("random data"); + file->write("random data"); timer->callback_(); } -TEST(FilesystemImpl, bigDataChunkShouldBeFlushedWithoutTimer) { +TEST_F(FileSystemImplTest, bigDataChunkShouldBeFlushedWithoutTimer) { NiceMock dispatcher; Thread::MutexBasicLockable mutex; Stats::IsolatedStoreImpl stats_store; NiceMock os_sys_calls; TestThreadsafeSingletonInjector os_calls(&os_sys_calls); - Filesystem::FileImpl file("", dispatcher, mutex, stats_store, std::chrono::milliseconds(40)); + Filesystem::FileSharedPtr file = file_system_.createFile("", dispatcher, mutex, timeout_40ms_); EXPECT_CALL(os_sys_calls, write_(_, _, _)) .WillOnce(Invoke([](int fd, const void* buffer, size_t num_bytes) -> ssize_t { @@ -359,7 +363,7 @@ TEST(FilesystemImpl, bigDataChunkShouldBeFlushedWithoutTimer) { return num_bytes; })); - file.write("a"); + file->write("a"); { Thread::LockGuard lock(os_sys_calls.write_mutex_); @@ -382,7 +386,7 @@ TEST(FilesystemImpl, bigDataChunkShouldBeFlushedWithoutTimer) { })); std::string big_string(1024 * 64 + 1, 'b'); - file.write(big_string); + file->write(big_string); { Thread::LockGuard lock(os_sys_calls.write_mutex_); diff --git a/test/integration/echo_integration_test.cc b/test/integration/echo_integration_test.cc index c23dd42c25f3b..c5bb013d89024 100644 --- a/test/integration/echo_integration_test.cc +++ b/test/integration/echo_integration_test.cc @@ -55,7 +55,7 @@ TEST_P(EchoIntegrationTest, Hello) { response.append(data.toString()); connection.close(); }, - version_); + version_, stats_store_); connection.run(); EXPECT_EQ("hello", response); @@ -104,7 +104,7 @@ TEST_P(EchoIntegrationTest, AddRemoveListener) { response.append(data.toString()); connection.close(); }, - version_); + version_, stats_store_); connection.run(); EXPECT_EQ("hello", response); @@ -129,7 +129,8 @@ TEST_P(EchoIntegrationTest, AddRemoveListener) { for (int i = 0; i < 10; ++i) { RawConnectionDriver connection2( new_listener_port, buffer, - [&](Network::ClientConnection&, const Buffer::Instance&) -> void { FAIL(); }, version_); + [&](Network::ClientConnection&, const Buffer::Instance&) -> void { FAIL(); }, version_, + stats_store_); while (connection2.connecting()) { // Don't busy loop, but OS X often needs a moment to decide this connection isn't happening. timeSystem().sleep(std::chrono::milliseconds(10)); diff --git a/test/integration/fake_upstream.cc b/test/integration/fake_upstream.cc index 38f46fc532a57..90db0e1381109 100644 --- a/test/integration/fake_upstream.cc +++ b/test/integration/fake_upstream.cc @@ -364,8 +364,9 @@ FakeUpstream::FakeUpstream(Network::TransportSocketFactoryPtr&& transport_socket FakeUpstream::FakeUpstream(Network::TransportSocketFactoryPtr&& transport_socket_factory, Network::SocketPtr&& listen_socket, FakeHttpConnection::Type type, Event::TestTimeSystem& time_system, bool enable_half_close) - : http_type_(type), socket_(std::move(listen_socket)), api_(new Api::Impl(milliseconds(10000))), - time_system_(time_system), dispatcher_(api_->allocateDispatcher(time_system_)), + : http_type_(type), socket_(std::move(listen_socket)), + api_(new Api::Impl(milliseconds(10000), stats_store_)), time_system_(time_system), + dispatcher_(api_->allocateDispatcher(time_system_)), handler_(new Server::ConnectionHandlerImpl(ENVOY_LOGGER(), *dispatcher_)), allow_unexpected_disconnects_(false), enable_half_close_(enable_half_close), listener_(*this), filter_chain_(Network::Test::createEmptyFilterChain(std::move(transport_socket_factory))) { diff --git a/test/integration/http2_integration_test.cc b/test/integration/http2_integration_test.cc index 3c4dd561b407b..e0ab532b707b7 100644 --- a/test/integration/http2_integration_test.cc +++ b/test/integration/http2_integration_test.cc @@ -145,7 +145,7 @@ TEST_P(Http2IntegrationTest, BadMagic) { [&](Network::ClientConnection&, const Buffer::Instance& data) -> void { response.append(data.toString()); }, - version_); + version_, stats_store_); connection.run(); EXPECT_EQ("", response); @@ -160,7 +160,7 @@ TEST_P(Http2IntegrationTest, BadFrame) { [&](Network::ClientConnection&, const Buffer::Instance& data) -> void { response.append(data.toString()); }, - version_); + version_, stats_store_); connection.run(); EXPECT_TRUE(response.find("SETTINGS expected") != std::string::npos); @@ -405,7 +405,7 @@ TEST_P(Http2IntegrationTest, DelayedCloseAfterBadFrame) { response.append(data.toString()); connection.dispatcher().exit(); }, - version_); + version_, stats_store_); connection.run(); EXPECT_THAT(response, HasSubstr("SETTINGS expected")); @@ -436,7 +436,7 @@ TEST_P(Http2IntegrationTest, DelayedCloseDisabled) { response.append(data.toString()); connection.dispatcher().exit(); }, - version_); + version_, stats_store_); connection.run(); EXPECT_THAT(response, HasSubstr("SETTINGS expected")); diff --git a/test/integration/integration.cc b/test/integration/integration.cc index 51bca91c44589..82eebdaa74af6 100644 --- a/test/integration/integration.cc +++ b/test/integration/integration.cc @@ -213,11 +213,11 @@ void IntegrationTcpClient::ConnectionCallbacks::onEvent(Network::ConnectionEvent BaseIntegrationTest::BaseIntegrationTest(Network::Address::IpVersion version, TestTimeSystemPtr time_system, const std::string& config) - : api_(new Api::Impl(std::chrono::milliseconds(10000))), - mock_buffer_factory_(new NiceMock), time_system_(std::move(time_system)), + : mock_buffer_factory_(new NiceMock), time_system_(std::move(time_system)), dispatcher_(new Event::DispatcherImpl(*time_system_, Buffer::WatermarkFactoryPtr{mock_buffer_factory_})), - version_(version), config_helper_(version, config), + api_(new Api::Impl(std::chrono::milliseconds(10000), stats_store_)), version_(version), + config_helper_(version, config), default_log_level_(TestEnvironment::getOptions().logLevel()) { // This is a hack, but there are situations where we disconnect fake upstream connections and // then we expect the server connection pool to get the disconnect before the next test starts. @@ -390,7 +390,7 @@ void BaseIntegrationTest::sendRawHttpAndWaitForResponse(int port, const char* ra client.close(Network::ConnectionCloseType::NoFlush); } }, - version_); + version_, stats_store_); connection.run(); } diff --git a/test/integration/integration.h b/test/integration/integration.h index f551a9c1badef..1bb7dffceb45e 100644 --- a/test/integration/integration.h +++ b/test/integration/integration.h @@ -12,6 +12,7 @@ #include "test/integration/server.h" #include "test/integration/utility.h" #include "test/mocks/buffer/mocks.h" +#include "test/mocks/stats/mocks.h" #include "test/test_common/environment.h" #include "test/test_common/printers.h" #include "test/test_common/simulated_time_system.h" @@ -183,13 +184,14 @@ class BaseIntegrationTest : Logger::Loggable { Event::TestTimeSystem& timeSystem() { return *time_system_; } - Api::ApiPtr api_; MockBufferFactory* mock_buffer_factory_; // Will point to the dispatcher's factory. private: TestTimeSystemPtr time_system_; public: Event::DispatcherPtr dispatcher_; + Stats::MockIsolatedStatsStore stats_store_; + Api::ApiPtr api_; /** * Open a connection to Envoy, send a series of bytes, and return the diff --git a/test/integration/integration_admin_test.cc b/test/integration/integration_admin_test.cc index 639098fa7eb45..0f84db42b901c 100644 --- a/test/integration/integration_admin_test.cc +++ b/test/integration/integration_admin_test.cc @@ -519,8 +519,7 @@ TEST_P(StatsMatcherIntegrationTest, IncludeExact) { "listener_manager.listener_create_success"); initialize(); makeRequest(); - EXPECT_THAT(response_->body(), - testing::Eq("listener_manager.listener_create_success: 1\nstats.overflow: 0\n")); + EXPECT_EQ(response_->body(), "listener_manager.listener_create_success: 1\n"); } } // namespace Envoy diff --git a/test/integration/utility.cc b/test/integration/utility.cc index ef4166830c893..6c79c3a939f27 100644 --- a/test/integration/utility.cc +++ b/test/integration/utility.cc @@ -18,6 +18,7 @@ #include "common/upstream/upstream_impl.h" #include "test/common/upstream/utility.h" +#include "test/mocks/stats/mocks.h" #include "test/mocks/upstream/mocks.h" #include "test/test_common/network_utility.h" #include "test/test_common/printers.h" @@ -61,7 +62,8 @@ IntegrationUtil::makeSingleRequest(const Network::Address::InstanceConstSharedPt const std::string& body, Http::CodecClient::Type type, const std::string& host, const std::string& content_type) { - Api::Impl api(std::chrono::milliseconds(9000)); + NiceMock mock_stats_store; + Api::Impl api(std::chrono::milliseconds(9000), mock_stats_store); Event::DispatcherPtr dispatcher(api.allocateDispatcher(evil_singleton_test_time_.timeSystem())); std::shared_ptr cluster{new NiceMock()}; Upstream::HostDescriptionConstSharedPtr host_description{ @@ -108,8 +110,9 @@ IntegrationUtil::makeSingleRequest(uint32_t port, const std::string& method, con RawConnectionDriver::RawConnectionDriver(uint32_t port, Buffer::Instance& initial_data, ReadCallback data_callback, - Network::Address::IpVersion version) { - api_ = std::make_unique(std::chrono::milliseconds(10000)); + Network::Address::IpVersion version, + Stats::Store& stats_store) { + api_ = std::make_unique(std::chrono::milliseconds(10000), stats_store); dispatcher_ = api_->allocateDispatcher(IntegrationUtil::evil_singleton_test_time_.timeSystem()); callbacks_ = std::make_unique(); client_ = dispatcher_->createClientConnection( diff --git a/test/integration/utility.h b/test/integration/utility.h index 2740b87463fc8..00ed4d45bc6df 100644 --- a/test/integration/utility.h +++ b/test/integration/utility.h @@ -60,7 +60,7 @@ class RawConnectionDriver { typedef std::function ReadCallback; RawConnectionDriver(uint32_t port, Buffer::Instance& initial_data, ReadCallback data_callback, - Network::Address::IpVersion version); + Network::Address::IpVersion version, Stats::Store& stat_store); ~RawConnectionDriver(); const Network::Connection& connection() { return *client_; } bool connecting() { return callbacks_->connecting_; } diff --git a/test/mocks/api/mocks.cc b/test/mocks/api/mocks.cc index 2a577efed1156..626a791d723c9 100644 --- a/test/mocks/api/mocks.cc +++ b/test/mocks/api/mocks.cc @@ -12,7 +12,7 @@ using testing::Return; namespace Envoy { namespace Api { -MockApi::MockApi() { ON_CALL(*this, createFile(_, _, _, _)).WillByDefault(Return(file_)); } +MockApi::MockApi() { ON_CALL(*this, createFile(_, _, _)).WillByDefault(Return(file_)); } MockApi::~MockApi() {} diff --git a/test/mocks/api/mocks.h b/test/mocks/api/mocks.h index 4f0ae32a9bdfb..d033b68b2c662 100644 --- a/test/mocks/api/mocks.h +++ b/test/mocks/api/mocks.h @@ -30,9 +30,9 @@ class MockApi : public Api { } MOCK_METHOD1(allocateDispatcher_, Event::Dispatcher*(Event::TimeSystem&)); - MOCK_METHOD4(createFile, + MOCK_METHOD3(createFile, Filesystem::FileSharedPtr(const std::string& path, Event::Dispatcher& dispatcher, - Thread::BasicLockable& lock, Stats::Store& stats_store)); + Thread::BasicLockable& lock)); MOCK_METHOD1(fileExists, bool(const std::string& path)); MOCK_METHOD1(fileReadToEnd, std::string(const std::string& path)); diff --git a/test/server/config_validation/BUILD b/test/server/config_validation/BUILD index 2777f1ddfcf59..1f5e4c5fbfa43 100644 --- a/test/server/config_validation/BUILD +++ b/test/server/config_validation/BUILD @@ -72,6 +72,7 @@ envoy_cc_test( srcs = ["dispatcher_test.cc"], deps = [ "//source/common/event:libevent_lib", + "//source/common/stats:isolated_store_lib", "//source/server/config_validation:api_lib", "//source/server/config_validation:dns_lib", "//test/test_common:environment_lib", diff --git a/test/server/config_validation/dispatcher_test.cc b/test/server/config_validation/dispatcher_test.cc index 8f9757bb7e72f..9fb7cac578dc3 100644 --- a/test/server/config_validation/dispatcher_test.cc +++ b/test/server/config_validation/dispatcher_test.cc @@ -4,6 +4,7 @@ #include "common/event/libevent.h" #include "common/network/address_impl.h" #include "common/network/utility.h" +#include "common/stats/isolated_store_impl.h" #include "server/config_validation/api.h" @@ -22,12 +23,14 @@ class ConfigValidation : public ::testing::TestWithParam(std::chrono::milliseconds(1000)); + validation_ = + std::make_unique(std::chrono::milliseconds(1000), stats_store_); dispatcher_ = validation_->allocateDispatcher(test_time_.timeSystem()); } DangerousDeprecatedTestTime test_time_; Event::DispatcherPtr dispatcher_; + Stats::IsolatedStoreImpl stats_store_; private: // Using config validation API. From e03c578de168a42628190bcf408f4a57b9b0a56b Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Tue, 13 Nov 2018 23:11:32 -0500 Subject: [PATCH 2/8] add comments and use new rejects() layer. Signed-off-by: Joshua Marantz --- source/common/filesystem/filesystem_impl.h | 21 ++++++++++++++++++++- source/common/stats/thread_local_store.cc | 8 ++++---- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/source/common/filesystem/filesystem_impl.h b/source/common/filesystem/filesystem_impl.h index 2b43b4a263baf..2a2ce9599cbbf 100644 --- a/source/common/filesystem/filesystem_impl.h +++ b/source/common/filesystem/filesystem_impl.h @@ -31,13 +31,32 @@ struct FileSystemStats { namespace Filesystem { +/** + * Captures state, properties, and stats of a file-system. + */ class Instance { public: - explicit Instance(std::chrono::milliseconds file_flush_interval_msec, Stats::Store& store); + Instance(std::chrono::milliseconds file_flush_interval_msec, Stats::Store& store); + /** + * Creates a file, overriding the flush-interval set in the class. + * + * @param path The path of the file to open. + * @param dispatcher The dispatcher used for set up timers to run flush(). + * @param lock The lock. + * @param file_flush_interval_msec Number of milliseconds to delay before flushing. + */ FileSharedPtr createFile(const std::string& path, Event::Dispatcher& dispatcher, Thread::BasicLockable& lock, std::chrono::milliseconds file_flush_interval_msec); + + /** + * Creates a file, using the default flush-interval for the class.. + * + * @param path The path of the file to open. + * @param dispatcher The dispatcher used for set up timers to run flush(). + * @param lock The lock. + */ FileSharedPtr createFile(const std::string& path, Event::Dispatcher& dispatcher, Thread::BasicLockable& lock) { return createFile(path, dispatcher, lock, file_flush_interval_msec_); diff --git a/source/common/stats/thread_local_store.cc b/source/common/stats/thread_local_store.cc index 0327387412884..da6bfa0462bf7 100644 --- a/source/common/stats/thread_local_store.cc +++ b/source/common/stats/thread_local_store.cc @@ -296,7 +296,7 @@ Counter& ThreadLocalStoreImpl::ScopeImpl::counter(const std::string& name) { // TODO(ambuc): If stats_matcher_ depends on regexes, this operation (on the hot path) could // become prohibitively expensive. Revisit this usage in the future. - if (parent_.stats_matcher_->rejects(final_name)) { + if (parent_.rejects(final_name.c_str())) { return null_counter_; } @@ -344,7 +344,7 @@ Gauge& ThreadLocalStoreImpl::ScopeImpl::gauge(const std::string& name) { std::string final_name = prefix_ + name; // See warning/comments in counter(). - if (parent_.stats_matcher_->rejects(final_name)) { + if (parent_.rejects(final_name.c_str())) { return null_gauge_; } @@ -374,7 +374,7 @@ Histogram& ThreadLocalStoreImpl::ScopeImpl::histogram(const std::string& name) { std::string final_name = prefix_ + name; // See warning/comments in counter(). - if (parent_.stats_matcher_->rejects(final_name)) { + if (parent_.rejects(final_name.c_str())) { return null_histogram_; } @@ -410,7 +410,7 @@ Histogram& ThreadLocalStoreImpl::ScopeImpl::histogram(const std::string& name) { Histogram& ThreadLocalStoreImpl::ScopeImpl::tlsHistogram(const std::string& name, ParentHistogramImpl& parent) { - if (parent_.stats_matcher_->rejects(name)) { + if (parent_.rejects(name.c_str())) { return null_histogram_; } From 5e9f0b2f21d360fd47112fe634b9261d9e5abb03 Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Wed, 14 Nov 2018 14:06:29 -0500 Subject: [PATCH 3/8] hack out API from base integration infra for now. Signed-off-by: Joshua Marantz --- test/integration/integration.cc | 5 ++--- test/integration/integration.h | 3 --- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/test/integration/integration.cc b/test/integration/integration.cc index 82eebdaa74af6..83838dab647a6 100644 --- a/test/integration/integration.cc +++ b/test/integration/integration.cc @@ -216,8 +216,7 @@ BaseIntegrationTest::BaseIntegrationTest(Network::Address::IpVersion version, : mock_buffer_factory_(new NiceMock), time_system_(std::move(time_system)), dispatcher_(new Event::DispatcherImpl(*time_system_, Buffer::WatermarkFactoryPtr{mock_buffer_factory_})), - api_(new Api::Impl(std::chrono::milliseconds(10000), stats_store_)), version_(version), - config_helper_(version, config), + version_(version), config_helper_(version, config), default_log_level_(TestEnvironment::getOptions().logLevel()) { // This is a hack, but there are situations where we disconnect fake upstream connections and // then we expect the server connection pool to get the disconnect before the next test starts. @@ -390,7 +389,7 @@ void BaseIntegrationTest::sendRawHttpAndWaitForResponse(int port, const char* ra client.close(Network::ConnectionCloseType::NoFlush); } }, - version_, stats_store_); + version_); connection.run(); } diff --git a/test/integration/integration.h b/test/integration/integration.h index 1bb7dffceb45e..4aa5e090585d8 100644 --- a/test/integration/integration.h +++ b/test/integration/integration.h @@ -12,7 +12,6 @@ #include "test/integration/server.h" #include "test/integration/utility.h" #include "test/mocks/buffer/mocks.h" -#include "test/mocks/stats/mocks.h" #include "test/test_common/environment.h" #include "test/test_common/printers.h" #include "test/test_common/simulated_time_system.h" @@ -190,8 +189,6 @@ class BaseIntegrationTest : Logger::Loggable { public: Event::DispatcherPtr dispatcher_; - Stats::MockIsolatedStatsStore stats_store_; - Api::ApiPtr api_; /** * Open a connection to Envoy, send a series of bytes, and return the From 14918d00380fb731e4e854881227aa3806c77934 Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Wed, 14 Nov 2018 15:20:53 -0500 Subject: [PATCH 4/8] Revert the RawConnectionDriver to avoid taking a Stats::Store and instead make an IsolatedStatsStore. This should avoid the problems copmiling echo2_integration_test.cc in a separate repo. Signed-off-by: Joshua Marantz --- test/integration/echo_integration_test.cc | 7 +++---- test/integration/http2_integration_test.cc | 8 ++++---- test/integration/utility.cc | 5 ++--- test/integration/utility.h | 4 +++- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/test/integration/echo_integration_test.cc b/test/integration/echo_integration_test.cc index c5bb013d89024..c23dd42c25f3b 100644 --- a/test/integration/echo_integration_test.cc +++ b/test/integration/echo_integration_test.cc @@ -55,7 +55,7 @@ TEST_P(EchoIntegrationTest, Hello) { response.append(data.toString()); connection.close(); }, - version_, stats_store_); + version_); connection.run(); EXPECT_EQ("hello", response); @@ -104,7 +104,7 @@ TEST_P(EchoIntegrationTest, AddRemoveListener) { response.append(data.toString()); connection.close(); }, - version_, stats_store_); + version_); connection.run(); EXPECT_EQ("hello", response); @@ -129,8 +129,7 @@ TEST_P(EchoIntegrationTest, AddRemoveListener) { for (int i = 0; i < 10; ++i) { RawConnectionDriver connection2( new_listener_port, buffer, - [&](Network::ClientConnection&, const Buffer::Instance&) -> void { FAIL(); }, version_, - stats_store_); + [&](Network::ClientConnection&, const Buffer::Instance&) -> void { FAIL(); }, version_); while (connection2.connecting()) { // Don't busy loop, but OS X often needs a moment to decide this connection isn't happening. timeSystem().sleep(std::chrono::milliseconds(10)); diff --git a/test/integration/http2_integration_test.cc b/test/integration/http2_integration_test.cc index e0ab532b707b7..3c4dd561b407b 100644 --- a/test/integration/http2_integration_test.cc +++ b/test/integration/http2_integration_test.cc @@ -145,7 +145,7 @@ TEST_P(Http2IntegrationTest, BadMagic) { [&](Network::ClientConnection&, const Buffer::Instance& data) -> void { response.append(data.toString()); }, - version_, stats_store_); + version_); connection.run(); EXPECT_EQ("", response); @@ -160,7 +160,7 @@ TEST_P(Http2IntegrationTest, BadFrame) { [&](Network::ClientConnection&, const Buffer::Instance& data) -> void { response.append(data.toString()); }, - version_, stats_store_); + version_); connection.run(); EXPECT_TRUE(response.find("SETTINGS expected") != std::string::npos); @@ -405,7 +405,7 @@ TEST_P(Http2IntegrationTest, DelayedCloseAfterBadFrame) { response.append(data.toString()); connection.dispatcher().exit(); }, - version_, stats_store_); + version_); connection.run(); EXPECT_THAT(response, HasSubstr("SETTINGS expected")); @@ -436,7 +436,7 @@ TEST_P(Http2IntegrationTest, DelayedCloseDisabled) { response.append(data.toString()); connection.dispatcher().exit(); }, - version_, stats_store_); + version_); connection.run(); EXPECT_THAT(response, HasSubstr("SETTINGS expected")); diff --git a/test/integration/utility.cc b/test/integration/utility.cc index 6c79c3a939f27..a04561e564679 100644 --- a/test/integration/utility.cc +++ b/test/integration/utility.cc @@ -110,9 +110,8 @@ IntegrationUtil::makeSingleRequest(uint32_t port, const std::string& method, con RawConnectionDriver::RawConnectionDriver(uint32_t port, Buffer::Instance& initial_data, ReadCallback data_callback, - Network::Address::IpVersion version, - Stats::Store& stats_store) { - api_ = std::make_unique(std::chrono::milliseconds(10000), stats_store); + Network::Address::IpVersion version) { + api_ = std::make_unique(std::chrono::milliseconds(10000), stats_store_); dispatcher_ = api_->allocateDispatcher(IntegrationUtil::evil_singleton_test_time_.timeSystem()); callbacks_ = std::make_unique(); client_ = dispatcher_->createClientConnection( diff --git a/test/integration/utility.h b/test/integration/utility.h index 00ed4d45bc6df..23630096ef289 100644 --- a/test/integration/utility.h +++ b/test/integration/utility.h @@ -13,6 +13,7 @@ #include "common/common/assert.h" #include "common/common/utility.h" #include "common/http/codec_client.h" +#include "common/stats/isolated_store_impl.h" #include "test/test_common/printers.h" #include "test/test_common/test_time.h" @@ -60,7 +61,7 @@ class RawConnectionDriver { typedef std::function ReadCallback; RawConnectionDriver(uint32_t port, Buffer::Instance& initial_data, ReadCallback data_callback, - Network::Address::IpVersion version, Stats::Store& stat_store); + Network::Address::IpVersion version); ~RawConnectionDriver(); const Network::Connection& connection() { return *client_; } bool connecting() { return callbacks_->connecting_; } @@ -99,6 +100,7 @@ class RawConnectionDriver { }; Api::ApiPtr api_; + Stats::IsolatedStoreImpl stats_store_; Event::DispatcherPtr dispatcher_; std::unique_ptr callbacks_; Network::ClientConnectionPtr client_; From 820477ff7fc3b152e2317a3413c3e93f08675335 Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Fri, 16 Nov 2018 08:32:33 -0500 Subject: [PATCH 5/8] get some tests working. Signed-off-by: Joshua Marantz --- test/common/event/dispatched_thread_impl_test.cc | 2 +- test/exe/main_common_test.cc | 4 +++- test/integration/integration.cc | 7 +++---- test/integration/utility.cc | 4 ++-- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/test/common/event/dispatched_thread_impl_test.cc b/test/common/event/dispatched_thread_impl_test.cc index ed1d8ba29a33f..db2d7893befda 100644 --- a/test/common/event/dispatched_thread_impl_test.cc +++ b/test/common/event/dispatched_thread_impl_test.cc @@ -23,7 +23,7 @@ namespace Event { class DispatchedThreadTest : public testing::Test { protected: DispatchedThreadTest() - : config_(1000, 1000, 1000, 1000), api_(std::chrono::milliseconds(1000)), + : config_(1000, 1000, 1000, 1000), api_(fakestats_, std::chrono::milliseconds(1000)), thread_(api_, test_time_.timeSystem()), guard_dog_(fakestats_, config_, test_time_.timeSystem(), api_) {} diff --git a/test/exe/main_common_test.cc b/test/exe/main_common_test.cc index 3f7ea52e04698..2f9357680b334 100644 --- a/test/exe/main_common_test.cc +++ b/test/exe/main_common_test.cc @@ -136,7 +136,8 @@ INSTANTIATE_TEST_CASE_P(IpVersions, MainCommonTest, class AdminRequestTest : public MainCommonTest { protected: AdminRequestTest() - : envoy_return_(false), envoy_started_(false), envoy_finished_(false), + : api_(stats_store_), + envoy_return_(false), envoy_started_(false), envoy_finished_(false), pause_before_run_(false), pause_after_run_(false) { addArg("--disable-hot-restart"); } @@ -194,6 +195,7 @@ class AdminRequestTest : public MainCommonTest { } Api::Impl api_; + Stats::IsolatedStoreImpl stats_store_; std::unique_ptr envoy_thread_; std::unique_ptr main_common_; absl::Notification started_; diff --git a/test/integration/integration.cc b/test/integration/integration.cc index 5377f059c444c..b4cb1e86782dd 100644 --- a/test/integration/integration.cc +++ b/test/integration/integration.cc @@ -350,7 +350,7 @@ void BaseIntegrationTest::createGeneratedApiTestServer(const std::string& bootst test_server_->waitForCounterGe("listener_manager.listener_create_success", 1); registerTestServerPorts(port_names); } - api_ = std::make_unique(test_server_->server_.stats()); + api_ = std::make_unique(test_server_->stat_store()); } void BaseIntegrationTest::createApiTestServer(const ApiFilesystemConfig& api_filesystem_config, @@ -398,10 +398,9 @@ void BaseIntegrationTest::sendRawHttpAndWaitForResponse(int port, const char* ra IntegrationTestServerPtr BaseIntegrationTest::createIntegrationTestServer(const std::string& bootstrap_path, std::function pre_worker_start_test_steps, - Event::TestTimeSystem& time_system, - Api::Api& api) { + Event::TestTimeSystem& time_system) { return IntegrationTestServer::create(bootstrap_path, version_, pre_worker_start_test_steps, - deterministic_, time_system, *api); + deterministic_, time_system, *api_); } } // namespace Envoy diff --git a/test/integration/utility.cc b/test/integration/utility.cc index a04561e564679..a882679889ed2 100644 --- a/test/integration/utility.cc +++ b/test/integration/utility.cc @@ -63,7 +63,7 @@ IntegrationUtil::makeSingleRequest(const Network::Address::InstanceConstSharedPt const std::string& host, const std::string& content_type) { NiceMock mock_stats_store; - Api::Impl api(std::chrono::milliseconds(9000), mock_stats_store); + Api::Impl api(mock_stats_store, std::chrono::milliseconds(9000)); Event::DispatcherPtr dispatcher(api.allocateDispatcher(evil_singleton_test_time_.timeSystem())); std::shared_ptr cluster{new NiceMock()}; Upstream::HostDescriptionConstSharedPtr host_description{ @@ -111,7 +111,7 @@ IntegrationUtil::makeSingleRequest(uint32_t port, const std::string& method, con RawConnectionDriver::RawConnectionDriver(uint32_t port, Buffer::Instance& initial_data, ReadCallback data_callback, Network::Address::IpVersion version) { - api_ = std::make_unique(std::chrono::milliseconds(10000), stats_store_); + api_ = std::make_unique(stats_store_, std::chrono::milliseconds(10000)); dispatcher_ = api_->allocateDispatcher(IntegrationUtil::evil_singleton_test_time_.timeSystem()); callbacks_ = std::make_unique(); client_ = dispatcher_->createClientConnection( From 27cd0c1c260e0310b6480503905d8fce7776593e Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Wed, 21 Nov 2018 09:38:56 -0500 Subject: [PATCH 6/8] format Signed-off-by: Joshua Marantz --- source/common/api/api_impl.cc | 3 +-- source/common/filesystem/filesystem_impl.h | 4 ++-- test/common/grpc/google_async_client_impl_test.cc | 9 ++++----- test/config_test/config_test.cc | 4 ++-- test/exe/main_common_test.cc | 3 +-- test/server/config_validation/dispatcher_test.cc | 5 ++--- 6 files changed, 12 insertions(+), 16 deletions(-) diff --git a/source/common/api/api_impl.cc b/source/common/api/api_impl.cc index f3251f949aba1..0fec894a38276 100644 --- a/source/common/api/api_impl.cc +++ b/source/common/api/api_impl.cc @@ -11,8 +11,7 @@ namespace Envoy { namespace Api { Impl::Impl(std::chrono::milliseconds file_flush_interval_msec, - Thread::ThreadFactory& thread_factory, - Stats::Store& stats_store) + Thread::ThreadFactory& thread_factory, Stats::Store& stats_store) : thread_factory_(thread_factory), file_system_(file_flush_interval_msec, thread_factory, stats_store) {} diff --git a/source/common/filesystem/filesystem_impl.h b/source/common/filesystem/filesystem_impl.h index 9670f069a8933..eaf8b6a564fce 100644 --- a/source/common/filesystem/filesystem_impl.h +++ b/source/common/filesystem/filesystem_impl.h @@ -37,8 +37,8 @@ namespace Filesystem { */ class Instance { public: - Instance(std::chrono::milliseconds file_flush_interval_msec, Thread::ThreadFactory& thread_factory, - Stats::Store& store); + Instance(std::chrono::milliseconds file_flush_interval_msec, + Thread::ThreadFactory& thread_factory, Stats::Store& store); /** * Creates a file, overriding the flush-interval set in the class. diff --git a/test/common/grpc/google_async_client_impl_test.cc b/test/common/grpc/google_async_client_impl_test.cc index c62e32fd730d9..4b9ea5fa299be 100644 --- a/test/common/grpc/google_async_client_impl_test.cc +++ b/test/common/grpc/google_async_client_impl_test.cc @@ -47,8 +47,7 @@ class MockStubFactory : public GoogleStubFactory { class EnvoyGoogleAsyncClientImplTest : public testing::Test { public: EnvoyGoogleAsyncClientImplTest() - : dispatcher_(test_time_.timeSystem()), - stats_store_(new Stats::IsolatedStoreImpl), + : dispatcher_(test_time_.timeSystem()), stats_store_(new Stats::IsolatedStoreImpl), api_(Api::createApiForTest(*stats_store_)), scope_(stats_store_), method_descriptor_(helloworld::Greeter::descriptor()->FindMethodByName("SayHello")) { envoy::api::v2::core::GrpcService config; @@ -56,13 +55,13 @@ class EnvoyGoogleAsyncClientImplTest : public testing::Test { google_grpc->set_target_uri("fake_address"); google_grpc->set_stat_prefix("test_cluster"); tls_ = std::make_unique(*api_); - grpc_client_ = std::make_unique(dispatcher_, *tls_, stub_factory_, - scope_, config); + grpc_client_ = + std::make_unique(dispatcher_, *tls_, stub_factory_, scope_, config); } DangerousDeprecatedTestTime test_time_; Event::DispatcherImpl dispatcher_; - Stats::IsolatedStoreImpl* stats_store_; // Ownership transerred to scope_. + Stats::IsolatedStoreImpl* stats_store_; // Ownership transerred to scope_. Api::ApiPtr api_; Stats::ScopeSharedPtr scope_; std::unique_ptr tls_; diff --git a/test/config_test/config_test.cc b/test/config_test/config_test.cc index a28a896ca54ba..770557df9717a 100644 --- a/test/config_test/config_test.cc +++ b/test/config_test/config_test.cc @@ -42,8 +42,8 @@ OptionsImpl asConfigYaml(const OptionsImpl& src) { class ConfigTest { public: - ConfigTest(const OptionsImpl& options) : api_(Api::createApiForTest(stats_store_)), - options_(options) { + ConfigTest(const OptionsImpl& options) + : api_(Api::createApiForTest(stats_store_)), options_(options) { ON_CALL(server_, options()).WillByDefault(ReturnRef(options_)); ON_CALL(server_, random()).WillByDefault(ReturnRef(random_)); ON_CALL(server_, sslContextManager()).WillByDefault(ReturnRef(ssl_context_manager_)); diff --git a/test/exe/main_common_test.cc b/test/exe/main_common_test.cc index ce84ae7f1b5e7..f4f4235bc7ab8 100644 --- a/test/exe/main_common_test.cc +++ b/test/exe/main_common_test.cc @@ -135,8 +135,7 @@ INSTANTIATE_TEST_CASE_P(IpVersions, MainCommonTest, class AdminRequestTest : public MainCommonTest { protected: AdminRequestTest() - : api_(stats_store_), - envoy_return_(false), envoy_started_(false), envoy_finished_(false), + : api_(stats_store_), envoy_return_(false), envoy_started_(false), envoy_finished_(false), pause_before_run_(false), pause_after_run_(false) { addArg("--disable-hot-restart"); } diff --git a/test/server/config_validation/dispatcher_test.cc b/test/server/config_validation/dispatcher_test.cc index 920f97f99398f..0c2474d9a5897 100644 --- a/test/server/config_validation/dispatcher_test.cc +++ b/test/server/config_validation/dispatcher_test.cc @@ -24,9 +24,8 @@ class ConfigValidation : public ::testing::TestWithParam(std::chrono::milliseconds(1000), - Thread::threadFactoryForTest(), - stats_store_); + validation_ = std::make_unique( + std::chrono::milliseconds(1000), Thread::threadFactoryForTest(), stats_store_); dispatcher_ = validation_->allocateDispatcher(test_time_.timeSystem()); } From 389561e9bda086074b8c3c3570180ba6fc2fbf09 Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Wed, 21 Nov 2018 10:44:07 -0500 Subject: [PATCH 7/8] all tests working. Signed-off-by: Joshua Marantz --- source/common/filesystem/filesystem_impl.h | 2 +- source/common/stats/thread_local_store.cc | 10 +++++----- source/common/stats/thread_local_store.h | 2 +- test/config_test/config_test.cc | 2 +- test/exe/main_common_test.cc | 3 +-- test/integration/integration.cc | 4 ++-- test/integration/integration.h | 1 + test/server/config_validation/cluster_manager_test.cc | 4 ++-- 8 files changed, 14 insertions(+), 14 deletions(-) diff --git a/source/common/filesystem/filesystem_impl.h b/source/common/filesystem/filesystem_impl.h index eaf8b6a564fce..f133e0e5d31e3 100644 --- a/source/common/filesystem/filesystem_impl.h +++ b/source/common/filesystem/filesystem_impl.h @@ -53,7 +53,7 @@ class Instance { std::chrono::milliseconds file_flush_interval_msec); /** - * Creates a file, using the default flush-interval for the class.. + * Creates a file, using the default flush-interval for the class. * * @param path The path of the file to open. * @param dispatcher The dispatcher used for set up timers to run flush(). diff --git a/source/common/stats/thread_local_store.cc b/source/common/stats/thread_local_store.cc index b3e5c7fefe2b9..1a5eb59a02083 100644 --- a/source/common/stats/thread_local_store.cc +++ b/source/common/stats/thread_local_store.cc @@ -65,7 +65,7 @@ void ThreadLocalStoreImpl::removeRejectedStats(StatMapClass& map, StatListClass& } } -bool ThreadLocalStoreImpl::rejects(const char* name) const { +bool ThreadLocalStoreImpl::rejects(const std::string& name) const { // TODO(ambuc): If stats_matcher_ depends on regexes, this operation (on the // hot path) could become prohibitively expensive. Revisit this usage in the // future. @@ -302,7 +302,7 @@ Counter& ThreadLocalStoreImpl::ScopeImpl::counter(const std::string& name) { // TODO(ambuc): If stats_matcher_ depends on regexes, this operation (on the hot path) could // become prohibitively expensive. Revisit this usage in the future. - if (parent_.rejects(final_name.c_str())) { + if (parent_.rejects(final_name)) { return null_counter_; } @@ -350,7 +350,7 @@ Gauge& ThreadLocalStoreImpl::ScopeImpl::gauge(const std::string& name) { std::string final_name = prefix_ + name; // See warning/comments in counter(). - if (parent_.rejects(final_name.c_str())) { + if (parent_.rejects(final_name)) { return null_gauge_; } @@ -380,7 +380,7 @@ Histogram& ThreadLocalStoreImpl::ScopeImpl::histogram(const std::string& name) { std::string final_name = prefix_ + name; // See warning/comments in counter(). - if (parent_.rejects(final_name.c_str())) { + if (parent_.rejects(final_name)) { return null_histogram_; } @@ -416,7 +416,7 @@ Histogram& ThreadLocalStoreImpl::ScopeImpl::histogram(const std::string& name) { Histogram& ThreadLocalStoreImpl::ScopeImpl::tlsHistogram(const std::string& name, ParentHistogramImpl& parent) { - if (parent_.rejects(name.c_str())) { + if (parent_.rejects(name)) { return null_histogram_; } diff --git a/source/common/stats/thread_local_store.h b/source/common/stats/thread_local_store.h index 99f85cb9455e2..9bc2d7aacb4eb 100644 --- a/source/common/stats/thread_local_store.h +++ b/source/common/stats/thread_local_store.h @@ -252,7 +252,7 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo void releaseScopeCrossThread(ScopeImpl* scope); void mergeInternal(PostMergeCb mergeCb); absl::string_view truncateStatNameIfNeeded(absl::string_view name); - bool rejects(const char* name) const; + bool rejects(const std::string& name) const; template void removeRejectedStats(StatMapClass& map, StatListClass& list); diff --git a/test/config_test/config_test.cc b/test/config_test/config_test.cc index 770557df9717a..93eab2cf3c728 100644 --- a/test/config_test/config_test.cc +++ b/test/config_test/config_test.cc @@ -91,10 +91,10 @@ class ConfigTest { server_.thread_local_.shutdownThread(); } + Stats::IsolatedStoreImpl stats_store_; Api::ApiPtr api_; NiceMock server_; NiceMock ssl_context_manager_; - Stats::IsolatedStoreImpl stats_store_; OptionsImpl options_; std::unique_ptr cluster_manager_factory_; NiceMock component_factory_; diff --git a/test/exe/main_common_test.cc b/test/exe/main_common_test.cc index f4f4235bc7ab8..fb885c85987bf 100644 --- a/test/exe/main_common_test.cc +++ b/test/exe/main_common_test.cc @@ -135,7 +135,7 @@ INSTANTIATE_TEST_CASE_P(IpVersions, MainCommonTest, class AdminRequestTest : public MainCommonTest { protected: AdminRequestTest() - : api_(stats_store_), envoy_return_(false), envoy_started_(false), envoy_finished_(false), + : envoy_return_(false), envoy_started_(false), envoy_finished_(false), pause_before_run_(false), pause_after_run_(false) { addArg("--disable-hot-restart"); } @@ -192,7 +192,6 @@ class AdminRequestTest : public MainCommonTest { return envoy_return_; } - Api::Impl api_; Stats::IsolatedStoreImpl stats_store_; std::unique_ptr envoy_thread_; std::unique_ptr main_common_; diff --git a/test/integration/integration.cc b/test/integration/integration.cc index bc2ac96f34486..85b07ebb8c8fe 100644 --- a/test/integration/integration.cc +++ b/test/integration/integration.cc @@ -213,7 +213,8 @@ void IntegrationTcpClient::ConnectionCallbacks::onEvent(Network::ConnectionEvent BaseIntegrationTest::BaseIntegrationTest(Network::Address::IpVersion version, TestTimeSystemPtr time_system, const std::string& config) - : mock_buffer_factory_(new NiceMock), time_system_(std::move(time_system)), + : api_(Api::createApiForTest(stats_store_)), + mock_buffer_factory_(new NiceMock), time_system_(std::move(time_system)), dispatcher_(new Event::DispatcherImpl(*time_system_, Buffer::WatermarkFactoryPtr{mock_buffer_factory_})), version_(version), config_helper_(version, config), @@ -350,7 +351,6 @@ void BaseIntegrationTest::createGeneratedApiTestServer(const std::string& bootst test_server_->waitForCounterGe("listener_manager.listener_create_success", 1); registerTestServerPorts(port_names); } - api_ = Api::createApiForTest(test_server_->stat_store()); } void BaseIntegrationTest::createApiTestServer(const ApiFilesystemConfig& api_filesystem_config, diff --git a/test/integration/integration.h b/test/integration/integration.h index 535b05d3af225..b637517b3c2b3 100644 --- a/test/integration/integration.h +++ b/test/integration/integration.h @@ -175,6 +175,7 @@ class BaseIntegrationTest : Logger::Loggable { Event::TestTimeSystem& timeSystem() { return *time_system_; } + Stats::IsolatedStoreImpl stats_store_; Api::ApiPtr api_; MockBufferFactory* mock_buffer_factory_; // Will point to the dispatcher's factory. private: diff --git a/test/server/config_validation/cluster_manager_test.cc b/test/server/config_validation/cluster_manager_test.cc index a6059bd98b924..b556a484c287b 100644 --- a/test/server/config_validation/cluster_manager_test.cc +++ b/test/server/config_validation/cluster_manager_test.cc @@ -36,14 +36,14 @@ TEST(ValidationClusterManagerTest, MockedMethods) { LocalInfo::MockLocalInfo local_info; NiceMock admin; - ValidationClusterManagerFactory factory(runtime, stats, tls, random, dns_resolver, + ValidationClusterManagerFactory factory(runtime, stats_store, tls, random, dns_resolver, ssl_context_manager, dispatcher, local_info, secret_manager, *api); AccessLog::MockAccessLogManager log_manager; const envoy::config::bootstrap::v2::Bootstrap bootstrap; ClusterManagerPtr cluster_manager = factory.clusterManagerFromProto( - bootstrap, stats, tls, runtime, random, local_info, log_manager, admin); + bootstrap, stats_store, tls, runtime, random, local_info, log_manager, admin); EXPECT_EQ(nullptr, cluster_manager->httpConnPoolForCluster("cluster", ResourcePriority::Default, Http::Protocol::Http11, nullptr)); Host::CreateConnectionData data = cluster_manager->tcpConnForCluster("cluster", nullptr); From c8a77de9507e40f7bac6331d552540d18a2db25c Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Sat, 24 Nov 2018 13:49:16 -0500 Subject: [PATCH 8/8] declare the default timeout as const. Signed-off-by: Joshua Marantz --- source/common/filesystem/filesystem_impl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/common/filesystem/filesystem_impl.h b/source/common/filesystem/filesystem_impl.h index f133e0e5d31e3..c14c8a560bf50 100644 --- a/source/common/filesystem/filesystem_impl.h +++ b/source/common/filesystem/filesystem_impl.h @@ -65,7 +65,7 @@ class Instance { } private: - std::chrono::milliseconds file_flush_interval_msec_; + const std::chrono::milliseconds file_flush_interval_msec_; FileSystemStats file_stats_; Thread::ThreadFactory& thread_factory_; };