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 \