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<size_t>`

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
This commit is contained in:
Peter Dillinger 2022-03-29 14:36:35 -07:00 committed by Facebook GitHub Bot
parent 5dbdb197f1
commit 40e3f30a28
5 changed files with 19 additions and 5 deletions

View File

@ -4,6 +4,8 @@
* 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 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 heap use-after-free race with DropColumnFamily.
* Fixed a bug that `rocksdb.read.block.compaction.micros` cannot track compaction stats (#9722). * 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`.
### New Features ### New Features
* For db_bench when --seed=0 or --seed is not set then it uses the current time as the seed value. Previously it used the value 1000. * For db_bench when --seed=0 or --seed is not set then it uses the current time as the seed value. Previously it used the value 1000.
* For db_bench when --benchmark lists multiple tests and each test uses a seed for a RNG then the seeds across tests will no longer be repeated. * For db_bench when --benchmark lists multiple tests and each test uses a seed for a RNG then the seeds across tests will no longer be repeated.

View File

@ -1078,6 +1078,11 @@ void CheckColumnFamilyMeta(
ASSERT_LE(file_meta_from_cf.file_creation_time, end_time); ASSERT_LE(file_meta_from_cf.file_creation_time, end_time);
ASSERT_GE(file_meta_from_cf.oldest_ancester_time, start_time); ASSERT_GE(file_meta_from_cf.oldest_ancester_time, start_time);
ASSERT_LE(file_meta_from_cf.oldest_ancester_time, end_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); ASSERT_EQ(level_meta_from_cf.size, level_size);
@ -1122,6 +1127,11 @@ void CheckLiveFilesMeta(
ASSERT_EQ(meta.oldest_blob_file_number, ASSERT_EQ(meta.oldest_blob_file_number,
expected_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; ++i;
} }
} }

View File

@ -1475,7 +1475,7 @@ void Version::GetColumnFamilyMetaData(ColumnFamilyMetaData* cf_meta) {
const uint64_t file_number = file->fd.GetNumber(); const uint64_t file_number = file->fd.GetNumber();
files.emplace_back( files.emplace_back(
MakeTableFileName("", file_number), file_number, file_path, MakeTableFileName("", file_number), file_number, file_path,
static_cast<size_t>(file->fd.GetFileSize()), file->fd.smallest_seqno, file->fd.GetFileSize(), file->fd.smallest_seqno,
file->fd.largest_seqno, file->smallest.user_key().ToString(), file->fd.largest_seqno, file->smallest.user_key().ToString(),
file->largest.user_key().ToString(), file->largest.user_key().ToString(),
file->stats.num_reads_sampled.load(std::memory_order_relaxed), file->stats.num_reads_sampled.load(std::memory_order_relaxed),
@ -5907,11 +5907,13 @@ void VersionSet::GetLiveFilesMetaData(std::vector<LiveFileMetaData>* metadata) {
assert(!cfd->ioptions()->cf_paths.empty()); assert(!cfd->ioptions()->cf_paths.empty());
filemetadata.db_path = cfd->ioptions()->cf_paths.back().path; filemetadata.db_path = cfd->ioptions()->cf_paths.back().path;
} }
filemetadata.directory = filemetadata.db_path;
const uint64_t file_number = file->fd.GetNumber(); const uint64_t file_number = file->fd.GetNumber();
filemetadata.name = MakeTableFileName("", file_number); filemetadata.name = MakeTableFileName("", file_number);
filemetadata.relative_filename = filemetadata.name.substr(1);
filemetadata.file_number = file_number; filemetadata.file_number = file_number;
filemetadata.level = level; filemetadata.level = level;
filemetadata.size = static_cast<size_t>(file->fd.GetFileSize()); filemetadata.size = file->fd.GetFileSize();
filemetadata.smallestkey = file->smallest.user_key().ToString(); filemetadata.smallestkey = file->smallest.user_key().ToString();
filemetadata.largestkey = file->largest.user_key().ToString(); filemetadata.largestkey = file->largest.user_key().ToString();
filemetadata.smallest_seqno = file->fd.smallest_seqno; filemetadata.smallest_seqno = file->fd.smallest_seqno;

View File

@ -72,10 +72,10 @@ struct LiveFileStorageInfo : public FileStorageInfo {
// The metadata that describes an SST file. (Does not need to extend // The metadata that describes an SST file. (Does not need to extend
// LiveFileStorageInfo because SST files are always immutable.) // LiveFileStorageInfo because SST files are always immutable.)
struct SstFileMetaData : public FileStorageInfo { struct SstFileMetaData : public FileStorageInfo {
SstFileMetaData() {} SstFileMetaData() { file_type = kTableFile; }
SstFileMetaData(const std::string& _file_name, uint64_t _file_number, 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, SequenceNumber _smallest_seqno, SequenceNumber _largest_seqno,
const std::string& _smallestkey, const std::string& _smallestkey,
const std::string& _largestkey, uint64_t _num_reads_sampled, const std::string& _largestkey, uint64_t _num_reads_sampled,

View File

@ -93,7 +93,7 @@ void BlobDBImpl::GetLiveFilesMetaData(std::vector<LiveFileMetaData>* metadata) {
for (auto bfile_pair : blob_files_) { for (auto bfile_pair : blob_files_) {
auto blob_file = bfile_pair.second; auto blob_file = bfile_pair.second;
LiveFileMetaData filemetadata; LiveFileMetaData filemetadata;
filemetadata.size = static_cast<size_t>(blob_file->GetFileSize()); filemetadata.size = blob_file->GetFileSize();
const uint64_t file_number = blob_file->BlobFileNumber(); const uint64_t file_number = blob_file->BlobFileNumber();
// Path should be relative to db_name, but begin with slash. // Path should be relative to db_name, but begin with slash.
filemetadata.name = BlobFileName("", bdb_options_.blob_dir, file_number); filemetadata.name = BlobFileName("", bdb_options_.blob_dir, file_number);