Put Cache and CacheWrapper in new public header (#11192)

Summary:
The definition of the Cache class should not be needed by the vast majority of RocksDB users, so I think it is just distracting to include it in cache.h, which is primarily needed for configuring and creating caches. This change moves the class to a new header advanced_cache.h. It is just cut-and-paste except for modifying the class API comment.

In general, operations on shared_ptr<Cache> should continue to work when only a forward declaration of Cache is available, as long as all the Cache instances provided are already shared_ptr. See https://stackoverflow.com/a/17650101/454544

Also, the most common way to customize a Cache is by wrapping an existing implementation, so it makes sense to provide CacheWrapper in the public API. This was a cut-and-paste job except removing the implementation of Name() so that derived classes must provide it.

Intended follow-up: consolidate Release() into one function to reduce customization bugs / confusion

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

Test Plan: `make check`

Reviewed By: anand1976

Differential Revision: D43055487

Pulled By: pdillinger

fbshipit-source-id: 7b05492df35e0f30b581b4c24c579bc275b6d110

HISTORY.md

@@ -22,6 +22,7 @@
 * Removed the deprecated version of these utility functions and the corresponding Java bindings: `LoadOptionsFromFile`, `LoadLatestOptions`, `CheckOptionsCompatibility`.
 
 ### Public API Changes
+* Moved rarely-needed Cache class definition to new advanced_cache.h, and added a CacheWrapper class to advanced_cache.h. Minor changes to SimCache API definitions.
 * Completely removed the following deprecated/obsolete statistics: the tickers `BLOCK_CACHE_INDEX_BYTES_EVICT`, `BLOCK_CACHE_FILTER_BYTES_EVICT`, `BLOOM_FILTER_MICROS`, `NO_FILE_CLOSES`, `STALL_L0_SLOWDOWN_MICROS`, `STALL_MEMTABLE_COMPACTION_MICROS`, `STALL_L0_NUM_FILES_MICROS`, `RATE_LIMIT_DELAY_MILLIS`, `NO_ITERATORS`, `NUMBER_FILTERED_DELETES`, `WRITE_TIMEDOUT`, `BLOB_DB_GC_NUM_KEYS_OVERWRITTEN`, `BLOB_DB_GC_NUM_KEYS_EXPIRED`, `BLOB_DB_GC_BYTES_OVERWRITTEN`, `BLOB_DB_GC_BYTES_EXPIRED`, `BLOCK_CACHE_COMPRESSION_DICT_BYTES_EVICT` as well as the histograms `STALL_L0_SLOWDOWN_COUNT`, `STALL_MEMTABLE_COMPACTION_COUNT`, `STALL_L0_NUM_FILES_COUNT`, `HARD_RATE_LIMIT_DELAY_COUNT`, `SOFT_RATE_LIMIT_DELAY_COUNT`, `BLOB_DB_GC_MICROS`, and `NUM_DATA_BLOCKS_READ_PER_LEVEL`. Note that as a result, the C++ enum values of the still supported statistics have changed. Developers are advised to not rely on the actual numeric values.
 * Deprecated IngestExternalFileOptions::write_global_seqno and change default to false. This option only needs to be set to true to generate a DB compatible with RocksDB versions before 5.16.0.
 * Remove deprecated APIs `GetColumnFamilyOptionsFrom{Map|String}(const ColumnFamilyOptions&, ..)`, `GetDBOptionsFrom{Map|String}(const DBOptions&, ..)`, `GetBlockBasedTableOptionsFrom{Map|String}(const BlockBasedTableOptions& table_options, ..)` and ` GetPlainTableOptionsFrom{Map|String}(const PlainTableOptions& table_options,..)`.

cache/cache_bench_tool.cc

@@ -16,7 +16,7 @@
 #include "db/db_impl/db_impl.h"
 #include "monitoring/histogram.h"
 #include "port/port.h"
-#include "rocksdb/cache.h"
+#include "rocksdb/advanced_cache.h"
 #include "rocksdb/convenience.h"
 #include "rocksdb/db.h"
 #include "rocksdb/env.h"

cache/cache_helpers.h

@@ -7,7 +7,7 @@
 
 #include <cassert>
 
-#include "rocksdb/cache.h"
+#include "rocksdb/advanced_cache.h"
 #include "rocksdb/rocksdb_namespace.h"
 
 namespace ROCKSDB_NAMESPACE {

cache/cache_key.cc

@@ -8,7 +8,7 @@
 #include <algorithm>
 #include <atomic>
 
-#include "rocksdb/cache.h"
+#include "rocksdb/advanced_cache.h"
 #include "table/unique_id_impl.h"
 #include "util/hash.h"
 #include "util/math.h"

cache/charged_cache.h

@@ -8,7 +8,7 @@
 #include <string>
 
 #include "port/port.h"
-#include "rocksdb/cache.h"
+#include "rocksdb/advanced_cache.h"
 
 namespace ROCKSDB_NAMESPACE {
 

cache/sharded_cache.h

@@ -15,7 +15,7 @@
 
 #include "port/lang.h"
 #include "port/port.h"
-#include "rocksdb/cache.h"
+#include "rocksdb/advanced_cache.h"
 #include "util/hash.h"
 #include "util/mutexlock.h"
 

cache/typed_cache.h

@@ -29,8 +29,8 @@
 #include <type_traits>
 
 #include "cache/cache_helpers.h"
+#include "rocksdb/advanced_cache.h"
 #include "rocksdb/advanced_options.h"
-#include "rocksdb/cache.h"
 
 namespace ROCKSDB_NAMESPACE {
 

db/blob/blob_contents.h

@@ -8,7 +8,7 @@
 #include <memory>
 
 #include "memory/memory_allocator.h"
-#include "rocksdb/cache.h"
+#include "rocksdb/advanced_cache.h"
 #include "rocksdb/rocksdb_namespace.h"
 #include "rocksdb/slice.h"
 #include "rocksdb/status.h"

db/c.cc

@@ -16,7 +16,7 @@
 #include <vector>
 
 #include "port/port.h"
-#include "rocksdb/cache.h"
+#include "rocksdb/advanced_cache.h"
 #include "rocksdb/compaction_filter.h"
 #include "rocksdb/comparator.h"
 #include "rocksdb/convenience.h"
@@ -6397,4 +6397,3 @@ void rocksdb_enable_manual_compaction(rocksdb_t* db) {
 }
 
 }  // end extern "C"
-

db/db_block_cache_test.cc

@@ -287,9 +287,11 @@ class PersistentCacheFromCache : public PersistentCache {
 };
 
 class ReadOnlyCacheWrapper : public CacheWrapper {
+ public:
   using CacheWrapper::CacheWrapper;
 
-  using Cache::Insert;
+  const char* Name() const override { return "ReadOnlyCacheWrapper"; }
+
   Status Insert(const Slice& /*key*/, Cache::ObjectPtr /*value*/,
                 const CacheItemHelper* /*helper*/, size_t /*charge*/,
                 Handle** /*handle*/, Priority /*priority*/) override {
@@ -711,7 +713,8 @@ class LookupLiarCache : public CacheWrapper {
   explicit LookupLiarCache(std::shared_ptr<Cache> target)
       : CacheWrapper(std::move(target)) {}
 
-  using Cache::Lookup;
+  const char* Name() const override { return "LookupLiarCache"; }
+
   Handle* Lookup(const Slice& key, const CacheItemHelper* helper = nullptr,
                  CreateContext* create_context = nullptr,
                  Priority priority = Priority::LOW, bool wait = true,

db/db_test_util.h

@@ -909,82 +909,6 @@ class TestPutOperator : public MergeOperator {
   virtual const char* Name() const override { return "TestPutOperator"; }
 };
 
-// A wrapper around Cache that can easily be extended with instrumentation,
-// etc.
-class CacheWrapper : public Cache {
- public:
-  explicit CacheWrapper(std::shared_ptr<Cache> target)
-      : target_(std::move(target)) {}
-
-  const char* Name() const override { return target_->Name(); }
-
-  Status Insert(const Slice& key, ObjectPtr value,
-                const CacheItemHelper* helper, size_t charge,
-                Handle** handle = nullptr,
-                Priority priority = Priority::LOW) override {
-    return target_->Insert(key, value, helper, charge, handle, priority);
-  }
-
-  Handle* Lookup(const Slice& key, const CacheItemHelper* helper,
-                 CreateContext* create_context,
-                 Priority priority = Priority::LOW, bool wait = true,
-                 Statistics* stats = nullptr) override {
-    return target_->Lookup(key, helper, create_context, priority, wait, stats);
-  }
-
-  bool Ref(Handle* handle) override { return target_->Ref(handle); }
-
-  using Cache::Release;
-  bool Release(Handle* handle, bool erase_if_last_ref = false) override {
-    return target_->Release(handle, erase_if_last_ref);
-  }
-
-  ObjectPtr Value(Handle* handle) override { return target_->Value(handle); }
-
-  void Erase(const Slice& key) override { target_->Erase(key); }
-  uint64_t NewId() override { return target_->NewId(); }
-
-  void SetCapacity(size_t capacity) override { target_->SetCapacity(capacity); }
-
-  void SetStrictCapacityLimit(bool strict_capacity_limit) override {
-    target_->SetStrictCapacityLimit(strict_capacity_limit);
-  }
-
-  bool HasStrictCapacityLimit() const override {
-    return target_->HasStrictCapacityLimit();
-  }
-
-  size_t GetCapacity() const override { return target_->GetCapacity(); }
-
-  size_t GetUsage() const override { return target_->GetUsage(); }
-
-  size_t GetUsage(Handle* handle) const override {
-    return target_->GetUsage(handle);
-  }
-
-  size_t GetPinnedUsage() const override { return target_->GetPinnedUsage(); }
-
-  size_t GetCharge(Handle* handle) const override {
-    return target_->GetCharge(handle);
-  }
-
-  const CacheItemHelper* GetCacheItemHelper(Handle* handle) const override {
-    return target_->GetCacheItemHelper(handle);
-  }
-
-  void ApplyToAllEntries(
-      const std::function<void(const Slice& key, ObjectPtr value, size_t charge,
-                               const CacheItemHelper* helper)>& callback,
-      const ApplyToAllEntriesOptions& opts) override {
-    target_->ApplyToAllEntries(callback, opts);
-  }
-
-  void EraseUnRefEntries() override { target_->EraseUnRefEntries(); }
-
- protected:
-  std::shared_ptr<Cache> target_;
-};
-
 /*
  * A cache wrapper that tracks certain CacheEntryRole's cache charge, its
  * peaks and increments
@@ -1002,6 +926,8 @@ class TargetCacheChargeTrackingCache : public CacheWrapper {
  public:
   explicit TargetCacheChargeTrackingCache(std::shared_ptr<Cache> target);
 
+  const char* Name() const override { return "TargetCacheChargeTrackingCache"; }
+
   Status Insert(const Slice& key, ObjectPtr value,
                 const CacheItemHelper* helper, size_t charge,
                 Handle** handle = nullptr,

db/version_edit.h

@@ -20,8 +20,8 @@
 #include "db/wal_edit.h"
 #include "memory/arena.h"
 #include "port/malloc.h"
+#include "rocksdb/advanced_cache.h"
 #include "rocksdb/advanced_options.h"
-#include "rocksdb/cache.h"
 #include "table/table_reader.h"
 #include "table/unique_id_impl.h"
 #include "util/autovector.h"

include/rocksdb/secondary_cache.h

@@ -9,7 +9,7 @@
 #include <memory>
 #include <string>
 
-#include "rocksdb/cache.h"
+#include "rocksdb/advanced_cache.h"
 #include "rocksdb/customizable.h"
 #include "rocksdb/slice.h"
 #include "rocksdb/statistics.h"

include/rocksdb/utilities/sim_cache.h

@@ -10,7 +10,7 @@
 #include <memory>
 #include <string>
 
-#include "rocksdb/cache.h"
+#include "rocksdb/advanced_cache.h"
 #include "rocksdb/env.h"
 #include "rocksdb/slice.h"
 #include "rocksdb/statistics.h"
@@ -42,13 +42,10 @@ extern std::shared_ptr<SimCache> NewSimCache(std::shared_ptr<Cache> sim_cache,
                                              std::shared_ptr<Cache> cache,
                                              int num_shard_bits);
 
-class SimCache : public Cache {
+// An abstract base class (public interface) to the SimCache implementation
+class SimCache : public CacheWrapper {
  public:
-  SimCache() {}
-
-  ~SimCache() override {}
-
-  const char* Name() const override { return "SimCache"; }
+  using CacheWrapper::CacheWrapper;
 
   // returns the maximum configured capacity of the simcache for simulation
   virtual size_t GetSimCapacity() const = 0;

java/rocksjni/cache.cc

@@ -6,11 +6,10 @@
 // This file implements the "bridge" between Java and C++ for
 // ROCKSDB_NAMESPACE::Cache.
 
-#include "rocksdb/cache.h"
-
 #include <jni.h>
 
 #include "include/org_rocksdb_Cache.h"
+#include "rocksdb/advanced_cache.h"
 
 /*
  * Class:     org_rocksdb_Cache

java/rocksjni/write_buffer_manager.cc

@@ -7,6 +7,8 @@
 
 #include <jni.h>
 
+#include <cassert>
+
 #include "include/org_rocksdb_WriteBufferManager.h"
 #include "rocksdb/cache.h"
 #include "rocksjni/cplusplus_to_java_convert.h"

memtable/write_buffer_manager_test.cc

@@ -9,6 +9,7 @@
 
 #include "rocksdb/write_buffer_manager.h"
 
+#include "rocksdb/advanced_cache.h"
 #include "test_util/testharness.h"
 
 namespace ROCKSDB_NAMESPACE {

options/cf_options.cc

@@ -16,6 +16,7 @@
 #include "options/options_helper.h"
 #include "options/options_parser.h"
 #include "port/port.h"
+#include "rocksdb/advanced_cache.h"
 #include "rocksdb/compaction_filter.h"
 #include "rocksdb/concurrent_task_limiter.h"
 #include "rocksdb/configurable.h"

options/db_options.cc

@@ -12,6 +12,7 @@
 #include "options/options_helper.h"
 #include "options/options_parser.h"
 #include "port/port.h"
+#include "rocksdb/advanced_cache.h"
 #include "rocksdb/configurable.h"
 #include "rocksdb/env.h"
 #include "rocksdb/file_system.h"

table/block_based/block.h

@@ -16,6 +16,7 @@
 
 #include "db/pinned_iterators_manager.h"
 #include "port/malloc.h"
+#include "rocksdb/advanced_cache.h"
 #include "rocksdb/iterator.h"
 #include "rocksdb/options.h"
 #include "rocksdb/statistics.h"

table/block_based/cachable_entry.h

@@ -13,7 +13,7 @@
 #include <type_traits>
 
 #include "port/likely.h"
-#include "rocksdb/cache.h"
+#include "rocksdb/advanced_cache.h"
 #include "rocksdb/cleanable.h"
 
 namespace ROCKSDB_NAMESPACE {

table/block_based/reader_common.h

@@ -8,7 +8,7 @@
 // found in the LICENSE file. See the AUTHORS file for names of contributors.
 #pragma once
 
-#include "rocksdb/cache.h"
+#include "rocksdb/advanced_cache.h"
 #include "rocksdb/table.h"
 
 namespace ROCKSDB_NAMESPACE {

utilities/leveldb_options/leveldb_options.cc

@@ -9,7 +9,7 @@
 
 #include "rocksdb/utilities/leveldb_options.h"
 
-#include "rocksdb/cache.h"
+#include "rocksdb/advanced_cache.h"
 #include "rocksdb/comparator.h"
 #include "rocksdb/env.h"
 #include "rocksdb/filter_policy.h"

utilities/simulator_cache/sim_cache.cc

@@ -151,20 +151,22 @@ class SimCacheImpl : public SimCache {
   // capacity for real cache (ShardedLRUCache)
   // test_capacity for key only cache
   SimCacheImpl(std::shared_ptr<Cache> sim_cache, std::shared_ptr<Cache> cache)
-      : cache_(cache),
+      : SimCache(cache),
         key_only_cache_(sim_cache),
         miss_times_(0),
         hit_times_(0),
         stats_(nullptr) {}
 
   ~SimCacheImpl() override {}
-  void SetCapacity(size_t capacity) override { cache_->SetCapacity(capacity); }
+
+  const char* Name() const override { return "SimCache"; }
+
+  void SetCapacity(size_t capacity) override { target_->SetCapacity(capacity); }
 
   void SetStrictCapacityLimit(bool strict_capacity_limit) override {
-    cache_->SetStrictCapacityLimit(strict_capacity_limit);
+    target_->SetStrictCapacityLimit(strict_capacity_limit);
   }
 
-  using Cache::Insert;
   Status Insert(const Slice& key, Cache::ObjectPtr value,
                 const CacheItemHelper* helper, size_t charge, Handle** handle,
                 Priority priority) override {
@@ -184,10 +186,10 @@ class SimCacheImpl : public SimCache {
     }
 
     cache_activity_logger_.ReportAdd(key, charge);
-    if (!cache_) {
+    if (!target_) {
       return Status::OK();
     }
-    return cache_->Insert(key, value, helper, charge, handle, priority);
+    return target_->Insert(key, value, helper, charge, handle, priority);
   }
 
   Handle* Lookup(const Slice& key, const CacheItemHelper* helper,
@@ -195,54 +197,54 @@ class SimCacheImpl : public SimCache {
                  Priority priority = Priority::LOW, bool wait = true,
                  Statistics* stats = nullptr) override {
     HandleLookup(key, stats);
-    if (!cache_) {
+    if (!target_) {
       return nullptr;
     }
-    return cache_->Lookup(key, helper, create_context, priority, wait, stats);
+    return target_->Lookup(key, helper, create_context, priority, wait, stats);
   }
 
-  bool Ref(Handle* handle) override { return cache_->Ref(handle); }
+  bool Ref(Handle* handle) override { return target_->Ref(handle); }
 
   using Cache::Release;
   bool Release(Handle* handle, bool erase_if_last_ref = false) override {
-    return cache_->Release(handle, erase_if_last_ref);
+    return target_->Release(handle, erase_if_last_ref);
   }
 
   void Erase(const Slice& key) override {
-    cache_->Erase(key);
+    target_->Erase(key);
     key_only_cache_->Erase(key);
   }
 
   Cache::ObjectPtr Value(Handle* handle) override {
-    return cache_->Value(handle);
+    return target_->Value(handle);
   }
 
-  uint64_t NewId() override { return cache_->NewId(); }
+  uint64_t NewId() override { return target_->NewId(); }
 
-  size_t GetCapacity() const override { return cache_->GetCapacity(); }
+  size_t GetCapacity() const override { return target_->GetCapacity(); }
 
   bool HasStrictCapacityLimit() const override {
-    return cache_->HasStrictCapacityLimit();
+    return target_->HasStrictCapacityLimit();
   }
 
-  size_t GetUsage() const override { return cache_->GetUsage(); }
+  size_t GetUsage() const override { return target_->GetUsage(); }
 
   size_t GetUsage(Handle* handle) const override {
-    return cache_->GetUsage(handle);
+    return target_->GetUsage(handle);
   }
 
   size_t GetCharge(Handle* handle) const override {
-    return cache_->GetCharge(handle);
+    return target_->GetCharge(handle);
   }
 
   const CacheItemHelper* GetCacheItemHelper(Handle* handle) const override {
-    return cache_->GetCacheItemHelper(handle);
+    return target_->GetCacheItemHelper(handle);
   }
 
-  size_t GetPinnedUsage() const override { return cache_->GetPinnedUsage(); }
+  size_t GetPinnedUsage() const override { return target_->GetPinnedUsage(); }
 
   void DisownData() override {
-    cache_->DisownData();
+    target_->DisownData();
     key_only_cache_->DisownData();
   }
 
@@ -250,11 +252,11 @@ class SimCacheImpl : public SimCache {
       const std::function<void(const Slice& key, ObjectPtr value, size_t charge,
                                const CacheItemHelper* helper)>& callback,
       const ApplyToAllEntriesOptions& opts) override {
-    cache_->ApplyToAllEntries(callback, opts);
+    target_->ApplyToAllEntries(callback, opts);
   }
 
   void EraseUnRefEntries() override {
-    cache_->EraseUnRefEntries();
+    target_->EraseUnRefEntries();
     key_only_cache_->EraseUnRefEntries();
   }
 
@@ -295,7 +297,7 @@ class SimCacheImpl : public SimCache {
   std::string GetPrintableOptions() const override {
     std::ostringstream oss;
     oss << "    cache_options:" << std::endl;
-    oss << cache_->GetPrintableOptions();
+    oss << target_->GetPrintableOptions();
     oss << "    sim_cache_options:" << std::endl;
     oss << key_only_cache_->GetPrintableOptions();
     return oss.str();
@@ -314,7 +316,6 @@ class SimCacheImpl : public SimCache {
   }
 
  private:
-  std::shared_ptr<Cache> cache_;
   std::shared_ptr<Cache> key_only_cache_;
   std::atomic<uint64_t> miss_times_;
   std::atomic<uint64_t> hit_times_;