diff --git a/db/compaction/compaction.cc b/db/compaction/compaction.cc index 02188d68466..729ce510737 100644 --- a/db/compaction/compaction.cc +++ b/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; diff --git a/db/compaction/compaction_job.cc b/db/compaction/compaction_job.cc index f2d867a7bce..330036cee39 100644 --- a/db/compaction/compaction_job.cc +++ b/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 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); } diff --git a/db/db_impl/db_impl.cc b/db/db_impl/db_impl.cc index cd5fa9cff59..68386575412 100644 --- a/db/db_impl/db_impl.cc +++ b/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; } diff --git a/db/db_impl/db_impl_secondary.cc b/db/db_impl/db_impl_secondary.cc index 0c966438f31..a0b3d14e262 100644 --- a/db/db_impl/db_impl_secondary.cc +++ b/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); } } } diff --git a/db/db_test.cc b/db/db_test.cc index e05bae88616..41e7b009fb9 100644 --- a/db/db_test.cc +++ b/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. diff --git a/db/repair.cc b/db/repair.cc index 37c01bc2a7b..2c0e9833db9 100644 --- a/db/repair.cc +++ b/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 = diff --git a/db/table_cache.cc b/db/table_cache.cc index 27da49eb92d..b05f9d0cbdd 100644 --- a/db/table_cache.cc +++ b/db/table_cache.cc @@ -731,12 +731,15 @@ Status TableCache::GetTableProperties( const InternalKeyComparator& internal_comparator, const FileMetaData& file_meta, std::shared_ptr* 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; } diff --git a/db/table_cache.h b/db/table_cache.h index e2dd3427cd5..12f95318c5a 100644 --- a/db/table_cache.h +++ b/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* 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, diff --git a/db/version_set.cc b/db/version_set.cc index 1e7cc68d3ca..9100542e972 100644 --- a/db/version_set.cc +++ b/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* 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 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 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 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(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]); } } diff --git a/db/version_set.h b/db/version_set.h index f91e58e958b..9bd516e4168 100644 --- a/db/version_set.h +++ b/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* 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 diff --git a/unreleased_history/performance_improvements/table_properties_through_table_cache.md b/unreleased_history/performance_improvements/table_properties_through_table_cache.md new file mode 100644 index 00000000000..cc71fcb00b9 --- /dev/null +++ b/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.