diff --git a/src/commands/cmd_cuckoo_filter.cc b/src/commands/cmd_cuckoo_filter.cc index 2002912f94d..cb4c2486432 100644 --- a/src/commands/cmd_cuckoo_filter.cc +++ b/src/commands/cmd_cuckoo_filter.cc @@ -131,8 +131,33 @@ class CommandCFAdd : public Commander { } }; -// Register the CF.RESERVE and CF.ADD commands +class CommandCFDel : public Commander { + public: + Status Parse(const std::vector &args) override { + // CF.DEL key item + if (args.size() != 3) { + return {Status::RedisParseErr, errWrongNumOfArguments}; + } + return Commander::Parse(args); + } + + Status Execute(engine::Context &ctx, Server *srv, Connection *conn, std::string *output) override { + redis::CuckooChain cuckoo_db(srv->storage, conn->GetNamespace()); + bool deleted = false; + auto s = cuckoo_db.Delete(ctx, args_[1], args_[2], &deleted); + + if (!s.ok()) { + return {Status::RedisExecErr, s.ToString()}; + } + + *output = redis::Integer(deleted ? 1 : 0); + return Status::OK(); + } +}; + +// Register the CF.RESERVE, CF.ADD and CF.DEL commands REDIS_REGISTER_COMMANDS(CuckooFilter, MakeCmdAttr("cf.reserve", -3, "write", 1, 1, 1), - MakeCmdAttr("cf.add", 3, "write", 1, 1, 1)) + MakeCmdAttr("cf.add", 3, "write", 1, 1, 1), + MakeCmdAttr("cf.del", 3, "write", 1, 1, 1)) } // namespace redis diff --git a/src/types/cuckoo_filter.h b/src/types/cuckoo_filter.h index a885a4c0890..a3357088ff7 100644 --- a/src/types/cuckoo_filter.h +++ b/src/types/cuckoo_filter.h @@ -36,6 +36,7 @@ constexpr long double kCuckooFilterLoadFactor = 0.955L; constexpr uint64_t kCuckooFilterMaxSupportedBuckets = std::numeric_limits::max() / 2 + 1ULL; constexpr uint64_t kCuckooFilterFingerprintModulus = 255; constexpr uint64_t kCuckooFilterAltHashMultiplier = 0x5bd1e995ULL; +constexpr uint8_t kEmptyCuckooFingerprint = 0; // Cuckoo filter implementation from the paper: // "Cuckoo Filter: Practically Better Than Bloom" by Fan et al. diff --git a/src/types/cuckoo_filter_page.cc b/src/types/cuckoo_filter_page.cc index 1ef4a4c803d..aa03b4c24b3 100644 --- a/src/types/cuckoo_filter_page.cc +++ b/src/types/cuckoo_filter_page.cc @@ -119,6 +119,13 @@ rocksdb::Status CuckooPageCache::SetBucketSlot(uint16_t filter_index, uint32_t n rocksdb::Status CuckooPageCache::WriteBackDirtyPages(rocksdb::WriteBatchBase *batch) { for (const auto &entry : pages_) { if (!entry.second.is_dirty) continue; + if (std::all_of(entry.second.data.begin(), entry.second.data.end(), [](char value) { return value == 0; })) { + if (entry.second.exists) { + auto s = batch->Delete(entry.first); + if (!s.ok()) return s; + } + continue; + } auto s = batch->Put(entry.first, entry.second.data); if (!s.ok()) return s; } @@ -167,6 +174,7 @@ rocksdb::Status CuckooPageCache::loadPage(const BucketLocation &location, PageEn PageEntry page_entry; auto s = storage_->Get(ctx_, ctx_.GetReadOptions(), location.page_key, &page_entry.data); if (!s.ok() && !s.IsNotFound()) return s; + page_entry.exists = s.ok(); s = normalizePage(s, location.expected_page_size, &page_entry); if (!s.ok()) return s; @@ -193,7 +201,10 @@ rocksdb::Status CuckooPageCache::loadPages(const std::vector &lo for (size_t i = 0; i < locations.size(); ++i) { PageEntry page_entry; - if (statuses[i].ok()) page_entry.data.assign(values[i].data(), values[i].size()); + if (statuses[i].ok()) { + page_entry.data.assign(values[i].data(), values[i].size()); + page_entry.exists = true; + } auto s = normalizePage(statuses[i], locations[i].expected_page_size, &page_entry); if (!s.ok()) return s; pages_.emplace(locations[i].page_key, std::move(page_entry)); diff --git a/src/types/cuckoo_filter_page.h b/src/types/cuckoo_filter_page.h index ff2efeb50c7..07f4c36f3a8 100644 --- a/src/types/cuckoo_filter_page.h +++ b/src/types/cuckoo_filter_page.h @@ -52,6 +52,7 @@ class CuckooPageCache { private: struct PageEntry { std::string data; + bool exists = false; bool is_dirty = false; }; diff --git a/src/types/cuckoo_filter_sub_filter.cc b/src/types/cuckoo_filter_sub_filter.cc index 0a0748cc354..e6538c7a629 100644 --- a/src/types/cuckoo_filter_sub_filter.cc +++ b/src/types/cuckoo_filter_sub_filter.cc @@ -45,6 +45,38 @@ rocksdb::Status CuckooSubFilter::TryInsert(uint64_t hash, uint8_t fingerprint, b return pages_.TryInsertInBucket(filter_index_, num_buckets_, bucket2_idx, fingerprint, inserted); } +rocksdb::Status CuckooSubFilter::Delete(uint64_t hash, uint8_t fingerprint, bool *deleted) { + *deleted = false; + uint32_t bucket1_idx = getPrimaryBucketIndex(hash); + uint32_t bucket2_idx = getSecondaryBucketIndex(hash, fingerprint); + auto s = pages_.PrefetchBuckets(filter_index_, num_buckets_, bucket1_idx, bucket2_idx); + if (!s.ok()) return s; + + for (uint32_t slot_idx = 0; slot_idx < bucket_size_; ++slot_idx) { + uint8_t current_fingerprint = kEmptyCuckooFingerprint; + s = pages_.GetBucketSlot(filter_index_, num_buckets_, bucket1_idx, slot_idx, ¤t_fingerprint); + if (!s.ok()) return s; + if (current_fingerprint != fingerprint) continue; + + *deleted = true; + return pages_.SetBucketSlot(filter_index_, num_buckets_, bucket1_idx, slot_idx, kEmptyCuckooFingerprint); + } + + if (bucket1_idx == bucket2_idx) return rocksdb::Status::OK(); + + for (uint32_t slot_idx = 0; slot_idx < bucket_size_; ++slot_idx) { + uint8_t current_fingerprint = kEmptyCuckooFingerprint; + s = pages_.GetBucketSlot(filter_index_, num_buckets_, bucket2_idx, slot_idx, ¤t_fingerprint); + if (!s.ok()) return s; + if (current_fingerprint != fingerprint) continue; + + *deleted = true; + return pages_.SetBucketSlot(filter_index_, num_buckets_, bucket2_idx, slot_idx, kEmptyCuckooFingerprint); + } + + return rocksdb::Status::OK(); +} + rocksdb::Status CuckooSubFilter::TryKickOutInsert(uint64_t hash, uint8_t fingerprint, uint16_t max_iterations, bool *inserted) { *inserted = false; diff --git a/src/types/cuckoo_filter_sub_filter.h b/src/types/cuckoo_filter_sub_filter.h index 2bd26df8541..dd34833790a 100644 --- a/src/types/cuckoo_filter_sub_filter.h +++ b/src/types/cuckoo_filter_sub_filter.h @@ -35,10 +35,8 @@ class CuckooSubFilter { uint64_t version, uint8_t bucket_size, uint32_t page_size, uint16_t filter_index, uint32_t num_buckets); - uint16_t Index() const { return filter_index_; } - uint32_t NumBuckets() const { return num_buckets_; } - rocksdb::Status TryInsert(uint64_t hash, uint8_t fingerprint, bool *inserted); + rocksdb::Status Delete(uint64_t hash, uint8_t fingerprint, bool *deleted); // Performs speculative kick-out mutations in the page cache. On success, dirty pages remain staged for // WriteToBatch(); on inserted=false or non-OK status, cached pages are discarded before returning. rocksdb::Status TryKickOutInsert(uint64_t hash, uint8_t fingerprint, uint16_t max_iterations, bool *inserted); diff --git a/src/types/redis_cuckoo_chain.cc b/src/types/redis_cuckoo_chain.cc index f7074ce9458..a3335d65584 100644 --- a/src/types/redis_cuckoo_chain.cc +++ b/src/types/redis_cuckoo_chain.cc @@ -179,6 +179,45 @@ rocksdb::Status CuckooChain::Add(engine::Context &ctx, const Slice &user_key, co return rocksdb::Status::Aborted("filter is full"); } +rocksdb::Status CuckooChain::Delete(engine::Context &ctx, const Slice &user_key, const Slice &item, bool *deleted) { + *deleted = false; + std::string ns_key = AppendNamespacePrefix(user_key); + + CuckooChainMetadata metadata(false); + auto s = getCuckooChainMetadata(ctx, ns_key, &metadata); + if (s.IsNotFound()) return rocksdb::Status::NotFound("Not found"); + if (!s.ok()) return s; + + s = validateMetadata(metadata); + if (!s.ok()) return s; + + uint64_t hash = CuckooFilterHelper::Hash(item.data(), item.size()); + uint8_t fingerprint = CuckooFilterHelper::GenerateFingerprint(hash); + + for (int filter_idx = static_cast(metadata.n_filters) - 1; filter_idx >= 0; --filter_idx) { + uint32_t num_buckets = 0; + s = CuckooFilterHelper::GetFilterNumBuckets(metadata.base_capacity, metadata.expansion, metadata.bucket_size, + static_cast(filter_idx), &num_buckets); + if (!s.ok()) return s; + + CuckooSubFilter sub_filter(storage_, ctx, ns_key, storage_->IsSlotIdEncoded(), metadata.version, + metadata.bucket_size, metadata.page_size, static_cast(filter_idx), + num_buckets); + bool found = false; + s = sub_filter.Delete(hash, fingerprint, &found); + if (!s.ok()) return s; + if (!found) continue; + + if (metadata.size == 0) return rocksdb::Status::Corruption("invalid metadata: size is 0"); + metadata.size--; + metadata.num_deleted_items++; + *deleted = true; + return commitDelete(ctx, user_key, ns_key, &metadata, &sub_filter); + } + + return rocksdb::Status::OK(); +} + rocksdb::Status CuckooChain::tryCuckooInsert(engine::Context &ctx, const Slice &user_key, const std::string &ns_key, CuckooChainMetadata *metadata, uint64_t hash, uint8_t fingerprint, bool *inserted) { @@ -291,4 +330,22 @@ rocksdb::Status CuckooChain::commitSubFilterAndMetadata(engine::Context &ctx, co return storage_->Write(ctx, storage_->DefaultWriteOptions(), batch->GetWriteBatch()); } +rocksdb::Status CuckooChain::commitDelete(engine::Context &ctx, const Slice &user_key, const std::string &ns_key, + CuckooChainMetadata *metadata, CuckooSubFilter *sub_filter) { + auto batch = storage_->GetWriteBatchBase(); + WriteBatchLogData log_data(kRedisCuckooFilter, std::vector{"del", user_key.ToString()}); + auto s = batch->PutLogData(log_data.Encode()); + if (!s.ok()) return s; + + s = sub_filter->WriteToBatch(batch.Get()); + if (!s.ok()) return s; + + std::string metadata_bytes; + metadata->Encode(&metadata_bytes); + s = batch->Put(metadata_cf_handle_, ns_key, metadata_bytes); + if (!s.ok()) return s; + + return storage_->Write(ctx, storage_->DefaultWriteOptions(), batch->GetWriteBatch()); +} + } // namespace redis diff --git a/src/types/redis_cuckoo_chain.h b/src/types/redis_cuckoo_chain.h index ae20b056cf3..05544b8a57f 100644 --- a/src/types/redis_cuckoo_chain.h +++ b/src/types/redis_cuckoo_chain.h @@ -47,6 +47,9 @@ class CuckooChain : public Database { // Duplicate items are allowed, so added is true whenever insertion succeeds. rocksdb::Status Add(engine::Context &ctx, const Slice &user_key, const Slice &item, bool *added); + // Deletes one matching fingerprint from the cuckoo filter. + rocksdb::Status Delete(engine::Context &ctx, const Slice &user_key, const Slice &item, bool *deleted); + private: // Loads metadata for a cuckoo filter key. rocksdb::Status getCuckooChainMetadata(engine::Context &ctx, const Slice &ns_key, CuckooChainMetadata *metadata); @@ -62,6 +65,8 @@ class CuckooChain : public Database { bool *inserted); rocksdb::Status commitSubFilterAndMetadata(engine::Context &ctx, const Slice &user_key, const std::string &ns_key, CuckooChainMetadata *metadata, CuckooSubFilter *sub_filter); + rocksdb::Status commitDelete(engine::Context &ctx, const Slice &user_key, const std::string &ns_key, + CuckooChainMetadata *metadata, CuckooSubFilter *sub_filter); }; } // namespace redis diff --git a/tests/cppunit/types/cuckoo_filter_page_test.cc b/tests/cppunit/types/cuckoo_filter_page_test.cc index 8be87aefada..eb89c3afab7 100644 --- a/tests/cppunit/types/cuckoo_filter_page_test.cc +++ b/tests/cppunit/types/cuckoo_filter_page_test.cc @@ -238,6 +238,43 @@ TEST_F(RedisCuckooPageCacheTest, SetBucketSlotWritesOnlyTargetSlot) { EXPECT_EQ(page, expected); } +TEST_F(RedisCuckooPageCacheTest, SkipsWritingNewPageWhenItBecomesEmpty) { + auto metadata = makeMetadata(1); + redis::CuckooPageCache pages(storage_.get(), *ctx_, ns_key_, storage_->IsSlotIdEncoded(), metadata.version, + metadata.bucket_size, metadata.page_size); + + auto s = pages.SetBucketSlot(0, 1, 0, 0, 88); + ASSERT_TRUE(s.ok()) << s.ToString(); + s = pages.SetBucketSlot(0, 1, 0, 0, 0); + ASSERT_TRUE(s.ok()) << s.ToString(); + + auto batch = storage_->GetWriteBatchBase(); + s = pages.WriteBackDirtyPages(batch.Get()); + ASSERT_TRUE(s.ok()) << s.ToString(); + EXPECT_EQ(batch->GetWriteBatch()->Count(), 0); +} + +TEST_F(RedisCuckooPageCacheTest, DeletesExistingPageWhenItBecomesEmpty) { + auto metadata = makeMetadata(1); + auto page_key = makePageKey(metadata, 0, 0); + writePage(page_key, std::string{static_cast(88)}); + redis::CuckooPageCache pages(storage_.get(), *ctx_, ns_key_, storage_->IsSlotIdEncoded(), metadata.version, + metadata.bucket_size, metadata.page_size); + + auto s = pages.SetBucketSlot(0, 1, 0, 0, 0); + ASSERT_TRUE(s.ok()) << s.ToString(); + + auto batch = storage_->GetWriteBatchBase(); + s = pages.WriteBackDirtyPages(batch.Get()); + ASSERT_TRUE(s.ok()) << s.ToString(); + EXPECT_EQ(batch->GetWriteBatch()->Count(), 1); + commitBatch(batch.Get()); + + std::string page; + s = readPage(page_key, &page); + EXPECT_TRUE(s.IsNotFound()) << s.ToString(); +} + TEST_F(RedisCuckooPageCacheTest, InvalidBucketAndSlotArguments) { auto metadata = makeMetadata(4); redis::CuckooPageCache pages(storage_.get(), *ctx_, ns_key_, storage_->IsSlotIdEncoded(), metadata.version, diff --git a/tests/cppunit/types/cuckoo_filter_test.cc b/tests/cppunit/types/cuckoo_filter_test.cc index 66a24ad425d..bff84de4a12 100644 --- a/tests/cppunit/types/cuckoo_filter_test.cc +++ b/tests/cppunit/types/cuckoo_filter_test.cc @@ -107,6 +107,46 @@ class RedisCuckooFilterTest : public TestBase { .Encode(); } + uint32_t bucketsPerPage(const CuckooChainMetadata &metadata) { + return std::max(1, metadata.page_size / metadata.bucket_size); + } + + uint32_t pageIndexForBucket(const CuckooChainMetadata &metadata, uint32_t bucket_index) { + return bucket_index / bucketsPerPage(metadata); + } + + uint32_t bucketOffsetInPage(const CuckooChainMetadata &metadata, uint32_t bucket_index) { + return (bucket_index % bucketsPerPage(metadata)) * metadata.bucket_size; + } + + void placeFingerprint(const std::string &key, const CuckooChainMetadata &metadata, uint16_t filter_index, + uint32_t num_buckets, uint32_t bucket_index, uint32_t slot_index, uint8_t fingerprint) { + ASSERT_LT(bucket_index, num_buckets); + ASSERT_LT(slot_index, metadata.bucket_size); + auto page_index = pageIndexForBucket(metadata, bucket_index); + auto first_bucket = page_index * bucketsPerPage(metadata); + auto page_bucket_count = std::min(bucketsPerPage(metadata), num_buckets - first_bucket); + std::string page; + auto page_key = makePageKey(key, metadata, filter_index, page_index); + auto s = readPage(page_key, &page); + if (s.IsNotFound()) { + page.assign(page_bucket_count * metadata.bucket_size, 0); + } else { + ASSERT_TRUE(s.ok()) << s.ToString(); + ASSERT_EQ(page.size(), page_bucket_count * metadata.bucket_size); + } + page[bucketOffsetInPage(metadata, bucket_index) + slot_index] = static_cast(fingerprint); + writePage(page_key, page); + } + + uint8_t readFingerprint(const std::string &key, const CuckooChainMetadata &metadata, uint16_t filter_index, + uint32_t bucket_index, uint32_t slot_index) { + std::string page; + auto s = readPage(makePageKey(key, metadata, filter_index, pageIndexForBucket(metadata, bucket_index)), &page); + EXPECT_TRUE(s.ok()) << s.ToString(); + return static_cast(page[bucketOffsetInPage(metadata, bucket_index) + slot_index]); + } + rocksdb::Status readPage(const std::string &page_key, std::string *value) { return storage_->Get(*ctx_, ctx_->GetReadOptions(), storage_->GetCFHandle(ColumnFamilyID::PrimarySubkey), page_key, value); @@ -743,3 +783,164 @@ TEST_F(RedisCuckooFilterTest, ExpansionWritesNewFilterIndexPage) { ASSERT_TRUE(s.ok()) << s.ToString(); EXPECT_EQ(page.size(), expected_page_size); } + +TEST_F(RedisCuckooFilterTest, DeleteMissingKeyReturnsNotFound) { + bool deleted = true; + auto s = cuckoo_->Delete(*ctx_, key_, "missing", &deleted); + EXPECT_TRUE(s.IsNotFound()) << s.ToString(); + EXPECT_NE(s.ToString().find("Not found"), std::string::npos); +} + +TEST_F(RedisCuckooFilterTest, DeleteBasicClearsOneItem) { + reserveAndVerify(key_, 1000, 4, 500, 2); + addAndVerify(key_, "item", 1000, 4, 500, 2, 1); + + bool deleted = false; + auto s = cuckoo_->Delete(*ctx_, key_, "item", &deleted); + ASSERT_TRUE(s.ok()) << s.ToString(); + EXPECT_TRUE(deleted); + verifyMetadata(key_, 1000, 4, 500, 2, 0, 1, 1); + + deleted = true; + s = cuckoo_->Delete(*ctx_, key_, "item", &deleted); + ASSERT_TRUE(s.ok()) << s.ToString(); + EXPECT_FALSE(deleted); + verifyMetadata(key_, 1000, 4, 500, 2, 0, 1, 1); +} + +TEST_F(RedisCuckooFilterTest, DeleteDuplicateItemsOneAtATime) { + reserveAndVerify(key_, 1000, 4, 500, 2); + for (int i = 0; i < 3; ++i) { + addAndVerify(key_, "duplicate", 1000, 4, 500, 2, i + 1); + } + + for (int i = 0; i < 3; ++i) { + bool deleted = false; + auto s = cuckoo_->Delete(*ctx_, key_, "duplicate", &deleted); + ASSERT_TRUE(s.ok()) << s.ToString(); + EXPECT_TRUE(deleted); + verifyMetadata(key_, 1000, 4, 500, 2, 2 - i, 1, i + 1); + } + + bool deleted = true; + auto s = cuckoo_->Delete(*ctx_, key_, "duplicate", &deleted); + ASSERT_TRUE(s.ok()) << s.ToString(); + EXPECT_FALSE(deleted); + verifyMetadata(key_, 1000, 4, 500, 2, 0, 1, 3); +} + +TEST_F(RedisCuckooFilterTest, DeleteMissingItemDoesNotMutateMetadata) { + reserveAndVerify(key_, 1000, 4, 500, 2); + addAndVerify(key_, "known", 1000, 4, 500, 2, 1); + + auto metadata = getMetadata(key_); + uint32_t num_buckets = 0; + auto s = redis::CuckooFilterHelper::GetFilterNumBuckets(metadata.base_capacity, metadata.expansion, + metadata.bucket_size, 0, &num_buckets); + ASSERT_TRUE(s.ok()) << s.ToString(); + auto known_hash = redis::CuckooFilterHelper::Hash("known"); + auto known_fp = redis::CuckooFilterHelper::GenerateFingerprint(known_hash); + auto known_bucket1 = static_cast(known_hash % num_buckets); + auto known_bucket2 = static_cast(redis::CuckooFilterHelper::GetAltHash(known_fp, known_hash) % num_buckets); + + std::string missing_item; + for (int i = 0; i < 10000; ++i) { + auto candidate = "missing_" + std::to_string(i); + auto hash = redis::CuckooFilterHelper::Hash(candidate); + auto fp = redis::CuckooFilterHelper::GenerateFingerprint(hash); + auto bucket1 = static_cast(hash % num_buckets); + auto bucket2 = static_cast(redis::CuckooFilterHelper::GetAltHash(fp, hash) % num_buckets); + if (fp != known_fp || (bucket1 != known_bucket1 && bucket1 != known_bucket2 && bucket2 != known_bucket1 && + bucket2 != known_bucket2)) { + missing_item = candidate; + break; + } + } + ASSERT_FALSE(missing_item.empty()); + + bool deleted = true; + s = cuckoo_->Delete(*ctx_, key_, missing_item, &deleted); + ASSERT_TRUE(s.ok()) << s.ToString(); + EXPECT_FALSE(deleted); + verifyMetadata(key_, 1000, 4, 500, 2, 1, 1, 0); +} + +TEST_F(RedisCuckooFilterTest, DeleteSearchesNewestFilterFirst) { + CuckooChainMetadata metadata; + metadata.size = 100; + metadata.base_capacity = 2; + metadata.bucket_size = 1; + metadata.max_iterations = 1; + metadata.expansion = 1; + metadata.n_filters = 2; + metadata.num_deleted_items = 0; + metadata.page_size = 1; + writeMetadata(key_, metadata); + + const std::string item = "item"; + auto hash = redis::CuckooFilterHelper::Hash(item); + auto fingerprint = redis::CuckooFilterHelper::GenerateFingerprint(hash); + uint32_t num_buckets = 0; + auto s = redis::CuckooFilterHelper::GetFilterNumBuckets(metadata.base_capacity, metadata.expansion, + metadata.bucket_size, 0, &num_buckets); + ASSERT_TRUE(s.ok()) << s.ToString(); + auto bucket = static_cast(hash % num_buckets); + placeFingerprint(key_, metadata, 0, num_buckets, bucket, 0, fingerprint); + placeFingerprint(key_, metadata, 1, num_buckets, bucket, 0, fingerprint); + + bool deleted = false; + s = cuckoo_->Delete(*ctx_, key_, item, &deleted); + ASSERT_TRUE(s.ok()) << s.ToString(); + EXPECT_TRUE(deleted); + + auto stored_metadata = getMetadata(key_); + EXPECT_EQ(stored_metadata.n_filters, 2); + EXPECT_EQ(readFingerprint(key_, stored_metadata, 0, bucket, 0), fingerprint); + std::string page; + s = readPage(makePageKey(key_, stored_metadata, 1, pageIndexForBucket(stored_metadata, bucket)), &page); + EXPECT_TRUE(s.IsNotFound()) << s.ToString(); +} + +TEST_F(RedisCuckooFilterTest, DeleteDoesNotCompactFilters) { + CuckooChainMetadata metadata; + metadata.size = 8; + metadata.base_capacity = 2; + metadata.bucket_size = 2; + metadata.max_iterations = 1; + metadata.expansion = 1; + metadata.n_filters = 2; + metadata.num_deleted_items = 1; + metadata.page_size = 4; + writeMetadata(key_, metadata); + + uint32_t num_buckets = 0; + auto s = redis::CuckooFilterHelper::GetFilterNumBuckets(metadata.base_capacity, metadata.expansion, + metadata.bucket_size, 0, &num_buckets); + ASSERT_TRUE(s.ok()) << s.ToString(); + ASSERT_EQ(num_buckets, 2); + + const std::string item = "delete_me"; + auto hash = redis::CuckooFilterHelper::Hash(item); + auto fingerprint = redis::CuckooFilterHelper::GenerateFingerprint(hash); + auto delete_bucket = static_cast(hash % num_buckets); + auto movable_bucket = static_cast((delete_bucket + 1) % num_buckets); + constexpr uint8_t movable_fingerprint = 77; + placeFingerprint(key_, metadata, 1, num_buckets, delete_bucket, 0, fingerprint); + placeFingerprint(key_, metadata, 1, num_buckets, movable_bucket, 0, movable_fingerprint); + + bool deleted = false; + s = cuckoo_->Delete(*ctx_, key_, item, &deleted); + ASSERT_TRUE(s.ok()) << s.ToString(); + EXPECT_TRUE(deleted); + + auto stored_metadata = getMetadata(key_); + EXPECT_EQ(stored_metadata.size, 7); + EXPECT_EQ(stored_metadata.n_filters, 2); + EXPECT_EQ(stored_metadata.num_deleted_items, 2); + EXPECT_EQ(readFingerprint(key_, stored_metadata, 1, movable_bucket, 0), movable_fingerprint); + EXPECT_EQ(readFingerprint(key_, stored_metadata, 1, delete_bucket, 0), 0); + + std::string page; + s = readPage(makePageKey(key_, metadata, 1, 0), &page); + EXPECT_TRUE(s.ok()) << s.ToString(); +} diff --git a/tests/gocase/unit/type/bloom/cuckoo_filter_test.go b/tests/gocase/unit/type/bloom/cuckoo_filter_test.go index 852dde75ed0..8e0ba24afb0 100644 --- a/tests/gocase/unit/type/bloom/cuckoo_filter_test.go +++ b/tests/gocase/unit/type/bloom/cuckoo_filter_test.go @@ -107,6 +107,42 @@ func TestCuckooFilter(t *testing.T) { require.Error(t, rdb.Do(ctx, "cf.add", "key", "item1", "item2").Err()) }) + t.Run("Del wrong number of arguments", func(t *testing.T) { + require.Error(t, rdb.Do(ctx, "cf.del").Err()) + require.Error(t, rdb.Do(ctx, "cf.del", "key_only").Err()) + require.Error(t, rdb.Do(ctx, "cf.del", "key", "item1", "item2").Err()) + }) + + t.Run("Del missing key", func(t *testing.T) { + key := "test_cuckoo_filter_del_missing" + require.NoError(t, rdb.Del(ctx, key).Err()) + require.ErrorContains(t, rdb.Do(ctx, "cf.del", key, "item").Err(), "Not found") + }) + + t.Run("Del wrong type", func(t *testing.T) { + key := "test_cuckoo_filter_del_wrong_type" + require.NoError(t, rdb.Set(ctx, key, "value", 0).Err()) + require.ErrorContains(t, rdb.Do(ctx, "cf.del", key, "item").Err(), "WRONGTYPE") + }) + + t.Run("Del basic", func(t *testing.T) { + key := "test_cuckoo_filter_del_basic" + require.NoError(t, rdb.Del(ctx, key).Err()) + require.Equal(t, int64(1), rdb.Do(ctx, "cf.add", key, "item").Val()) + require.Equal(t, int64(1), rdb.Do(ctx, "cf.del", key, "item").Val()) + require.Equal(t, int64(0), rdb.Do(ctx, "cf.del", key, "item").Val()) + }) + + t.Run("Del duplicate item", func(t *testing.T) { + key := "test_cuckoo_filter_del_dup" + require.NoError(t, rdb.Del(ctx, key).Err()) + require.Equal(t, int64(1), rdb.Do(ctx, "cf.add", key, "same_item").Val()) + require.Equal(t, int64(1), rdb.Do(ctx, "cf.add", key, "same_item").Val()) + require.Equal(t, int64(1), rdb.Do(ctx, "cf.del", key, "same_item").Val()) + require.Equal(t, int64(1), rdb.Do(ctx, "cf.del", key, "same_item").Val()) + require.Equal(t, int64(0), rdb.Do(ctx, "cf.del", key, "same_item").Val()) + }) + t.Run("Add many items", func(t *testing.T) { key := "test_cuckoo_filter_add_many" require.NoError(t, rdb.Del(ctx, key).Err())