From 30b08878d8fa511700c70fdb2a72b5259777576a Mon Sep 17 00:00:00 2001 From: mrambacher Date: Fri, 18 Feb 2022 12:23:48 -0800 Subject: [PATCH] Make FilterPolicy Customizable (#9590) Summary: Make FilterPolicy into a Customizable class. Allow new FilterPolicy to be discovered through the ObjectRegistry Pull Request resolved: https://github.com/facebook/rocksdb/pull/9590 Reviewed By: pdillinger Differential Revision: D34327367 Pulled By: mrambacher fbshipit-source-id: 37e7edac90ec9457422b72f359ab8ef48829c190 --- HISTORY.md | 3 +- db/db_bloom_filter_test.cc | 28 +- include/rocksdb/filter_policy.h | 11 +- include/rocksdb/utilities/options_type.h | 1 - options/customizable_test.cc | 73 +++++ options/options_helper.cc | 6 - options/options_test.cc | 24 +- .../block_based/block_based_table_factory.cc | 44 +-- table/block_based/filter_policy.cc | 261 +++++++++++++----- table/block_based/filter_policy_internal.h | 45 +-- table/table_test.cc | 2 +- util/bloom_test.cc | 7 +- utilities/object_registry.cc | 6 + 13 files changed, 347 insertions(+), 164 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index d40e9b89a..2e6a3f596 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -43,8 +43,7 @@ string. Use something like "filter_policy=ribbonfilter:10" instead. * Allow configuration string like "filter_policy=bloomfilter:10" without bool, to minimize acknowledgement of obsolete block-based filter. - * A `filter_policy` loaded from an OPTIONS file can read existing filters - but still does not support writing new filters. + * Made FilterPolicy Customizable. Configuration of filter_policy is now accurately saved in OPTIONS file and can be loaded with LoadOptionsFromFile. (Loading an OPTIONS file generated by a previous version only enables reading and using existing filters, not generating new filters. Previously, no filter_policy would be configured from a saved OPTIONS file.) * Change meaning of nullptr return from GetBuilderWithContext() from "use block-based filter" to "generate no filter in this case." * Also, when user specifies bits_per_key < 0.5, we now round this down diff --git a/db/db_bloom_filter_test.cc b/db/db_bloom_filter_test.cc index d398d9269..c2630d2e5 100644 --- a/db/db_bloom_filter_test.cc +++ b/db/db_bloom_filter_test.cc @@ -33,14 +33,15 @@ std::shared_ptr Create(double bits_per_key, const std::string& name) { return BloomLikeFilterPolicy::Create(name, bits_per_key); } -const std::string kLegacyBloom = test::LegacyBloomFilterPolicy::kName(); +const std::string kLegacyBloom = test::LegacyBloomFilterPolicy::kClassName(); const std::string kDeprecatedBlock = - DeprecatedBlockBasedBloomFilterPolicy::kName(); -const std::string kFastLocalBloom = test::FastLocalBloomFilterPolicy::kName(); + DeprecatedBlockBasedBloomFilterPolicy::kClassName(); +const std::string kFastLocalBloom = + test::FastLocalBloomFilterPolicy::kClassName(); const std::string kStandard128Ribbon = - test::Standard128RibbonFilterPolicy::kName(); -const std::string kAutoBloom = BloomFilterPolicy::kName(); -const std::string kAutoRibbon = RibbonFilterPolicy::kName(); + test::Standard128RibbonFilterPolicy::kClassName(); +const std::string kAutoBloom = BloomFilterPolicy::kClassName(); +const std::string kAutoRibbon = RibbonFilterPolicy::kClassName(); } // namespace // DB tests related to bloom filter. @@ -593,10 +594,9 @@ class AlwaysTrueBitsBuilder : public FilterBitsBuilder { size_t ApproximateNumEntries(size_t) override { return SIZE_MAX; } }; -class AlwaysTrueFilterPolicy : public BloomLikeFilterPolicy { +class AlwaysTrueFilterPolicy : public ReadOnlyBuiltinFilterPolicy { public: - explicit AlwaysTrueFilterPolicy(bool skip) - : BloomLikeFilterPolicy(/* ignored */ 10), skip_(skip) {} + explicit AlwaysTrueFilterPolicy(bool skip) : skip_(skip) {} FilterBitsBuilder* GetBuilderWithContext( const FilterBuildingContext&) const override { @@ -607,10 +607,6 @@ class AlwaysTrueFilterPolicy : public BloomLikeFilterPolicy { } } - std::string GetId() const override { - return "rocksdb.test.AlwaysTrueFilterPolicy"; - } - private: bool skip_; }; @@ -1606,11 +1602,11 @@ class TestingContextCustomFilterPolicy test_report_ += OptionsHelper::compaction_style_to_string[context.compaction_style]; test_report_ += ",n="; - test_report_ += ToString(context.num_levels); + test_report_ += ROCKSDB_NAMESPACE::ToString(context.num_levels); test_report_ += ",l="; - test_report_ += ToString(context.level_at_creation); + test_report_ += ROCKSDB_NAMESPACE::ToString(context.level_at_creation); test_report_ += ",b="; - test_report_ += ToString(int{context.is_bottommost}); + test_report_ += ROCKSDB_NAMESPACE::ToString(int{context.is_bottommost}); test_report_ += ",r="; test_report_ += table_file_creation_reason_to_string[context.reason]; test_report_ += "\n"; diff --git a/include/rocksdb/filter_policy.h b/include/rocksdb/filter_policy.h index d8fb33641..873520ee0 100644 --- a/include/rocksdb/filter_policy.h +++ b/include/rocksdb/filter_policy.h @@ -28,7 +28,7 @@ #include #include "rocksdb/advanced_options.h" -#include "rocksdb/memory_allocator.h" +#include "rocksdb/customizable.h" #include "rocksdb/status.h" #include "rocksdb/types.h" @@ -85,9 +85,10 @@ struct FilterBuildingContext { // defer to other built-in policies (see NewBloomFilterPolicy and // NewRibbonFilterPolicy) based on the context provided to // GetBuilderWithContext. -class FilterPolicy { +class FilterPolicy : public Customizable { public: virtual ~FilterPolicy(); + static const char* Type() { return "FilterPolicy"; } // Creates a new FilterPolicy based on the input value string and returns the // result The value might be an ID, and ID with properties, or an old-style @@ -101,12 +102,6 @@ class FilterPolicy { const std::string& value, std::shared_ptr* result); - // Return the name of this policy. Note that if the filter encoding - // changes in an incompatible way, the name returned by this method - // must be changed. Otherwise, old incompatible filters may be - // passed to methods of this type. - virtual const char* Name() const = 0; - // Return a new FilterBitsBuilder for constructing full or partitioned // filter blocks, or return nullptr to indicate "no filter". Custom // implementations should defer to a built-in FilterPolicy to get a diff --git a/include/rocksdb/utilities/options_type.h b/include/rocksdb/utilities/options_type.h index 33292b3b4..76ab3bafe 100644 --- a/include/rocksdb/utilities/options_type.h +++ b/include/rocksdb/utilities/options_type.h @@ -35,7 +35,6 @@ enum class OptionType { kCompactionPri, kCompressionType, kCompactionStopStyle, - kFilterPolicy, kChecksumType, kEncodingType, kEnv, diff --git a/options/customizable_test.cc b/options/customizable_test.cc index 02391d20b..690b394ea 100644 --- a/options/customizable_test.cc +++ b/options/customizable_test.cc @@ -21,6 +21,7 @@ #include "rocksdb/convenience.h" #include "rocksdb/env_encryption.h" #include "rocksdb/file_checksum.h" +#include "rocksdb/filter_policy.h" #include "rocksdb/flush_block_policy.h" #include "rocksdb/memory_allocator.h" #include "rocksdb/rate_limiter.h" @@ -31,6 +32,7 @@ #include "rocksdb/utilities/customizable_util.h" #include "rocksdb/utilities/object_registry.h" #include "rocksdb/utilities/options_type.h" +#include "table/block_based/filter_policy_internal.h" #include "table/block_based/flush_block_policy.h" #include "table/mock_table.h" #include "test_util/mock_time_env.h" @@ -1481,6 +1483,20 @@ class MockRateLimiter : public RateLimiter { } }; +class MockFilterPolicy : public FilterPolicy { + public: + static const char* kClassName() { return "MockFilterPolicy"; } + const char* Name() const override { return kClassName(); } + FilterBitsBuilder* GetBuilderWithContext( + const FilterBuildingContext&) const override { + return nullptr; + } + FilterBitsReader* GetFilterBitsReader( + const Slice& /*contents*/) const override { + return nullptr; + } +}; + #ifndef ROCKSDB_LITE static int RegisterLocalObjects(ObjectLibrary& library, const std::string& /*arg*/) { @@ -1605,6 +1621,14 @@ static int RegisterLocalObjects(ObjectLibrary& library, return guard->get(); }); + library.AddFactory( + MockFilterPolicy::kClassName(), + [](const std::string& /*uri*/, std::unique_ptr* guard, + std::string* /* errmsg */) { + guard->reset(new MockFilterPolicy()); + return guard->get(); + }); + return static_cast(library.GetFactoryCount(&num_types)); } #endif // !ROCKSDB_LITE @@ -2114,6 +2138,55 @@ TEST_F(LoadCustomizableTest, LoadRateLimiterTest) { #endif // ROCKSDB_LITE } +TEST_F(LoadCustomizableTest, LoadFilterPolicyTest) { + std::shared_ptr table; + std::shared_ptr result; + ASSERT_NOK(FilterPolicy::CreateFromString( + config_options_, MockFilterPolicy::kClassName(), &result)); + + ASSERT_OK(FilterPolicy::CreateFromString(config_options_, "", &result)); + ASSERT_EQ(result, nullptr); + ASSERT_OK(FilterPolicy::CreateFromString( + config_options_, ReadOnlyBuiltinFilterPolicy::kClassName(), &result)); + ASSERT_NE(result, nullptr); + ASSERT_STREQ(result->Name(), ReadOnlyBuiltinFilterPolicy::kClassName()); + +#ifndef ROCKSDB_LITE + std::string table_opts = "id=BlockBasedTable; filter_policy="; + ASSERT_OK(TableFactory::CreateFromString(config_options_, + table_opts + "nullptr", &table)); + ASSERT_NE(table.get(), nullptr); + auto bbto = table->GetOptions(); + ASSERT_NE(bbto, nullptr); + ASSERT_EQ(bbto->filter_policy.get(), nullptr); + ASSERT_OK(TableFactory::CreateFromString( + config_options_, table_opts + ReadOnlyBuiltinFilterPolicy::kClassName(), + &table)); + bbto = table->GetOptions(); + ASSERT_NE(bbto, nullptr); + ASSERT_NE(bbto->filter_policy.get(), nullptr); + ASSERT_STREQ(bbto->filter_policy->Name(), + ReadOnlyBuiltinFilterPolicy::kClassName()); + ASSERT_OK(TableFactory::CreateFromString( + config_options_, table_opts + MockFilterPolicy::kClassName(), &table)); + bbto = table->GetOptions(); + ASSERT_NE(bbto, nullptr); + ASSERT_EQ(bbto->filter_policy.get(), nullptr); + if (RegisterTests("Test")) { + ASSERT_OK(FilterPolicy::CreateFromString( + config_options_, MockFilterPolicy::kClassName(), &result)); + ASSERT_NE(result, nullptr); + ASSERT_STREQ(result->Name(), MockFilterPolicy::kClassName()); + ASSERT_OK(TableFactory::CreateFromString( + config_options_, table_opts + MockFilterPolicy::kClassName(), &table)); + bbto = table->GetOptions(); + ASSERT_NE(bbto, nullptr); + ASSERT_NE(bbto->filter_policy.get(), nullptr); + ASSERT_STREQ(bbto->filter_policy->Name(), MockFilterPolicy::kClassName()); + } +#endif // ROCKSDB_LITE +} + TEST_F(LoadCustomizableTest, LoadFlushBlockPolicyFactoryTest) { std::shared_ptr table; std::shared_ptr result; diff --git a/options/options_helper.cc b/options/options_helper.cc index 50d4e2300..80265d2d3 100644 --- a/options/options_helper.cc +++ b/options/options_helper.cc @@ -512,12 +512,6 @@ bool SerializeSingleOptionHelper(const void* opt_address, compression_type_string_map, *(static_cast(opt_address)), value); break; - case OptionType::kFilterPolicy: { - const auto* ptr = - static_cast*>(opt_address); - *value = ptr->get() ? ptr->get()->Name() : kNullptrString; - break; - } case OptionType::kChecksumType: return SerializeEnum( checksum_type_string_map, diff --git a/options/options_test.cc b/options/options_test.cc index 3dd068925..565159c9c 100644 --- a/options/options_test.cc +++ b/options/options_test.cc @@ -844,6 +844,7 @@ TEST_F(OptionsTest, GetBlockBasedTableOptionsFromString) { ConfigOptions config_options; config_options.input_strings_escaped = false; config_options.ignore_unknown_options = false; + config_options.ignore_unsupported_options = false; // make sure default values are overwritten by something else ASSERT_OK(GetBlockBasedTableOptionsFromString( @@ -878,8 +879,8 @@ TEST_F(OptionsTest, GetBlockBasedTableOptionsFromString) { ASSERT_EQ(new_opt.detect_filter_construct_corruption, true); ASSERT_EQ(new_opt.reserve_table_builder_memory, true); ASSERT_TRUE(new_opt.filter_policy != nullptr); - const BloomFilterPolicy* bfp = - dynamic_cast(new_opt.filter_policy.get()); + auto bfp = new_opt.filter_policy->CheckedCast(); + ASSERT_NE(bfp, nullptr); EXPECT_EQ(bfp->GetMillibitsPerKey(), 4567); EXPECT_EQ(bfp->GetWholeBitsPerKey(), 5); // Verify that only the lower 32bits are stored in @@ -1104,6 +1105,25 @@ TEST_F(OptionsTest, GetBlockBasedTableOptionsFromString) { ASSERT_EQ(std::dynamic_pointer_cast(new_opt.block_cache_compressed) ->GetHighPriPoolRatio(), 0.5); + + ASSERT_OK(GetBlockBasedTableOptionsFromString( + config_options, table_opt, "filter_policy=rocksdb.BloomFilter:1.234", + &new_opt)); + ASSERT_TRUE(new_opt.filter_policy != nullptr); + ASSERT_TRUE( + new_opt.filter_policy->IsInstanceOf(BloomFilterPolicy::kClassName())); + ASSERT_TRUE( + new_opt.filter_policy->IsInstanceOf(BloomFilterPolicy::kNickName())); + + // Ribbon filter policy alternative name + ASSERT_OK(GetBlockBasedTableOptionsFromString( + config_options, table_opt, "filter_policy=rocksdb.RibbonFilter:6.789:5;", + &new_opt)); + ASSERT_TRUE(new_opt.filter_policy != nullptr); + ASSERT_TRUE( + new_opt.filter_policy->IsInstanceOf(RibbonFilterPolicy::kClassName())); + ASSERT_TRUE( + new_opt.filter_policy->IsInstanceOf(RibbonFilterPolicy::kNickName())); } #endif // !ROCKSDB_LITE diff --git a/table/block_based/block_based_table_factory.cc b/table/block_based/block_based_table_factory.cc index d4b75f2ec..d69b296dc 100644 --- a/table/block_based/block_based_table_factory.cc +++ b/table/block_based/block_based_table_factory.cc @@ -314,44 +314,10 @@ static std::unordered_map OptionType::kBoolean, OptionVerificationType::kNormal, OptionTypeFlags::kNone}}, {"filter_policy", - {offsetof(struct BlockBasedTableOptions, filter_policy), - OptionType::kUnknown, OptionVerificationType::kByNameAllowFromNull, - OptionTypeFlags::kNone, - // Parses the Filter policy - [](const ConfigOptions& opts, const std::string&, - const std::string& value, void* addr) { - auto* policy = - static_cast*>(addr); - return FilterPolicy::CreateFromString(opts, value, policy); - }, - // Converts the FilterPolicy to its string representation - [](const ConfigOptions&, const std::string&, const void* addr, - std::string* value) { - const auto* policy = - static_cast*>(addr); - if (policy->get()) { - *value = (*policy)->Name(); - } else { - *value = kNullptrString; - } - return Status::OK(); - }, - // Compares two FilterPolicy objects for equality - [](const ConfigOptions&, const std::string&, const void* addr1, - const void* addr2, std::string*) { - const auto* policy1 = - static_cast*>(addr1) - ->get(); - const auto* policy2 = - static_cast*>(addr2)->get(); - if (policy1 == policy2) { - return true; - } else if (policy1 != nullptr && policy2 != nullptr) { - return (strcmp(policy1->Name(), policy2->Name()) == 0); - } else { - return false; - } - }}}, + OptionTypeInfo::AsCustomSharedPtr( + offsetof(struct BlockBasedTableOptions, filter_policy), + OptionVerificationType::kByNameAllowFromNull, + OptionTypeFlags::kNone)}, {"whole_key_filtering", {offsetof(struct BlockBasedTableOptions, whole_key_filtering), OptionType::kBoolean, OptionVerificationType::kNormal, @@ -913,6 +879,8 @@ Status GetBlockBasedTableOptionsFromString( config_options.input_strings_escaped = false; config_options.ignore_unknown_options = false; config_options.invoke_prepare_options = false; + config_options.ignore_unsupported_options = false; + return GetBlockBasedTableOptionsFromString(config_options, table_options, opts_str, new_table_options); } diff --git a/table/block_based/filter_policy.cc b/table/block_based/filter_policy.cc index 968231fb4..6594bfb4d 100644 --- a/table/block_based/filter_policy.cc +++ b/table/block_based/filter_policy.cc @@ -20,8 +20,10 @@ #include "cache/cache_reservation_manager.h" #include "logging/logging.h" #include "port/lang.h" +#include "rocksdb/convenience.h" #include "rocksdb/rocksdb_namespace.h" #include "rocksdb/slice.h" +#include "rocksdb/utilities/object_registry.h" #include "table/block_based/block_based_filter_block.h" #include "table/block_based/block_based_table_reader.h" #include "table/block_based/filter_policy_internal.h" @@ -1311,6 +1313,18 @@ Status XXPH3FilterBitsBuilder::MaybePostVerify(const Slice& filter_content) { } } // namespace +const char* BuiltinFilterPolicy::kClassName() { + return "rocksdb.internal.BuiltinFilter"; +} + +bool BuiltinFilterPolicy::IsInstanceOf(const std::string& name) const { + if (name == kClassName()) { + return true; + } else { + return FilterPolicy::IsInstanceOf(name); + } +} + BloomLikeFilterPolicy::BloomLikeFilterPolicy(double bits_per_key) : warned_(false), aggregate_rounding_balance_(0) { // Sanitize bits_per_key @@ -1345,17 +1359,28 @@ BloomLikeFilterPolicy::BloomLikeFilterPolicy(double bits_per_key) } BloomLikeFilterPolicy::~BloomLikeFilterPolicy() {} +const char* BloomLikeFilterPolicy::kClassName() { + return "rocksdb.internal.BloomLikeFilter"; +} + +bool BloomLikeFilterPolicy::IsInstanceOf(const std::string& name) const { + if (name == kClassName()) { + return true; + } else { + return BuiltinFilterPolicy::IsInstanceOf(name); + } +} -const char* BuiltinFilterPolicy::Name() const { +const char* ReadOnlyBuiltinFilterPolicy::kClassName() { return "rocksdb.BuiltinBloomFilter"; } -const char* DeprecatedBlockBasedBloomFilterPolicy::kName() { +const char* DeprecatedBlockBasedBloomFilterPolicy::kClassName() { return "rocksdb.internal.DeprecatedBlockBasedBloomFilter"; } -std::string DeprecatedBlockBasedBloomFilterPolicy::GetId() const { - return kName() + GetBitsPerKeySuffix(); +std::string BloomLikeFilterPolicy::GetId() const { + return Name() + GetBitsPerKeySuffix(); } DeprecatedBlockBasedBloomFilterPolicy::DeprecatedBlockBasedBloomFilterPolicy( @@ -1446,12 +1471,13 @@ FilterBitsBuilder* BloomFilterPolicy::GetBuilderWithContext( } } -const char* BloomFilterPolicy::kName() { return "bloomfilter"; } +const char* BloomFilterPolicy::kClassName() { return "bloomfilter"; } +const char* BloomFilterPolicy::kNickName() { return "rocksdb.BloomFilter"; } std::string BloomFilterPolicy::GetId() const { // Including ":false" for better forward-compatibility with 6.29 and earlier // which required a boolean `use_block_based_builder` parameter - return kName() + GetBitsPerKeySuffix() + ":false"; + return BloomLikeFilterPolicy::GetId() + ":false"; } FilterBitsBuilder* BloomLikeFilterPolicy::GetFastLocalBloomBuilderWithContext( @@ -1543,14 +1569,10 @@ FilterBitsBuilder* BuiltinFilterPolicy::GetBuilderFromContext( // For testing only, but always constructable with internal names namespace test { -const char* LegacyBloomFilterPolicy::kName() { +const char* LegacyBloomFilterPolicy::kClassName() { return "rocksdb.internal.LegacyBloomFilter"; } -std::string LegacyBloomFilterPolicy::GetId() const { - return kName() + GetBitsPerKeySuffix(); -} - FilterBitsBuilder* LegacyBloomFilterPolicy::GetBuilderWithContext( const FilterBuildingContext& context) const { if (GetMillibitsPerKey() == 0) { @@ -1560,14 +1582,10 @@ FilterBitsBuilder* LegacyBloomFilterPolicy::GetBuilderWithContext( return GetLegacyBloomBuilderWithContext(context); } -const char* FastLocalBloomFilterPolicy::kName() { +const char* FastLocalBloomFilterPolicy::kClassName() { return "rocksdb.internal.FastLocalBloomFilter"; } -std::string FastLocalBloomFilterPolicy::GetId() const { - return kName() + GetBitsPerKeySuffix(); -} - FilterBitsBuilder* FastLocalBloomFilterPolicy::GetBuilderWithContext( const FilterBuildingContext& context) const { if (GetMillibitsPerKey() == 0) { @@ -1577,14 +1595,10 @@ FilterBitsBuilder* FastLocalBloomFilterPolicy::GetBuilderWithContext( return GetFastLocalBloomBuilderWithContext(context); } -const char* Standard128RibbonFilterPolicy::kName() { +const char* Standard128RibbonFilterPolicy::kClassName() { return "rocksdb.internal.Standard128RibbonFilter"; } -std::string Standard128RibbonFilterPolicy::GetId() const { - return kName() + GetBitsPerKeySuffix(); -} - FilterBitsBuilder* Standard128RibbonFilterPolicy::GetBuilderWithContext( const FilterBuildingContext& context) const { if (GetMillibitsPerKey() == 0) { @@ -1816,10 +1830,11 @@ FilterBitsBuilder* RibbonFilterPolicy::GetBuilderWithContext( } } -const char* RibbonFilterPolicy::kName() { return "ribbonfilter"; } +const char* RibbonFilterPolicy::kClassName() { return "ribbonfilter"; } +const char* RibbonFilterPolicy::kNickName() { return "rocksdb.RibbonFilter"; } std::string RibbonFilterPolicy::GetId() const { - return kName() + GetBitsPerKeySuffix() + ":" + + return BloomLikeFilterPolicy::GetId() + ":" + ROCKSDB_NAMESPACE::ToString(bloom_before_level_); } @@ -1837,19 +1852,19 @@ FilterPolicy::~FilterPolicy() { } std::shared_ptr BloomLikeFilterPolicy::Create( const std::string& name, double bits_per_key) { - if (name == test::LegacyBloomFilterPolicy::kName()) { + if (name == test::LegacyBloomFilterPolicy::kClassName()) { return std::make_shared(bits_per_key); - } else if (name == test::FastLocalBloomFilterPolicy::kName()) { + } else if (name == test::FastLocalBloomFilterPolicy::kClassName()) { return std::make_shared(bits_per_key); - } else if (name == test::Standard128RibbonFilterPolicy::kName()) { + } else if (name == test::Standard128RibbonFilterPolicy::kClassName()) { return std::make_shared(bits_per_key); - } else if (name == DeprecatedBlockBasedBloomFilterPolicy::kName()) { + } else if (name == DeprecatedBlockBasedBloomFilterPolicy::kClassName()) { return std::make_shared( bits_per_key); - } else if (name == BloomFilterPolicy::kName()) { + } else if (name == BloomFilterPolicy::kClassName()) { // For testing return std::make_shared(bits_per_key); - } else if (name == RibbonFilterPolicy::kName()) { + } else if (name == RibbonFilterPolicy::kClassName()) { // For testing return std::make_shared(bits_per_key, /*bloom_before_level*/ 0); @@ -1858,59 +1873,173 @@ std::shared_ptr BloomLikeFilterPolicy::Create( } } +#ifndef ROCKSDB_LITE +namespace { +static ObjectLibrary::PatternEntry FilterPatternEntryWithBits( + const char* name) { + return ObjectLibrary::PatternEntry(name, false).AddNumber(":", false); +} + +template +T* NewBuiltinFilterPolicyWithBits(const std::string& uri) { + const std::vector vals = StringSplit(uri, ':'); + double bits_per_key = ParseDouble(vals[1]); + return new T(bits_per_key); +} +static int RegisterBuiltinFilterPolicies(ObjectLibrary& library, + const std::string& /*arg*/) { + library.AddFactory( + ReadOnlyBuiltinFilterPolicy::kClassName(), + [](const std::string& /*uri*/, std::unique_ptr* guard, + std::string* /* errmsg */) { + guard->reset(new ReadOnlyBuiltinFilterPolicy()); + return guard->get(); + }); + + library.AddFactory( + FilterPatternEntryWithBits(BloomFilterPolicy::kClassName()) + .AnotherName(BloomFilterPolicy::kNickName()), + [](const std::string& uri, std::unique_ptr* guard, + std::string* /* errmsg */) { + guard->reset(NewBuiltinFilterPolicyWithBits(uri)); + return guard->get(); + }); + library.AddFactory( + FilterPatternEntryWithBits(BloomFilterPolicy::kClassName()) + .AnotherName(BloomFilterPolicy::kNickName()) + .AddSuffix(":false"), + [](const std::string& uri, std::unique_ptr* guard, + std::string* /* errmsg */) { + guard->reset(NewBuiltinFilterPolicyWithBits(uri)); + return guard->get(); + }); + library.AddFactory( + FilterPatternEntryWithBits(BloomFilterPolicy::kClassName()) + .AnotherName(BloomFilterPolicy::kNickName()) + .AddSuffix(":true"), + [](const std::string& uri, std::unique_ptr* guard, + std::string* /* errmsg */) { + const std::vector vals = StringSplit(uri, ':'); + double bits_per_key = ParseDouble(vals[1]); + // NOTE: This case previously configured the deprecated block-based + // filter, but old ways of configuring that now map to full filter. We + // defer to the corresponding API to ensure consistency in case that + // change is reverted. + guard->reset(NewBloomFilterPolicy(bits_per_key, true)); + return guard->get(); + }); + library.AddFactory( + FilterPatternEntryWithBits(RibbonFilterPolicy::kClassName()) + .AnotherName(RibbonFilterPolicy::kNickName()), + [](const std::string& uri, std::unique_ptr* guard, + std::string* /* errmsg */) { + const std::vector vals = StringSplit(uri, ':'); + double bits_per_key = ParseDouble(vals[1]); + guard->reset(NewRibbonFilterPolicy(bits_per_key)); + return guard->get(); + }); + library.AddFactory( + FilterPatternEntryWithBits(RibbonFilterPolicy::kClassName()) + .AnotherName(RibbonFilterPolicy::kNickName()) + .AddNumber(":", true), + [](const std::string& uri, std::unique_ptr* guard, + std::string* /* errmsg */) { + const std::vector vals = StringSplit(uri, ':'); + double bits_per_key = ParseDouble(vals[1]); + int bloom_before_level = ParseInt(vals[2]); + guard->reset(NewRibbonFilterPolicy(bits_per_key, bloom_before_level)); + return guard->get(); + }); + library.AddFactory( + FilterPatternEntryWithBits(test::LegacyBloomFilterPolicy::kClassName()), + [](const std::string& uri, std::unique_ptr* guard, + std::string* /* errmsg */) { + guard->reset( + NewBuiltinFilterPolicyWithBits(uri)); + return guard->get(); + }); + library.AddFactory( + FilterPatternEntryWithBits( + test::FastLocalBloomFilterPolicy::kClassName()), + [](const std::string& uri, std::unique_ptr* guard, + std::string* /* errmsg */) { + guard->reset( + NewBuiltinFilterPolicyWithBits( + uri)); + return guard->get(); + }); + library.AddFactory( + FilterPatternEntryWithBits( + test::Standard128RibbonFilterPolicy::kClassName()), + [](const std::string& uri, std::unique_ptr* guard, + std::string* /* errmsg */) { + guard->reset( + NewBuiltinFilterPolicyWithBits( + uri)); + return guard->get(); + }); + library.AddFactory( + FilterPatternEntryWithBits( + DeprecatedBlockBasedBloomFilterPolicy::kClassName()), + [](const std::string& uri, std::unique_ptr* guard, + std::string* /* errmsg */) { + guard->reset(NewBuiltinFilterPolicyWithBits< + DeprecatedBlockBasedBloomFilterPolicy>(uri)); + return guard->get(); + }); + size_t num_types; + return static_cast(library.GetFactoryCount(&num_types)); +} +} // namespace +#endif // ROCKSDB_LITE + Status FilterPolicy::CreateFromString( - const ConfigOptions& /*options*/, const std::string& value, + const ConfigOptions& options, const std::string& value, std::shared_ptr* policy) { - if (value == kNullptrString) { + if (value == kNullptrString || value.empty()) { policy->reset(); return Status::OK(); - } else if (value == "rocksdb.BuiltinBloomFilter") { + } else if (value == ReadOnlyBuiltinFilterPolicy::kClassName()) { *policy = std::make_shared(); return Status::OK(); } -#ifndef ROCKSDB_LITE - const std::vector vals = StringSplit(value, ':'); - if (vals.size() < 2) { - return Status::NotFound("Invalid filter policy name ", value); - } - const std::string& name = vals[0]; - double bits_per_key = ParseDouble(trim(vals[1])); - if (name == BloomFilterPolicy::kName()) { - bool use_block_based_builder = false; - if (vals.size() > 2) { - use_block_based_builder = - ParseBoolean("use_block_based_builder", trim(vals[2])); - } - policy->reset(NewBloomFilterPolicy(bits_per_key, use_block_based_builder)); - } else if (name == RibbonFilterPolicy::kName()) { - int bloom_before_level; - if (vals.size() < 3) { - bloom_before_level = 0; - } else { - bloom_before_level = ParseInt(trim(vals[2])); - } - policy->reset(NewRibbonFilterPolicy(/*bloom_equivalent*/ bits_per_key, - bloom_before_level)); + + std::string id; + std::unordered_map opt_map; + Status status = + Customizable::GetOptionsMap(options, policy->get(), value, &id, &opt_map); + if (!status.ok()) { // GetOptionsMap failed + return status; + } else if (id.empty()) { // We have no Id but have options. Not good + return Status::NotSupported("Cannot reset object ", id); } else { - *policy = BloomLikeFilterPolicy::Create(name, bits_per_key); +#ifndef ROCKSDB_LITE + static std::once_flag loaded; + std::call_once(loaded, [&]() { + RegisterBuiltinFilterPolicies(*(ObjectLibrary::Default().get()), ""); + }); + status = options.registry->NewSharedObject(id, policy); +#else + status = + Status::NotSupported("Cannot load filter policy in LITE mode ", value); +#endif // ROCKSDB_LITE } - if (*policy) { + if (options.ignore_unsupported_options && status.IsNotSupported()) { return Status::OK(); - } else { - return Status::NotFound("Invalid filter policy name ", value); + } else if (status.ok()) { + status = Customizable::ConfigureNewObject( + options, const_cast(policy->get()), opt_map); } -#else - return Status::NotSupported("Cannot load filter policy in LITE mode ", value); -#endif // ROCKSDB_LITE + return status; } const std::vector& BloomLikeFilterPolicy::GetAllFixedImpls() { STATIC_AVOID_DESTRUCTION(std::vector, impls){ // Match filter_bench -impl=x ordering - test::LegacyBloomFilterPolicy::kName(), - DeprecatedBlockBasedBloomFilterPolicy::kName(), - test::FastLocalBloomFilterPolicy::kName(), - test::Standard128RibbonFilterPolicy::kName(), + test::LegacyBloomFilterPolicy::kClassName(), + DeprecatedBlockBasedBloomFilterPolicy::kClassName(), + test::FastLocalBloomFilterPolicy::kClassName(), + test::Standard128RibbonFilterPolicy::kClassName(), }; return impls; } diff --git a/table/block_based/filter_policy_internal.h b/table/block_based/filter_policy_internal.h index 39fc4104e..4ceeed0d0 100644 --- a/table/block_based/filter_policy_internal.h +++ b/table/block_based/filter_policy_internal.h @@ -128,20 +128,13 @@ class BuiltinFilterBitsReader : public FilterBitsReader { // be used even when you change between built-in policies). class BuiltinFilterPolicy : public FilterPolicy { public: // overrides - // Shared name because any built-in policy can read filters from - // any other - // FIXME when making filter policies Configurable. For now, this - // is still rocksdb.BuiltinBloomFilter - const char* Name() const override; - - // Convert to a string understood by FilterPolicy::CreateFromString - virtual std::string GetId() const = 0; - // Read metadata to determine what kind of FilterBitsReader is needed // and return a new one. This must successfully process any filter data // generated by a built-in FilterBitsBuilder, regardless of the impl // chosen for this BloomFilterPolicy. Not compatible with CreateFilter. FilterBitsReader* GetFilterBitsReader(const Slice& contents) const override; + static const char* kClassName(); + bool IsInstanceOf(const std::string& id) const override; public: // new // An internal function for the implementation of @@ -175,8 +168,8 @@ class BuiltinFilterPolicy : public FilterPolicy { // This class is considered internal API and subject to change. class ReadOnlyBuiltinFilterPolicy : public BuiltinFilterPolicy { public: - // Convert to a string understood by FilterPolicy::CreateFromString - virtual std::string GetId() const override { return Name(); } + const char* Name() const override { return kClassName(); } + static const char* kClassName(); // Does not write filters. FilterBitsBuilder* GetBuilderWithContext( @@ -194,6 +187,10 @@ class BloomLikeFilterPolicy : public BuiltinFilterPolicy { explicit BloomLikeFilterPolicy(double bits_per_key); ~BloomLikeFilterPolicy() override; + static const char* kClassName(); + bool IsInstanceOf(const std::string& id) const override; + + std::string GetId() const override; // Essentially for testing only: configured millibits/key int GetMillibitsPerKey() const { return millibits_per_key_; } @@ -268,7 +265,10 @@ class BloomFilterPolicy : public BloomLikeFilterPolicy { FilterBitsBuilder* GetBuilderWithContext( const FilterBuildingContext&) const override; - static const char* kName(); + static const char* kClassName(); + const char* Name() const override { return kClassName(); } + static const char* kNickName(); + const char* NickName() const override { return kNickName(); } std::string GetId() const override; }; @@ -287,7 +287,10 @@ class RibbonFilterPolicy : public BloomLikeFilterPolicy { int GetBloomBeforeLevel() const { return bloom_before_level_; } - static const char* kName(); + static const char* kClassName(); + const char* Name() const override { return kClassName(); } + static const char* kNickName(); + const char* NickName() const override { return kNickName(); } std::string GetId() const override; private: @@ -310,8 +313,8 @@ class DeprecatedBlockBasedBloomFilterPolicy : public BloomLikeFilterPolicy { const FilterBuildingContext&) const override; static constexpr size_t kSecretBitsPerKeyStart = 1234567890U; - static const char* kName(); - std::string GetId() const override; + static const char* kClassName(); + const char* Name() const override { return kClassName(); } static void CreateFilter(const Slice* keys, int n, int bits_per_key, std::string* dst); @@ -329,8 +332,8 @@ class LegacyBloomFilterPolicy : public BloomLikeFilterPolicy { FilterBitsBuilder* GetBuilderWithContext( const FilterBuildingContext& context) const override; - static const char* kName(); - std::string GetId() const override; + static const char* kClassName(); + const char* Name() const override { return kClassName(); } }; class FastLocalBloomFilterPolicy : public BloomLikeFilterPolicy { @@ -341,8 +344,8 @@ class FastLocalBloomFilterPolicy : public BloomLikeFilterPolicy { FilterBitsBuilder* GetBuilderWithContext( const FilterBuildingContext& context) const override; - static const char* kName(); - std::string GetId() const override; + static const char* kClassName(); + const char* Name() const override { return kClassName(); } }; class Standard128RibbonFilterPolicy : public BloomLikeFilterPolicy { @@ -353,8 +356,8 @@ class Standard128RibbonFilterPolicy : public BloomLikeFilterPolicy { FilterBitsBuilder* GetBuilderWithContext( const FilterBuildingContext& context) const override; - static const char* kName(); - std::string GetId() const override; + static const char* kClassName(); + const char* Name() const override { return kClassName(); } }; } // namespace test diff --git a/table/table_test.cc b/table/table_test.cc index 78c126272..b847cb70a 100644 --- a/table/table_test.cc +++ b/table/table_test.cc @@ -1872,7 +1872,7 @@ TEST_P(BlockBasedTableTest, FilterPolicyNameProperties) { c.Finish(options, ioptions, moptions, table_options, GetPlainInternalComparator(options.comparator), &keys, &kvmap); auto& props = *c.GetTableReader()->GetTableProperties(); - ASSERT_EQ("rocksdb.BuiltinBloomFilter", props.filter_policy_name); + ASSERT_EQ(table_options.filter_policy->Name(), props.filter_policy_name); c.ResetTableReader(); } diff --git a/util/bloom_test.cc b/util/bloom_test.cc index 791f71ba4..f18e07a6d 100644 --- a/util/bloom_test.cc +++ b/util/bloom_test.cc @@ -40,10 +40,11 @@ DEFINE_int32(bits_per_key, 10, ""); namespace ROCKSDB_NAMESPACE { namespace { -const std::string kLegacyBloom = test::LegacyBloomFilterPolicy::kName(); -const std::string kFastLocalBloom = test::FastLocalBloomFilterPolicy::kName(); +const std::string kLegacyBloom = test::LegacyBloomFilterPolicy::kClassName(); +const std::string kFastLocalBloom = + test::FastLocalBloomFilterPolicy::kClassName(); const std::string kStandard128Ribbon = - test::Standard128RibbonFilterPolicy::kName(); + test::Standard128RibbonFilterPolicy::kClassName(); } // namespace static const int kVerbose = 1; diff --git a/utilities/object_registry.cc b/utilities/object_registry.cc index 6f643ba9b..f46343492 100644 --- a/utilities/object_registry.cc +++ b/utilities/object_registry.cc @@ -19,6 +19,9 @@ namespace { bool MatchesInteger(const std::string &target, size_t start, size_t pos) { // If it is numeric, everything up to the match must be a number int digits = 0; + if (target[start] == '-') { + start++; // Allow negative numbers + } while (start < pos) { if (!isdigit(target[start++])) { return false; @@ -31,6 +34,9 @@ bool MatchesInteger(const std::string &target, size_t start, size_t pos) { bool MatchesDecimal(const std::string &target, size_t start, size_t pos) { int digits = 0; + if (target[start] == '-') { + start++; // Allow negative numbers + } for (bool point = false; start < pos; start++) { if (target[start] == '.') { if (point) {