option for timing measurement of non-blocking ops during compaction (#4029)

Summary:
For example calling CompactionFilter is always timed and gives the user no way to disable.
This PR will disable the timer if `Statistics::stats_level_` (which is part of DBOptions) is `kExceptDetailedTimers`
Closes https://github.com/facebook/rocksdb/pull/4029

Differential Revision: D8583670

Pulled By: miasantreble

fbshipit-source-id: 913be9fe433ae0c06e88193b59d41920a532307f
This commit is contained in:
Zhongyi Xie 2018-06-21 21:16:49 -07:00 committed by Facebook Github Bot
parent 0a5b16c7c5
commit 795e663df0
7 changed files with 22 additions and 13 deletions

View File

@ -28,6 +28,7 @@
#include "rocksdb/options.h" #include "rocksdb/options.h"
#include "rocksdb/table.h" #include "rocksdb/table.h"
#include "table/block_based_table_builder.h" #include "table/block_based_table_builder.h"
#include "table/format.h"
#include "table/internal_iterator.h" #include "table/internal_iterator.h"
#include "util/file_reader_writer.h" #include "util/file_reader_writer.h"
#include "util/filename.h" #include "util/filename.h"
@ -139,6 +140,7 @@ Status BuildTable(
CompactionIterator c_iter( CompactionIterator c_iter(
iter, internal_comparator.user_comparator(), &merge, kMaxSequenceNumber, iter, internal_comparator.user_comparator(), &merge, kMaxSequenceNumber,
&snapshots, earliest_write_conflict_snapshot, snapshot_checker, env, &snapshots, earliest_write_conflict_snapshot, snapshot_checker, env,
ShouldReportDetailedTime(env, ioptions.statistics),
true /* internal key corruption is not ok */, range_del_agg.get()); true /* internal key corruption is not ok */, range_del_agg.get());
c_iter.SeekToFirst(); c_iter.SeekToFirst();
for (; c_iter.Valid(); c_iter.Next()) { for (; c_iter.Valid(); c_iter.Next()) {

View File

@ -17,14 +17,15 @@ CompactionIterator::CompactionIterator(
SequenceNumber last_sequence, std::vector<SequenceNumber>* snapshots, SequenceNumber last_sequence, std::vector<SequenceNumber>* snapshots,
SequenceNumber earliest_write_conflict_snapshot, SequenceNumber earliest_write_conflict_snapshot,
const SnapshotChecker* snapshot_checker, Env* env, const SnapshotChecker* snapshot_checker, Env* env,
bool expect_valid_internal_key, RangeDelAggregator* range_del_agg, bool report_detailed_time, bool expect_valid_internal_key,
RangeDelAggregator* range_del_agg,
const Compaction* compaction, const CompactionFilter* compaction_filter, const Compaction* compaction, const CompactionFilter* compaction_filter,
const std::atomic<bool>* shutting_down, const std::atomic<bool>* shutting_down,
const SequenceNumber preserve_deletes_seqnum) const SequenceNumber preserve_deletes_seqnum)
: CompactionIterator( : CompactionIterator(
input, cmp, merge_helper, last_sequence, snapshots, input, cmp, merge_helper, last_sequence, snapshots,
earliest_write_conflict_snapshot, snapshot_checker, env, earliest_write_conflict_snapshot, snapshot_checker, env,
expect_valid_internal_key, range_del_agg, report_detailed_time, expect_valid_internal_key, range_del_agg,
std::unique_ptr<CompactionProxy>( std::unique_ptr<CompactionProxy>(
compaction ? new CompactionProxy(compaction) : nullptr), compaction ? new CompactionProxy(compaction) : nullptr),
compaction_filter, shutting_down, preserve_deletes_seqnum) {} compaction_filter, shutting_down, preserve_deletes_seqnum) {}
@ -34,7 +35,8 @@ CompactionIterator::CompactionIterator(
SequenceNumber /*last_sequence*/, std::vector<SequenceNumber>* snapshots, SequenceNumber /*last_sequence*/, std::vector<SequenceNumber>* snapshots,
SequenceNumber earliest_write_conflict_snapshot, SequenceNumber earliest_write_conflict_snapshot,
const SnapshotChecker* snapshot_checker, Env* env, const SnapshotChecker* snapshot_checker, Env* env,
bool expect_valid_internal_key, RangeDelAggregator* range_del_agg, bool report_detailed_time, bool expect_valid_internal_key,
RangeDelAggregator* range_del_agg,
std::unique_ptr<CompactionProxy> compaction, std::unique_ptr<CompactionProxy> compaction,
const CompactionFilter* compaction_filter, const CompactionFilter* compaction_filter,
const std::atomic<bool>* shutting_down, const std::atomic<bool>* shutting_down,
@ -46,6 +48,7 @@ CompactionIterator::CompactionIterator(
earliest_write_conflict_snapshot_(earliest_write_conflict_snapshot), earliest_write_conflict_snapshot_(earliest_write_conflict_snapshot),
snapshot_checker_(snapshot_checker), snapshot_checker_(snapshot_checker),
env_(env), env_(env),
report_detailed_time_(report_detailed_time),
expect_valid_internal_key_(expect_valid_internal_key), expect_valid_internal_key_(expect_valid_internal_key),
range_del_agg_(range_del_agg), range_del_agg_(range_del_agg),
compaction_(std::move(compaction)), compaction_(std::move(compaction)),
@ -171,12 +174,12 @@ void CompactionIterator::InvokeFilterIfNeeded(bool* need_skip,
// to get sequence number. // to get sequence number.
Slice& filter_key = ikey_.type == kTypeValue ? ikey_.user_key : key_; Slice& filter_key = ikey_.type == kTypeValue ? ikey_.user_key : key_;
{ {
StopWatchNano timer(env_, true); StopWatchNano timer(env_, report_detailed_time_);
filter = compaction_filter_->FilterV2( filter = compaction_filter_->FilterV2(
compaction_->level(), filter_key, value_type, value_, compaction_->level(), filter_key, value_type, value_,
&compaction_filter_value_, compaction_filter_skip_until_.rep()); &compaction_filter_value_, compaction_filter_skip_until_.rep());
iter_stats_.total_filter_time += iter_stats_.total_filter_time +=
env_ != nullptr ? timer.ElapsedNanos() : 0; env_ != nullptr && report_detailed_time_ ? timer.ElapsedNanos() : 0;
} }
if (filter == CompactionFilter::Decision::kRemoveAndSkipUntil && if (filter == CompactionFilter::Decision::kRemoveAndSkipUntil &&

View File

@ -63,7 +63,7 @@ class CompactionIterator {
std::vector<SequenceNumber>* snapshots, std::vector<SequenceNumber>* snapshots,
SequenceNumber earliest_write_conflict_snapshot, SequenceNumber earliest_write_conflict_snapshot,
const SnapshotChecker* snapshot_checker, Env* env, const SnapshotChecker* snapshot_checker, Env* env,
bool expect_valid_internal_key, bool report_detailed_time, bool expect_valid_internal_key,
RangeDelAggregator* range_del_agg, RangeDelAggregator* range_del_agg,
const Compaction* compaction = nullptr, const Compaction* compaction = nullptr,
const CompactionFilter* compaction_filter = nullptr, const CompactionFilter* compaction_filter = nullptr,
@ -76,7 +76,7 @@ class CompactionIterator {
std::vector<SequenceNumber>* snapshots, std::vector<SequenceNumber>* snapshots,
SequenceNumber earliest_write_conflict_snapshot, SequenceNumber earliest_write_conflict_snapshot,
const SnapshotChecker* snapshot_checker, Env* env, const SnapshotChecker* snapshot_checker, Env* env,
bool expect_valid_internal_key, bool report_detailed_time, bool expect_valid_internal_key,
RangeDelAggregator* range_del_agg, RangeDelAggregator* range_del_agg,
std::unique_ptr<CompactionProxy> compaction, std::unique_ptr<CompactionProxy> compaction,
const CompactionFilter* compaction_filter = nullptr, const CompactionFilter* compaction_filter = nullptr,
@ -139,6 +139,7 @@ class CompactionIterator {
const SequenceNumber earliest_write_conflict_snapshot_; const SequenceNumber earliest_write_conflict_snapshot_;
const SnapshotChecker* const snapshot_checker_; const SnapshotChecker* const snapshot_checker_;
Env* env_; Env* env_;
bool report_detailed_time_;
bool expect_valid_internal_key_; bool expect_valid_internal_key_;
RangeDelAggregator* range_del_agg_; RangeDelAggregator* range_del_agg_;
std::unique_ptr<CompactionProxy> compaction_; std::unique_ptr<CompactionProxy> compaction_;

View File

@ -247,8 +247,9 @@ class CompactionIteratorTest : public testing::TestWithParam<bool> {
c_iter_.reset(new CompactionIterator( c_iter_.reset(new CompactionIterator(
iter_.get(), cmp_, merge_helper_.get(), last_sequence, &snapshots_, iter_.get(), cmp_, merge_helper_.get(), last_sequence, &snapshots_,
earliest_write_conflict_snapshot, snapshot_checker_.get(), earliest_write_conflict_snapshot, snapshot_checker_.get(),
Env::Default(), false, range_del_agg_.get(), std::move(compaction), Env::Default(), false /* report_detailed_time */,
filter, &shutting_down_)); false, range_del_agg_.get(), std::move(compaction), filter,
&shutting_down_));
} }
void AddSnapshot(SequenceNumber snapshot, void AddSnapshot(SequenceNumber snapshot,

View File

@ -879,9 +879,9 @@ void CompactionJob::ProcessKeyValueCompaction(SubcompactionState* sub_compact) {
sub_compact->c_iter.reset(new CompactionIterator( sub_compact->c_iter.reset(new CompactionIterator(
input.get(), cfd->user_comparator(), &merge, versions_->LastSequence(), input.get(), cfd->user_comparator(), &merge, versions_->LastSequence(),
&existing_snapshots_, earliest_write_conflict_snapshot_, &existing_snapshots_, earliest_write_conflict_snapshot_,
snapshot_checker_, env_, false, range_del_agg.get(), snapshot_checker_, env_, ShouldReportDetailedTime(env_, stats_), false,
sub_compact->compaction, compaction_filter, shutting_down_, range_del_agg.get(), sub_compact->compaction, compaction_filter,
preserve_deletes_seqnum_)); shutting_down_, preserve_deletes_seqnum_));
auto c_iter = sub_compact->c_iter.get(); auto c_iter = sub_compact->c_iter.get();
c_iter->SeekToFirst(); c_iter->SeekToFirst();
if (c_iter->Valid() && sub_compact->compaction->output_level() != 0) { if (c_iter->Valid() && sub_compact->compaction->output_level() != 0) {

View File

@ -5099,6 +5099,7 @@ TEST_P(DBTestWithParam, FilterCompactionTimeTest) {
options.disable_auto_compactions = true; options.disable_auto_compactions = true;
options.create_if_missing = true; options.create_if_missing = true;
options.statistics = rocksdb::CreateDBStatistics(); options.statistics = rocksdb::CreateDBStatistics();
options.statistics->stats_level_ = kExceptTimeForMutex;
options.max_subcompactions = max_subcompactions_; options.max_subcompactions = max_subcompactions_;
DestroyAndReopen(options); DestroyAndReopen(options);

View File

@ -14,6 +14,7 @@
#include "rocksdb/comparator.h" #include "rocksdb/comparator.h"
#include "rocksdb/db.h" #include "rocksdb/db.h"
#include "rocksdb/merge_operator.h" #include "rocksdb/merge_operator.h"
#include "table/format.h"
#include "table/internal_iterator.h" #include "table/internal_iterator.h"
namespace rocksdb { namespace rocksdb {
@ -372,7 +373,7 @@ CompactionFilter::Decision MergeHelper::FilterMerge(const Slice& user_key,
if (compaction_filter_ == nullptr) { if (compaction_filter_ == nullptr) {
return CompactionFilter::Decision::kKeep; return CompactionFilter::Decision::kKeep;
} }
if (stats_ != nullptr) { if (stats_ != nullptr && ShouldReportDetailedTime(env_, stats_)) {
filter_timer_.Start(); filter_timer_.Start();
} }
compaction_filter_value_.clear(); compaction_filter_value_.clear();