Skip to content

Commit

Permalink
Partially revert the "apply subrange of table property collectors cha…
Browse files Browse the repository at this point in the history
…nge"

Summary:
We ended up using a different approach for tracking the amount of
garbage in blob files (see e.g. facebook#8450),
so the ability to apply only a range of table property collectors is
now unnecessary. The patch reverts this part of
facebook#8298 while keeping the cleanup.

Test Plan:
`make check`
  • Loading branch information
ltamasi committed Jul 6, 2021
1 parent 9dc887e commit 0ba1143
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 39 deletions.
4 changes: 0 additions & 4 deletions db/table_properties_collector.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,6 @@ class IntTblPropCollectorFactory {

using IntTblPropCollectorFactories =
std::vector<std::unique_ptr<IntTblPropCollectorFactory>>;
using IntTblPropCollectorFactoryIter =
IntTblPropCollectorFactories::const_iterator;
using IntTblPropCollectorFactoryRange =
std::pair<IntTblPropCollectorFactoryIter, IntTblPropCollectorFactoryIter>;

// When rocksdb creates a new table, it will encode all "user keys" into
// "internal keys", which contains meta information of a given entry.
Expand Down
8 changes: 4 additions & 4 deletions table/block_based/block_based_table_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
10 changes: 5 additions & 5 deletions table/plain/plain_table_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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));
}
}

Expand Down
2 changes: 1 addition & 1 deletion table/plain/plain_table_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
27 changes: 2 additions & 25 deletions table/table_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 0ba1143

Please sign in to comment.