Rename IntTblPropCollector -> InternalTblPropColl (facebook#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: facebook#12320

Test Plan: existing tests, CI

Reviewed By: ajkr

Differential Revision: D53336945

Pulled By: pdillinger

fbshipit-source-id: 02535bcb30bbfb00e29e8478af62e5dad50a63b8

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) {

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<bool> 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_;

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(),

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 */),

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 */,

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,

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 */,

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<std::unique_ptr<IntTblPropCollectorFactory>>;
+using InternalTblPropCollFactories =
+    std::vector<std::unique_ptr<InternalTblPropCollFactory>>;
 
 // 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<TablePropertiesCollectorFactory> 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),

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<WritableFileWriter>* writable,
     std::unique_ptr<TableBuilder>* builder) {
   std::unique_ptr<FSWritableFile> 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::pair<std::string, ValueType>, std::string> kvs = {
@@ -265,15 +265,15 @@ void TestCustomizedTablePropertiesCollector(
   std::unique_ptr<WritableFileWriter> 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,15 +406,15 @@ 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);
   MutableCFOptions moptions(options);
 
   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");
     }

db/version_set_test.cc

@@ -3494,15 +3494,15 @@ class VersionSetTestMissingFiles : public VersionSetTestBase,
       ASSERT_OK(s);
       std::unique_ptr<WritableFileWriter> 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;
       std::unique_ptr<TableBuilder> builder(table_factory_->NewTableBuilder(
           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),

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.

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<FlushBlockPolicy> flush_block_policy;
 
-  std::vector<std::unique_ptr<IntTblPropCollector>> table_properties_collectors;
+  std::vector<std::unique_ptr<InternalTblPropColl>> table_properties_collectors;
 
   std::unique_ptr<ParallelCompressionRep> 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<IntTblPropCollector> collector{
-          factory->CreateIntTblPropCollector(tbo.column_family_id,
+      std::unique_ptr<InternalTblPropColl> collector{
+          factory->CreateInternalTblPropColl(tbo.column_family_id,
                                              tbo.level_at_creation)};
       if (collector) {
         table_properties_collectors.emplace_back(std::move(collector));

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<TableBuilder> table_builder(

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<TableBuilder> 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_),

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<TableBuilder> table_builder(table_factory_.NewTableBuilder(