From 7818560194c82b6e875dd7cf13906afc1a9553fe Mon Sep 17 00:00:00 2001
From: Levi Tamasi <ltamasi@fb.com>
Date: Mon, 29 Aug 2022 16:11:59 -0700
Subject: [PATCH] Add a dedicated cache entry role for blobs (#10601)

Summary:
The patch adds a dedicated cache entry role for blob values and switches
to a registered deleter so that blobs show up as a separate bucket
(as opposed to "Misc") in the cache occupancy statistics, e.g.

```
Block cache entry stats(count,size,portion): DataBlock(133515,531.73 MB,13.6866%) BlobValue(1824855,3.10 GB,81.7071%) Misc(1,0.00 KB,0%)
```

Pull Request resolved: https://github.com/facebook/rocksdb/pull/10601

Test Plan: Ran `make check` and tested the cache occupancy statistics using `db_bench`.

Reviewed By: riversand963

Differential Revision: D39107915

Pulled By: ltamasi

fbshipit-source-id: 8446c3b190a41a144030df73f318eeda4398c125
---
 cache/cache_entry_roles.cc   |  2 ++
 db/blob/blob_contents.cc     | 10 ++++------
 db/blob/blob_contents.h      |  2 --
 db/blob/blob_file_builder.cc |  6 +++++-
 db/blob/blob_source.cc       | 10 +++++++---
 include/rocksdb/cache.h      |  5 ++++-
 6 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/cache/cache_entry_roles.cc b/cache/cache_entry_roles.cc
index 1aebedd24..b27349554 100644
--- a/cache/cache_entry_roles.cc
+++ b/cache/cache_entry_roles.cc
@@ -23,6 +23,7 @@ std::array<std::string, kNumCacheEntryRoles> kCacheEntryRoleToCamelString{{
     "FilterConstruction",
     "BlockBasedTableReader",
     "FileMetadata",
+    "BlobValue",
     "BlobCache",
     "Misc",
 }};
@@ -39,6 +40,7 @@ std::array<std::string, kNumCacheEntryRoles> kCacheEntryRoleToHyphenString{{
     "filter-construction",
     "block-based-table-reader",
     "file-metadata",
+    "blob-value",
     "blob-cache",
     "misc",
 }};
diff --git a/db/blob/blob_contents.cc b/db/blob/blob_contents.cc
index df5e47659..50f6132de 100644
--- a/db/blob/blob_contents.cc
+++ b/db/blob/blob_contents.cc
@@ -7,6 +7,7 @@
 
 #include <cassert>
 
+#include "cache/cache_entry_roles.h"
 #include "cache/cache_helpers.h"
 #include "port/malloc.h"
 
@@ -62,13 +63,10 @@ Status BlobContents::SaveToCallback(void* from_obj, size_t from_offset,
   return Status::OK();
 }
 
-void BlobContents::DeleteCallback(const Slice& key, void* value) {
-  DeleteCacheEntry<BlobContents>(key, value);
-}
-
 Cache::CacheItemHelper* BlobContents::GetCacheItemHelper() {
-  static Cache::CacheItemHelper cache_helper(&SizeCallback, &SaveToCallback,
-                                             &DeleteCallback);
+  static Cache::CacheItemHelper cache_helper(
+      &SizeCallback, &SaveToCallback,
+      GetCacheEntryDeleterForRole<BlobContents, CacheEntryRole::kBlobValue>());
 
   return &cache_helper;
 }
diff --git a/db/blob/blob_contents.h b/db/blob/blob_contents.h
index 9ea9babf2..02d0e3f18 100644
--- a/db/blob/blob_contents.h
+++ b/db/blob/blob_contents.h
@@ -40,8 +40,6 @@ class BlobContents {
   static Status SaveToCallback(void* from_obj, size_t from_offset,
                                size_t length, void* out);
 
-  static void DeleteCallback(const Slice& key, void* value);
-
   static Cache::CacheItemHelper* GetCacheItemHelper();
 
   static Status CreateCallback(const void* buf, size_t size, void** out_obj,
diff --git a/db/blob/blob_file_builder.cc b/db/blob/blob_file_builder.cc
index 65939d3e3..a8e94b6f4 100644
--- a/db/blob/blob_file_builder.cc
+++ b/db/blob/blob_file_builder.cc
@@ -416,8 +416,12 @@ Status BlobFileBuilder::PutBlobIntoCacheIfNeeded(const Slice& blob,
     std::unique_ptr<BlobContents> buf =
         BlobContents::Create(std::move(allocation), blob.size());
 
+    Cache::CacheItemHelper* const cache_item_helper =
+        BlobContents::GetCacheItemHelper();
+    assert(cache_item_helper);
+
     s = blob_cache->Insert(key, buf.get(), buf->ApproximateMemoryUsage(),
-                           &BlobContents::DeleteCallback,
+                           cache_item_helper->del_cb,
                            nullptr /* cache_handle */, priority);
     if (s.ok()) {
       RecordTick(statistics, BLOB_DB_CACHE_ADD);
diff --git a/db/blob/blob_source.cc b/db/blob/blob_source.cc
index f2571be90..11a91d9e4 100644
--- a/db/blob/blob_source.cc
+++ b/db/blob/blob_source.cc
@@ -128,11 +128,15 @@ Status BlobSource::InsertEntryIntoCache(const Slice& key, BlobContents* value,
                                         Cache::Priority priority) const {
   Status s;
 
+  Cache::CacheItemHelper* const cache_item_helper =
+      BlobContents::GetCacheItemHelper();
+  assert(cache_item_helper);
+
   if (lowest_used_cache_tier_ == CacheTier::kNonVolatileBlockTier) {
-    s = blob_cache_->Insert(key, value, BlobContents::GetCacheItemHelper(),
-                            charge, cache_handle, priority);
+    s = blob_cache_->Insert(key, value, cache_item_helper, charge, cache_handle,
+                            priority);
   } else {
-    s = blob_cache_->Insert(key, value, charge, &BlobContents::DeleteCallback,
+    s = blob_cache_->Insert(key, value, charge, cache_item_helper->del_cb,
                             cache_handle, priority);
   }
 
diff --git a/include/rocksdb/cache.h b/include/rocksdb/cache.h
index d38fc37c4..8bad61b52 100644
--- a/include/rocksdb/cache.h
+++ b/include/rocksdb/cache.h
@@ -600,7 +600,10 @@ enum class CacheEntryRole {
   kBlockBasedTableReader,
   // FileMetadata's charge to account for its memory usage
   kFileMetadata,
-  // Blob cache's charge to account for its memory usage
+  // Blob value (when using the same cache as block cache and blob cache)
+  kBlobValue,
+  // Blob cache's charge to account for its memory usage (when using a
+  // separate block cache and blob cache)
   kBlobCache,
   // Default bucket, for miscellaneous cache entries. Do not use for
   // entries that could potentially add up to large usage.
-- 
GitLab