andreacavalli/rocksdb
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix assertion failure when releasing a handle after secondary cache lookup fails (#8470)

Browse Source 
Summary:
When the secondary cache lookup fails, we may still allocate a handle and charge the cache for metadata usage. If the cache is full, this can cause the usage to go over capacity. Later, when a (unrelated) handle is released, it trips up an assertion that checks that usage is less than capacity. To prevent this assertion failure, don't charge the cache for a failed secondary cache lookup.

Tests:
Run crash_test

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

Reviewed By: zhichao-cao

Differential Revision: D29474713

Pulled By: anand1976

fbshipit-source-id: 27191969c95470a7b070d292b458efce71395bf2
This commit is contained in:
anand76 2021-06-30 13:28:45 -07:00 committed by Facebook GitHub Bot
parent 93a7389442
commit a0cbb69421
1 changed files with 7 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
cache/lru_cache.cc vendored
View File

@ -407,11 +407,10 @@ void LRUCacheShard::Promote(LRUHandle* e) {
} }
} else { } else {
// Since the secondary cache lookup failed, mark the item as not in cache // Since the secondary cache lookup failed, mark the item as not in cache
// and charge the cache only for metadata usage, i.e handle, key etc // Don't charge the cache as its only metadata that'll shortly be released
MutexLock l(&mutex_); MutexLock l(&mutex_);
e->charge = 0; e->charge = 0;
e->SetInCache(false); e->SetInCache(false);
usage_ += e->CalcTotalCharge(metadata_charge_policy_);
} }
} }
@ -522,7 +521,12 @@ bool LRUCacheShard::Release(Cache::Handle* handle, bool force_erase) {
last_reference = false; last_reference = false;
} }
} }
if (last_reference) { // If it was the last reference, and the entry is either not secondary
// cache compatible (i.e a dummy entry for accounting), or is secondary
// cache compatible and has a non-null value, then decrement the cache
// usage. If value is null in the latter case, taht means the lookup
// failed and we didn't charge the cache.
if (last_reference && (!e->IsSecondaryCacheCompatible() || e->value)) {
size_t total_charge = e->CalcTotalCharge(metadata_charge_policy_); size_t total_charge = e->CalcTotalCharge(metadata_charge_policy_);
assert(usage_ >= total_charge); assert(usage_ >= total_charge);
usage_ -= total_charge; usage_ -= total_charge;