From a99fb6723355954a0bc537deb2e6176e0d17ccb6 Mon Sep 17 00:00:00 2001 From: Levi Tamasi Date: Tue, 11 Aug 2020 09:22:00 -0700 Subject: [PATCH] Remove redundant consistency check from VersionStorageInfo::AddFile (#7237) Summary: `VersionStorageInfo::AddFile` currently has a debug-mode consistency check to make sure the newly added file does not overlap with the previous one (for levels below L0). Considering that `VersionBuilder::CheckConsistency` also performs similar checks (in fact, those checks are more comprehensive and cover L0 as well), this check is redundant. The patch removes it and also cleans up `AddFile` a little. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7237 Test Plan: `make check` Reviewed By: riversand963 Differential Revision: D23041937 Pulled By: ltamasi fbshipit-source-id: e00665f3b83bfd17f86c54c238800f3d77d739bd --- db/version_builder.cc | 3 +-- db/version_set.cc | 30 +++++------------------------- db/version_set.h | 2 +- 3 files changed, 7 insertions(+), 28 deletions(-) diff --git a/db/version_builder.cc b/db/version_builder.cc index 126f752941..aecf446230 100644 --- a/db/version_builder.cc +++ b/db/version_builder.cc @@ -989,8 +989,7 @@ class VersionBuilder::Rep { if (add_it != add_files.end() && add_it->second != f) { vstorage->RemoveCurrentStats(f); } else { - assert(ioptions_); - vstorage->AddFile(level, f, ioptions_->info_log); + vstorage->AddFile(level, f); } } } diff --git a/db/version_set.cc b/db/version_set.cc index beb029d1f6..d9795b72f7 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -2645,37 +2645,17 @@ bool CompareCompensatedSizeDescending(const Fsize& first, const Fsize& second) { } } // anonymous namespace -void VersionStorageInfo::AddFile(int level, FileMetaData* f, Logger* info_log) { - auto* level_files = &files_[level]; - // Must not overlap -#ifndef NDEBUG - if (level > 0 && !level_files->empty() && - internal_comparator_->Compare( - (*level_files)[level_files->size() - 1]->largest, f->smallest) >= 0) { - auto* f2 = (*level_files)[level_files->size() - 1]; - if (info_log != nullptr) { - Error(info_log, "Adding new file %" PRIu64 - " range (%s, %s) to level %d but overlapping " - "with existing file %" PRIu64 " %s %s", - f->fd.GetNumber(), f->smallest.DebugString(true).c_str(), - f->largest.DebugString(true).c_str(), level, f2->fd.GetNumber(), - f2->smallest.DebugString(true).c_str(), - f2->largest.DebugString(true).c_str()); - LogFlush(info_log); - } - assert(false); - } -#else - (void)info_log; -#endif +void VersionStorageInfo::AddFile(int level, FileMetaData* f) { + auto& level_files = files_[level]; + level_files.push_back(f); + f->refs++; - level_files->push_back(f); const uint64_t file_number = f->fd.GetNumber(); assert(file_locations_.find(file_number) == file_locations_.end()); file_locations_.emplace(file_number, - FileLocation(level, level_files->size() - 1)); + FileLocation(level, level_files.size() - 1)); } void VersionStorageInfo::AddBlobFile( diff --git a/db/version_set.h b/db/version_set.h index 3d188d1035..741cb77243 100644 --- a/db/version_set.h +++ b/db/version_set.h @@ -119,7 +119,7 @@ class VersionStorageInfo { void Reserve(int level, size_t size) { files_[level].reserve(size); } - void AddFile(int level, FileMetaData* f, Logger* info_log = nullptr); + void AddFile(int level, FileMetaData* f); void AddBlobFile(std::shared_ptr blob_file_meta);