Fix assertion failure in FastLocalBloomBitsBuilder (#9585)

Summary:
As in

```
db_stress: table/block_based/filter_policy.cc:316: rocksdb::{anonymous}::FastLocalBloomBitsBuilder::FastLocalBloomBitsBuilder(int, std::atomic<long int>*, std::shared_ptr<rocksdb::CacheReservationManager>, bool): Assertion `millibits_per_key >= 1000' failed.
```

This assertion failure was actually happening with our RibbonFilterPolicy
which falls back to Bloom for some cases, often for flush, but was
missing new special logic to skip generating filter for 0 bits per key
case. Fixed by adding the logic in other builtin FilterPolicy
implementations.

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

Test Plan:
Updated db_bloom_filter_test to do more integration testing
of the RibbonFilterPolicy ("auto Ribbon") class, incl regression test
this with SkipFilterOnEssentiallyZeroBpk

Reviewed By: ajkr

Differential Revision: D34295101

Pulled By: pdillinger

fbshipit-source-id: 3488eb207fc1d67bbbd1301313714aa1b6406e6e
This commit is contained in:
Peter Dillinger 2022-02-16 22:42:28 -08:00 committed by Facebook GitHub Bot
parent 8286469b9a
commit 1e403a0c6c
2 changed files with 19 additions and 4 deletions

View File

@ -22,6 +22,7 @@
#include "rocksdb/table.h" #include "rocksdb/table.h"
#include "table/block_based/block_based_table_reader.h" #include "table/block_based/block_based_table_reader.h"
#include "table/block_based/filter_policy_internal.h" #include "table/block_based/filter_policy_internal.h"
#include "table/format.h"
#include "test_util/testutil.h" #include "test_util/testutil.h"
#include "util/string_util.h" #include "util/string_util.h"
@ -39,6 +40,7 @@ const std::string kFastLocalBloom = test::FastLocalBloomFilterPolicy::kName();
const std::string kStandard128Ribbon = const std::string kStandard128Ribbon =
test::Standard128RibbonFilterPolicy::kName(); test::Standard128RibbonFilterPolicy::kName();
const std::string kAutoBloom = BloomFilterPolicy::kName(); const std::string kAutoBloom = BloomFilterPolicy::kName();
const std::string kAutoRibbon = RibbonFilterPolicy::kName();
} // namespace } // namespace
// DB tests related to bloom filter. // DB tests related to bloom filter.
@ -559,7 +561,13 @@ TEST_P(DBBloomFilterTestWithParam, BloomFilter) {
uint64_t filter_size = ParseUint64(props["filter_size"]); uint64_t filter_size = ParseUint64(props["filter_size"]);
EXPECT_LE(filter_size, EXPECT_LE(filter_size,
(partition_filters_ ? 12 : 11) * nkeys / /*bits / byte*/ 8); (partition_filters_ ? 12 : 11) * nkeys / /*bits / byte*/ 8);
EXPECT_GE(filter_size, 10 * nkeys / /*bits / byte*/ 8); if (bfp_impl_ == kAutoRibbon) {
// Sometimes using Ribbon filter which is more space-efficient
EXPECT_GE(filter_size, 7 * nkeys / /*bits / byte*/ 8);
} else {
// Always Bloom
EXPECT_GE(filter_size, 10 * nkeys / /*bits / byte*/ 8);
}
uint64_t num_filter_entries = ParseUint64(props["num_filter_entries"]); uint64_t num_filter_entries = ParseUint64(props["num_filter_entries"]);
EXPECT_EQ(num_filter_entries, nkeys); EXPECT_EQ(num_filter_entries, nkeys);
@ -741,21 +749,24 @@ INSTANTIATE_TEST_CASE_P(
::testing::Values( ::testing::Values(
std::make_tuple(kDeprecatedBlock, false, test::kDefaultFormatVersion), std::make_tuple(kDeprecatedBlock, false, test::kDefaultFormatVersion),
std::make_tuple(kAutoBloom, true, test::kDefaultFormatVersion), std::make_tuple(kAutoBloom, true, test::kDefaultFormatVersion),
std::make_tuple(kAutoBloom, false, test::kDefaultFormatVersion))); std::make_tuple(kAutoBloom, false, test::kDefaultFormatVersion),
std::make_tuple(kAutoRibbon, false, test::kDefaultFormatVersion)));
INSTANTIATE_TEST_CASE_P( INSTANTIATE_TEST_CASE_P(
FormatDef, DBBloomFilterTestWithParam, FormatDef, DBBloomFilterTestWithParam,
::testing::Values( ::testing::Values(
std::make_tuple(kDeprecatedBlock, false, test::kDefaultFormatVersion), std::make_tuple(kDeprecatedBlock, false, test::kDefaultFormatVersion),
std::make_tuple(kAutoBloom, true, test::kDefaultFormatVersion), std::make_tuple(kAutoBloom, true, test::kDefaultFormatVersion),
std::make_tuple(kAutoBloom, false, test::kDefaultFormatVersion))); std::make_tuple(kAutoBloom, false, test::kDefaultFormatVersion),
std::make_tuple(kAutoRibbon, false, test::kDefaultFormatVersion)));
INSTANTIATE_TEST_CASE_P( INSTANTIATE_TEST_CASE_P(
FormatLatest, DBBloomFilterTestWithParam, FormatLatest, DBBloomFilterTestWithParam,
::testing::Values( ::testing::Values(
std::make_tuple(kDeprecatedBlock, false, kLatestFormatVersion), std::make_tuple(kDeprecatedBlock, false, kLatestFormatVersion),
std::make_tuple(kAutoBloom, true, kLatestFormatVersion), std::make_tuple(kAutoBloom, true, kLatestFormatVersion),
std::make_tuple(kAutoBloom, false, kLatestFormatVersion))); std::make_tuple(kAutoBloom, false, kLatestFormatVersion),
std::make_tuple(kAutoRibbon, false, kLatestFormatVersion)));
#endif // !defined(ROCKSDB_VALGRIND_RUN) || defined(ROCKSDB_FULL_VALGRIND_RUN) #endif // !defined(ROCKSDB_VALGRIND_RUN) || defined(ROCKSDB_FULL_VALGRIND_RUN)
TEST_F(DBBloomFilterTest, BloomFilterRate) { TEST_F(DBBloomFilterTest, BloomFilterRate) {

View File

@ -1781,6 +1781,10 @@ RibbonFilterPolicy::RibbonFilterPolicy(double bloom_equivalent_bits_per_key,
FilterBitsBuilder* RibbonFilterPolicy::GetBuilderWithContext( FilterBitsBuilder* RibbonFilterPolicy::GetBuilderWithContext(
const FilterBuildingContext& context) const { const FilterBuildingContext& context) const {
if (GetMillibitsPerKey() == 0) {
// "No filter" special case
return nullptr;
}
// Treat unknown same as bottommost // Treat unknown same as bottommost
int levelish = INT_MAX; int levelish = INT_MAX;