From 42e0751b3a364d9bd26bae86c56d2477ef341e42 Mon Sep 17 00:00:00 2001 From: Levi Tamasi Date: Fri, 4 Feb 2022 08:18:18 -0800 Subject: [PATCH] Clean up VersionStorageInfo a bit (#9494) Summary: The patch does some cleanup in and around `VersionStorageInfo`: * Renames the method `PrepareApply` to `PrepareAppend` in `Version` to make it clear that it is to be called before appending the `Version` to `VersionSet` (via `AppendVersion`), not before applying any `VersionEdit`s. * Introduces a helper method `VersionStorageInfo::PrepareForVersionAppend` (called by `Version::PrepareAppend`) that encapsulates the population of the various derived data structures in `VersionStorageInfo`, and turns the methods computing the derived structures (`UpdateNumNonEmptyLevels`, `CalculateBaseBytes` etc.) into private helpers. * Changes `Version::PrepareAppend` so it only calls `UpdateAccumulatedStats` if the `update_stats` flag is set. (Earlier, this was checked by the callee.) Related to this, it also moves the call to `ComputeCompensatedSizes` to `VersionStorageInfo::PrepareForVersionAppend`. * Updates and cleans up `version_builder_test`, `version_set_test`, and `compaction_picker_test` so `PrepareForVersionAppend` is called anytime a new `VersionStorageInfo` is set up or saved. This cleanup also involves splitting `VersionStorageInfoTest.MaxBytesForLevelDynamic` into multiple smaller test cases. * Fixes up a bunch of comments that were outdated or just plain incorrect. Pull Request resolved: https://github.com/facebook/rocksdb/pull/9494 Test Plan: Ran `make check` and the crash test script for a while. Reviewed By: riversand963 Differential Revision: D33971666 Pulled By: ltamasi fbshipit-source-id: fda52faac7783041126e4f8dec0fe01bdcadf65a --- db/compaction/compaction_picker_test.cc | 29 +---- db/version_builder_test.cc | 90 +++++++++++++-- db/version_edit.h | 1 - db/version_edit_handler.cc | 4 +- db/version_set.cc | 145 ++++++++++++------------ db/version_set.h | 81 ++++++------- db/version_set_test.cc | 115 ++++++++++++++----- tools/ldb_cmd_test.cc | 4 +- 8 files changed, 278 insertions(+), 191 deletions(-) diff --git a/db/compaction/compaction_picker_test.cc b/db/compaction/compaction_picker_test.cc index 1c928ad4a..1bbf7bc27 100644 --- a/db/compaction/compaction_picker_test.cc +++ b/db/compaction/compaction_picker_test.cc @@ -76,7 +76,7 @@ class CompactionPickerTest : public testing::Test { options_.num_levels = num_levels; vstorage_.reset(new VersionStorageInfo(&icmp_, ucmp_, options_.num_levels, style, nullptr, false)); - vstorage_->CalculateBaseBytes(ioptions_, mutable_cf_options_); + vstorage_->PrepareForVersionAppend(ioptions_, mutable_cf_options_); } // Create a new VersionStorageInfo object so we can add mode files and then @@ -148,35 +148,10 @@ class CompactionPickerTest : public testing::Test { ASSERT_OK(builder.SaveTo(temp_vstorage_.get())); vstorage_ = std::move(temp_vstorage_); } - vstorage_->CalculateBaseBytes(ioptions_, mutable_cf_options_); - vstorage_->UpdateFilesByCompactionPri(ioptions_, mutable_cf_options_); - vstorage_->UpdateNumNonEmptyLevels(); - vstorage_->GenerateFileIndexer(); - vstorage_->GenerateLevelFilesBrief(); + vstorage_->PrepareForVersionAppend(ioptions_, mutable_cf_options_); vstorage_->ComputeCompactionScore(ioptions_, mutable_cf_options_); - vstorage_->GenerateLevel0NonOverlapping(); - vstorage_->ComputeFilesMarkedForCompaction(); vstorage_->SetFinalized(); } - void AddFileToVersionStorage(int level, uint32_t file_number, - const char* smallest, const char* largest, - uint64_t file_size = 1, uint32_t path_id = 0, - SequenceNumber smallest_seq = 100, - SequenceNumber largest_seq = 100, - size_t compensated_file_size = 0, - bool marked_for_compact = false) { - VersionStorageInfo* base_vstorage = vstorage_.release(); - vstorage_.reset(new VersionStorageInfo(&icmp_, ucmp_, options_.num_levels, - kCompactionStyleUniversal, - base_vstorage, false)); - Add(level, file_number, smallest, largest, file_size, path_id, smallest_seq, - largest_seq, compensated_file_size, marked_for_compact); - - VersionBuilder builder(FileOptions(), &ioptions_, nullptr, base_vstorage, - nullptr); - builder.SaveTo(vstorage_.get()); - UpdateVersionStorageInfo(); - } private: std::unique_ptr temp_vstorage_; diff --git a/db/version_builder_test.cc b/db/version_builder_test.cc index 0af81d986..62ed05326 100644 --- a/db/version_builder_test.cc +++ b/db/version_builder_test.cc @@ -152,15 +152,14 @@ class VersionBuilderTest : public testing::Test { return meta; } - void UpdateVersionStorageInfo() { - vstorage_.UpdateFilesByCompactionPri(ioptions_, mutable_cf_options_); - vstorage_.UpdateNumNonEmptyLevels(); - vstorage_.GenerateFileIndexer(); - vstorage_.GenerateLevelFilesBrief(); - vstorage_.CalculateBaseBytes(ioptions_, mutable_cf_options_); - vstorage_.GenerateLevel0NonOverlapping(); - vstorage_.SetFinalized(); + void UpdateVersionStorageInfo(VersionStorageInfo* vstorage) { + assert(vstorage); + + vstorage->PrepareForVersionAppend(ioptions_, mutable_cf_options_); + vstorage->SetFinalized(); } + + void UpdateVersionStorageInfo() { UpdateVersionStorageInfo(&vstorage_); } }; void UnrefFilesInVersion(VersionStorageInfo* new_vstorage) { @@ -187,6 +186,7 @@ TEST_F(VersionBuilderTest, ApplyAndSaveTo) { Add(3, 27U, "171", "179", 100U); Add(3, 28U, "191", "220", 100U); Add(3, 29U, "221", "300", 100U); + UpdateVersionStorageInfo(); VersionEdit version_edit; @@ -210,6 +210,8 @@ TEST_F(VersionBuilderTest, ApplyAndSaveTo) { ASSERT_OK(version_builder.Apply(&version_edit)); ASSERT_OK(version_builder.SaveTo(&new_vstorage)); + UpdateVersionStorageInfo(&new_vstorage); + ASSERT_EQ(400U, new_vstorage.NumLevelBytes(2)); ASSERT_EQ(300U, new_vstorage.NumLevelBytes(3)); @@ -228,6 +230,7 @@ TEST_F(VersionBuilderTest, ApplyAndSaveToDynamic) { Add(5, 26U, "150", "170", 100U); Add(5, 27U, "171", "179", 100U); + UpdateVersionStorageInfo(); VersionEdit version_edit; @@ -252,6 +255,8 @@ TEST_F(VersionBuilderTest, ApplyAndSaveToDynamic) { ASSERT_OK(version_builder.Apply(&version_edit)); ASSERT_OK(version_builder.SaveTo(&new_vstorage)); + UpdateVersionStorageInfo(&new_vstorage); + ASSERT_EQ(0U, new_vstorage.NumLevelBytes(0)); ASSERT_EQ(100U, new_vstorage.NumLevelBytes(3)); ASSERT_EQ(300U, new_vstorage.NumLevelBytes(4)); @@ -272,6 +277,7 @@ TEST_F(VersionBuilderTest, ApplyAndSaveToDynamic2) { Add(5, 26U, "150", "170", 100U); Add(5, 27U, "171", "179", 100U); + UpdateVersionStorageInfo(); VersionEdit version_edit; @@ -299,6 +305,8 @@ TEST_F(VersionBuilderTest, ApplyAndSaveToDynamic2) { ASSERT_OK(version_builder.Apply(&version_edit)); ASSERT_OK(version_builder.SaveTo(&new_vstorage)); + UpdateVersionStorageInfo(&new_vstorage); + ASSERT_EQ(0U, new_vstorage.NumLevelBytes(0)); ASSERT_EQ(100U, new_vstorage.NumLevelBytes(4)); ASSERT_EQ(200U, new_vstorage.NumLevelBytes(5)); @@ -353,6 +361,8 @@ TEST_F(VersionBuilderTest, ApplyMultipleAndSaveTo) { ASSERT_OK(version_builder.Apply(&version_edit)); ASSERT_OK(version_builder.SaveTo(&new_vstorage)); + UpdateVersionStorageInfo(&new_vstorage); + ASSERT_EQ(500U, new_vstorage.NumLevelBytes(2)); UnrefFilesInVersion(&new_vstorage); @@ -423,6 +433,8 @@ TEST_F(VersionBuilderTest, ApplyDeleteAndSaveTo) { ASSERT_OK(version_builder.Apply(&version_edit2)); ASSERT_OK(version_builder.SaveTo(&new_vstorage)); + UpdateVersionStorageInfo(&new_vstorage); + ASSERT_EQ(300U, new_vstorage.NumLevelBytes(2)); UnrefFilesInVersion(&new_vstorage); @@ -433,8 +445,11 @@ TEST_F(VersionBuilderTest, ApplyFileDeletionIncorrectLevel) { constexpr uint64_t file_number = 2345; constexpr char smallest[] = "bar"; constexpr char largest[] = "foo"; + constexpr uint64_t file_size = 100; - Add(level, file_number, smallest, largest); + Add(level, file_number, smallest, largest, file_size); + + UpdateVersionStorageInfo(); EnvOptions env_options; constexpr TableCache* table_cache = nullptr; @@ -457,6 +472,8 @@ TEST_F(VersionBuilderTest, ApplyFileDeletionIncorrectLevel) { } TEST_F(VersionBuilderTest, ApplyFileDeletionNotInLSMTree) { + UpdateVersionStorageInfo(); + EnvOptions env_options; constexpr TableCache* table_cache = nullptr; constexpr VersionSet* version_set = nullptr; @@ -497,6 +514,8 @@ TEST_F(VersionBuilderTest, ApplyFileDeletionAndAddition) { largest_seq, num_entries, num_deletions, sampled, smallest_seqno, largest_seqno); + UpdateVersionStorageInfo(); + EnvOptions env_options; constexpr TableCache* table_cache = nullptr; constexpr VersionSet* version_set = nullptr; @@ -531,6 +550,9 @@ TEST_F(VersionBuilderTest, ApplyFileDeletionAndAddition) { force_consistency_checks); ASSERT_OK(builder.SaveTo(&new_vstorage)); + + UpdateVersionStorageInfo(&new_vstorage); + ASSERT_EQ(new_vstorage.GetFileLocation(file_number).GetLevel(), level); UnrefFilesInVersion(&new_vstorage); @@ -541,8 +563,11 @@ TEST_F(VersionBuilderTest, ApplyFileAdditionAlreadyInBase) { constexpr uint64_t file_number = 2345; constexpr char smallest[] = "bar"; constexpr char largest[] = "foo"; + constexpr uint64_t file_size = 10000; + + Add(level, file_number, smallest, largest, file_size); - Add(level, file_number, smallest, largest); + UpdateVersionStorageInfo(); EnvOptions env_options; constexpr TableCache* table_cache = nullptr; @@ -555,7 +580,6 @@ TEST_F(VersionBuilderTest, ApplyFileAdditionAlreadyInBase) { constexpr int new_level = 2; constexpr uint32_t path_id = 0; - constexpr uint64_t file_size = 10000; constexpr SequenceNumber smallest_seqno = 100; constexpr SequenceNumber largest_seqno = 1000; constexpr bool marked_for_compaction = false; @@ -576,6 +600,8 @@ TEST_F(VersionBuilderTest, ApplyFileAdditionAlreadyInBase) { } TEST_F(VersionBuilderTest, ApplyFileAdditionAlreadyApplied) { + UpdateVersionStorageInfo(); + EnvOptions env_options; constexpr TableCache* table_cache = nullptr; constexpr VersionSet* version_set = nullptr; @@ -625,6 +651,8 @@ TEST_F(VersionBuilderTest, ApplyFileAdditionAlreadyApplied) { } TEST_F(VersionBuilderTest, ApplyFileAdditionAndDeletion) { + UpdateVersionStorageInfo(); + constexpr int level = 1; constexpr uint64_t file_number = 2345; constexpr uint32_t path_id = 0; @@ -666,12 +694,17 @@ TEST_F(VersionBuilderTest, ApplyFileAdditionAndDeletion) { force_consistency_checks); ASSERT_OK(builder.SaveTo(&new_vstorage)); + + UpdateVersionStorageInfo(&new_vstorage); + ASSERT_FALSE(new_vstorage.GetFileLocation(file_number).IsValid()); UnrefFilesInVersion(&new_vstorage); } TEST_F(VersionBuilderTest, ApplyBlobFileAddition) { + UpdateVersionStorageInfo(); + EnvOptions env_options; constexpr TableCache* table_cache = nullptr; constexpr VersionSet* version_set = nullptr; @@ -705,6 +738,8 @@ TEST_F(VersionBuilderTest, ApplyBlobFileAddition) { ASSERT_OK(builder.SaveTo(&new_vstorage)); + UpdateVersionStorageInfo(&new_vstorage); + const auto& new_blob_files = new_vstorage.GetBlobFiles(); ASSERT_EQ(new_blob_files.size(), 1); @@ -741,6 +776,8 @@ TEST_F(VersionBuilderTest, ApplyBlobFileAdditionAlreadyInBase) { checksum_value, BlobFileMetaData::LinkedSsts(), garbage_blob_count, garbage_blob_bytes); + UpdateVersionStorageInfo(); + EnvOptions env_options; constexpr TableCache* table_cache = nullptr; constexpr VersionSet* version_set = nullptr; @@ -761,6 +798,8 @@ TEST_F(VersionBuilderTest, ApplyBlobFileAdditionAlreadyInBase) { TEST_F(VersionBuilderTest, ApplyBlobFileAdditionAlreadyApplied) { // Attempt to add the same blob file twice using version edits. + UpdateVersionStorageInfo(); + EnvOptions env_options; constexpr TableCache* table_cache = nullptr; constexpr VersionSet* version_set = nullptr; @@ -813,6 +852,8 @@ TEST_F(VersionBuilderTest, ApplyBlobFileGarbageFileInBase) { // Add dummy table file to ensure the blob file is referenced. AddDummyFile(table_file_number, blob_file_number); + UpdateVersionStorageInfo(); + EnvOptions env_options; constexpr TableCache* table_cache = nullptr; constexpr VersionSet* version_set = nullptr; @@ -837,6 +878,8 @@ TEST_F(VersionBuilderTest, ApplyBlobFileGarbageFileInBase) { ASSERT_OK(builder.SaveTo(&new_vstorage)); + UpdateVersionStorageInfo(&new_vstorage); + const auto& new_blob_files = new_vstorage.GetBlobFiles(); ASSERT_EQ(new_blob_files.size(), 1); @@ -862,6 +905,8 @@ TEST_F(VersionBuilderTest, ApplyBlobFileGarbageFileInBase) { TEST_F(VersionBuilderTest, ApplyBlobFileGarbageFileAdditionApplied) { // Increase the amount of garbage for a blob file added using a version edit. + UpdateVersionStorageInfo(); + EnvOptions env_options; constexpr TableCache* table_cache = nullptr; constexpr VersionSet* version_set = nullptr; @@ -905,6 +950,8 @@ TEST_F(VersionBuilderTest, ApplyBlobFileGarbageFileAdditionApplied) { ASSERT_OK(builder.SaveTo(&new_vstorage)); + UpdateVersionStorageInfo(&new_vstorage); + const auto& new_blob_files = new_vstorage.GetBlobFiles(); ASSERT_EQ(new_blob_files.size(), 1); @@ -928,6 +975,8 @@ TEST_F(VersionBuilderTest, ApplyBlobFileGarbageFileNotFound) { // Attempt to increase the amount of garbage for a blob file that is // neither in the base version, nor was it added using a version edit. + UpdateVersionStorageInfo(); + EnvOptions env_options; constexpr TableCache* table_cache = nullptr; constexpr VersionSet* version_set = nullptr; @@ -953,6 +1002,8 @@ TEST_F(VersionBuilderTest, BlobFileGarbageOverflow) { // Test that VersionEdits that would result in the count/total size of garbage // exceeding the count/total size of all blobs are rejected. + UpdateVersionStorageInfo(); + EnvOptions env_options; constexpr TableCache* table_cache = nullptr; constexpr VersionSet* version_set = nullptr; @@ -1032,6 +1083,8 @@ TEST_F(VersionBuilderTest, SaveBlobFilesTo) { AddDummyFile(table_file_number, blob_file_number); } + UpdateVersionStorageInfo(); + EnvOptions env_options; constexpr TableCache* table_cache = nullptr; constexpr VersionSet* version_set = nullptr; @@ -1069,6 +1122,8 @@ TEST_F(VersionBuilderTest, SaveBlobFilesTo) { ASSERT_OK(builder.SaveTo(&new_vstorage)); + UpdateVersionStorageInfo(&new_vstorage); + const auto& new_blob_files = new_vstorage.GetBlobFiles(); ASSERT_EQ(new_blob_files.size(), 3); @@ -1115,6 +1170,8 @@ TEST_F(VersionBuilderTest, SaveBlobFilesTo) { ASSERT_OK(second_builder.SaveTo(&newer_vstorage)); + UpdateVersionStorageInfo(&newer_vstorage); + const auto& newer_blob_files = newer_vstorage.GetBlobFiles(); ASSERT_EQ(newer_blob_files.size(), 2); @@ -1149,6 +1206,8 @@ TEST_F(VersionBuilderTest, SaveBlobFilesToConcurrentJobs) { BlobFileMetaData::LinkedSsts{base_table_file_number}, garbage_blob_count, garbage_blob_bytes); + UpdateVersionStorageInfo(); + EnvOptions env_options; constexpr TableCache* table_cache = nullptr; constexpr VersionSet* version_set = nullptr; @@ -1194,6 +1253,8 @@ TEST_F(VersionBuilderTest, SaveBlobFilesToConcurrentJobs) { ASSERT_OK(builder.SaveTo(&new_vstorage)); + UpdateVersionStorageInfo(&new_vstorage); + const auto& new_blob_files = new_vstorage.GetBlobFiles(); ASSERT_EQ(new_blob_files.size(), 2); @@ -1293,6 +1354,8 @@ TEST_F(VersionBuilderTest, CheckConsistencyForBlobFiles) { ASSERT_OK(builder.SaveTo(&new_vstorage)); + UpdateVersionStorageInfo(&new_vstorage); + UnrefFilesInVersion(&new_vstorage); } @@ -1581,6 +1644,8 @@ TEST_F(VersionBuilderTest, MaintainLinkedSstsForBlobFiles) { ASSERT_OK(builder.SaveTo(&new_vstorage)); + UpdateVersionStorageInfo(&new_vstorage); + { const auto& blob_files = new_vstorage.GetBlobFiles(); ASSERT_EQ(blob_files.size(), 5); @@ -1605,6 +1670,7 @@ TEST_F(VersionBuilderTest, MaintainLinkedSstsForBlobFiles) { TEST_F(VersionBuilderTest, CheckConsistencyForFileDeletedTwice) { Add(0, 1U, "150", "200", 100U); + UpdateVersionStorageInfo(); VersionEdit version_edit; @@ -1622,6 +1688,8 @@ TEST_F(VersionBuilderTest, CheckConsistencyForFileDeletedTwice) { ASSERT_OK(version_builder.Apply(&version_edit)); ASSERT_OK(version_builder.SaveTo(&new_vstorage)); + UpdateVersionStorageInfo(&new_vstorage); + VersionBuilder version_builder2(env_options, &ioptions_, table_cache, &new_vstorage, version_set); VersionStorageInfo new_vstorage2(&icmp_, ucmp_, options_.num_levels, diff --git a/db/version_edit.h b/db/version_edit.h index 4ba1639ac..74a71cd85 100644 --- a/db/version_edit.h +++ b/db/version_edit.h @@ -396,7 +396,6 @@ class VersionEdit { const DeletedFiles& GetDeletedFiles() const { return deleted_files_; } // Add the specified table file at the specified level. - // REQUIRES: This version has not been saved (see VersionSet::SaveTo) // REQUIRES: "smallest" and "largest" are smallest and largest keys in file // REQUIRES: "oldest_blob_file_number" is the number of the oldest blob file // referred to by this file if any, kInvalidBlobFileNumber otherwise. diff --git a/db/version_edit_handler.cc b/db/version_edit_handler.cc index 4d44cb917..208527fb8 100644 --- a/db/version_edit_handler.cc +++ b/db/version_edit_handler.cc @@ -532,7 +532,7 @@ Status VersionEditHandler::MaybeCreateVersion(const VersionEdit& /*edit*/, s = builder->SaveTo(v->storage_info()); if (s.ok()) { // Install new version - v->PrepareApply( + v->PrepareAppend( *cfd->GetLatestMutableCFOptions(), !(version_set_->db_options_->skip_stats_update_on_db_open)); version_set_->AppendVersion(cfd, v); @@ -806,7 +806,7 @@ Status VersionEditHandlerPointInTime::MaybeCreateVersion( version_set_->current_version_number_++); s = builder->SaveTo(version->storage_info()); if (s.ok()) { - version->PrepareApply( + version->PrepareAppend( *cfd->GetLatestMutableCFOptions(), !version_set_->db_options_->skip_stats_update_on_db_open); auto v_iter = versions_.find(cfd->GetID()); diff --git a/db/version_set.cc b/db/version_set.cc index 342c7462f..532e37361 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -2402,21 +2402,30 @@ void VersionStorageInfo::GenerateLevelFilesBrief() { } } -void Version::PrepareApply( - const MutableCFOptions& mutable_cf_options, - bool update_stats) { +void VersionStorageInfo::PrepareForVersionAppend( + const ImmutableOptions& immutable_options, + const MutableCFOptions& mutable_cf_options) { + ComputeCompensatedSizes(); + UpdateNumNonEmptyLevels(); + CalculateBaseBytes(immutable_options, mutable_cf_options); + UpdateFilesByCompactionPri(immutable_options, mutable_cf_options); + GenerateFileIndexer(); + GenerateLevelFilesBrief(); + GenerateLevel0NonOverlapping(); + GenerateBottommostFiles(); +} + +void Version::PrepareAppend(const MutableCFOptions& mutable_cf_options, + bool update_stats) { TEST_SYNC_POINT_CALLBACK( - "Version::PrepareApply:forced_check", + "Version::PrepareAppend:forced_check", reinterpret_cast(&storage_info_.force_consistency_checks_)); - UpdateAccumulatedStats(update_stats); - storage_info_.UpdateNumNonEmptyLevels(); - storage_info_.CalculateBaseBytes(*cfd_->ioptions(), mutable_cf_options); - storage_info_.UpdateFilesByCompactionPri(*cfd_->ioptions(), - mutable_cf_options); - storage_info_.GenerateFileIndexer(); - storage_info_.GenerateLevelFilesBrief(); - storage_info_.GenerateLevel0NonOverlapping(); - storage_info_.GenerateBottommostFiles(); + + if (update_stats) { + UpdateAccumulatedStats(); + } + + storage_info_.PrepareForVersionAppend(*cfd_->ioptions(), mutable_cf_options); } bool Version::MaybeInitializeFileMetaData(FileMetaData* file_meta) { @@ -2470,59 +2479,54 @@ void VersionStorageInfo::RemoveCurrentStats(FileMetaData* file_meta) { } } -void Version::UpdateAccumulatedStats(bool update_stats) { - if (update_stats) { - // maximum number of table properties loaded from files. - const int kMaxInitCount = 20; - int init_count = 0; - // here only the first kMaxInitCount files which haven't been - // initialized from file will be updated with num_deletions. - // The motivation here is to cap the maximum I/O per Version creation. - // The reason for choosing files from lower-level instead of higher-level - // is that such design is able to propagate the initialization from - // lower-level to higher-level: When the num_deletions of lower-level - // files are updated, it will make the lower-level files have accurate - // compensated_file_size, making lower-level to higher-level compaction - // will be triggered, which creates higher-level files whose num_deletions - // will be updated here. - for (int level = 0; - level < storage_info_.num_levels_ && init_count < kMaxInitCount; - ++level) { - for (auto* file_meta : storage_info_.files_[level]) { - if (MaybeInitializeFileMetaData(file_meta)) { - // each FileMeta will be initialized only once. - storage_info_.UpdateAccumulatedStats(file_meta); - // when option "max_open_files" is -1, all the file metadata has - // already been read, so MaybeInitializeFileMetaData() won't incur - // any I/O cost. "max_open_files=-1" means that the table cache passed - // to the VersionSet and then to the ColumnFamilySet has a size of - // TableCache::kInfiniteCapacity - if (vset_->GetColumnFamilySet()->get_table_cache()->GetCapacity() == - TableCache::kInfiniteCapacity) { - continue; - } - if (++init_count >= kMaxInitCount) { - break; - } +void Version::UpdateAccumulatedStats() { + // maximum number of table properties loaded from files. + const int kMaxInitCount = 20; + int init_count = 0; + // here only the first kMaxInitCount files which haven't been + // initialized from file will be updated with num_deletions. + // The motivation here is to cap the maximum I/O per Version creation. + // The reason for choosing files from lower-level instead of higher-level + // is that such design is able to propagate the initialization from + // lower-level to higher-level: When the num_deletions of lower-level + // files are updated, it will make the lower-level files have accurate + // compensated_file_size, making lower-level to higher-level compaction + // will be triggered, which creates higher-level files whose num_deletions + // will be updated here. + for (int level = 0; + level < storage_info_.num_levels_ && init_count < kMaxInitCount; + ++level) { + for (auto* file_meta : storage_info_.files_[level]) { + if (MaybeInitializeFileMetaData(file_meta)) { + // each FileMeta will be initialized only once. + storage_info_.UpdateAccumulatedStats(file_meta); + // when option "max_open_files" is -1, all the file metadata has + // already been read, so MaybeInitializeFileMetaData() won't incur + // any I/O cost. "max_open_files=-1" means that the table cache passed + // to the VersionSet and then to the ColumnFamilySet has a size of + // TableCache::kInfiniteCapacity + if (vset_->GetColumnFamilySet()->get_table_cache()->GetCapacity() == + TableCache::kInfiniteCapacity) { + continue; + } + if (++init_count >= kMaxInitCount) { + break; } } } - // In case all sampled-files contain only deletion entries, then we - // load the table-property of a file in higher-level to initialize - // that value. - for (int level = storage_info_.num_levels_ - 1; - storage_info_.accumulated_raw_value_size_ == 0 && level >= 0; - --level) { - for (int i = static_cast(storage_info_.files_[level].size()) - 1; - storage_info_.accumulated_raw_value_size_ == 0 && i >= 0; --i) { - if (MaybeInitializeFileMetaData(storage_info_.files_[level][i])) { - storage_info_.UpdateAccumulatedStats(storage_info_.files_[level][i]); - } + } + // In case all sampled-files contain only deletion entries, then we + // load the table-property of a file in higher-level to initialize + // that value. + for (int level = storage_info_.num_levels_ - 1; + storage_info_.accumulated_raw_value_size_ == 0 && level >= 0; --level) { + for (int i = static_cast(storage_info_.files_[level].size()) - 1; + storage_info_.accumulated_raw_value_size_ == 0 && i >= 0; --i) { + if (MaybeInitializeFileMetaData(storage_info_.files_[level][i])) { + storage_info_.UpdateAccumulatedStats(storage_info_.files_[level][i]); } } } - - storage_info_.ComputeCompensatedSizes(); } void VersionStorageInfo::ComputeCompensatedSizes() { @@ -3062,17 +3066,9 @@ void VersionStorageInfo::AddBlobFile( blob_files_.emplace_hint(it, blob_file_number, std::move(blob_file_meta)); } -// Version::PrepareApply() need to be called before calling the function, or -// following functions called: -// 1. UpdateNumNonEmptyLevels(); -// 2. CalculateBaseBytes(); -// 3. UpdateFilesByCompactionPri(); -// 4. GenerateFileIndexer(); -// 5. GenerateLevelFilesBrief(); -// 6. GenerateLevel0NonOverlapping(); -// 7. GenerateBottommostFiles(); void VersionStorageInfo::SetFinalized() { finalized_ = true; + #ifndef NDEBUG if (compaction_style_ != kCompactionStyleLevel) { // Not level based compaction. @@ -4418,8 +4414,10 @@ Status VersionSet::ProcessManifestWrites( if (s.ok()) { if (!first_writer.edit_list.front()->IsColumnFamilyManipulation()) { + constexpr bool update_stats = true; + for (int i = 0; i < static_cast(versions.size()); ++i) { - versions[i]->PrepareApply(*mutable_cf_options_ptrs[i], true); + versions[i]->PrepareAppend(*mutable_cf_options_ptrs[i], update_stats); } } @@ -5914,9 +5912,10 @@ ColumnFamilyData* VersionSet::CreateColumnFamily( *new_cfd->GetLatestMutableCFOptions(), io_tracer_, current_version_number_++); - // Fill level target base information. - v->storage_info()->CalculateBaseBytes(*new_cfd->ioptions(), - *new_cfd->GetLatestMutableCFOptions()); + constexpr bool update_stats = false; + + v->PrepareAppend(*new_cfd->GetLatestMutableCFOptions(), update_stats); + AppendVersion(new_cfd, v); // GetLatestMutableCFOptions() is safe here without mutex since the // cfd is not available to client diff --git a/db/version_set.h b/db/version_set.h index ee76713af..53f97012d 100644 --- a/db/version_set.h +++ b/db/version_set.h @@ -129,14 +129,11 @@ class VersionStorageInfo { void AddBlobFile(std::shared_ptr blob_file_meta); - void SetFinalized(); - - // Update num_non_empty_levels_. - void UpdateNumNonEmptyLevels(); + void PrepareForVersionAppend(const ImmutableOptions& immutable_options, + const MutableCFOptions& mutable_cf_options); - void GenerateFileIndexer() { - file_indexer_.UpdateIndex(&arena_, num_non_empty_levels_, files_); - } + // REQUIRES: PrepareForVersionAppend has been called + void SetFinalized(); // Update the accumulated stats from a file-meta. void UpdateAccumulatedStats(FileMetaData* file_meta); @@ -144,8 +141,6 @@ class VersionStorageInfo { // Decrease the current stat from a to-be-deleted file-meta void RemoveCurrentStats(FileMetaData* file_meta); - void ComputeCompensatedSizes(); - // Updates internal structures that keep track of compaction scores // We use compaction scores to figure out which compaction to do next // REQUIRES: db_mutex held!! @@ -192,24 +187,10 @@ class VersionStorageInfo { double blob_garbage_collection_age_cutoff, double blob_garbage_collection_force_threshold); - // Generate level_files_brief_ from files_ - void GenerateLevelFilesBrief(); - // Sort all files for this version based on their file size and - // record results in files_by_compaction_pri_. The largest files are listed - // first. - void UpdateFilesByCompactionPri(const ImmutableOptions& immutable_options, - const MutableCFOptions& mutable_cf_options); - - void GenerateLevel0NonOverlapping(); bool level0_non_overlapping() const { return level0_non_overlapping_; } - // Check whether each file in this version is bottommost (i.e., nothing in its - // key-range could possibly exist in an older file/level). - // REQUIRES: This version has not been saved - void GenerateBottommostFiles(); - // Updates the oldest snapshot and related internal state, like the bottommost // files marked for compaction. // REQUIRES: DB mutex held @@ -270,14 +251,13 @@ class VersionStorageInfo { int num_levels() const { return num_levels_; } - // REQUIRES: This version has been saved (see VersionSet::SaveTo) + // REQUIRES: PrepareForVersionAppend has been called int num_non_empty_levels() const { assert(finalized_); return num_non_empty_levels_; } - // REQUIRES: This version has been finalized. - // (CalculateBaseBytes() is called) + // REQUIRES: PrepareForVersionAppend has been called // This may or may not return number of level files. It is to keep backward // compatible behavior in universal compaction. int l0_delay_trigger_count() const { return l0_delay_trigger_count_; } @@ -372,13 +352,13 @@ class VersionStorageInfo { return level_files_brief_[level]; } - // REQUIRES: This version has been saved (see VersionSet::SaveTo) + // REQUIRES: PrepareForVersionAppend has been called const std::vector& FilesByCompactionPri(int level) const { assert(finalized_); return files_by_compaction_pri_[level]; } - // REQUIRES: This version has been saved (see VersionSet::SaveTo) + // REQUIRES: ComputeCompactionScore has been called // REQUIRES: DB mutex held during access const autovector>& FilesMarkedForCompaction() const { @@ -386,14 +366,14 @@ class VersionStorageInfo { return files_marked_for_compaction_; } - // REQUIRES: This version has been saved (see VersionSet::SaveTo) + // REQUIRES: ComputeCompactionScore has been called // REQUIRES: DB mutex held during access const autovector>& ExpiredTtlFiles() const { assert(finalized_); return expired_ttl_files_; } - // REQUIRES: This version has been saved (see VersionSet::SaveTo) + // REQUIRES: ComputeCompactionScore has been called // REQUIRES: DB mutex held during access const autovector>& FilesMarkedForPeriodicCompaction() const { @@ -405,7 +385,7 @@ class VersionStorageInfo { files_marked_for_periodic_compaction_.emplace_back(level, f); } - // REQUIRES: This version has been saved (see VersionSet::SaveTo) + // REQUIRES: ComputeCompactionScore has been called // REQUIRES: DB mutex held during access const autovector>& BottommostFilesMarkedForCompaction() const { @@ -413,7 +393,7 @@ class VersionStorageInfo { return bottommost_files_marked_for_compaction_; } - // REQUIRES: This version has been saved (see VersionSet::SaveTo) + // REQUIRES: ComputeCompactionScore has been called // REQUIRES: DB mutex held during access const autovector>& FilesMarkedForForcedBlobGC() const { @@ -436,7 +416,7 @@ class VersionStorageInfo { return next_file_to_compact_by_size_[level]; } - // REQUIRES: This version has been saved (see VersionSet::SaveTo) + // REQUIRES: PrepareForVersionAppend has been called const FileIndexer& file_indexer() const { assert(finalized_); return file_indexer_; @@ -498,10 +478,6 @@ class VersionStorageInfo { // Returns maximum total bytes of data on a given level. uint64_t MaxBytesForLevel(int level) const; - // Must be called after any change to MutableCFOptions. - void CalculateBaseBytes(const ImmutableOptions& ioptions, - const MutableCFOptions& options); - // Returns an estimate of the amount of live data in bytes. uint64_t EstimateLiveDataSize() const; @@ -530,6 +506,21 @@ class VersionStorageInfo { int last_level, int last_l0_idx); private: + void ComputeCompensatedSizes(); + void UpdateNumNonEmptyLevels(); + void CalculateBaseBytes(const ImmutableOptions& ioptions, + const MutableCFOptions& options); + void UpdateFilesByCompactionPri(const ImmutableOptions& immutable_options, + const MutableCFOptions& mutable_cf_options); + + void GenerateFileIndexer() { + file_indexer_.UpdateIndex(&arena_, num_non_empty_levels_, files_); + } + + void GenerateLevelFilesBrief(); + void GenerateLevel0NonOverlapping(); + void GenerateBottommostFiles(); + const InternalKeyComparator* internal_comparator_; const Comparator* user_comparator_; int num_levels_; // Number of levels @@ -619,7 +610,7 @@ class VersionStorageInfo { // Level that should be compacted next and its compaction score. // Score < 1 means compaction is not strictly needed. These fields - // are initialized by Finalize(). + // are initialized by ComputeCompactionScore. // The most critical level to be compacted is listed first // These are used to pick the best compaction level std::vector compaction_score_; @@ -667,7 +658,6 @@ class Version { // yield the contents of this Version when merged together. // @param read_options Must outlive any iterator built by // `merger_iter_builder`. - // REQUIRES: This version has been saved (see VersionSet::SaveTo). void AddIterators(const ReadOptions& read_options, const FileOptions& soptions, MergeIteratorBuilder* merger_iter_builder, @@ -740,10 +730,11 @@ class Version { void MultiGetBlob(const ReadOptions& read_options, MultiGetRange& range, std::unordered_map& blob_rqs); - // Loads some stats information from files. Call without mutex held. It needs - // to be called before applying the version to the version set. - void PrepareApply(const MutableCFOptions& mutable_cf_options, - bool update_stats); + // Loads some stats information from files (if update_stats is set) and + // populates derived data structures. Call without mutex held. It needs to be + // called before appending the version to the version set. + void PrepareAppend(const MutableCFOptions& mutable_cf_options, + bool update_stats); // Reference count management (so Versions do not disappear out from // under live iterators) @@ -849,7 +840,7 @@ class Version { // Update the accumulated stats associated with the current version. // This accumulated stats will be used in compaction. - void UpdateAccumulatedStats(bool update_stats); + void UpdateAccumulatedStats(); ColumnFamilyData* cfd_; // ColumnFamilyData to which this Version belongs Logger* info_log_; @@ -1296,7 +1287,7 @@ class VersionSet { new Version(cfd, this, file_options_, mutable_cf_options, io_tracer_); constexpr bool update_stats = false; - version->PrepareApply(mutable_cf_options, update_stats); + version->PrepareAppend(mutable_cf_options, update_stats); AppendVersion(cfd, version); } diff --git a/db/version_set_test.cc b/db/version_set_test.cc index bb3df039b..2974849f1 100644 --- a/db/version_set_test.cc +++ b/db/version_set_test.cc @@ -178,15 +178,8 @@ class VersionStorageInfoTestBase : public testing::Test { vstorage_.AddBlobFile(std::move(meta)); } - void Finalize() { - vstorage_.UpdateNumNonEmptyLevels(); - vstorage_.CalculateBaseBytes(ioptions_, mutable_cf_options_); - vstorage_.UpdateFilesByCompactionPri(ioptions_, mutable_cf_options_); - vstorage_.GenerateFileIndexer(); - vstorage_.GenerateLevelFilesBrief(); - vstorage_.GenerateLevel0NonOverlapping(); - vstorage_.GenerateBottommostFiles(); - + void UpdateVersionStorageInfo() { + vstorage_.PrepareForVersionAppend(ioptions_, mutable_cf_options_); vstorage_.SetFinalized(); } @@ -217,10 +210,12 @@ TEST_F(VersionStorageInfoTest, MaxBytesForLevelStatic) { ioptions_.level_compaction_dynamic_level_bytes = false; mutable_cf_options_.max_bytes_for_level_base = 10; mutable_cf_options_.max_bytes_for_level_multiplier = 5; - Add(4, 100U, "1", "2"); - Add(5, 101U, "1", "2"); - vstorage_.CalculateBaseBytes(ioptions_, mutable_cf_options_); + Add(4, 100U, "1", "2", 100U); + Add(5, 101U, "1", "2", 100U); + + UpdateVersionStorageInfo(); + ASSERT_EQ(vstorage_.MaxBytesForLevel(1), 10U); ASSERT_EQ(vstorage_.MaxBytesForLevel(2), 50U); ASSERT_EQ(vstorage_.MaxBytesForLevel(3), 250U); @@ -229,40 +224,84 @@ TEST_F(VersionStorageInfoTest, MaxBytesForLevelStatic) { ASSERT_EQ(0, logger_->log_count); } -TEST_F(VersionStorageInfoTest, MaxBytesForLevelDynamic) { +TEST_F(VersionStorageInfoTest, MaxBytesForLevelDynamic_1) { ioptions_.level_compaction_dynamic_level_bytes = true; mutable_cf_options_.max_bytes_for_level_base = 1000; mutable_cf_options_.max_bytes_for_level_multiplier = 5; + Add(5, 1U, "1", "2", 500U); - vstorage_.CalculateBaseBytes(ioptions_, mutable_cf_options_); + UpdateVersionStorageInfo(); + ASSERT_EQ(0, logger_->log_count); ASSERT_EQ(vstorage_.base_level(), 5); +} +TEST_F(VersionStorageInfoTest, MaxBytesForLevelDynamic_2) { + ioptions_.level_compaction_dynamic_level_bytes = true; + mutable_cf_options_.max_bytes_for_level_base = 1000; + mutable_cf_options_.max_bytes_for_level_multiplier = 5; + + Add(5, 1U, "1", "2", 500U); Add(5, 2U, "3", "4", 550U); - vstorage_.CalculateBaseBytes(ioptions_, mutable_cf_options_); + + UpdateVersionStorageInfo(); + ASSERT_EQ(0, logger_->log_count); ASSERT_EQ(vstorage_.MaxBytesForLevel(4), 1000U); ASSERT_EQ(vstorage_.base_level(), 4); +} + +TEST_F(VersionStorageInfoTest, MaxBytesForLevelDynamic_3) { + ioptions_.level_compaction_dynamic_level_bytes = true; + mutable_cf_options_.max_bytes_for_level_base = 1000; + mutable_cf_options_.max_bytes_for_level_multiplier = 5; + Add(5, 1U, "1", "2", 500U); + Add(5, 2U, "3", "4", 550U); Add(4, 3U, "3", "4", 550U); - vstorage_.CalculateBaseBytes(ioptions_, mutable_cf_options_); + + UpdateVersionStorageInfo(); + ASSERT_EQ(0, logger_->log_count); ASSERT_EQ(vstorage_.MaxBytesForLevel(4), 1000U); ASSERT_EQ(vstorage_.base_level(), 4); +} +TEST_F(VersionStorageInfoTest, MaxBytesForLevelDynamic_4) { + ioptions_.level_compaction_dynamic_level_bytes = true; + mutable_cf_options_.max_bytes_for_level_base = 1000; + mutable_cf_options_.max_bytes_for_level_multiplier = 5; + + Add(5, 1U, "1", "2", 500U); + Add(5, 2U, "3", "4", 550U); + Add(4, 3U, "3", "4", 550U); Add(3, 4U, "3", "4", 250U); Add(3, 5U, "5", "7", 300U); - vstorage_.CalculateBaseBytes(ioptions_, mutable_cf_options_); + + UpdateVersionStorageInfo(); + ASSERT_EQ(1, logger_->log_count); ASSERT_EQ(vstorage_.MaxBytesForLevel(4), 1005U); ASSERT_EQ(vstorage_.MaxBytesForLevel(3), 1000U); ASSERT_EQ(vstorage_.base_level(), 3); +} + +TEST_F(VersionStorageInfoTest, MaxBytesForLevelDynamic_5) { + ioptions_.level_compaction_dynamic_level_bytes = true; + mutable_cf_options_.max_bytes_for_level_base = 1000; + mutable_cf_options_.max_bytes_for_level_multiplier = 5; + Add(5, 1U, "1", "2", 500U); + Add(5, 2U, "3", "4", 550U); + Add(4, 3U, "3", "4", 550U); + Add(3, 4U, "3", "4", 250U); + Add(3, 5U, "5", "7", 300U); Add(1, 6U, "3", "4", 5U); Add(1, 7U, "8", "9", 5U); - logger_->log_count = 0; - vstorage_.CalculateBaseBytes(ioptions_, mutable_cf_options_); + + UpdateVersionStorageInfo(); + ASSERT_EQ(1, logger_->log_count); ASSERT_GT(vstorage_.MaxBytesForLevel(4), 1005U); ASSERT_GT(vstorage_.MaxBytesForLevel(3), 1005U); @@ -275,6 +314,7 @@ TEST_F(VersionStorageInfoTest, MaxBytesForLevelDynamicLotsOfData) { ioptions_.level_compaction_dynamic_level_bytes = true; mutable_cf_options_.max_bytes_for_level_base = 100; mutable_cf_options_.max_bytes_for_level_multiplier = 2; + Add(0, 1U, "1", "2", 50U); Add(1, 2U, "1", "2", 50U); Add(2, 3U, "1", "2", 500U); @@ -282,7 +322,8 @@ TEST_F(VersionStorageInfoTest, MaxBytesForLevelDynamicLotsOfData) { Add(4, 5U, "1", "2", 1700U); Add(5, 6U, "1", "2", 500U); - vstorage_.CalculateBaseBytes(ioptions_, mutable_cf_options_); + UpdateVersionStorageInfo(); + ASSERT_EQ(vstorage_.MaxBytesForLevel(4), 800U); ASSERT_EQ(vstorage_.MaxBytesForLevel(3), 400U); ASSERT_EQ(vstorage_.MaxBytesForLevel(2), 200U); @@ -296,12 +337,14 @@ TEST_F(VersionStorageInfoTest, MaxBytesForLevelDynamicLargeLevel) { ioptions_.level_compaction_dynamic_level_bytes = true; mutable_cf_options_.max_bytes_for_level_base = 10U * kOneGB; mutable_cf_options_.max_bytes_for_level_multiplier = 10; + Add(0, 1U, "1", "2", 50U); Add(3, 4U, "1", "2", 32U * kOneGB); Add(4, 5U, "1", "2", 500U * kOneGB); Add(5, 6U, "1", "2", 3000U * kOneGB); - vstorage_.CalculateBaseBytes(ioptions_, mutable_cf_options_); + UpdateVersionStorageInfo(); + ASSERT_EQ(vstorage_.MaxBytesForLevel(5), 3000U * kOneGB); ASSERT_EQ(vstorage_.MaxBytesForLevel(4), 300U * kOneGB); ASSERT_EQ(vstorage_.MaxBytesForLevel(3), 30U * kOneGB); @@ -325,7 +368,8 @@ TEST_F(VersionStorageInfoTest, MaxBytesForLevelDynamicWithLargeL0_1) { Add(3, 6U, "1", "2", 40000U); Add(2, 7U, "1", "2", 8000U); - vstorage_.CalculateBaseBytes(ioptions_, mutable_cf_options_); + UpdateVersionStorageInfo(); + ASSERT_EQ(0, logger_->log_count); ASSERT_EQ(2, vstorage_.base_level()); // level multiplier should be 3.5 @@ -351,7 +395,8 @@ TEST_F(VersionStorageInfoTest, MaxBytesForLevelDynamicWithLargeL0_2) { Add(3, 6U, "1", "2", 40000U); Add(2, 7U, "1", "2", 8000U); - vstorage_.CalculateBaseBytes(ioptions_, mutable_cf_options_); + UpdateVersionStorageInfo(); + ASSERT_EQ(0, logger_->log_count); ASSERT_EQ(2, vstorage_.base_level()); // level multiplier should be 3.5 @@ -383,7 +428,8 @@ TEST_F(VersionStorageInfoTest, MaxBytesForLevelDynamicWithLargeL0_3) { Add(3, 6U, "1", "2", 40000U); Add(2, 7U, "1", "2", 8000U); - vstorage_.CalculateBaseBytes(ioptions_, mutable_cf_options_); + UpdateVersionStorageInfo(); + ASSERT_EQ(0, logger_->log_count); ASSERT_EQ(2, vstorage_.base_level()); // level multiplier should be 3.5 @@ -406,6 +452,9 @@ TEST_F(VersionStorageInfoTest, EstimateLiveDataSize) { Add(4, 5U, "4", "5", 1U); // Inside range of last level Add(4, 6U, "6", "7", 1U); // Inside range of last level Add(5, 7U, "4", "7", 10U); + + UpdateVersionStorageInfo(); + ASSERT_EQ(10U, vstorage_.EstimateLiveDataSize()); } @@ -417,6 +466,9 @@ TEST_F(VersionStorageInfoTest, EstimateLiveDataSize2) { Add(1, 5U, "5", "6", 1U); Add(2, 6U, "2", "3", 1U); Add(3, 7U, "7", "8", 1U); + + UpdateVersionStorageInfo(); + ASSERT_EQ(4U, vstorage_.EstimateLiveDataSize()); } @@ -430,8 +482,8 @@ TEST_F(VersionStorageInfoTest, GetOverlappingInputs) { // Two files that do not overlap. Add(1, 5U, {"g", 0, kTypeValue}, {"h", 0, kTypeValue}, 1); Add(1, 6U, {"i", 0, kTypeValue}, {"j", 0, kTypeValue}, 1); - vstorage_.UpdateNumNonEmptyLevels(); - vstorage_.GenerateLevelFilesBrief(); + + UpdateVersionStorageInfo(); ASSERT_EQ("1,2", GetOverlappingFiles( 1, {"a", 0, kTypeValue}, {"b", 0, kTypeValue})); @@ -459,6 +511,8 @@ TEST_F(VersionStorageInfoTest, FileLocationAndMetaDataByNumber) { Add(2, 7U, "1", "2", 8000U); + UpdateVersionStorageInfo(); + ASSERT_EQ(vstorage_.GetFileLocation(11U), VersionStorageInfo::FileLocation(0, 0)); ASSERT_NE(vstorage_.GetFileMetaDataByNumber(11U), nullptr); @@ -477,7 +531,7 @@ TEST_F(VersionStorageInfoTest, FileLocationAndMetaDataByNumber) { TEST_F(VersionStorageInfoTest, ForcedBlobGCEmpty) { // No SST or blob files in VersionStorageInfo - Finalize(); + UpdateVersionStorageInfo(); constexpr double age_cutoff = 0.5; constexpr double force_threshold = 0.75; @@ -573,7 +627,7 @@ TEST_F(VersionStorageInfoTest, ForcedBlobGC) { garbage_blob_bytes); } - Finalize(); + UpdateVersionStorageInfo(); assert(vstorage_.num_levels() > 0); const auto& level_files = vstorage_.LevelFiles(level); @@ -677,8 +731,9 @@ TEST_F(VersionStorageInfoTimestampTest, GetOverlappingInputs) { /*largest=*/ {PackUserKeyAndTimestamp("d", /*ts=*/1), /*s=*/0, kTypeValue}, /*file_size=*/100); - vstorage_.UpdateNumNonEmptyLevels(); - vstorage_.GenerateLevelFilesBrief(); + + UpdateVersionStorageInfo(); + ASSERT_EQ( "1,2", GetOverlappingFiles( diff --git a/tools/ldb_cmd_test.cc b/tools/ldb_cmd_test.cc index c0e412789..eaaaea6a5 100644 --- a/tools/ldb_cmd_test.cc +++ b/tools/ldb_cmd_test.cc @@ -788,7 +788,7 @@ TEST_F(LdbCmdTest, DisableConsistencyChecks) { char* argv[] = {arg1, arg2, arg3}; SyncPoint::GetInstance()->SetCallBack( - "Version::PrepareApply:forced_check", [&](void* arg) { + "Version::PrepareAppend:forced_check", [&](void* arg) { bool* forced = reinterpret_cast(arg); ASSERT_TRUE(*forced); }); @@ -808,7 +808,7 @@ TEST_F(LdbCmdTest, DisableConsistencyChecks) { char* argv[] = {arg1, arg2, arg3}; SyncPoint::GetInstance()->SetCallBack( - "Version::PrepareApply:forced_check", [&](void* arg) { + "Version::PrepareAppend:forced_check", [&](void* arg) { bool* forced = reinterpret_cast(arg); ASSERT_TRUE(*forced); });