diff --git a/HISTORY.md b/HISTORY.md index f90b460e1..1840dc7ac 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -17,6 +17,7 @@ * `rocksdb_file_metadata_t` and its and get functions & destroy functions. * Add suggest_compact_range() and suggest_compact_range_cf() to C API. * When using block cache strict capacity limit (`LRUCache` with `strict_capacity_limit=true`), DB operations now fail with Status code `kAborted` subcode `kMemoryLimit` (`IsMemoryLimit()`) instead of `kIncomplete` (`IsIncomplete()`) when the capacity limit is reached, because Incomplete can mean other specific things for some operations. In more detail, `Cache::Insert()` now returns the updated Status code and this usually propagates through RocksDB to the user on failure. +* NewClockCache calls temporarily return an LRUCache (with similar characteristics as the desired ClockCache). This is because ClockCache is being replaced by a new version (the old one had unknown bugs) but this is still under development. * Add two functions `int ReserveThreads(int threads_to_be_reserved)` and `int ReleaseThreads(threads_to_be_released)` into `Env` class. In the default implementation, both return 0. Newly added `xxxEnv` class that inherits `Env` should implement these two functions for thread reservation/releasing features. ### Bug Fixes diff --git a/cache/cache_bench_tool.cc b/cache/cache_bench_tool.cc index f2b32daba..61818cb34 100644 --- a/cache/cache_bench_tool.cc +++ b/cache/cache_bench_tool.cc @@ -13,6 +13,7 @@ #include #include +#include "cache/clock_cache.h" #include "cache/fast_lru_cache.h" #include "db/db_impl/db_impl.h" #include "monitoring/histogram.h" @@ -284,7 +285,7 @@ class CacheBench { } if (FLAGS_cache_type == "clock_cache") { - cache_ = NewClockCache( + cache_ = ExperimentalNewClockCache( FLAGS_cache_size, FLAGS_value_bytes, FLAGS_num_shard_bits, false /*strict_capacity_limit*/, kDefaultCacheMetadataChargePolicy); if (!cache_) { diff --git a/cache/cache_test.cc b/cache/cache_test.cc index f9acb88f0..0a008a1d9 100644 --- a/cache/cache_test.cc +++ b/cache/cache_test.cc @@ -111,7 +111,7 @@ class CacheTest : public testing::TestWithParam { return NewLRUCache(capacity); } if (type == kClock) { - return NewClockCache( + return ExperimentalNewClockCache( capacity, 1 /*estimated_value_size*/, -1 /*num_shard_bits*/, false /*strict_capacity_limit*/, kDefaultCacheMetadataChargePolicy); } @@ -137,8 +137,9 @@ class CacheTest : public testing::TestWithParam { return NewLRUCache(co); } if (type == kClock) { - return NewClockCache(capacity, 1 /*estimated_value_size*/, num_shard_bits, - strict_capacity_limit, charge_policy); + return ExperimentalNewClockCache(capacity, 1 /*estimated_value_size*/, + num_shard_bits, strict_capacity_limit, + charge_policy); } if (type == kFast) { return NewFastLRUCache(capacity, 1 /*estimated_value_size*/, @@ -954,8 +955,9 @@ TEST_P(CacheTest, GetChargeAndDeleter) { cache_->Release(h1); } -std::shared_ptr (*new_clock_cache_func)( - size_t, size_t, int, bool, CacheMetadataChargePolicy) = NewClockCache; +std::shared_ptr (*new_clock_cache_func)(size_t, size_t, int, bool, + CacheMetadataChargePolicy) = + ExperimentalNewClockCache; INSTANTIATE_TEST_CASE_P(CacheTestInstance, CacheTest, testing::Values(kLRU, kClock, kFast)); INSTANTIATE_TEST_CASE_P(CacheTestInstance, LRUCacheTest, diff --git a/cache/clock_cache.cc b/cache/clock_cache.cc index e9dd77f9d..86b4f0471 100644 --- a/cache/clock_cache.cc +++ b/cache/clock_cache.cc @@ -584,6 +584,14 @@ void ClockCache::DisownData() { } // namespace clock_cache std::shared_ptr NewClockCache( + size_t capacity, size_t /*estimated_value_size*/, int num_shard_bits, + bool strict_capacity_limit, + CacheMetadataChargePolicy metadata_charge_policy) { + return NewLRUCache(capacity, num_shard_bits, strict_capacity_limit, 0.5, + nullptr, kDefaultToAdaptiveMutex, metadata_charge_policy); +} + +std::shared_ptr ExperimentalNewClockCache( size_t capacity, size_t estimated_value_size, int num_shard_bits, bool strict_capacity_limit, CacheMetadataChargePolicy metadata_charge_policy) { diff --git a/cache/clock_cache.h b/cache/clock_cache.h index dadaa88f3..ca6205b83 100644 --- a/cache/clock_cache.h +++ b/cache/clock_cache.h @@ -472,4 +472,11 @@ class ClockCache } // namespace clock_cache +// Only for internal testing, temporarily replacing NewClockCache. +// TODO(Guido) Remove once NewClockCache constructs a ClockCache again. +extern std::shared_ptr ExperimentalNewClockCache( + size_t capacity, size_t estimated_value_size, int num_shard_bits, + bool strict_capacity_limit, + CacheMetadataChargePolicy metadata_charge_policy); + } // namespace ROCKSDB_NAMESPACE diff --git a/db/db_block_cache_test.cc b/db/db_block_cache_test.cc index d4ee1d8b2..d2cf32316 100644 --- a/db/db_block_cache_test.cc +++ b/db/db_block_cache_test.cc @@ -13,6 +13,7 @@ #include "cache/cache_entry_roles.h" #include "cache/cache_key.h" +#include "cache/clock_cache.h" #include "cache/fast_lru_cache.h" #include "cache/lru_cache.h" #include "db/column_family.h" @@ -935,9 +936,9 @@ TEST_F(DBBlockCacheTest, AddRedundantStats) { int iterations_tested = 0; for (std::shared_ptr base_cache : {NewLRUCache(capacity, num_shard_bits), - NewClockCache(capacity, 1 /*estimated_value_size*/, num_shard_bits, - false /*strict_capacity_limit*/, - kDefaultCacheMetadataChargePolicy), + ExperimentalNewClockCache( + capacity, 1 /*estimated_value_size*/, num_shard_bits, + false /*strict_capacity_limit*/, kDefaultCacheMetadataChargePolicy), NewFastLRUCache(capacity, 1 /*estimated_value_size*/, num_shard_bits, false /*strict_capacity_limit*/, kDefaultCacheMetadataChargePolicy)}) { diff --git a/include/rocksdb/cache.h b/include/rocksdb/cache.h index 6194b0c66..49f5b80bc 100644 --- a/include/rocksdb/cache.h +++ b/include/rocksdb/cache.h @@ -174,11 +174,18 @@ extern std::shared_ptr NewCompressedSecondaryCache( extern std::shared_ptr NewCompressedSecondaryCache( const CompressedSecondaryCacheOptions& opts); +// EXPERIMENTAL Currently ClockCache is under development, although it's +// already exposed in the public API. To avoid unreliable performance and +// correctness issues, NewClockCache will temporarily return an LRUCache +// constructed with the corresponding arguments. +// +// TODO(Guido) When ClockCache is complete, roll back to the old text: +// `` // Similar to NewLRUCache, but create a cache based on clock algorithm with // better concurrent performance in some cases. See util/clock_cache.cc for // more detail. -// // Return nullptr if it is not supported. +// `` extern std::shared_ptr NewClockCache( size_t capacity, size_t estimated_value_size, int num_shard_bits, bool strict_capacity_limit, diff --git a/tools/db_bench_tool.cc b/tools/db_bench_tool.cc index 6fe45bb40..61aa184d3 100644 --- a/tools/db_bench_tool.cc +++ b/tools/db_bench_tool.cc @@ -37,6 +37,7 @@ #include #include +#include "cache/clock_cache.h" #include "cache/fast_lru_cache.h" #include "db/db_impl/db_impl.h" #include "db/malloc_stats.h" @@ -2977,10 +2978,10 @@ class Benchmark { return nullptr; } if (FLAGS_cache_type == "clock_cache") { - auto cache = NewClockCache(static_cast(capacity), - FLAGS_block_size, FLAGS_cache_numshardbits, - false /*strict_capacity_limit*/, - kDefaultCacheMetadataChargePolicy); + auto cache = ExperimentalNewClockCache( + static_cast(capacity), FLAGS_block_size, + FLAGS_cache_numshardbits, false /*strict_capacity_limit*/, + kDefaultCacheMetadataChargePolicy); if (!cache) { fprintf(stderr, "Clock cache not supported."); exit(1);