Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make statistics's stats_level change thread-safe #5030

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions HISTORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
### New Features
* Introduce two more stats levels, kExceptHistogramOrTimers and kExceptTimers.

### Public API Change
* statistics.stats_level_ becomes atomic. It is preferred to use statistics.set_stats_level() and statistics.get_stats_level() to access it.

## 6.0.0 (2/19/2019)
### New Features
* Enabled checkpoint on readonly db (DBImplReadOnly).
Expand Down
4 changes: 2 additions & 2 deletions db/db_statistics_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ TEST_F(DBStatisticsTest, CompressionStatsTest) {
Options options = CurrentOptions();
options.compression = type;
options.statistics = rocksdb::CreateDBStatistics();
options.statistics->stats_level_ = StatsLevel::kExceptTimeForMutex;
options.statistics->set_stats_level(StatsLevel::kExceptTimeForMutex);
DestroyAndReopen(options);

int kNumKeysWritten = 100000;
Expand Down Expand Up @@ -105,7 +105,7 @@ TEST_F(DBStatisticsTest, MutexWaitStats) {
Options options = CurrentOptions();
options.create_if_missing = true;
options.statistics = rocksdb::CreateDBStatistics();
options.statistics->stats_level_ = StatsLevel::kAll;
options.statistics->set_stats_level(StatsLevel::kAll);
CreateAndReopenWithCF({"pikachu"}, options);
const uint64_t kMutexWaitDelay = 100;
ThreadStatusUtil::TEST_SetStateDelay(ThreadStatus::STATE_MUTEX_WAIT,
Expand Down
2 changes: 1 addition & 1 deletion db/db_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5102,7 +5102,7 @@ TEST_P(DBTestWithParam, FilterCompactionTimeTest) {
options.disable_auto_compactions = true;
options.create_if_missing = true;
options.statistics = rocksdb::CreateDBStatistics();
options.statistics->stats_level_ = kExceptTimeForMutex;
options.statistics->set_stats_level(kExceptTimeForMutex);
options.max_subcompactions = max_subcompactions_;
DestroyAndReopen(options);

Expand Down
11 changes: 9 additions & 2 deletions include/rocksdb/statistics.h
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ class Statistics {
virtual void setTickerCount(uint32_t tickerType, uint64_t count) = 0;
virtual uint64_t getAndResetTickerCount(uint32_t tickerType) = 0;
virtual void reportTimeToHistogram(uint32_t histogramType, uint64_t time) {
if (stats_level_ <= StatsLevel::kExceptTimers) {
if (get_stats_level() <= StatsLevel::kExceptTimers) {
return;
}
recordInHistogram(histogramType, time);
Expand Down Expand Up @@ -514,8 +514,15 @@ class Statistics {
virtual bool HistEnabledForType(uint32_t type) const {
return type < HISTOGRAM_ENUM_MAX;
}
void set_stats_level(StatsLevel sl) {
stats_level_.store(sl, std::memory_order_relaxed);
}
StatsLevel get_stats_level() const {
return stats_level_.load(std::memory_order_relaxed);
}

StatsLevel stats_level_ = kExceptDetailedTimers;
private:
std::atomic<StatsLevel> stats_level_{kExceptDetailedTimers};
};

// Create a concrete DBStatistics object
Expand Down
4 changes: 2 additions & 2 deletions java/rocksjni/statistics.cc
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ jbyte Java_org_rocksdb_Statistics_statsLevel(
reinterpret_cast<std::shared_ptr<rocksdb::Statistics>*>(jhandle);
assert(pSptr_statistics != nullptr);
return rocksdb::StatsLevelJni::toJavaStatsLevel(
pSptr_statistics->get()->stats_level_);
pSptr_statistics->get()->get_stats_level());
}

/*
Expand All @@ -132,7 +132,7 @@ void Java_org_rocksdb_Statistics_setStatsLevel(
reinterpret_cast<std::shared_ptr<rocksdb::Statistics>*>(jhandle);
assert(pSptr_statistics != nullptr);
auto stats_level = rocksdb::StatsLevelJni::toCppStatsLevel(jstats_level);
pSptr_statistics->get()->stats_level_ = stats_level;
pSptr_statistics->get()->set_stats_level(stats_level);
}

/*
Expand Down
2 changes: 1 addition & 1 deletion monitoring/instrumented_mutex.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace rocksdb {
namespace {
Statistics* stats_for_report(Env* env, Statistics* stats) {
if (env != nullptr && stats != nullptr &&
stats->stats_level_ > kExceptTimeForMutex) {
stats->get_stats_level() > kExceptTimeForMutex) {
return stats;
} else {
return nullptr;
Expand Down
2 changes: 1 addition & 1 deletion monitoring/statistics.cc
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ void StatisticsImpl::recordTick(uint32_t tickerType, uint64_t count) {

void StatisticsImpl::recordInHistogram(uint32_t histogramType, uint64_t value) {
assert(histogramType < HISTOGRAM_ENUM_MAX);
if (stats_level_ <= StatsLevel::kExceptHistogramOrTimers) {
if (get_stats_level() <= StatsLevel::kExceptHistogramOrTimers) {
return;
}
per_core_stats_.Access()->histograms_[histogramType].Add(value);
Expand Down
2 changes: 1 addition & 1 deletion table/format.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ const uint64_t kPlainTableMagicNumber = 0;

bool ShouldReportDetailedTime(Env* env, Statistics* stats) {
return env != nullptr && stats != nullptr &&
stats->stats_level_ > kExceptDetailedTimers;
stats->get_stats_level() > kExceptDetailedTimers;
}

void BlockHandle::EncodeTo(std::string* dst) const {
Expand Down
4 changes: 2 additions & 2 deletions table/table_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1128,7 +1128,7 @@ TEST_P(BlockBasedTableTest, BasicBlockBasedTableProperties) {
Options options;
options.compression = kNoCompression;
options.statistics = CreateDBStatistics();
options.statistics->stats_level_ = StatsLevel::kAll;
options.statistics->set_stats_level(StatsLevel::kAll);
BlockBasedTableOptions table_options = GetBlockBasedTableOptions();
table_options.block_restart_interval = 1;
options.table_factory.reset(NewBlockBasedTableFactory(table_options));
Expand Down Expand Up @@ -1176,7 +1176,7 @@ uint64_t BlockBasedTableTest::IndexUncompressedHelper(bool compressed) {
Options options;
options.compression = kSnappyCompression;
options.statistics = CreateDBStatistics();
options.statistics->stats_level_ = StatsLevel::kAll;
options.statistics->set_stats_level(StatsLevel::kAll);
BlockBasedTableOptions table_options = GetBlockBasedTableOptions();
table_options.block_restart_interval = 1;
table_options.enable_index_compression = compressed;
Expand Down
2 changes: 1 addition & 1 deletion tools/db_bench_tool.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6082,7 +6082,7 @@ int db_bench_tool(int argc, char** argv) {
dbstats = rocksdb::CreateDBStatistics();
}
if (dbstats) {
dbstats->stats_level_ = static_cast<StatsLevel>(FLAGS_stats_level);
dbstats->set_stats_level(static_cast<StatsLevel>(FLAGS_stats_level));
}
FLAGS_compaction_pri_e = (rocksdb::CompactionPri)FLAGS_compaction_pri;

Expand Down
3 changes: 2 additions & 1 deletion util/stop_watch.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ class StopWatch {
elapsed_(elapsed),
overwrite_(overwrite),
stats_enabled_(statistics &&
statistics->stats_level_ >= StatsLevel::kExceptTimers &&
statistics->get_stats_level() >=
StatsLevel::kExceptTimers &&
statistics->HistEnabledForType(hist_type)),
delay_enabled_(delay_enabled),
total_delay_(0),
Expand Down