Update Cache::Release param from force_erase to erase_if_last_ref (#9728)

Summary:
The param name force_erase may be misleading, since the handle is erased only if it has last reference even if the param is set true.

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

Reviewed By: pdillinger

Differential Revision: D35038673

Pulled By: gitbw95

fbshipit-source-id: 0d16d1e8fed17b97eba7fb53207119332f659a5f
main
gitbw95 2 years ago committed by Facebook GitHub Bot
parent b360d25deb
commit 8102690a52
  1. 10
      cache/clock_cache.cc
  2. 6
      cache/lru_cache.cc
  3. 6
      cache/lru_cache.h
  4. 9
      cache/sharded_cache.cc
  5. 8
      cache/sharded_cache.h
  6. 4
      db/db_bloom_filter_test.cc
  7. 4
      db/db_test_util.h
  8. 14
      include/rocksdb/cache.h
  9. 2
      table/block_based/block_based_table_reader.cc
  10. 2
      table/block_based/reader_common.cc
  11. 4
      utilities/simulator_cache/sim_cache.cc

@ -286,8 +286,8 @@ class ClockCacheShard final : public CacheShard {
return Lookup(key, hash); return Lookup(key, hash);
} }
bool Release(Cache::Handle* handle, bool /*useful*/, bool Release(Cache::Handle* handle, bool /*useful*/,
bool force_erase) override { bool erase_if_last_ref) override {
return Release(handle, force_erase); return Release(handle, erase_if_last_ref);
} }
bool IsReady(Cache::Handle* /*handle*/) override { return true; } bool IsReady(Cache::Handle* /*handle*/) override { return true; }
void Wait(Cache::Handle* /*handle*/) override {} void Wait(Cache::Handle* /*handle*/) override {}
@ -297,7 +297,7 @@ class ClockCacheShard final : public CacheShard {
// //
// Not necessary to hold mutex_ before being called. // Not necessary to hold mutex_ before being called.
bool Ref(Cache::Handle* handle) override; bool Ref(Cache::Handle* handle) override;
bool Release(Cache::Handle* handle, bool force_erase = false) override; bool Release(Cache::Handle* handle, bool erase_if_last_ref = false) override;
void Erase(const Slice& key, uint32_t hash) override; void Erase(const Slice& key, uint32_t hash) override;
bool EraseAndConfirm(const Slice& key, uint32_t hash, bool EraseAndConfirm(const Slice& key, uint32_t hash,
CleanupContext* context); CleanupContext* context);
@ -725,11 +725,11 @@ Cache::Handle* ClockCacheShard::Lookup(const Slice& key, uint32_t hash) {
return reinterpret_cast<Cache::Handle*>(handle); return reinterpret_cast<Cache::Handle*>(handle);
} }
bool ClockCacheShard::Release(Cache::Handle* h, bool force_erase) { bool ClockCacheShard::Release(Cache::Handle* h, bool erase_if_last_ref) {
CleanupContext context; CleanupContext context;
CacheHandle* handle = reinterpret_cast<CacheHandle*>(h); CacheHandle* handle = reinterpret_cast<CacheHandle*>(h);
bool erased = Unref(handle, true, &context); bool erased = Unref(handle, true, &context);
if (force_erase && !erased) { if (erase_if_last_ref && !erased) {
erased = EraseAndConfirm(handle->key, handle->hash, &context); erased = EraseAndConfirm(handle->key, handle->hash, &context);
} }
Cleanup(context); Cleanup(context);

@ -507,7 +507,7 @@ void LRUCacheShard::SetHighPriorityPoolRatio(double high_pri_pool_ratio) {
MaintainPoolSize(); MaintainPoolSize();
} }
bool LRUCacheShard::Release(Cache::Handle* handle, bool force_erase) { bool LRUCacheShard::Release(Cache::Handle* handle, bool erase_if_last_ref) {
if (handle == nullptr) { if (handle == nullptr) {
return false; return false;
} }
@ -518,9 +518,9 @@ bool LRUCacheShard::Release(Cache::Handle* handle, bool force_erase) {
last_reference = e->Unref(); last_reference = e->Unref();
if (last_reference && e->InCache()) { if (last_reference && e->InCache()) {
// The item is still in cache, and nobody else holds a reference to it // The item is still in cache, and nobody else holds a reference to it
if (usage_ > capacity_ || force_erase) { if (usage_ > capacity_ || erase_if_last_ref) {
// The LRU list must be empty since the cache is full // The LRU list must be empty since the cache is full
assert(lru_.next == &lru_ || force_erase); assert(lru_.next == &lru_ || erase_if_last_ref);
// Take this opportunity and remove the item // Take this opportunity and remove the item
table_.Remove(e->key(), e->hash); table_.Remove(e->key(), e->hash);
e->SetInCache(false); e->SetInCache(false);

6
cache/lru_cache.h vendored

@ -326,14 +326,14 @@ class ALIGN_AS(CACHE_LINE_SIZE) LRUCacheShard final : public CacheShard {
nullptr); nullptr);
} }
virtual bool Release(Cache::Handle* handle, bool /*useful*/, virtual bool Release(Cache::Handle* handle, bool /*useful*/,
bool force_erase) override { bool erase_if_last_ref) override {
return Release(handle, force_erase); return Release(handle, erase_if_last_ref);
} }
virtual bool IsReady(Cache::Handle* /*handle*/) override; virtual bool IsReady(Cache::Handle* /*handle*/) override;
virtual void Wait(Cache::Handle* /*handle*/) override {} virtual void Wait(Cache::Handle* /*handle*/) override {}
virtual bool Ref(Cache::Handle* handle) override; virtual bool Ref(Cache::Handle* handle) override;
virtual bool Release(Cache::Handle* handle, virtual bool Release(Cache::Handle* handle,
bool force_erase = false) override; bool erase_if_last_ref = false) override;
virtual void Erase(const Slice& key, uint32_t hash) override; virtual void Erase(const Slice& key, uint32_t hash) override;
// Although in some platforms the update of size_t is atomic, to make sure // Although in some platforms the update of size_t is atomic, to make sure

@ -104,14 +104,15 @@ bool ShardedCache::Ref(Handle* handle) {
return GetShard(Shard(hash))->Ref(handle); return GetShard(Shard(hash))->Ref(handle);
} }
bool ShardedCache::Release(Handle* handle, bool force_erase) { bool ShardedCache::Release(Handle* handle, bool erase_if_last_ref) {
uint32_t hash = GetHash(handle); uint32_t hash = GetHash(handle);
return GetShard(Shard(hash))->Release(handle, force_erase); return GetShard(Shard(hash))->Release(handle, erase_if_last_ref);
} }
bool ShardedCache::Release(Handle* handle, bool useful, bool force_erase) { bool ShardedCache::Release(Handle* handle, bool useful,
bool erase_if_last_ref) {
uint32_t hash = GetHash(handle); uint32_t hash = GetHash(handle);
return GetShard(Shard(hash))->Release(handle, useful, force_erase); return GetShard(Shard(hash))->Release(handle, useful, erase_if_last_ref);
} }
void ShardedCache::Erase(const Slice& key) { void ShardedCache::Erase(const Slice& key) {

@ -37,11 +37,11 @@ class CacheShard {
Cache::Priority priority, bool wait, Cache::Priority priority, bool wait,
Statistics* stats) = 0; Statistics* stats) = 0;
virtual bool Release(Cache::Handle* handle, bool useful, virtual bool Release(Cache::Handle* handle, bool useful,
bool force_erase) = 0; bool erase_if_last_ref) = 0;
virtual bool IsReady(Cache::Handle* handle) = 0; virtual bool IsReady(Cache::Handle* handle) = 0;
virtual void Wait(Cache::Handle* handle) = 0; virtual void Wait(Cache::Handle* handle) = 0;
virtual bool Ref(Cache::Handle* handle) = 0; virtual bool Ref(Cache::Handle* handle) = 0;
virtual bool Release(Cache::Handle* handle, bool force_erase) = 0; virtual bool Release(Cache::Handle* handle, bool erase_if_last_ref) = 0;
virtual void Erase(const Slice& key, uint32_t hash) = 0; virtual void Erase(const Slice& key, uint32_t hash) = 0;
virtual void SetCapacity(size_t capacity) = 0; virtual void SetCapacity(size_t capacity) = 0;
virtual void SetStrictCapacityLimit(bool strict_capacity_limit) = 0; virtual void SetStrictCapacityLimit(bool strict_capacity_limit) = 0;
@ -94,11 +94,11 @@ class ShardedCache : public Cache {
const CreateCallback& create_cb, Priority priority, const CreateCallback& create_cb, Priority priority,
bool wait, Statistics* stats = nullptr) override; bool wait, Statistics* stats = nullptr) override;
virtual bool Release(Handle* handle, bool useful, virtual bool Release(Handle* handle, bool useful,
bool force_erase = false) override; bool erase_if_last_ref = false) override;
virtual bool IsReady(Handle* handle) override; virtual bool IsReady(Handle* handle) override;
virtual void Wait(Handle* handle) override; virtual void Wait(Handle* handle) override;
virtual bool Ref(Handle* handle) override; virtual bool Ref(Handle* handle) override;
virtual bool Release(Handle* handle, bool force_erase = false) override; virtual bool Release(Handle* handle, bool erase_if_last_ref = false) override;
virtual void Erase(const Slice& key) override; virtual void Erase(const Slice& key) override;
virtual uint64_t NewId() override; virtual uint64_t NewId() override;
virtual size_t GetCapacity() const override; virtual size_t GetCapacity() const override;

@ -919,7 +919,7 @@ class FilterConstructResPeakTrackingCache : public CacheWrapper {
} }
using Cache::Release; using Cache::Release;
bool Release(Handle* handle, bool force_erase = false) override { bool Release(Handle* handle, bool erase_if_last_ref = false) override {
auto deleter = GetDeleter(handle); auto deleter = GetDeleter(handle);
if (deleter == kNoopDeleterForFilterConstruction) { if (deleter == kNoopDeleterForFilterConstruction) {
if (!last_peak_tracked_) { if (!last_peak_tracked_) {
@ -929,7 +929,7 @@ class FilterConstructResPeakTrackingCache : public CacheWrapper {
} }
cur_cache_res_ -= GetCharge(handle); cur_cache_res_ -= GetCharge(handle);
} }
bool is_successful = target_->Release(handle, force_erase); bool is_successful = target_->Release(handle, erase_if_last_ref);
return is_successful; return is_successful;
} }

@ -894,8 +894,8 @@ class CacheWrapper : public Cache {
bool Ref(Handle* handle) override { return target_->Ref(handle); } bool Ref(Handle* handle) override { return target_->Ref(handle); }
using Cache::Release; using Cache::Release;
bool Release(Handle* handle, bool force_erase = false) override { bool Release(Handle* handle, bool erase_if_last_ref = false) override {
return target_->Release(handle, force_erase); return target_->Release(handle, erase_if_last_ref);
} }
void* Value(Handle* handle) override { return target_->Value(handle); } void* Value(Handle* handle) override { return target_->Value(handle); }

@ -329,16 +329,15 @@ class Cache {
/** /**
* Release a mapping returned by a previous Lookup(). A released entry might * Release a mapping returned by a previous Lookup(). A released entry might
* still remain in cache in case it is later looked up by others. If * still remain in cache in case it is later looked up by others. If
* force_erase is set then it also erase it from the cache if there is no * erase_if_last_ref is set then it also erase it from the cache if there is
* other reference to it. Erasing it should call the deleter function that * no other reference to it. Erasing it should call the deleter function that
* was provided when the * was provided when the entry was inserted.
* entry was inserted.
* *
* Returns true if the entry was also erased. * Returns true if the entry was also erased.
*/ */
// REQUIRES: handle must not have been released yet. // REQUIRES: handle must not have been released yet.
// REQUIRES: handle must have been returned by a method on *this. // REQUIRES: handle must have been returned by a method on *this.
virtual bool Release(Handle* handle, bool force_erase = false) = 0; virtual bool Release(Handle* handle, bool erase_if_last_ref = false) = 0;
// Return the value encapsulated in a handle returned by a // Return the value encapsulated in a handle returned by a
// successful Lookup(). // successful Lookup().
@ -517,8 +516,9 @@ class Cache {
// parameter specifies whether the data was actually used or not, // parameter specifies whether the data was actually used or not,
// which may be used by the cache implementation to decide whether // which may be used by the cache implementation to decide whether
// to consider it as a hit for retention purposes. // to consider it as a hit for retention purposes.
virtual bool Release(Handle* handle, bool /*useful*/, bool force_erase) { virtual bool Release(Handle* handle, bool /*useful*/,
return Release(handle, force_erase); bool erase_if_last_ref) {
return Release(handle, erase_if_last_ref);
} }
// Determines if the handle returned by Lookup() has a valid value yet. The // Determines if the handle returned by Lookup() has a valid value yet. The

@ -119,7 +119,7 @@ Status ReadBlockFromFile(
void ReleaseCachedEntry(void* arg, void* h) { void ReleaseCachedEntry(void* arg, void* h) {
Cache* cache = reinterpret_cast<Cache*>(arg); Cache* cache = reinterpret_cast<Cache*>(arg);
Cache::Handle* handle = reinterpret_cast<Cache::Handle*>(h); Cache::Handle* handle = reinterpret_cast<Cache::Handle*>(h);
cache->Release(handle, false /* force_erase */); cache->Release(handle, false /* erase_if_last_ref */);
} }
// For hash based index, return false if table_properties->prefix_extractor_name // For hash based index, return false if table_properties->prefix_extractor_name

@ -19,7 +19,7 @@ namespace ROCKSDB_NAMESPACE {
void ForceReleaseCachedEntry(void* arg, void* h) { void ForceReleaseCachedEntry(void* arg, void* h) {
Cache* cache = reinterpret_cast<Cache*>(arg); Cache* cache = reinterpret_cast<Cache*>(arg);
Cache::Handle* handle = reinterpret_cast<Cache::Handle*>(h); Cache::Handle* handle = reinterpret_cast<Cache::Handle*>(h);
cache->Release(handle, true /* force_erase */); cache->Release(handle, true /* erase_if_last_ref */);
} }
// WART: this is specific to block-based table // WART: this is specific to block-based table

@ -213,8 +213,8 @@ class SimCacheImpl : public SimCache {
bool Ref(Handle* handle) override { return cache_->Ref(handle); } bool Ref(Handle* handle) override { return cache_->Ref(handle); }
using Cache::Release; using Cache::Release;
bool Release(Handle* handle, bool force_erase = false) override { bool Release(Handle* handle, bool erase_if_last_ref = false) override {
return cache_->Release(handle, force_erase); return cache_->Release(handle, erase_if_last_ref);
} }
void Erase(const Slice& key) override { void Erase(const Slice& key) override {

Loading…
Cancel
Save