You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kvrocks.apache.org by tw...@apache.org on 2022/06/24 03:35:20 UTC
[incubator-kvrocks] branch unstable updated: Add a unique_ptr wrapper for rocksdb::Iterator (#650)
This is an automated email from the ASF dual-hosted git repository.
twice pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/incubator-kvrocks.git
The following commit(s) were added to refs/heads/unstable by this push:
new fc9b3fc Add a unique_ptr wrapper for rocksdb::Iterator (#650)
fc9b3fc is described below
commit fc9b3fcf8064f9841cd711ecdb958797cf5ca7a2
Author: Twice <tw...@gmail.com>
AuthorDate: Fri Jun 24 11:35:15 2022 +0800
Add a unique_ptr wrapper for rocksdb::Iterator (#650)
In #650, we added a `unique_ptr` wrapper for `rocksdb::Iterator` to avoid manual deallocation.
Co-authored-by: Wang Yuan <wa...@baidu.com>
---
src/db_util.h | 40 ++++++++++++++++++++++++++++++++++++++++
src/redis_bitmap.cc | 4 ++--
src/redis_db.cc | 22 +++++++---------------
src/redis_hash.cc | 4 ++--
src/redis_list.cc | 13 +++++--------
src/redis_set.cc | 8 ++++----
src/redis_sortedint.cc | 8 ++++----
src/redis_zset.cc | 18 ++++++++----------
src/util.h | 2 +-
9 files changed, 73 insertions(+), 46 deletions(-)
diff --git a/src/db_util.h b/src/db_util.h
new file mode 100644
index 0000000..d9ded71
--- /dev/null
+++ b/src/db_util.h
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#pragma once
+
+#include <memory>
+
+#include "rocksdb/db.h"
+#include "rocksdb/iterator.h"
+
+namespace DBUtil {
+
+struct UniqueIterator : std::unique_ptr<rocksdb::Iterator> {
+ using base_type = std::unique_ptr<rocksdb::Iterator>;
+
+ explicit UniqueIterator(rocksdb::Iterator *iter) : base_type(iter) {}
+ UniqueIterator(rocksdb::DB* db, const rocksdb::ReadOptions& options, rocksdb::ColumnFamilyHandle* column_family)
+ : base_type(db->NewIterator(options, column_family)) {}
+ UniqueIterator(rocksdb::DB* db, const rocksdb::ReadOptions& options)
+ : base_type(db->NewIterator(options)) {}
+};
+
+} // namespace DBUtil
diff --git a/src/redis_bitmap.cc b/src/redis_bitmap.cc
index 5d759a2..91b24b2 100644
--- a/src/redis_bitmap.cc
+++ b/src/redis_bitmap.cc
@@ -25,6 +25,7 @@
#include <algorithm>
#include "redis_bitmap_string.h"
+#include "db_util.h"
namespace Redis {
@@ -130,7 +131,7 @@ rocksdb::Status Bitmap::GetString(const Slice &user_key, const uint32_t max_btos
read_options.fill_cache = false;
uint32_t frag_index, valid_size;
- auto iter = db_->NewIterator(read_options);
+ auto iter = DBUtil::UniqueIterator(db_, read_options);
for (iter->Seek(prefix_key);
iter->Valid() && iter->key().starts_with(prefix_key);
iter->Next()) {
@@ -158,7 +159,6 @@ rocksdb::Status Bitmap::GetString(const Slice &user_key, const uint32_t max_btos
}
value->replace(frag_index, valid_size, fragment.data(), valid_size);
}
- delete iter;
return rocksdb::Status::OK();
}
diff --git a/src/redis_db.cc b/src/redis_db.cc
index 35361af..df7538c 100644
--- a/src/redis_db.cc
+++ b/src/redis_db.cc
@@ -22,8 +22,10 @@
#include "redis_db.h"
#include <ctime>
#include <map>
+#include "rocksdb/iterator.h"
#include "server.h"
#include "util.h"
+#include "db_util.h"
namespace Redis {
@@ -179,7 +181,7 @@ void Database::Keys(std::string prefix, std::vector<std::string> *keys, KeyNumSt
rocksdb::ReadOptions read_options;
read_options.snapshot = ss.GetSnapShot();
read_options.fill_cache = false;
- auto iter = db_->NewIterator(read_options, metadata_cf_handle_);
+ auto iter = DBUtil::UniqueIterator(db_, read_options, metadata_cf_handle_);
while (true) {
ns_prefix.empty() ? iter->SeekToFirst() : iter->Seek(ns_prefix);
@@ -220,7 +222,6 @@ void Database::Keys(std::string prefix, std::vector<std::string> *keys, KeyNumSt
if (stats && stats->n_expires > 0) {
stats->avg_ttl = ttl_sum / stats->n_expires;
}
- delete iter;
}
rocksdb::Status Database::Scan(const std::string &cursor,
@@ -237,7 +238,7 @@ rocksdb::Status Database::Scan(const std::string &cursor,
rocksdb::ReadOptions read_options;
read_options.snapshot = ss.GetSnapShot();
read_options.fill_cache = false;
- auto iter = db_->NewIterator(read_options, metadata_cf_handle_);
+ auto iter = DBUtil::UniqueIterator(db_, read_options, metadata_cf_handle_);
AppendNamespacePrefix(cursor, &ns_cursor);
if (storage_->IsSlotIdEncoded()) {
@@ -315,7 +316,6 @@ rocksdb::Status Database::Scan(const std::string &cursor,
ns_prefix.append(prefix);
iter->Seek(ns_prefix);
}
- delete iter;
return rocksdb::Status::OK();
}
@@ -362,25 +362,21 @@ rocksdb::Status Database::FlushAll() {
rocksdb::ReadOptions read_options;
read_options.snapshot = ss.GetSnapShot();
read_options.fill_cache = false;
- auto iter = db_->NewIterator(read_options, metadata_cf_handle_);
+ auto iter = DBUtil::UniqueIterator(db_, read_options, metadata_cf_handle_);
iter->SeekToFirst();
if (!iter->Valid()) {
- delete iter;
return rocksdb::Status::OK();
}
auto first_key = iter->key().ToString();
iter->SeekToLast();
if (!iter->Valid()) {
- delete iter;
return rocksdb::Status::OK();
}
auto last_key = iter->key().ToString();
auto s = storage_->DeleteRange(first_key, last_key);
if (!s.ok()) {
- delete iter;
return s;
}
- delete iter;
return rocksdb::Status::OK();
}
@@ -474,10 +470,9 @@ rocksdb::Status Database::FindKeyRangeWithPrefix(const std::string &prefix,
rocksdb::ReadOptions read_options;
read_options.snapshot = ss.GetSnapShot();
read_options.fill_cache = false;
- auto iter = storage_->GetDB()->NewIterator(read_options, cf_handle);
+ auto iter = DBUtil::UniqueIterator(storage_->GetDB(), read_options, cf_handle);
iter->Seek(prefix);
if (!iter->Valid() || !iter->key().starts_with(prefix)) {
- delete iter;
return rocksdb::Status::NotFound();
}
*begin = iter->key().ToString();
@@ -502,11 +497,9 @@ rocksdb::Status Database::FindKeyRangeWithPrefix(const std::string &prefix,
iter->Prev();
}
if (!iter->Valid() || !iter->key().starts_with(prefix)) {
- delete iter;
return rocksdb::Status::NotFound();
}
*end = iter->key().ToString();
- delete iter;
return rocksdb::Status::OK();
}
@@ -585,7 +578,7 @@ rocksdb::Status SubKeyScanner::Scan(RedisType type,
rocksdb::ReadOptions read_options;
read_options.snapshot = ss.GetSnapShot();
read_options.fill_cache = false;
- auto iter = db_->NewIterator(read_options);
+ auto iter = DBUtil::UniqueIterator(db_, read_options);
std::string match_prefix_key;
if (!subkey_prefix.empty()) {
InternalKey(ns_key, subkey_prefix, metadata.version, storage_->IsSlotIdEncoded()).Encode(&match_prefix_key);
@@ -618,7 +611,6 @@ rocksdb::Status SubKeyScanner::Scan(RedisType type,
break;
}
}
- delete iter;
return rocksdb::Status::OK();
}
diff --git a/src/redis_hash.cc b/src/redis_hash.cc
index 40d415c..36c1597 100644
--- a/src/redis_hash.cc
+++ b/src/redis_hash.cc
@@ -24,6 +24,7 @@
#include <cmath>
#include <iostream>
#include <rocksdb/status.h>
+#include "db_util.h"
namespace Redis {
rocksdb::Status Hash::GetMetadata(const Slice &ns_key, HashMetadata *metadata) {
@@ -293,7 +294,7 @@ rocksdb::Status Hash::GetAll(const Slice &user_key, std::vector<FieldValue> *fie
read_options.iterate_upper_bound = &upper_bound;
read_options.fill_cache = false;
- auto iter = db_->NewIterator(read_options);
+ auto iter = DBUtil::UniqueIterator(db_, read_options);
for (iter->Seek(prefix_key);
iter->Valid() && iter->key().starts_with(prefix_key);
iter->Next()) {
@@ -310,7 +311,6 @@ rocksdb::Status Hash::GetAll(const Slice &user_key, std::vector<FieldValue> *fie
}
field_values->emplace_back(fv);
}
- delete iter;
return rocksdb::Status::OK();
}
diff --git a/src/redis_list.cc b/src/redis_list.cc
index 9792ed0..fe851fe 100644
--- a/src/redis_list.cc
+++ b/src/redis_list.cc
@@ -21,6 +21,8 @@
#include "redis_list.h"
#include <stdlib.h>
+#include "db_util.h"
+
namespace Redis {
rocksdb::Status List::GetMetadata(const Slice &ns_key, ListMetadata *metadata) {
@@ -194,7 +196,7 @@ rocksdb::Status List::Rem(const Slice &user_key, int count, const Slice &elem, i
read_options.iterate_lower_bound = &lower_bound;
read_options.fill_cache = false;
- auto iter = db_->NewIterator(read_options);
+ auto iter = DBUtil::UniqueIterator(db_, read_options);
for (iter->Seek(start_key);
iter->Valid() && iter->key().starts_with(prefix);
!reversed ? iter->Next() : iter->Prev()) {
@@ -207,7 +209,6 @@ rocksdb::Status List::Rem(const Slice &user_key, int count, const Slice &elem, i
}
}
if (to_delete_indexes.empty()) {
- delete iter;
return rocksdb::Status::NotFound();
}
@@ -258,7 +259,6 @@ rocksdb::Status List::Rem(const Slice &user_key, int count, const Slice &elem, i
batch.Put(metadata_cf_handle_, ns_key, bytes);
}
- delete iter;
*ret = static_cast<int>(to_delete_indexes.size());
return storage_->Write(rocksdb::WriteOptions(), &batch);
}
@@ -287,7 +287,7 @@ rocksdb::Status List::Insert(const Slice &user_key, const Slice &pivot, const Sl
read_options.iterate_upper_bound = &upper_bound;
read_options.fill_cache = false;
- auto iter = db_->NewIterator(read_options);
+ auto iter = DBUtil::UniqueIterator(db_, read_options);
for (iter->Seek(start_key);
iter->Valid() && iter->key().starts_with(prefix);
iter->Next()) {
@@ -299,7 +299,6 @@ rocksdb::Status List::Insert(const Slice &user_key, const Slice &pivot, const Sl
}
}
if (pivot_index == (metadata.head - 1)) {
- delete iter;
*ret = -1;
return rocksdb::Status::NotFound();
}
@@ -345,7 +344,6 @@ rocksdb::Status List::Insert(const Slice &user_key, const Slice &pivot, const Sl
metadata.Encode(&bytes);
batch.Put(metadata_cf_handle_, ns_key, bytes);
- delete iter;
*ret = metadata.size;
return storage_->Write(rocksdb::WriteOptions(), &batch);
}
@@ -405,7 +403,7 @@ rocksdb::Status List::Range(const Slice &user_key, int start, int stop, std::vec
read_options.iterate_upper_bound = &upper_bound;
read_options.fill_cache = false;
- auto iter = db_->NewIterator(read_options);
+ auto iter = DBUtil::UniqueIterator(db_, read_options);
for (iter->Seek(start_key);
iter->Valid() && iter->key().starts_with(prefix);
iter->Next()) {
@@ -417,7 +415,6 @@ rocksdb::Status List::Range(const Slice &user_key, int start, int stop, std::vec
if (index > metadata.head + stop) break;
elems->push_back(iter->value().ToString());
}
- delete iter;
return rocksdb::Status::OK();
}
diff --git a/src/redis_set.cc b/src/redis_set.cc
index 41772c6..f0aef9f 100644
--- a/src/redis_set.cc
+++ b/src/redis_set.cc
@@ -24,6 +24,8 @@
#include <iostream>
#include <memory>
+#include "db_util.h"
+
namespace Redis {
rocksdb::Status Set::GetMetadata(const Slice &ns_key, SetMetadata *metadata) {
@@ -152,14 +154,13 @@ rocksdb::Status Set::Members(const Slice &user_key, std::vector<std::string> *me
read_options.iterate_upper_bound = &upper_bound;
read_options.fill_cache = false;
- auto iter = db_->NewIterator(read_options);
+ auto iter = DBUtil::UniqueIterator(db_, read_options);
for (iter->Seek(prefix);
iter->Valid() && iter->key().starts_with(prefix);
iter->Next()) {
InternalKey ikey(iter->key(), storage_->IsSlotIdEncoded());
members->emplace_back(ikey.GetSubKey().ToString());
}
- delete iter;
return rocksdb::Status::OK();
}
@@ -216,7 +217,7 @@ rocksdb::Status Set::Take(const Slice &user_key, std::vector<std::string> *membe
read_options.iterate_upper_bound = &upper_bound;
read_options.fill_cache = false;
- auto iter = db_->NewIterator(read_options);
+ auto iter = DBUtil::UniqueIterator(db_, read_options);
for (iter->Seek(prefix);
iter->Valid() && iter->key().starts_with(prefix);
iter->Next()) {
@@ -225,7 +226,6 @@ rocksdb::Status Set::Take(const Slice &user_key, std::vector<std::string> *membe
if (pop) batch.Delete(iter->key());
if (++n >= count) break;
}
- delete iter;
if (pop && n > 0) {
metadata.size -= n;
std::string bytes;
diff --git a/src/redis_sortedint.cc b/src/redis_sortedint.cc
index a72552b..638afcd 100644
--- a/src/redis_sortedint.cc
+++ b/src/redis_sortedint.cc
@@ -24,6 +24,8 @@
#include <iostream>
#include <limits>
+#include "db_util.h"
+
namespace Redis {
rocksdb::Status Sortedint::GetMetadata(const Slice &ns_key, SortedintMetadata *metadata) {
@@ -143,7 +145,7 @@ rocksdb::Status Sortedint::Range(const Slice &user_key,
read_options.fill_cache = false;
uint64_t id, pos = 0;
- auto iter = db_->NewIterator(read_options);
+ auto iter = DBUtil::UniqueIterator(db_, read_options);
for (!reversed ? iter->Seek(start_key) : iter->SeekForPrev(start_key);
iter->Valid() && iter->key().starts_with(prefix);
!reversed ? iter->Next() : iter->Prev()) {
@@ -154,7 +156,6 @@ rocksdb::Status Sortedint::Range(const Slice &user_key,
ids->emplace_back(id);
if (limit > 0 && ids && ids->size() >= limit) break;
}
- delete iter;
return rocksdb::Status::OK();
}
@@ -188,7 +189,7 @@ rocksdb::Status Sortedint::RangeByValue(const Slice &user_key,
read_options.fill_cache = false;
int pos = 0;
- auto iter = db_->NewIterator(read_options);
+ auto iter = DBUtil::UniqueIterator(db_, read_options);
if (!spec.reversed) {
iter->Seek(start_key);
} else {
@@ -214,7 +215,6 @@ rocksdb::Status Sortedint::RangeByValue(const Slice &user_key,
if (size) *size += 1;
if (spec.count > 0 && ids && ids->size() >= static_cast<unsigned>(spec.count)) break;
}
- delete iter;
return rocksdb::Status::OK();
}
diff --git a/src/redis_zset.cc b/src/redis_zset.cc
index c7803a2..c03b5b6 100644
--- a/src/redis_zset.cc
+++ b/src/redis_zset.cc
@@ -27,6 +27,9 @@
#include <memory>
#include <set>
+#include "util.h"
+#include "db_util.h"
+
namespace Redis {
rocksdb::Status ZSet::GetMetadata(const Slice &ns_key, ZSetMetadata *metadata) {
@@ -175,7 +178,7 @@ rocksdb::Status ZSet::Pop(const Slice &user_key, int count, bool min, std::vecto
read_options.iterate_lower_bound = &lower_bound;
read_options.fill_cache = false;
- auto iter = db_->NewIterator(read_options, score_cf_handle_);
+ auto iter = DBUtil::UniqueIterator(db_, read_options, score_cf_handle_);
iter->Seek(start_key);
// see comment in rangebyscore()
if (!min && (!iter->Valid() || !iter->key().starts_with(prefix_key))) {
@@ -194,7 +197,6 @@ rocksdb::Status ZSet::Pop(const Slice &user_key, int count, bool min, std::vecto
batch.Delete(score_cf_handle_, iter->key());
if (mscores->size() >= static_cast<unsigned>(count)) break;
}
- delete iter;
if (!mscores->empty()) {
metadata.size -= mscores->size();
@@ -247,7 +249,7 @@ rocksdb::Status ZSet::Range(const Slice &user_key, int start, int stop, uint8_t
read_options.fill_cache = false;
rocksdb::WriteBatch batch;
- auto iter = db_->NewIterator(read_options, score_cf_handle_);
+ auto iter = DBUtil::UniqueIterator(db_, read_options, score_cf_handle_);
iter->Seek(start_key);
// see comment in rangebyscore()
if (reversed && (!iter->Valid() || !iter->key().starts_with(prefix_key))) {
@@ -272,7 +274,6 @@ rocksdb::Status ZSet::Range(const Slice &user_key, int start, int stop, uint8_t
}
if (count++ >= stop) break;
}
- delete iter;
if (removed_subkey) {
metadata.size -= removed_subkey;
@@ -354,7 +355,7 @@ rocksdb::Status ZSet::RangeByScore(const Slice &user_key,
read_options.fill_cache = false;
int pos = 0;
- auto iter = db_->NewIterator(read_options, score_cf_handle_);
+ auto iter = DBUtil::UniqueIterator(db_, read_options, score_cf_handle_);
rocksdb::WriteBatch batch;
WriteBatchLogData log_data(kRedisZSet);
batch.PutLogData(log_data.Encode());
@@ -393,7 +394,6 @@ rocksdb::Status ZSet::RangeByScore(const Slice &user_key,
if (size) *size += 1;
if (spec.count > 0 && mscores && mscores->size() >= static_cast<unsigned>(spec.count)) break;
}
- delete iter;
if (spec.removed && *size > 0) {
metadata.size -= *size;
@@ -438,7 +438,7 @@ rocksdb::Status ZSet::RangeByLex(const Slice &user_key,
read_options.fill_cache = false;
int pos = 0;
- auto iter = db_->NewIterator(read_options);
+ auto iter = DBUtil::UniqueIterator(db_, read_options);
rocksdb::WriteBatch batch;
WriteBatchLogData log_data(kRedisZSet);
batch.PutLogData(log_data.Encode());
@@ -483,7 +483,6 @@ rocksdb::Status ZSet::RangeByLex(const Slice &user_key,
if (size) *size += 1;
if (spec.count > 0 && members && members->size() >= static_cast<unsigned>(spec.count)) break;
}
- delete iter;
if (spec.removed && *size > 0) {
metadata.size -= *size;
@@ -601,7 +600,7 @@ rocksdb::Status ZSet::Rank(const Slice &user_key, const Slice &member, bool reve
read_options.iterate_lower_bound = &lower_bound;
read_options.fill_cache = false;
- auto iter = db_->NewIterator(read_options, score_cf_handle_);
+ auto iter = DBUtil::UniqueIterator(db_, read_options, score_cf_handle_);
iter->Seek(start_key);
// see comment in rangebyscore()
if (reversed && (!iter->Valid() || !iter->key().starts_with(prefix_key))) {
@@ -617,7 +616,6 @@ rocksdb::Status ZSet::Rank(const Slice &user_key, const Slice &member, bool reve
if (score == target_score && score_key == member) break;
rank++;
}
- delete iter;
*ret = rank;
return rocksdb::Status::OK();
diff --git a/src/util.h b/src/util.h
index 516ef98..f673527 100644
--- a/src/util.h
+++ b/src/util.h
@@ -72,7 +72,7 @@ uint64_t GetTimeStampUS(void);
// refer to https://en.cppreference.com/w/cpp/memory/unique_ptr/make_unique
template <typename T, typename... Args>
std::unique_ptr<T> MakeUnique(Args&& ... args) {
- return std::unique_ptr<T>(new T(std::forward<Args>(args)...));
+ return std::unique_ptr<T>(new T(std::forward<Args>(args)...));
}
} // namespace Util