From 8c5cb8eb045d575601ffa16092bc0d626c399688 Mon Sep 17 00:00:00 2001 From: Z-G-H1 <1582696958@qq.com> Date: Sun, 7 Dec 2025 20:34:29 +0800 Subject: [PATCH 1/5] fix blockcache --- src/storage/src/redis.cc | 14 ++++++++++++-- src/storage/src/redis.h | 1 + src/storage/src/redis_hashes.cc | 10 ++++++++++ src/storage/src/redis_lists.cc | 10 ++++++++++ src/storage/src/redis_sets.cc | 10 ++++++++++ src/storage/src/redis_streams.cc | 10 ++++++++++ src/storage/src/redis_zsets.cc | 11 +++++++++++ 7 files changed, 64 insertions(+), 2 deletions(-) diff --git a/src/storage/src/redis.cc b/src/storage/src/redis.cc index 3066a62759..5e390cb7e1 100644 --- a/src/storage/src/redis.cc +++ b/src/storage/src/redis.cc @@ -115,8 +115,18 @@ void Redis::GetRocksDBInfo(std::string &info, const char *prefix) { string_stream << "#" << prefix << "RocksDB" << "\r\n"; auto write_stream_key_value=[&](const Slice& property, const char *metric) { - uint64_t value; - db_->GetAggregatedIntProperty(property, &value); + uint64_t value = 0; + // Avoids double-counting of shared cache. + if (share_block_cache_ && handles_.size() > 0 && + (property == rocksdb::DB::Properties::kBlockCacheCapacity || + property == rocksdb::DB::Properties::kBlockCacheUsage || + property == rocksdb::DB::Properties::kBlockCachePinnedUsage)) { + std::string sval; + db_->GetProperty(handles_[0], property, &sval); + value = std::strtoull(sval.c_str(), nullptr, 10); + } else { + db_->GetAggregatedIntProperty(property, &value); + } string_stream << prefix << metric << ':' << value << "\r\n"; }; diff --git a/src/storage/src/redis.h b/src/storage/src/redis.h index 21eaa2aa94..29fdac0369 100644 --- a/src/storage/src/redis.h +++ b/src/storage/src/redis.h @@ -122,6 +122,7 @@ class Redis { std::shared_ptr lock_mgr_; rocksdb::DB* db_ = nullptr; + bool share_block_cache_ = false; std::vector handles_; rocksdb::WriteOptions default_write_options_; rocksdb::ReadOptions default_read_options_; diff --git a/src/storage/src/redis_hashes.cc b/src/storage/src/redis_hashes.cc index b885a487dd..22cab8e0d1 100644 --- a/src/storage/src/redis_hashes.cc +++ b/src/storage/src/redis_hashes.cc @@ -64,6 +64,7 @@ Status RedisHashes::Open(const StorageOptions& storage_options, const std::strin column_families.emplace_back(rocksdb::kDefaultColumnFamilyName, meta_cf_ops); // Data CF column_families.emplace_back("data_cf", data_cf_ops); + share_block_cache_ = storage_options.share_block_cache; return rocksdb::DB::Open(db_ops, db_path, column_families, &handles_, &db_); } @@ -81,6 +82,15 @@ Status RedisHashes::GetProperty(const std::string& property, uint64_t* out) { std::string value; db_->GetProperty(handles_[0], property, &value); *out = std::strtoull(value.c_str(), nullptr, 10); + + // Avoids double counting when share_block_cache is enabled. + if (share_block_cache_ && + (property == "rocksdb.block-cache-usage" || + property == "rocksdb.block-cache-capacity" || + property == "rocksdb.block-cache-pinned-usage")) { + return Status::OK(); + } + db_->GetProperty(handles_[1], property, &value); *out += std::strtoull(value.c_str(), nullptr, 10); return Status::OK(); diff --git a/src/storage/src/redis_lists.cc b/src/storage/src/redis_lists.cc index 09a045a4d4..f941e889c4 100644 --- a/src/storage/src/redis_lists.cc +++ b/src/storage/src/redis_lists.cc @@ -71,6 +71,7 @@ Status RedisLists::Open(const StorageOptions& storage_options, const std::string column_families.emplace_back(rocksdb::kDefaultColumnFamilyName, meta_cf_ops); // Data CF column_families.emplace_back("data_cf", data_cf_ops); + share_block_cache_ = storage_options.share_block_cache; return rocksdb::DB::Open(db_ops, db_path, column_families, &handles_, &db_); } @@ -88,6 +89,15 @@ Status RedisLists::GetProperty(const std::string& property, uint64_t* out) { std::string value; db_->GetProperty(handles_[0], property, &value); *out = std::strtoull(value.c_str(), nullptr, 10); + + // Avoids double counting when share_block_cache is enabled. + if (share_block_cache_ && + (property == "rocksdb.block-cache-usage" || + property == "rocksdb.block-cache-capacity" || + property == "rocksdb.block-cache-pinned-usage")) { + return Status::OK(); + } + db_->GetProperty(handles_[1], property, &value); *out += std::strtoull(value.c_str(), nullptr, 10); return Status::OK(); diff --git a/src/storage/src/redis_sets.cc b/src/storage/src/redis_sets.cc index 5707e032d4..ec67ee1b55 100644 --- a/src/storage/src/redis_sets.cc +++ b/src/storage/src/redis_sets.cc @@ -71,6 +71,7 @@ rocksdb::Status RedisSets::Open(const StorageOptions& storage_options, const std column_families.emplace_back(rocksdb::kDefaultColumnFamilyName, meta_cf_ops); // Member CF column_families.emplace_back("member_cf", member_cf_ops); + share_block_cache_ = storage_options.share_block_cache; return rocksdb::DB::Open(db_ops, db_path, column_families, &handles_, &db_); } @@ -88,6 +89,15 @@ rocksdb::Status RedisSets::GetProperty(const std::string& property, uint64_t* ou std::string value; db_->GetProperty(handles_[0], property, &value); *out = std::strtoull(value.c_str(), nullptr, 10); + + // Avoids double counting when share_block_cache is enabled. + if (share_block_cache_ && + (property == "rocksdb.block-cache-usage" || + property == "rocksdb.block-cache-capacity" || + property == "rocksdb.block-cache-pinned-usage")) { + return rocksdb::Status::OK(); + } + db_->GetProperty(handles_[1], property, &value); *out += std::strtoull(value.c_str(), nullptr, 10); return rocksdb::Status::OK(); diff --git a/src/storage/src/redis_streams.cc b/src/storage/src/redis_streams.cc index 48578ae5b5..d3d752c2a3 100644 --- a/src/storage/src/redis_streams.cc +++ b/src/storage/src/redis_streams.cc @@ -367,6 +367,7 @@ Status RedisStreams::Open(const StorageOptions& storage_options, const std::stri column_families.emplace_back(rocksdb::kDefaultColumnFamilyName, meta_cf_ops); // Data CF column_families.emplace_back("data_cf", data_cf_ops); + share_block_cache_ = storage_options.share_block_cache; return rocksdb::DB::Open(db_ops, db_path, column_families, &handles_, &db_); } @@ -385,6 +386,15 @@ Status RedisStreams::GetProperty(const std::string& property, uint64_t* out) { std::string value; db_->GetProperty(handles_[0], property, &value); *out = std::strtoull(value.c_str(), nullptr, 10); + + // Avoids double counting when share_block_cache is enabled. + if (share_block_cache_ && + (property == "rocksdb.block-cache-usage" || + property == "rocksdb.block-cache-capacity" || + property == "rocksdb.block-cache-pinned-usage")) { + return Status::OK(); + } + db_->GetProperty(handles_[1], property, &value); *out += std::strtoull(value.c_str(), nullptr, 10); return Status::OK(); diff --git a/src/storage/src/redis_zsets.cc b/src/storage/src/redis_zsets.cc index a4153c9d9a..533d262869 100644 --- a/src/storage/src/redis_zsets.cc +++ b/src/storage/src/redis_zsets.cc @@ -83,6 +83,7 @@ Status RedisZSets::Open(const StorageOptions& storage_options, const std::string column_families.emplace_back(rocksdb::kDefaultColumnFamilyName, meta_cf_ops); column_families.emplace_back("data_cf", data_cf_ops); column_families.emplace_back("score_cf", score_cf_ops); + share_block_cache_ = storage_options.share_block_cache; return rocksdb::DB::Open(db_ops, db_path, column_families, &handles_, &db_); } @@ -101,6 +102,16 @@ Status RedisZSets::GetProperty(const std::string& property, uint64_t* out) { std::string value; db_->GetProperty(handles_[0], property, &value); *out = std::strtoull(value.c_str(), nullptr, 10); + + // If share_block_cache is enabled, block cache related properties are shared among CFs, + // so we only need to get the value from the first CF to avoid double counting. + if (share_block_cache_ && + (property == "rocksdb.block-cache-usage" || + property == "rocksdb.block-cache-capacity" || + property == "rocksdb.block-cache-pinned-usage")) { + return Status::OK(); + } + db_->GetProperty(handles_[1], property, &value); *out += std::strtoull(value.c_str(), nullptr, 10); db_->GetProperty(handles_[2], property, &value); From 81733b924c9676442efc5cc0801714c2266681de Mon Sep 17 00:00:00 2001 From: Z-G-H1 <1582696958@qq.com> Date: Sun, 7 Dec 2025 20:34:49 +0800 Subject: [PATCH 2/5] add test --- tests/integration/server_test.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/integration/server_test.go b/tests/integration/server_test.go index 9ef0c63e91..490657fc07 100644 --- a/tests/integration/server_test.go +++ b/tests/integration/server_test.go @@ -792,6 +792,30 @@ var _ = Describe("Server", func() { Expect(adminCmdList).To(ContainSubstring("ping")) Expect(adminCmdList).To(ContainSubstring("monitor")) }) + // fix: verify that shared block cache does not cause double counting of block cache metrics + It("should report correct block cache capacity when share-block-cache is yes", func() { + // Verify share-block-cache is enabled + shareBlockCacheConfig := client.ConfigGet(ctx, "share-block-cache") + Expect(shareBlockCacheConfig.Err()).NotTo(HaveOccurred()) + Expect(shareBlockCacheConfig.Val()).To(Equal(map[string]string{"share-block-cache": "yes"})) + + info := client.Info(ctx, "rocksdb") + Expect(info.Err()).NotTo(HaveOccurred()) + infoStr := info.Val() + + Expect(infoStr).To(ContainSubstring("strings_block_cache_capacity:")) + Expect(infoStr).To(ContainSubstring("hashes_block_cache_capacity:")) + Expect(infoStr).To(ContainSubstring("lists_block_cache_capacity:")) + Expect(infoStr).To(ContainSubstring("sets_block_cache_capacity:")) + Expect(infoStr).To(ContainSubstring("zsets_block_cache_capacity:")) + + // Simple string-based verification that values are present + Expect(infoStr).To(MatchRegexp(`strings_block_cache_capacity:\d+`)) + Expect(infoStr).To(MatchRegexp(`hashes_block_cache_capacity:\d+`)) + Expect(infoStr).To(MatchRegexp(`lists_block_cache_capacity:\d+`)) + Expect(infoStr).To(MatchRegexp(`sets_block_cache_capacity:\d+`)) + Expect(infoStr).To(MatchRegexp(`zsets_block_cache_capacity:\d+`)) + }) // fix add auth command to admin-thread-pool It("should process auth command in admin thread pool", func() { From 318cb7bee0335380062ed1334aba10574b609e41 Mon Sep 17 00:00:00 2001 From: Z-G-H1 <1582696958@qq.com> Date: Tue, 16 Dec 2025 12:37:32 +0800 Subject: [PATCH 3/5] modify test --- tests/integration/server_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/integration/server_test.go b/tests/integration/server_test.go index 490657fc07..0f79ea0a2a 100644 --- a/tests/integration/server_test.go +++ b/tests/integration/server_test.go @@ -797,7 +797,10 @@ var _ = Describe("Server", func() { // Verify share-block-cache is enabled shareBlockCacheConfig := client.ConfigGet(ctx, "share-block-cache") Expect(shareBlockCacheConfig.Err()).NotTo(HaveOccurred()) - Expect(shareBlockCacheConfig.Val()).To(Equal(map[string]string{"share-block-cache": "yes"})) + shareBlockCacheVal := shareBlockCacheConfig.Val()["share-block-cache"] + if shareBlockCacheVal != "yes" { + return + } info := client.Info(ctx, "rocksdb") Expect(info.Err()).NotTo(HaveOccurred()) From 8230c3328cd32424e2c34e67ebe0ad724e889119 Mon Sep 17 00:00:00 2001 From: Z-G-H1 <1582696958@qq.com> Date: Tue, 16 Dec 2025 14:17:35 +0800 Subject: [PATCH 4/5] ci: trigger From 557d8c965c6bbf24224e15c4d760b56bfcc883e7 Mon Sep 17 00:00:00 2001 From: Z-G-H1 <1582696958@qq.com> Date: Tue, 16 Dec 2025 15:34:40 +0800 Subject: [PATCH 5/5] ci: trigger