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.

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 the support of the bloom BF.INFO command #1710

Merged
merged 15 commits into from
Aug 31, 2023
Prev Previous commit
Next Next commit
Update BF.INFO execute
zncleon committed Aug 30, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
commit c55afdeb128693194c30b77ade9a42ff7533bd4e
60 changes: 39 additions & 21 deletions src/commands/cmd_bloom_filter.cc
Original file line number Diff line number Diff line change
@@ -118,15 +118,15 @@ class CommandBFInfo : public Commander {
CommandParser parser(args, 2);
mapleFU marked this conversation as resolved.
Show resolved Hide resolved
while (parser.Good()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
while (parser.Good()) {
if (parser.Good()) {

if (parser.EatEqICase("capacity")) {
type_ = CAPACITY;
type_ = BloomInfoType::CAPACITY;
} else if (parser.EatEqICase("size")) {
type_ = SIZE;
type_ = BloomInfoType::SIZE;
} else if (parser.EatEqICase("filters")) {
type_ = FILTERS;
type_ = BloomInfoType::FILTERS;
} else if (parser.EatEqICase("items")) {
type_ = ITEMS;
type_ = BloomInfoType::ITEMS;
} else if (parser.EatEqICase("expansion")) {
type_ = EXPANSION;
type_ = BloomInfoType::EXPANSION;
} else {
return {Status::RedisParseErr, "Invalid info argument"};
}
@@ -137,30 +137,48 @@ class CommandBFInfo : public Commander {

Status Execute(Server *svr, Connection *conn, std::string *output) override {
redis::BloomChain bloom_db(svr->storage, conn->GetNamespace());
std::vector<int> rets;
rets.reserve(all_nums_);
auto s = bloom_db.Info(args_[1], type_, &rets);
BloomFilterInfo info;
auto s = bloom_db.Info(args_[1], &info);
if (!s.ok()) return {Status::RedisExecErr, s.ToString()};

if (type_ == ALL) {
*output = "*" + std::to_string(2 * all_nums_) +
CRLF; // todo: this reply only used in here, whether should I place it in redis_reply.h
for (int i = 0; i < all_nums_; ++i) {
*output += redis::SimpleString(all_info_rets_[i]);
*output += redis::Integer(rets[i]);
}
} else {
*output = redis::Integer(rets[0]);
switch (type_) {
case BloomInfoType::ALL:
*output = "*" + std::to_string(2 * 5) + CRLF;
mapleFU marked this conversation as resolved.
Show resolved Hide resolved
*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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

break;
case BloomInfoType::CAPACITY:
*output = redis::Integer(info.capacity);
break;
case BloomInfoType::SIZE:
*output = redis::Integer(info.bloom_bytes);
break;
case BloomInfoType::FILTERS:
*output = redis::Integer(info.n_filters);
break;
case BloomInfoType::ITEMS:
*output = redis::Integer(info.size);
break;
case BloomInfoType::EXPANSION:
*output = redis::Integer(info.expansion);
break;
default:
LOG(ERROR) << "Failed to parse the type of BF.INFO command";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return error here?

Copy link
Contributor Author

@zncleon zncleon Aug 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I add a return here, lint would prompt an error: "Unreachable code ".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah, we can just remove this branch.

}

return Status::OK();
}

private:
BloomInfoType type_ = ALL;
const int all_nums_ = 5;
const std::vector<std::string> all_info_rets_ = {"Capacity", "Size", "Number of filters", "Number of items inserted",
"Expansion rate"};
BloomInfoType type_ = BloomInfoType::ALL;
};

REDIS_REGISTER_COMMANDS(MakeCmdAttr<CommandBFReserve>("bf.reserve", -4, "write", 1, 1, 1),
33 changes: 6 additions & 27 deletions src/types/redis_bloom_chain.cc
Original file line number Diff line number Diff line change
@@ -196,7 +196,7 @@ rocksdb::Status BloomChain::Exist(const Slice &user_key, const Slice &item, int
return rocksdb::Status::OK();
}

rocksdb::Status BloomChain::Info(const Slice &user_key, const BloomInfoType &type, std::vector<int> *rets) {
rocksdb::Status BloomChain::Info(const Slice &user_key, BloomFilterInfo *info) {
std::string ns_key = AppendNamespacePrefix(user_key);
LockGuard guard(storage_->GetLockManager(), ns_key);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Info doesn't need to lock since it just atomicly get the metadata?


@@ -205,32 +205,11 @@ rocksdb::Status BloomChain::Info(const Slice &user_key, const BloomInfoType &typ
if (s.IsNotFound()) return rocksdb::Status::NotFound("key is not found");
mapleFU marked this conversation as resolved.
Show resolved Hide resolved
if (!s.ok()) return s;

switch (type) {
case ALL:
rets->push_back(static_cast<int>(metadata.GetCapacity()));
rets->push_back(static_cast<int>(metadata.bloom_bytes));
rets->push_back(static_cast<int>(metadata.n_filters));
rets->push_back(static_cast<int>(metadata.size));
rets->push_back(static_cast<int>(metadata.expansion));
break;
case CAPACITY:
rets->push_back(static_cast<int>(metadata.GetCapacity()));
break;
case SIZE:
rets->push_back(static_cast<int>(metadata.bloom_bytes));
break;
case FILTERS:
rets->push_back(static_cast<int>(metadata.n_filters));
break;
case ITEMS:
rets->push_back(static_cast<int>(metadata.size));
break;
case EXPANSION:
rets->push_back(static_cast<int>(metadata.expansion));
break;
default:
LOG(ERROR) << "Failed to parse the type of BF.INFO command";
}
info->capacity = metadata.GetCapacity();
info->bloom_bytes = metadata.bloom_bytes;
info->n_filters = metadata.n_filters;
info->size = metadata.size;
info->expansion = metadata.expansion;

return rocksdb::Status::OK();
}
12 changes: 10 additions & 2 deletions src/types/redis_bloom_chain.h
Original file line number Diff line number Diff line change
@@ -30,7 +30,7 @@ const uint32_t kBFDefaultInitCapacity = 100;
const double kBFDefaultErrorRate = 0.01;
const uint16_t kBFDefaultExpansion = 2;

enum BloomInfoType {
enum class BloomInfoType {
ALL,
mapleFU marked this conversation as resolved.
Show resolved Hide resolved
CAPACITY,
SIZE,
@@ -39,13 +39,21 @@ enum BloomInfoType {
EXPANSION,
};

struct BloomFilterInfo {
uint32_t capacity;
uint32_t bloom_bytes;
uint16_t n_filters;
uint64_t size;
uint16_t expansion;
};

class BloomChain : public Database {
public:
BloomChain(engine::Storage *storage, const std::string &ns) : Database(storage, ns) {}
rocksdb::Status Reserve(const Slice &user_key, uint32_t capacity, double error_rate, uint16_t expansion);
rocksdb::Status Add(const Slice &user_key, const Slice &item, int *ret);
rocksdb::Status Exist(const Slice &user_key, const Slice &item, int *ret);
rocksdb::Status Info(const Slice &user_key, const BloomInfoType &type, std::vector<int> *rets);
rocksdb::Status Info(const Slice &user_key, BloomFilterInfo *info);

private:
std::string getBFKey(const Slice &ns_key, const BloomChainMetadata &metadata, uint16_t filters_index);