From 3d2e709b1cb1f049d53339300e04c1efa7ffacd1 Mon Sep 17 00:00:00 2001 From: Jim Lin Date: Wed, 25 Aug 2021 18:41:18 +0800 Subject: [PATCH] feat: skip value get (#114) * feat: skip value get Co-authored-by: wangyi --- db/db_impl.cc | 5 ++--- db/db_test.cc | 32 ++++++++++++++++++++++++++++++++ db/memtable.cc | 20 +++++++++++--------- db/merge_test.cc | 3 +++ db/version_set.cc | 12 +++++++----- include/rocksdb/db.h | 25 +++++++++++++++++++------ utilities/ttl/db_ttl_impl.cc | 4 ++++ 7 files changed, 78 insertions(+), 23 deletions(-) diff --git a/db/db_impl.cc b/db/db_impl.cc index 492d8c684..27c2bf8c4 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -1637,7 +1637,7 @@ Status DBImpl::Get(const ReadOptions& read_options, ColumnFamilyHandle* column_family, const Slice& key, LazyBuffer* value) { auto s = GetImpl(read_options, column_family, key, value); - assert(!s.ok() || value->valid()); + assert(!s.ok() || value == nullptr || value->valid()); return s; } @@ -1648,7 +1648,6 @@ Status DBImpl::GetImpl(const ReadOptions& read_options, LatencyHistGuard guard(&read_latency_reporter_); read_qps_reporter_.AddCount(1); - assert(lazy_val != nullptr); StopWatch sw(env_, stats_, DB_GET); PERF_TIMER_GUARD(get_snapshot_time); @@ -1742,7 +1741,7 @@ Status DBImpl::GetImpl(const ReadOptions& read_options, RecordTick(stats_, MEMTABLE_MISS); } - if (s.ok()) { + if (s.ok() && lazy_val != nullptr) { lazy_val->pin(LazyBufferPinLevel::DB); s = lazy_val->fetch(); } diff --git a/db/db_test.cc b/db/db_test.cc index 4816f268a..cef733325 100644 --- a/db/db_test.cc +++ b/db/db_test.cc @@ -5684,6 +5684,38 @@ TEST_F(DBTest, ThreadLocalPtrDeadlock) { fprintf(stderr, "Done. Flushed %d times, destroyed %d threads\n", flushes_done.load(), threads_destroyed.load()); } + +TEST_F(DBTest, SkipValueGet) { + Options options = CurrentOptions(); + DestroyAndReopen(options); + + ASSERT_OK(Put(Key(0), "")); + + // memtable get + auto s = db_->Get(ReadOptions{}, db_->DefaultColumnFamily(), Key(0)); + ASSERT_OK(s); + s = db_->Get(ReadOptions{}, db_->DefaultColumnFamily(), Key(0), + static_cast(nullptr)); + ASSERT_OK(s); + s = db_->Get(ReadOptions{}, Key(0), static_cast(nullptr)); + ASSERT_OK(s); + s = db_->Get(ReadOptions{}, Key(1), static_cast(nullptr)); + ASSERT_TRUE(s.IsNotFound()); + + ASSERT_OK(Flush()); + dbfull()->TEST_WaitForFlushMemTable(); + + // sst get + s = db_->Get(ReadOptions{}, db_->DefaultColumnFamily(), Key(0)); + ASSERT_OK(s); + s = db_->Get(ReadOptions{}, db_->DefaultColumnFamily(), Key(0), + static_cast(nullptr)); + ASSERT_OK(s); + s = db_->Get(ReadOptions{}, Key(0), static_cast(nullptr)); + ASSERT_OK(s); + s = db_->Get(ReadOptions{}, Key(1), static_cast(nullptr)); + ASSERT_TRUE(s.IsNotFound()); +} } // namespace TERARKDB_NAMESPACE int main(int argc, char** argv) { diff --git a/db/memtable.cc b/db/memtable.cc index 779472896..77e984b97 100644 --- a/db/memtable.cc +++ b/db/memtable.cc @@ -773,7 +773,7 @@ static bool SaveValue(void* arg, const Slice& internal_key, const char* value) { } *s->status = Status::OK(); if (*s->merge_in_progress) { - if (s->value != nullptr) { + if (LIKELY(s->value != nullptr)) { LazyBuffer lazy_val(GetLengthPrefixedSlice(value), false); *s->status = MergeHelper::TimedFullMerge( merge_operator, s->key->user_key(), &lazy_val, @@ -783,7 +783,7 @@ static bool SaveValue(void* arg, const Slice& internal_key, const char* value) { s->value->pin(LazyBufferPinLevel::Internal); } } - } else if (s->value != nullptr) { + } else if (LIKELY(s->value != nullptr)) { s->value->reset(GetLengthPrefixedSlice(value), s->inplace_update_support); } @@ -797,7 +797,7 @@ static bool SaveValue(void* arg, const Slice& internal_key, const char* value) { case kTypeSingleDeletion: case kTypeRangeDeletion: { if (*s->merge_in_progress) { - if (s->value != nullptr) { + if (LIKELY(s->value != nullptr)) { *s->status = MergeHelper::TimedFullMerge( merge_operator, s->key->user_key(), nullptr, merge_context->GetOperands(), s->value, s->logger, @@ -831,12 +831,14 @@ static bool SaveValue(void* arg, const Slice& internal_key, const char* value) { LazyBuffer(GetLengthPrefixedSlice(value), Cleanable())); if (merge_operator->ShouldMerge( merge_context->GetOperandsDirectionBackward())) { - *s->status = MergeHelper::TimedFullMerge( - merge_operator, s->key->user_key(), nullptr, - merge_context->GetOperands(), s->value, s->logger, s->statistics, - s->env_, true); - if (s->status->ok()) { - s->value->pin(LazyBufferPinLevel::Internal); + if (LIKELY(s->value != nullptr)) { + *s->status = MergeHelper::TimedFullMerge( + merge_operator, s->key->user_key(), nullptr, + merge_context->GetOperands(), s->value, s->logger, + s->statistics, s->env_, true); + if (s->status->ok()) { + s->value->pin(LazyBufferPinLevel::Internal); + } } *s->found_final_value = true; return false; diff --git a/db/merge_test.cc b/db/merge_test.cc index c699ddaa9..4bc1bfa0a 100644 --- a/db/merge_test.cc +++ b/db/merge_test.cc @@ -169,6 +169,9 @@ class Counters { bool get(const std::string& key, uint64_t* value) { std::string str; auto s = db_->Get(get_option_, key, &str); + auto x = num_merge_operator_calls; + assert(db_->Get(get_option_, key).code() == s.code()); + num_merge_operator_calls = x; if (s.IsNotFound()) { // return default value if not found; diff --git a/db/version_set.cc b/db/version_set.cc index e26e0eede..452ad8ed2 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -1410,11 +1410,13 @@ void Version::Get(const ReadOptions& read_options, const Slice& user_key, } // merge_operands are in saver and we hit the beginning of the key history // do a final merge of nullptr and operands; - *status = MergeHelper::TimedFullMerge( - merge_operator_, user_key, nullptr, merge_context->GetOperands(), value, - info_log_, db_statistics_, env_, true); - if (status->ok()) { - value->pin(LazyBufferPinLevel::Internal); + if (value != nullptr) { + *status = MergeHelper::TimedFullMerge( + merge_operator_, user_key, nullptr, merge_context->GetOperands(), + value, info_log_, db_statistics_, env_, true); + if (status->ok()) { + value->pin(LazyBufferPinLevel::Internal); + } } } else { if (key_exists != nullptr) { diff --git a/include/rocksdb/db.h b/include/rocksdb/db.h index 5b7ae7c69..4523ee7bb 100644 --- a/include/rocksdb/db.h +++ b/include/rocksdb/db.h @@ -353,6 +353,7 @@ class DB { // If the database contains an entry for "key" store the // corresponding value in *value and return OK. + // If "value" equals to nullptr, the corresponding value will not be loaded. // // If there is no entry for "key" leave *value unchanged and return // a status for which Status::IsNotFound() returns true. @@ -361,21 +362,33 @@ class DB { virtual inline Status Get(const ReadOptions& options, ColumnFamilyHandle* column_family, const Slice& key, std::string* value) { - assert(value != nullptr); - LazyBuffer lazy_val(value); - auto s = Get(options, column_family, key, &lazy_val); - if (s.ok()) { - s = std::move(lazy_val).dump(value); + if (value != nullptr) { + LazyBuffer lazy_val(value); + auto s = Get(options, column_family, key, &lazy_val); + if (s.ok()) { + s = std::move(lazy_val).dump(value); + } + return s; + } else { + return Get(options, column_family, key, + static_cast(nullptr)); } - return s; } virtual Status Get(const ReadOptions& options, ColumnFamilyHandle* column_family, const Slice& key, LazyBuffer* value) = 0; + virtual Status Get(const ReadOptions& options, + ColumnFamilyHandle* column_family, const Slice& key) { + return Get(options, column_family, key, static_cast(nullptr)); + } virtual Status Get(const ReadOptions& options, const Slice& key, std::string* value) { return Get(options, DefaultColumnFamily(), key, value); } + virtual Status Get(const ReadOptions& options, const Slice& key) { + return Get(options, DefaultColumnFamily(), key, + static_cast(nullptr)); + } #ifdef BOOSTLIB static void CallOnMainStack(const std::function&); static void SubmitAsyncTask(std::function); diff --git a/utilities/ttl/db_ttl_impl.cc b/utilities/ttl/db_ttl_impl.cc index 82b5b617c..7ce2a402b 100644 --- a/utilities/ttl/db_ttl_impl.cc +++ b/utilities/ttl/db_ttl_impl.cc @@ -209,6 +209,10 @@ Status DBWithTTLImpl::Put(const WriteOptions& options, Status DBWithTTLImpl::Get(const ReadOptions& options, ColumnFamilyHandle* column_family, const Slice& key, LazyBuffer* value) { + LazyBuffer buffer; + if (value == nullptr) { + value = &buffer; + } Status st = db_->Get(options, column_family, key, value); if (!st.ok()) { return st; -- GitLab