Read table properties via TableCache if max_open_files=-1

db/compaction/compaction.cc

@@ -257,7 +257,7 @@ Status Compaction::InitInputTableProperties() {
       std::string file_name =
           TableFileName(immutable_options_.cf_paths, fmd->fd.GetNumber(),
                         fmd->fd.GetPathId());
-      s = input_version_->GetTableProperties(read_options, &tp, fmd,
+      s = input_version_->GetTableProperties(read_options, &tp, fmd, level(i),
                                              &file_name);
       if (s.ok()) {
         input_table_properties_[file_name] = tp;

db/compaction/compaction_job.cc

@@ -341,8 +341,8 @@ void CompactionJob::Prepare(
     for (const auto& each_level : *c->inputs()) {
       for (const auto& fmd : each_level.files) {
         std::shared_ptr<const TableProperties> tp;
-        Status s = c->input_version()->GetTableProperties(read_options, &tp,
-                                                          fmd, nullptr);
+        Status s = c->input_version()->GetTableProperties(
+            read_options, &tp, fmd, each_level.level, nullptr);
         if (s.ok()) {
           s = seqno_to_time_mapping_.DecodeFrom(tp->seqno_to_time_mapping);
         }

db/db_impl/db_impl.cc

@@ -6841,7 +6841,9 @@ Status DBImpl::GetPreparedFileInfoForExternalSstIngestion(
   Status s = file_cfd->table_cache()->GetTableProperties(
       file_options_, read_options, file_cfd->internal_comparator(), *file_meta,
       &table_properties, file_version->GetMutableCFOptions(),
-      false /* no_io */);
+      /*no_io=*/false, file_level,
+      file_cfd->internal_stats()->GetFileReadHist(file_level),
+      /*prefetch_index_and_filter_in_cache=*/true);
   if (!s.ok()) {
     return s;
   }

db/db_impl/db_impl_secondary.cc

@@ -1984,7 +1984,8 @@ Status DBImplSecondary::CompactWithoutInstallation(
     const ReadOptions read_options(Env::IOActivity::kCompaction);
     for (const auto& level_files : input_files) {
       for (FileMetaData* file_meta : level_files.files) {
-        version->MaybeInitializeFileMetaData(read_options, file_meta);
+        version->MaybeInitializeFileMetaData(read_options, file_meta,
+                                             level_files.level);
       }
     }
   }

db/db_test.cc

@@ -8270,6 +8270,66 @@ TEST_P(OpenFilesAsyncTest, BeforeRead) {
   }
 }
 
+TEST_P(OpenFilesAsyncTest, TableProperties) {
+  Options options = CurrentOptions();
+  ASSERT_NO_FATAL_FAILURE(SetupData(options));
+
+  for (bool before_async_open : {true, false}) {
+    SCOPED_TRACE("before_async_open=" + std::to_string(before_async_open));
+    if (before_async_open) {
+      ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->LoadDependency({
+          {"OpenFilesAsyncTest::TableProperties::AfterGet",
+           "DBImpl::BGWorkAsyncFileOpen::Start"},
+          {"DBImpl::BGWorkAsyncFileOpen:Done",
+           "OpenFilesAsyncTest::TableProperties::BeforeCheck"},
+      });
+    } else {
+      ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->LoadDependency({
+          {"DBImpl::BGWorkAsyncFileOpen:Done",
+           "OpenFilesAsyncTest::TableProperties::BeforeCheck"},
+      });
+    }
+
+    ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing();
+
+    OpenTestDB(options);
+
+    TablePropertiesCollection props;
+    ASSERT_OK(db_->GetPropertiesOfAllTables(&props));
+    ASSERT_EQ(props.size(), num_flushes_);
+
+    if (before_async_open) {
+      if (max_open_files_ == -1) {
+        // Opened through the table cache, as the async opener would
+        EXPECT_EQ(
+            TestGetTickerCount(options, NUMBER_DIRECT_LOAD_TABLE_PROPERTIES),
+            0);
+        EXPECT_EQ(TestGetTickerCount(options, NO_FILE_OPENS), num_flushes_);
+      } else {
+        // Read directly from the files, without using up table cache capacity
+        EXPECT_EQ(
+            TestGetTickerCount(options, NUMBER_DIRECT_LOAD_TABLE_PROPERTIES),
+            num_flushes_);
+        EXPECT_EQ(TestGetTickerCount(options, NO_FILE_OPENS), 0);
+      }
+    }
+
+    TEST_SYNC_POINT("OpenFilesAsyncTest::TableProperties::AfterGet");
+    TEST_SYNC_POINT("OpenFilesAsyncTest::TableProperties::BeforeCheck");
+
+    // Each file is opened only once in total
+    EXPECT_EQ(TestGetTickerCount(options, NO_FILE_OPENS), num_flushes_);
+    if (max_open_files_ == -1) {
+      EXPECT_EQ(
+          TestGetTickerCount(options, NUMBER_DIRECT_LOAD_TABLE_PROPERTIES), 0);
+    }
+
+    ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->DisableProcessing();
+    ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->ClearAllCallBacks();
+    Close();
+  }
+}
+
 // For modern DBs (manifest carries file_creation_time),
 // GetCreationTimeOfOldestFile returns the real value without waiting for async
 // file open.

db/repair.cc

@@ -552,7 +552,9 @@ class Repairer {
       // TODO: plumb Env::IOActivity, Env::IOPriority
       const ReadOptions read_options;
       status = table_cache_->GetTableProperties(
-          file_options_, read_options, icmp_, t->meta, &props, default_mopts_);
+          file_options_, read_options, icmp_, t->meta, &props, default_mopts_,
+          /*no_io=*/false, /*level=*/-1, /*file_read_hist=*/nullptr,
+          /*prefetch_index_and_filter_in_cache=*/true);
     }
     if (status.ok()) {
       auto s =

db/table_cache.cc

@@ -731,12 +731,15 @@ Status TableCache::GetTableProperties(
     const InternalKeyComparator& internal_comparator,
     const FileMetaData& file_meta,
     std::shared_ptr<const TableProperties>* properties,
-    const MutableCFOptions& mutable_cf_options, bool no_io) {
+    const MutableCFOptions& mutable_cf_options, bool no_io, int level,
+    HistogramImpl* file_read_hist, bool prefetch_index_and_filter_in_cache) {
   TypedHandle* table_handle = nullptr;
   TableReader* table = nullptr;
-  Status s =
-      FindTable(read_options, file_options, internal_comparator, file_meta,
-                &table_handle, mutable_cf_options, &table, no_io);
+  Status s = FindTable(
+      read_options, file_options, internal_comparator, file_meta, &table_handle,
+      mutable_cf_options, &table, no_io, file_read_hist, /*skip_filters=*/false,
+      level, prefetch_index_and_filter_in_cache,
+      MaxFileSizeForL0MetaPin(mutable_cf_options), file_meta.temperature);
   if (!s.ok()) {
     return s;
   }

db/table_cache.h

@@ -249,6 +249,9 @@ class TableCache {
   // Get the table properties of a given table.
   // @no_io: indicates if we should load table to the cache if it is not present
   //         in table cache yet.
+  // @level, @file_read_hist, @prefetch_index_and_filter_in_cache: as for
+  //         FindTable(), applied if the table is opened here. Pass -1 and
+  //         nullptr if the table's level is unknown.
   // @returns: `properties` will be reset on success. Please note that we will
   //            return Status::Incomplete() if table is not present in cache and
   //            we set `no_io` to be true.
@@ -258,7 +261,9 @@ class TableCache {
                             const FileMetaData& file_meta,
                             std::shared_ptr<const TableProperties>* properties,
                             const MutableCFOptions& mutable_cf_options,
-                            bool no_io = false);
+                            bool no_io, int level,
+                            HistogramImpl* file_read_hist,
+                            bool prefetch_index_and_filter_in_cache);
 
   Status ApproximateKeyAnchors(const ReadOptions& ro,
                                const InternalKeyComparator& internal_comparator,

db/version_set.cc

@@ -1970,20 +1970,25 @@ void LevelIterator::InitFileIterator(size_t new_file_index) {
 
 Status Version::GetTableProperties(const ReadOptions& read_options,
                                    std::shared_ptr<const TableProperties>* tp,
-                                   const FileMetaData* file_meta,
+                                   const FileMetaData* file_meta, int level,
                                    const std::string* fname) const {
   auto* table_cache = cfd_->table_cache();
   const auto& ioptions = cfd_->ioptions();
+  // If every table file gets opened anyway, open this one now (coalesced with
+  // any concurrent open) rather than reading its metadata an extra time.
+  const bool open_table = vset_->OpensAllTableFiles();
   Status s = table_cache->GetTableProperties(
       file_options_, read_options, cfd_->internal_comparator(), *file_meta, tp,
-      mutable_cf_options_, true /* no io */);
+      mutable_cf_options_, /*no_io=*/!open_table, level,
+      cfd_->internal_stats()->GetFileReadHist(level),
+      /*prefetch_index_and_filter_in_cache=*/false);
   if (s.ok()) {
     return s;
   }
 
   // We only ignore error type `Incomplete` since it's by design that we
   // disallow table when it's not in table cache.
-  if (!s.IsIncomplete()) {
+  if (open_table || !s.IsIncomplete()) {
     return s;
   }
 
@@ -2107,8 +2112,8 @@ Status Version::GetPropertiesOfAllTables(const ReadOptions& read_options,
     // 1. If the table is already present in table cache, load table
     // properties from there.
     std::shared_ptr<const TableProperties> table_properties;
-    Status s =
-        GetTableProperties(read_options, &table_properties, file_meta, &fname);
+    Status s = GetTableProperties(read_options, &table_properties, file_meta,
+                                  level, &fname);
     if (s.ok()) {
       props->insert({fname, table_properties});
     } else {
@@ -2139,7 +2144,7 @@ Status Version::GetPropertiesOfTablesInRange(
           // properties from there.
           std::shared_ptr<const TableProperties> table_properties;
           Status s = GetTableProperties(read_options, &table_properties,
-                                        file_meta, &fname);
+                                        file_meta, level, &fname);
           if (s.ok()) {
             props->insert({fname, table_properties});
           } else {
@@ -3350,12 +3355,12 @@ void Version::PrepareAppend(const ReadOptions& read_options,
 }
 
 bool Version::MaybeInitializeFileMetaData(const ReadOptions& read_options,
-                                          FileMetaData* file_meta) {
+                                          FileMetaData* file_meta, int level) {
   if (file_meta->init_stats_from_file || file_meta->compensated_file_size > 0) {
     return false;
   }
   std::shared_ptr<const TableProperties> tp;
-  Status s = GetTableProperties(read_options, &tp, file_meta);
+  Status s = GetTableProperties(read_options, &tp, file_meta, level);
   file_meta->init_stats_from_file = true;
   if (!s.ok()) {
     ROCKS_LOG_ERROR(vset_->db_options_->info_log,
@@ -3425,7 +3430,7 @@ void Version::UpdateAccumulatedStats(const ReadOptions& read_options) {
        level < storage_info_.num_levels_ && init_count < kMaxInitCount;
        ++level) {
     for (auto* file_meta : storage_info_.files_[level]) {
-      if (MaybeInitializeFileMetaData(read_options, file_meta)) {
+      if (MaybeInitializeFileMetaData(read_options, file_meta, level)) {
         // each FileMeta will be initialized only once.
         storage_info_.UpdateAccumulatedStats(file_meta);
         // when option "max_open_files" is -1, all the file metadata has
@@ -3451,7 +3456,7 @@ void Version::UpdateAccumulatedStats(const ReadOptions& read_options) {
     for (int i = static_cast<int>(storage_info_.files_[level].size()) - 1;
          storage_info_.accumulated_raw_value_size_ == 0 && i >= 0; --i) {
       if (MaybeInitializeFileMetaData(read_options,
-                                      storage_info_.files_[level][i])) {
+                                      storage_info_.files_[level][i], level)) {
         storage_info_.UpdateAccumulatedStats(storage_info_.files_[level][i]);
       }
     }

db/version_set.h

@@ -1124,12 +1124,17 @@ class Version {
 
   // REQUIRES: lock is held
   // On success, "tp" will contains the table properties of the file
-  // specified in "file_meta".  If the file name of "file_meta" is
-  // known ahead, passing it by a non-null "fname" can save a
-  // file-name conversion.
+  // specified in "file_meta", which is at "level" of this version. If the
+  // file name of "file_meta" is known ahead, passing it by a non-null "fname"
+  // can save a file-name conversion.
+  //
+  // If VersionSet::OpensAllTableFiles(), this opens the file's TableReader
+  // through the table cache if it is not already open. Otherwise, the
+  // properties block is read directly from the file unless its TableReader is
+  // already in the table cache.
   Status GetTableProperties(const ReadOptions& read_options,
                             std::shared_ptr<const TableProperties>* tp,
-                            const FileMetaData* file_meta,
+                            const FileMetaData* file_meta, int level,
                             const std::string* fname = nullptr) const;
 
   // On success, *props will be populated with all SSTables' table properties.
@@ -1228,7 +1233,7 @@ class Version {
   // fields of file_meta from its associated TableProperties.
   // Returns true if it does initialize FileMetaData.
   bool MaybeInitializeFileMetaData(const ReadOptions& read_options,
-                                   FileMetaData* file_meta);
+                                   FileMetaData* file_meta, int level);
 
   // Update the accumulated stats associated with the current version.
   // This accumulated stats will be used in compaction.
@@ -1546,6 +1551,13 @@ class VersionSet {
   virtual void ApplyCommittedOptionsFileNumber(uint64_t number);
   void ApplyLegacyOptionsFileNumber(uint64_t number);
 
+  // True if every live table file is opened and kept open in the table cache
+  // regardless of reads (during recovery and LogAndApply, or by the
+  // open_files_async background opener), as with max_open_files == -1.
+  virtual bool OpensAllTableFiles() const {
+    return table_cache_->GetCapacity() >= TableCache::kInfiniteCapacity;
+  }
+
   uint64_t pending_manifest_file_number() const {
     return pending_manifest_file_number_;
   }
@@ -2192,6 +2204,12 @@ class ReactiveVersionSet : public VersionSet {
   // or blob files. Only used by the DB::OpenAndCompact remote-compaction path.
   void SetTrustManifestRecovery(bool v) { trust_manifest_recovery_ = v; }
 
+  // With trusted MANIFEST recovery, only the table files that are read get
+  // opened.
+  bool OpensAllTableFiles() const override {
+    return !trust_manifest_recovery_ && VersionSet::OpensAllTableFiles();
+  }
+
   // Returns the column family's log number as of the Version currently
   // installed for it, i.e. the log number that the MANIFEST records that
   // Version was built from had put in effect. Data written to WALs older than

unreleased_history/performance_improvements/table_properties_through_table_cache.md

@@ -0,0 +1 @@
+With `max_open_files = -1`, getting the table properties of an SST file that is not yet open (e.g. with `open_files_async`), as in `GetPropertiesOfAllTables()` or compaction, now opens it through the table cache, rather than reading its properties separately and opening it again later.