Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add nodiscard to methods of redis::Database #1752

Merged
merged 2 commits into from
Sep 11, 2023
Merged

Add nodiscard to methods of redis::Database #1752

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2148506
Add nodiscard to methods of redis::Database
PragmaTwice Sep 11, 2023
01a7abd
Merge branch 'unstable' into nodiscard-database
git-hulk Sep 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/cluster/cluster.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,10 @@ Status Cluster::SetSlotRanges(const std::vector<SlotRange> &slot_ranges, const s
if (old_node == myself_ && old_node != to_assign_node) {
// If slot is migrated from this node
if (migrated_slots_.count(slot) > 0) {
svr_->slot_migrator->ClearKeysOfSlot(kDefaultNamespace, slot);
auto s = svr_->slot_migrator->ClearKeysOfSlot(kDefaultNamespace, slot);
if (!s.ok()) {
LOG(ERROR) << "failed to clear data of migrated slot: " << s.ToString();
}
migrated_slots_.erase(slot);
}
// If slot is imported into this node
Expand Down Expand Up @@ -209,7 +212,10 @@ Status Cluster::SetClusterNodes(const std::string &nodes_str, int64_t version, b
if (!migrated_slots_.empty()) {
for (auto &it : migrated_slots_) {
if (slots_nodes_[it.first] != myself_) {
svr_->slot_migrator->ClearKeysOfSlot(kDefaultNamespace, it.first);
auto s = svr_->slot_migrator->ClearKeysOfSlot(kDefaultNamespace, it.first);
if (!s.ok()) {
LOG(ERROR) << "failed to clear data of migrated slots: " << s.ToString();
}
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/commands/cmd_key.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,8 @@ class CommandExists : public Commander {

int cnt = 0;
redis::Database redis(svr->storage, conn->GetNamespace());
redis.Exists(keys, &cnt);
auto s = redis.Exists(keys, &cnt);
if (!s.ok()) return {Status::RedisExecErr, s.ToString()};
*output = redis::Integer(cnt);

return Status::OK();
Expand Down
14 changes: 11 additions & 3 deletions src/commands/cmd_server.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,14 +136,19 @@ class CommandKeys : public Commander {
std::string prefix = args_[1];
std::vector<std::string> keys;
redis::Database redis(svr->storage, conn->GetNamespace());

rocksdb::Status s;
if (prefix == "*") {
redis.Keys(std::string(), &keys);
s = redis.Keys(std::string(), &keys);
} else {
if (prefix[prefix.size() - 1] != '*') {
return {Status::RedisExecErr, "only keys prefix match was supported"};
}

redis.Keys(prefix.substr(0, prefix.size() - 1), &keys);
s = redis.Keys(prefix.substr(0, prefix.size() - 1), &keys);
}
if (!s.ok()) {
return {Status::RedisExecErr, s.ToString()};
}
*output = redis::MultiBulkString(keys);
return Status::OK();
Expand Down Expand Up @@ -802,7 +807,10 @@ class CommandRandomKey : public Commander {
std::string key;
auto cursor = svr->GetLastRandomKeyCursor();
redis::Database redis(svr->storage, conn->GetNamespace());
redis.RandomKey(cursor, &key);
auto s = redis.RandomKey(cursor, &key);
if (!s.ok()) {
return {Status::RedisExecErr, s.ToString()};
}
svr->SetLastRandomKeyCursor(key);
*output = redis::BulkString(key);
return Status::OK();
Expand Down
7 changes: 5 additions & 2 deletions src/commands/cmd_string.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,14 +296,17 @@ class CommandSet : public Commander {
Status Execute(Server *svr, Connection *conn, std::string *output) override {
bool ret = false;
redis::String string_db(svr->storage, conn->GetNamespace());
rocksdb::Status s;

if (ttl_ < 0) {
string_db.Del(args_[1]);
auto s = string_db.Del(args_[1]);
if (!s.ok()) {
return {Status::RedisExecErr, s.ToString()};
}
*output = redis::SimpleString("OK");
return Status::OK();
}

rocksdb::Status s;
if (set_flag_ == NX) {
s = string_db.SetNX(args_[1], args_[2], ttl_, &ret);
} else if (set_flag_ == XX) {
Expand Down
6 changes: 5 additions & 1 deletion src/server/server.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1304,8 +1304,12 @@ Status Server::AsyncScanDBSize(const std::string &ns) {

return task_runner_.TryPublish([ns, this] {
redis::Database db(storage, ns);

KeyNumStats stats;
db.GetKeyNumStats("", &stats);
auto s = db.GetKeyNumStats("", &stats);
if (!s.ok()) {
LOG(ERROR) << "failed to retrieve key num stats: " << s.ToString();
}

std::lock_guard<std::mutex> lg(db_job_mu_);

Expand Down
5 changes: 2 additions & 3 deletions src/storage/redis_db.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,7 @@ rocksdb::Status Database::GetRawMetadata(const Slice &ns_key, std::string *bytes
}

rocksdb::Status Database::GetRawMetadataByUserKey(const Slice &user_key, std::string *bytes) {
std::string ns_key = AppendNamespacePrefix(user_key);
return GetRawMetadata(ns_key, bytes);
return GetRawMetadata(AppendNamespacePrefix(user_key), bytes);
}

rocksdb::Status Database::Expire(const Slice &user_key, uint64_t timestamp) {
Expand Down Expand Up @@ -479,7 +478,7 @@ rocksdb::Status Database::Dump(const Slice &user_key, std::vector<std::string> *

if (metadata.Type() == kRedisList) {
ListMetadata list_metadata(false);
GetMetadata(kRedisList, ns_key, &list_metadata);
s = GetMetadata(kRedisList, ns_key, &list_metadata);
if (!s.ok()) return s.IsNotFound() ? rocksdb::Status::OK() : s;
infos->emplace_back("head");
infos->emplace_back(std::to_string(list_metadata.head));
Expand Down
47 changes: 24 additions & 23 deletions src/storage/redis_db.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,30 +34,31 @@ class Database {
static constexpr uint64_t RANDOM_KEY_SCAN_LIMIT = 60;

explicit Database(engine::Storage *storage, std::string ns = "");
rocksdb::Status GetMetadata(RedisType type, const Slice &ns_key, Metadata *metadata);
rocksdb::Status GetRawMetadata(const Slice &ns_key, std::string *bytes);
rocksdb::Status GetRawMetadataByUserKey(const Slice &user_key, std::string *bytes);
rocksdb::Status Expire(const Slice &user_key, uint64_t timestamp);
rocksdb::Status Del(const Slice &user_key);
rocksdb::Status MDel(const std::vector<Slice> &keys, uint64_t *deleted_cnt);
rocksdb::Status Exists(const std::vector<Slice> &keys, int *ret);
rocksdb::Status TTL(const Slice &user_key, int64_t *ttl);
rocksdb::Status Type(const Slice &user_key, RedisType *type);
rocksdb::Status Dump(const Slice &user_key, std::vector<std::string> *infos);
rocksdb::Status FlushDB();
rocksdb::Status FlushAll();
rocksdb::Status GetKeyNumStats(const std::string &prefix, KeyNumStats *stats);
rocksdb::Status Keys(const std::string &prefix, std::vector<std::string> *keys = nullptr,
KeyNumStats *stats = nullptr);
rocksdb::Status Scan(const std::string &cursor, uint64_t limit, const std::string &prefix,
std::vector<std::string> *keys, std::string *end_cursor = nullptr);
rocksdb::Status RandomKey(const std::string &cursor, std::string *key);
[[nodiscard]] rocksdb::Status GetMetadata(RedisType type, const Slice &ns_key, Metadata *metadata);
[[nodiscard]] rocksdb::Status GetRawMetadata(const Slice &ns_key, std::string *bytes);
[[nodiscard]] rocksdb::Status GetRawMetadataByUserKey(const Slice &user_key, std::string *bytes);
[[nodiscard]] rocksdb::Status Expire(const Slice &user_key, uint64_t timestamp);
[[nodiscard]] rocksdb::Status Del(const Slice &user_key);
[[nodiscard]] rocksdb::Status MDel(const std::vector<Slice> &keys, uint64_t *deleted_cnt);
[[nodiscard]] rocksdb::Status Exists(const std::vector<Slice> &keys, int *ret);
[[nodiscard]] rocksdb::Status TTL(const Slice &user_key, int64_t *ttl);
[[nodiscard]] rocksdb::Status Type(const Slice &user_key, RedisType *type);
[[nodiscard]] rocksdb::Status Dump(const Slice &user_key, std::vector<std::string> *infos);
[[nodiscard]] rocksdb::Status FlushDB();
[[nodiscard]] rocksdb::Status FlushAll();
[[nodiscard]] rocksdb::Status GetKeyNumStats(const std::string &prefix, KeyNumStats *stats);
[[nodiscard]] rocksdb::Status Keys(const std::string &prefix, std::vector<std::string> *keys = nullptr,
KeyNumStats *stats = nullptr);
[[nodiscard]] rocksdb::Status Scan(const std::string &cursor, uint64_t limit, const std::string &prefix,
std::vector<std::string> *keys, std::string *end_cursor = nullptr);
[[nodiscard]] rocksdb::Status RandomKey(const std::string &cursor, std::string *key);
std::string AppendNamespacePrefix(const Slice &user_key);
rocksdb::Status FindKeyRangeWithPrefix(const std::string &prefix, const std::string &prefix_end, std::string *begin,
std::string *end, rocksdb::ColumnFamilyHandle *cf_handle = nullptr);
rocksdb::Status ClearKeysOfSlot(const rocksdb::Slice &ns, int slot);
rocksdb::Status GetSlotKeysInfo(int slot, std::map<int, uint64_t> *slotskeys, std::vector<std::string> *keys,
int count);
[[nodiscard]] rocksdb::Status FindKeyRangeWithPrefix(const std::string &prefix, const std::string &prefix_end,
std::string *begin, std::string *end,
rocksdb::ColumnFamilyHandle *cf_handle = nullptr);
[[nodiscard]] rocksdb::Status ClearKeysOfSlot(const rocksdb::Slice &ns, int slot);
[[nodiscard]] rocksdb::Status GetSlotKeysInfo(int slot, std::map<int, uint64_t> *slotskeys,
std::vector<std::string> *keys, int count);

protected:
engine::Storage *storage_;
Expand Down
3 changes: 2 additions & 1 deletion src/types/redis_geo.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ rocksdb::Status Geo::SearchStore(const Slice &user_key, GeoShape geo_shape, Orig
auto result_length = static_cast<int64_t>(geo_points->size());
int64_t returned_items_count = (count == 0 || result_length < count) ? result_length : count;
if (returned_items_count == 0) {
ZSet::Del(user_key);
auto s = ZSet::Del(user_key);
if (!s.ok()) return s;
} else {
std::vector<MemberScore> member_scores;
for (const auto &geo_point : *geo_points) {
Expand Down
3 changes: 2 additions & 1 deletion src/types/redis_string.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,8 @@ rocksdb::Status String::SetXX(const std::string &user_key, const std::string &va

std::string ns_key = AppendNamespacePrefix(user_key);
LockGuard guard(storage_->GetLockManager(), ns_key);
Exists({user_key}, &exists);
auto s = Exists({user_key}, &exists);
if (!s.ok()) return s;
if (exists != 1) return rocksdb::Status::OK();

*flag = true;
Expand Down
4 changes: 2 additions & 2 deletions tests/cppunit/compact_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ TEST(Compact, Filter) {
std::string live_hash_key = "live_hash_key";
hash->Set(expired_hash_key, "f1", "v1", &ret);
hash->Set(expired_hash_key, "f2", "v2", &ret);
hash->Expire(expired_hash_key, 1); // expired
auto st = hash->Expire(expired_hash_key, 1); // expired
usleep(10000);
hash->Set(live_hash_key, "f1", "v1", &ret);
hash->Set(live_hash_key, "f2", "v2", &ret);
Expand Down Expand Up @@ -75,7 +75,7 @@ TEST(Compact, Filter) {
std::string expired_zset_key = "expire_zset_key";
std::vector<MemberScore> member_scores = {MemberScore{"z1", 1.1}, MemberScore{"z2", 0.4}};
zset->Add(expired_zset_key, ZAddFlags::Default(), &member_scores, &ret);
zset->Expire(expired_zset_key, 1); // expired
st = zset->Expire(expired_zset_key, 1); // expired
usleep(10000);

status = storage->Compact(nullptr, nullptr);
Expand Down
20 changes: 10 additions & 10 deletions tests/cppunit/disk_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ TEST_F(RedisDiskTest, StringDisk) {
EXPECT_TRUE(disk->GetKeySize(key_, kRedisString, &result).ok());
EXPECT_GE(result, value_size[0] * estimation_factor_);
EXPECT_LE(result, value_size[0] / estimation_factor_);
string->Del(key_);
auto s = string->Del(key_);
}

TEST_F(RedisDiskTest, HashDisk) {
Expand All @@ -78,7 +78,7 @@ TEST_F(RedisDiskTest, HashDisk) {
EXPECT_TRUE(disk->GetKeySize(key_, kRedisHash, &key_size).ok());
EXPECT_GE(key_size, approximate_size * estimation_factor_);
EXPECT_LE(key_size, approximate_size / estimation_factor_);
hash->Del(key_);
auto s = hash->Del(key_);
}

TEST_F(RedisDiskTest, SetDisk) {
Expand All @@ -103,7 +103,7 @@ TEST_F(RedisDiskTest, SetDisk) {
EXPECT_GE(key_size, approximate_size * estimation_factor_);
EXPECT_LE(key_size, approximate_size / estimation_factor_);

set->Del(key_);
s = set->Del(key_);
}

TEST_F(RedisDiskTest, ListDisk) {
Expand All @@ -126,7 +126,7 @@ TEST_F(RedisDiskTest, ListDisk) {
EXPECT_TRUE(disk->GetKeySize(key_, kRedisList, &key_size).ok());
EXPECT_GE(key_size, approximate_size * estimation_factor_);
EXPECT_LE(key_size, approximate_size / estimation_factor_);
list->Del(key_);
s = list->Del(key_);
}

TEST_F(RedisDiskTest, ZsetDisk) {
Expand All @@ -148,7 +148,7 @@ TEST_F(RedisDiskTest, ZsetDisk) {
EXPECT_TRUE(disk->GetKeySize(key_, kRedisZSet, &key_size).ok());
EXPECT_GE(key_size, approximate_size * estimation_factor_);
EXPECT_LE(key_size, approximate_size / estimation_factor_);
zset->Del(key_);
s = zset->Del(key_);
}

TEST_F(RedisDiskTest, BitmapDisk) {
Expand All @@ -165,7 +165,7 @@ TEST_F(RedisDiskTest, BitmapDisk) {
EXPECT_TRUE(disk->GetKeySize(key_, kRedisBitmap, &key_size).ok());
EXPECT_GE(key_size, approximate_size * estimation_factor_);
EXPECT_LE(key_size, approximate_size / estimation_factor_);
bitmap->Del(key_);
auto s = bitmap->Del(key_);
}

TEST_F(RedisDiskTest, BitmapDisk2) {
Expand All @@ -175,7 +175,7 @@ TEST_F(RedisDiskTest, BitmapDisk2) {
std::unique_ptr<redis::Bitmap> bitmap = std::make_unique<redis::Bitmap>(storage_, "disk_ns_bitmap2");
std::unique_ptr<redis::Disk> disk = std::make_unique<redis::Disk>(storage_, "disk_ns_bitmap2");
key_ = "bitmapdisk_key2";
bitmap->Del(key_);
auto s = bitmap->Del(key_);
bool bit = false;

for (size_t i = 0; i < num_bits; i += kGroupSize) {
Expand Down Expand Up @@ -222,7 +222,7 @@ TEST_F(RedisDiskTest, SortedintDisk) {
EXPECT_TRUE(disk->GetKeySize(key_, kRedisSortedint, &key_size).ok());
EXPECT_GE(key_size, approximate_size * estimation_factor_);
EXPECT_LE(key_size, approximate_size / estimation_factor_);
sortedint->Del(key_);
auto s = sortedint->Del(key_);
}

TEST_F(RedisDiskTest, StreamDisk) {
Expand All @@ -243,5 +243,5 @@ TEST_F(RedisDiskTest, StreamDisk) {
EXPECT_TRUE(disk->GetKeySize(key_, kRedisStream, &key_size).ok());
EXPECT_GE(key_size, approximate_size * estimation_factor_);
EXPECT_LE(key_size, approximate_size / estimation_factor_);
stream->Del(key_);
}
auto s = stream->Del(key_);
}
8 changes: 4 additions & 4 deletions tests/cppunit/metadata_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ TEST_F(RedisTypeTest, GetMetadata) {
EXPECT_TRUE(s.ok() && fvs.size() == ret);
HashMetadata metadata;
std::string ns_key = redis_->AppendNamespacePrefix(key_);
redis_->GetMetadata(kRedisHash, ns_key, &metadata);
s = redis_->GetMetadata(kRedisHash, ns_key, &metadata);
EXPECT_EQ(fvs.size(), metadata.size);
s = redis_->Del(key_);
EXPECT_TRUE(s.ok());
Expand All @@ -106,12 +106,12 @@ TEST_F(RedisTypeTest, Expire) {
EXPECT_TRUE(s.ok() && fvs.size() == ret);
int64_t now = 0;
rocksdb::Env::Default()->GetCurrentTime(&now);
redis_->Expire(key_, now * 1000 + 2000);
s = redis_->Expire(key_, now * 1000 + 2000);
int64_t ttl = 0;
redis_->TTL(key_, &ttl);
s = redis_->TTL(key_, &ttl);
ASSERT_GT(ttl, 0);
ASSERT_LE(ttl, 2000);
redis_->Del(key_);
s = redis_->Del(key_);
}

TEST(Metadata, MetadataDecodingBackwardCompatibleSimpleKey) {
Expand Down
8 changes: 4 additions & 4 deletions tests/cppunit/types/bitmap_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ TEST_F(RedisBitmapTest, GetAndSetBit) {
bitmap_->GetBit(key_, offset, &bit);
EXPECT_TRUE(bit);
}
bitmap_->Del(key_);
auto s = bitmap_->Del(key_);
}

TEST_F(RedisBitmapTest, BitCount) {
Expand All @@ -60,7 +60,7 @@ TEST_F(RedisBitmapTest, BitCount) {
EXPECT_EQ(cnt, 6);
bitmap_->BitCount(key_, 0, -1, &cnt);
EXPECT_EQ(cnt, 6);
bitmap_->Del(key_);
auto s = bitmap_->Del(key_);
}

TEST_F(RedisBitmapTest, BitPosClearBit) {
Expand All @@ -72,7 +72,7 @@ TEST_F(RedisBitmapTest, BitPosClearBit) {
bitmap_->SetBit(key_, i, true, &old_bit);
EXPECT_FALSE(old_bit);
}
bitmap_->Del(key_);
auto s = bitmap_->Del(key_);
}

TEST_F(RedisBitmapTest, BitPosSetBit) {
Expand All @@ -87,5 +87,5 @@ TEST_F(RedisBitmapTest, BitPosSetBit) {
bitmap_->BitPos(key_, true, start_indexes[i], -1, true, &pos);
EXPECT_EQ(pos, offsets[i]);
}
bitmap_->Del(key_);
auto s = bitmap_->Del(key_);
}
6 changes: 3 additions & 3 deletions tests/cppunit/types/bloom_chain_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,15 @@ TEST_F(RedisBloomChainTest, Reserve) {
EXPECT_FALSE(s.ok());
EXPECT_EQ(s.ToString(), "Invalid argument: the key already exists");

sb_chain_->Del(key_);
s = sb_chain_->Del(key_);
}

TEST_F(RedisBloomChainTest, BasicAddAndTest) {
int ret = 0;

auto s = sb_chain_->Exist("no_exist_key", "test_item", &ret);
EXPECT_EQ(ret, 0);
sb_chain_->Del("no_exist_key");
s = sb_chain_->Del("no_exist_key");

std::string insert_items[] = {"item1", "item2", "item3", "item101", "item202", "303"};
for (const auto& insert_item : insert_items) {
Expand All @@ -78,5 +78,5 @@ TEST_F(RedisBloomChainTest, BasicAddAndTest) {
EXPECT_TRUE(s.ok());
EXPECT_EQ(ret, 0);
}
sb_chain_->Del(key_);
s = sb_chain_->Del(key_);
}
Loading