From 1ae026c400fb0801d714c8123d26b93a2dc63eb0 Mon Sep 17 00:00:00 2001 From: Levi Tamasi Date: Tue, 6 Jul 2021 10:13:40 -0700 Subject: [PATCH] Partially revert the "apply subrange of table property collectors" change (#8465) Summary: We ended up using a different approach for tracking the amount of garbage in blob files (see e.g. https://github.com/facebook/rocksdb/pull/8450), so the ability to apply only a range of table property collectors is now unnecessary. The patch reverts this part of https://github.com/facebook/rocksdb/pull/8298 while keeping the cleanup done in that PR. Pull Request resolved: https://github.com/facebook/rocksdb/pull/8465 Test Plan: `make check` Reviewed By: jay-zhuang Differential Revision: D29399921 Pulled By: ltamasi fbshipit-source-id: af64816c357d0829b9d7ba8ca1477038138f6f0a --- db/table_properties_collector.h | 4 --- .../block_based/block_based_table_builder.cc | 8 +++--- table/plain/plain_table_builder.cc | 10 +++---- table/plain/plain_table_builder.h | 2 +- table/table_builder.h | 27 ++----------------- 5 files changed, 12 insertions(+), 39 deletions(-) diff --git a/db/table_properties_collector.h b/db/table_properties_collector.h index befb43652..f80570e4a 100644 --- a/db/table_properties_collector.h +++ b/db/table_properties_collector.h @@ -50,10 +50,6 @@ class IntTblPropCollectorFactory { using IntTblPropCollectorFactories = std::vector>; -using IntTblPropCollectorFactoryIter = - IntTblPropCollectorFactories::const_iterator; -using IntTblPropCollectorFactoryRange = - std::pair; // When rocksdb creates a new table, it will encode all "user keys" into // "internal keys", which contains meta information of a given entry. diff --git a/table/block_based/block_based_table_builder.cc b/table/block_based/block_based_table_builder.cc index ac0d45cdd..bb8cfa14d 100644 --- a/table/block_based/block_based_table_builder.cc +++ b/table/block_based/block_based_table_builder.cc @@ -508,12 +508,12 @@ struct BlockBasedTableBuilder::Rep { use_delta_encoding_for_index_values, p_index_builder_)); } - const auto& factory_range = tbo.int_tbl_prop_collector_factories; - for (auto it = factory_range.first; it != factory_range.second; ++it) { - assert(*it); + assert(tbo.int_tbl_prop_collector_factories); + for (auto& factory : *tbo.int_tbl_prop_collector_factories) { + assert(factory); table_properties_collectors.emplace_back( - (*it)->CreateIntTblPropCollector(column_family_id)); + factory->CreateIntTblPropCollector(column_family_id)); } table_properties_collectors.emplace_back( new BlockBasedTablePropertiesCollector( diff --git a/table/plain/plain_table_builder.cc b/table/plain/plain_table_builder.cc index 3a1f0a41b..5754f7c29 100644 --- a/table/plain/plain_table_builder.cc +++ b/table/plain/plain_table_builder.cc @@ -58,7 +58,7 @@ extern const uint64_t kLegacyPlainTableMagicNumber = 0x4f3418eb7a8f13b8ull; PlainTableBuilder::PlainTableBuilder( const ImmutableOptions& ioptions, const MutableCFOptions& moptions, - const IntTblPropCollectorFactoryRange& int_tbl_prop_collector_factories, + const IntTblPropCollectorFactories* int_tbl_prop_collector_factories, uint32_t column_family_id, 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, @@ -112,12 +112,12 @@ PlainTableBuilder::PlainTableBuilder( properties_.user_collected_properties [PlainTablePropertyNames::kEncodingType] = val; - for (auto it = int_tbl_prop_collector_factories.first; - it != int_tbl_prop_collector_factories.second; ++it) { - assert(*it); + assert(int_tbl_prop_collector_factories); + for (auto& factory : *int_tbl_prop_collector_factories) { + assert(factory); table_properties_collectors_.emplace_back( - (*it)->CreateIntTblPropCollector(column_family_id)); + factory->CreateIntTblPropCollector(column_family_id)); } } diff --git a/table/plain/plain_table_builder.h b/table/plain/plain_table_builder.h index 7305cb153..07ed7e5dc 100644 --- a/table/plain/plain_table_builder.h +++ b/table/plain/plain_table_builder.h @@ -38,7 +38,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 IntTblPropCollectorFactoryRange& int_tbl_prop_collector_factories, + const IntTblPropCollectorFactories* int_tbl_prop_collector_factories, uint32_t column_family_id, WritableFileWriter* file, uint32_t user_key_size, EncodingType encoding_type, size_t index_sparseness, uint32_t bloom_bits_per_key, diff --git a/table/table_builder.h b/table/table_builder.h index f22b10750..92011910d 100644 --- a/table/table_builder.h +++ b/table/table_builder.h @@ -104,7 +104,7 @@ struct TableBuilderOptions { TableBuilderOptions( const ImmutableOptions& _ioptions, const MutableCFOptions& _moptions, const InternalKeyComparator& _internal_comparator, - const IntTblPropCollectorFactoryRange& _int_tbl_prop_collector_factories, + const IntTblPropCollectorFactories* _int_tbl_prop_collector_factories, CompressionType _compression_type, const CompressionOptions& _compression_opts, uint32_t _column_family_id, const std::string& _column_family_name, int _level, @@ -133,33 +133,10 @@ struct TableBuilderOptions { reason(_reason), cur_file_num(_cur_file_num) {} - TableBuilderOptions( - const ImmutableOptions& _ioptions, const MutableCFOptions& _moptions, - const InternalKeyComparator& _internal_comparator, - const IntTblPropCollectorFactories* _int_tbl_prop_collector_factories, - CompressionType _compression_type, - const CompressionOptions& _compression_opts, uint32_t _column_family_id, - const std::string& _column_family_name, int _level, - bool _is_bottommost = false, - TableFileCreationReason _reason = TableFileCreationReason::kMisc, - const uint64_t _creation_time = 0, const int64_t _oldest_key_time = 0, - const uint64_t _file_creation_time = 0, const std::string& _db_id = "", - const std::string& _db_session_id = "", - const uint64_t _target_file_size = 0, const uint64_t _cur_file_num = 0) - : TableBuilderOptions(_ioptions, _moptions, _internal_comparator, - IntTblPropCollectorFactoryRange( - _int_tbl_prop_collector_factories->begin(), - _int_tbl_prop_collector_factories->end()), - _compression_type, _compression_opts, - _column_family_id, _column_family_name, _level, - _is_bottommost, _reason, _creation_time, - _oldest_key_time, _file_creation_time, _db_id, - _db_session_id, _target_file_size, _cur_file_num) {} - const ImmutableOptions& ioptions; const MutableCFOptions& moptions; const InternalKeyComparator& internal_comparator; - const IntTblPropCollectorFactoryRange int_tbl_prop_collector_factories; + const IntTblPropCollectorFactories* int_tbl_prop_collector_factories; const CompressionType compression_type; const CompressionOptions& compression_opts; const uint32_t column_family_id;