Add test for memtable bloom filter with WriteBufferManager #13398
Conversation
Summary: ... to ensure proper cache charging. However, this is a somewhat hazardous combination if there are many CFs and could be the target of future work. Test Plan: this is the test
|
@pdillinger has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
db/db_bloom_filter_test.cc
Outdated
|
|
||
| // Just the start of a memtable, no Bloom, rounded up to kSizeDummyEntry | ||
| ASSERT_GE(cache->GetUsage(), 1024U); | ||
| ASSERT_LE(cache->GetUsage(), 256U * 1024U); |
There was a problem hiding this comment.
nit: use static constexpr std::size_t kSizeDummyEntry = CacheReservationManagerImplCacheEntryRole::kMisc::GetDummyEntrySize(); ? Same for upper bound below just so it's easier to sea how many kSizeDummyEntry in the upper bound
db/db_bloom_filter_test.cc
Outdated
| ASSERT_OK(Put("foo2", "bar2")); | ||
|
|
||
| // Expecting a memtable Bloom of ratio times write_buffer_size, memory tracked | ||
| ASSERT_GE(cache->GetUsage(), 1000000U); |
There was a problem hiding this comment.
nit: options.write_buffer_size * options.memtable_prefix_bloom_size_ratio instead of hard-coded 1000000U ? Same for 2000000U below
|
@pdillinger has updated the pull request. You must reimport the pull request before landing. |
|
@pdillinger has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@pdillinger merged this pull request in 836e88a. |
Summary: ... to ensure proper cache charging. However, this is a somewhat hazardous combination if there are many CFs and could be the target of future work.
Test Plan: this is the test