Release cache reservation of hash entries of the fall-back Ribbon Filter earlier (#9345)

Summary:
Note: rebase on and merge after https://github.com/facebook/rocksdb/pull/9349, as part of https://github.com/facebook/rocksdb/pull/9342
**Context:**
https://github.com/facebook/rocksdb/pull/9073 charged the hash entries' memory in block cache with `CacheReservationHandle`. However, in the edge case where Ribbon Filter falls back to Bloom Filter and swaps its hash entries to the embedded bloom filter object, the handles associated with those entries are not swapped and thus not released as soon as those entries are cleared during Bloom Filter's finish process.

Although this is a minor issue since RocksDB internal calls `FilterBitsBuilder->Reset()` right after `FilterBitsBuilder->Finish()` on the main path, which releases all the cache reservation related to both the Ribbon Filter and its embedded Bloom Filter, it still worths this fix to avoid confusion.

**Summary:**
- Swapped the `CacheReservationHandle` associated with the hash entries on Ribbon Filter's fallback

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

Test Plan: - Added a unit test to verify the number of cache reservation after clearing hash entries, which failed before the change and now succeeds

Reviewed By: pdillinger

Differential Revision: D33377225

Pulled By: hx235

fbshipit-source-id: 7487f4c40dfb6ee7928232021f93ef2c5329cffa
main
Hui Xiao 3 years ago committed by Facebook GitHub Bot
parent f62efb9d35
commit 9110685e8c
  1. 4
      table/block_based/filter_policy.cc
  2. 12
      util/bloom_test.cc

@ -97,6 +97,10 @@ class XXPH3FilterBitsBuilder : public BuiltinFilterBitsBuilder {
// For delegating between XXPH3FilterBitsBuilders
void SwapEntriesWith(XXPH3FilterBitsBuilder* other) {
std::swap(hash_entries_, other->hash_entries_);
if (cache_res_mgr_) {
std::swap(hash_entry_cache_res_bucket_handles_,
other->hash_entry_cache_res_bucket_handles_);
}
}
virtual size_t RoundDownUsableSpace(size_t available_size) = 0;

@ -644,6 +644,18 @@ TEST(FullBloomFilterConstructionReserveMemTest,
} else {
EXPECT_EQ(filter.data()[filter.size() - 5], static_cast<char>(-2));
}
if (reserve_builder_mem) {
const size_t dummy_entry_num = static_cast<std::size_t>(std::ceil(
filter.size() * 1.0 / CacheReservationManager::GetDummyEntrySize()));
EXPECT_GE(cache->GetPinnedUsage(),
dummy_entry_num * CacheReservationManager::GetDummyEntrySize());
EXPECT_LT(
cache->GetPinnedUsage(),
(dummy_entry_num + 1) * CacheReservationManager::GetDummyEntrySize());
} else {
EXPECT_EQ(cache->GetPinnedUsage(), 0);
}
}
}

Loading…
Cancel
Save