Refactor: use TableBuilderOptions to reduce parameter lists (#8240)

Summary:
Greatly reduced the not-quite-copy-paste giant parameter lists
of rocksdb::NewTableBuilder, rocksdb::BuildTable,
BlockBasedTableBuilder::Rep ctor, and BlockBasedTableBuilder ctor.

Moved weird separate parameter `uint32_t column_family_id` of
TableFactory::NewTableBuilder into TableBuilderOptions.

Re-ordered parameters to TableBuilderOptions ctor, so that `uint64_t
target_file_size` is not randomly placed between uint64_t timestamps
(was easy to mix up).

Replaced a couple of fields of BlockBasedTableBuilder::Rep with a
FilterBuildingContext. The motivation for this change is making it
easier to pass along more data into new fields in FilterBuildingContext
(follow-up PR).

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

Test Plan: ASAN make check

Reviewed By: mrambacher

Differential Revision: D28075891

Pulled By: pdillinger

fbshipit-source-id: fddb3dbb8260a0e8bdcbb51b877ebabf9a690d4f

HISTORY.md

@@ -15,6 +15,9 @@
 ### Performace Improvements
 * BlockPrefetcher is used by iterators to prefetch data if they anticipate more data to be used in future. It is enabled implicitly by rocksdb. Added change to take in account read pattern if reads are sequential. This would disable prefetching for random reads in MultiGet and iterators as readahead_size is increased exponential doing large prefetches.
 
+### Public API change
+* Removed a parameter from TableFactory::NewTableBuilder, which should not be called by user code because TableBuilder is not a public API.
+
 ## 6.20.0 (04/16/2021)
 ### Behavior Changes
 * `ColumnFamilyOptions::sample_for_compression` now takes effect for creation of all block-based tables. Previously it only took effect for block-based tables created by flush.

db/builder.cc

@@ -45,62 +45,40 @@ namespace ROCKSDB_NAMESPACE {
 
 class TableFactory;
 
-TableBuilder* NewTableBuilder(
-    const ImmutableCFOptions& ioptions, const MutableCFOptions& moptions,
-    const InternalKeyComparator& internal_comparator,
-    const std::vector<std::unique_ptr<IntTblPropCollectorFactory>>*
-        int_tbl_prop_collector_factories,
-    uint32_t column_family_id, const std::string& column_family_name,
-    WritableFileWriter* file, const CompressionType compression_type,
-    const CompressionOptions& compression_opts, int level,
-    const bool skip_filters, const uint64_t creation_time,
-    const uint64_t oldest_key_time, const uint64_t target_file_size,
-    const uint64_t file_creation_time, const std::string& db_id,
-    const std::string& db_session_id) {
-  assert((column_family_id ==
+TableBuilder* NewTableBuilder(const TableBuilderOptions& tboptions,
+                              WritableFileWriter* file) {
+  assert((tboptions.column_family_id ==
           TablePropertiesCollectorFactory::Context::kUnknownColumnFamily) ==
-         column_family_name.empty());
-  return ioptions.table_factory->NewTableBuilder(
-      TableBuilderOptions(ioptions, moptions, internal_comparator,
-                          int_tbl_prop_collector_factories, compression_type,
-                          compression_opts, skip_filters, column_family_name,
-                          level, creation_time, oldest_key_time,
-                          target_file_size, file_creation_time, db_id,
-                          db_session_id),
-      column_family_id, file);
+         tboptions.column_family_name.empty());
+  return tboptions.ioptions.table_factory->NewTableBuilder(tboptions, file);
 }
 
 Status BuildTable(
     const std::string& dbname, VersionSet* versions,
-    const ImmutableDBOptions& db_options, const ImmutableCFOptions& ioptions,
-    const MutableCFOptions& mutable_cf_options, const FileOptions& file_options,
-    TableCache* table_cache, InternalIterator* iter,
+    const ImmutableDBOptions& db_options, const TableBuilderOptions& tboptions,
+    const FileOptions& file_options, TableCache* table_cache,
+    InternalIterator* iter,
     std::vector<std::unique_ptr<FragmentedRangeTombstoneIterator>>
         range_del_iters,
     FileMetaData* meta, std::vector<BlobFileAddition>* blob_file_additions,
-    const InternalKeyComparator& internal_comparator,
-    const std::vector<std::unique_ptr<IntTblPropCollectorFactory>>*
-        int_tbl_prop_collector_factories,
-    uint32_t column_family_id, const std::string& column_family_name,
     std::vector<SequenceNumber> snapshots,
     SequenceNumber earliest_write_conflict_snapshot,
-    SnapshotChecker* snapshot_checker, const CompressionType compression,
-    const CompressionOptions& compression_opts, bool paranoid_file_checks,
+    SnapshotChecker* snapshot_checker, bool paranoid_file_checks,
     InternalStats* internal_stats, TableFileCreationReason reason,
     IOStatus* io_status, const std::shared_ptr<IOTracer>& io_tracer,
     EventLogger* event_logger, int job_id, const Env::IOPriority io_priority,
-    TableProperties* table_properties, int level, const uint64_t creation_time,
-    const uint64_t oldest_key_time, Env::WriteLifeTimeHint write_hint,
-    const uint64_t file_creation_time, const std::string& db_id,
-    const std::string& db_session_id, const std::string* full_history_ts_low,
+    TableProperties* table_properties, Env::WriteLifeTimeHint write_hint,
+    const std::string* full_history_ts_low,
     BlobFileCompletionCallback* blob_callback) {
-  assert((column_family_id ==
+  assert((tboptions.column_family_id ==
           TablePropertiesCollectorFactory::Context::kUnknownColumnFamily) ==
-         column_family_name.empty());
+         tboptions.column_family_name.empty());
+  auto& mutable_cf_options = tboptions.moptions;
+  auto& ioptions = tboptions.ioptions;
   // Reports the IOStats for flush for every following bytes.
   const size_t kReportFlushIOStatsEvery = 1048576;
   OutputValidator output_validator(
-      internal_comparator,
+      tboptions.internal_comparator,
       /*enable_order_check=*/
       mutable_cf_options.check_flush_compaction_key_order,
       /*enable_hash=*/paranoid_file_checks);
@@ -108,7 +86,8 @@ Status BuildTable(
   meta->fd.file_size = 0;
   iter->SeekToFirst();
   std::unique_ptr<CompactionRangeDelAggregator> range_del_agg(
-      new CompactionRangeDelAggregator(&internal_comparator, snapshots));
+      new CompactionRangeDelAggregator(&tboptions.internal_comparator,
+                                       snapshots));
   for (auto& range_del_iter : range_del_iters) {
     range_del_agg->AddTombstones(std::move(range_del_iter));
   }
@@ -119,8 +98,9 @@ Status BuildTable(
   std::string file_checksum = kUnknownFileChecksum;
   std::string file_checksum_func_name = kUnknownFileChecksumFuncName;
 #ifndef ROCKSDB_LITE
-  EventHelpers::NotifyTableFileCreationStarted(
-      ioptions.listeners, dbname, column_family_name, fname, job_id, reason);
+  EventHelpers::NotifyTableFileCreationStarted(ioptions.listeners, dbname,
+                                               tboptions.column_family_name,
+                                               fname, job_id, reason);
 #endif  // !ROCKSDB_LITE
   Env* env = db_options.env;
   assert(env);
@@ -145,9 +125,10 @@ Status BuildTable(
       }
       if (!s.ok()) {
         EventHelpers::LogAndNotifyTableFileCreationFinished(
-            event_logger, ioptions.listeners, dbname, column_family_name, fname,
-            job_id, meta->fd, kInvalidBlobFileNumber, tp, reason, s,
-            file_checksum, file_checksum_func_name);
+            event_logger, ioptions.listeners, dbname,
+            tboptions.column_family_name, fname, job_id, meta->fd,
+            kInvalidBlobFileNumber, tp, reason, s, file_checksum,
+            file_checksum_func_name);
         return s;
       }
       FileTypeSet tmp_set = ioptions.checksum_handoff_file_types;
@@ -159,15 +140,10 @@ Status BuildTable(
           ioptions.file_checksum_gen_factory.get(),
           tmp_set.Contains(FileType::kTableFile)));
 
-      builder = NewTableBuilder(
-          ioptions, mutable_cf_options, internal_comparator,
-          int_tbl_prop_collector_factories, column_family_id,
-          column_family_name, file_writer.get(), compression, compression_opts,
-          level, false /* skip_filters */, creation_time, oldest_key_time,
-          0 /*target_file_size*/, file_creation_time, db_id, db_session_id);
+      builder = NewTableBuilder(tboptions, file_writer.get());
     }
 
-    MergeHelper merge(env, internal_comparator.user_comparator(),
+    MergeHelper merge(env, tboptions.internal_comparator.user_comparator(),
                       ioptions.merge_operator.get(), nullptr, ioptions.logger,
                       true /* internal key corruption is not ok */,
                       snapshots.empty() ? 0 : snapshots.back(),
@@ -176,16 +152,17 @@ Status BuildTable(
     std::unique_ptr<BlobFileBuilder> blob_file_builder(
         (mutable_cf_options.enable_blob_files && blob_file_additions)
             ? new BlobFileBuilder(versions, fs, &ioptions, &mutable_cf_options,
-                                  &file_options, job_id, column_family_id,
-                                  column_family_name, io_priority, write_hint,
-                                  io_tracer, blob_callback, &blob_file_paths,
-                                  blob_file_additions)
+                                  &file_options, job_id,
+                                  tboptions.column_family_id,
+                                  tboptions.column_family_name, io_priority,
+                                  write_hint, io_tracer, blob_callback,
+                                  &blob_file_paths, blob_file_additions)
             : nullptr);
 
     CompactionIterator c_iter(
-        iter, internal_comparator.user_comparator(), &merge, kMaxSequenceNumber,
-        &snapshots, earliest_write_conflict_snapshot, snapshot_checker, env,
-        ShouldReportDetailedTime(env, ioptions.stats),
+        iter, tboptions.internal_comparator.user_comparator(), &merge,
+        kMaxSequenceNumber, &snapshots, earliest_write_conflict_snapshot,
+        snapshot_checker, env, ShouldReportDetailedTime(env, ioptions.stats),
         true /* internal key corruption is not ok */, range_del_agg.get(),
         blob_file_builder.get(), ioptions.allow_data_in_errors,
         /*compaction=*/nullptr,
@@ -227,7 +204,8 @@ Status BuildTable(
         auto kv = tombstone.Serialize();
         builder->Add(kv.first.Encode(), kv.second);
         meta->UpdateBoundariesForRange(kv.first, tombstone.SerializeEndKey(),
-                                       tombstone.seq_, internal_comparator);
+                                       tombstone.seq_,
+                                       tboptions.internal_comparator);
       }
     }
 
@@ -296,20 +274,20 @@ Status BuildTable(
       // to cache it here for further user reads
       ReadOptions read_options;
       std::unique_ptr<InternalIterator> it(table_cache->NewIterator(
-          read_options, file_options, internal_comparator, *meta,
+          read_options, file_options, tboptions.internal_comparator, *meta,
           nullptr /* range_del_agg */,
           mutable_cf_options.prefix_extractor.get(), nullptr,
           (internal_stats == nullptr) ? nullptr
                                       : internal_stats->GetFileReadHist(0),
           TableReaderCaller::kFlush, /*arena=*/nullptr,
-          /*skip_filter=*/false, level,
+          /*skip_filter=*/false, tboptions.level,
           MaxFileSizeForL0MetaPin(mutable_cf_options),
           /*smallest_compaction_key=*/nullptr,
           /*largest_compaction_key*/ nullptr,
           /*allow_unprepared_value*/ false));
       s = it->status();
       if (s.ok() && paranoid_file_checks) {
-        OutputValidator file_validator(internal_comparator,
+        OutputValidator file_validator(tboptions.internal_comparator,
                                        /*enable_order_check=*/true,
                                        /*enable_hash=*/true);
         for (it->SeekToFirst(); it->Valid(); it->Next()) {
@@ -354,8 +332,8 @@ Status BuildTable(
   }
   // Output to event logger and fire events.
   EventHelpers::LogAndNotifyTableFileCreationFinished(
-      event_logger, ioptions.listeners, dbname, column_family_name, fname,
-      job_id, meta->fd, meta->oldest_blob_file_number, tp, reason, s,
+      event_logger, ioptions.listeners, dbname, tboptions.column_family_name,
+      fname, job_id, meta->fd, meta->oldest_blob_file_number, tp, reason, s,
       file_checksum, file_checksum_func_name);
 
   return s;

db/builder.h

@@ -24,37 +24,20 @@
 
 namespace ROCKSDB_NAMESPACE {
 
-struct Options;
 struct FileMetaData;
 
 class VersionSet;
-class Env;
-struct EnvOptions;
 class BlobFileAddition;
-class Iterator;
 class SnapshotChecker;
 class TableCache;
-class VersionEdit;
 class TableBuilder;
 class WritableFileWriter;
 class InternalStats;
 class BlobFileCompletionCallback;
 
-// @param column_family_name Name of the column family that is also identified
-//    by column_family_id, or empty string if unknown. It must outlive the
-//    TableBuilder returned by this function.
-TableBuilder* NewTableBuilder(
-    const ImmutableCFOptions& options, const MutableCFOptions& moptions,
-    const InternalKeyComparator& internal_comparator,
-    const std::vector<std::unique_ptr<IntTblPropCollectorFactory>>*
-        int_tbl_prop_collector_factories,
-    uint32_t column_family_id, const std::string& column_family_name,
-    WritableFileWriter* file, const CompressionType compression_type,
-    const CompressionOptions& compression_opts, int level,
-    const bool skip_filters = false, const uint64_t creation_time = 0,
-    const uint64_t oldest_key_time = 0, const uint64_t target_file_size = 0,
-    const uint64_t file_creation_time = 0, const std::string& db_id = "",
-    const std::string& db_session_id = "");
+// Convenience function for NewTableBuilder on the embedded table_factory.
+TableBuilder* NewTableBuilder(const TableBuilderOptions& tboptions,
+                              WritableFileWriter* file);
 
 // Build a Table file from the contents of *iter.  The generated file
 // will be named according to number specified in meta. On success, the rest of
@@ -66,29 +49,21 @@ TableBuilder* NewTableBuilder(
 //    by column_family_id, or empty string if unknown.
 extern Status BuildTable(
     const std::string& dbname, VersionSet* versions,
-    const ImmutableDBOptions& db_options, const ImmutableCFOptions& options,
-    const MutableCFOptions& mutable_cf_options, const FileOptions& file_options,
-    TableCache* table_cache, InternalIterator* iter,
+    const ImmutableDBOptions& db_options, const TableBuilderOptions& tboptions,
+    const FileOptions& file_options, TableCache* table_cache,
+    InternalIterator* iter,
     std::vector<std::unique_ptr<FragmentedRangeTombstoneIterator>>
         range_del_iters,
     FileMetaData* meta, std::vector<BlobFileAddition>* blob_file_additions,
-    const InternalKeyComparator& internal_comparator,
-    const std::vector<std::unique_ptr<IntTblPropCollectorFactory>>*
-        int_tbl_prop_collector_factories,
-    uint32_t column_family_id, const std::string& column_family_name,
     std::vector<SequenceNumber> snapshots,
     SequenceNumber earliest_write_conflict_snapshot,
-    SnapshotChecker* snapshot_checker, const CompressionType compression,
-    const CompressionOptions& compression_opts, bool paranoid_file_checks,
+    SnapshotChecker* snapshot_checker, bool paranoid_file_checks,
     InternalStats* internal_stats, TableFileCreationReason reason,
     IOStatus* io_status, const std::shared_ptr<IOTracer>& io_tracer,
     EventLogger* event_logger = nullptr, int job_id = 0,
     const Env::IOPriority io_priority = Env::IO_HIGH,
-    TableProperties* table_properties = nullptr, int level = -1,
-    const uint64_t creation_time = 0, const uint64_t oldest_key_time = 0,
+    TableProperties* table_properties = nullptr,
     Env::WriteLifeTimeHint write_hint = Env::WLTH_NOT_SET,
-    const uint64_t file_creation_time = 0, const std::string& db_id = "",
-    const std::string& db_session_id = "",
     const std::string* full_history_ts_low = nullptr,
     BlobFileCompletionCallback* blob_callback = nullptr);
 

db/compaction/compaction.h

@@ -162,7 +162,7 @@ class Compaction {
   CompressionType output_compression() const { return output_compression_; }
 
   // What compression options for output
-  CompressionOptions output_compression_opts() const {
+  const CompressionOptions& output_compression_opts() const {
     return output_compression_opts_;
   }
 

db/compaction/compaction_job.cc

@@ -1761,16 +1761,16 @@ Status CompactionJob::OpenCompactionOutputFile(
   bool skip_filters =
       cfd->ioptions()->optimize_filters_for_hits && bottommost_level_;
 
-  sub_compact->builder.reset(NewTableBuilder(
+  TableBuilderOptions tboptions(
       *cfd->ioptions(), *(sub_compact->compaction->mutable_cf_options()),
       cfd->internal_comparator(), cfd->int_tbl_prop_collector_factories(),
-      cfd->GetID(), cfd->GetName(), sub_compact->outfile.get(),
       sub_compact->compaction->output_compression(),
-      sub_compact->compaction->output_compression_opts(),
-      sub_compact->compaction->output_level(), skip_filters,
-      oldest_ancester_time, 0 /* oldest_key_time */,
-      sub_compact->compaction->max_output_file_size(), current_time, db_id_,
-      db_session_id_));
+      sub_compact->compaction->output_compression_opts(), skip_filters,
+      cfd->GetID(), cfd->GetName(), sub_compact->compaction->output_level(),
+      oldest_ancester_time, 0 /* oldest_key_time */, current_time, db_id_,
+      db_session_id_, sub_compact->compaction->max_output_file_size());
+  sub_compact->builder.reset(
+      NewTableBuilder(tboptions, sub_compact->outfile.get()));
   LogFlush(db_options_.info_log);
   return s;
 }

db/db_impl/db_impl_open.cc

@@ -1393,21 +1393,23 @@ Status DBImpl::WriteLevel0TableForRecovery(int job_id, ColumnFamilyData* cfd,
       }
 
       IOStatus io_s;
-      s = BuildTable(
-          dbname_, versions_.get(), immutable_db_options_, *cfd->ioptions(),
-          mutable_cf_options, file_options_for_compaction_, cfd->table_cache(),
-          iter.get(), std::move(range_del_iters), &meta, &blob_file_additions,
-          cfd->internal_comparator(), cfd->int_tbl_prop_collector_factories(),
-          cfd->GetID(), cfd->GetName(), snapshot_seqs,
-          earliest_write_conflict_snapshot, snapshot_checker,
+      TableBuilderOptions tboptions(
+          *cfd->ioptions(), mutable_cf_options, cfd->internal_comparator(),
+          cfd->int_tbl_prop_collector_factories(),
           GetCompressionFlush(*cfd->ioptions(), mutable_cf_options),
-          mutable_cf_options.compression_opts, paranoid_file_checks,
-          cfd->internal_stats(), TableFileCreationReason::kRecovery, &io_s,
-          io_tracer_, &event_logger_, job_id, Env::IO_HIGH,
-          nullptr /* table_properties */, 0 /* level */, current_time,
-          0 /* oldest_key_time */, write_hint, 0 /* file_creation_time */,
-          db_id_, db_session_id_, nullptr /*full_history_ts_low*/,
-          &blob_callback_);
+          mutable_cf_options.compression_opts, false /* skip_filters */,
+          cfd->GetID(), cfd->GetName(), 0 /* level */, current_time,
+          0 /* oldest_key_time */, 0 /* file_creation_time */, db_id_,
+          db_session_id_, 0 /* target_file_size */);
+      s = BuildTable(
+          dbname_, versions_.get(), immutable_db_options_, tboptions,
+          file_options_for_compaction_, cfd->table_cache(), iter.get(),
+          std::move(range_del_iters), &meta, &blob_file_additions,
+          snapshot_seqs, earliest_write_conflict_snapshot, snapshot_checker,
+          paranoid_file_checks, cfd->internal_stats(),
+          TableFileCreationReason::kRecovery, &io_s, io_tracer_, &event_logger_,
+          job_id, Env::IO_HIGH, nullptr /* table_properties */, write_hint,
+          nullptr /*full_history_ts_low*/, &blob_callback_);
       LogFlush(immutable_db_options_.info_log);
       ROCKS_LOG_DEBUG(immutable_db_options_.info_log,
                       "[%s] [WriteLevel0TableForRecovery]"

db/flush_job.cc

@@ -406,19 +406,22 @@ Status FlushJob::WriteLevel0Table() {
       IOStatus io_s;
       const std::string* const full_history_ts_low =
           (full_history_ts_low_.empty()) ? nullptr : &full_history_ts_low_;
+      TableBuilderOptions tboptions(
+          *cfd_->ioptions(), mutable_cf_options_, cfd_->internal_comparator(),
+          cfd_->int_tbl_prop_collector_factories(), output_compression_,
+          mutable_cf_options_.compression_opts, false /* skip_filters */,
+          cfd_->GetID(), cfd_->GetName(), 0 /* level */, creation_time,
+          oldest_key_time, current_time, db_id_, db_session_id_,
+          0 /* target_file_size */);
       s = BuildTable(
-          dbname_, versions_, db_options_, *cfd_->ioptions(),
-          mutable_cf_options_, file_options_, cfd_->table_cache(), iter.get(),
-          std::move(range_del_iters), &meta_, &blob_file_additions,
-          cfd_->internal_comparator(), cfd_->int_tbl_prop_collector_factories(),
-          cfd_->GetID(), cfd_->GetName(), existing_snapshots_,
+          dbname_, versions_, db_options_, tboptions, file_options_,
+          cfd_->table_cache(), iter.get(), std::move(range_del_iters), &meta_,
+          &blob_file_additions, existing_snapshots_,
           earliest_write_conflict_snapshot_, snapshot_checker_,
-          output_compression_, mutable_cf_options_.compression_opts,
           mutable_cf_options_.paranoid_file_checks, cfd_->internal_stats(),
           TableFileCreationReason::kFlush, &io_s, io_tracer_, event_logger_,
-          job_context_->job_id, Env::IO_HIGH, &table_properties_, 0 /* level */,
-          creation_time, oldest_key_time, write_hint, current_time, db_id_,
-          db_session_id_, full_history_ts_low, blob_callback_);
+          job_context_->job_id, Env::IO_HIGH, &table_properties_, write_hint,
+          full_history_ts_low, blob_callback_);
       if (!io_s.ok()) {
         io_status_ = io_s;
       }

db/repair.cc

@@ -440,19 +440,23 @@ class Repairer {
       }
 
       IOStatus io_s;
+      CompressionOptions default_compression;
+      TableBuilderOptions tboptions(
+          *cfd->ioptions(), *cfd->GetLatestMutableCFOptions(),
+          cfd->internal_comparator(), cfd->int_tbl_prop_collector_factories(),
+          kNoCompression, default_compression, false /* skip_filters */,
+          cfd->GetID(), cfd->GetName(), -1 /* level */, current_time,
+          0 /* oldest_key_time */, 0 /* file_creation_time */,
+          "DB Repairer" /* db_id */, db_session_id_, 0 /*target_file_size*/);
       status = BuildTable(
-          dbname_, /* versions */ nullptr, immutable_db_options_,
-          *cfd->ioptions(), *cfd->GetLatestMutableCFOptions(), env_options_,
-          table_cache_.get(), iter.get(), std::move(range_del_iters), &meta,
-          nullptr /* blob_file_additions */, cfd->internal_comparator(),
-          cfd->int_tbl_prop_collector_factories(), cfd->GetID(), cfd->GetName(),
-          {}, kMaxSequenceNumber, snapshot_checker, kNoCompression,
-          CompressionOptions(), false, nullptr /* internal_stats */,
+          dbname_, /* versions */ nullptr, immutable_db_options_, tboptions,
+          env_options_, table_cache_.get(), iter.get(),
+          std::move(range_del_iters), &meta, nullptr /* blob_file_additions */,
+          {}, kMaxSequenceNumber, snapshot_checker,
+          false /* paranoid_file_checks*/, nullptr /* internal_stats */,
           TableFileCreationReason::kRecovery, &io_s, nullptr /*IOTracer*/,
           nullptr /* event_logger */, 0 /* job_id */, Env::IO_HIGH,
-          nullptr /* table_properties */, -1 /* level */, current_time,
-          0 /* oldest_key_time */, write_hint, 0 /* file_creation_time */,
-          "DB Repairer" /* db_id */, db_session_id_);
+          nullptr /* table_properties */, write_hint);
       ROCKS_LOG_INFO(db_options_.info_log,
                      "Log #%" PRIu64 ": %d ops saved to Table #%" PRIu64 " %s",
                      log, counter, meta.fd.GetNumber(),

db/table_properties_collector_test.cc

@@ -52,10 +52,11 @@ void MakeBuilder(const Options& options, const ImmutableCFOptions& ioptions,
   writable->reset(
       new WritableFileWriter(std::move(wf), "" /* don't care */, EnvOptions()));
   int unknown_level = -1;
-  builder->reset(NewTableBuilder(
+  TableBuilderOptions tboptions(
       ioptions, moptions, internal_comparator, int_tbl_prop_collector_factories,
-      kTestColumnFamilyId, kTestColumnFamilyName, writable->get(),
-      options.compression, options.compression_opts, unknown_level));
+      options.compression, options.compression_opts, false /*skip_filters*/,
+      kTestColumnFamilyId, kTestColumnFamilyName, unknown_level);
+  builder->reset(NewTableBuilder(tboptions, writable->get()));
 }
 }  // namespace
 

db/version_set_test.cc

@@ -2782,8 +2782,9 @@ class VersionSetTestMissingFiles : public VersionSetTestBase,
               immutable_cf_options_, mutable_cf_options_, *internal_comparator_,
               &int_tbl_prop_collector_factories, kNoCompression,
               CompressionOptions(),
-              /*_skip_filters=*/false, info.column_family, info.level),
-          TablePropertiesCollectorFactory::Context::kUnknownColumnFamily,
+              /*_skip_filters=*/false,
+              TablePropertiesCollectorFactory::Context::kUnknownColumnFamily,
+              info.column_family, info.level),
           fwriter.get()));
       InternalKey ikey(info.key, 0, ValueType::kTypeValue);
       builder->Add(ikey.Encode(), "value");

include/rocksdb/filter_policy.h

@@ -100,6 +100,7 @@ struct FilterBuildingContext {
   const BlockBasedTableOptions& table_options;
 
   // Name of the column family for the table (or empty string if unknown)
+  // TODO: consider changing to Slice
   std::string column_family_name;
 
   // The compactions style in effect for the table

include/rocksdb/table.h

@@ -711,7 +711,7 @@ class TableFactory : public Customizable {
   // to use in this table.
   virtual TableBuilder* NewTableBuilder(
       const TableBuilderOptions& table_builder_options,
-      uint32_t column_family_id, WritableFileWriter* file) const = 0;
+      WritableFileWriter* file) const = 0;
 
   // Return is delete range supported
   virtual bool IsDeleteRangeSupported() const { return false; }

table/adaptive/adaptive_table_factory.cc

@@ -71,10 +71,9 @@ Status AdaptiveTableFactory::NewTableReader(
 }
 
 TableBuilder* AdaptiveTableFactory::NewTableBuilder(
-    const TableBuilderOptions& table_builder_options, uint32_t column_family_id,
+    const TableBuilderOptions& table_builder_options,
     WritableFileWriter* file) const {
-  return table_factory_to_write_->NewTableBuilder(table_builder_options,
-                                                  column_family_id, file);
+  return table_factory_to_write_->NewTableBuilder(table_builder_options, file);
 }
 
 std::string AdaptiveTableFactory::GetPrintableOptions() const {

table/adaptive/adaptive_table_factory.h

@@ -42,7 +42,7 @@ class AdaptiveTableFactory : public TableFactory {
 
   TableBuilder* NewTableBuilder(
       const TableBuilderOptions& table_builder_options,
-      uint32_t column_family_id, WritableFileWriter* file) const override;
+      WritableFileWriter* file) const override;
 
   std::string GetPrintableOptions() const override;
 

table/block_based/block_based_table_builder.cc

@@ -27,6 +27,7 @@
 #include "rocksdb/cache.h"
 #include "rocksdb/comparator.h"
 #include "rocksdb/env.h"
+#include "rocksdb/filter_policy.h"
 #include "rocksdb/flush_block_policy.h"
 #include "rocksdb/merge_operator.h"
 #include "rocksdb/table.h"
@@ -320,9 +321,8 @@ struct BlockBasedTableBuilder::Rep {
 
   std::string compressed_output;
   std::unique_ptr<FlushBlockPolicy> flush_block_policy;
-  int level_at_creation;
   uint32_t column_family_id;
-  const std::string& column_family_name;
+  std::string column_family_name;
   uint64_t creation_time = 0;
   uint64_t oldest_key_time = 0;
   uint64_t file_creation_time = 0;
@@ -399,22 +399,12 @@ struct BlockBasedTableBuilder::Rep {
     }
   }
 
-  Rep(const ImmutableCFOptions& _ioptions, const MutableCFOptions& _moptions,
-      const BlockBasedTableOptions& table_opt,
-      const InternalKeyComparator& icomparator,
-      const std::vector<std::unique_ptr<IntTblPropCollectorFactory>>*
-          int_tbl_prop_collector_factories,
-      uint32_t _column_family_id, WritableFileWriter* f,
-      const CompressionType _compression_type,
-      const CompressionOptions& _compression_opts, const bool skip_filters,
-      const int _level_at_creation, const std::string& _column_family_name,
-      const uint64_t _creation_time, const uint64_t _oldest_key_time,
-      const uint64_t target_file_size, const uint64_t _file_creation_time,
-      const std::string& _db_id, const std::string& _db_session_id)
-      : ioptions(_ioptions),
-        moptions(_moptions),
+  Rep(const BlockBasedTableOptions& table_opt, const TableBuilderOptions& tbo,
+      WritableFileWriter* f)
+      : ioptions(tbo.ioptions),
+        moptions(tbo.moptions),
         table_options(table_opt),
-        internal_comparator(icomparator),
+        internal_comparator(tbo.internal_comparator),
         file(f),
         offset(0),
         alignment(table_options.block_align
@@ -423,51 +413,50 @@ struct BlockBasedTableBuilder::Rep {
         data_block(table_options.block_restart_interval,
                    table_options.use_delta_encoding,
                    false /* use_value_delta_encoding */,
-                   icomparator.user_comparator()
+                   tbo.internal_comparator.user_comparator()
                            ->CanKeysWithDifferentByteContentsBeEqual()
                        ? BlockBasedTableOptions::kDataBlockBinarySearch
                        : table_options.data_block_index_type,
                    table_options.data_block_hash_table_util_ratio),
         range_del_block(1 /* block_restart_interval */),
-        internal_prefix_transform(_moptions.prefix_extractor.get()),
-        compression_type(_compression_type),
-        sample_for_compression(_moptions.sample_for_compression),
+        internal_prefix_transform(tbo.moptions.prefix_extractor.get()),
+        compression_type(tbo.compression_type),
+        sample_for_compression(tbo.moptions.sample_for_compression),
         compressible_input_data_bytes(0),
         uncompressible_input_data_bytes(0),
         sampled_input_data_bytes(0),
         sampled_output_slow_data_bytes(0),
         sampled_output_fast_data_bytes(0),
-        compression_opts(_compression_opts),
+        compression_opts(tbo.compression_opts),
         compression_dict(),
-        compression_ctxs(_compression_opts.parallel_threads),
-        verify_ctxs(_compression_opts.parallel_threads),
+        compression_ctxs(tbo.compression_opts.parallel_threads),
+        verify_ctxs(tbo.compression_opts.parallel_threads),
         verify_dict(),
-        state((_compression_opts.max_dict_bytes > 0) ? State::kBuffered
-                                                     : State::kUnbuffered),
+        state((tbo.compression_opts.max_dict_bytes > 0) ? State::kBuffered
+                                                        : State::kUnbuffered),
         use_delta_encoding_for_index_values(table_opt.format_version >= 4 &&
                                             !table_opt.block_align),
         compressed_cache_key_prefix_size(0),
         flush_block_policy(
             table_options.flush_block_policy_factory->NewFlushBlockPolicy(
                 table_options, data_block)),
-        level_at_creation(_level_at_creation),
-        column_family_id(_column_family_id),
-        column_family_name(_column_family_name),
-        creation_time(_creation_time),
-        oldest_key_time(_oldest_key_time),
-        file_creation_time(_file_creation_time),
-        db_id(_db_id),
-        db_session_id(_db_session_id),
+        column_family_id(tbo.column_family_id),
+        column_family_name(tbo.column_family_name),
+        creation_time(tbo.creation_time),
+        oldest_key_time(tbo.oldest_key_time),
+        file_creation_time(tbo.file_creation_time),
+        db_id(tbo.db_id),
+        db_session_id(tbo.db_session_id),
         db_host_id(ioptions.db_host_id),
         status_ok(true),
         io_status_ok(true) {
-    if (target_file_size == 0) {
+    if (tbo.target_file_size == 0) {
       buffer_limit = compression_opts.max_dict_buffer_bytes;
     } else if (compression_opts.max_dict_buffer_bytes == 0) {
-      buffer_limit = target_file_size;
+      buffer_limit = tbo.target_file_size;
     } else {
-      buffer_limit =
-          std::min(target_file_size, compression_opts.max_dict_buffer_bytes);
+      buffer_limit = std::min(tbo.target_file_size,
+                              compression_opts.max_dict_buffer_bytes);
     }
     for (uint32_t i = 0; i < compression_opts.parallel_threads; i++) {
       compression_ctxs[i].reset(new CompressionContext(compression_type));
@@ -484,27 +473,29 @@ struct BlockBasedTableBuilder::Rep {
           &this->internal_prefix_transform, use_delta_encoding_for_index_values,
           table_options));
     }
-    if (skip_filters) {
+    if (tbo.skip_filters) {
       filter_builder = nullptr;
     } else {
-      FilterBuildingContext context(table_options);
-      context.column_family_name = column_family_name;
-      context.compaction_style = ioptions.compaction_style;
-      context.level_at_creation = level_at_creation;
-      context.info_log = ioptions.logger;
+      FilterBuildingContext filter_context(table_options);
+
+      filter_context.level_at_creation = tbo.level;
+      filter_context.column_family_name = column_family_name;
+      filter_context.compaction_style = ioptions.compaction_style;
+      filter_context.info_log = ioptions.logger;
+
       filter_builder.reset(CreateFilterBlockBuilder(
-          ioptions, moptions, context, use_delta_encoding_for_index_values,
-          p_index_builder_));
+          ioptions, moptions, filter_context,
+          use_delta_encoding_for_index_values, p_index_builder_));
     }
 
-    for (auto& collector_factories : *int_tbl_prop_collector_factories) {
+    for (auto& collector_factories : *tbo.int_tbl_prop_collector_factories) {
       table_properties_collectors.emplace_back(
           collector_factories->CreateIntTblPropCollector(column_family_id));
     }
     table_properties_collectors.emplace_back(
         new BlockBasedTablePropertiesCollector(
             table_options.index_type, table_options.whole_key_filtering,
-            _moptions.prefix_extractor != nullptr));
+            moptions.prefix_extractor != nullptr));
     if (table_options.verify_compression) {
       for (uint32_t i = 0; i < compression_opts.parallel_threads; i++) {
         verify_ctxs[i].reset(new UncompressionContext(compression_type));
@@ -839,23 +830,13 @@ struct BlockBasedTableBuilder::ParallelCompressionRep {
 };
 
 BlockBasedTableBuilder::BlockBasedTableBuilder(
-    const ImmutableCFOptions& ioptions, const MutableCFOptions& moptions,
-    const BlockBasedTableOptions& table_options,
-    const InternalKeyComparator& internal_comparator,
-    const std::vector<std::unique_ptr<IntTblPropCollectorFactory>>*
-        int_tbl_prop_collector_factories,
-    uint32_t column_family_id, WritableFileWriter* file,
-    const CompressionType compression_type,
-    const CompressionOptions& compression_opts, const bool skip_filters,
-    const std::string& column_family_name, const int level_at_creation,
-    const uint64_t creation_time, const uint64_t oldest_key_time,
-    const uint64_t target_file_size, const uint64_t file_creation_time,
-    const std::string& db_id, const std::string& db_session_id) {
+    const BlockBasedTableOptions& table_options, const TableBuilderOptions& tbo,
+    WritableFileWriter* file) {
   BlockBasedTableOptions sanitized_table_options(table_options);
   if (sanitized_table_options.format_version == 0 &&
       sanitized_table_options.checksum != kCRC32c) {
     ROCKS_LOG_WARN(
-        ioptions.logger,
+        tbo.ioptions.logger,
         "Silently converting format_version to 1 because checksum is "
         "non-default");
     // silently convert format_version to 1 to keep consistent with current
@@ -863,12 +844,7 @@ BlockBasedTableBuilder::BlockBasedTableBuilder(
     sanitized_table_options.format_version = 1;
   }
 
-  rep_ = new Rep(ioptions, moptions, sanitized_table_options,
-                 internal_comparator, int_tbl_prop_collector_factories,
-                 column_family_id, file, compression_type, compression_opts,
-                 skip_filters, level_at_creation, column_family_name,
-                 creation_time, oldest_key_time, target_file_size,
-                 file_creation_time, db_id, db_session_id);
+  rep_ = new Rep(sanitized_table_options, tbo, file);
 
   if (rep_->filter_builder != nullptr) {
     rep_->filter_builder->StartBlock(0);

table/block_based/block_based_table_builder.h

@@ -38,20 +38,9 @@ class BlockBasedTableBuilder : public TableBuilder {
   // Create a builder that will store the contents of the table it is
   // building in *file.  Does not close the file.  It is up to the
   // caller to close the file after calling Finish().
-  BlockBasedTableBuilder(
-      const ImmutableCFOptions& ioptions, const MutableCFOptions& moptions,
-      const BlockBasedTableOptions& table_options,
-      const InternalKeyComparator& internal_comparator,
-      const std::vector<std::unique_ptr<IntTblPropCollectorFactory>>*
-          int_tbl_prop_collector_factories,
-      uint32_t column_family_id, WritableFileWriter* file,
-      const CompressionType compression_type,
-      const CompressionOptions& compression_opts, const bool skip_filters,
-      const std::string& column_family_name, const int level_at_creation,
-      const uint64_t creation_time = 0, const uint64_t oldest_key_time = 0,
-      const uint64_t target_file_size = 0,
-      const uint64_t file_creation_time = 0, const std::string& db_id = "",
-      const std::string& db_session_id = "");
+  BlockBasedTableBuilder(const BlockBasedTableOptions& table_options,
+                         const TableBuilderOptions& table_builder_options,
+                         WritableFileWriter* file);
 
   // No copying allowed
   BlockBasedTableBuilder(const BlockBasedTableBuilder&) = delete;

table/block_based/block_based_table_factory.cc

@@ -492,23 +492,10 @@ Status BlockBasedTableFactory::NewTableReader(
 }
 
 TableBuilder* BlockBasedTableFactory::NewTableBuilder(
-    const TableBuilderOptions& table_builder_options, uint32_t column_family_id,
+    const TableBuilderOptions& table_builder_options,
     WritableFileWriter* file) const {
-  auto table_builder = new BlockBasedTableBuilder(
-      table_builder_options.ioptions, table_builder_options.moptions,
-      table_options_, table_builder_options.internal_comparator,
-      table_builder_options.int_tbl_prop_collector_factories, column_family_id,
-      file, table_builder_options.compression_type,
-      table_builder_options.compression_opts,
-      table_builder_options.skip_filters,
-      table_builder_options.column_family_name, table_builder_options.level,
-      table_builder_options.creation_time,
-      table_builder_options.oldest_key_time,
-      table_builder_options.target_file_size,
-      table_builder_options.file_creation_time, table_builder_options.db_id,
-      table_builder_options.db_session_id);
-
-  return table_builder;
+  return new BlockBasedTableBuilder(table_options_, table_builder_options,
+                                    file);
 }
 
 Status BlockBasedTableFactory::ValidateOptions(

table/block_based/block_based_table_factory.h

@@ -60,7 +60,7 @@ class BlockBasedTableFactory : public TableFactory {
 
   TableBuilder* NewTableBuilder(
       const TableBuilderOptions& table_builder_options,
-      uint32_t column_family_id, WritableFileWriter* file) const override;
+      WritableFileWriter* file) const override;
 
   // Valdates the specified DB Options.
   Status ValidateOptions(const DBOptions& db_opts,

table/block_based/block_based_table_reader_test.cc

@@ -67,9 +67,9 @@ class BlockBasedTableReaderTest
     std::unique_ptr<TableBuilder> table_builder(table_factory_->NewTableBuilder(
         TableBuilderOptions(ioptions, moptions, comparator, &factories,
                             compression_type, CompressionOptions(),
-                            false /* skip_filters */, kDefaultColumnFamilyName,
-                            -1 /* level */),
-        0 /* column_family_id */, writer.get()));
+                            false /* skip_filters */, 0 /* column_family_id */,
+                            kDefaultColumnFamilyName, -1 /* level */),
+        writer.get()));
 
     // Build table.
     for (auto it = kv.begin(); it != kv.end(); it++) {

table/block_based/data_block_hash_index_test.cc

@@ -555,11 +555,12 @@ void TestBoundary(InternalKey& ik1, std::string& v1, InternalKey& ik2,
       int_tbl_prop_collector_factories;
   std::string column_family_name;
   builder.reset(ioptions.table_factory->NewTableBuilder(
-      TableBuilderOptions(ioptions, moptions, internal_comparator,
-                          &int_tbl_prop_collector_factories,
-                          options.compression, CompressionOptions(),
-                          false /* skip_filters */, column_family_name, level_),
-      TablePropertiesCollectorFactory::Context::kUnknownColumnFamily,
+      TableBuilderOptions(
+          ioptions, moptions, internal_comparator,
+          &int_tbl_prop_collector_factories, options.compression,
+          CompressionOptions(), false /* skip_filters */,
+          TablePropertiesCollectorFactory::Context::kUnknownColumnFamily,
+          column_family_name, level_),
       file_writer.get()));
 
   builder->Add(ik1.Encode().ToString(), v1);

table/block_fetcher_test.cc

@@ -101,9 +101,9 @@ class BlockFetcherTest : public testing::Test {
     std::unique_ptr<TableBuilder> table_builder(table_factory_.NewTableBuilder(
         TableBuilderOptions(ioptions, moptions, comparator, &factories,
                             compression_type, CompressionOptions(),
-                            false /* skip_filters */, kDefaultColumnFamilyName,
-                            -1 /* level */),
-        0 /* column_family_id */, writer.get()));
+                            false /* skip_filters */, 0 /* column_family_id */,
+                            kDefaultColumnFamilyName, -1 /* level */),
+        writer.get()));
 
     // Build table.
     for (int i = 0; i < 9; i++) {

table/cuckoo/cuckoo_table_factory.cc

@@ -30,11 +30,8 @@ Status CuckooTableFactory::NewTableReader(
 }
 
 TableBuilder* CuckooTableFactory::NewTableBuilder(
-    const TableBuilderOptions& table_builder_options, uint32_t column_family_id,
+    const TableBuilderOptions& table_builder_options,
     WritableFileWriter* file) const {
-  // Ignore the skipFIlters flag. Does not apply to this file format
-  //
-
   // TODO: change builder to take the option struct
   return new CuckooTableBuilder(
       file, table_options_.hash_table_ratio, 64,
@@ -42,8 +39,9 @@ TableBuilder* CuckooTableFactory::NewTableBuilder(
       table_builder_options.internal_comparator.user_comparator(),
       table_options_.cuckoo_block_size, table_options_.use_module_hash,
       table_options_.identity_as_first_hash, nullptr /* get_slice_hash */,
-      column_family_id, table_builder_options.column_family_name,
-      table_builder_options.db_id, table_builder_options.db_session_id);
+      table_builder_options.column_family_id,
+      table_builder_options.column_family_name, table_builder_options.db_id,
+      table_builder_options.db_session_id);
 }
 
 std::string CuckooTableFactory::GetPrintableOptions() const {

table/cuckoo/cuckoo_table_factory.h

@@ -69,7 +69,7 @@ class CuckooTableFactory : public TableFactory {
 
   TableBuilder* NewTableBuilder(
       const TableBuilderOptions& table_builder_options,
-      uint32_t column_family_id, WritableFileWriter* file) const override;
+      WritableFileWriter* file) const override;
 
   std::string GetPrintableOptions() const override;
 

table/mock_table.cc

@@ -255,7 +255,7 @@ Status MockTableFactory::NewTableReader(
 
 TableBuilder* MockTableFactory::NewTableBuilder(
     const TableBuilderOptions& /*table_builder_options*/,
-    uint32_t /*column_family_id*/, WritableFileWriter* file) const {
+    WritableFileWriter* file) const {
   uint32_t id;
   Status s = GetAndWriteNextID(file, &id);
   assert(s.ok());

table/mock_table.h

@@ -58,7 +58,7 @@ class MockTableFactory : public TableFactory {
       bool prefetch_index_and_filter_in_cache = true) const override;
   TableBuilder* NewTableBuilder(
       const TableBuilderOptions& table_builder_options,
-      uint32_t column_familly_id, WritableFileWriter* file) const override;
+      WritableFileWriter* file) const override;
 
   // This function will directly create mock table instead of going through
   // MockTableBuilder. file_contents has to have a format of <internal_key,

table/plain/plain_table_factory.cc

@@ -70,7 +70,7 @@ Status PlainTableFactory::NewTableReader(
 }
 
 TableBuilder* PlainTableFactory::NewTableBuilder(
-    const TableBuilderOptions& table_builder_options, uint32_t column_family_id,
+    const TableBuilderOptions& table_builder_options,
     WritableFileWriter* file) const {
   // Ignore the skip_filters flag. PlainTable format is optimized for small
   // in-memory dbs. The skip_filters optimization is not useful for plain
@@ -78,9 +78,10 @@ TableBuilder* PlainTableFactory::NewTableBuilder(
   //
   return new PlainTableBuilder(
       table_builder_options.ioptions, table_builder_options.moptions,
-      table_builder_options.int_tbl_prop_collector_factories, column_family_id,
-      file, table_options_.user_key_len, table_options_.encoding_type,
-      table_options_.index_sparseness, table_options_.bloom_bits_per_key,
+      table_builder_options.int_tbl_prop_collector_factories,
+      table_builder_options.column_family_id, file, table_options_.user_key_len,
+      table_options_.encoding_type, table_options_.index_sparseness,
+      table_options_.bloom_bits_per_key,
       table_builder_options.column_family_name, 6,
       table_options_.huge_page_tlb_size, table_options_.hash_table_ratio,
       table_options_.store_index_in_file, table_builder_options.db_id,

table/plain/plain_table_factory.h

@@ -168,7 +168,7 @@ class PlainTableFactory : public TableFactory {
 
   TableBuilder* NewTableBuilder(
       const TableBuilderOptions& table_builder_options,
-      uint32_t column_family_id, WritableFileWriter* file) const override;
+      WritableFileWriter* file) const override;
 
   std::string GetPrintableOptions() const override;
   static const char kValueTypeSeqId0 = char(~0);

table/sst_file_dumper.cc

@@ -207,7 +207,6 @@ Status SstFileDumper::CalculateCompressedTableSize(
   std::unique_ptr<TableBuilder> table_builder;
   table_builder.reset(block_based_tf.NewTableBuilder(
       tb_options,
-      TablePropertiesCollectorFactory::Context::kUnknownColumnFamily,
       dest_writer.get()));
   std::unique_ptr<InternalIterator> iter(table_reader_->NewIterator(
       read_options_, moptions_.prefix_extractor.get(), /*arena=*/nullptr,
@@ -274,10 +273,11 @@ Status SstFileDumper::ShowCompressionSize(
 
   std::string column_family_name;
   int unknown_level = -1;
-  TableBuilderOptions tb_opts(imoptions, moptions, ikc,
-                              &block_based_table_factories, compress_type,
-                              compress_opt, false /* skip_filters */,
-                              column_family_name, unknown_level);
+  TableBuilderOptions tb_opts(
+      imoptions, moptions, ikc, &block_based_table_factories, compress_type,
+      compress_opt, false /* skip_filters */,
+      TablePropertiesCollectorFactory::Context::kUnknownColumnFamily,
+      column_family_name, unknown_level);
   uint64_t num_data_blocks = 0;
   std::chrono::steady_clock::time_point start =
       std::chrono::steady_clock::now();

table/sst_file_writer.cc

@@ -253,9 +253,10 @@ Status SstFileWriter::Open(const std::string& file_path) {
   TableBuilderOptions table_builder_options(
       r->ioptions, r->mutable_cf_options, r->internal_comparator,
       &int_tbl_prop_collector_factories, compression_type, compression_opts,
-      r->skip_filters, r->column_family_name, unknown_level,
-      0 /* creation_time */, 0 /* oldest_key_time */, 0 /* target_file_size */,
-      0 /* file_creation_time */, "SST Writer" /* db_id */, db_session_id);
+      r->skip_filters, cf_id, r->column_family_name, unknown_level,
+      0 /* creation_time */, 0 /* oldest_key_time */,
+      0 /* file_creation_time */, "SST Writer" /* db_id */, db_session_id,
+      0 /* target_file_size */);
   FileTypeSet tmp_set = r->ioptions.checksum_handoff_file_types;
   r->file_writer.reset(new WritableFileWriter(
       std::move(sst_file), file_path, r->env_options, r->ioptions.clock,
@@ -266,7 +267,7 @@ Status SstFileWriter::Open(const std::string& file_path) {
   // TODO(tec) : If table_factory is using compressed block cache, we will
   // be adding the external sst file blocks into it, which is wasteful.
   r->builder.reset(r->ioptions.table_factory->NewTableBuilder(
-      table_builder_options, cf_id, r->file_writer.get()));
+      table_builder_options, r->file_writer.get()));
 
   r->file_info = ExternalSstFileInfo();
   r->file_info.file_path = file_path;

table/table_builder.h

@@ -94,11 +94,12 @@ struct TableBuilderOptions {
           _int_tbl_prop_collector_factories,
       CompressionType _compression_type,
       const CompressionOptions& _compression_opts, bool _skip_filters,
-      const std::string& _column_family_name, int _level,
-      const uint64_t _creation_time = 0, const int64_t _oldest_key_time = 0,
-      const uint64_t _target_file_size = 0,
+      uint32_t _column_family_id, const std::string& _column_family_name,
+      int _level, 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 std::string& _db_session_id = "",
+      const uint64_t _target_file_size = 0)
       : ioptions(_ioptions),
         moptions(_moptions),
         internal_comparator(_internal_comparator),
@@ -106,6 +107,7 @@ struct TableBuilderOptions {
         compression_type(_compression_type),
         compression_opts(_compression_opts),
         skip_filters(_skip_filters),
+        column_family_id(_column_family_id),
         column_family_name(_column_family_name),
         level(_level),
         creation_time(_creation_time),
@@ -120,11 +122,13 @@ struct TableBuilderOptions {
   const InternalKeyComparator& internal_comparator;
   const std::vector<std::unique_ptr<IntTblPropCollectorFactory>>*
       int_tbl_prop_collector_factories;
-  CompressionType compression_type;
+  const CompressionType compression_type;
   const CompressionOptions& compression_opts;
-  bool skip_filters;  // only used by BlockBasedTableBuilder
+  const bool skip_filters;  // only used by BlockBasedTableBuilder
+  const uint32_t column_family_id;
   const std::string& column_family_name;
-  int level; // what level this table/file is on, -1 for "not set, don't know"
+  // what level this table/file is on, -1 for "not set, don't know"
+  const int level;
   const uint64_t creation_time;
   const int64_t oldest_key_time;
   const uint64_t target_file_size;

table/table_reader_bench.cc

@@ -103,8 +103,9 @@ void TableReaderBenchmark(Options& opts, EnvOptions& env_options,
         TableBuilderOptions(
             ioptions, moptions, ikc, &int_tbl_prop_collector_factories,
             CompressionType::kNoCompression, CompressionOptions(),
-            false /* skip_filters */, kDefaultColumnFamilyName, unknown_level),
-        0 /* column_family_id */, file_writer.get());
+            false /* skip_filters */, 0 /* column_family_id */,
+            kDefaultColumnFamilyName, unknown_level),
+        file_writer.get());
   } else {
     s = DB::Open(opts, dbname, &db);
     ASSERT_OK(s);

table/table_test.cc

@@ -155,6 +155,9 @@ void Increment(const Comparator* cmp, std::string* key) {
   }
 }
 
+const auto kUnknownColumnFamily =
+    TablePropertiesCollectorFactory::Context::kUnknownColumnFamily;
+
 }  // namespace
 
 // Helper class for tests to unify the interface between
@@ -364,9 +367,8 @@ class TableConstructor : public Constructor {
         TableBuilderOptions(ioptions, moptions, internal_comparator,
                             &int_tbl_prop_collector_factories,
                             options.compression, options.compression_opts,
-                            false /* skip_filters */, column_family_name,
-                            level_),
-        TablePropertiesCollectorFactory::Context::kUnknownColumnFamily,
+                            false /* skip_filters */, kUnknownColumnFamily,
+                            column_family_name, level_),
         file_writer_.get()));
 
     for (const auto& kv : kv_map) {
@@ -3324,8 +3326,8 @@ TEST_P(BlockBasedTableTest, NoFileChecksum) {
       TableBuilderOptions(ioptions, moptions, *comparator,
                           &int_tbl_prop_collector_factories,
                           options.compression, options.compression_opts,
-                          false /* skip_filters */, column_family_name, level),
-      TablePropertiesCollectorFactory::Context::kUnknownColumnFamily,
+                          false /* skip_filters */, kUnknownColumnFamily,
+                          column_family_name, level),
       f.GetFileWriter()));
   ASSERT_OK(f.ResetTableBuilder(std::move(builder)));
   f.AddKVtoKVMap(1000);
@@ -3362,8 +3364,8 @@ TEST_P(BlockBasedTableTest, Crc32cFileChecksum) {
       TableBuilderOptions(ioptions, moptions, *comparator,
                           &int_tbl_prop_collector_factories,
                           options.compression, options.compression_opts,
-                          false /* skip_filters */, column_family_name, level),
-      TablePropertiesCollectorFactory::Context::kUnknownColumnFamily,
+                          false /* skip_filters */, kUnknownColumnFamily,
+                          column_family_name, level),
       f.GetFileWriter()));
   ASSERT_OK(f.ResetTableBuilder(std::move(builder)));
   f.AddKVtoKVMap(1000);
@@ -3409,11 +3411,10 @@ TEST_F(PlainTableTest, BasicPlainTableProperties) {
   std::string column_family_name;
   int unknown_level = -1;
   std::unique_ptr<TableBuilder> builder(factory.NewTableBuilder(
-      TableBuilderOptions(ioptions, moptions, ikc,
-                          &int_tbl_prop_collector_factories, kNoCompression,
-                          CompressionOptions(), false /* skip_filters */,
-                          column_family_name, unknown_level),
-      TablePropertiesCollectorFactory::Context::kUnknownColumnFamily,
+      TableBuilderOptions(
+          ioptions, moptions, ikc, &int_tbl_prop_collector_factories,
+          kNoCompression, CompressionOptions(), false /* skip_filters */,
+          kUnknownColumnFamily, column_family_name, unknown_level),
       file_writer.get()));
 
   for (char c = 'a'; c <= 'z'; ++c) {
@@ -3466,11 +3467,10 @@ TEST_F(PlainTableTest, NoFileChecksum) {
   f.CreateWriteableFile();
 
   std::unique_ptr<TableBuilder> builder(factory.NewTableBuilder(
-      TableBuilderOptions(ioptions, moptions, ikc,
-                          &int_tbl_prop_collector_factories, kNoCompression,
-                          CompressionOptions(), false /* skip_filters */,
-                          column_family_name, unknown_level),
-      TablePropertiesCollectorFactory::Context::kUnknownColumnFamily,
+      TableBuilderOptions(
+          ioptions, moptions, ikc, &int_tbl_prop_collector_factories,
+          kNoCompression, CompressionOptions(), false /* skip_filters */,
+          kUnknownColumnFamily, column_family_name, unknown_level),
       f.GetFileWriter()));
   ASSERT_OK(f.ResetTableBuilder(std::move(builder)));
   f.AddKVtoKVMap(1000);
@@ -3508,11 +3508,10 @@ TEST_F(PlainTableTest, Crc32cFileChecksum) {
   f.SetFileChecksumGenerator(checksum_crc32c_gen1.release());
 
   std::unique_ptr<TableBuilder> builder(factory.NewTableBuilder(
-      TableBuilderOptions(ioptions, moptions, ikc,
-                          &int_tbl_prop_collector_factories, kNoCompression,
-                          CompressionOptions(), false /* skip_filters */,
-                          column_family_name, unknown_level),
-      TablePropertiesCollectorFactory::Context::kUnknownColumnFamily,
+      TableBuilderOptions(
+          ioptions, moptions, ikc, &int_tbl_prop_collector_factories,
+          kNoCompression, CompressionOptions(), false /* skip_filters */,
+          kUnknownColumnFamily, column_family_name, unknown_level),
       f.GetFileWriter()));
   ASSERT_OK(f.ResetTableBuilder(std::move(builder)));
   f.AddKVtoKVMap(1000);
@@ -4077,8 +4076,7 @@ TEST_P(BlockBasedTableTest, DISABLED_TableWithGlobalSeqno) {
       TableBuilderOptions(ioptions, moptions, ikc,
                           &int_tbl_prop_collector_factories, kNoCompression,
                           CompressionOptions(), false /* skip_filters */,
-                          column_family_name, -1),
-      TablePropertiesCollectorFactory::Context::kUnknownColumnFamily,
+                          kUnknownColumnFamily, column_family_name, -1),
       file_writer.get()));
 
   for (char c = 'a'; c <= 'z'; ++c) {
@@ -4263,8 +4261,7 @@ TEST_P(BlockBasedTableTest, BlockAlignTest) {
       TableBuilderOptions(ioptions, moptions, ikc,
                           &int_tbl_prop_collector_factories, kNoCompression,
                           CompressionOptions(), false /* skip_filters */,
-                          column_family_name, -1),
-      TablePropertiesCollectorFactory::Context::kUnknownColumnFamily,
+                          kUnknownColumnFamily, column_family_name, -1),
       file_writer.get()));
 
   for (int i = 1; i <= 10000; ++i) {
@@ -4358,8 +4355,7 @@ TEST_P(BlockBasedTableTest, PropertiesBlockRestartPointTest) {
       TableBuilderOptions(ioptions, moptions, ikc,
                           &int_tbl_prop_collector_factories, kNoCompression,
                           CompressionOptions(), false /* skip_filters */,
-                          column_family_name, -1),
-      TablePropertiesCollectorFactory::Context::kUnknownColumnFamily,
+                          kUnknownColumnFamily, column_family_name, -1),
       file_writer.get()));
 
   for (int i = 1; i <= 10000; ++i) {

tools/sst_dump_test.cc

@@ -114,8 +114,9 @@ class SSTDumpToolTest : public testing::Test {
         TableBuilderOptions(
             imoptions, moptions, ikc, &int_tbl_prop_collector_factories,
             CompressionType::kNoCompression, CompressionOptions(),
-            false /* skip_filters */, column_family_name, unknown_level),
-        TablePropertiesCollectorFactory::Context::kUnknownColumnFamily,
+            false /* skip_filters */,
+            TablePropertiesCollectorFactory::Context::kUnknownColumnFamily,
+            column_family_name, unknown_level),
         file_writer.get()));
 
     // Populate slightly more than 1K keys

utilities/options/options_util_test.cc

@@ -178,7 +178,6 @@ class DummyTableFactory : public TableFactory {
 
   TableBuilder* NewTableBuilder(
       const TableBuilderOptions& /*table_builder_options*/,
-      uint32_t /*column_family_id*/,
       WritableFileWriter* /*file*/) const override {
     return nullptr;
   }