Simplify tracking entries already in SecondaryCache (facebook#11299)

Summary:
In preparation for factoring secondary cache support out of individual Cache implementations, we can get rid of the "in secondary cache" flag on entries through a workable hack: when an entry is promoted from secondary, it is inserted in primary using a helper that lacks secondary cache support, thus preventing re-insertion into secondary cache through existing logic.

This adds to the complexity of building CacheItemHelpers, because you always have to be able to get to an equivalent helper without secondary cache support, but that complexity is reasonably isolated within RocksDB typed_cache.h and test code.

gcc-7 seems to have problems with constexpr constructor referencing `this` so removed constexpr support on CacheItemHelper.

Also refactored some related test code to share common code / functionality.

Pull Request resolved: facebook#11299

Test Plan: existing tests

Reviewed By: anand1976

Differential Revision: D44101453

Pulled By: pdillinger

fbshipit-source-id: 7a59d0a3938ee40159c90c3e65d7004f6a272345

CMakeLists.txt

@@ -1279,6 +1279,7 @@ if(WITH_TESTS OR WITH_BENCHMARK_TOOLS)
   add_subdirectory(third-party/gtest-1.8.1/fused-src/gtest)
   add_library(testharness STATIC
   test_util/mock_time_env.cc
+  test_util/secondary_cache_test_util.cc
   test_util/testharness.cc)
   target_link_libraries(testharness gtest)
 endif()

TARGETS

@@ -701,6 +701,7 @@ cpp_library_wrapper(name="rocksdb_test_lib", srcs=[
         "db/db_with_timestamp_test_util.cc",
         "table/mock_table.cc",
         "test_util/mock_time_env.cc",
+        "test_util/secondary_cache_test_util.cc",
         "test_util/testharness.cc",
         "test_util/testutil.cc",
         "tools/block_cache_analyzer/block_cache_trace_analyzer.cc",

cache/cache.cc

@@ -16,6 +16,8 @@
 #include "util/string_util.h"
 
 namespace ROCKSDB_NAMESPACE {
+const Cache::CacheItemHelper kNoopCacheItemHelper{};
+
 static std::unordered_map<std::string, OptionTypeInfo>
     lru_cache_options_type_info = {
         {"capacity",

cache/cache_bench_tool.cc

@@ -255,12 +255,15 @@ void DeleteFn(Cache::ObjectPtr value, MemoryAllocator* /*alloc*/) {
   delete[] static_cast<char*>(value);
 }
 
+Cache::CacheItemHelper helper1_wos(CacheEntryRole::kDataBlock, DeleteFn);
 Cache::CacheItemHelper helper1(CacheEntryRole::kDataBlock, DeleteFn, SizeFn,
-                               SaveToFn, CreateFn);
+                               SaveToFn, CreateFn, &helper1_wos);
+Cache::CacheItemHelper helper2_wos(CacheEntryRole::kIndexBlock, DeleteFn);
 Cache::CacheItemHelper helper2(CacheEntryRole::kIndexBlock, DeleteFn, SizeFn,
-                               SaveToFn, CreateFn);
+                               SaveToFn, CreateFn, &helper2_wos);
+Cache::CacheItemHelper helper3_wos(CacheEntryRole::kFilterBlock, DeleteFn);
 Cache::CacheItemHelper helper3(CacheEntryRole::kFilterBlock, DeleteFn, SizeFn,
-                               SaveToFn, CreateFn);
+                               SaveToFn, CreateFn, &helper3_wos);
 }  // namespace
 
 class CacheBench {

cache/cache_entry_stats.h

@@ -143,7 +143,7 @@ class CacheEntryStatsCollector {
       }
     }
     // If we reach here, shared entry is in cache with handle `h`.
-    assert(cache.get()->GetCacheItemHelper(h) == &cache.kBasicHelper);
+    assert(cache.get()->GetCacheItemHelper(h) == cache.GetBasicHelper());
 
     // Build an aliasing shared_ptr that keeps `ptr` in cache while there
     // are references.

cache/cache_reservation_manager.cc

@@ -169,7 +169,7 @@ Slice CacheReservationManagerImpl<R>::GetNextCacheKey() {
 template <CacheEntryRole R>
 const Cache::CacheItemHelper*
 CacheReservationManagerImpl<R>::TEST_GetCacheItemHelperForRole() {
-  return &CacheInterface::kHelper;
+  return CacheInterface::GetHelper();
 }
 
 template class CacheReservationManagerImpl<

cache/cache_test.cc

@@ -95,8 +95,7 @@ class CacheTest : public testing::TestWithParam<std::string> {
   static void Deleter(Cache::ObjectPtr v, MemoryAllocator*) {
     current_->deleted_values_.push_back(DecodeValue(v));
   }
-  static constexpr Cache::CacheItemHelper kHelper{CacheEntryRole::kMisc,
-                                                  &Deleter};
+  static const Cache::CacheItemHelper kHelper;
 
   static const int kCacheSize = 1000;
   static const int kNumShardBits = 4;
@@ -212,6 +211,9 @@ class CacheTest : public testing::TestWithParam<std::string> {
   void Erase2(int key) { Erase(cache2_, key); }
 };
 
+const Cache::CacheItemHelper CacheTest::kHelper{CacheEntryRole::kMisc,
+                                                &CacheTest::Deleter};
+
 CacheTest* CacheTest::current_;
 std::string CacheTest::type_;