From acd7d5869535c5e7c89c48f5c37ceffb944f3590 Mon Sep 17 00:00:00 2001 From: Andrew Kryczka Date: Mon, 25 Jan 2016 16:26:53 -0800 Subject: [PATCH] [directory includes cleanup] Remove util->db dependency for ThreadStatusUtil Summary: We can avoid the dependency by forward-declaring ColumnFamilyData and then treating it as a black box. That means callers of ThreadStatusUtil need to explicitly provide more options, even if they can be derived from the ColumnFamilyData, since ThreadStatusUtil doesn't include the definition. This is part of a series of diffs to eliminate circular dependencies between directories (e.g., db/* files depending on util/* files and vice-versa). Test Plan: $ ./db_test --gtest_filter=DBTest.GetThreadStatus $ make -j32 commit-prereq Reviewers: sdong, yhchiang, IslamAbdelRahman Subscribers: dhruba, leveldb Differential Revision: https://reviews.facebook.net/D53361 --- db/compaction_job.cc | 18 +++++++++--------- db/db_impl.cc | 7 +++++-- db/flush_job.cc | 3 ++- db/perf_context_test.cc | 1 + util/env_posix.cc | 1 + util/thread_status_util.cc | 35 +++++++++++++++++++++-------------- util/thread_status_util.h | 13 ++++++++----- 7 files changed, 47 insertions(+), 31 deletions(-) diff --git a/db/compaction_job.cc b/db/compaction_job.cc index 2d0711ff0..c30ee7736 100644 --- a/db/compaction_job.cc +++ b/db/compaction_job.cc @@ -237,7 +237,9 @@ CompactionJob::CompactionJob( paranoid_file_checks_(paranoid_file_checks), measure_io_stats_(measure_io_stats) { assert(log_buffer_ != nullptr); - ThreadStatusUtil::SetColumnFamily(compact_->compaction->column_family_data()); + const auto* cfd = compact_->compaction->column_family_data(); + ThreadStatusUtil::SetColumnFamily(cfd, cfd->ioptions()->env, + cfd->options()->enable_thread_tracking); ThreadStatusUtil::SetThreadOperation(ThreadStatus::OP_COMPACTION); ReportStartedCompaction(compaction); } @@ -249,8 +251,9 @@ CompactionJob::~CompactionJob() { void CompactionJob::ReportStartedCompaction( Compaction* compaction) { - ThreadStatusUtil::SetColumnFamily( - compact_->compaction->column_family_data()); + const auto* cfd = compact_->compaction->column_family_data(); + ThreadStatusUtil::SetColumnFamily(cfd, cfd->ioptions()->env, + cfd->options()->enable_thread_tracking); ThreadStatusUtil::SetThreadOperationProperty( ThreadStatus::COMPACTION_JOB_ID, @@ -415,12 +418,9 @@ void CompactionJob::GenSubcompactionBoundaries() { // Group the ranges into subcompactions const double min_file_fill_percent = 4.0 / 5; - uint64_t max_output_files = - static_cast( - std::ceil( - sum / min_file_fill_percent / - cfd->GetCurrentMutableCFOptions()->MaxFileSizeForLevel(out_lvl)) - ); + uint64_t max_output_files = static_cast(std::ceil( + sum / min_file_fill_percent / + cfd->GetCurrentMutableCFOptions()->MaxFileSizeForLevel(out_lvl))); uint64_t subcompactions = std::min({static_cast(ranges.size()), static_cast(db_options_.max_subcompactions), diff --git a/db/db_impl.cc b/db/db_impl.cc index d4f441715..2ddf26347 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -2898,7 +2898,9 @@ Status DBImpl::BackgroundCompaction(bool* made_progress, TEST_SYNC_POINT("DBImpl::BackgroundCompaction:TrivialMove"); // Instrument for event update // TODO(yhchiang): add op details for showing trivial-move. - ThreadStatusUtil::SetColumnFamily(c->column_family_data()); + ThreadStatusUtil::SetColumnFamily( + c->column_family_data(), c->column_family_data()->ioptions()->env, + c->column_family_data()->options()->enable_thread_tracking); ThreadStatusUtil::SetThreadOperation(ThreadStatus::OP_COMPACTION); compaction_job_stats.num_input_files = c->num_input_files(0); @@ -5640,7 +5642,8 @@ Status DBImpl::RenameTempFileToOptionsFile(const std::string& file_name) { void DBImpl::NewThreadStatusCfInfo( ColumnFamilyData* cfd) const { if (db_options_.enable_thread_tracking) { - ThreadStatusUtil::NewColumnFamilyInfo(this, cfd); + ThreadStatusUtil::NewColumnFamilyInfo(this, cfd, cfd->GetName(), + cfd->ioptions()->env); } } diff --git a/db/flush_job.cc b/db/flush_job.cc index a565f8f25..da1124474 100644 --- a/db/flush_job.cc +++ b/db/flush_job.cc @@ -94,7 +94,8 @@ FlushJob::~FlushJob() { } void FlushJob::ReportStartedFlush() { - ThreadStatusUtil::SetColumnFamily(cfd_); + ThreadStatusUtil::SetColumnFamily(cfd_, cfd_->ioptions()->env, + cfd_->options()->enable_thread_tracking); ThreadStatusUtil::SetThreadOperation(ThreadStatus::OP_FLUSH); ThreadStatusUtil::SetThreadOperationProperty( ThreadStatus::COMPACTION_JOB_ID, diff --git a/db/perf_context_test.cc b/db/perf_context_test.cc index 72b52f6e8..9494ac92b 100644 --- a/db/perf_context_test.cc +++ b/db/perf_context_test.cc @@ -13,6 +13,7 @@ #include "rocksdb/slice_transform.h" #include "rocksdb/memtablerep.h" #include "util/histogram.h" +#include "util/instrumented_mutex.h" #include "util/stop_watch.h" #include "util/testharness.h" #include "util/thread_status_util.h" diff --git a/util/env_posix.cc b/util/env_posix.cc index 9d549b44d..2ea8eebb1 100644 --- a/util/env_posix.cc +++ b/util/env_posix.cc @@ -46,6 +46,7 @@ #include "util/iostats_context_imp.h" #include "util/logging.h" #include "util/posix_logger.h" +#include "util/random.h" #include "util/string_util.h" #include "util/sync_point.h" #include "util/thread_local.h" diff --git a/util/thread_status_util.cc b/util/thread_status_util.cc index e67a8e4ef..6039c5f1d 100644 --- a/util/thread_status_util.cc +++ b/util/thread_status_util.cc @@ -3,9 +3,10 @@ // LICENSE file in the root directory of this source tree. An additional grant // of patent rights can be found in the PATENTS file in the same directory. +#include "util/thread_status_util.h" + #include "rocksdb/env.h" #include "util/thread_status_updater.h" -#include "util/thread_status_util.h" namespace rocksdb { @@ -33,12 +34,14 @@ void ThreadStatusUtil::UnregisterThread() { } } -void ThreadStatusUtil::SetColumnFamily(const ColumnFamilyData* cfd) { - if (!MaybeInitThreadLocalUpdater(cfd->ioptions()->env)) { +void ThreadStatusUtil::SetColumnFamily(const ColumnFamilyData* cfd, + const Env* env, + bool enable_thread_tracking) { + if (!MaybeInitThreadLocalUpdater(env)) { return; } assert(thread_updater_local_cache_); - if (cfd != nullptr && cfd->options()->enable_thread_tracking) { + if (cfd != nullptr && enable_thread_tracking) { thread_updater_local_cache_->SetColumnFamilyInfoKey(cfd); } else { // When cfd == nullptr or enable_thread_tracking == false, we set @@ -118,15 +121,17 @@ void ThreadStatusUtil::ResetThreadStatus() { thread_updater_local_cache_->ResetThreadStatus(); } -void ThreadStatusUtil::NewColumnFamilyInfo( - const DB* db, const ColumnFamilyData* cfd) { - if (!MaybeInitThreadLocalUpdater(cfd->ioptions()->env)) { +void ThreadStatusUtil::NewColumnFamilyInfo(const DB* db, + const ColumnFamilyData* cfd, + const std::string& cf_name, + const Env* env) { + if (!MaybeInitThreadLocalUpdater(env)) { return; } assert(thread_updater_local_cache_); if (thread_updater_local_cache_) { - thread_updater_local_cache_->NewColumnFamilyInfo( - db, db->GetName(), cfd, cfd->GetName()); + thread_updater_local_cache_->NewColumnFamilyInfo(db, db->GetName(), cfd, + cf_name); } } @@ -171,8 +176,9 @@ bool ThreadStatusUtil::MaybeInitThreadLocalUpdater(const Env* env) { return false; } -void ThreadStatusUtil::SetColumnFamily(const ColumnFamilyData* cfd) { -} +void ThreadStatusUtil::SetColumnFamily(const ColumnFamilyData* cfd, + const Env* env, + bool enable_thread_tracking) {} void ThreadStatusUtil::SetThreadOperation(ThreadStatus::OperationType op) { } @@ -188,9 +194,10 @@ void ThreadStatusUtil::IncreaseThreadOperationProperty( void ThreadStatusUtil::SetThreadState(ThreadStatus::StateType state) { } -void ThreadStatusUtil::NewColumnFamilyInfo( - const DB* db, const ColumnFamilyData* cfd) { -} +void ThreadStatusUtil::NewColumnFamilyInfo(const DB* db, + const ColumnFamilyData* cfd, + const std::string& cf_name, + const Env* env) {} void ThreadStatusUtil::EraseColumnFamilyInfo( const ColumnFamilyData* cfd) { diff --git a/util/thread_status_util.h b/util/thread_status_util.h index aa13a6c40..101cd0ef1 100644 --- a/util/thread_status_util.h +++ b/util/thread_status_util.h @@ -5,14 +5,16 @@ #pragma once -#include "db/column_family.h" +#include + +#include "rocksdb/db.h" #include "rocksdb/env.h" #include "rocksdb/thread_status.h" #include "util/thread_status_updater.h" namespace rocksdb { -class ColumnFamilyData; +class ColumnFamilyData; // The static utility class for updating thread-local status. // @@ -37,8 +39,8 @@ class ThreadStatusUtil { // Create an entry in the global ColumnFamilyInfo table for the // specified column family. This function should be called only // when the current thread does not hold db_mutex. - static void NewColumnFamilyInfo( - const DB* db, const ColumnFamilyData* cfd); + static void NewColumnFamilyInfo(const DB* db, const ColumnFamilyData* cfd, + const std::string& cf_name, const Env* env); // Erase the ConstantColumnFamilyInfo that is associated with the // specified ColumnFamilyData. This function should be called only @@ -52,7 +54,8 @@ class ThreadStatusUtil { // Update the thread status to indicate the current thread is doing // something related to the specified column family. - static void SetColumnFamily(const ColumnFamilyData* cfd); + static void SetColumnFamily(const ColumnFamilyData* cfd, const Env* env, + bool enable_thread_tracking); static void SetThreadOperation(ThreadStatus::OperationType type);