From add68bd28a512da751e2bdc612685fdeb7e6dde4 Mon Sep 17 00:00:00 2001 From: anand76 Date: Mon, 16 Aug 2021 21:00:17 -0700 Subject: [PATCH] Add a stat to count secondary cache hits (#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: https://github.com/facebook/rocksdb/pull/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 --- HISTORY.md | 1 + cache/clock_cache.cc | 3 ++- cache/lru_cache.cc | 8 +++++++- cache/lru_cache.h | 7 ++++--- cache/lru_cache_test.cc | 15 ++++++++++----- cache/sharded_cache.cc | 4 ++-- cache/sharded_cache.h | 3 ++- include/rocksdb/statistics.h | 3 +++ java/rocksjni/portal.h | 4 ++++ java/src/main/java/org/rocksdb/TickerType.java | 5 +++++ monitoring/statistics.cc | 1 + 11 files changed, 41 insertions(+), 13 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 8fe057683..a85ad6335 100644 --- 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 index a3cb7d2c3..3364e2512 100644 --- 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 index db60828c0..710a155c4 100644 --- a/cache/lru_cache.cc +++ b/cache/lru_cache.cc @@ -13,6 +13,7 @@ #include #include +#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 index 8ce0043de..7013c9328 100644 --- 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 index 404530fbe..74409421c 100644 --- a/cache/lru_cache_test.cc +++ b/cache/lru_cache_test.cc @@ -468,6 +468,7 @@ TEST_F(LRUSecondaryCacheTest, BasicTest) { std::make_shared(2048); opts.secondary_cache = secondary_cache; std::shared_ptr cache = NewLRUCache(opts); + std::shared_ptr 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 index bf90ea3b1..d2876ce36 100644 --- 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 index 3e2a20aba..8e6f60d03 100644 --- 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 index f5e83b36d..fa59397b1 100644 --- 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 index c5f34a8a2..5c9be330e 100644 --- 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 index 0d6cc5a92..1381a0a6d 100644 --- 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 index 18d8eb160..7b30c7fcc 100644 --- a/monitoring/statistics.cc +++ b/monitoring/statistics.cc @@ -205,6 +205,7 @@ const std::vector> 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> HistogramsNameMap = {