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

@@ -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

db/db_bloom_filter_test.cc

@@ -33,14 +33,15 @@ std::shared_ptr<const FilterPolicy> 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";

include/rocksdb/filter_policy.h

@@ -28,7 +28,7 @@
 #include <vector>
 
 #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<const FilterPolicy>* 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

include/rocksdb/utilities/options_type.h

@@ -35,7 +35,6 @@ enum class OptionType {
   kCompactionPri,
   kCompressionType,
   kCompactionStopStyle,
-  kFilterPolicy,
   kChecksumType,
   kEncodingType,
   kEnv,

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<const FilterPolicy>(
+      MockFilterPolicy::kClassName(),
+      [](const std::string& /*uri*/, std::unique_ptr<const FilterPolicy>* guard,
+         std::string* /* errmsg */) {
+        guard->reset(new MockFilterPolicy());
+        return guard->get();
+      });
+
   return static_cast<int>(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<TableFactory> table;
+  std::shared_ptr<const FilterPolicy> 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<BlockBasedTableOptions>();
+  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<BlockBasedTableOptions>();
+  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<BlockBasedTableOptions>();
+  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<BlockBasedTableOptions>();
+    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<TableFactory> table;
   std::shared_ptr<FlushBlockPolicyFactory> result;

options/options_helper.cc

@@ -512,12 +512,6 @@ bool SerializeSingleOptionHelper(const void* opt_address,
           compression_type_string_map,
           *(static_cast<const CompressionType*>(opt_address)), value);
       break;
-    case OptionType::kFilterPolicy: {
-      const auto* ptr =
-          static_cast<const std::shared_ptr<FilterPolicy>*>(opt_address);
-      *value = ptr->get() ? ptr->get()->Name() : kNullptrString;
-      break;
-    }
     case OptionType::kChecksumType:
       return SerializeEnum<ChecksumType>(
           checksum_type_string_map,

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<const BloomFilterPolicy*>(new_opt.filter_policy.get());
+  auto bfp = new_opt.filter_policy->CheckedCast<BloomFilterPolicy>();
+  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<LRUCache>(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
 

table/block_based/block_based_table_factory.cc

@@ -314,44 +314,10 @@ static std::unordered_map<std::string, OptionTypeInfo>
           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<std::shared_ptr<const FilterPolicy>*>(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<const std::shared_ptr<const FilterPolicy>*>(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<const std::shared_ptr<const FilterPolicy>*>(addr1)
-                    ->get();
-            const auto* policy2 =
-                static_cast<const std::shared_ptr<FilterPolicy>*>(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<const FilterPolicy>(
+             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);
 }

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<const FilterPolicy> BloomLikeFilterPolicy::Create(
     const std::string& name, double bits_per_key) {
-  if (name == test::LegacyBloomFilterPolicy::kName()) {
+  if (name == test::LegacyBloomFilterPolicy::kClassName()) {
     return std::make_shared<test::LegacyBloomFilterPolicy>(bits_per_key);
-  } else if (name == test::FastLocalBloomFilterPolicy::kName()) {
+  } else if (name == test::FastLocalBloomFilterPolicy::kClassName()) {
     return std::make_shared<test::FastLocalBloomFilterPolicy>(bits_per_key);
-  } else if (name == test::Standard128RibbonFilterPolicy::kName()) {
+  } else if (name == test::Standard128RibbonFilterPolicy::kClassName()) {
     return std::make_shared<test::Standard128RibbonFilterPolicy>(bits_per_key);
-  } else if (name == DeprecatedBlockBasedBloomFilterPolicy::kName()) {
+  } else if (name == DeprecatedBlockBasedBloomFilterPolicy::kClassName()) {
     return std::make_shared<DeprecatedBlockBasedBloomFilterPolicy>(
         bits_per_key);
-  } else if (name == BloomFilterPolicy::kName()) {
+  } else if (name == BloomFilterPolicy::kClassName()) {
     // For testing
     return std::make_shared<BloomFilterPolicy>(bits_per_key);
-  } else if (name == RibbonFilterPolicy::kName()) {
+  } else if (name == RibbonFilterPolicy::kClassName()) {
     // For testing
     return std::make_shared<RibbonFilterPolicy>(bits_per_key,
                                                 /*bloom_before_level*/ 0);
@@ -1858,59 +1873,173 @@ std::shared_ptr<const FilterPolicy> BloomLikeFilterPolicy::Create(
   }
 }
 
+#ifndef ROCKSDB_LITE
+namespace {
+static ObjectLibrary::PatternEntry FilterPatternEntryWithBits(
+    const char* name) {
+  return ObjectLibrary::PatternEntry(name, false).AddNumber(":", false);
+}
+
+template <typename T>
+T* NewBuiltinFilterPolicyWithBits(const std::string& uri) {
+  const std::vector<std::string> 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<const FilterPolicy>(
+      ReadOnlyBuiltinFilterPolicy::kClassName(),
+      [](const std::string& /*uri*/, std::unique_ptr<const FilterPolicy>* guard,
+         std::string* /* errmsg */) {
+        guard->reset(new ReadOnlyBuiltinFilterPolicy());
+        return guard->get();
+      });
+
+  library.AddFactory<const FilterPolicy>(
+      FilterPatternEntryWithBits(BloomFilterPolicy::kClassName())
+          .AnotherName(BloomFilterPolicy::kNickName()),
+      [](const std::string& uri, std::unique_ptr<const FilterPolicy>* guard,
+         std::string* /* errmsg */) {
+        guard->reset(NewBuiltinFilterPolicyWithBits<BloomFilterPolicy>(uri));
+        return guard->get();
+      });
+  library.AddFactory<const FilterPolicy>(
+      FilterPatternEntryWithBits(BloomFilterPolicy::kClassName())
+          .AnotherName(BloomFilterPolicy::kNickName())
+          .AddSuffix(":false"),
+      [](const std::string& uri, std::unique_ptr<const FilterPolicy>* guard,
+         std::string* /* errmsg */) {
+        guard->reset(NewBuiltinFilterPolicyWithBits<BloomFilterPolicy>(uri));
+        return guard->get();
+      });
+  library.AddFactory<const FilterPolicy>(
+      FilterPatternEntryWithBits(BloomFilterPolicy::kClassName())
+          .AnotherName(BloomFilterPolicy::kNickName())
+          .AddSuffix(":true"),
+      [](const std::string& uri, std::unique_ptr<const FilterPolicy>* guard,
+         std::string* /* errmsg */) {
+        const std::vector<std::string> 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<const FilterPolicy>(
+      FilterPatternEntryWithBits(RibbonFilterPolicy::kClassName())
+          .AnotherName(RibbonFilterPolicy::kNickName()),
+      [](const std::string& uri, std::unique_ptr<const FilterPolicy>* guard,
+         std::string* /* errmsg */) {
+        const std::vector<std::string> vals = StringSplit(uri, ':');
+        double bits_per_key = ParseDouble(vals[1]);
+        guard->reset(NewRibbonFilterPolicy(bits_per_key));
+        return guard->get();
+      });
+  library.AddFactory<const FilterPolicy>(
+      FilterPatternEntryWithBits(RibbonFilterPolicy::kClassName())
+          .AnotherName(RibbonFilterPolicy::kNickName())
+          .AddNumber(":", true),
+      [](const std::string& uri, std::unique_ptr<const FilterPolicy>* guard,
+         std::string* /* errmsg */) {
+        const std::vector<std::string> 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<const FilterPolicy>(
+      FilterPatternEntryWithBits(test::LegacyBloomFilterPolicy::kClassName()),
+      [](const std::string& uri, std::unique_ptr<const FilterPolicy>* guard,
+         std::string* /* errmsg */) {
+        guard->reset(
+            NewBuiltinFilterPolicyWithBits<test::LegacyBloomFilterPolicy>(uri));
+        return guard->get();
+      });
+  library.AddFactory<const FilterPolicy>(
+      FilterPatternEntryWithBits(
+          test::FastLocalBloomFilterPolicy::kClassName()),
+      [](const std::string& uri, std::unique_ptr<const FilterPolicy>* guard,
+         std::string* /* errmsg */) {
+        guard->reset(
+            NewBuiltinFilterPolicyWithBits<test::FastLocalBloomFilterPolicy>(
+                uri));
+        return guard->get();
+      });
+  library.AddFactory<const FilterPolicy>(
+      FilterPatternEntryWithBits(
+          test::Standard128RibbonFilterPolicy::kClassName()),
+      [](const std::string& uri, std::unique_ptr<const FilterPolicy>* guard,
+         std::string* /* errmsg */) {
+        guard->reset(
+            NewBuiltinFilterPolicyWithBits<test::Standard128RibbonFilterPolicy>(
+                uri));
+        return guard->get();
+      });
+  library.AddFactory<const FilterPolicy>(
+      FilterPatternEntryWithBits(
+          DeprecatedBlockBasedBloomFilterPolicy::kClassName()),
+      [](const std::string& uri, std::unique_ptr<const FilterPolicy>* guard,
+         std::string* /* errmsg */) {
+        guard->reset(NewBuiltinFilterPolicyWithBits<
+                     DeprecatedBlockBasedBloomFilterPolicy>(uri));
+        return guard->get();
+      });
+  size_t num_types;
+  return static_cast<int>(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<const FilterPolicy>* 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<ReadOnlyBuiltinFilterPolicy>();
     return Status::OK();
   }
-#ifndef ROCKSDB_LITE
-  const std::vector<std::string> 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<std::string, std::string> 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<FilterPolicy*>(policy->get()), opt_map);
   }
-#else
-  return Status::NotSupported("Cannot load filter policy in LITE mode ", value);
-#endif  // ROCKSDB_LITE
+  return status;
 }
 
 const std::vector<std::string>& BloomLikeFilterPolicy::GetAllFixedImpls() {
   STATIC_AVOID_DESTRUCTION(std::vector<std::string>, 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;
 }

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

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();
 }
 

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;

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) {