Add the support of the bloom BF.INFO command #1710
Conversation
src/types/redis_bloom_chain.cc
Outdated
| @@ -151,6 +151,7 @@ rocksdb::Status BloomChain::Add(const Slice &user_key, const Slice &item, int *r | |||
| s = bloomCheck(bf_key_list[check_index], item_string, ret); | |||
There was a problem hiding this comment.
I think we should add another variable to identify the check result(e.g. exists or check_ret) instead of resuing the add result.
src/commands/cmd_bloom_filter.cc
Outdated
| *output += redis::SimpleString("Number of items inserted"); | ||
| *output += redis::Integer(info.size); | ||
| *output += redis::SimpleString("Expansion rate"); | ||
| *output += redis::Integer(info.expansion); |
There was a problem hiding this 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
Outdated
| public: | ||
| Status Parse(const std::vector<std::string> &args) override { | ||
| CommandParser parser(args, 2); | ||
| while (parser.Good()) { |
There was a problem hiding this comment.
| while (parser.Good()) { | |
| if (parser.Good()) { |
src/commands/cmd_bloom_filter.cc
Outdated
| *output = redis::Integer(info.expansion); | ||
| break; | ||
| default: | ||
| LOG(ERROR) << "Failed to parse the type of BF.INFO command"; |
There was a problem hiding this comment.
If I add a return here, lint would prompt an error: "Unreachable code ".
There was a problem hiding this comment.
Ah yeah, we can just remove this branch.
src/types/redis_bloom_chain.cc
Outdated
|
|
||
| rocksdb::Status BloomChain::Info(const Slice &user_key, BloomFilterInfo *info) { | ||
| std::string ns_key = AppendNamespacePrefix(user_key); | ||
| LockGuard guard(storage_->GetLockManager(), ns_key); |
There was a problem hiding this comment.
Info doesn't need to lock since it just atomicly get the metadata?
|
@zncleon BTW, according to the Redis documentation, the BF.EXISTS command returns 0 if the key doesn't exist. But I see that currently, the code returns an error |
Sure, I think I made a mistake. It should return 0 in this case. I will fix it. |
|
Thanks all, merging |
This PR introduce the bloom BF.INFO command.
[1] https://redis.io/commands/bf.info/