Skip to content

Commit

Permalink
fix ThreadStatus for bottom-pri compaction threads
Browse files Browse the repository at this point in the history
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 facebook#3270

Differential Revision: D6559428

Pulled By: ajkr

fbshipit-source-id: 96b1a50a9c19492b1a5fd1b77cf7061a6f9f1d1c
  • Loading branch information
ajkr authored and facebook-github-bot committed Dec 14, 2017
1 parent b4d88d7 commit 5a7e084
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 18 deletions.
15 changes: 10 additions & 5 deletions db/db_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion include/rocksdb/thread_status.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
};

Expand Down Expand Up @@ -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);
Expand Down
21 changes: 14 additions & 7 deletions monitoring/thread_status_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -120,7 +127,7 @@ std::map<std::string, uint64_t>

#else

const std::string& ThreadStatus::GetThreadTypeName(
std::string ThreadStatus::GetThreadTypeName(
ThreadStatus::ThreadType thread_type) {
static std::string dummy_str = "";
return dummy_str;
Expand Down
24 changes: 19 additions & 5 deletions util/threadpool_imp.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 5a7e084

Please sign in to comment.