[GitHub] [kvrocks] mapleFU commented on a diff in pull request #1756: Add the support of the BF.MEXISTS and BF.CARD Command

["mapleFU (via GitHub)" <gi...@apache.org>]

mapleFU commented on code in PR #1756:
URL: https://github.com/apache/kvrocks/pull/1756#discussion_r1323087921


##########
src/commands/cmd_bloom_filter.cc:
##########
@@ -105,14 +105,37 @@ class CommandBFExists : public Commander {
   Status Execute(Server *svr, Connection *conn, std::string *output) override {
     redis::BloomChain bloom_db(svr->storage, conn->GetNamespace());
     int ret = 0;
-    auto s = bloom_db.Exist(args_[1], args_[2], &ret);
+    auto s = bloom_db.Exists(args_[1], args_[2], &ret);
     if (!s.ok()) return {Status::RedisExecErr, s.ToString()};
 
     *output = redis::Integer(ret);
     return Status::OK();
   }
 };
 
+class CommandBFMExists : public Commander {
+ public:
+  Status Parse(const std::vector<std::string> &args) override {
+    for (size_t i = 2; i < args_.size(); ++i) {

Review Comment:
   reserve before emplace



##########
src/types/redis_bloom_chain.cc:
##########
@@ -185,31 +185,40 @@ rocksdb::Status BloomChain::Add(const Slice &user_key, const Slice &item, int *r
   return storage_->Write(storage_->DefaultWriteOptions(), batch->GetWriteBatch());
 }
 
-rocksdb::Status BloomChain::Exist(const Slice &user_key, const Slice &item, int *ret) {
+rocksdb::Status BloomChain::Exists(const Slice &user_key, const Slice &item, int *ret) {
+  std::vector<int> tmp(1, 0);
+  rocksdb::Status s = MExists(user_key, {item}, &tmp);
+  *ret = tmp[0];
+  return s;
+}
+
+rocksdb::Status BloomChain::MExists(const Slice &user_key, const std::vector<Slice> &items, std::vector<int> *rets) {
   std::string ns_key = AppendNamespacePrefix(user_key);
 
   BloomChainMetadata metadata;
   rocksdb::Status s = getBloomChainMetadata(ns_key, &metadata);
   if (s.IsNotFound()) {
-    *ret = 0;
+    std::fill(rets->begin(), rets->end(), 0);
     return rocksdb::Status::OK();
   }
   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, &exist);
-    if (!s.ok()) return s;
-    if (exist) {
-      break;
+  for (size_t i = 0; i < items.size(); ++i) {
+    std::string item_string = items[i].ToString();
+    // check
+    bool exist = false;
+    for (int ii = metadata.n_filters - 1; ii >= 0; --ii) {  // TODO: to test which direction for searching is better
+      s = bloomCheck(bf_key_list[ii], item_string, &exist);

Review Comment:
   Can we support `bloomCheck` with vector? Otherwise, bloom filter will be get multiple times, which might not good for LRU and BlockCache



-- 
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: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org