Add a stat to count secondary cache hits (facebook#8666)

Summary: Add a stat for secondary cache hits. The ```Cache::Lookup``` API had an unused ```stats``` parameter. This PR uses that to pass the pointer to a ```Statistics``` object that ```LRUCache``` uses to record the stat. Pull Request resolved: facebook#8666 Test Plan: Update a unit test in lru_cache_test Reviewed By: zhichao-cao Differential Revision: D30353816 Pulled By: anand1976 fbshipit-source-id: 2046f78b460428877a26ffdd2bb914ae47dfbe77
sandeeptiwari · Aug 17, 2021 · add68bd · add68bd
1 parent a207c27
commit add68bd
Show file tree

Hide file tree

Showing 11 changed files with 41 additions and 13 deletions.
diff --git a/HISTORY.md b/HISTORY.md
@@ -17,6 +17,7 @@
 * Add a comment to suggest btrfs user to disable file preallocation by setting `options.allow_fallocate=false`.
 * Fast forward option in Trace replay changed to double type to allow replaying at a lower speed, by settings the value between 0 and 1. This option can be set via `ReplayOptions` in `Replayer::Replay()`, or via `--trace_replay_fast_forward` in db_bench.
 * Add property `LiveSstFilesSizeAtTemperature` to retrieve sst file size at different temperature.
+* Added a stat rocksdb.secondary.cache.hits
 
 ## Public API change
 * Added APIs to decode and replay trace file via Replayer class. Added `DB::NewDefaultReplayer()` to create a default Replayer instance. Added `TraceReader::Reset()` to restart reading a trace file. Created trace_record.h and utilities/replayer.h files to access decoded Trace records and replay them.

diff --git a/cache/clock_cache.cc b/cache/clock_cache.cc
@@ -280,7 +280,8 @@ class ClockCacheShard final : public CacheShard {
   Cache::Handle* Lookup(const Slice& key, uint32_t hash,
                         const Cache::CacheItemHelper* /*helper*/,
                         const Cache::CreateCallback& /*create_cb*/,
-                        Cache::Priority /*priority*/, bool /*wait*/) override {
+                        Cache::Priority /*priority*/, bool /*wait*/,
+                        Statistics* /*stats*/) override {
     return Lookup(key, hash);
   }
   bool Release(Cache::Handle* handle, bool /*useful*/,

diff --git a/cache/lru_cache.cc b/cache/lru_cache.cc
@@ -13,6 +13,7 @@
 #include <cstdint>
 #include <cstdio>
 
+#include "monitoring/statistics.h"
 #include "util/mutexlock.h"
 
 namespace ROCKSDB_NAMESPACE {
@@ -418,7 +419,7 @@ Cache::Handle* LRUCacheShard::Lookup(
     const Slice& key, uint32_t hash,
     const ShardedCache::CacheItemHelper* helper,
     const ShardedCache::CreateCallback& create_cb, Cache::Priority priority,
-    bool wait) {
+    bool wait, Statistics* stats) {
   LRUHandle* e = nullptr;
   {
     MutexLock l(&mutex_);
@@ -471,11 +472,16 @@ Cache::Handle* LRUCacheShard::Lookup(
           e->Unref();
           e->Free();
           e = nullptr;
+        } else {
+          RecordTick(stats, SECONDARY_CACHE_HITS);
         }
       } else {
         // If wait is false, we always return a handle and let the caller
         // release the handle after checking for success or failure
         e->SetIncomplete(true);
+        // This may be slightly inaccurate, if the lookup eventually fails.
+        // But the probability is very low.
+        RecordTick(stats, SECONDARY_CACHE_HITS);
       }
     }
   }

diff --git a/cache/lru_cache.h b/cache/lru_cache.h
@@ -319,10 +319,11 @@ class ALIGN_AS(CACHE_LINE_SIZE) LRUCacheShard final : public CacheShard {
   virtual Cache::Handle* Lookup(const Slice& key, uint32_t hash,
                                 const ShardedCache::CacheItemHelper* helper,
                                 const ShardedCache::CreateCallback& create_cb,
-                                ShardedCache::Priority priority,
-                                bool wait) override;
+                                ShardedCache::Priority priority, bool wait,
+                                Statistics* stats) override;
   virtual Cache::Handle* Lookup(const Slice& key, uint32_t hash) override {
-    return Lookup(key, hash, nullptr, nullptr, Cache::Priority::LOW, true);
+    return Lookup(key, hash, nullptr, nullptr, Cache::Priority::LOW, true,
+                  nullptr);
   }
   virtual bool Release(Cache::Handle* handle, bool /*useful*/,
                        bool force_erase) override {

diff --git a/cache/lru_cache_test.cc b/cache/lru_cache_test.cc
@@ -468,6 +468,7 @@ TEST_F(LRUSecondaryCacheTest, BasicTest) {
       std::make_shared<TestSecondaryCache>(2048);
   opts.secondary_cache = secondary_cache;
   std::shared_ptr<Cache> cache = NewLRUCache(opts);
+  std::shared_ptr<Statistics> stats = CreateDBStatistics();
 
   Random rnd(301);
   std::string str1 = rnd.RandomString(1020);
@@ -476,22 +477,26 @@ TEST_F(LRUSecondaryCacheTest, BasicTest) {
                           str1.length()));
   std::string str2 = rnd.RandomString(1020);
   TestItem* item2 = new TestItem(str2.data(), str2.length());
-  // k2 should be demoted to NVM
+  // k1 should be demoted to NVM
   ASSERT_OK(cache->Insert("k2", item2, &LRUSecondaryCacheTest::helper_,
                           str2.length()));
 
   Cache::Handle* handle;
-  handle = cache->Lookup("k2", &LRUSecondaryCacheTest::helper_,
-                         test_item_creator, Cache::Priority::LOW, true);
+  handle =
+      cache->Lookup("k2", &LRUSecondaryCacheTest::helper_, test_item_creator,
+                    Cache::Priority::LOW, true, stats.get());
   ASSERT_NE(handle, nullptr);
   cache->Release(handle);
   // This lookup should promote k1 and demote k2
-  handle = cache->Lookup("k1", &LRUSecondaryCacheTest::helper_,
-                         test_item_creator, Cache::Priority::LOW, true);
+  handle =
+      cache->Lookup("k1", &LRUSecondaryCacheTest::helper_, test_item_creator,
+                    Cache::Priority::LOW, true, stats.get());
   ASSERT_NE(handle, nullptr);
   cache->Release(handle);
   ASSERT_EQ(secondary_cache->num_inserts(), 2u);
   ASSERT_EQ(secondary_cache->num_lookups(), 1u);
+  ASSERT_EQ(stats->getTickerCount(SECONDARY_CACHE_HITS),
+            secondary_cache->num_lookups());
 
   cache.reset();
   secondary_cache.reset();

diff --git a/cache/sharded_cache.cc b/cache/sharded_cache.cc
@@ -83,10 +83,10 @@ Cache::Handle* ShardedCache::Lookup(const Slice& key,
                                     const CacheItemHelper* helper,
                                     const CreateCallback& create_cb,
                                     Priority priority, bool wait,
-                                    Statistics* /*stats*/) {
+                                    Statistics* stats) {
   uint32_t hash = HashSlice(key);
   return GetShard(Shard(hash))
-      ->Lookup(key, hash, helper, create_cb, priority, wait);
+      ->Lookup(key, hash, helper, create_cb, priority, wait, stats);
 }
 
 bool ShardedCache::IsReady(Handle* handle) {

diff --git a/cache/sharded_cache.h b/cache/sharded_cache.h
@@ -34,7 +34,8 @@ class CacheShard {
   virtual Cache::Handle* Lookup(const Slice& key, uint32_t hash,
                                 const Cache::CacheItemHelper* helper,
                                 const Cache::CreateCallback& create_cb,
-                                Cache::Priority priority, bool wait) = 0;
+                                Cache::Priority priority, bool wait,
+                                Statistics* stats) = 0;
   virtual bool Release(Cache::Handle* handle, bool useful,
                        bool force_erase) = 0;
   virtual bool IsReady(Cache::Handle* handle) = 0;

diff --git a/include/rocksdb/statistics.h b/include/rocksdb/statistics.h
@@ -389,6 +389,9 @@ enum Tickers : uint32_t {
   // Outdated bytes of data present on memtable at flush time.
   MEMTABLE_GARBAGE_BYTES_AT_FLUSH,
 
+  // Secondary cache statistics
+  SECONDARY_CACHE_HITS,
+
   TICKER_ENUM_MAX
 };
 

diff --git a/java/rocksjni/portal.h b/java/rocksjni/portal.h
@@ -5000,6 +5000,8 @@ class TickerTypeJni {
         return -0x1C;
       case ROCKSDB_NAMESPACE::Tickers::MEMTABLE_GARBAGE_BYTES_AT_FLUSH:
         return -0x1D;
+      case ROCKSDB_NAMESPACE::Tickers::SECONDARY_CACHE_HITS:
+        return -0x1E;
       case ROCKSDB_NAMESPACE::Tickers::TICKER_ENUM_MAX:
         // 0x5F for backwards compatibility on current minor version.
         return 0x5F;
@@ -5330,6 +5332,8 @@ class TickerTypeJni {
         return ROCKSDB_NAMESPACE::Tickers::MEMTABLE_PAYLOAD_BYTES_AT_FLUSH;
       case -0x1D:
         return ROCKSDB_NAMESPACE::Tickers::MEMTABLE_GARBAGE_BYTES_AT_FLUSH;
+      case -0x1E:
+        return ROCKSDB_NAMESPACE::Tickers::SECONDARY_CACHE_HITS;
       case 0x5F:
         // 0x5F for backwards compatibility on current minor version.
         return ROCKSDB_NAMESPACE::Tickers::TICKER_ENUM_MAX;

diff --git a/java/src/main/java/org/rocksdb/TickerType.java b/java/src/main/java/org/rocksdb/TickerType.java
@@ -764,6 +764,11 @@ public enum TickerType {
      */
     MEMTABLE_GARBAGE_BYTES_AT_FLUSH((byte) -0x1D),
 
+    /**
+     * Number of secondary cache hits
+     */
+    SECONDARY_CACHE_HITS((byte) -0x1E),
+
     TICKER_ENUM_MAX((byte) 0x5F);
 
     private final byte value;

diff --git a/monitoring/statistics.cc b/monitoring/statistics.cc
@@ -205,6 +205,7 @@ const std::vector<std::pair<Tickers, std::string>> TickersNameMap = {
      "rocksdb.memtable.payload.bytes.at.flush"},
     {MEMTABLE_GARBAGE_BYTES_AT_FLUSH,
      "rocksdb.memtable.garbage.bytes.at.flush"},
+    {SECONDARY_CACHE_HITS, "rocksdb.secondary.cache.hits"},
 };
 
 const std::vector<std::pair<Histograms, std::string>> HistogramsNameMap = {