Make it possible to apply only a subrange of table property collectors (#8298)

Summary:
This patch does two things:
1) Introduces some aliases in order to eliminate/prevent long-winded type names
w/r/t the internal table property collectors (see e.g.
`std::vector<std::unique_ptr<IntTblPropCollectorFactory>>`).
2) Makes it possible to apply only a subrange of table property collectors during
table building by turning `TableBuilderOptions::int_tbl_prop_collector_factories`
from a pointer to a `vector` into a range (i.e. a pair of iterators).

Rationale: I plan to introduce a BlobDB related table property collector, which
should only be applied during table creation if blob storage is enabled at the moment
(which can be changed dynamically). This change will make it possible to include/
exclude the BlobDB related collector as needed without having to introduce
a second `vector` of collectors in `ColumnFamilyData` with pretty much the same
contents.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/8298

Test Plan: `make check`

Reviewed By: jay-zhuang

Differential Revision: D28430910

Pulled By: ltamasi

fbshipit-source-id: a81d28f2c59495865300f43deb2257d2e6977c8e

db/column_family.cc

@@ -109,8 +109,9 @@ const Comparator* ColumnFamilyHandleImpl::GetComparator() const {
 
 void GetIntTblPropCollectorFactory(
     const ImmutableCFOptions& ioptions,
-    std::vector<std::unique_ptr<IntTblPropCollectorFactory>>*
+    IntTblPropCollectorFactories* int_tbl_prop_collector_factories) {
-        int_tbl_prop_collector_factories) {
+  assert(int_tbl_prop_collector_factories);
+
   auto& collector_factories = ioptions.table_properties_collector_factories;
   for (size_t i = 0; i < ioptions.table_properties_collector_factories.size();
        ++i) {

db/column_family.h

@@ -258,8 +258,7 @@ extern ColumnFamilyOptions SanitizeOptions(const ImmutableDBOptions& db_options,
 // one too.
 extern void GetIntTblPropCollectorFactory(
     const ImmutableCFOptions& ioptions,
-    std::vector<std::unique_ptr<IntTblPropCollectorFactory>>*
+    IntTblPropCollectorFactories* int_tbl_prop_collector_factories);
-        int_tbl_prop_collector_factories);
 
 class ColumnFamilySet;
 
@@ -428,8 +427,7 @@ class ColumnFamilyData {
     return internal_comparator_;
   }
 
-  const std::vector<std::unique_ptr<IntTblPropCollectorFactory>>*
+  const IntTblPropCollectorFactories* int_tbl_prop_collector_factories() const {
-  int_tbl_prop_collector_factories() const {
     return &int_tbl_prop_collector_factories_;
   }
 
@@ -548,8 +546,7 @@ class ColumnFamilyData {
   std::atomic<bool> dropped_;  // true if client dropped it
 
   const InternalKeyComparator internal_comparator_;
-  std::vector<std::unique_ptr<IntTblPropCollectorFactory>>
+  IntTblPropCollectorFactories int_tbl_prop_collector_factories_;
-      int_tbl_prop_collector_factories_;
 
   const ColumnFamilyOptions initial_cf_options_;
   const ImmutableOptions ioptions_;

db/table_properties_collector.h

@@ -48,6 +48,13 @@ class IntTblPropCollectorFactory {
   virtual const char* Name() const = 0;
 };
 
+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.
 //

db/table_properties_collector_test.cc

@@ -41,11 +41,11 @@ namespace {
 static const uint32_t kTestColumnFamilyId = 66;
 static const std::string kTestColumnFamilyName = "test_column_fam";
 
-void MakeBuilder(const Options& options, const ImmutableOptions& ioptions,
+void MakeBuilder(
+    const Options& options, const ImmutableOptions& ioptions,
     const MutableCFOptions& moptions,
     const InternalKeyComparator& internal_comparator,
-                 const std::vector<std::unique_ptr<IntTblPropCollectorFactory>>*
+    const IntTblPropCollectorFactories* int_tbl_prop_collector_factories,
-                     int_tbl_prop_collector_factories,
     std::unique_ptr<WritableFileWriter>* writable,
     std::unique_ptr<TableBuilder>* builder) {
   std::unique_ptr<FSWritableFile> wf(new test::StringSink);
@@ -264,8 +264,7 @@ void TestCustomizedTablePropertiesCollector(
   std::unique_ptr<WritableFileWriter> writer;
   const ImmutableOptions ioptions(options);
   const MutableCFOptions moptions(options);
-  std::vector<std::unique_ptr<IntTblPropCollectorFactory>>
+  IntTblPropCollectorFactories int_tbl_prop_collector_factories;
-      int_tbl_prop_collector_factories;
   if (test_int_tbl_prop_collector) {
     int_tbl_prop_collector_factories.emplace_back(
         new RegularKeysStartWithAFactory(backward_mode));
@@ -395,8 +394,7 @@ void TestInternalKeyPropertiesCollector(
   Options options;
   test::PlainInternalKeyComparator pikc(options.comparator);
 
-  std::vector<std::unique_ptr<IntTblPropCollectorFactory>>
+  IntTblPropCollectorFactories int_tbl_prop_collector_factories;
-      int_tbl_prop_collector_factories;
   options.table_factory = table_factory;
   if (sanitized) {
     options.table_properties_collector_factories.emplace_back(

db/version_set_test.cc

@@ -2775,8 +2775,7 @@ class VersionSetTestMissingFiles : public VersionSetTestBase,
       ASSERT_OK(s);
       std::unique_ptr<WritableFileWriter> fwriter(new WritableFileWriter(
           std::move(file), fname, FileOptions(), env_->GetSystemClock().get()));
-      std::vector<std::unique_ptr<IntTblPropCollectorFactory>>
+      IntTblPropCollectorFactories int_tbl_prop_collector_factories;
-          int_tbl_prop_collector_factories;
 
       std::unique_ptr<TableBuilder> builder(table_factory_->NewTableBuilder(
           TableBuilderOptions(

table/block_based/block_based_table_builder.cc

@@ -505,9 +505,12 @@ struct BlockBasedTableBuilder::Rep {
           use_delta_encoding_for_index_values, p_index_builder_));
     }
 
-    for (auto& collector_factories : *tbo.int_tbl_prop_collector_factories) {
+    const auto& factory_range = tbo.int_tbl_prop_collector_factories;
+    for (auto it = factory_range.first; it != factory_range.second; ++it) {
+      assert(*it);
+
       table_properties_collectors.emplace_back(
-          collector_factories->CreateIntTblPropCollector(column_family_id));
+          (*it)->CreateIntTblPropCollector(column_family_id));
     }
     table_properties_collectors.emplace_back(
         new BlockBasedTablePropertiesCollector(

table/block_based/block_based_table_reader_test.cc

@@ -63,7 +63,7 @@ class BlockBasedTableReaderTest
     InternalKeyComparator comparator(options.comparator);
     ColumnFamilyOptions cf_options;
     MutableCFOptions moptions(cf_options);
-    std::vector<std::unique_ptr<IntTblPropCollectorFactory>> factories;
+    IntTblPropCollectorFactories factories;
     std::unique_ptr<TableBuilder> table_builder(table_factory_->NewTableBuilder(
         TableBuilderOptions(ioptions, moptions, comparator, &factories,
                             compression_type, CompressionOptions(),

table/block_based/data_block_hash_index_test.cc

@@ -551,8 +551,7 @@ void TestBoundary(InternalKey& ik1, std::string& v1, InternalKey& ik2,
   file_writer.reset(
       new WritableFileWriter(std::move(f), "" /* don't care */, FileOptions()));
   std::unique_ptr<TableBuilder> builder;
-  std::vector<std::unique_ptr<IntTblPropCollectorFactory>>
+  IntTblPropCollectorFactories int_tbl_prop_collector_factories;
-      int_tbl_prop_collector_factories;
   std::string column_family_name;
   builder.reset(ioptions.table_factory->NewTableBuilder(
       TableBuilderOptions(

table/block_fetcher_test.cc

@@ -97,7 +97,7 @@ class BlockFetcherTest : public testing::Test {
     InternalKeyComparator comparator(options_.comparator);
     ColumnFamilyOptions cf_options(options_);
     MutableCFOptions moptions(cf_options);
-    std::vector<std::unique_ptr<IntTblPropCollectorFactory>> factories;
+    IntTblPropCollectorFactories factories;
     std::unique_ptr<TableBuilder> table_builder(table_factory_.NewTableBuilder(
         TableBuilderOptions(ioptions, moptions, comparator, &factories,
                             compression_type, CompressionOptions(),

table/plain/plain_table_builder.cc

@@ -58,8 +58,7 @@ extern const uint64_t kLegacyPlainTableMagicNumber = 0x4f3418eb7a8f13b8ull;
 
 PlainTableBuilder::PlainTableBuilder(
     const ImmutableOptions& ioptions, const MutableCFOptions& moptions,
-    const std::vector<std::unique_ptr<IntTblPropCollectorFactory>>*
+    const IntTblPropCollectorFactoryRange& int_tbl_prop_collector_factories,
-        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,
@@ -113,9 +112,12 @@ PlainTableBuilder::PlainTableBuilder(
   properties_.user_collected_properties
       [PlainTablePropertyNames::kEncodingType] = val;
 
-  for (auto& collector_factories : *int_tbl_prop_collector_factories) {
+  for (auto it = int_tbl_prop_collector_factories.first;
+       it != int_tbl_prop_collector_factories.second; ++it) {
+    assert(*it);
+
     table_properties_collectors_.emplace_back(
-        collector_factories->CreateIntTblPropCollector(column_family_id));
+        (*it)->CreateIntTblPropCollector(column_family_id));
   }
 }
 

table/plain/plain_table_builder.h

@@ -38,8 +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 std::vector<std::unique_ptr<IntTblPropCollectorFactory>>*
+      const IntTblPropCollectorFactoryRange& int_tbl_prop_collector_factories,
-          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,

table/sst_file_dumper.cc

@@ -268,8 +268,7 @@ Status SstFileDumper::ShowCompressionSize(
   const ColumnFamilyOptions cfo(opts);
   const MutableCFOptions moptions(cfo);
   ROCKSDB_NAMESPACE::InternalKeyComparator ikc(opts.comparator);
-  std::vector<std::unique_ptr<IntTblPropCollectorFactory>>
+  IntTblPropCollectorFactories block_based_table_factories;
-      block_based_table_factories;
 
   std::string column_family_name;
   int unknown_level = -1;

table/sst_file_writer.cc

@@ -213,8 +213,7 @@ Status SstFileWriter::Open(const std::string& file_path) {
     compression_opts = r->mutable_cf_options.compression_opts;
   }
 
-  std::vector<std::unique_ptr<IntTblPropCollectorFactory>>
+  IntTblPropCollectorFactories int_tbl_prop_collector_factories;
-      int_tbl_prop_collector_factories;
 
   // SstFileWriter properties collector to add SstFileWriter version.
   int_tbl_prop_collector_factories.emplace_back(

table/table_builder.h

@@ -93,8 +93,7 @@ struct TableBuilderOptions {
   TableBuilderOptions(
       const ImmutableOptions& _ioptions, const MutableCFOptions& _moptions,
       const InternalKeyComparator& _internal_comparator,
-      const std::vector<std::unique_ptr<IntTblPropCollectorFactory>>*
+      const IntTblPropCollectorFactoryRange& _int_tbl_prop_collector_factories,
-          _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,
@@ -122,11 +121,33 @@ struct TableBuilderOptions {
         is_bottommost(_is_bottommost),
         reason(_reason) {}
 
+  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)
+      : 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) {}
+
   const ImmutableOptions& ioptions;
   const MutableCFOptions& moptions;
   const InternalKeyComparator& internal_comparator;
-  const std::vector<std::unique_ptr<IntTblPropCollectorFactory>>*
+  const IntTblPropCollectorFactoryRange int_tbl_prop_collector_factories;
-      int_tbl_prop_collector_factories;
   const CompressionType compression_type;
   const CompressionOptions& compression_opts;
   const uint32_t column_family_id;

table/table_reader_bench.cc

@@ -95,8 +95,7 @@ void TableReaderBenchmark(Options& opts, EnvOptions& env_options,
                                          FileOptions(env_options), &file_writer,
                                          nullptr));
 
-    std::vector<std::unique_ptr<IntTblPropCollectorFactory> >
+    IntTblPropCollectorFactories int_tbl_prop_collector_factories;
-        int_tbl_prop_collector_factories;
 
     int unknown_level = -1;
     tb = opts.table_factory->NewTableBuilder(

table/table_test.cc

@@ -352,8 +352,7 @@ class TableConstructor : public Constructor {
     file_writer_.reset(new WritableFileWriter(
         std::move(sink), "" /* don't care */, FileOptions()));
     std::unique_ptr<TableBuilder> builder;
-    std::vector<std::unique_ptr<IntTblPropCollectorFactory>>
+    IntTblPropCollectorFactories int_tbl_prop_collector_factories;
-        int_tbl_prop_collector_factories;
 
     if (largest_seqno_ != 0) {
       // Pretend that it's an external file written by SstFileWriter.
@@ -3314,8 +3313,7 @@ TEST_P(BlockBasedTableTest, NoFileChecksum) {
   std::unique_ptr<InternalKeyComparator> comparator(
       new InternalKeyComparator(BytewiseComparator()));
   int level = 0;
-  std::vector<std::unique_ptr<IntTblPropCollectorFactory>>
+  IntTblPropCollectorFactories int_tbl_prop_collector_factories;
-      int_tbl_prop_collector_factories;
   std::string column_family_name;
 
   FileChecksumTestHelper f(true);
@@ -3345,8 +3343,7 @@ TEST_P(BlockBasedTableTest, Crc32cFileChecksum) {
   std::unique_ptr<InternalKeyComparator> comparator(
       new InternalKeyComparator(BytewiseComparator()));
   int level = 0;
-  std::vector<std::unique_ptr<IntTblPropCollectorFactory>>
+  IntTblPropCollectorFactories int_tbl_prop_collector_factories;
-      int_tbl_prop_collector_factories;
   std::string column_family_name;
 
   FileChecksumGenContext gen_context;
@@ -3403,8 +3400,7 @@ TEST_F(PlainTableTest, BasicPlainTableProperties) {
   const ImmutableOptions ioptions(options);
   const MutableCFOptions moptions(options);
   InternalKeyComparator ikc(options.comparator);
-  std::vector<std::unique_ptr<IntTblPropCollectorFactory>>
+  IntTblPropCollectorFactories int_tbl_prop_collector_factories;
-      int_tbl_prop_collector_factories;
   std::string column_family_name;
   int unknown_level = -1;
   std::unique_ptr<TableBuilder> builder(factory.NewTableBuilder(
@@ -3456,8 +3452,7 @@ TEST_F(PlainTableTest, NoFileChecksum) {
   const ImmutableOptions ioptions(options);
   const MutableCFOptions moptions(options);
   InternalKeyComparator ikc(options.comparator);
-  std::vector<std::unique_ptr<IntTblPropCollectorFactory>>
+  IntTblPropCollectorFactories int_tbl_prop_collector_factories;
-      int_tbl_prop_collector_factories;
   std::string column_family_name;
   int unknown_level = -1;
   FileChecksumTestHelper f(true);
@@ -3490,8 +3485,7 @@ TEST_F(PlainTableTest, Crc32cFileChecksum) {
   const ImmutableOptions ioptions(options);
   const MutableCFOptions moptions(options);
   InternalKeyComparator ikc(options.comparator);
-  std::vector<std::unique_ptr<IntTblPropCollectorFactory>>
+  IntTblPropCollectorFactories int_tbl_prop_collector_factories;
-      int_tbl_prop_collector_factories;
   std::string column_family_name;
   int unknown_level = -1;
 
@@ -4063,8 +4057,7 @@ TEST_P(BlockBasedTableTest, DISABLED_TableWithGlobalSeqno) {
   const ImmutableOptions ioptions(options);
   const MutableCFOptions moptions(options);
   InternalKeyComparator ikc(options.comparator);
-  std::vector<std::unique_ptr<IntTblPropCollectorFactory>>
+  IntTblPropCollectorFactories int_tbl_prop_collector_factories;
-      int_tbl_prop_collector_factories;
   int_tbl_prop_collector_factories.emplace_back(
       new SstFileWriterPropertiesCollectorFactory(2 /* version */,
                                                   0 /* global_seqno*/));
@@ -4251,8 +4244,7 @@ TEST_P(BlockBasedTableTest, BlockAlignTest) {
   const ImmutableOptions ioptions(options);
   const MutableCFOptions moptions(options);
   InternalKeyComparator ikc(options.comparator);
-  std::vector<std::unique_ptr<IntTblPropCollectorFactory>>
+  IntTblPropCollectorFactories int_tbl_prop_collector_factories;
-      int_tbl_prop_collector_factories;
   std::string column_family_name;
   std::unique_ptr<TableBuilder> builder(options.table_factory->NewTableBuilder(
       TableBuilderOptions(ioptions, moptions, ikc,
@@ -4344,8 +4336,7 @@ TEST_P(BlockBasedTableTest, PropertiesBlockRestartPointTest) {
   const ImmutableOptions ioptions(options);
   const MutableCFOptions moptions(options);
   InternalKeyComparator ikc(options.comparator);
-  std::vector<std::unique_ptr<IntTblPropCollectorFactory>>
+  IntTblPropCollectorFactories int_tbl_prop_collector_factories;
-      int_tbl_prop_collector_factories;
   std::string column_family_name;
 
   std::unique_ptr<TableBuilder> builder(options.table_factory->NewTableBuilder(

tools/sst_dump_test.cc

@@ -101,9 +101,7 @@ class SSTDumpToolTest : public testing::Test {
     ROCKSDB_NAMESPACE::InternalKeyComparator ikc(opts.comparator);
     std::unique_ptr<TableBuilder> tb;
 
-
+    IntTblPropCollectorFactories int_tbl_prop_collector_factories;
-    std::vector<std::unique_ptr<IntTblPropCollectorFactory> >
-        int_tbl_prop_collector_factories;
     std::unique_ptr<WritableFileWriter> file_writer;
     ASSERT_OK(WritableFileWriter::Create(test_env->GetFileSystem(), file_name,
                                          file_options, &file_writer, nullptr));