From e0f697d2bdbe8633c6828356105b3c6e4654118c Mon Sep 17 00:00:00 2001 From: mrambacher Date: Mon, 27 Sep 2021 07:42:36 -0700 Subject: [PATCH] Make SliceTransform into a Customizable class (#8641) Summary: Made SliceTransform into a Customizable class. Would be nice to write a test that stored and used a custom transform in an SST table. There are a set of tests (DBBlockFliterTest.PrefixExtractor*, SamePrefixTest.InDomainTest, PrefixTest.PrefixAndWholeKeyTest that run the same with or without a SliceTransform/PrefixFilter. Is this expected? Pull Request resolved: https://github.com/facebook/rocksdb/pull/8641 Reviewed By: zhichao-cao Differential Revision: D31142793 Pulled By: mrambacher fbshipit-source-id: bb08672fccbfdc263dcae21f25a62307e1facda1 --- HISTORY.md | 1 + db/db_bloom_filter_test.cc | 30 ++- include/rocksdb/slice_transform.h | 15 +- include/rocksdb/utilities/options_type.h | 2 +- options/cf_options.cc | 22 +- options/customizable_test.cc | 52 +++++ options/options_helper.cc | 71 +----- options/options_helper.h | 5 - options/options_test.cc | 73 +++++- .../block_based/block_based_table_builder.cc | 3 +- table/block_based/block_based_table_reader.cc | 28 ++- table/plain/plain_table_builder.cc | 7 +- table/plain/plain_table_reader.cc | 3 +- util/slice.cc | 219 +++++++++++++++--- 14 files changed, 372 insertions(+), 159 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index a00290102..819a343c8 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -5,6 +5,7 @@ ### New Features ### Public API change * Made SystemClock extend the Customizable class and added a CreateFromString method. Implementations need to be registered with the ObjectRegistry and to implement a Name() method in order to be created via this method. +* Made SliceTransform extend the Customizable class and added a CreateFromString method. Implementations need to be registered with the ObjectRegistry and to implement a Name() method in order to be created via this method. The Capped and Prefixed transform classes return a short name (no length); use GetId for the fully qualified name. ## 6.25.0 (2021-09-20) ### Bug Fixes diff --git a/db/db_bloom_filter_test.cc b/db/db_bloom_filter_test.cc index 882bf2109..b856e0de9 100644 --- a/db/db_bloom_filter_test.cc +++ b/db/db_bloom_filter_test.cc @@ -1820,8 +1820,8 @@ TEST_F(DBBloomFilterTest, DynamicBloomFilterUpperBound) { ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_PREFIX_USEFUL), 0); } ASSERT_OK(dbfull()->SetOptions({{"prefix_extractor", "fixed:5"}})); - ASSERT_EQ(0, strcmp(dbfull()->GetOptions().prefix_extractor->Name(), - "rocksdb.FixedPrefix.5")); + ASSERT_EQ(dbfull()->GetOptions().prefix_extractor->AsString(), + "rocksdb.FixedPrefix.5"); { // BF changed, [abcdxx00, abce) is a valid bound, will trigger BF read Slice upper_bound("abce"); @@ -1940,8 +1940,8 @@ TEST_F(DBBloomFilterTest, DynamicBloomFilterMultipleSST) { ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_PREFIX_CHECKED), 1); ASSERT_OK(dbfull()->SetOptions({{"prefix_extractor", "capped:3"}})); - ASSERT_EQ(0, strcmp(dbfull()->GetOptions().prefix_extractor->Name(), - "rocksdb.CappedPrefix.3")); + ASSERT_EQ(dbfull()->GetOptions().prefix_extractor->AsString(), + "rocksdb.CappedPrefix.3"); read_options.iterate_upper_bound = &upper_bound; std::unique_ptr iter(db_->NewIterator(read_options)); ASSERT_EQ(CountIter(iter, "foo"), 2); @@ -1974,8 +1974,8 @@ TEST_F(DBBloomFilterTest, DynamicBloomFilterMultipleSST) { } ASSERT_OK(dbfull()->SetOptions({{"prefix_extractor", "fixed:2"}})); - ASSERT_EQ(0, strcmp(dbfull()->GetOptions().prefix_extractor->Name(), - "rocksdb.FixedPrefix.2")); + ASSERT_EQ(dbfull()->GetOptions().prefix_extractor->AsString(), + "rocksdb.FixedPrefix.2"); // third SST with fixed:2 BF ASSERT_OK(Put("foo6", "bar6")); ASSERT_OK(Put("foo7", "bar7")); @@ -2022,8 +2022,8 @@ TEST_F(DBBloomFilterTest, DynamicBloomFilterMultipleSST) { ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_PREFIX_USEFUL), 3); } ASSERT_OK(dbfull()->SetOptions({{"prefix_extractor", "capped:3"}})); - ASSERT_EQ(0, strcmp(dbfull()->GetOptions().prefix_extractor->Name(), - "rocksdb.CappedPrefix.3")); + ASSERT_EQ(dbfull()->GetOptions().prefix_extractor->AsString(), + "rocksdb.CappedPrefix.3"); { std::unique_ptr iter_all(db_->NewIterator(read_options)); ASSERT_EQ(CountIter(iter_all, "foo"), 6); @@ -2063,9 +2063,8 @@ TEST_F(DBBloomFilterTest, DynamicBloomFilterNewColumnFamily) { // create a new CF and set prefix_extractor dynamically options.prefix_extractor.reset(NewCappedPrefixTransform(3)); CreateColumnFamilies({"ramen_dojo_" + std::to_string(iteration)}, options); - ASSERT_EQ(0, - strcmp(dbfull()->GetOptions(handles_[2]).prefix_extractor->Name(), - "rocksdb.CappedPrefix.3")); + ASSERT_EQ(dbfull()->GetOptions(handles_[2]).prefix_extractor->AsString(), + "rocksdb.CappedPrefix.3"); ASSERT_OK(Put(2, "foo3", "bar3")); ASSERT_OK(Put(2, "foo4", "bar4")); ASSERT_OK(Put(2, "foo5", "bar5")); @@ -2081,9 +2080,8 @@ TEST_F(DBBloomFilterTest, DynamicBloomFilterNewColumnFamily) { } ASSERT_OK( dbfull()->SetOptions(handles_[2], {{"prefix_extractor", "fixed:2"}})); - ASSERT_EQ(0, - strcmp(dbfull()->GetOptions(handles_[2]).prefix_extractor->Name(), - "rocksdb.FixedPrefix.2")); + ASSERT_EQ(dbfull()->GetOptions(handles_[2]).prefix_extractor->AsString(), + "rocksdb.FixedPrefix.2"); { std::unique_ptr iter( db_->NewIterator(read_options, handles_[2])); @@ -2148,8 +2146,8 @@ TEST_F(DBBloomFilterTest, DynamicBloomFilterOptions) { ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_PREFIX_USEFUL), 0); ASSERT_OK(dbfull()->SetOptions({{"prefix_extractor", "capped:3"}})); - ASSERT_EQ(0, strcmp(dbfull()->GetOptions().prefix_extractor->Name(), - "rocksdb.CappedPrefix.3")); + ASSERT_EQ(dbfull()->GetOptions().prefix_extractor->AsString(), + "rocksdb.CappedPrefix.3"); { std::unique_ptr iter(db_->NewIterator(read_options)); // "fp*" should be skipped diff --git a/include/rocksdb/slice_transform.h b/include/rocksdb/slice_transform.h index 589636744..ac147025a 100644 --- a/include/rocksdb/slice_transform.h +++ b/include/rocksdb/slice_transform.h @@ -14,13 +14,16 @@ #pragma once +#include #include +#include "rocksdb/customizable.h" #include "rocksdb/rocksdb_namespace.h" namespace ROCKSDB_NAMESPACE { class Slice; +struct ConfigOptions; /* * A SliceTransform is a generic pluggable way of transforming one string @@ -28,12 +31,22 @@ class Slice; * to store prefix blooms by setting prefix_extractor in * ColumnFamilyOptions. */ -class SliceTransform { +class SliceTransform : public Customizable { public: virtual ~SliceTransform(){}; // Return the name of this transformation. virtual const char* Name() const = 0; + static const char* Type() { return "SliceTransform"; } + + // Creates and configures a new SliceTransform from the input options and id. + static Status CreateFromString(const ConfigOptions& config_options, + const std::string& id, + std::shared_ptr* result); + + // Returns a string representation of this SliceTransform, representing the ID + // and any additional properties + std::string AsString() const; // Extract a prefix from a specified key. This method is called when // a key is inserted into the db, and the returned slice is used to diff --git a/include/rocksdb/utilities/options_type.h b/include/rocksdb/utilities/options_type.h index 906e2d433..25a07ef49 100644 --- a/include/rocksdb/utilities/options_type.h +++ b/include/rocksdb/utilities/options_type.h @@ -33,7 +33,6 @@ enum class OptionType { kDouble, kCompactionStyle, kCompactionPri, - kSliceTransform, kCompressionType, kCompactionStopStyle, kFilterPolicy, @@ -575,6 +574,7 @@ class OptionTypeInfo { // or if the flags specify allow null. bool CanBeNull() const { return (IsEnabled(OptionTypeFlags::kAllowNull) || + IsEnabled(OptionVerificationType::kByNameAllowNull) || IsEnabled(OptionVerificationType::kByNameAllowFromNull)); } diff --git a/options/cf_options.cc b/options/cf_options.cc index 823f43274..3204f3b95 100644 --- a/options/cf_options.cc +++ b/options/cf_options.cc @@ -360,9 +360,10 @@ static std::unordered_map OptionType::kCompressionType, OptionVerificationType::kNormal, OptionTypeFlags::kMutable}}, {"prefix_extractor", - {offsetof(struct MutableCFOptions, prefix_extractor), - OptionType::kSliceTransform, OptionVerificationType::kByNameAllowNull, - OptionTypeFlags::kMutable}}, + OptionTypeInfo::AsCustomSharedPtr( + offsetof(struct MutableCFOptions, prefix_extractor), + OptionVerificationType::kByNameAllowNull, + (OptionTypeFlags::kMutable | OptionTypeFlags::kAllowNull))}, {"compaction_options_fifo", OptionTypeInfo::Struct( "compaction_options_fifo", &fifo_compaction_options_type_info, @@ -567,10 +568,10 @@ static std::unordered_map }, /* Use the default match function*/ nullptr)}, {"memtable_insert_with_hint_prefix_extractor", - {offset_of( - &ImmutableCFOptions::memtable_insert_with_hint_prefix_extractor), - OptionType::kSliceTransform, OptionVerificationType::kByNameAllowNull, - OptionTypeFlags::kNone}}, + OptionTypeInfo::AsCustomSharedPtr( + offset_of(&ImmutableCFOptions:: + memtable_insert_with_hint_prefix_extractor), + OptionVerificationType::kByNameAllowNull, OptionTypeFlags::kNone)}, {"memtable_factory", {offset_of(&ImmutableCFOptions::memtable_factory), OptionType::kCustomizable, OptionVerificationType::kByName, @@ -935,9 +936,10 @@ void MutableCFOptions::Dump(Logger* log) const { ROCKS_LOG_INFO(log, " inplace_update_num_locks: %" ROCKSDB_PRIszt, inplace_update_num_locks); - ROCKS_LOG_INFO( - log, " prefix_extractor: %s", - prefix_extractor == nullptr ? "nullptr" : prefix_extractor->Name()); + ROCKS_LOG_INFO(log, " prefix_extractor: %s", + prefix_extractor == nullptr + ? "nullptr" + : prefix_extractor->GetId().c_str()); ROCKS_LOG_INFO(log, " disable_auto_compactions: %d", disable_auto_compactions); ROCKS_LOG_INFO(log, " soft_pending_compaction_bytes_limit: %" PRIu64, diff --git a/options/customizable_test.cc b/options/customizable_test.cc index 80b509c9d..470bf7dee 100644 --- a/options/customizable_test.cc +++ b/options/customizable_test.cc @@ -22,6 +22,7 @@ #include "rocksdb/env_encryption.h" #include "rocksdb/flush_block_policy.h" #include "rocksdb/secondary_cache.h" +#include "rocksdb/slice_transform.h" #include "rocksdb/statistics.h" #include "rocksdb/utilities/customizable_util.h" #include "rocksdb/utilities/object_registry.h" @@ -1241,6 +1242,18 @@ class TestFlushBlockPolicyFactory : public FlushBlockPolicyFactory { } }; +class MockSliceTransform : public SliceTransform { + public: + const char* Name() const override { return kClassName(); } + static const char* kClassName() { return "Mock"; } + + Slice Transform(const Slice& /*key*/) const override { return Slice(); } + + bool InDomain(const Slice& /*key*/) const override { return false; } + + bool InRange(const Slice& /*key*/) const override { return false; } +}; + #ifndef ROCKSDB_LITE class MockEncryptionProvider : public EncryptionProvider { public: @@ -1309,6 +1322,14 @@ static int RegisterLocalObjects(ObjectLibrary& library, return guard->get(); }); // Load any locally defined objects here + library.Register( + MockSliceTransform::kClassName(), + [](const std::string& /*uri*/, + std::unique_ptr* guard, + std::string* /* errmsg */) { + guard->reset(new MockSliceTransform()); + return guard->get(); + }); library.Register( TestStatistics::kClassName(), [](const std::string& /*uri*/, std::unique_ptr* guard, @@ -1447,6 +1468,37 @@ TEST_F(LoadCustomizableTest, LoadComparatorTest) { } } +TEST_F(LoadCustomizableTest, LoadSliceTransformFactoryTest) { + std::shared_ptr result; + ASSERT_NOK( + SliceTransform::CreateFromString(config_options_, "Mock", &result)); + ASSERT_OK( + SliceTransform::CreateFromString(config_options_, "fixed:16", &result)); + ASSERT_NE(result.get(), nullptr); + ASSERT_TRUE(result->IsInstanceOf("fixed")); + ASSERT_OK(SliceTransform::CreateFromString( + config_options_, "rocksdb.FixedPrefix.22", &result)); + ASSERT_NE(result.get(), nullptr); + ASSERT_TRUE(result->IsInstanceOf("fixed")); + + ASSERT_OK( + SliceTransform::CreateFromString(config_options_, "capped:16", &result)); + ASSERT_NE(result.get(), nullptr); + ASSERT_TRUE(result->IsInstanceOf("capped")); + + ASSERT_OK(SliceTransform::CreateFromString( + config_options_, "rocksdb.CappedPrefix.11", &result)); + ASSERT_NE(result.get(), nullptr); + ASSERT_TRUE(result->IsInstanceOf("capped")); + + if (RegisterTests("Test")) { + ASSERT_OK( + SliceTransform::CreateFromString(config_options_, "Mock", &result)); + ASSERT_NE(result, nullptr); + ASSERT_STREQ(result->Name(), "Mock"); + } +} + TEST_F(LoadCustomizableTest, LoadStatisticsTest) { std::shared_ptr stats; ASSERT_NOK(Statistics::CreateFromString( diff --git a/options/options_helper.cc b/options/options_helper.cc index 215c2f1ba..668a6caa1 100644 --- a/options/options_helper.cc +++ b/options/options_helper.cc @@ -364,61 +364,6 @@ std::vector GetSupportedDictCompressions() { } #ifndef ROCKSDB_LITE -bool ParseSliceTransformHelper( - const std::string& kFixedPrefixName, const std::string& kCappedPrefixName, - const std::string& value, - std::shared_ptr* slice_transform) { - const char* no_op_name = "rocksdb.Noop"; - size_t no_op_length = strlen(no_op_name); - auto& pe_value = value; - if (pe_value.size() > kFixedPrefixName.size() && - pe_value.compare(0, kFixedPrefixName.size(), kFixedPrefixName) == 0) { - int prefix_length = ParseInt(trim(value.substr(kFixedPrefixName.size()))); - slice_transform->reset(NewFixedPrefixTransform(prefix_length)); - } else if (pe_value.size() > kCappedPrefixName.size() && - pe_value.compare(0, kCappedPrefixName.size(), kCappedPrefixName) == - 0) { - int prefix_length = - ParseInt(trim(pe_value.substr(kCappedPrefixName.size()))); - slice_transform->reset(NewCappedPrefixTransform(prefix_length)); - } else if (pe_value.size() == no_op_length && - pe_value.compare(0, no_op_length, no_op_name) == 0) { - const SliceTransform* no_op_transform = NewNoopTransform(); - slice_transform->reset(no_op_transform); - } else if (value == kNullptrString) { - slice_transform->reset(); - } else { - return false; - } - - return true; -} - -bool ParseSliceTransform( - const std::string& value, - std::shared_ptr* slice_transform) { - // While we normally don't convert the string representation of a - // pointer-typed option into its instance, here we do so for backward - // compatibility as we allow this action in SetOption(). - - // TODO(yhchiang): A possible better place for these serialization / - // deserialization is inside the class definition of pointer-typed - // option itself, but this requires a bigger change of public API. - bool result = - ParseSliceTransformHelper("fixed:", "capped:", value, slice_transform); - if (result) { - return result; - } - result = ParseSliceTransformHelper( - "rocksdb.FixedPrefix.", "rocksdb.CappedPrefix.", value, slice_transform); - if (result) { - return result; - } - // TODO(yhchiang): we can further support other default - // SliceTransforms here. - return false; -} - static bool ParseOptionHelper(void* opt_address, const OptionType& opt_type, const std::string& value) { switch (opt_type) { @@ -466,10 +411,6 @@ static bool ParseOptionHelper(void* opt_address, const OptionType& opt_type, return ParseEnum( compression_type_string_map, value, static_cast(opt_address)); - case OptionType::kSliceTransform: - return ParseSliceTransform( - value, - static_cast*>(opt_address)); case OptionType::kChecksumType: return ParseEnum(checksum_type_string_map, value, static_cast(opt_address)); @@ -554,14 +495,7 @@ bool SerializeSingleOptionHelper(const void* opt_address, return SerializeEnum( compression_type_string_map, *(static_cast(opt_address)), value); - case OptionType::kSliceTransform: { - const auto* slice_transform_ptr = - static_cast*>( - opt_address); - *value = slice_transform_ptr->get() ? slice_transform_ptr->get()->Name() - : kNullptrString; break; - } case OptionType::kFilterPolicy: { const auto* ptr = static_cast*>(opt_address); @@ -1068,6 +1002,7 @@ Status OptionTypeInfo::Serialize(const ConfigOptions& config_options, return serialize_func_(config_options, opt_name, opt_addr, opt_value); } else if (IsCustomizable()) { const Customizable* custom = AsRawPointer(opt_ptr); + opt_value->clear(); if (custom == nullptr) { // We do not have a custom object to serialize. // If the option is not mutable and we are doing only mutable options, @@ -1081,7 +1016,9 @@ Status OptionTypeInfo::Serialize(const ConfigOptions& config_options, } } else if (IsEnabled(OptionTypeFlags::kStringNameOnly) && !config_options.IsDetailed()) { - *opt_value = custom->GetId(); + if (!config_options.mutable_options_only || IsMutable()) { + *opt_value = custom->GetId(); + } } else { ConfigOptions embedded = config_options; embedded.delimiter = ";"; diff --git a/options/options_helper.h b/options/options_helper.h index a16c265ed..147f6b3ee 100644 --- a/options/options_helper.h +++ b/options/options_helper.h @@ -54,11 +54,6 @@ std::unique_ptr CFOptionsAsConfigurable( const ColumnFamilyOptions& opts, const std::unordered_map* opt_map = nullptr); - -bool ParseSliceTransform( - const std::string& value, - std::shared_ptr* slice_transform); - extern Status StringToMap( const std::string& opts_str, std::unordered_map* opts_map); diff --git a/options/options_test.cc b/options/options_test.cc index bc9d8aed9..4de9c282d 100644 --- a/options/options_test.cc +++ b/options/options_test.cc @@ -231,8 +231,7 @@ TEST_F(OptionsTest, GetOptionsFromMapTest) { ASSERT_EQ(new_cf_opt.max_successive_merges, 30U); ASSERT_TRUE(new_cf_opt.prefix_extractor != nullptr); ASSERT_EQ(new_cf_opt.optimize_filters_for_hits, true); - ASSERT_EQ(std::string(new_cf_opt.prefix_extractor->Name()), - "rocksdb.FixedPrefix.31"); + ASSERT_EQ(new_cf_opt.prefix_extractor->AsString(), "rocksdb.FixedPrefix.31"); ASSERT_EQ(new_cf_opt.enable_blob_files, true); ASSERT_EQ(new_cf_opt.min_blob_size, 1ULL << 10); ASSERT_EQ(new_cf_opt.blob_file_size, 1ULL << 30); @@ -456,8 +455,7 @@ TEST_F(OptionsTest, GetColumnFamilyOptionsFromStringTest) { ASSERT_EQ(new_cf_opt.write_buffer_size, 18 * giga); ASSERT_EQ(new_cf_opt.arena_block_size, 19 * giga); ASSERT_TRUE(new_cf_opt.prefix_extractor.get() != nullptr); - std::string prefix_name(new_cf_opt.prefix_extractor->Name()); - ASSERT_EQ(prefix_name, "rocksdb.CappedPrefix.8"); + ASSERT_EQ(new_cf_opt.prefix_extractor->AsString(), "rocksdb.CappedPrefix.8"); // Units (t) ASSERT_OK(GetColumnFamilyOptionsFromString( @@ -2254,8 +2252,7 @@ TEST_F(OptionsOldApiTest, GetOptionsFromMapTest) { ASSERT_EQ(new_cf_opt.max_successive_merges, 30U); ASSERT_TRUE(new_cf_opt.prefix_extractor != nullptr); ASSERT_EQ(new_cf_opt.optimize_filters_for_hits, true); - ASSERT_EQ(std::string(new_cf_opt.prefix_extractor->Name()), - "rocksdb.FixedPrefix.31"); + ASSERT_EQ(new_cf_opt.prefix_extractor->AsString(), "rocksdb.FixedPrefix.31"); ASSERT_EQ(new_cf_opt.enable_blob_files, true); ASSERT_EQ(new_cf_opt.min_blob_size, 1ULL << 10); ASSERT_EQ(new_cf_opt.blob_file_size, 1ULL << 30); @@ -2448,8 +2445,7 @@ TEST_F(OptionsOldApiTest, GetColumnFamilyOptionsFromStringTest) { ASSERT_EQ(new_cf_opt.write_buffer_size, 18 * giga); ASSERT_EQ(new_cf_opt.arena_block_size, 19 * giga); ASSERT_TRUE(new_cf_opt.prefix_extractor.get() != nullptr); - std::string prefix_name(new_cf_opt.prefix_extractor->Name()); - ASSERT_EQ(prefix_name, "rocksdb.CappedPrefix.8"); + ASSERT_EQ(new_cf_opt.prefix_extractor->AsString(), "rocksdb.CappedPrefix.8"); // Units (t) ASSERT_OK(GetColumnFamilyOptionsFromString(base_cf_opt, @@ -2551,6 +2547,67 @@ TEST_F(OptionsOldApiTest, GetColumnFamilyOptionsFromStringTest) { ASSERT_TRUE(new_cf_opt.memtable_factory->IsInstanceOf("SkipListFactory")); } +TEST_F(OptionsTest, SliceTransformCreateFromString) { + std::shared_ptr transform = nullptr; + ConfigOptions config_options; + config_options.ignore_unsupported_options = false; + config_options.ignore_unknown_options = false; + + ASSERT_OK( + SliceTransform::CreateFromString(config_options, "fixed:31", &transform)); + ASSERT_NE(transform, nullptr); + ASSERT_FALSE(transform->IsInstanceOf("capped")); + ASSERT_TRUE(transform->IsInstanceOf("fixed")); + ASSERT_TRUE(transform->IsInstanceOf("rocksdb.FixedPrefix")); + ASSERT_EQ(transform->GetId(), "rocksdb.FixedPrefix.31"); + ASSERT_OK(SliceTransform::CreateFromString( + config_options, "rocksdb.FixedPrefix.42", &transform)); + ASSERT_NE(transform, nullptr); + ASSERT_EQ(transform->GetId(), "rocksdb.FixedPrefix.42"); + + ASSERT_OK(SliceTransform::CreateFromString(config_options, "capped:16", + &transform)); + ASSERT_NE(transform, nullptr); + ASSERT_FALSE(transform->IsInstanceOf("fixed")); + ASSERT_TRUE(transform->IsInstanceOf("capped")); + ASSERT_TRUE(transform->IsInstanceOf("rocksdb.CappedPrefix")); + ASSERT_EQ(transform->GetId(), "rocksdb.CappedPrefix.16"); + ASSERT_OK(SliceTransform::CreateFromString( + config_options, "rocksdb.CappedPrefix.42", &transform)); + ASSERT_NE(transform, nullptr); + ASSERT_EQ(transform->GetId(), "rocksdb.CappedPrefix.42"); + + ASSERT_OK(SliceTransform::CreateFromString(config_options, "rocksdb.Noop", + &transform)); + ASSERT_NE(transform, nullptr); + + ASSERT_NOK(SliceTransform::CreateFromString(config_options, + "fixed:21:invalid", &transform)); + ASSERT_NOK(SliceTransform::CreateFromString(config_options, + "capped:21:invalid", &transform)); + ASSERT_NOK( + SliceTransform::CreateFromString(config_options, "fixed", &transform)); + ASSERT_NOK( + SliceTransform::CreateFromString(config_options, "capped", &transform)); + ASSERT_NOK(SliceTransform::CreateFromString( + config_options, "rocksdb.FixedPrefix:42", &transform)); + ASSERT_NOK(SliceTransform::CreateFromString( + config_options, "rocksdb.CappedPrefix:42", &transform)); + ASSERT_NOK( + SliceTransform::CreateFromString(config_options, "invalid", &transform)); + +#ifndef ROCKSDB_LITE + ASSERT_OK(SliceTransform::CreateFromString( + config_options, "id=rocksdb.CappedPrefix; length=11", &transform)); + ASSERT_NE(transform, nullptr); + ASSERT_EQ(transform->GetId(), "rocksdb.CappedPrefix.11"); + + ASSERT_NOK(SliceTransform::CreateFromString( + config_options, "id=rocksdb.CappedPrefix; length=11; invalid=true", + &transform)); +#endif // ROCKSDB_LITE +} + TEST_F(OptionsOldApiTest, GetBlockBasedTableOptionsFromString) { BlockBasedTableOptions table_opt; BlockBasedTableOptions new_opt; diff --git a/table/block_based/block_based_table_builder.cc b/table/block_based/block_based_table_builder.cc index afac09fc7..0dd864fc3 100644 --- a/table/block_based/block_based_table_builder.cc +++ b/table/block_based/block_based_table_builder.cc @@ -1670,9 +1670,8 @@ void BlockBasedTableBuilder::WritePropertiesBlock( CompressionOptionsToString(rep_->compression_opts); rep_->props.prefix_extractor_name = rep_->moptions.prefix_extractor != nullptr - ? rep_->moptions.prefix_extractor->Name() + ? rep_->moptions.prefix_extractor->AsString() : "nullptr"; - std::string property_collectors_names = "["; for (size_t i = 0; i < rep_->ioptions.table_properties_collector_factories.size(); ++i) { diff --git a/table/block_based/block_based_table_reader.cc b/table/block_based/block_based_table_reader.cc index 277c35a7d..8d4ad1928 100644 --- a/table/block_based/block_based_table_reader.cc +++ b/table/block_based/block_based_table_reader.cc @@ -23,10 +23,10 @@ #include "file/file_util.h" #include "file/random_access_file_reader.h" #include "monitoring/perf_context_imp.h" -#include "options/options_helper.h" #include "port/lang.h" #include "rocksdb/cache.h" #include "rocksdb/comparator.h" +#include "rocksdb/convenience.h" #include "rocksdb/env.h" #include "rocksdb/file_system.h" #include "rocksdb/filter_policy.h" @@ -134,8 +134,7 @@ bool PrefixExtractorChanged(const TableProperties* table_properties, } // prefix_extractor and prefix_extractor_block are both non-empty - if (table_properties->prefix_extractor_name.compare( - prefix_extractor->Name()) != 0) { + if (table_properties->prefix_extractor_name != prefix_extractor->AsString()) { return true; } else { return false; @@ -816,8 +815,19 @@ Status BlockBasedTable::ReadPropertiesBlock( } #ifndef ROCKSDB_LITE if (rep_->table_properties) { - ParseSliceTransform(rep_->table_properties->prefix_extractor_name, - &(rep_->table_prefix_extractor)); + //**TODO: If/When the DBOptions has a registry in it, the ConfigOptions + // will need to use it + ConfigOptions config_options; + Status st = SliceTransform::CreateFromString( + config_options, rep_->table_properties->prefix_extractor_name, + &(rep_->table_prefix_extractor)); + if (!st.ok()) { + //**TODO: Should this be error be returned or swallowed? + ROCKS_LOG_ERROR(rep_->ioptions.logger, + "Failed to create prefix extractor[%s]: %s", + rep_->table_properties->prefix_extractor_name.c_str(), + st.ToString().c_str()); + } } #endif // ROCKSDB_LITE @@ -2223,8 +2233,8 @@ bool BlockBasedTable::FullFilterKeyMayMatch( filter->KeyMayMatch(user_key_without_ts, prefix_extractor, kNotValid, no_io, const_ikey_ptr, get_context, lookup_context); } else if (!read_options.total_order_seek && prefix_extractor && - rep_->table_properties->prefix_extractor_name.compare( - prefix_extractor->Name()) == 0 && + rep_->table_properties->prefix_extractor_name == + prefix_extractor->AsString() && prefix_extractor->InDomain(user_key_without_ts) && !filter->PrefixMayMatch( prefix_extractor->Transform(user_key_without_ts), @@ -2265,8 +2275,8 @@ void BlockBasedTable::FullFilterKeysMayMatch( rep_->level); } } else if (!read_options.total_order_seek && prefix_extractor && - rep_->table_properties->prefix_extractor_name.compare( - prefix_extractor->Name()) == 0) { + rep_->table_properties->prefix_extractor_name == + prefix_extractor->AsString()) { filter->PrefixesMayMatch(range, prefix_extractor, kNotValid, false, lookup_context); RecordTick(rep_->ioptions.stats, BLOOM_FILTER_PREFIX_CHECKED, before_keys); diff --git a/table/plain/plain_table_builder.cc b/table/plain/plain_table_builder.cc index 61a9cec8b..79e5fbfb3 100644 --- a/table/plain/plain_table_builder.cc +++ b/table/plain/plain_table_builder.cc @@ -104,9 +104,10 @@ PlainTableBuilder::PlainTableBuilder( ROCKS_LOG_INFO(ioptions_.logger, "db_host_id property will not be set"); } properties_.orig_file_number = file_number; - properties_.prefix_extractor_name = moptions_.prefix_extractor != nullptr - ? moptions_.prefix_extractor->Name() - : "nullptr"; + properties_.prefix_extractor_name = + moptions_.prefix_extractor != nullptr + ? moptions_.prefix_extractor->AsString() + : "nullptr"; std::string val; PutFixed32(&val, static_cast(encoder_.GetEncodingType())); diff --git a/table/plain/plain_table_reader.cc b/table/plain/plain_table_reader.cc index fbd624268..9a9e21850 100644 --- a/table/plain/plain_table_reader.cc +++ b/table/plain/plain_table_reader.cc @@ -149,8 +149,7 @@ Status PlainTableReader::Open( return Status::InvalidArgument( "Prefix extractor is missing when opening a PlainTable built " "using a prefix extractor"); - } else if (prefix_extractor_in_file.compare(prefix_extractor->Name()) != - 0) { + } else if (prefix_extractor_in_file != prefix_extractor->AsString()) { return Status::InvalidArgument( "Prefix extractor given doesn't match the one used to build " "PlainTable"); diff --git a/util/slice.cc b/util/slice.cc index 6db11cc94..eb3f0e6a0 100644 --- a/util/slice.cc +++ b/util/slice.cc @@ -7,32 +7,47 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. See the AUTHORS file for names of contributors. +#include "rocksdb/slice.h" + +#include + #include + +#include "rocksdb/convenience.h" #include "rocksdb/slice_transform.h" -#include "rocksdb/slice.h" +#include "rocksdb/utilities/object_registry.h" +#include "rocksdb/utilities/options_type.h" #include "util/string_util.h" -#include namespace ROCKSDB_NAMESPACE { namespace { +static std::unordered_map + slice_transform_length_info = { +#ifndef ROCKSDB_LITE + {"length", + {0, OptionType::kSizeT, OptionVerificationType::kNormal, + OptionTypeFlags::kDontSerialize | OptionTypeFlags::kCompareNever}}, +#endif // ROCKSDB_LITE +}; class FixedPrefixTransform : public SliceTransform { private: size_t prefix_len_; - std::string name_; public: - explicit FixedPrefixTransform(size_t prefix_len) - : prefix_len_(prefix_len), - // Note that if any part of the name format changes, it will require - // changes on options_helper in order to make RocksDBOptionsParser work - // for the new change. - // TODO(yhchiang): move serialization / deserializaion code inside - // the class implementation itself. - name_("rocksdb.FixedPrefix." + ToString(prefix_len_)) {} + explicit FixedPrefixTransform(size_t prefix_len) : prefix_len_(prefix_len) { + RegisterOptions(Name(), &prefix_len_, &slice_transform_length_info); + } - const char* Name() const override { return name_.c_str(); } + static const char* kClassName() { return "rocksdb.FixedPrefix"; } + static const char* kNickName() { return "fixed"; } + const char* Name() const override { return kClassName(); } + const char* NickName() const override { return kNickName(); } + + std::string GetId() const override { + return std::string(Name()) + "." + ROCKSDB_NAMESPACE::ToString(prefix_len_); + } Slice Transform(const Slice& src) const override { assert(InDomain(src)); @@ -60,19 +75,19 @@ class FixedPrefixTransform : public SliceTransform { class CappedPrefixTransform : public SliceTransform { private: size_t cap_len_; - std::string name_; public: - explicit CappedPrefixTransform(size_t cap_len) - : cap_len_(cap_len), - // Note that if any part of the name format changes, it will require - // changes on options_helper in order to make RocksDBOptionsParser work - // for the new change. - // TODO(yhchiang): move serialization / deserializaion code inside - // the class implementation itself. - name_("rocksdb.CappedPrefix." + ToString(cap_len_)) {} + explicit CappedPrefixTransform(size_t cap_len) : cap_len_(cap_len) { + RegisterOptions(Name(), &cap_len_, &slice_transform_length_info); + } - const char* Name() const override { return name_.c_str(); } + static const char* kClassName() { return "rocksdb.CappedPrefix"; } + static const char* kNickName() { return "capped"; } + const char* Name() const override { return kClassName(); } + const char* NickName() const override { return kNickName(); } + std::string GetId() const override { + return std::string(Name()) + "." + ROCKSDB_NAMESPACE::ToString(cap_len_); + } Slice Transform(const Slice& src) const override { assert(InDomain(src)); @@ -99,7 +114,8 @@ class NoopTransform : public SliceTransform { public: explicit NoopTransform() { } - const char* Name() const override { return "rocksdb.Noop"; } + static const char* kClassName() { return "rocksdb.Noop"; } + const char* Name() const override { return kClassName(); } Slice Transform(const Slice& src) const override { return src; } @@ -112,6 +128,151 @@ class NoopTransform : public SliceTransform { } }; +} // end namespace + +const SliceTransform* NewFixedPrefixTransform(size_t prefix_len) { + return new FixedPrefixTransform(prefix_len); +} + +const SliceTransform* NewCappedPrefixTransform(size_t cap_len) { + return new CappedPrefixTransform(cap_len); +} + +const SliceTransform* NewNoopTransform() { return new NoopTransform; } + +#ifndef ROCKSDB_LITE +static int RegisterBuiltinSliceTransform(ObjectLibrary& library, + const std::string& /*arg*/) { + library.Register( + NoopTransform::kClassName(), + [](const std::string& /*uri*/, + std::unique_ptr* guard, + std::string* /*errmsg*/) { + guard->reset(NewNoopTransform()); + return guard->get(); + }); + library.Register( + std::string(FixedPrefixTransform::kNickName()) + ":[0-9]+", + [](const std::string& uri, std::unique_ptr* guard, + std::string* /*errmsg*/) { + auto colon = uri.find(":"); + auto len = ParseSizeT(uri.substr(colon + 1)); + guard->reset(NewFixedPrefixTransform(len)); + return guard->get(); + }); + library.Register( + FixedPrefixTransform::kClassName(), + [](const std::string& /*uri*/, + std::unique_ptr* guard, + std::string* /*errmsg*/) { + guard->reset(NewFixedPrefixTransform(0)); + return guard->get(); + }); + library.Register( + std::string(FixedPrefixTransform::kClassName()) + "\\.[0-9]+", + [](const std::string& uri, std::unique_ptr* guard, + std::string* /*errmsg*/) { + auto len = ParseSizeT( + uri.substr(strlen(FixedPrefixTransform::kClassName()) + 1)); + guard->reset(NewFixedPrefixTransform(len)); + return guard->get(); + }); + library.Register( + std::string(CappedPrefixTransform::kNickName()) + ":[0-9]+", + [](const std::string& uri, std::unique_ptr* guard, + std::string* /*errmsg*/) { + auto colon = uri.find(":"); + auto len = ParseSizeT(uri.substr(colon + 1)); + guard->reset(NewCappedPrefixTransform(len)); + return guard->get(); + }); + library.Register( + std::string(CappedPrefixTransform::kClassName()) + "(\\.[0-9]+)?", + [](const std::string& uri, std::unique_ptr* guard, + std::string* /*errmsg*/) { + if (uri == CappedPrefixTransform::kClassName()) { + guard->reset(NewCappedPrefixTransform(0)); + } else { // Length + "." + auto len = ParseSizeT( + uri.substr(strlen(CappedPrefixTransform::kClassName()) + 1)); + guard->reset(NewCappedPrefixTransform(len)); + } + return guard->get(); + }); + return 5; +} +#endif // ROCKSDB_LITE + +Status SliceTransform::CreateFromString( + const ConfigOptions& config_options, const std::string& value, + std::shared_ptr* result) { +#ifndef ROCKSDB_LITE + static std::once_flag once; + std::call_once(once, [&]() { + RegisterBuiltinSliceTransform(*(ObjectLibrary::Default().get()), ""); + }); +#endif // ROCKSDB_LITE + std::string id; + std::unordered_map opt_map; + Status status = Customizable::GetOptionsMap(config_options, result->get(), + value, &id, &opt_map); + if (!status.ok()) { // GetOptionsMap failed + return status; + } +#ifndef ROCKSDB_LITE + status = config_options.registry->NewSharedObject(id, result); +#else + auto Matches = [](const std::string& input, size_t size, const char* pattern, + char sep) { + auto plen = strlen(pattern); + return (size > plen + 2 && input[plen] == sep && + StartsWith(input, pattern)); + }; + + auto size = id.size(); + if (id == NoopTransform::kClassName()) { + result->reset(NewNoopTransform()); + } else if (Matches(id, size, FixedPrefixTransform::kNickName(), ':')) { + auto fixed = strlen(FixedPrefixTransform::kNickName()); + auto len = ParseSizeT(id.substr(fixed + 1)); + result->reset(NewFixedPrefixTransform(len)); + } else if (Matches(id, size, CappedPrefixTransform::kNickName(), ':')) { + auto capped = strlen(CappedPrefixTransform::kNickName()); + auto len = ParseSizeT(id.substr(capped + 1)); + result->reset(NewCappedPrefixTransform(len)); + } else if (Matches(id, size, CappedPrefixTransform::kClassName(), '.')) { + auto capped = strlen(CappedPrefixTransform::kClassName()); + auto len = ParseSizeT(id.substr(capped + 1)); + result->reset(NewCappedPrefixTransform(len)); + } else if (Matches(id, size, FixedPrefixTransform::kClassName(), '.')) { + auto fixed = strlen(FixedPrefixTransform::kClassName()); + auto len = ParseSizeT(id.substr(fixed + 1)); + result->reset(NewFixedPrefixTransform(len)); + } else { + status = Status::NotSupported("Cannot load object in LITE mode ", id); + } +#endif // ROCKSDB_LITE + if (!status.ok()) { + if (config_options.ignore_unsupported_options && status.IsNotSupported()) { + return Status::OK(); + } else { + return status; + } + } else if (result->get() != nullptr) { + SliceTransform* transform = const_cast(result->get()); + status = transform->ConfigureFromMap(config_options, opt_map); + } + return status; +} // namespace ROCKSDB_NAMESPACE + +std::string SliceTransform::AsString() const { +#ifndef ROCKSDB_LITE + ConfigOptions config_options; + config_options.delimiter = ";"; + return ToString(config_options); +#else + return GetId(); +#endif // ROCKSDB_LITE } // 2 small internal utility functions, for efficient hex conversions @@ -197,18 +358,6 @@ bool Slice::DecodeHex(std::string* result) const { return true; } -const SliceTransform* NewFixedPrefixTransform(size_t prefix_len) { - return new FixedPrefixTransform(prefix_len); -} - -const SliceTransform* NewCappedPrefixTransform(size_t cap_len) { - return new CappedPrefixTransform(cap_len); -} - -const SliceTransform* NewNoopTransform() { - return new NoopTransform; -} - PinnableSlice::PinnableSlice(PinnableSlice&& other) { *this = std::move(other); }