From 1d6dbfb8b73cb435b3548cfa12a624a6e8ae2c28 Mon Sep 17 00:00:00 2001 From: Peter Dillinger Date: Fri, 2 Feb 2024 14:14:43 -0800 Subject: [PATCH] Rename IntTblPropCollector -> InternalTblPropColl (#12320) Summary: I've always found this name difficult to read, because it sounds like it's for collecting int(eger) table properties. I'm fixing this now to set up for a change that I have stubbed out in the public API (table_properties.h): a new adapter function `TablePropertiesCollector::AsInternal()` that allows RocksDB-provided TablePropertiesCollectors (such as CompactOnDeletionCollector) to implement the easier-to-upgrade internal interface while still (superficially) implementing the public interface. In addition to added flexibility, this should be a performance improvement as the adapter class UserKeyTablePropertiesCollector can be avoided for such cases where a RocksDB-provided collector is used (AsInternal() returns non-nullptr). table_properties.h is the only file with changes that aren't simple find-replace renaming. Pull Request resolved: https://github.com/facebook/rocksdb/pull/12320 Test Plan: existing tests, CI Reviewed By: ajkr Differential Revision: D53336945 Pulled By: pdillinger fbshipit-source-id: 02535bcb30bbfb00e29e8478af62e5dad50a63b8 --- db/column_family.cc | 10 ++-- db/column_family.h | 12 ++--- db/compaction/compaction_job.cc | 2 +- db/compaction/compaction_job_test.cc | 2 +- db/db_impl/db_impl_open.cc | 2 +- db/flush_job.cc | 2 +- db/repair.cc | 2 +- db/table_properties_collector.h | 22 ++++---- db/table_properties_collector_test.cc | 30 +++++------ db/version_set_test.cc | 4 +- include/rocksdb/table_properties.h | 5 ++ .../block_based/block_based_table_builder.cc | 12 ++--- .../block_based_table_reader_test.cc | 2 +- .../block_based/data_block_hash_index_test.cc | 4 +- table/block_fetcher_test.cc | 2 +- table/meta_blocks.cc | 6 +-- table/meta_blocks.h | 6 +-- table/plain/plain_table_builder.cc | 10 ++-- table/plain/plain_table_builder.h | 4 +- table/plain/plain_table_factory.cc | 2 +- table/sst_file_dumper.cc | 2 +- table/sst_file_writer.cc | 8 +-- table/sst_file_writer_collectors.h | 6 +-- table/table_builder.h | 6 +-- table/table_reader_bench.cc | 4 +- table/table_test.cc | 52 +++++++++---------- tools/sst_dump_test.cc | 4 +- 27 files changed, 114 insertions(+), 109 deletions(-) diff --git a/db/column_family.cc b/db/column_family.cc index 94ca034ef9..1bdd8cf3d1 100644 --- a/db/column_family.cc +++ b/db/column_family.cc @@ -96,16 +96,16 @@ const Comparator* ColumnFamilyHandleImpl::GetComparator() const { return cfd()->user_comparator(); } -void GetIntTblPropCollectorFactory( +void GetInternalTblPropCollFactory( const ImmutableCFOptions& ioptions, - IntTblPropCollectorFactories* int_tbl_prop_collector_factories) { - assert(int_tbl_prop_collector_factories); + InternalTblPropCollFactories* internal_tbl_prop_coll_factories) { + assert(internal_tbl_prop_coll_factories); auto& collector_factories = ioptions.table_properties_collector_factories; for (size_t i = 0; i < ioptions.table_properties_collector_factories.size(); ++i) { assert(collector_factories[i]); - int_tbl_prop_collector_factories->emplace_back( + internal_tbl_prop_coll_factories->emplace_back( new UserKeyTablePropertiesCollectorFactory(collector_factories[i])); } } @@ -572,7 +572,7 @@ ColumnFamilyData::ColumnFamilyData( Ref(); // Convert user defined table properties collector factories to internal ones. - GetIntTblPropCollectorFactory(ioptions_, &int_tbl_prop_collector_factories_); + GetInternalTblPropCollFactory(ioptions_, &internal_tbl_prop_coll_factories_); // if _dummy_versions is nullptr, then this is a dummy column family. if (_dummy_versions != nullptr) { diff --git a/db/column_family.h b/db/column_family.h index 30d7ad0bca..ecad83b0b5 100644 --- a/db/column_family.h +++ b/db/column_family.h @@ -261,11 +261,11 @@ Status CheckCFPathsSupported(const DBOptions& db_options, ColumnFamilyOptions SanitizeOptions(const ImmutableDBOptions& db_options, const ColumnFamilyOptions& src); // Wrap user defined table properties collector factories `from cf_options` -// into internal ones in int_tbl_prop_collector_factories. Add a system internal +// into internal ones in internal_tbl_prop_coll_factories. Add a system internal // one too. -void GetIntTblPropCollectorFactory( +void GetInternalTblPropCollFactory( const ImmutableCFOptions& ioptions, - IntTblPropCollectorFactories* int_tbl_prop_collector_factories); + InternalTblPropCollFactories* internal_tbl_prop_coll_factories); class ColumnFamilySet; @@ -429,8 +429,8 @@ class ColumnFamilyData { return internal_comparator_; } - const IntTblPropCollectorFactories* int_tbl_prop_collector_factories() const { - return &int_tbl_prop_collector_factories_; + const InternalTblPropCollFactories* internal_tbl_prop_coll_factories() const { + return &internal_tbl_prop_coll_factories_; } SuperVersion* GetSuperVersion() { return super_version_; } @@ -573,7 +573,7 @@ class ColumnFamilyData { std::atomic dropped_; // true if client dropped it const InternalKeyComparator internal_comparator_; - IntTblPropCollectorFactories int_tbl_prop_collector_factories_; + InternalTblPropCollFactories internal_tbl_prop_coll_factories_; const ColumnFamilyOptions initial_cf_options_; const ImmutableOptions ioptions_; diff --git a/db/compaction/compaction_job.cc b/db/compaction/compaction_job.cc index 867f8090dc..7464bd6fa8 100644 --- a/db/compaction/compaction_job.cc +++ b/db/compaction/compaction_job.cc @@ -1969,7 +1969,7 @@ Status CompactionJob::OpenCompactionOutputFile(SubcompactionState* sub_compact, TableBuilderOptions tboptions( *cfd->ioptions(), *(sub_compact->compaction->mutable_cf_options()), read_options, write_options, cfd->internal_comparator(), - cfd->int_tbl_prop_collector_factories(), + cfd->internal_tbl_prop_coll_factories(), sub_compact->compaction->output_compression(), sub_compact->compaction->output_compression_opts(), cfd->GetID(), cfd->GetName(), sub_compact->compaction->output_level(), diff --git a/db/compaction/compaction_job_test.cc b/db/compaction/compaction_job_test.cc index bd805358eb..ef38831013 100644 --- a/db/compaction/compaction_job_test.cc +++ b/db/compaction/compaction_job_test.cc @@ -302,7 +302,7 @@ class CompactionJobTestBase : public testing::Test { TableBuilderOptions(*cfd_->ioptions(), mutable_cf_options_, read_options, write_options, cfd_->internal_comparator(), - cfd_->int_tbl_prop_collector_factories(), + cfd_->internal_tbl_prop_coll_factories(), CompressionType::kNoCompression, CompressionOptions(), 0 /* column_family_id */, kDefaultColumnFamilyName, -1 /* level */), diff --git a/db/db_impl/db_impl_open.cc b/db/db_impl/db_impl_open.cc index 4389118621..9417721318 100644 --- a/db/db_impl/db_impl_open.cc +++ b/db/db_impl/db_impl_open.cc @@ -1682,7 +1682,7 @@ Status DBImpl::WriteLevel0TableForRecovery(int job_id, ColumnFamilyData* cfd, const WriteOptions write_option(Env::IO_HIGH, Env::IOActivity::kDBOpen); TableBuilderOptions tboptions( *cfd->ioptions(), mutable_cf_options, read_option, write_option, - cfd->internal_comparator(), cfd->int_tbl_prop_collector_factories(), + cfd->internal_comparator(), cfd->internal_tbl_prop_coll_factories(), GetCompressionFlush(*cfd->ioptions(), mutable_cf_options), mutable_cf_options.compression_opts, cfd->GetID(), cfd->GetName(), 0 /* level */, false /* is_bottommost */, diff --git a/db/flush_job.cc b/db/flush_job.cc index d31419deec..393d710089 100644 --- a/db/flush_job.cc +++ b/db/flush_job.cc @@ -965,7 +965,7 @@ Status FlushJob::WriteLevel0Table() { const WriteOptions write_options(io_priority, Env::IOActivity::kFlush); TableBuilderOptions tboptions( *cfd_->ioptions(), mutable_cf_options_, read_options, write_options, - cfd_->internal_comparator(), cfd_->int_tbl_prop_collector_factories(), + cfd_->internal_comparator(), cfd_->internal_tbl_prop_coll_factories(), output_compression_, mutable_cf_options_.compression_opts, cfd_->GetID(), cfd_->GetName(), 0 /* level */, false /* is_bottommost */, TableFileCreationReason::kFlush, diff --git a/db/repair.cc b/db/repair.cc index bf409e22ac..789454d361 100644 --- a/db/repair.cc +++ b/db/repair.cc @@ -468,7 +468,7 @@ class Repairer { TableBuilderOptions tboptions( *cfd->ioptions(), *cfd->GetLatestMutableCFOptions(), read_options, write_option, cfd->internal_comparator(), - cfd->int_tbl_prop_collector_factories(), kNoCompression, + cfd->internal_tbl_prop_coll_factories(), kNoCompression, default_compression, cfd->GetID(), cfd->GetName(), -1 /* level */, false /* is_bottommost */, TableFileCreationReason::kRecovery, 0 /* oldest_key_time */, 0 /* file_creation_time */, diff --git a/db/table_properties_collector.h b/db/table_properties_collector.h index ea5d16c780..53aff51cba 100644 --- a/db/table_properties_collector.h +++ b/db/table_properties_collector.h @@ -17,9 +17,9 @@ namespace ROCKSDB_NAMESPACE { // Base class for internal table properties collector. -class IntTblPropCollector { +class InternalTblPropColl { public: - virtual ~IntTblPropCollector() {} + virtual ~InternalTblPropColl() {} virtual Status Finish(UserCollectedProperties* properties) = 0; virtual const char* Name() const = 0; @@ -39,26 +39,26 @@ class IntTblPropCollector { }; // Factory for internal table properties collector. -class IntTblPropCollectorFactory { +class InternalTblPropCollFactory { public: - virtual ~IntTblPropCollectorFactory() {} + virtual ~InternalTblPropCollFactory() {} // has to be thread-safe - virtual IntTblPropCollector* CreateIntTblPropCollector( + virtual InternalTblPropColl* CreateInternalTblPropColl( uint32_t column_family_id, int level_at_creation) = 0; // The name of the properties collector can be used for debugging purpose. virtual const char* Name() const = 0; }; -using IntTblPropCollectorFactories = - std::vector>; +using InternalTblPropCollFactories = + std::vector>; // When rocksdb creates a new table, it will encode all "user keys" into // "internal keys", which contains meta information of a given entry. // // This class extracts user key from the encoded internal key when Add() is // invoked. -class UserKeyTablePropertiesCollector : public IntTblPropCollector { +class UserKeyTablePropertiesCollector : public InternalTblPropColl { public: // transfer of ownership explicit UserKeyTablePropertiesCollector(TablePropertiesCollector* collector) @@ -86,12 +86,12 @@ class UserKeyTablePropertiesCollector : public IntTblPropCollector { }; class UserKeyTablePropertiesCollectorFactory - : public IntTblPropCollectorFactory { + : public InternalTblPropCollFactory { public: explicit UserKeyTablePropertiesCollectorFactory( std::shared_ptr user_collector_factory) : user_collector_factory_(user_collector_factory) {} - IntTblPropCollector* CreateIntTblPropCollector( + InternalTblPropColl* CreateInternalTblPropColl( uint32_t column_family_id, int level_at_creation) override { TablePropertiesCollectorFactory::Context context; context.column_family_id = column_family_id; @@ -116,7 +116,7 @@ class UserKeyTablePropertiesCollectorFactory // internal key when Add() is invoked. // // @param cmp the user comparator to compare the timestamps in internal key. -class TimestampTablePropertiesCollector : public IntTblPropCollector { +class TimestampTablePropertiesCollector : public InternalTblPropColl { public: explicit TimestampTablePropertiesCollector(const Comparator* cmp) : cmp_(cmp), diff --git a/db/table_properties_collector_test.cc b/db/table_properties_collector_test.cc index a8fff53d0e..a10ebdc24d 100644 --- a/db/table_properties_collector_test.cc +++ b/db/table_properties_collector_test.cc @@ -46,7 +46,7 @@ void MakeBuilder( const Options& options, const ImmutableOptions& ioptions, const MutableCFOptions& moptions, const InternalKeyComparator& internal_comparator, - const IntTblPropCollectorFactories* int_tbl_prop_collector_factories, + const InternalTblPropCollFactories* internal_tbl_prop_coll_factories, std::unique_ptr* writable, std::unique_ptr* builder) { std::unique_ptr wf(new test::StringSink); @@ -56,7 +56,7 @@ void MakeBuilder( const WriteOptions write_options; TableBuilderOptions tboptions( ioptions, moptions, read_options, write_options, internal_comparator, - int_tbl_prop_collector_factories, options.compression, + internal_tbl_prop_coll_factories, options.compression, options.compression_opts, kTestColumnFamilyId, kTestColumnFamilyName, kTestLevel); builder->reset(NewTableBuilder(tboptions, writable->get())); @@ -158,7 +158,7 @@ class RegularKeysStartWithABackwardCompatible uint32_t count_ = 0; }; -class RegularKeysStartWithAInternal : public IntTblPropCollector { +class RegularKeysStartWithAInternal : public InternalTblPropColl { public: const char* Name() const override { return "RegularKeysStartWithA"; } @@ -193,7 +193,7 @@ class RegularKeysStartWithAInternal : public IntTblPropCollector { uint32_t count_ = 0; }; -class RegularKeysStartWithAFactory : public IntTblPropCollectorFactory, +class RegularKeysStartWithAFactory : public InternalTblPropCollFactory, public TablePropertiesCollectorFactory { public: explicit RegularKeysStartWithAFactory(bool backward_mode) @@ -208,7 +208,7 @@ class RegularKeysStartWithAFactory : public IntTblPropCollectorFactory, return new RegularKeysStartWithABackwardCompatible(); } } - IntTblPropCollector* CreateIntTblPropCollector( + InternalTblPropColl* CreateInternalTblPropColl( uint32_t /*column_family_id*/, int /* level_at_creation */) override { return new RegularKeysStartWithAInternal(); } @@ -244,7 +244,7 @@ class FlushBlockEveryThreePolicyFactory : public FlushBlockPolicyFactory { namespace { void TestCustomizedTablePropertiesCollector( - bool backward_mode, uint64_t magic_number, bool test_int_tbl_prop_collector, + bool backward_mode, uint64_t magic_number, bool test_internal_tbl_prop_coll, const Options& options, const InternalKeyComparator& internal_comparator) { // make sure the entries will be inserted with order. std::map, std::string> kvs = { @@ -265,15 +265,15 @@ void TestCustomizedTablePropertiesCollector( std::unique_ptr writer; const ImmutableOptions ioptions(options); const MutableCFOptions moptions(options); - IntTblPropCollectorFactories int_tbl_prop_collector_factories; - if (test_int_tbl_prop_collector) { - int_tbl_prop_collector_factories.emplace_back( + InternalTblPropCollFactories internal_tbl_prop_coll_factories; + if (test_internal_tbl_prop_coll) { + internal_tbl_prop_coll_factories.emplace_back( new RegularKeysStartWithAFactory(backward_mode)); } else { - GetIntTblPropCollectorFactory(ioptions, &int_tbl_prop_collector_factories); + GetInternalTblPropCollFactory(ioptions, &internal_tbl_prop_coll_factories); } MakeBuilder(options, ioptions, moptions, internal_comparator, - &int_tbl_prop_collector_factories, &writer, &builder); + &internal_tbl_prop_coll_factories, &writer, &builder); SequenceNumber seqNum = 0U; for (const auto& kv : kvs) { @@ -308,7 +308,7 @@ void TestCustomizedTablePropertiesCollector( ASSERT_TRUE(GetVarint32(&key, &starts_with_A)); ASSERT_EQ(3u, starts_with_A); - if (!backward_mode && !test_int_tbl_prop_collector) { + if (!backward_mode && !test_internal_tbl_prop_coll) { uint32_t num_puts; ASSERT_NE(user_collected.find("NumPuts"), user_collected.end()); Slice key_puts(user_collected.at("NumPuts")); @@ -392,7 +392,7 @@ void TestInternalKeyPropertiesCollector( Options options; test::PlainInternalKeyComparator pikc(options.comparator); - IntTblPropCollectorFactories int_tbl_prop_collector_factories; + InternalTblPropCollFactories internal_tbl_prop_coll_factories; options.table_factory = table_factory; if (sanitized) { options.table_properties_collector_factories.emplace_back( @@ -406,7 +406,7 @@ void TestInternalKeyPropertiesCollector( options = SanitizeOptions("db", // just a place holder options); ImmutableOptions ioptions(options); - GetIntTblPropCollectorFactory(ioptions, &int_tbl_prop_collector_factories); + GetInternalTblPropCollFactory(ioptions, &internal_tbl_prop_coll_factories); options.comparator = comparator; } const ImmutableOptions ioptions(options); @@ -414,7 +414,7 @@ void TestInternalKeyPropertiesCollector( for (int iter = 0; iter < 2; ++iter) { MakeBuilder(options, ioptions, moptions, pikc, - &int_tbl_prop_collector_factories, &writable, &builder); + &internal_tbl_prop_coll_factories, &writable, &builder); for (const auto& k : keys) { builder->Add(k.Encode(), "val"); } diff --git a/db/version_set_test.cc b/db/version_set_test.cc index b16ffd0359..344576164d 100644 --- a/db/version_set_test.cc +++ b/db/version_set_test.cc @@ -3494,7 +3494,7 @@ class VersionSetTestMissingFiles : public VersionSetTestBase, ASSERT_OK(s); std::unique_ptr fwriter(new WritableFileWriter( std::move(file), fname, FileOptions(), env_->GetSystemClock().get())); - IntTblPropCollectorFactories int_tbl_prop_collector_factories; + InternalTblPropCollFactories internal_tbl_prop_coll_factories; const ReadOptions read_options; const WriteOptions write_options; @@ -3502,7 +3502,7 @@ class VersionSetTestMissingFiles : public VersionSetTestBase, TableBuilderOptions( immutable_options_, mutable_cf_options_, read_options, write_options, *internal_comparator_, - &int_tbl_prop_collector_factories, kNoCompression, + &internal_tbl_prop_coll_factories, kNoCompression, CompressionOptions(), TablePropertiesCollectorFactory::Context::kUnknownColumnFamily, info.column_family, info.level), diff --git a/include/rocksdb/table_properties.h b/include/rocksdb/table_properties.h index 429d8bfc52..f444275b96 100644 --- a/include/rocksdb/table_properties.h +++ b/include/rocksdb/table_properties.h @@ -16,6 +16,8 @@ namespace ROCKSDB_NAMESPACE { +class InternalTblPropColl; + // -- Table Properties // Other than basic table properties, each table may also have the user // collected properties. @@ -138,6 +140,9 @@ class TablePropertiesCollector { // EXPERIMENTAL Return whether the output file should be further compacted virtual bool NeedCompact() const { return false; } + + // For internal use only. + virtual InternalTblPropColl* AsInternal() { return nullptr; } }; // Constructs TablePropertiesCollector instances for each table file creation. diff --git a/table/block_based/block_based_table_builder.cc b/table/block_based/block_based_table_builder.cc index 3f9c15a981..7640e8fc27 100644 --- a/table/block_based/block_based_table_builder.cc +++ b/table/block_based/block_based_table_builder.cc @@ -209,7 +209,7 @@ const uint64_t kLegacyBlockBasedTableMagicNumber = 0xdb4775248b80fb57ull; // But in the foreseeable future, we will add more and more properties that are // specific to block-based table. class BlockBasedTableBuilder::BlockBasedTablePropertiesCollector - : public IntTblPropCollector { + : public InternalTblPropColl { public: explicit BlockBasedTablePropertiesCollector( BlockBasedTableOptions::IndexType index_type, bool whole_key_filtering, @@ -353,7 +353,7 @@ struct BlockBasedTableBuilder::Rep { std::string compressed_output; std::unique_ptr flush_block_policy; - std::vector> table_properties_collectors; + std::vector> table_properties_collectors; std::unique_ptr pc_rep; BlockCreateContext create_context; @@ -575,12 +575,12 @@ struct BlockBasedTableBuilder::Rep { persist_user_defined_timestamps)); } - assert(tbo.int_tbl_prop_collector_factories); - for (auto& factory : *tbo.int_tbl_prop_collector_factories) { + assert(tbo.internal_tbl_prop_coll_factories); + for (auto& factory : *tbo.internal_tbl_prop_coll_factories) { assert(factory); - std::unique_ptr collector{ - factory->CreateIntTblPropCollector(tbo.column_family_id, + std::unique_ptr collector{ + factory->CreateInternalTblPropColl(tbo.column_family_id, tbo.level_at_creation)}; if (collector) { table_properties_collectors.emplace_back(std::move(collector)); diff --git a/table/block_based/block_based_table_reader_test.cc b/table/block_based/block_based_table_reader_test.cc index 38495ce969..bf09ac7198 100644 --- a/table/block_based/block_based_table_reader_test.cc +++ b/table/block_based/block_based_table_reader_test.cc @@ -133,7 +133,7 @@ class BlockBasedTableReaderBaseTest : public testing::Test { // as each block's size. compression_opts.max_dict_bytes = compression_dict_bytes; compression_opts.max_dict_buffer_bytes = compression_dict_bytes; - IntTblPropCollectorFactories factories; + InternalTblPropCollFactories factories; const ReadOptions read_options; const WriteOptions write_options; std::unique_ptr table_builder( diff --git a/table/block_based/data_block_hash_index_test.cc b/table/block_based/data_block_hash_index_test.cc index b4ccfce44b..d7bee16751 100644 --- a/table/block_based/data_block_hash_index_test.cc +++ b/table/block_based/data_block_hash_index_test.cc @@ -551,14 +551,14 @@ void TestBoundary(InternalKey& ik1, std::string& v1, InternalKey& ik2, file_writer.reset( new WritableFileWriter(std::move(f), "" /* don't care */, FileOptions())); std::unique_ptr builder; - IntTblPropCollectorFactories int_tbl_prop_collector_factories; + InternalTblPropCollFactories internal_tbl_prop_coll_factories; std::string column_family_name; const ReadOptions read_options; const WriteOptions write_options; builder.reset(ioptions.table_factory->NewTableBuilder( TableBuilderOptions( ioptions, moptions, read_options, write_options, internal_comparator, - &int_tbl_prop_collector_factories, options.compression, + &internal_tbl_prop_coll_factories, options.compression, CompressionOptions(), TablePropertiesCollectorFactory::Context::kUnknownColumnFamily, column_family_name, level_), diff --git a/table/block_fetcher_test.cc b/table/block_fetcher_test.cc index 95a0255743..10b81344ca 100644 --- a/table/block_fetcher_test.cc +++ b/table/block_fetcher_test.cc @@ -76,7 +76,7 @@ class BlockFetcherTest : public testing::Test { InternalKeyComparator comparator(options_.comparator); ColumnFamilyOptions cf_options(options_); MutableCFOptions moptions(cf_options); - IntTblPropCollectorFactories factories; + InternalTblPropCollFactories factories; const ReadOptions read_options; const WriteOptions write_options; std::unique_ptr table_builder(table_factory_.NewTableBuilder( diff --git a/table/meta_blocks.cc b/table/meta_blocks.cc index 2cbaacec08..55f5935b11 100644 --- a/table/meta_blocks.cc +++ b/table/meta_blocks.cc @@ -186,7 +186,7 @@ void LogPropertiesCollectionError(Logger* info_log, const std::string& method, bool NotifyCollectTableCollectorsOnAdd( const Slice& key, const Slice& value, uint64_t file_size, - const std::vector>& collectors, + const std::vector>& collectors, Logger* info_log) { bool all_succeeded = true; for (auto& collector : collectors) { @@ -201,7 +201,7 @@ bool NotifyCollectTableCollectorsOnAdd( } void NotifyCollectTableCollectorsOnBlockAdd( - const std::vector>& collectors, + const std::vector>& collectors, const uint64_t block_uncomp_bytes, const uint64_t block_compressed_bytes_fast, const uint64_t block_compressed_bytes_slow) { @@ -212,7 +212,7 @@ void NotifyCollectTableCollectorsOnBlockAdd( } bool NotifyCollectTableCollectorsOnFinish( - const std::vector>& collectors, + const std::vector>& collectors, Logger* info_log, PropertyBlockBuilder* builder, UserCollectedProperties& user_collected_properties, UserCollectedProperties& readable_properties) { diff --git a/table/meta_blocks.h b/table/meta_blocks.h index 0a404dc9cf..3d1edb5018 100644 --- a/table/meta_blocks.h +++ b/table/meta_blocks.h @@ -88,11 +88,11 @@ void LogPropertiesCollectionError(Logger* info_log, const std::string& method, // property collectors. bool NotifyCollectTableCollectorsOnAdd( const Slice& key, const Slice& value, uint64_t file_size, - const std::vector>& collectors, + const std::vector>& collectors, Logger* info_log); void NotifyCollectTableCollectorsOnBlockAdd( - const std::vector>& collectors, + const std::vector>& collectors, uint64_t block_uncomp_bytes, uint64_t block_compressed_bytes_fast, uint64_t block_compressed_bytes_slow); @@ -101,7 +101,7 @@ void NotifyCollectTableCollectorsOnBlockAdd( // It will also populate `user_collected_properties` and `readable_properties` // with the collected properties. bool NotifyCollectTableCollectorsOnFinish( - const std::vector>& collectors, + const std::vector>& collectors, Logger* info_log, PropertyBlockBuilder* builder, UserCollectedProperties& user_collected_properties, UserCollectedProperties& readable_properties); diff --git a/table/plain/plain_table_builder.cc b/table/plain/plain_table_builder.cc index 9ba54b3b27..f0443bd945 100644 --- a/table/plain/plain_table_builder.cc +++ b/table/plain/plain_table_builder.cc @@ -57,7 +57,7 @@ const uint64_t kLegacyPlainTableMagicNumber = 0x4f3418eb7a8f13b8ull; PlainTableBuilder::PlainTableBuilder( const ImmutableOptions& ioptions, const MutableCFOptions& moptions, - const IntTblPropCollectorFactories* int_tbl_prop_collector_factories, + const InternalTblPropCollFactories* internal_tbl_prop_coll_factories, uint32_t column_family_id, int level_at_creation, WritableFileWriter* file, uint32_t user_key_len, EncodingType encoding_type, size_t index_sparseness, uint32_t bloom_bits_per_key, const std::string& column_family_name, @@ -114,12 +114,12 @@ PlainTableBuilder::PlainTableBuilder( properties_ .user_collected_properties[PlainTablePropertyNames::kEncodingType] = val; - assert(int_tbl_prop_collector_factories); - for (auto& factory : *int_tbl_prop_collector_factories) { + assert(internal_tbl_prop_coll_factories); + for (auto& factory : *internal_tbl_prop_coll_factories) { assert(factory); - std::unique_ptr collector{ - factory->CreateIntTblPropCollector(column_family_id, + std::unique_ptr collector{ + factory->CreateInternalTblPropColl(column_family_id, level_at_creation)}; if (collector) { table_properties_collectors_.emplace_back(std::move(collector)); diff --git a/table/plain/plain_table_builder.h b/table/plain/plain_table_builder.h index 77f3c7acb6..30e9558628 100644 --- a/table/plain/plain_table_builder.h +++ b/table/plain/plain_table_builder.h @@ -39,7 +39,7 @@ class PlainTableBuilder : public TableBuilder { // that the caller does not know which level the output file will reside. PlainTableBuilder( const ImmutableOptions& ioptions, const MutableCFOptions& moptions, - const IntTblPropCollectorFactories* int_tbl_prop_collector_factories, + const InternalTblPropCollFactories* internal_tbl_prop_coll_factories, uint32_t column_family_id, int level_at_creation, WritableFileWriter* file, uint32_t user_key_size, EncodingType encoding_type, size_t index_sparseness, @@ -102,7 +102,7 @@ class PlainTableBuilder : public TableBuilder { Arena arena_; const ImmutableOptions& ioptions_; const MutableCFOptions& moptions_; - std::vector> + std::vector> table_properties_collectors_; BloomBlockBuilder bloom_block_; diff --git a/table/plain/plain_table_factory.cc b/table/plain/plain_table_factory.cc index e211974d17..7d01b07f30 100644 --- a/table/plain/plain_table_factory.cc +++ b/table/plain/plain_table_factory.cc @@ -77,7 +77,7 @@ TableBuilder* PlainTableFactory::NewTableBuilder( // return new PlainTableBuilder( table_builder_options.ioptions, table_builder_options.moptions, - table_builder_options.int_tbl_prop_collector_factories, + table_builder_options.internal_tbl_prop_coll_factories, table_builder_options.column_family_id, table_builder_options.level_at_creation, file, table_options_.user_key_len, table_options_.encoding_type, diff --git a/table/sst_file_dumper.cc b/table/sst_file_dumper.cc index 72060fd1b2..348a68b5a5 100644 --- a/table/sst_file_dumper.cc +++ b/table/sst_file_dumper.cc @@ -304,7 +304,7 @@ Status SstFileDumper::ShowCompressionSize( const ReadOptions read_options; const WriteOptions write_options; ROCKSDB_NAMESPACE::InternalKeyComparator ikc(opts.comparator); - IntTblPropCollectorFactories block_based_table_factories; + InternalTblPropCollFactories block_based_table_factories; std::string column_family_name; int unknown_level = -1; diff --git a/table/sst_file_writer.cc b/table/sst_file_writer.cc index 1cab0613cd..f35911250c 100644 --- a/table/sst_file_writer.cc +++ b/table/sst_file_writer.cc @@ -321,10 +321,10 @@ Status SstFileWriter::Open(const std::string& file_path) { compression_opts = r->mutable_cf_options.compression_opts; } - IntTblPropCollectorFactories int_tbl_prop_collector_factories; + InternalTblPropCollFactories internal_tbl_prop_coll_factories; // SstFileWriter properties collector to add SstFileWriter version. - int_tbl_prop_collector_factories.emplace_back( + internal_tbl_prop_coll_factories.emplace_back( new SstFileWriterPropertiesCollectorFactory(2 /* version */, 0 /* global_seqno*/)); @@ -332,7 +332,7 @@ Status SstFileWriter::Open(const std::string& file_path) { auto user_collector_factories = r->ioptions.table_properties_collector_factories; for (size_t i = 0; i < user_collector_factories.size(); i++) { - int_tbl_prop_collector_factories.emplace_back( + internal_tbl_prop_coll_factories.emplace_back( new UserKeyTablePropertiesCollectorFactory( user_collector_factories[i])); } @@ -354,7 +354,7 @@ Status SstFileWriter::Open(const std::string& file_path) { // TODO: plumb Env::IOActivity, Env::IOPriority TableBuilderOptions table_builder_options( r->ioptions, r->mutable_cf_options, ReadOptions(), r->write_options, - r->internal_comparator, &int_tbl_prop_collector_factories, + r->internal_comparator, &internal_tbl_prop_coll_factories, compression_type, compression_opts, cf_id, r->column_family_name, unknown_level, false /* is_bottommost */, TableFileCreationReason::kMisc, 0 /* oldest_key_time */, 0 /* file_creation_time */, diff --git a/table/sst_file_writer_collectors.h b/table/sst_file_writer_collectors.h index 3f5af4240a..5f421dffb9 100644 --- a/table/sst_file_writer_collectors.h +++ b/table/sst_file_writer_collectors.h @@ -23,7 +23,7 @@ struct ExternalSstFilePropertyNames { // PropertiesCollector used to add properties specific to tables // generated by SstFileWriter -class SstFileWriterPropertiesCollector : public IntTblPropCollector { +class SstFileWriterPropertiesCollector : public InternalTblPropColl { public: explicit SstFileWriterPropertiesCollector(int32_t version, SequenceNumber global_seqno) @@ -72,13 +72,13 @@ class SstFileWriterPropertiesCollector : public IntTblPropCollector { }; class SstFileWriterPropertiesCollectorFactory - : public IntTblPropCollectorFactory { + : public InternalTblPropCollFactory { public: explicit SstFileWriterPropertiesCollectorFactory(int32_t version, SequenceNumber global_seqno) : version_(version), global_seqno_(global_seqno) {} - IntTblPropCollector* CreateIntTblPropCollector( + InternalTblPropColl* CreateInternalTblPropColl( uint32_t /*column_family_id*/, int /* level_at_creation */) override { return new SstFileWriterPropertiesCollector(version_, global_seqno_); } diff --git a/table/table_builder.h b/table/table_builder.h index 1ae1998078..f5525315f3 100644 --- a/table/table_builder.h +++ b/table/table_builder.h @@ -104,7 +104,7 @@ struct TableBuilderOptions { const ImmutableOptions& _ioptions, const MutableCFOptions& _moptions, const ReadOptions& _read_options, const WriteOptions& _write_options, const InternalKeyComparator& _internal_comparator, - const IntTblPropCollectorFactories* _int_tbl_prop_collector_factories, + const InternalTblPropCollFactories* _internal_tbl_prop_coll_factories, CompressionType _compression_type, const CompressionOptions& _compression_opts, uint32_t _column_family_id, const std::string& _column_family_name, int _level, @@ -119,7 +119,7 @@ struct TableBuilderOptions { read_options(_read_options), write_options(_write_options), internal_comparator(_internal_comparator), - int_tbl_prop_collector_factories(_int_tbl_prop_collector_factories), + internal_tbl_prop_coll_factories(_internal_tbl_prop_coll_factories), compression_type(_compression_type), compression_opts(_compression_opts), column_family_id(_column_family_id), @@ -139,7 +139,7 @@ struct TableBuilderOptions { const ReadOptions& read_options; const WriteOptions& write_options; const InternalKeyComparator& internal_comparator; - const IntTblPropCollectorFactories* int_tbl_prop_collector_factories; + const InternalTblPropCollFactories* internal_tbl_prop_coll_factories; const CompressionType compression_type; const CompressionOptions& compression_opts; const uint32_t column_family_id; diff --git a/table/table_reader_bench.cc b/table/table_reader_bench.cc index 9b24e3c433..2e9094bfcb 100644 --- a/table/table_reader_bench.cc +++ b/table/table_reader_bench.cc @@ -95,13 +95,13 @@ void TableReaderBenchmark(Options& opts, EnvOptions& env_options, FileOptions(env_options), &file_writer, nullptr)); - IntTblPropCollectorFactories int_tbl_prop_collector_factories; + InternalTblPropCollFactories internal_tbl_prop_coll_factories; int unknown_level = -1; const WriteOptions write_options; tb = opts.table_factory->NewTableBuilder( TableBuilderOptions(ioptions, moptions, read_options, write_options, - ikc, &int_tbl_prop_collector_factories, + ikc, &internal_tbl_prop_coll_factories, CompressionType::kNoCompression, CompressionOptions(), 0 /* column_family_id */, kDefaultColumnFamilyName, unknown_level), diff --git a/table/table_test.cc b/table/table_test.cc index a3a7bc1fb5..1be37af27c 100644 --- a/table/table_test.cc +++ b/table/table_test.cc @@ -368,11 +368,11 @@ class TableConstructor : public Constructor { file_writer_.reset(new WritableFileWriter( std::move(sink), "" /* don't care */, FileOptions())); std::unique_ptr builder; - IntTblPropCollectorFactories int_tbl_prop_collector_factories; + InternalTblPropCollFactories internal_tbl_prop_coll_factories; if (largest_seqno_ != 0) { // Pretend that it's an external file written by SstFileWriter. - int_tbl_prop_collector_factories.emplace_back( + internal_tbl_prop_coll_factories.emplace_back( new SstFileWriterPropertiesCollectorFactory(2 /* version */, 0 /* global_seqno*/)); } @@ -383,7 +383,7 @@ class TableConstructor : public Constructor { builder.reset(ioptions.table_factory->NewTableBuilder( TableBuilderOptions(ioptions, moptions, read_options, write_options, internal_comparator, - &int_tbl_prop_collector_factories, + &internal_tbl_prop_coll_factories, options.compression, options.compression_opts, kUnknownColumnFamily, column_family_name, level_), file_writer_.get())); @@ -4445,7 +4445,7 @@ TEST_P(BlockBasedTableTest, NoFileChecksum) { std::unique_ptr comparator( new InternalKeyComparator(BytewiseComparator())); int level = 0; - IntTblPropCollectorFactories int_tbl_prop_collector_factories; + InternalTblPropCollFactories internal_tbl_prop_coll_factories; std::string column_family_name; FileChecksumTestHelper f(true); @@ -4455,7 +4455,7 @@ TEST_P(BlockBasedTableTest, NoFileChecksum) { const WriteOptions write_options; builder.reset(ioptions.table_factory->NewTableBuilder( TableBuilderOptions(ioptions, moptions, read_options, write_options, - *comparator, &int_tbl_prop_collector_factories, + *comparator, &internal_tbl_prop_coll_factories, options.compression, options.compression_opts, kUnknownColumnFamily, column_family_name, level), f.GetFileWriter())); @@ -4477,7 +4477,7 @@ TEST_P(BlockBasedTableTest, Crc32cFileChecksum) { std::unique_ptr comparator( new InternalKeyComparator(BytewiseComparator())); int level = 0; - IntTblPropCollectorFactories int_tbl_prop_collector_factories; + InternalTblPropCollFactories internal_tbl_prop_coll_factories; std::string column_family_name; FileChecksumGenContext gen_context; @@ -4493,7 +4493,7 @@ TEST_P(BlockBasedTableTest, Crc32cFileChecksum) { const WriteOptions write_options; builder.reset(ioptions.table_factory->NewTableBuilder( TableBuilderOptions(ioptions, moptions, read_options, write_options, - *comparator, &int_tbl_prop_collector_factories, + *comparator, &internal_tbl_prop_coll_factories, options.compression, options.compression_opts, kUnknownColumnFamily, column_family_name, level), f.GetFileWriter())); @@ -4534,14 +4534,14 @@ TEST_F(PlainTableTest, BasicPlainTableProperties) { const ImmutableOptions ioptions(options); const MutableCFOptions moptions(options); InternalKeyComparator ikc(options.comparator); - IntTblPropCollectorFactories int_tbl_prop_collector_factories; + InternalTblPropCollFactories internal_tbl_prop_coll_factories; std::string column_family_name; int unknown_level = -1; const ReadOptions read_options; const WriteOptions write_options; std::unique_ptr builder(factory.NewTableBuilder( TableBuilderOptions(ioptions, moptions, read_options, write_options, ikc, - &int_tbl_prop_collector_factories, kNoCompression, + &internal_tbl_prop_coll_factories, kNoCompression, CompressionOptions(), kUnknownColumnFamily, column_family_name, unknown_level), file_writer.get())); @@ -4587,7 +4587,7 @@ TEST_F(PlainTableTest, NoFileChecksum) { const ImmutableOptions ioptions(options); const MutableCFOptions moptions(options); InternalKeyComparator ikc(options.comparator); - IntTblPropCollectorFactories int_tbl_prop_collector_factories; + InternalTblPropCollFactories internal_tbl_prop_coll_factories; std::string column_family_name; int unknown_level = -1; FileChecksumTestHelper f(true); @@ -4596,7 +4596,7 @@ TEST_F(PlainTableTest, NoFileChecksum) { const WriteOptions write_options; std::unique_ptr builder(factory.NewTableBuilder( TableBuilderOptions(ioptions, moptions, read_options, write_options, ikc, - &int_tbl_prop_collector_factories, kNoCompression, + &internal_tbl_prop_coll_factories, kNoCompression, CompressionOptions(), kUnknownColumnFamily, column_family_name, unknown_level), f.GetFileWriter())); @@ -4621,7 +4621,7 @@ TEST_F(PlainTableTest, Crc32cFileChecksum) { const ImmutableOptions ioptions(options); const MutableCFOptions moptions(options); InternalKeyComparator ikc(options.comparator); - IntTblPropCollectorFactories int_tbl_prop_collector_factories; + InternalTblPropCollFactories internal_tbl_prop_coll_factories; std::string column_family_name; int unknown_level = -1; @@ -4637,7 +4637,7 @@ TEST_F(PlainTableTest, Crc32cFileChecksum) { const WriteOptions write_options; std::unique_ptr builder(factory.NewTableBuilder( TableBuilderOptions(ioptions, moptions, read_options, write_options, ikc, - &int_tbl_prop_collector_factories, kNoCompression, + &internal_tbl_prop_coll_factories, kNoCompression, CompressionOptions(), kUnknownColumnFamily, column_family_name, unknown_level), f.GetFileWriter())); @@ -5239,8 +5239,8 @@ TEST_P(BlockBasedTableTest, DISABLED_TableWithGlobalSeqno) { const ImmutableOptions ioptions(options); const MutableCFOptions moptions(options); InternalKeyComparator ikc(options.comparator); - IntTblPropCollectorFactories int_tbl_prop_collector_factories; - int_tbl_prop_collector_factories.emplace_back( + InternalTblPropCollFactories internal_tbl_prop_coll_factories; + internal_tbl_prop_coll_factories.emplace_back( new SstFileWriterPropertiesCollectorFactory(2 /* version */, 0 /* global_seqno*/)); std::string column_family_name; @@ -5248,7 +5248,7 @@ TEST_P(BlockBasedTableTest, DISABLED_TableWithGlobalSeqno) { const WriteOptions write_options; std::unique_ptr builder(options.table_factory->NewTableBuilder( TableBuilderOptions(ioptions, moptions, read_options, write_options, ikc, - &int_tbl_prop_collector_factories, kNoCompression, + &internal_tbl_prop_coll_factories, kNoCompression, CompressionOptions(), kUnknownColumnFamily, column_family_name, -1), file_writer.get())); @@ -5424,13 +5424,13 @@ TEST_P(BlockBasedTableTest, BlockAlignTest) { const ImmutableOptions ioptions(options); const MutableCFOptions moptions(options); InternalKeyComparator ikc(options.comparator); - IntTblPropCollectorFactories int_tbl_prop_collector_factories; + InternalTblPropCollFactories internal_tbl_prop_coll_factories; std::string column_family_name; const ReadOptions read_options; const WriteOptions write_options; std::unique_ptr builder(options.table_factory->NewTableBuilder( TableBuilderOptions(ioptions, moptions, read_options, write_options, ikc, - &int_tbl_prop_collector_factories, kNoCompression, + &internal_tbl_prop_coll_factories, kNoCompression, CompressionOptions(), kUnknownColumnFamily, column_family_name, -1), file_writer.get())); @@ -5516,13 +5516,13 @@ TEST_P(BlockBasedTableTest, PropertiesBlockRestartPointTest) { const ImmutableOptions ioptions(options); const MutableCFOptions moptions(options); InternalKeyComparator ikc(options.comparator); - IntTblPropCollectorFactories int_tbl_prop_collector_factories; + InternalTblPropCollFactories internal_tbl_prop_coll_factories; std::string column_family_name; const ReadOptions read_options; const WriteOptions write_options; std::unique_ptr builder(options.table_factory->NewTableBuilder( TableBuilderOptions(ioptions, moptions, read_options, write_options, ikc, - &int_tbl_prop_collector_factories, kNoCompression, + &internal_tbl_prop_coll_factories, kNoCompression, CompressionOptions(), kUnknownColumnFamily, column_family_name, -1), file_writer.get())); @@ -6107,14 +6107,14 @@ TEST_F(ChargeCompressionDictionaryBuildingBufferTest, Basic) { ImmutableOptions ioptions(options); MutableCFOptions moptions(options); InternalKeyComparator ikc(options.comparator); - IntTblPropCollectorFactories int_tbl_prop_collector_factories; + InternalTblPropCollFactories internal_tbl_prop_coll_factories; const ReadOptions read_options; const WriteOptions write_options; std::unique_ptr builder( options.table_factory->NewTableBuilder( TableBuilderOptions(ioptions, moptions, read_options, write_options, - ikc, &int_tbl_prop_collector_factories, + ikc, &internal_tbl_prop_coll_factories, kSnappyCompression, options.compression_opts, kUnknownColumnFamily, "test_cf", -1 /* level */), @@ -6186,13 +6186,13 @@ TEST_F(ChargeCompressionDictionaryBuildingBufferTest, ImmutableOptions ioptions(options); MutableCFOptions moptions(options); InternalKeyComparator ikc(options.comparator); - IntTblPropCollectorFactories int_tbl_prop_collector_factories; + InternalTblPropCollFactories internal_tbl_prop_coll_factories; const ReadOptions read_options; const WriteOptions write_options; std::unique_ptr builder(options.table_factory->NewTableBuilder( TableBuilderOptions(ioptions, moptions, read_options, write_options, ikc, - &int_tbl_prop_collector_factories, kSnappyCompression, + &internal_tbl_prop_coll_factories, kSnappyCompression, options.compression_opts, kUnknownColumnFamily, "test_cf", -1 /* level */), file_writer.get())); @@ -6273,13 +6273,13 @@ TEST_F(ChargeCompressionDictionaryBuildingBufferTest, BasicWithCacheFull) { ImmutableOptions ioptions(options); MutableCFOptions moptions(options); InternalKeyComparator ikc(options.comparator); - IntTblPropCollectorFactories int_tbl_prop_collector_factories; + InternalTblPropCollFactories internal_tbl_prop_coll_factories; const ReadOptions read_options; const WriteOptions write_options; std::unique_ptr builder(options.table_factory->NewTableBuilder( TableBuilderOptions(ioptions, moptions, read_options, write_options, ikc, - &int_tbl_prop_collector_factories, kSnappyCompression, + &internal_tbl_prop_coll_factories, kSnappyCompression, options.compression_opts, kUnknownColumnFamily, "test_cf", -1 /* level */), file_writer.get())); diff --git a/tools/sst_dump_test.cc b/tools/sst_dump_test.cc index da425e4b04..53721e3302 100644 --- a/tools/sst_dump_test.cc +++ b/tools/sst_dump_test.cc @@ -118,7 +118,7 @@ class SSTDumpToolTest : public testing::Test { ROCKSDB_NAMESPACE::InternalKeyComparator ikc(opts.comparator); std::unique_ptr tb; - IntTblPropCollectorFactories int_tbl_prop_collector_factories; + InternalTblPropCollFactories internal_tbl_prop_coll_factories; std::unique_ptr file_writer; ASSERT_OK(WritableFileWriter::Create(test_env->GetFileSystem(), file_name, file_options, &file_writer, nullptr)); @@ -129,7 +129,7 @@ class SSTDumpToolTest : public testing::Test { tb.reset(opts.table_factory->NewTableBuilder( TableBuilderOptions( imoptions, moptions, read_options, write_options, ikc, - &int_tbl_prop_collector_factories, CompressionType::kNoCompression, + &internal_tbl_prop_coll_factories, CompressionType::kNoCompression, CompressionOptions(), TablePropertiesCollectorFactory::Context::kUnknownColumnFamily, column_family_name, unknown_level),