From 51ac91f5869a0554b6d255d063d0601ffec7add4 Mon Sep 17 00:00:00 2001 From: Siying Dong Date: Wed, 31 May 2017 07:27:40 -0700 Subject: [PATCH] Histogram of number of merge operands Summary: Add a histogram in statistics to help users understand how many merge operands they merge. Closes https://github.com/facebook/rocksdb/pull/2373 Differential Revision: D5139983 Pulled By: siying fbshipit-source-id: 61b9ba8ca83f358530a4833d68f0103b56a0e182 --- db/db_iter.cc | 12 ++++++------ db/memtable.cc | 4 ++-- db/merge_helper.cc | 8 +++++++- db/merge_helper.h | 5 ++++- db/version_set.cc | 3 ++- include/rocksdb/statistics.h | 4 ++++ 6 files changed, 25 insertions(+), 11 deletions(-) diff --git a/db/db_iter.cc b/db/db_iter.cc index ec7ed936d..37be16ab0 100644 --- a/db/db_iter.cc +++ b/db/db_iter.cc @@ -562,7 +562,7 @@ void DBIter::MergeValuesNewToOld() { const Slice val = iter_->value(); s = MergeHelper::TimedFullMerge( merge_operator_, ikey.user_key, &val, merge_context_.GetOperands(), - &saved_value_, logger_, statistics_, env_, &pinned_value_); + &saved_value_, logger_, statistics_, env_, &pinned_value_, true); if (!s.ok()) { status_ = s; } @@ -587,7 +587,7 @@ void DBIter::MergeValuesNewToOld() { s = MergeHelper::TimedFullMerge(merge_operator_, saved_key_.GetUserKey(), nullptr, merge_context_.GetOperands(), &saved_value_, logger_, statistics_, env_, - &pinned_value_); + &pinned_value_, true); if (!s.ok()) { status_ = s; } @@ -806,13 +806,13 @@ bool DBIter::FindValueForCurrentKey() { s = MergeHelper::TimedFullMerge( merge_operator_, saved_key_.GetUserKey(), nullptr, merge_context_.GetOperands(), &saved_value_, logger_, statistics_, - env_, &pinned_value_); + env_, &pinned_value_, true); } else { assert(last_not_merge_type == kTypeValue); s = MergeHelper::TimedFullMerge( merge_operator_, saved_key_.GetUserKey(), &pinned_value_, merge_context_.GetOperands(), &saved_value_, logger_, statistics_, - env_, &pinned_value_); + env_, &pinned_value_, true); } break; case kTypeValue: @@ -884,7 +884,7 @@ bool DBIter::FindValueForCurrentKeyUsingSeek() { s = MergeHelper::TimedFullMerge(merge_operator_, saved_key_.GetUserKey(), nullptr, merge_context_.GetOperands(), &saved_value_, logger_, statistics_, env_, - &pinned_value_); + &pinned_value_, true); // Make iter_ valid and point to saved_key_ if (!iter_->Valid() || !user_comparator_->Equal(ikey.user_key, saved_key_.GetUserKey())) { @@ -902,7 +902,7 @@ bool DBIter::FindValueForCurrentKeyUsingSeek() { s = MergeHelper::TimedFullMerge(merge_operator_, saved_key_.GetUserKey(), &val, merge_context_.GetOperands(), &saved_value_, logger_, statistics_, env_, - &pinned_value_); + &pinned_value_, true); valid_ = true; if (!s.ok()) { status_ = s; diff --git a/db/memtable.cc b/db/memtable.cc index 43858cb55..6ee0b603e 100644 --- a/db/memtable.cc +++ b/db/memtable.cc @@ -578,7 +578,7 @@ static bool SaveValue(void* arg, const char* entry) { *(s->status) = MergeHelper::TimedFullMerge( merge_operator, s->key->user_key(), &v, merge_context->GetOperands(), s->value, s->logger, s->statistics, - s->env_); + s->env_, nullptr /* result_operand */, true); } else if (s->value != nullptr) { s->value->assign(v.data(), v.size()); } @@ -595,7 +595,7 @@ static bool SaveValue(void* arg, const char* entry) { *(s->status) = MergeHelper::TimedFullMerge( merge_operator, s->key->user_key(), nullptr, merge_context->GetOperands(), s->value, s->logger, s->statistics, - s->env_); + s->env_, nullptr /* result_operand */, true); } else { *(s->status) = Status::NotFound(); } diff --git a/db/merge_helper.cc b/db/merge_helper.cc index 892c1b373..a72b04564 100644 --- a/db/merge_helper.cc +++ b/db/merge_helper.cc @@ -25,7 +25,8 @@ Status MergeHelper::TimedFullMerge(const MergeOperator* merge_operator, const std::vector& operands, std::string* result, Logger* logger, Statistics* statistics, Env* env, - Slice* result_operand) { + Slice* result_operand, + bool update_num_ops_stats) { assert(merge_operator != nullptr); if (operands.size() == 0) { @@ -34,6 +35,11 @@ Status MergeHelper::TimedFullMerge(const MergeOperator* merge_operator, return Status::OK(); } + if (update_num_ops_stats) { + MeasureTime(statistics, READ_NUM_MERGE_OPERANDS, + static_cast(operands.size())); + } + bool success; Slice tmp_result_operand(nullptr, 0); const MergeOperator::MergeOperationInput merge_in(key, value, operands, diff --git a/db/merge_helper.h b/db/merge_helper.h index 4b91d5551..5f3ccd0d1 100644 --- a/db/merge_helper.h +++ b/db/merge_helper.h @@ -56,6 +56,8 @@ class MergeHelper { // Wrapper around MergeOperator::FullMergeV2() that records perf statistics. // Result of merge will be written to result if status returned is OK. // If operands is empty, the value will simply be copied to result. + // Set `update_num_ops_stats` to true if it is from a user read, so that + // the latency is sensitive. // Returns one of the following statuses: // - OK: Entries were successfully merged. // - Corruption: Merge operator reported unsuccessful merge. @@ -64,7 +66,8 @@ class MergeHelper { const std::vector& operands, std::string* result, Logger* logger, Statistics* statistics, Env* env, - Slice* result_operand = nullptr); + Slice* result_operand = nullptr, + bool update_num_ops_stats = false); // Merge entries until we hit // - a corrupted key diff --git a/db/version_set.cc b/db/version_set.cc index b071a1261..49850c850 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -1016,7 +1016,8 @@ void Version::Get(const ReadOptions& read_options, const LookupKey& k, std::string* str_value = value != nullptr ? value->GetSelf() : nullptr; *status = MergeHelper::TimedFullMerge( merge_operator_, user_key, nullptr, merge_context->GetOperands(), - str_value, info_log_, db_statistics_, env_); + str_value, info_log_, db_statistics_, env_, + nullptr /* result_operand */, true); if (LIKELY(value != nullptr)) { value->PinSelf(); } diff --git a/include/rocksdb/statistics.h b/include/rocksdb/statistics.h index 041e7f368..b139d739a 100644 --- a/include/rocksdb/statistics.h +++ b/include/rocksdb/statistics.h @@ -370,6 +370,9 @@ enum Histograms : uint32_t { BYTES_DECOMPRESSED, COMPRESSION_TIMES_NANOS, DECOMPRESSION_TIMES_NANOS, + // Number of merge operands passed to the merge operator in user read + // requests. + READ_NUM_MERGE_OPERANDS, HISTOGRAM_ENUM_MAX, // TODO(ldemailly): enforce HistogramsNameMap match }; @@ -405,6 +408,7 @@ const std::vector> HistogramsNameMap = { {BYTES_DECOMPRESSED, "rocksdb.bytes.decompressed"}, {COMPRESSION_TIMES_NANOS, "rocksdb.compression.times.nanos"}, {DECOMPRESSION_TIMES_NANOS, "rocksdb.decompression.times.nanos"}, + {READ_NUM_MERGE_OPERANDS, "rocksdb.read.num.merge_operands"}, }; struct HistogramData {