Make statistics's stats_level change thread-safe (#5030)

Summary:
Right now, users can change statistics.stats_level while DB is running, but TSAN may report
data race. We make stats_level_ to be atomic, and access them using accessors.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/5030

Differential Revision: D14267519

Pulled By: siying

fbshipit-source-id: 37d7ebeff7a43a406230143422a16af899163f73
This commit is contained in:
Siying Dong 2019-03-01 10:39:00 -08:00 committed by Facebook Github Bot
parent 916e524134
commit aef763b6d6
11 changed files with 25 additions and 14 deletions

View File

@ -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).

View File

@ -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;
@ -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,

View File

@ -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);

View File

@ -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);
@ -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

View File

@ -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());
}
/*
@ -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);
}
/*

View File

@ -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;

View File

@ -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);

View File

@ -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 {

View File

@ -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));
@ -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;

View File

@ -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;

View File

@ -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),