From 5a7e08468ad2f03b957e255aefdfa81b95a6810a Mon Sep 17 00:00:00 2001 From: Andrew Kryczka Date: Thu, 14 Dec 2017 14:41:59 -0800 Subject: [PATCH] fix ThreadStatus for bottom-pri compaction threads Summary: added `ThreadType::BOTTOM_PRIORITY` which is used in the `ThreadStatus` object to indicate the thread is used for bottom-pri compactions. Previously there was a bug where we mislabeled such threads as `ThreadType::LOW_PRIORITY`. Closes https://github.com/facebook/rocksdb/pull/3270 Differential Revision: D6559428 Pulled By: ajkr fbshipit-source-id: 96b1a50a9c19492b1a5fd1b77cf7061a6f9f1d1c --- db/db_test.cc | 15 ++++++++++----- include/rocksdb/thread_status.h | 3 ++- monitoring/thread_status_impl.cc | 21 ++++++++++++++------- util/threadpool_imp.cc | 24 +++++++++++++++++++----- 4 files changed, 45 insertions(+), 18 deletions(-) diff --git a/db/db_test.cc b/db/db_test.cc index 3fb78e6e0..c9233b87c 100644 --- a/db/db_test.cc +++ b/db/db_test.cc @@ -3488,12 +3488,16 @@ TEST_F(DBTest, GetThreadStatus) { const int kTestCount = 3; const unsigned int kHighPriCounts[kTestCount] = {3, 2, 5}; const unsigned int kLowPriCounts[kTestCount] = {10, 15, 3}; + const unsigned int kBottomPriCounts[kTestCount] = {2, 1, 4}; for (int test = 0; test < kTestCount; ++test) { // Change the number of threads in high / low priority pool. env_->SetBackgroundThreads(kHighPriCounts[test], Env::HIGH); env_->SetBackgroundThreads(kLowPriCounts[test], Env::LOW); + env_->SetBackgroundThreads(kBottomPriCounts[test], Env::BOTTOM); // Wait to ensure the all threads has been registered unsigned int thread_type_counts[ThreadStatus::NUM_THREAD_TYPES]; + // TODO(ajkr): it'd be better if SetBackgroundThreads returned only after + // all threads have been registered. // Try up to 60 seconds. for (int num_try = 0; num_try < 60000; num_try++) { env_->SleepForMicroseconds(1000); @@ -3508,20 +3512,21 @@ TEST_F(DBTest, GetThreadStatus) { if (thread_type_counts[ThreadStatus::HIGH_PRIORITY] == kHighPriCounts[test] && thread_type_counts[ThreadStatus::LOW_PRIORITY] == - kLowPriCounts[test]) { + kLowPriCounts[test] && + thread_type_counts[ThreadStatus::BOTTOM_PRIORITY] == + kBottomPriCounts[test]) { break; } } - // Verify the total number of threades - ASSERT_EQ(thread_type_counts[ThreadStatus::HIGH_PRIORITY] + - thread_type_counts[ThreadStatus::LOW_PRIORITY], - kHighPriCounts[test] + kLowPriCounts[test]); // Verify the number of high-priority threads ASSERT_EQ(thread_type_counts[ThreadStatus::HIGH_PRIORITY], kHighPriCounts[test]); // Verify the number of low-priority threads ASSERT_EQ(thread_type_counts[ThreadStatus::LOW_PRIORITY], kLowPriCounts[test]); + // Verify the number of bottom-priority threads + ASSERT_EQ(thread_type_counts[ThreadStatus::BOTTOM_PRIORITY], + kBottomPriCounts[test]); } if (i == 0) { // repeat the test with multiple column families diff --git a/include/rocksdb/thread_status.h b/include/rocksdb/thread_status.h index 55c32ed6d..e7a25f190 100644 --- a/include/rocksdb/thread_status.h +++ b/include/rocksdb/thread_status.h @@ -45,6 +45,7 @@ struct ThreadStatus { HIGH_PRIORITY = 0, // RocksDB BG thread in high-pri thread pool LOW_PRIORITY, // RocksDB BG thread in low-pri thread pool USER, // User thread (Non-RocksDB BG thread) + BOTTOM_PRIORITY, // RocksDB BG thread in bottom-pri thread pool NUM_THREAD_TYPES }; @@ -163,7 +164,7 @@ struct ThreadStatus { // The followings are a set of utility functions for interpreting // the information of ThreadStatus - static const std::string& GetThreadTypeName(ThreadType thread_type); + static std::string GetThreadTypeName(ThreadType thread_type); // Obtain the name of an operation given its type. static const std::string& GetOperationName(OperationType op_type); diff --git a/monitoring/thread_status_impl.cc b/monitoring/thread_status_impl.cc index e263ce661..d3555c4bd 100644 --- a/monitoring/thread_status_impl.cc +++ b/monitoring/thread_status_impl.cc @@ -14,14 +14,21 @@ namespace rocksdb { #ifdef ROCKSDB_USING_THREAD_STATUS -const std::string& ThreadStatus::GetThreadTypeName( +std::string ThreadStatus::GetThreadTypeName( ThreadStatus::ThreadType thread_type) { - static std::string thread_type_names[NUM_THREAD_TYPES + 1] = { - "High Pri", "Low Pri", "User", "Unknown"}; - if (thread_type < 0 || thread_type >= NUM_THREAD_TYPES) { - return thread_type_names[NUM_THREAD_TYPES]; // "Unknown" + switch (thread_type) { + case ThreadStatus::ThreadType::HIGH_PRIORITY: + return "High Pri"; + case ThreadStatus::ThreadType::LOW_PRIORITY: + return "Low Pri"; + case ThreadStatus::ThreadType::USER: + return "User"; + case ThreadStatus::ThreadType::BOTTOM_PRIORITY: + return "Bottom Pri"; + case ThreadStatus::ThreadType::NUM_THREAD_TYPES: + assert(false); } - return thread_type_names[thread_type]; + return "Unknown"; } const std::string& ThreadStatus::GetOperationName( @@ -120,7 +127,7 @@ std::map #else -const std::string& ThreadStatus::GetThreadTypeName( +std::string ThreadStatus::GetThreadTypeName( ThreadStatus::ThreadType thread_type) { static std::string dummy_str = ""; return dummy_str; diff --git a/util/threadpool_imp.cc b/util/threadpool_imp.cc index 10b5d4361..916004820 100644 --- a/util/threadpool_imp.cc +++ b/util/threadpool_imp.cc @@ -254,11 +254,25 @@ void* ThreadPoolImpl::Impl::BGThreadWrapper(void* arg) { size_t thread_id = meta->thread_id_; ThreadPoolImpl::Impl* tp = meta->thread_pool_; #ifdef ROCKSDB_USING_THREAD_STATUS - // for thread-status - ThreadStatusUtil::RegisterThread( - tp->GetHostEnv(), (tp->GetThreadPriority() == Env::Priority::HIGH - ? ThreadStatus::HIGH_PRIORITY - : ThreadStatus::LOW_PRIORITY)); + // initialize it because compiler isn't good enough to see we don't use it + // uninitialized + ThreadStatus::ThreadType thread_type = ThreadStatus::NUM_THREAD_TYPES; + switch (tp->GetThreadPriority()) { + case Env::Priority::HIGH: + thread_type = ThreadStatus::HIGH_PRIORITY; + break; + case Env::Priority::LOW: + thread_type = ThreadStatus::LOW_PRIORITY; + break; + case Env::Priority::BOTTOM: + thread_type = ThreadStatus::BOTTOM_PRIORITY; + break; + case Env::Priority::TOTAL: + assert(false); + return nullptr; + } + assert(thread_type != ThreadStatus::NUM_THREAD_TYPES); + ThreadStatusUtil::RegisterThread(tp->GetHostEnv(), thread_type); #endif delete meta; tp->BGThread(thread_id);