From 09ba348a181f880d4b4153fdbca2006e2d7344cf Mon Sep 17 00:00:00 2001 From: nagisa-kun <1434936049@qq.com> Date: Sun, 12 Jul 2026 22:59:50 +0800 Subject: [PATCH 1/4] feat: del --- src/commands/cmd_cuckoo_filter.cc | 29 +- src/types/cuckoo_filter_page.cc | 10 +- src/types/cuckoo_filter_page.h | 4 + src/types/cuckoo_filter_sub_filter.cc | 40 +++ src/types/cuckoo_filter_sub_filter.h | 4 + src/types/redis_cuckoo_chain.cc | 154 ++++++++++ src/types/redis_cuckoo_chain.h | 18 ++ tests/cppunit/types/cuckoo_filter_test.cc | 264 ++++++++++++++++++ .../unit/type/bloom/cuckoo_filter_test.go | 36 +++ 9 files changed, 555 insertions(+), 4 deletions(-) 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_page.cc b/src/types/cuckoo_filter_page.cc index 1ef4a4c803d..75c0734e80f 100644 --- a/src/types/cuckoo_filter_page.cc +++ b/src/types/cuckoo_filter_page.cc @@ -46,6 +46,14 @@ uint32_t GetExpectedPageSize(uint32_t page_index, uint32_t num_buckets, uint32_t return page_bucket_count * bucket_size; } +} // namespace + +uint32_t GetCuckooPageCount(uint32_t num_buckets, uint32_t page_size, uint8_t bucket_size) { + if (num_buckets == 0) return 0; + uint32_t buckets_per_page = GetBucketsPerPage(page_size, bucket_size); + return GetPageIndex(num_buckets - 1, buckets_per_page) + 1; +} + std::string GetCuckooPageKey(const Slice &ns_key, uint64_t version, bool slot_id_encoded, uint16_t filter_index, uint32_t page_index) { std::string sub_key; @@ -54,8 +62,6 @@ std::string GetCuckooPageKey(const Slice &ns_key, uint64_t version, bool slot_id return InternalKey(ns_key, sub_key, version, slot_id_encoded).Encode(); } -} // namespace - CuckooPageCache::CuckooPageCache(engine::Storage *storage, engine::Context &ctx, const Slice &ns_key, bool slot_id_encoded, uint64_t version, uint8_t bucket_size, uint32_t page_size) : storage_(storage), diff --git a/src/types/cuckoo_filter_page.h b/src/types/cuckoo_filter_page.h index ff2efeb50c7..ade49848397 100644 --- a/src/types/cuckoo_filter_page.h +++ b/src/types/cuckoo_filter_page.h @@ -32,6 +32,10 @@ namespace redis { +uint32_t GetCuckooPageCount(uint32_t num_buckets, uint32_t page_size, uint8_t bucket_size); +std::string GetCuckooPageKey(const Slice &ns_key, uint64_t version, bool slot_id_encoded, uint16_t filter_index, + uint32_t page_index); + class CuckooPageCache { public: CuckooPageCache(engine::Storage *storage, engine::Context &ctx, const Slice &ns_key, bool slot_id_encoded, diff --git a/src/types/cuckoo_filter_sub_filter.cc b/src/types/cuckoo_filter_sub_filter.cc index 0a0748cc354..d0a68e86d02 100644 --- a/src/types/cuckoo_filter_sub_filter.cc +++ b/src/types/cuckoo_filter_sub_filter.cc @@ -45,6 +45,46 @@ 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 = 0; + 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, 0); + } + + 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 = 0; + 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, 0); + } + + return rocksdb::Status::OK(); +} + +rocksdb::Status CuckooSubFilter::GetBucketSlot(uint32_t bucket_index, uint32_t slot_index, uint8_t *fingerprint) { + return pages_.GetBucketSlot(filter_index_, num_buckets_, bucket_index, slot_index, fingerprint); +} + +rocksdb::Status CuckooSubFilter::SetBucketSlot(uint32_t bucket_index, uint32_t slot_index, uint8_t fingerprint) { + return pages_.SetBucketSlot(filter_index_, num_buckets_, bucket_index, slot_index, fingerprint); +} + 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..427865ef9d4 100644 --- a/src/types/cuckoo_filter_sub_filter.h +++ b/src/types/cuckoo_filter_sub_filter.h @@ -36,9 +36,13 @@ class CuckooSubFilter { uint32_t num_buckets); uint16_t Index() const { return filter_index_; } + uint8_t BucketSize() const { return bucket_size_; } 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); + rocksdb::Status GetBucketSlot(uint32_t bucket_index, uint32_t slot_index, uint8_t *fingerprint); + rocksdb::Status SetBucketSlot(uint32_t bucket_index, uint32_t slot_index, uint8_t fingerprint); // 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..cc4a32629fd 100644 --- a/src/types/redis_cuckoo_chain.cc +++ b/src/types/redis_cuckoo_chain.cc @@ -21,6 +21,7 @@ #include "redis_cuckoo_chain.h" #include "cuckoo_filter.h" +#include "cuckoo_filter_page.h" #include "cuckoo_filter_sub_filter.h" #include "logging.h" @@ -179,6 +180,50 @@ 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::OK(); + 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); + + CuckooSubFilters sub_filters; + s = buildSubFilters(ctx, ns_key, metadata, &sub_filters); + if (!s.ok()) return s; + + for (int filter_idx = static_cast(metadata.n_filters) - 1; filter_idx >= 0; --filter_idx) { + bool found = false; + s = sub_filters[filter_idx]->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; + break; + } + + if (!*deleted) return rocksdb::Status::OK(); + + std::vector freed_filter_indexes; + if (metadata.n_filters > 1 && + static_cast(metadata.num_deleted_items) > static_cast(metadata.size) * 0.10L) { + s = compactCuckooChain(&metadata, &sub_filters, &freed_filter_indexes, false); + if (!s.ok()) return s; + } + + return commitDelete(ctx, user_key, ns_key, &metadata, &sub_filters, freed_filter_indexes); +} + 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 +336,113 @@ rocksdb::Status CuckooChain::commitSubFilterAndMetadata(engine::Context &ctx, co return storage_->Write(ctx, storage_->DefaultWriteOptions(), batch->GetWriteBatch()); } +rocksdb::Status CuckooChain::buildSubFilters(engine::Context &ctx, const std::string &ns_key, + const CuckooChainMetadata &metadata, CuckooSubFilters *sub_filters) { + sub_filters->clear(); + sub_filters->reserve(metadata.n_filters); + for (uint16_t filter_idx = 0; filter_idx < metadata.n_filters; ++filter_idx) { + uint32_t num_buckets = 0; + auto s = CuckooFilterHelper::GetFilterNumBuckets(metadata.base_capacity, metadata.expansion, metadata.bucket_size, + filter_idx, &num_buckets); + if (!s.ok()) return s; + + sub_filters->push_back(std::make_unique(storage_, ctx, ns_key, storage_->IsSlotIdEncoded(), + metadata.version, metadata.bucket_size, metadata.page_size, + filter_idx, num_buckets)); + } + return rocksdb::Status::OK(); +} + +rocksdb::Status CuckooChain::compactCuckooChain(CuckooChainMetadata *metadata, CuckooSubFilters *sub_filters, + std::vector *freed_filter_indexes, bool cont) { + for (int source_idx = static_cast(metadata->n_filters) - 1; source_idx >= 1; --source_idx) { + bool fully_compacted = false; + auto s = compactSingleSubFilter(static_cast(source_idx), sub_filters, &fully_compacted); + if (!s.ok()) return s; + + if (fully_compacted && source_idx == static_cast(metadata->n_filters) - 1) { + freed_filter_indexes->push_back(static_cast(source_idx)); + metadata->n_filters--; + } + + if (!fully_compacted && !cont) break; + } + + metadata->num_deleted_items = 0; + return rocksdb::Status::OK(); +} + +rocksdb::Status CuckooChain::compactSingleSubFilter(uint16_t source_index, CuckooSubFilters *sub_filters, + bool *fully_compacted) { + *fully_compacted = true; + auto *source_filter = (*sub_filters)[source_index].get(); + for (uint32_t bucket_idx = 0; bucket_idx < source_filter->NumBuckets(); ++bucket_idx) { + for (uint32_t slot_idx = 0; slot_idx < source_filter->BucketSize(); ++slot_idx) { + uint8_t fingerprint = 0; + auto s = source_filter->GetBucketSlot(bucket_idx, slot_idx, &fingerprint); + if (!s.ok()) return s; + if (fingerprint == 0) continue; + + bool relocated = false; + for (uint16_t target_idx = 0; target_idx < source_index; ++target_idx) { + bool inserted = false; + s = (*sub_filters)[target_idx]->TryInsert(bucket_idx, fingerprint, &inserted); + if (!s.ok()) return s; + if (!inserted) continue; + + s = source_filter->SetBucketSlot(bucket_idx, slot_idx, 0); + if (!s.ok()) return s; + relocated = true; + break; + } + + if (!relocated) *fully_compacted = false; + } + } + return rocksdb::Status::OK(); +} + +rocksdb::Status CuckooChain::deleteSubFilterPages(rocksdb::WriteBatchBase *batch, const std::string &ns_key, + const CuckooChainMetadata &metadata, uint16_t filter_index) { + uint32_t num_buckets = 0; + auto s = CuckooFilterHelper::GetFilterNumBuckets(metadata.base_capacity, metadata.expansion, metadata.bucket_size, + filter_index, &num_buckets); + if (!s.ok()) return s; + + uint32_t page_count = GetCuckooPageCount(num_buckets, metadata.page_size, metadata.bucket_size); + for (uint32_t page_index = 0; page_index < page_count; ++page_index) { + std::string page_key = + GetCuckooPageKey(ns_key, metadata.version, storage_->IsSlotIdEncoded(), filter_index, page_index); + s = batch->Delete(page_key); + if (!s.ok()) return s; + } + return rocksdb::Status::OK(); +} + +rocksdb::Status CuckooChain::commitDelete(engine::Context &ctx, const Slice &user_key, const std::string &ns_key, + CuckooChainMetadata *metadata, CuckooSubFilters *sub_filters, + const std::vector &freed_filter_indexes) { + 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; + + for (auto &sub_filter : *sub_filters) { + s = sub_filter->WriteToBatch(batch.Get()); + if (!s.ok()) return s; + } + + for (uint16_t filter_index : freed_filter_indexes) { + s = deleteSubFilterPages(batch.Get(), ns_key, *metadata, filter_index); + 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..4e69c963c03 100644 --- a/src/types/redis_cuckoo_chain.h +++ b/src/types/redis_cuckoo_chain.h @@ -20,6 +20,9 @@ #pragma once +#include +#include + #include "cuckoo_filter.h" #include "storage/redis_db.h" #include "storage/redis_metadata.h" @@ -47,7 +50,12 @@ 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: + using CuckooSubFilters = std::vector>; + // Loads metadata for a cuckoo filter key. rocksdb::Status getCuckooChainMetadata(engine::Context &ctx, const Slice &ns_key, CuckooChainMetadata *metadata); @@ -62,6 +70,16 @@ 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 buildSubFilters(engine::Context &ctx, const std::string &ns_key, const CuckooChainMetadata &metadata, + CuckooSubFilters *sub_filters); + rocksdb::Status compactCuckooChain(CuckooChainMetadata *metadata, CuckooSubFilters *sub_filters, + std::vector *freed_filter_indexes, bool cont); + rocksdb::Status compactSingleSubFilter(uint16_t source_index, CuckooSubFilters *sub_filters, bool *fully_compacted); + rocksdb::Status deleteSubFilterPages(rocksdb::WriteBatchBase *batch, const std::string &ns_key, + const CuckooChainMetadata &metadata, uint16_t filter_index); + rocksdb::Status commitDelete(engine::Context &ctx, const Slice &user_key, const std::string &ns_key, + CuckooChainMetadata *metadata, CuckooSubFilters *sub_filters, + const std::vector &freed_filter_indexes); }; } // namespace redis diff --git a/tests/cppunit/types/cuckoo_filter_test.cc b/tests/cppunit/types/cuckoo_filter_test.cc index 66a24ad425d..1a4153645a1 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,227 @@ TEST_F(RedisCuckooFilterTest, ExpansionWritesNewFilterIndexPage) { ASSERT_TRUE(s.ok()) << s.ToString(); EXPECT_EQ(page.size(), expected_page_size); } + +TEST_F(RedisCuckooFilterTest, DeleteMissingKeyReturnsFalse) { + bool deleted = true; + auto s = cuckoo_->Delete(*ctx_, key_, "missing", &deleted); + ASSERT_TRUE(s.ok()) << s.ToString(); + EXPECT_FALSE(deleted); +} + +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); + EXPECT_EQ(readFingerprint(key_, stored_metadata, 1, bucket, 0), 0); +} + +TEST_F(RedisCuckooFilterTest, DeleteTriggersCompactAndFreesLatestFilter) { + 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, 1); + EXPECT_EQ(stored_metadata.num_deleted_items, 0); + EXPECT_EQ(readFingerprint(key_, stored_metadata, 0, movable_bucket, 0), movable_fingerprint); + + std::string page; + s = readPage(makePageKey(key_, metadata, 1, 0), &page); + EXPECT_TRUE(s.IsNotFound()) << s.ToString(); +} + +TEST_F(RedisCuckooFilterTest, CompactStopsOnFailedRelocation) { + CuckooChainMetadata metadata; + metadata.size = 8; + metadata.base_capacity = 2; + metadata.bucket_size = 1; + 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, 4); + + constexpr uint32_t blocked_bucket = 0; + constexpr uint8_t blocked_fingerprint = 7; + auto blocked_alt = redis::CuckooFilterHelper::GetAltBucketIndex(blocked_bucket, blocked_fingerprint, num_buckets); + ASSERT_NE(blocked_alt, blocked_bucket); + placeFingerprint(key_, metadata, 0, num_buckets, blocked_bucket, 0, 101); + placeFingerprint(key_, metadata, 0, num_buckets, blocked_alt, 0, 102); + placeFingerprint(key_, metadata, 1, num_buckets, blocked_bucket, 0, blocked_fingerprint); + + uint32_t movable_bucket = num_buckets; + for (uint32_t bucket = 0; bucket < num_buckets; ++bucket) { + if (bucket != blocked_bucket && bucket != blocked_alt) { + movable_bucket = bucket; + break; + } + } + ASSERT_LT(movable_bucket, num_buckets); + constexpr uint8_t movable_fingerprint = 88; + placeFingerprint(key_, metadata, 1, num_buckets, movable_bucket, 0, movable_fingerprint); + + std::string item; + uint8_t delete_fingerprint = 0; + uint32_t delete_bucket = num_buckets; + for (int i = 0; i < 10000; ++i) { + auto candidate = "delete_" + std::to_string(i); + auto hash = redis::CuckooFilterHelper::Hash(candidate); + auto bucket = static_cast(hash % num_buckets); + if (bucket == blocked_bucket || bucket == movable_bucket) continue; + item = candidate; + delete_fingerprint = redis::CuckooFilterHelper::GenerateFingerprint(hash); + delete_bucket = bucket; + break; + } + ASSERT_FALSE(item.empty()); + placeFingerprint(key_, metadata, 1, num_buckets, delete_bucket, 0, delete_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, 0); + EXPECT_EQ(readFingerprint(key_, stored_metadata, 0, movable_bucket, 0), movable_fingerprint); + EXPECT_EQ(readFingerprint(key_, stored_metadata, 1, blocked_bucket, 0), blocked_fingerprint); +} diff --git a/tests/gocase/unit/type/bloom/cuckoo_filter_test.go b/tests/gocase/unit/type/bloom/cuckoo_filter_test.go index 852dde75ed0..09e7fe7e194 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.Equal(t, int64(0), rdb.Do(ctx, "cf.del", key, "item").Val()) + }) + + 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()) From 21512562e3708ed3a9b313c82bb2c7961957cfa1 Mon Sep 17 00:00:00 2001 From: nagisa-kun <1434936049@qq.com> Date: Sat, 8 Aug 2026 19:15:25 +0800 Subject: [PATCH 2/4] fix: remove compact --- src/types/cuckoo_filter_page.cc | 10 +- src/types/cuckoo_filter_page.h | 4 - src/types/cuckoo_filter_sub_filter.cc | 8 -- src/types/cuckoo_filter_sub_filter.h | 6 - src/types/redis_cuckoo_chain.cc | 127 +++--------------- src/types/redis_cuckoo_chain.h | 15 +-- tests/cppunit/types/cuckoo_filter_test.cc | 83 ++---------- .../unit/type/bloom/cuckoo_filter_test.go | 2 +- 8 files changed, 28 insertions(+), 227 deletions(-) diff --git a/src/types/cuckoo_filter_page.cc b/src/types/cuckoo_filter_page.cc index 75c0734e80f..1ef4a4c803d 100644 --- a/src/types/cuckoo_filter_page.cc +++ b/src/types/cuckoo_filter_page.cc @@ -46,14 +46,6 @@ uint32_t GetExpectedPageSize(uint32_t page_index, uint32_t num_buckets, uint32_t return page_bucket_count * bucket_size; } -} // namespace - -uint32_t GetCuckooPageCount(uint32_t num_buckets, uint32_t page_size, uint8_t bucket_size) { - if (num_buckets == 0) return 0; - uint32_t buckets_per_page = GetBucketsPerPage(page_size, bucket_size); - return GetPageIndex(num_buckets - 1, buckets_per_page) + 1; -} - std::string GetCuckooPageKey(const Slice &ns_key, uint64_t version, bool slot_id_encoded, uint16_t filter_index, uint32_t page_index) { std::string sub_key; @@ -62,6 +54,8 @@ std::string GetCuckooPageKey(const Slice &ns_key, uint64_t version, bool slot_id return InternalKey(ns_key, sub_key, version, slot_id_encoded).Encode(); } +} // namespace + CuckooPageCache::CuckooPageCache(engine::Storage *storage, engine::Context &ctx, const Slice &ns_key, bool slot_id_encoded, uint64_t version, uint8_t bucket_size, uint32_t page_size) : storage_(storage), diff --git a/src/types/cuckoo_filter_page.h b/src/types/cuckoo_filter_page.h index ade49848397..ff2efeb50c7 100644 --- a/src/types/cuckoo_filter_page.h +++ b/src/types/cuckoo_filter_page.h @@ -32,10 +32,6 @@ namespace redis { -uint32_t GetCuckooPageCount(uint32_t num_buckets, uint32_t page_size, uint8_t bucket_size); -std::string GetCuckooPageKey(const Slice &ns_key, uint64_t version, bool slot_id_encoded, uint16_t filter_index, - uint32_t page_index); - class CuckooPageCache { public: CuckooPageCache(engine::Storage *storage, engine::Context &ctx, const Slice &ns_key, bool slot_id_encoded, diff --git a/src/types/cuckoo_filter_sub_filter.cc b/src/types/cuckoo_filter_sub_filter.cc index d0a68e86d02..c1c49606d5b 100644 --- a/src/types/cuckoo_filter_sub_filter.cc +++ b/src/types/cuckoo_filter_sub_filter.cc @@ -77,14 +77,6 @@ rocksdb::Status CuckooSubFilter::Delete(uint64_t hash, uint8_t fingerprint, bool return rocksdb::Status::OK(); } -rocksdb::Status CuckooSubFilter::GetBucketSlot(uint32_t bucket_index, uint32_t slot_index, uint8_t *fingerprint) { - return pages_.GetBucketSlot(filter_index_, num_buckets_, bucket_index, slot_index, fingerprint); -} - -rocksdb::Status CuckooSubFilter::SetBucketSlot(uint32_t bucket_index, uint32_t slot_index, uint8_t fingerprint) { - return pages_.SetBucketSlot(filter_index_, num_buckets_, bucket_index, slot_index, fingerprint); -} - 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 427865ef9d4..dd34833790a 100644 --- a/src/types/cuckoo_filter_sub_filter.h +++ b/src/types/cuckoo_filter_sub_filter.h @@ -35,14 +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_; } - uint8_t BucketSize() const { return bucket_size_; } - 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); - rocksdb::Status GetBucketSlot(uint32_t bucket_index, uint32_t slot_index, uint8_t *fingerprint); - rocksdb::Status SetBucketSlot(uint32_t bucket_index, uint32_t slot_index, uint8_t fingerprint); // 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 cc4a32629fd..a3335d65584 100644 --- a/src/types/redis_cuckoo_chain.cc +++ b/src/types/redis_cuckoo_chain.cc @@ -21,7 +21,6 @@ #include "redis_cuckoo_chain.h" #include "cuckoo_filter.h" -#include "cuckoo_filter_page.h" #include "cuckoo_filter_sub_filter.h" #include "logging.h" @@ -186,7 +185,7 @@ rocksdb::Status CuckooChain::Delete(engine::Context &ctx, const Slice &user_key, CuckooChainMetadata metadata(false); auto s = getCuckooChainMetadata(ctx, ns_key, &metadata); - if (s.IsNotFound()) return rocksdb::Status::OK(); + if (s.IsNotFound()) return rocksdb::Status::NotFound("Not found"); if (!s.ok()) return s; s = validateMetadata(metadata); @@ -195,13 +194,17 @@ rocksdb::Status CuckooChain::Delete(engine::Context &ctx, const Slice &user_key, uint64_t hash = CuckooFilterHelper::Hash(item.data(), item.size()); uint8_t fingerprint = CuckooFilterHelper::GenerateFingerprint(hash); - CuckooSubFilters sub_filters; - s = buildSubFilters(ctx, ns_key, metadata, &sub_filters); - if (!s.ok()) return s; - 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_filters[filter_idx]->Delete(hash, fingerprint, &found); + s = sub_filter.Delete(hash, fingerprint, &found); if (!s.ok()) return s; if (!found) continue; @@ -209,19 +212,10 @@ rocksdb::Status CuckooChain::Delete(engine::Context &ctx, const Slice &user_key, metadata.size--; metadata.num_deleted_items++; *deleted = true; - break; - } - - if (!*deleted) return rocksdb::Status::OK(); - - std::vector freed_filter_indexes; - if (metadata.n_filters > 1 && - static_cast(metadata.num_deleted_items) > static_cast(metadata.size) * 0.10L) { - s = compactCuckooChain(&metadata, &sub_filters, &freed_filter_indexes, false); - if (!s.ok()) return s; + return commitDelete(ctx, user_key, ns_key, &metadata, &sub_filter); } - return commitDelete(ctx, user_key, ns_key, &metadata, &sub_filters, freed_filter_indexes); + return rocksdb::Status::OK(); } rocksdb::Status CuckooChain::tryCuckooInsert(engine::Context &ctx, const Slice &user_key, const std::string &ns_key, @@ -336,106 +330,15 @@ rocksdb::Status CuckooChain::commitSubFilterAndMetadata(engine::Context &ctx, co return storage_->Write(ctx, storage_->DefaultWriteOptions(), batch->GetWriteBatch()); } -rocksdb::Status CuckooChain::buildSubFilters(engine::Context &ctx, const std::string &ns_key, - const CuckooChainMetadata &metadata, CuckooSubFilters *sub_filters) { - sub_filters->clear(); - sub_filters->reserve(metadata.n_filters); - for (uint16_t filter_idx = 0; filter_idx < metadata.n_filters; ++filter_idx) { - uint32_t num_buckets = 0; - auto s = CuckooFilterHelper::GetFilterNumBuckets(metadata.base_capacity, metadata.expansion, metadata.bucket_size, - filter_idx, &num_buckets); - if (!s.ok()) return s; - - sub_filters->push_back(std::make_unique(storage_, ctx, ns_key, storage_->IsSlotIdEncoded(), - metadata.version, metadata.bucket_size, metadata.page_size, - filter_idx, num_buckets)); - } - return rocksdb::Status::OK(); -} - -rocksdb::Status CuckooChain::compactCuckooChain(CuckooChainMetadata *metadata, CuckooSubFilters *sub_filters, - std::vector *freed_filter_indexes, bool cont) { - for (int source_idx = static_cast(metadata->n_filters) - 1; source_idx >= 1; --source_idx) { - bool fully_compacted = false; - auto s = compactSingleSubFilter(static_cast(source_idx), sub_filters, &fully_compacted); - if (!s.ok()) return s; - - if (fully_compacted && source_idx == static_cast(metadata->n_filters) - 1) { - freed_filter_indexes->push_back(static_cast(source_idx)); - metadata->n_filters--; - } - - if (!fully_compacted && !cont) break; - } - - metadata->num_deleted_items = 0; - return rocksdb::Status::OK(); -} - -rocksdb::Status CuckooChain::compactSingleSubFilter(uint16_t source_index, CuckooSubFilters *sub_filters, - bool *fully_compacted) { - *fully_compacted = true; - auto *source_filter = (*sub_filters)[source_index].get(); - for (uint32_t bucket_idx = 0; bucket_idx < source_filter->NumBuckets(); ++bucket_idx) { - for (uint32_t slot_idx = 0; slot_idx < source_filter->BucketSize(); ++slot_idx) { - uint8_t fingerprint = 0; - auto s = source_filter->GetBucketSlot(bucket_idx, slot_idx, &fingerprint); - if (!s.ok()) return s; - if (fingerprint == 0) continue; - - bool relocated = false; - for (uint16_t target_idx = 0; target_idx < source_index; ++target_idx) { - bool inserted = false; - s = (*sub_filters)[target_idx]->TryInsert(bucket_idx, fingerprint, &inserted); - if (!s.ok()) return s; - if (!inserted) continue; - - s = source_filter->SetBucketSlot(bucket_idx, slot_idx, 0); - if (!s.ok()) return s; - relocated = true; - break; - } - - if (!relocated) *fully_compacted = false; - } - } - return rocksdb::Status::OK(); -} - -rocksdb::Status CuckooChain::deleteSubFilterPages(rocksdb::WriteBatchBase *batch, const std::string &ns_key, - const CuckooChainMetadata &metadata, uint16_t filter_index) { - uint32_t num_buckets = 0; - auto s = CuckooFilterHelper::GetFilterNumBuckets(metadata.base_capacity, metadata.expansion, metadata.bucket_size, - filter_index, &num_buckets); - if (!s.ok()) return s; - - uint32_t page_count = GetCuckooPageCount(num_buckets, metadata.page_size, metadata.bucket_size); - for (uint32_t page_index = 0; page_index < page_count; ++page_index) { - std::string page_key = - GetCuckooPageKey(ns_key, metadata.version, storage_->IsSlotIdEncoded(), filter_index, page_index); - s = batch->Delete(page_key); - if (!s.ok()) return s; - } - return rocksdb::Status::OK(); -} - rocksdb::Status CuckooChain::commitDelete(engine::Context &ctx, const Slice &user_key, const std::string &ns_key, - CuckooChainMetadata *metadata, CuckooSubFilters *sub_filters, - const std::vector &freed_filter_indexes) { + 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; - for (auto &sub_filter : *sub_filters) { - s = sub_filter->WriteToBatch(batch.Get()); - if (!s.ok()) return s; - } - - for (uint16_t filter_index : freed_filter_indexes) { - s = deleteSubFilterPages(batch.Get(), ns_key, *metadata, filter_index); - if (!s.ok()) return s; - } + s = sub_filter->WriteToBatch(batch.Get()); + if (!s.ok()) return s; std::string metadata_bytes; metadata->Encode(&metadata_bytes); diff --git a/src/types/redis_cuckoo_chain.h b/src/types/redis_cuckoo_chain.h index 4e69c963c03..05544b8a57f 100644 --- a/src/types/redis_cuckoo_chain.h +++ b/src/types/redis_cuckoo_chain.h @@ -20,9 +20,6 @@ #pragma once -#include -#include - #include "cuckoo_filter.h" #include "storage/redis_db.h" #include "storage/redis_metadata.h" @@ -54,8 +51,6 @@ class CuckooChain : public Database { rocksdb::Status Delete(engine::Context &ctx, const Slice &user_key, const Slice &item, bool *deleted); private: - using CuckooSubFilters = std::vector>; - // Loads metadata for a cuckoo filter key. rocksdb::Status getCuckooChainMetadata(engine::Context &ctx, const Slice &ns_key, CuckooChainMetadata *metadata); @@ -70,16 +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 buildSubFilters(engine::Context &ctx, const std::string &ns_key, const CuckooChainMetadata &metadata, - CuckooSubFilters *sub_filters); - rocksdb::Status compactCuckooChain(CuckooChainMetadata *metadata, CuckooSubFilters *sub_filters, - std::vector *freed_filter_indexes, bool cont); - rocksdb::Status compactSingleSubFilter(uint16_t source_index, CuckooSubFilters *sub_filters, bool *fully_compacted); - rocksdb::Status deleteSubFilterPages(rocksdb::WriteBatchBase *batch, const std::string &ns_key, - const CuckooChainMetadata &metadata, uint16_t filter_index); rocksdb::Status commitDelete(engine::Context &ctx, const Slice &user_key, const std::string &ns_key, - CuckooChainMetadata *metadata, CuckooSubFilters *sub_filters, - const std::vector &freed_filter_indexes); + CuckooChainMetadata *metadata, CuckooSubFilter *sub_filter); }; } // namespace redis diff --git a/tests/cppunit/types/cuckoo_filter_test.cc b/tests/cppunit/types/cuckoo_filter_test.cc index 1a4153645a1..f4e86c24492 100644 --- a/tests/cppunit/types/cuckoo_filter_test.cc +++ b/tests/cppunit/types/cuckoo_filter_test.cc @@ -784,11 +784,11 @@ TEST_F(RedisCuckooFilterTest, ExpansionWritesNewFilterIndexPage) { EXPECT_EQ(page.size(), expected_page_size); } -TEST_F(RedisCuckooFilterTest, DeleteMissingKeyReturnsFalse) { +TEST_F(RedisCuckooFilterTest, DeleteMissingKeyReturnsNotFound) { bool deleted = true; auto s = cuckoo_->Delete(*ctx_, key_, "missing", &deleted); - ASSERT_TRUE(s.ok()) << s.ToString(); - EXPECT_FALSE(deleted); + EXPECT_TRUE(s.IsNotFound()) << s.ToString(); + EXPECT_NE(s.ToString().find("Not found"), std::string::npos); } TEST_F(RedisCuckooFilterTest, DeleteBasicClearsOneItem) { @@ -899,7 +899,7 @@ TEST_F(RedisCuckooFilterTest, DeleteSearchesNewestFilterFirst) { EXPECT_EQ(readFingerprint(key_, stored_metadata, 1, bucket, 0), 0); } -TEST_F(RedisCuckooFilterTest, DeleteTriggersCompactAndFreesLatestFilter) { +TEST_F(RedisCuckooFilterTest, DeleteDoesNotCompactFilters) { CuckooChainMetadata metadata; metadata.size = 8; metadata.base_capacity = 2; @@ -933,77 +933,12 @@ TEST_F(RedisCuckooFilterTest, DeleteTriggersCompactAndFreesLatestFilter) { auto stored_metadata = getMetadata(key_); EXPECT_EQ(stored_metadata.size, 7); - EXPECT_EQ(stored_metadata.n_filters, 1); - EXPECT_EQ(stored_metadata.num_deleted_items, 0); - EXPECT_EQ(readFingerprint(key_, stored_metadata, 0, movable_bucket, 0), movable_fingerprint); + 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.IsNotFound()) << s.ToString(); -} - -TEST_F(RedisCuckooFilterTest, CompactStopsOnFailedRelocation) { - CuckooChainMetadata metadata; - metadata.size = 8; - metadata.base_capacity = 2; - metadata.bucket_size = 1; - 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, 4); - - constexpr uint32_t blocked_bucket = 0; - constexpr uint8_t blocked_fingerprint = 7; - auto blocked_alt = redis::CuckooFilterHelper::GetAltBucketIndex(blocked_bucket, blocked_fingerprint, num_buckets); - ASSERT_NE(blocked_alt, blocked_bucket); - placeFingerprint(key_, metadata, 0, num_buckets, blocked_bucket, 0, 101); - placeFingerprint(key_, metadata, 0, num_buckets, blocked_alt, 0, 102); - placeFingerprint(key_, metadata, 1, num_buckets, blocked_bucket, 0, blocked_fingerprint); - - uint32_t movable_bucket = num_buckets; - for (uint32_t bucket = 0; bucket < num_buckets; ++bucket) { - if (bucket != blocked_bucket && bucket != blocked_alt) { - movable_bucket = bucket; - break; - } - } - ASSERT_LT(movable_bucket, num_buckets); - constexpr uint8_t movable_fingerprint = 88; - placeFingerprint(key_, metadata, 1, num_buckets, movable_bucket, 0, movable_fingerprint); - - std::string item; - uint8_t delete_fingerprint = 0; - uint32_t delete_bucket = num_buckets; - for (int i = 0; i < 10000; ++i) { - auto candidate = "delete_" + std::to_string(i); - auto hash = redis::CuckooFilterHelper::Hash(candidate); - auto bucket = static_cast(hash % num_buckets); - if (bucket == blocked_bucket || bucket == movable_bucket) continue; - item = candidate; - delete_fingerprint = redis::CuckooFilterHelper::GenerateFingerprint(hash); - delete_bucket = bucket; - break; - } - ASSERT_FALSE(item.empty()); - placeFingerprint(key_, metadata, 1, num_buckets, delete_bucket, 0, delete_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, 0); - EXPECT_EQ(readFingerprint(key_, stored_metadata, 0, movable_bucket, 0), movable_fingerprint); - EXPECT_EQ(readFingerprint(key_, stored_metadata, 1, blocked_bucket, 0), blocked_fingerprint); + 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 09e7fe7e194..8e0ba24afb0 100644 --- a/tests/gocase/unit/type/bloom/cuckoo_filter_test.go +++ b/tests/gocase/unit/type/bloom/cuckoo_filter_test.go @@ -116,7 +116,7 @@ func TestCuckooFilter(t *testing.T) { t.Run("Del missing key", func(t *testing.T) { key := "test_cuckoo_filter_del_missing" require.NoError(t, rdb.Del(ctx, key).Err()) - require.Equal(t, int64(0), rdb.Do(ctx, "cf.del", key, "item").Val()) + require.ErrorContains(t, rdb.Do(ctx, "cf.del", key, "item").Err(), "Not found") }) t.Run("Del wrong type", func(t *testing.T) { From bc8f986faf56b6720db098ad7dcbf066bad49cf4 Mon Sep 17 00:00:00 2001 From: nagisa-kun <1434936049@qq.com> Date: Sun, 9 Aug 2026 15:11:47 +0800 Subject: [PATCH 3/4] fix: const --- src/types/cuckoo_filter.h | 1 + src/types/cuckoo_filter_sub_filter.cc | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) 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_sub_filter.cc b/src/types/cuckoo_filter_sub_filter.cc index c1c49606d5b..e6538c7a629 100644 --- a/src/types/cuckoo_filter_sub_filter.cc +++ b/src/types/cuckoo_filter_sub_filter.cc @@ -53,25 +53,25 @@ rocksdb::Status CuckooSubFilter::Delete(uint64_t hash, uint8_t fingerprint, bool if (!s.ok()) return s; for (uint32_t slot_idx = 0; slot_idx < bucket_size_; ++slot_idx) { - uint8_t current_fingerprint = 0; + 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, 0); + 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 = 0; + 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, 0); + return pages_.SetBucketSlot(filter_index_, num_buckets_, bucket2_idx, slot_idx, kEmptyCuckooFingerprint); } return rocksdb::Status::OK(); From dae7110607fe016467d2c6c2ab20c331e820149c Mon Sep 17 00:00:00 2001 From: nagisa-kun <1434936049@qq.com> Date: Sun, 16 Aug 2026 14:28:19 +0800 Subject: [PATCH 4/4] fix: delete when page is empty --- src/types/cuckoo_filter_page.cc | 13 ++++++- src/types/cuckoo_filter_page.h | 1 + .../cppunit/types/cuckoo_filter_page_test.cc | 37 +++++++++++++++++++ tests/cppunit/types/cuckoo_filter_test.cc | 4 +- 4 files changed, 53 insertions(+), 2 deletions(-) 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/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 f4e86c24492..bff84de4a12 100644 --- a/tests/cppunit/types/cuckoo_filter_test.cc +++ b/tests/cppunit/types/cuckoo_filter_test.cc @@ -896,7 +896,9 @@ TEST_F(RedisCuckooFilterTest, DeleteSearchesNewestFilterFirst) { auto stored_metadata = getMetadata(key_); EXPECT_EQ(stored_metadata.n_filters, 2); EXPECT_EQ(readFingerprint(key_, stored_metadata, 0, bucket, 0), fingerprint); - EXPECT_EQ(readFingerprint(key_, stored_metadata, 1, bucket, 0), 0); + 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) {