mapleFU commented on code in PR #1710:
URL: https://github.com/apache/kvrocks/pull/1710#discussion_r1311108658
##########
src/commands/cmd_bloom_filter.cc:
##########
@@ -104,14 +105,86 @@ class CommandBFExists : public Commander {
redis::BloomChain bloom_db(svr->storage, conn->GetNamespace());
int ret = 0;
auto s = bloom_db.Exist(args_[1], args_[2], &ret);
+ if (s.IsNotFound()) return {Status::RedisExecErr, "key is not found"};
if (!s.ok()) return {Status::RedisExecErr, s.ToString()};
*output = redis::Integer(ret);
return Status::OK();
}
};
+class CommandBFInfo : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ CommandParser parser(args, 2);
+ while (parser.Good()) {
+ if (parser.EatEqICase("capacity")) {
+ type_ = BloomInfoType::kCapacity;
+ } else if (parser.EatEqICase("size")) {
+ type_ = BloomInfoType::kSize;
+ } else if (parser.EatEqICase("filters")) {
+ type_ = BloomInfoType::kFilters;
+ } else if (parser.EatEqICase("items")) {
+ type_ = BloomInfoType::kItems;
+ } else if (parser.EatEqICase("expansion")) {
+ type_ = BloomInfoType::kExpansion;
+ } else {
+ return {Status::RedisParseErr, "Invalid info argument"};
+ }
+ }
+
+ return Commander::Parse(args);
+ }
+
+ Status Execute(Server *svr, Connection *conn, std::string *output) override {
+ redis::BloomChain bloom_db(svr->storage, conn->GetNamespace());
+ BloomFilterInfo info;
+ auto s = bloom_db.Info(args_[1], &info);
+ if (s.IsNotFound()) return {Status::RedisExecErr, "key is not found"};
+ if (!s.ok()) return {Status::RedisExecErr, s.ToString()};
+
+ switch (type_) {
+ case BloomInfoType::kAll:
+ *output = redis::MultiLen(2 * 5);
+ *output += redis::SimpleString("Capacity");
+ *output += redis::Integer(info.capacity);
+ *output += redis::SimpleString("Size");
+ *output += redis::Integer(info.bloom_bytes);
+ *output += redis::SimpleString("Number of filters");
+ *output += redis::Integer(info.n_filters);
+ *output += redis::SimpleString("Number of items inserted");
+ *output += redis::Integer(info.size);
+ *output += redis::SimpleString("Expansion rate");
+ *output += redis::Integer(info.expansion);
Review Comment:
https://github.com/RedisBloom/RedisBloom/blob/master/src/rebloom.c#L990
Since `info.expansion` might be null if it's 0?
##########
src/commands/cmd_bloom_filter.cc:
##########
@@ -104,14 +105,86 @@ class CommandBFExists : public Commander {
redis::BloomChain bloom_db(svr->storage, conn->GetNamespace());
int ret = 0;
auto s = bloom_db.Exist(args_[1], args_[2], &ret);
+ if (s.IsNotFound()) return {Status::RedisExecErr, "key is not found"};
if (!s.ok()) return {Status::RedisExecErr, s.ToString()};
*output = redis::Integer(ret);
return Status::OK();
}
};
+class CommandBFInfo : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ CommandParser parser(args, 2);
+ while (parser.Good()) {
Review Comment:
```suggestion
if (parser.Good()) {
```
##########
src/commands/cmd_bloom_filter.cc:
##########
@@ -104,14 +105,86 @@ class CommandBFExists : public Commander {
redis::BloomChain bloom_db(svr->storage, conn->GetNamespace());
int ret = 0;
auto s = bloom_db.Exist(args_[1], args_[2], &ret);
+ if (s.IsNotFound()) return {Status::RedisExecErr, "key is not found"};
if (!s.ok()) return {Status::RedisExecErr, s.ToString()};
*output = redis::Integer(ret);
return Status::OK();
}
};
+class CommandBFInfo : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ CommandParser parser(args, 2);
Review Comment:
We'd better `args` should not greater than 3?
https://github.com/RedisBloom/RedisBloom/blob/master/src/rebloom.c#L949
##########
src/types/redis_bloom_chain.cc:
##########
@@ -177,22 +168,41 @@ rocksdb::Status BloomChain::Exist(const Slice &user_key,
const Slice &item, int
BloomChainMetadata metadata;
rocksdb::Status s = getBloomChainMetadata(ns_key, &metadata);
- if (s.IsNotFound()) return rocksdb::Status::NotFound("key is not found");
if (!s.ok()) return s;
std::vector<std::string> bf_key_list;
getBFKeyList(ns_key, metadata, &bf_key_list);
std::string item_string = item.ToString();
// check
+ bool exist = false;
for (int i = metadata.n_filters - 1; i >= 0; --i) { // TODO: to test which
direction for searching is better
- s = bloomCheck(bf_key_list[i], item_string, ret);
+ s = bloomCheck(bf_key_list[i], item_string, &exist);
if (!s.ok()) return s;
- if (*ret == 1) {
+ if (exist) {
break;
}
}
+ *ret = exist ? 1 : 0;
return rocksdb::Status::OK();
}
+
+rocksdb::Status BloomChain::Info(const Slice &user_key, BloomFilterInfo *info)
{
+ std::string ns_key = AppendNamespacePrefix(user_key);
+ LockGuard guard(storage_->GetLockManager(), ns_key);
Review Comment:
`Info` doesn't need to lock since it just atomicly get the metadata?
##########
src/commands/cmd_bloom_filter.cc:
##########
@@ -104,14 +105,86 @@ class CommandBFExists : public Commander {
redis::BloomChain bloom_db(svr->storage, conn->GetNamespace());
int ret = 0;
auto s = bloom_db.Exist(args_[1], args_[2], &ret);
+ if (s.IsNotFound()) return {Status::RedisExecErr, "key is not found"};
if (!s.ok()) return {Status::RedisExecErr, s.ToString()};
*output = redis::Integer(ret);
return Status::OK();
}
};
+class CommandBFInfo : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ CommandParser parser(args, 2);
+ while (parser.Good()) {
+ if (parser.EatEqICase("capacity")) {
+ type_ = BloomInfoType::kCapacity;
+ } else if (parser.EatEqICase("size")) {
+ type_ = BloomInfoType::kSize;
+ } else if (parser.EatEqICase("filters")) {
+ type_ = BloomInfoType::kFilters;
+ } else if (parser.EatEqICase("items")) {
+ type_ = BloomInfoType::kItems;
+ } else if (parser.EatEqICase("expansion")) {
+ type_ = BloomInfoType::kExpansion;
+ } else {
+ return {Status::RedisParseErr, "Invalid info argument"};
+ }
+ }
+
+ return Commander::Parse(args);
+ }
+
+ Status Execute(Server *svr, Connection *conn, std::string *output) override {
+ redis::BloomChain bloom_db(svr->storage, conn->GetNamespace());
+ BloomFilterInfo info;
+ auto s = bloom_db.Info(args_[1], &info);
+ if (s.IsNotFound()) return {Status::RedisExecErr, "key is not found"};
+ if (!s.ok()) return {Status::RedisExecErr, s.ToString()};
+
+ switch (type_) {
+ case BloomInfoType::kAll:
+ *output = redis::MultiLen(2 * 5);
+ *output += redis::SimpleString("Capacity");
+ *output += redis::Integer(info.capacity);
+ *output += redis::SimpleString("Size");
+ *output += redis::Integer(info.bloom_bytes);
+ *output += redis::SimpleString("Number of filters");
+ *output += redis::Integer(info.n_filters);
+ *output += redis::SimpleString("Number of items inserted");
+ *output += redis::Integer(info.size);
+ *output += redis::SimpleString("Expansion rate");
+ *output += redis::Integer(info.expansion);
+ break;
+ case BloomInfoType::kCapacity:
+ *output = redis::Integer(info.capacity);
+ break;
+ case BloomInfoType::kSize:
+ *output = redis::Integer(info.bloom_bytes);
+ break;
+ case BloomInfoType::kFilters:
+ *output = redis::Integer(info.n_filters);
+ break;
+ case BloomInfoType::kItems:
+ *output = redis::Integer(info.size);
+ break;
+ case BloomInfoType::kExpansion:
+ *output = redis::Integer(info.expansion);
+ break;
+ default:
+ LOG(ERROR) << "Failed to parse the type of BF.INFO command";
Review Comment:
return error here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
