From 1551f1011ab419d21ac50be4b894cf3f688b30d7 Mon Sep 17 00:00:00 2001 From: Levi Tamasi Date: Tue, 19 May 2020 09:57:49 -0700 Subject: [PATCH] Refactor the blob file related logic in VersionBuilder (#6835) Summary: This patch is groundwork for an upcoming change to store the set of linked SSTs in `BlobFileMetaData`. With the current code, a new `BlobFileMetaData` object is created each time a `VersionEdit` touches a certain blob file. This is fine as long as these objects are lightweight and cheap to create; however, with the addition of the linked SST set, it would be very inefficient since the set would have to be copied over and over again. Note that this is the same kind of problem that `VersionBuilder` is solving w/r/t `Version`s and files, and we can apply the same solution; that is, we can accumulate the changes in a different mutable object, and apply the delta in one shot when the changes are committed. The patch does exactly that by adding a new `BlobFileMetaDataDelta` class to `VersionBuilder`. In addition, it turns the existing `GetBlobFileMetaData` helper into `IsBlobFileInVersion` (which is fine since that's the only thing the method's clients care about now), and adds a couple of helper methods that can create a `BlobFileMetaData` object from the `BlobFileMetaData` in the base (if applicable) and the delta when the `Version` is saved. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6835 Test Plan: `make check` Reviewed By: riversand963 Differential Revision: D21505187 Pulled By: ltamasi fbshipit-source-id: d81a48c5f2ca7b79d7124c935332a6bcf3d5d988 --- db/version_builder.cc | 180 ++++++++++++++++++++++++++++-------------- 1 file changed, 119 insertions(+), 61 deletions(-) diff --git a/db/version_builder.cc b/db/version_builder.cc index 6cc4480fa2..c8ef9b9860 100644 --- a/db/version_builder.cc +++ b/db/version_builder.cc @@ -86,6 +86,43 @@ class VersionBuilder::Rep { std::unordered_map added_files; }; + class BlobFileMetaDataDelta { + public: + bool IsEmpty() const { + return !shared_meta_ && !additional_garbage_count_ && + !additional_garbage_bytes_; + } + + std::shared_ptr GetSharedMeta() const { + return shared_meta_; + } + + uint64_t GetAdditionalGarbageCount() const { + return additional_garbage_count_; + } + + uint64_t GetAdditionalGarbageBytes() const { + return additional_garbage_bytes_; + } + + void SetSharedMeta(std::shared_ptr shared_meta) { + assert(!shared_meta_); + assert(shared_meta); + + shared_meta_ = std::move(shared_meta); + } + + void AddGarbage(uint64_t count, uint64_t bytes) { + additional_garbage_count_ += count; + additional_garbage_bytes_ += bytes; + } + + private: + std::shared_ptr shared_meta_; + uint64_t additional_garbage_count_ = 0; + uint64_t additional_garbage_bytes_ = 0; + }; + const FileOptions& file_options_; const ImmutableCFOptions* const ioptions_; TableCache* table_cache_; @@ -104,8 +141,8 @@ class VersionBuilder::Rep { FileComparator level_zero_cmp_; FileComparator level_nonzero_cmp_; - // Metadata for all blob files affected by the series of version edits. - std::map> changed_blob_files_; + // Metadata delta for all blob files affected by the series of version edits. + std::map blob_file_meta_deltas_; public: Rep(const FileOptions& file_options, const ImmutableCFOptions* ioptions, @@ -150,14 +187,12 @@ class VersionBuilder::Rep { } } - std::shared_ptr GetBlobFileMetaData( - uint64_t blob_file_number) const { - auto changed_it = changed_blob_files_.find(blob_file_number); - if (changed_it != changed_blob_files_.end()) { - const auto& meta = changed_it->second; - assert(meta); - - return meta; + bool IsBlobFileInVersion(uint64_t blob_file_number) const { + auto delta_it = blob_file_meta_deltas_.find(blob_file_number); + if (delta_it != blob_file_meta_deltas_.end()) { + if (delta_it->second.GetSharedMeta()) { + return true; + } } assert(base_vstorage_); @@ -166,13 +201,13 @@ class VersionBuilder::Rep { auto base_it = base_blob_files.find(blob_file_number); if (base_it != base_blob_files.end()) { - const auto& meta = base_it->second; - assert(meta); + assert(base_it->second); + assert(base_it->second->GetSharedMeta()); - return meta; + return true; } - return std::shared_ptr(); + return false; } Status CheckConsistencyOfOldestBlobFileReference( @@ -189,8 +224,7 @@ class VersionBuilder::Rep { return Status::OK(); } - const auto meta = GetBlobFileMetaData(blob_file_number); - if (!meta) { + if (!IsBlobFileInVersion(blob_file_number)) { std::ostringstream oss; oss << "Blob file #" << blob_file_number << " is not part of this version"; @@ -391,8 +425,7 @@ class VersionBuilder::Rep { Status ApplyBlobFileAddition(const BlobFileAddition& blob_file_addition) { const uint64_t blob_file_number = blob_file_addition.GetBlobFileNumber(); - auto meta = GetBlobFileMetaData(blob_file_number); - if (meta) { + if (IsBlobFileInVersion(blob_file_number)) { std::ostringstream oss; oss << "Blob file #" << blob_file_number << " already added"; @@ -423,12 +456,8 @@ class VersionBuilder::Rep { blob_file_addition.GetChecksumMethod(), blob_file_addition.GetChecksumValue(), deleter); - constexpr uint64_t garbage_blob_count = 0; - constexpr uint64_t garbage_blob_bytes = 0; - auto new_meta = BlobFileMetaData::Create( - std::move(shared_meta), garbage_blob_count, garbage_blob_bytes); - - changed_blob_files_.emplace(blob_file_number, std::move(new_meta)); + blob_file_meta_deltas_[blob_file_number].SetSharedMeta( + std::move(shared_meta)); return Status::OK(); } @@ -436,22 +465,16 @@ class VersionBuilder::Rep { Status ApplyBlobFileGarbage(const BlobFileGarbage& blob_file_garbage) { const uint64_t blob_file_number = blob_file_garbage.GetBlobFileNumber(); - auto meta = GetBlobFileMetaData(blob_file_number); - if (!meta) { + if (!IsBlobFileInVersion(blob_file_number)) { std::ostringstream oss; oss << "Blob file #" << blob_file_number << " not found"; return Status::Corruption("VersionBuilder", oss.str()); } - assert(meta->GetBlobFileNumber() == blob_file_number); - - auto new_meta = BlobFileMetaData::Create( - meta->GetSharedMeta(), - meta->GetGarbageBlobCount() + blob_file_garbage.GetGarbageBlobCount(), - meta->GetGarbageBlobBytes() + blob_file_garbage.GetGarbageBlobBytes()); - - changed_blob_files_[blob_file_number] = std::move(new_meta); + blob_file_meta_deltas_[blob_file_number].AddGarbage( + blob_file_garbage.GetGarbageBlobCount(), + blob_file_garbage.GetGarbageBlobBytes()); return Status::OK(); } @@ -530,6 +553,40 @@ class VersionBuilder::Rep { return s; } + static std::shared_ptr CreateMetaDataForNewBlobFile( + const BlobFileMetaDataDelta& delta) { + auto shared_meta = delta.GetSharedMeta(); + assert(shared_meta); + + auto meta = BlobFileMetaData::Create(std::move(shared_meta), + delta.GetAdditionalGarbageCount(), + delta.GetAdditionalGarbageBytes()); + + return meta; + } + + static std::shared_ptr + GetOrCreateMetaDataForExistingBlobFile( + const std::shared_ptr& base_meta, + const BlobFileMetaDataDelta& delta) { + assert(base_meta); + assert(!delta.GetSharedMeta()); + + if (delta.IsEmpty()) { + return base_meta; + } + + auto shared_meta = base_meta->GetSharedMeta(); + assert(shared_meta); + + auto meta = BlobFileMetaData::Create( + std::move(shared_meta), + base_meta->GetGarbageBlobCount() + delta.GetAdditionalGarbageCount(), + base_meta->GetGarbageBlobBytes() + delta.GetAdditionalGarbageBytes()); + + return meta; + } + void AddBlobFileIfNeeded( VersionStorageInfo* vstorage, const std::shared_ptr& meta) const { @@ -551,42 +608,41 @@ class VersionBuilder::Rep { auto base_it = base_blob_files.begin(); const auto base_it_end = base_blob_files.end(); - auto changed_it = changed_blob_files_.begin(); - const auto changed_it_end = changed_blob_files_.end(); + auto delta_it = blob_file_meta_deltas_.begin(); + const auto delta_it_end = blob_file_meta_deltas_.end(); - while (base_it != base_it_end && changed_it != changed_it_end) { + while (base_it != base_it_end && delta_it != delta_it_end) { const uint64_t base_blob_file_number = base_it->first; - const uint64_t changed_blob_file_number = changed_it->first; + const uint64_t delta_blob_file_number = delta_it->first; - const auto& base_meta = base_it->second; - const auto& changed_meta = changed_it->second; - - assert(base_meta); - assert(changed_meta); - - if (base_blob_file_number < changed_blob_file_number) { + if (base_blob_file_number < delta_blob_file_number) { + const auto& base_meta = base_it->second; + assert(base_meta); assert(base_meta->GetGarbageBlobCount() < base_meta->GetTotalBlobCount()); vstorage->AddBlobFile(base_meta); ++base_it; - } else if (changed_blob_file_number < base_blob_file_number) { - AddBlobFileIfNeeded(vstorage, changed_meta); + } else if (delta_blob_file_number < base_blob_file_number) { + // Note: blob file numbers are strictly increasing over time and + // once blob files get marked obsolete, they never reappear. Thus, + // this case is not possible. + assert(false); - ++changed_it; + ++delta_it; } else { - assert(base_blob_file_number == changed_blob_file_number); - assert(base_meta->GetSharedMeta() == changed_meta->GetSharedMeta()); - assert(base_meta->GetGarbageBlobCount() <= - changed_meta->GetGarbageBlobCount()); - assert(base_meta->GetGarbageBlobBytes() <= - changed_meta->GetGarbageBlobBytes()); + assert(base_blob_file_number == delta_blob_file_number); - AddBlobFileIfNeeded(vstorage, changed_meta); + const auto& base_meta = base_it->second; + const auto& delta = delta_it->second; + + auto meta = GetOrCreateMetaDataForExistingBlobFile(base_meta, delta); + + AddBlobFileIfNeeded(vstorage, meta); ++base_it; - ++changed_it; + ++delta_it; } } @@ -599,12 +655,14 @@ class VersionBuilder::Rep { ++base_it; } - while (changed_it != changed_it_end) { - const auto& changed_meta = changed_it->second; - assert(changed_meta); + while (delta_it != delta_it_end) { + const auto& delta = delta_it->second; - AddBlobFileIfNeeded(vstorage, changed_meta); - ++changed_it; + auto meta = CreateMetaDataForNewBlobFile(delta); + + AddBlobFileIfNeeded(vstorage, meta); + + ++delta_it; } }