From 1e00909730e552fe9fe41af1404f055423120204 Mon Sep 17 00:00:00 2001 From: Andrew Kryczka Date: Thu, 1 Oct 2020 19:12:26 -0700 Subject: [PATCH] Periodically flush info log out of application buffer (#7488) Summary: This PR schedules a background thread (shared across all DB instances) to flush info log every ten seconds. This improves debuggability in case of RocksDB hanging since it ensures the log messages leading up to the hang will eventually become visible in the log. The bulk of this PR is moving monitoring/stats_dump_scheduler* to db/periodic_work_scheduler* and making the corresponding name changes since now the scheduler handles info log flushing, not just stats dumping. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7488 Reviewed By: riversand963 Differential Revision: D24065165 Pulled By: ajkr fbshipit-source-id: 339c47a0ff43b79fdbd055fbd9fefbb6f9d8d3b5 --- CMakeLists.txt | 4 +- Makefile | 2 +- TARGETS | 18 ++-- db/compacted_db_impl.cc | 2 +- db/db_impl/db_impl.cc | 49 +++++------ db/db_impl/db_impl.h | 20 +++-- db/db_impl/db_impl_debug.cc | 10 +-- db/db_impl/db_impl_open.cc | 4 +- .../periodic_work_scheduler.cc | 41 +++++---- db/periodic_work_scheduler.h | 70 ++++++++++++++++ .../periodic_work_scheduler_test.cc | 83 ++++++++++++------- monitoring/stats_dump_scheduler.h | 64 -------------- monitoring/stats_history_test.cc | 12 +-- src.mk | 4 +- 14 files changed, 209 insertions(+), 174 deletions(-) rename monitoring/stats_dump_scheduler.cc => db/periodic_work_scheduler.cc (66%) create mode 100644 db/periodic_work_scheduler.h rename monitoring/stats_dump_scheduler_test.cc => db/periodic_work_scheduler_test.cc (68%) delete mode 100644 monitoring/stats_dump_scheduler.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 7eccd754d..82c96a4e3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -621,6 +621,7 @@ set(SOURCES db/merge_helper.cc db/merge_operator.cc db/output_validator.cc + db/periodic_work_scheduler.cc db/range_del_aggregator.cc db/range_tombstone_fragmenter.cc db/repair.cc @@ -678,7 +679,6 @@ set(SOURCES monitoring/perf_level.cc monitoring/persistent_stats_history.cc monitoring/statistics.cc - monitoring/stats_dump_scheduler.cc monitoring/thread_status_impl.cc monitoring/thread_status_updater.cc monitoring/thread_status_util.cc @@ -1103,6 +1103,7 @@ if(WITH_TESTS) db/merge_test.cc db/options_file_test.cc db/perf_context_test.cc + db/periodic_work_scheduler_test.cc db/plain_table_db_test.cc db/prefix_test.cc db/range_del_aggregator_test.cc @@ -1134,7 +1135,6 @@ if(WITH_TESTS) monitoring/histogram_test.cc monitoring/iostats_context_test.cc monitoring/statistics_test.cc - monitoring/stats_dump_scheduler_test.cc monitoring/stats_history_test.cc options/configurable_test.cc options/options_settable_test.cc diff --git a/Makefile b/Makefile index da7f8ab7d..d33985e54 100644 --- a/Makefile +++ b/Makefile @@ -1904,7 +1904,7 @@ blob_file_garbage_test: $(OBJ_DIR)/db/blob/blob_file_garbage_test.o $(TEST_LIBRA timer_test: $(OBJ_DIR)/util/timer_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) -stats_dump_scheduler_test: $(OBJ_DIR)/monitoring/stats_dump_scheduler_test.o $(TEST_LIBRARY) $(LIBRARY) +periodic_work_scheduler_test: $(OBJ_DIR)/db/periodic_work_scheduler_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) testutil_test: $(OBJ_DIR)/test_util/testutil_test.o $(TEST_LIBRARY) $(LIBRARY) diff --git a/TARGETS b/TARGETS index c7ef952a4..ba8c885e0 100644 --- a/TARGETS +++ b/TARGETS @@ -185,6 +185,7 @@ cpp_library( "db/merge_helper.cc", "db/merge_operator.cc", "db/output_validator.cc", + "db/periodic_work_scheduler.cc", "db/range_del_aggregator.cc", "db/range_tombstone_fragmenter.cc", "db/repair.cc", @@ -245,7 +246,6 @@ cpp_library( "monitoring/perf_level.cc", "monitoring/persistent_stats_history.cc", "monitoring/statistics.cc", - "monitoring/stats_dump_scheduler.cc", "monitoring/thread_status_impl.cc", "monitoring/thread_status_updater.cc", "monitoring/thread_status_updater_debug.cc", @@ -472,6 +472,7 @@ cpp_library( "db/merge_helper.cc", "db/merge_operator.cc", "db/output_validator.cc", + "db/periodic_work_scheduler.cc", "db/range_del_aggregator.cc", "db/range_tombstone_fragmenter.cc", "db/repair.cc", @@ -532,7 +533,6 @@ cpp_library( "monitoring/perf_level.cc", "monitoring/persistent_stats_history.cc", "monitoring/statistics.cc", - "monitoring/stats_dump_scheduler.cc", "monitoring/thread_status_impl.cc", "monitoring/thread_status_updater.cc", "monitoring/thread_status_updater_debug.cc", @@ -1681,6 +1681,13 @@ ROCKS_TESTS = [ [], [], ], + [ + "periodic_work_scheduler_test", + "db/periodic_work_scheduler_test.cc", + "serial", + [], + [], + ], [ "persistent_cache_test", "utilities/persistent_cache/persistent_cache_test.cc", @@ -1814,13 +1821,6 @@ ROCKS_TESTS = [ [], [], ], - [ - "stats_dump_scheduler_test", - "monitoring/stats_dump_scheduler_test.cc", - "serial", - [], - [], - ], [ "stats_history_test", "monitoring/stats_history_test.cc", diff --git a/db/compacted_db_impl.cc b/db/compacted_db_impl.cc index c704ae9cc..cd4f27b9e 100644 --- a/db/compacted_db_impl.cc +++ b/db/compacted_db_impl.cc @@ -156,7 +156,7 @@ Status CompactedDBImpl::Open(const Options& options, std::unique_ptr db(new CompactedDBImpl(db_options, dbname)); Status s = db->Init(options); if (s.ok()) { - db->StartStatsDumpScheduler(); + db->StartPeriodicWorkScheduler(); ROCKS_LOG_INFO(db->immutable_db_options_.info_log, "Opened the db as fully compacted mode"); LogFlush(db->immutable_db_options_.info_log); diff --git a/db/db_impl/db_impl.cc b/db/db_impl/db_impl.cc index 496118403..de41052c7 100644 --- a/db/db_impl/db_impl.cc +++ b/db/db_impl/db_impl.cc @@ -44,6 +44,7 @@ #include "db/memtable_list.h" #include "db/merge_context.h" #include "db/merge_helper.h" +#include "db/periodic_work_scheduler.h" #include "db/range_tombstone_fragmenter.h" #include "db/table_cache.h" #include "db/table_properties_collector.h" @@ -65,7 +66,6 @@ #include "monitoring/iostats_context_imp.h" #include "monitoring/perf_context_imp.h" #include "monitoring/persistent_stats_history.h" -#include "monitoring/stats_dump_scheduler.h" #include "monitoring/thread_status_updater.h" #include "monitoring/thread_status_util.h" #include "options/cf_options.h" @@ -207,7 +207,7 @@ DBImpl::DBImpl(const DBOptions& options, const std::string& dbname, refitting_level_(false), opened_successfully_(false), #ifndef ROCKSDB_LITE - stats_dump_scheduler_(nullptr), + periodic_work_scheduler_(nullptr), #endif // ROCKSDB_LITE two_write_queues_(options.two_write_queues), manual_wal_flush_(options.manual_wal_flush), @@ -446,8 +446,8 @@ void DBImpl::CancelAllBackgroundWork(bool wait) { "Shutdown: canceling all background work"); #ifndef ROCKSDB_LITE - if (stats_dump_scheduler_ != nullptr) { - stats_dump_scheduler_->Unregister(this); + if (periodic_work_scheduler_ != nullptr) { + periodic_work_scheduler_->Unregister(this); } #endif // !ROCKSDB_LITE @@ -685,18 +685,18 @@ void DBImpl::PrintStatistics() { } } -void DBImpl::StartStatsDumpScheduler() { +void DBImpl::StartPeriodicWorkScheduler() { #ifndef ROCKSDB_LITE { InstrumentedMutexLock l(&mutex_); - stats_dump_scheduler_ = StatsDumpScheduler::Default(); - TEST_SYNC_POINT_CALLBACK("DBImpl::StartStatsDumpScheduler:Init", - &stats_dump_scheduler_); + periodic_work_scheduler_ = PeriodicWorkScheduler::Default(); + TEST_SYNC_POINT_CALLBACK("DBImpl::StartPeriodicWorkScheduler:Init", + &periodic_work_scheduler_); } - stats_dump_scheduler_->Register(this, - mutable_db_options_.stats_dump_period_sec, - mutable_db_options_.stats_persist_period_sec); + periodic_work_scheduler_->Register( + this, mutable_db_options_.stats_dump_period_sec, + mutable_db_options_.stats_persist_period_sec); #endif // !ROCKSDB_LITE } @@ -907,6 +907,14 @@ void DBImpl::DumpStats() { PrintStatistics(); } +void DBImpl::FlushInfoLog() { + if (shutdown_initiated_) { + return; + } + TEST_SYNC_POINT("DBImpl::FlushInfoLog:StartRunning"); + LogFlush(immutable_db_options_.info_log); +} + Status DBImpl::TablesRangeTombstoneSummary(ColumnFamilyHandle* column_family, int max_entries_to_print, std::string* out_str) { @@ -1082,19 +1090,12 @@ Status DBImpl::SetDBOptions( mutable_db_options_.stats_dump_period_sec || new_options.stats_persist_period_sec != mutable_db_options_.stats_persist_period_sec) { - if (stats_dump_scheduler_) { - mutex_.Unlock(); - stats_dump_scheduler_->Unregister(this); - mutex_.Lock(); - } - if (new_options.stats_dump_period_sec > 0 || - new_options.stats_persist_period_sec > 0) { - mutex_.Unlock(); - stats_dump_scheduler_->Register(this, - new_options.stats_dump_period_sec, - new_options.stats_persist_period_sec); - mutex_.Lock(); - } + mutex_.Unlock(); + periodic_work_scheduler_->Unregister(this); + periodic_work_scheduler_->Register( + this, new_options.stats_dump_period_sec, + new_options.stats_persist_period_sec); + mutex_.Lock(); } write_controller_.set_max_delayed_write_rate( new_options.delayed_write_rate); diff --git a/db/db_impl/db_impl.h b/db/db_impl/db_impl.h index 1bc60b0d2..b843093db 100644 --- a/db/db_impl/db_impl.h +++ b/db/db_impl/db_impl.h @@ -70,9 +70,9 @@ class ArenaWrappedDBIter; class InMemoryStatsHistoryIterator; class MemTable; class PersistentStatsHistoryIterator; -class StatsDumpScheduler; +class PeriodicWorkScheduler; #ifndef NDEBUG -class StatsDumpTestScheduler; +class PeriodicWorkTestScheduler; #endif // !NDEBUG class TableCache; class TaskLimiterToken; @@ -1002,7 +1002,7 @@ class DBImpl : public DB { } #ifndef ROCKSDB_LITE - StatsDumpTestScheduler* TEST_GetStatsDumpScheduler() const; + PeriodicWorkTestScheduler* TEST_GetPeriodicWorkScheduler() const; #endif // !ROCKSDB_LITE #endif // NDEBUG @@ -1013,6 +1013,9 @@ class DBImpl : public DB { // dump rocksdb.stats to LOG void DumpStats(); + // flush LOG out of application buffer + void FlushInfoLog(); + protected: const std::string dbname_; std::string db_id_; @@ -1652,7 +1655,7 @@ class DBImpl : public DB { LogBuffer* log_buffer); // Schedule background tasks - void StartStatsDumpScheduler(); + void StartPeriodicWorkScheduler(); void PrintStatistics(); @@ -2111,10 +2114,11 @@ class DBImpl : public DB { std::unique_ptr recoverable_state_pre_release_callback_; #ifndef ROCKSDB_LITE - // Scheduler to run DumpStats() and PersistStats(). Currently, it always use - // a global instance from StatsDumpScheduler::Default(). Only in unittest, it - // can be overrided by StatsDumpTestSchduler. - StatsDumpScheduler* stats_dump_scheduler_; + // Scheduler to run DumpStats(), PersistStats(), and FlushInfoLog(). + // Currently, it always use a global instance from + // PeriodicWorkScheduler::Default(). Only in unittest, it can be overrided by + // PeriodicWorkTestScheduler. + PeriodicWorkScheduler* periodic_work_scheduler_; #endif // When set, we use a separate queue for writes that don't write to memtable. diff --git a/db/db_impl/db_impl_debug.cc b/db/db_impl/db_impl_debug.cc index 7669d1e9e..c21c9fa8f 100644 --- a/db/db_impl/db_impl_debug.cc +++ b/db/db_impl/db_impl_debug.cc @@ -12,7 +12,7 @@ #include "db/column_family.h" #include "db/db_impl/db_impl.h" #include "db/error_handler.h" -#include "monitoring/stats_dump_scheduler.h" +#include "db/periodic_work_scheduler.h" #include "monitoring/thread_status_updater.h" #include "util/cast_util.h" @@ -274,14 +274,14 @@ size_t DBImpl::TEST_GetWalPreallocateBlockSize( #ifndef ROCKSDB_LITE void DBImpl::TEST_WaitForStatsDumpRun(std::function callback) const { - if (stats_dump_scheduler_ != nullptr) { - static_cast(stats_dump_scheduler_) + if (periodic_work_scheduler_ != nullptr) { + static_cast(periodic_work_scheduler_) ->TEST_WaitForRun(callback); } } -StatsDumpTestScheduler* DBImpl::TEST_GetStatsDumpScheduler() const { - return static_cast(stats_dump_scheduler_); +PeriodicWorkTestScheduler* DBImpl::TEST_GetPeriodicWorkScheduler() const { + return static_cast(periodic_work_scheduler_); } #endif // !ROCKSDB_LITE diff --git a/db/db_impl/db_impl_open.cc b/db/db_impl/db_impl_open.cc index 9e3a5b549..1377a2dbd 100644 --- a/db/db_impl/db_impl_open.cc +++ b/db/db_impl/db_impl_open.cc @@ -11,12 +11,12 @@ #include "db/builder.h" #include "db/db_impl/db_impl.h" #include "db/error_handler.h" +#include "db/periodic_work_scheduler.h" #include "env/composite_env_wrapper.h" #include "file/read_write_util.h" #include "file/sst_file_manager_impl.h" #include "file/writable_file_writer.h" #include "monitoring/persistent_stats_history.h" -#include "monitoring/stats_dump_scheduler.h" #include "options/options_helper.h" #include "rocksdb/table.h" #include "rocksdb/wal_filter.h" @@ -1763,7 +1763,7 @@ Status DBImpl::Open(const DBOptions& db_options, const std::string& dbname, persist_options_status.ToString().c_str()); } if (s.ok()) { - impl->StartStatsDumpScheduler(); + impl->StartPeriodicWorkScheduler(); } else { for (auto* h : *handles) { delete h; diff --git a/monitoring/stats_dump_scheduler.cc b/db/periodic_work_scheduler.cc similarity index 66% rename from monitoring/stats_dump_scheduler.cc rename to db/periodic_work_scheduler.cc index 69fb6cf39..89303ca0f 100644 --- a/monitoring/stats_dump_scheduler.cc +++ b/db/periodic_work_scheduler.cc @@ -3,7 +3,7 @@ // COPYING file in the root directory) and Apache 2.0 License // (found in the LICENSE.Apache file in the root directory). -#include "monitoring/stats_dump_scheduler.h" +#include "db/periodic_work_scheduler.h" #include "db/db_impl/db_impl.h" #include "util/cast_util.h" @@ -11,16 +11,16 @@ #ifndef ROCKSDB_LITE namespace ROCKSDB_NAMESPACE { -StatsDumpScheduler::StatsDumpScheduler(Env* env) { +PeriodicWorkScheduler::PeriodicWorkScheduler(Env* env) { timer = std::unique_ptr(new Timer(env)); } -void StatsDumpScheduler::Register(DBImpl* dbi, - unsigned int stats_dump_period_sec, - unsigned int stats_persist_period_sec) { +void PeriodicWorkScheduler::Register(DBImpl* dbi, + unsigned int stats_dump_period_sec, + unsigned int stats_persist_period_sec) { static std::atomic initial_delay(0); + timer->Start(); if (stats_dump_period_sec > 0) { - timer->Start(); timer->Add([dbi]() { dbi->DumpStats(); }, GetTaskName(dbi, "dump_st"), initial_delay.fetch_add(1) % static_cast(stats_dump_period_sec) * @@ -28,33 +28,38 @@ void StatsDumpScheduler::Register(DBImpl* dbi, static_cast(stats_dump_period_sec) * kMicrosInSecond); } if (stats_persist_period_sec > 0) { - timer->Start(); timer->Add( [dbi]() { dbi->PersistStats(); }, GetTaskName(dbi, "pst_st"), initial_delay.fetch_add(1) % static_cast(stats_persist_period_sec) * kMicrosInSecond, static_cast(stats_persist_period_sec) * kMicrosInSecond); } + timer->Add([dbi]() { dbi->FlushInfoLog(); }, + GetTaskName(dbi, "flush_info_log"), + initial_delay.fetch_add(1) % kDefaultFlushInfoLogPeriodSec * + kMicrosInSecond, + kDefaultFlushInfoLogPeriodSec * kMicrosInSecond); } -void StatsDumpScheduler::Unregister(DBImpl* dbi) { +void PeriodicWorkScheduler::Unregister(DBImpl* dbi) { timer->Cancel(GetTaskName(dbi, "dump_st")); timer->Cancel(GetTaskName(dbi, "pst_st")); + timer->Cancel(GetTaskName(dbi, "flush_info_log")); if (!timer->HasPendingTask()) { timer->Shutdown(); } } -StatsDumpScheduler* StatsDumpScheduler::Default() { +PeriodicWorkScheduler* PeriodicWorkScheduler::Default() { // Always use the default Env for the scheduler, as we only use the NowMicros // which is the same for all env. // The Env could only be overridden in test. - static StatsDumpScheduler scheduler(Env::Default()); + static PeriodicWorkScheduler scheduler(Env::Default()); return &scheduler; } -std::string StatsDumpScheduler::GetTaskName(DBImpl* dbi, - const std::string& func_name) { +std::string PeriodicWorkScheduler::GetTaskName(DBImpl* dbi, + const std::string& func_name) { std::string db_session_id; // TODO: Should this error be ignored? dbi->GetDbSessionId(db_session_id).PermitUncheckedError(); @@ -67,8 +72,8 @@ std::string StatsDumpScheduler::GetTaskName(DBImpl* dbi, // timer, so only re-create it when there's no running task. Otherwise, return // the existing scheduler. Which means if the unittest needs to update MockEnv, // Close all db instances and then re-open them. -StatsDumpTestScheduler* StatsDumpTestScheduler::Default(Env* env) { - static StatsDumpTestScheduler scheduler(env); +PeriodicWorkTestScheduler* PeriodicWorkTestScheduler::Default(Env* env) { + static PeriodicWorkTestScheduler scheduler(env); static port::Mutex mutex; { MutexLock l(&mutex); @@ -81,22 +86,22 @@ StatsDumpTestScheduler* StatsDumpTestScheduler::Default(Env* env) { return &scheduler; } -void StatsDumpTestScheduler::TEST_WaitForRun( +void PeriodicWorkTestScheduler::TEST_WaitForRun( std::function callback) const { if (timer != nullptr) { timer->TEST_WaitForRun(callback); } } -size_t StatsDumpTestScheduler::TEST_GetValidTaskNum() const { +size_t PeriodicWorkTestScheduler::TEST_GetValidTaskNum() const { if (timer != nullptr) { return timer->TEST_GetPendingTaskNum(); } return 0; } -StatsDumpTestScheduler::StatsDumpTestScheduler(Env* env) - : StatsDumpScheduler(env) {} +PeriodicWorkTestScheduler::PeriodicWorkTestScheduler(Env* env) + : PeriodicWorkScheduler(env) {} #endif // !NDEBUG } // namespace ROCKSDB_NAMESPACE diff --git a/db/periodic_work_scheduler.h b/db/periodic_work_scheduler.h new file mode 100644 index 000000000..6c1ce314c --- /dev/null +++ b/db/periodic_work_scheduler.h @@ -0,0 +1,70 @@ +// Copyright (c) 2011-present, Facebook, Inc. All rights reserved. +// This source code is licensed under both the GPLv2 (found in the +// COPYING file in the root directory) and Apache 2.0 License +// (found in the LICENSE.Apache file in the root directory). + +#pragma once + +#ifndef ROCKSDB_LITE + +#include "db/db_impl/db_impl.h" +#include "util/timer.h" + +namespace ROCKSDB_NAMESPACE { + +// PeriodicWorkScheduler is a singleton object, which is scheduling/running +// DumpStats(), PersistStats(), and FlushInfoLog() for all DB instances. All DB +// instances use the same object from `Default()`. +// +// Internally, it uses a single threaded timer object to run the periodic work +// functions. Timer thread will always be started since the info log flushing +// cannot be disabled. +class PeriodicWorkScheduler { + public: + static PeriodicWorkScheduler* Default(); + + PeriodicWorkScheduler() = delete; + PeriodicWorkScheduler(const PeriodicWorkScheduler&) = delete; + PeriodicWorkScheduler(PeriodicWorkScheduler&&) = delete; + PeriodicWorkScheduler& operator=(const PeriodicWorkScheduler&) = delete; + PeriodicWorkScheduler& operator=(PeriodicWorkScheduler&&) = delete; + + void Register(DBImpl* dbi, unsigned int stats_dump_period_sec, + unsigned int stats_persist_period_sec); + + void Unregister(DBImpl* dbi); + + // Periodically flush info log out of application buffer at a low frequency. + // This improves debuggability in case of RocksDB hanging since it ensures the + // log messages leading up to the hang will eventually become visible in the + // log. + static const uint64_t kDefaultFlushInfoLogPeriodSec = 10; + + protected: + std::unique_ptr timer; + + explicit PeriodicWorkScheduler(Env* env); + + private: + std::string GetTaskName(DBImpl* dbi, const std::string& func_name); +}; + +#ifndef NDEBUG +// PeriodicWorkTestScheduler is for unittest, which can specify the Env like +// SafeMockTimeEnv. It also contains functions for unittest. +class PeriodicWorkTestScheduler : public PeriodicWorkScheduler { + public: + static PeriodicWorkTestScheduler* Default(Env* env); + + void TEST_WaitForRun(std::function callback) const; + + size_t TEST_GetValidTaskNum() const; + + private: + explicit PeriodicWorkTestScheduler(Env* env); +}; +#endif // !NDEBUG + +} // namespace ROCKSDB_NAMESPACE + +#endif // ROCKSDB_LITE diff --git a/monitoring/stats_dump_scheduler_test.cc b/db/periodic_work_scheduler_test.cc similarity index 68% rename from monitoring/stats_dump_scheduler_test.cc rename to db/periodic_work_scheduler_test.cc index ece60f372..d53265389 100644 --- a/monitoring/stats_dump_scheduler_test.cc +++ b/db/periodic_work_scheduler_test.cc @@ -3,17 +3,17 @@ // COPYING file in the root directory) and Apache 2.0 License // (found in the LICENSE.Apache file in the root directory). -#include "monitoring/stats_dump_scheduler.h" +#include "db/periodic_work_scheduler.h" #include "db/db_test_util.h" namespace ROCKSDB_NAMESPACE { #ifndef ROCKSDB_LITE -class StatsDumpSchedulerTest : public DBTestBase { +class PeriodicWorkSchedulerTest : public DBTestBase { public: - StatsDumpSchedulerTest() - : DBTestBase("/stats_dump_scheduler_test", /*env_do_fsync=*/true), + PeriodicWorkSchedulerTest() + : DBTestBase("/periodic_work_scheduler_test", /*env_do_fsync=*/true), mock_env_(new MockTimeEnv(Env::Default())) {} protected: @@ -22,17 +22,18 @@ class StatsDumpSchedulerTest : public DBTestBase { void SetUp() override { mock_env_->InstallTimedWaitFixCallback(); SyncPoint::GetInstance()->SetCallBack( - "DBImpl::StartStatsDumpScheduler:Init", [&](void* arg) { - auto* stats_dump_scheduler_ptr = - reinterpret_cast(arg); - *stats_dump_scheduler_ptr = - StatsDumpTestScheduler::Default(mock_env_.get()); + "DBImpl::StartPeriodicWorkScheduler:Init", [&](void* arg) { + auto* periodic_work_scheduler_ptr = + reinterpret_cast(arg); + *periodic_work_scheduler_ptr = + PeriodicWorkTestScheduler::Default(mock_env_.get()); }); } }; -TEST_F(StatsDumpSchedulerTest, Basic) { - constexpr int kPeriodSec = 5; +TEST_F(PeriodicWorkSchedulerTest, Basic) { + constexpr unsigned int kPeriodSec = + PeriodicWorkScheduler::kDefaultFlushInfoLogPeriodSec; Close(); Options options; options.stats_dump_period_sec = kPeriodSec; @@ -47,34 +48,44 @@ TEST_F(StatsDumpSchedulerTest, Basic) { int pst_st_counter = 0; SyncPoint::GetInstance()->SetCallBack("DBImpl::PersistStats:StartRunning", [&](void*) { pst_st_counter++; }); + + int flush_info_log_counter = 0; + SyncPoint::GetInstance()->SetCallBack( + "DBImpl::FlushInfoLog:StartRunning", + [&](void*) { flush_info_log_counter++; }); SyncPoint::GetInstance()->EnableProcessing(); Reopen(options); - ASSERT_EQ(5u, dbfull()->GetDBOptions().stats_dump_period_sec); - ASSERT_EQ(5u, dbfull()->GetDBOptions().stats_persist_period_sec); + ASSERT_EQ(kPeriodSec, dbfull()->GetDBOptions().stats_dump_period_sec); + ASSERT_EQ(kPeriodSec, dbfull()->GetDBOptions().stats_persist_period_sec); - dbfull()->TEST_WaitForStatsDumpRun( - [&] { mock_env_->MockSleepForSeconds(kPeriodSec - 1); }); + ASSERT_GT(kPeriodSec, 1u); + dbfull()->TEST_WaitForStatsDumpRun([&] { + mock_env_->MockSleepForSeconds(static_cast(kPeriodSec) - 1); + }); - auto scheduler = dbfull()->TEST_GetStatsDumpScheduler(); + auto scheduler = dbfull()->TEST_GetPeriodicWorkScheduler(); ASSERT_NE(nullptr, scheduler); - ASSERT_EQ(2, scheduler->TEST_GetValidTaskNum()); + ASSERT_EQ(3, scheduler->TEST_GetValidTaskNum()); ASSERT_EQ(1, dump_st_counter); ASSERT_EQ(1, pst_st_counter); + ASSERT_EQ(1, flush_info_log_counter); dbfull()->TEST_WaitForStatsDumpRun( - [&] { mock_env_->MockSleepForSeconds(kPeriodSec); }); + [&] { mock_env_->MockSleepForSeconds(static_cast(kPeriodSec)); }); ASSERT_EQ(2, dump_st_counter); ASSERT_EQ(2, pst_st_counter); + ASSERT_EQ(2, flush_info_log_counter); dbfull()->TEST_WaitForStatsDumpRun( - [&] { mock_env_->MockSleepForSeconds(kPeriodSec); }); + [&] { mock_env_->MockSleepForSeconds(static_cast(kPeriodSec)); }); ASSERT_EQ(3, dump_st_counter); ASSERT_EQ(3, pst_st_counter); + ASSERT_EQ(3, flush_info_log_counter); // Disable scheduler with SetOption ASSERT_OK(dbfull()->SetDBOptions( @@ -82,27 +93,35 @@ TEST_F(StatsDumpSchedulerTest, Basic) { ASSERT_EQ(0u, dbfull()->GetDBOptions().stats_dump_period_sec); ASSERT_EQ(0u, dbfull()->GetDBOptions().stats_persist_period_sec); - scheduler = dbfull()->TEST_GetStatsDumpScheduler(); - ASSERT_EQ(0u, scheduler->TEST_GetValidTaskNum()); + // Info log flush should still run. + dbfull()->TEST_WaitForStatsDumpRun( + [&] { mock_env_->MockSleepForSeconds(static_cast(kPeriodSec)); }); + ASSERT_EQ(3, dump_st_counter); + ASSERT_EQ(3, pst_st_counter); + ASSERT_EQ(4, flush_info_log_counter); + + scheduler = dbfull()->TEST_GetPeriodicWorkScheduler(); + ASSERT_EQ(1u, scheduler->TEST_GetValidTaskNum()); // Re-enable one task ASSERT_OK(dbfull()->SetDBOptions({{"stats_dump_period_sec", "5"}})); ASSERT_EQ(5u, dbfull()->GetDBOptions().stats_dump_period_sec); ASSERT_EQ(0u, dbfull()->GetDBOptions().stats_persist_period_sec); - scheduler = dbfull()->TEST_GetStatsDumpScheduler(); + scheduler = dbfull()->TEST_GetPeriodicWorkScheduler(); ASSERT_NE(nullptr, scheduler); - ASSERT_EQ(1, scheduler->TEST_GetValidTaskNum()); + ASSERT_EQ(2, scheduler->TEST_GetValidTaskNum()); - dump_st_counter = 0; dbfull()->TEST_WaitForStatsDumpRun( - [&] { mock_env_->MockSleepForSeconds(kPeriodSec); }); - ASSERT_EQ(1, dump_st_counter); + [&] { mock_env_->MockSleepForSeconds(static_cast(kPeriodSec)); }); + ASSERT_EQ(4, dump_st_counter); + ASSERT_EQ(3, pst_st_counter); + ASSERT_EQ(5, flush_info_log_counter); Close(); } -TEST_F(StatsDumpSchedulerTest, MultiInstances) { +TEST_F(PeriodicWorkSchedulerTest, MultiInstances) { constexpr int kPeriodSec = 5; const int kInstanceNum = 10; @@ -129,8 +148,8 @@ TEST_F(StatsDumpSchedulerTest, MultiInstances) { } auto dbi = static_cast_with_check(dbs[kInstanceNum - 1]); - auto scheduler = dbi->TEST_GetStatsDumpScheduler(); - ASSERT_EQ(kInstanceNum * 2, scheduler->TEST_GetValidTaskNum()); + auto scheduler = dbi->TEST_GetPeriodicWorkScheduler(); + ASSERT_EQ(kInstanceNum * 3, scheduler->TEST_GetValidTaskNum()); int expected_run = kInstanceNum; dbi->TEST_WaitForStatsDumpRun( @@ -170,7 +189,7 @@ TEST_F(StatsDumpSchedulerTest, MultiInstances) { } } -TEST_F(StatsDumpSchedulerTest, MultiEnv) { +TEST_F(PeriodicWorkSchedulerTest, MultiEnv) { constexpr int kDumpPeriodSec = 5; constexpr int kPersistPeriodSec = 10; Close(); @@ -194,8 +213,8 @@ TEST_F(StatsDumpSchedulerTest, MultiEnv) { ASSERT_OK(DB::Open(options2, dbname, &db)); DBImpl* dbi = static_cast_with_check(db); - ASSERT_EQ(dbi->TEST_GetStatsDumpScheduler(), - dbfull()->TEST_GetStatsDumpScheduler()); + ASSERT_EQ(dbi->TEST_GetPeriodicWorkScheduler(), + dbfull()->TEST_GetPeriodicWorkScheduler()); db->Close(); delete db; diff --git a/monitoring/stats_dump_scheduler.h b/monitoring/stats_dump_scheduler.h deleted file mode 100644 index 1045991d4..000000000 --- a/monitoring/stats_dump_scheduler.h +++ /dev/null @@ -1,64 +0,0 @@ -// Copyright (c) 2011-present, Facebook, Inc. All rights reserved. -// This source code is licensed under both the GPLv2 (found in the -// COPYING file in the root directory) and Apache 2.0 License -// (found in the LICENSE.Apache file in the root directory). - -#pragma once - -#ifndef ROCKSDB_LITE - -#include "db/db_impl/db_impl.h" -#include "util/timer.h" - -namespace ROCKSDB_NAMESPACE { - -// StatsDumpScheduler is a singleton object, which is scheduling/running -// DumpStats() and PersistStats() for all DB instances. All DB instances uses -// the same object from `Default()`. -// Internally, it uses a single threaded timer object to run the stats dump -// functions. Timer thread won't be started if there's no function needs to run, -// for example, option.stats_dump_period_sec and option.stats_persist_period_sec -// are set to 0. -class StatsDumpScheduler { - public: - static StatsDumpScheduler* Default(); - - StatsDumpScheduler() = delete; - StatsDumpScheduler(const StatsDumpScheduler&) = delete; - StatsDumpScheduler(StatsDumpScheduler&&) = delete; - StatsDumpScheduler& operator=(const StatsDumpScheduler&) = delete; - StatsDumpScheduler& operator=(StatsDumpScheduler&&) = delete; - - void Register(DBImpl* dbi, unsigned int stats_dump_period_sec, - unsigned int stats_persist_period_sec); - - void Unregister(DBImpl* dbi); - - protected: - std::unique_ptr timer; - - explicit StatsDumpScheduler(Env* env); - - private: - std::string GetTaskName(DBImpl* dbi, const std::string& func_name); -}; - -#ifndef NDEBUG -// StatsDumpTestScheduler is for unittest, which can specify the Env like -// SafeMockTimeEnv. It also contains functions for unittest. -class StatsDumpTestScheduler : public StatsDumpScheduler { - public: - static StatsDumpTestScheduler* Default(Env* env); - - void TEST_WaitForRun(std::function callback) const; - - size_t TEST_GetValidTaskNum() const; - - private: - explicit StatsDumpTestScheduler(Env* env); -}; -#endif // !NDEBUG - -} // namespace ROCKSDB_NAMESPACE - -#endif // ROCKSDB_LITE diff --git a/monitoring/stats_history_test.cc b/monitoring/stats_history_test.cc index 9ac0d1839..775cc9b6e 100644 --- a/monitoring/stats_history_test.cc +++ b/monitoring/stats_history_test.cc @@ -15,8 +15,8 @@ #include "db/column_family.h" #include "db/db_impl/db_impl.h" #include "db/db_test_util.h" +#include "db/periodic_work_scheduler.h" #include "monitoring/persistent_stats_history.h" -#include "monitoring/stats_dump_scheduler.h" #include "options/options_helper.h" #include "port/stack_trace.h" #include "rocksdb/cache.h" @@ -41,11 +41,11 @@ class StatsHistoryTest : public DBTestBase { void SetUp() override { mock_env_->InstallTimedWaitFixCallback(); SyncPoint::GetInstance()->SetCallBack( - "DBImpl::StartStatsDumpScheduler:Init", [&](void* arg) { - auto* stats_dump_scheduler_ptr = - reinterpret_cast(arg); - *stats_dump_scheduler_ptr = - StatsDumpTestScheduler::Default(mock_env_.get()); + "DBImpl::StartPeriodicWorkScheduler:Init", [&](void* arg) { + auto* periodic_work_scheduler_ptr = + reinterpret_cast(arg); + *periodic_work_scheduler_ptr = + PeriodicWorkTestScheduler::Default(mock_env_.get()); }); } }; diff --git a/src.mk b/src.mk index 344915b37..c6e1defeb 100644 --- a/src.mk +++ b/src.mk @@ -57,6 +57,7 @@ LIB_SOURCES = \ db/merge_helper.cc \ db/merge_operator.cc \ db/output_validator.cc \ + db/periodic_work_scheduler.cc \ db/range_del_aggregator.cc \ db/range_tombstone_fragmenter.cc \ db/repair.cc \ @@ -117,7 +118,6 @@ LIB_SOURCES = \ monitoring/perf_level.cc \ monitoring/persistent_stats_history.cc \ monitoring/statistics.cc \ - monitoring/stats_dump_scheduler.cc \ monitoring/thread_status_impl.cc \ monitoring/thread_status_updater.cc \ monitoring/thread_status_updater_debug.cc \ @@ -419,6 +419,7 @@ TEST_MAIN_SOURCES = \ db/obsolete_files_test.cc \ db/options_file_test.cc \ db/perf_context_test.cc \ + db/periodic_work_scheduler_test.cc \ db/plain_table_db_test.cc \ db/prefix_test.cc \ db/repair_test.cc \ @@ -450,7 +451,6 @@ TEST_MAIN_SOURCES = \ monitoring/histogram_test.cc \ monitoring/iostats_context_test.cc \ monitoring/statistics_test.cc \ - monitoring/stats_dump_scheduler_test.cc \ monitoring/stats_history_test.cc \ options/configurable_test.cc \ options/options_settable_test.cc \