From e6bca1a54e9428134661daf3d2dcbad10a85825d Mon Sep 17 00:00:00 2001 From: Peter Dillinger Date: Tue, 29 Mar 2022 14:36:35 -0700 Subject: [PATCH] Fix FileStorageInfo fields from GetLiveFilesMetaData (#9769) Summary: In making `SstFileMetaData` inherit from `FileStorageInfo`, I overlooked setting some `FileStorageInfo` fields when then default `SstFileMetaData()` ctor is used. This affected `GetLiveFilesMetaData()`. Also removed some buggy `static_cast` Pull Request resolved: https://github.com/facebook/rocksdb/pull/9769 Test Plan: Updated tests Reviewed By: jay-zhuang Differential Revision: D35220383 Pulled By: pdillinger fbshipit-source-id: 05b4ee468258dbd3699517e1124838bf405fe7f8 --- HISTORY.md | 1 + db/db_test.cc | 10 ++++++++++ db/version_set.cc | 6 ++++-- include/rocksdb/metadata.h | 4 ++-- utilities/blob_db/blob_db_impl_filesnapshot.cc | 2 +- 5 files changed, 18 insertions(+), 5 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index a685269a1..c22254d8c 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -4,6 +4,7 @@ * Fixed a race condition when 2PC is disabled and WAL tracking in the MANIFEST is enabled. The race condition is between two background flush threads trying to install flush results, causing a WAL deletion not tracked in the MANIFEST. A future DB open may fail. * Fixed a heap use-after-free race with DropColumnFamily. * Fixed a bug that `rocksdb.read.block.compaction.micros` cannot track compaction stats (#9722). +* Fixed `file_type`, `relative_filename` and `directory` fields returned by `GetLiveFilesMetaData()`, which were added in inheriting from `FileStorageInfo`. ## 7.1.1 (04/07/2022) ### Bug Fixes diff --git a/db/db_test.cc b/db/db_test.cc index a6e378ec5..a8f74d8ca 100644 --- a/db/db_test.cc +++ b/db/db_test.cc @@ -1078,6 +1078,11 @@ void CheckColumnFamilyMeta( ASSERT_LE(file_meta_from_cf.file_creation_time, end_time); ASSERT_GE(file_meta_from_cf.oldest_ancester_time, start_time); ASSERT_LE(file_meta_from_cf.oldest_ancester_time, end_time); + // More from FileStorageInfo + ASSERT_EQ(file_meta_from_cf.file_type, kTableFile); + ASSERT_EQ(file_meta_from_cf.name, + "/" + file_meta_from_cf.relative_filename); + ASSERT_EQ(file_meta_from_cf.directory, file_meta_from_cf.db_path); } ASSERT_EQ(level_meta_from_cf.size, level_size); @@ -1122,6 +1127,11 @@ void CheckLiveFilesMeta( ASSERT_EQ(meta.oldest_blob_file_number, expected_meta.oldest_blob_file_number); + // More from FileStorageInfo + ASSERT_EQ(meta.file_type, kTableFile); + ASSERT_EQ(meta.name, "/" + meta.relative_filename); + ASSERT_EQ(meta.directory, meta.db_path); + ++i; } } diff --git a/db/version_set.cc b/db/version_set.cc index 8c6c9fce1..a820edeb3 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -1475,7 +1475,7 @@ void Version::GetColumnFamilyMetaData(ColumnFamilyMetaData* cf_meta) { const uint64_t file_number = file->fd.GetNumber(); files.emplace_back( MakeTableFileName("", file_number), file_number, file_path, - static_cast(file->fd.GetFileSize()), file->fd.smallest_seqno, + file->fd.GetFileSize(), file->fd.smallest_seqno, file->fd.largest_seqno, file->smallest.user_key().ToString(), file->largest.user_key().ToString(), file->stats.num_reads_sampled.load(std::memory_order_relaxed), @@ -5893,11 +5893,13 @@ void VersionSet::GetLiveFilesMetaData(std::vector* metadata) { assert(!cfd->ioptions()->cf_paths.empty()); filemetadata.db_path = cfd->ioptions()->cf_paths.back().path; } + filemetadata.directory = filemetadata.db_path; const uint64_t file_number = file->fd.GetNumber(); filemetadata.name = MakeTableFileName("", file_number); + filemetadata.relative_filename = filemetadata.name.substr(1); filemetadata.file_number = file_number; filemetadata.level = level; - filemetadata.size = static_cast(file->fd.GetFileSize()); + filemetadata.size = file->fd.GetFileSize(); filemetadata.smallestkey = file->smallest.user_key().ToString(); filemetadata.largestkey = file->largest.user_key().ToString(); filemetadata.smallest_seqno = file->fd.smallest_seqno; diff --git a/include/rocksdb/metadata.h b/include/rocksdb/metadata.h index c0923d896..6040e56aa 100644 --- a/include/rocksdb/metadata.h +++ b/include/rocksdb/metadata.h @@ -72,10 +72,10 @@ struct LiveFileStorageInfo : public FileStorageInfo { // The metadata that describes an SST file. (Does not need to extend // LiveFileStorageInfo because SST files are always immutable.) struct SstFileMetaData : public FileStorageInfo { - SstFileMetaData() {} + SstFileMetaData() { file_type = kTableFile; } SstFileMetaData(const std::string& _file_name, uint64_t _file_number, - const std::string& _directory, size_t _size, + const std::string& _directory, uint64_t _size, SequenceNumber _smallest_seqno, SequenceNumber _largest_seqno, const std::string& _smallestkey, const std::string& _largestkey, uint64_t _num_reads_sampled, diff --git a/utilities/blob_db/blob_db_impl_filesnapshot.cc b/utilities/blob_db/blob_db_impl_filesnapshot.cc index 7ec4443d4..87e3f33cc 100644 --- a/utilities/blob_db/blob_db_impl_filesnapshot.cc +++ b/utilities/blob_db/blob_db_impl_filesnapshot.cc @@ -93,7 +93,7 @@ void BlobDBImpl::GetLiveFilesMetaData(std::vector* metadata) { for (auto bfile_pair : blob_files_) { auto blob_file = bfile_pair.second; LiveFileMetaData filemetadata; - filemetadata.size = static_cast(blob_file->GetFileSize()); + filemetadata.size = blob_file->GetFileSize(); const uint64_t file_number = blob_file->BlobFileNumber(); // Path should be relative to db_name, but begin with slash. filemetadata.name = BlobFileName("", bdb_options_.blob_dir, file_number);