From beeb74be6f8b19f99e6878b3b14c8ba4e2e862c5 Mon Sep 17 00:00:00 2001 From: Igor Canadi Date: Fri, 1 Nov 2013 12:32:27 -0700 Subject: [PATCH] Move I/O outside of lock Summary: I'm figuring out how Version[Set, Edit, ] classes work and I stumbled on this. It doesn't seem that the comment is accurate anymore. What I read is when the manifest grows too big, create a new file (and not only when we call LogAndApply for the first time). Test Plan: make check (currently running) Reviewers: dhruba, haobo, kailiu, emayanke Reviewed By: dhruba CC: leveldb Differential Revision: https://reviews.facebook.net/D13839 --- db/version_set.cc | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/db/version_set.cc b/db/version_set.cc index 578fefe863..55293ca2e3 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -1248,18 +1248,8 @@ Status VersionSet::LogAndApply(VersionEdit* edit, port::Mutex* mu, } if (!descriptor_log_ || new_descriptor_log) { - // No reason to unlock *mu here since we only hit this path in the - // first call to LogAndApply (when opening the database). - assert(!descriptor_log_ || new_descriptor_log); new_manifest_file = DescriptorFileName(dbname_, manifest_file_number_); edit->SetNextFile(next_file_number_); - unique_ptr descriptor_file; - s = env_->NewWritableFile(new_manifest_file, &descriptor_file, - storage_options_); - if (s.ok()) { - descriptor_log_.reset(new log::Writer(std::move(descriptor_file))); - s = WriteSnapshot(descriptor_log_.get()); - } } // Unlock during expensive MANIFEST log write. New writes cannot get here @@ -1271,6 +1261,18 @@ Status VersionSet::LogAndApply(VersionEdit* edit, port::Mutex* mu, mu->Unlock(); + // This is fine because everything inside of this block is serialized -- + // only one thread can be here at the same time + if (!new_manifest_file.empty()) { + unique_ptr descriptor_file; + s = env_->NewWritableFile(new_manifest_file, &descriptor_file, + storage_options_); + if (s.ok()) { + descriptor_log_.reset(new log::Writer(std::move(descriptor_file))); + s = WriteSnapshot(descriptor_log_.get()); + } + } + // The calls to Finalize and UpdateFilesBySize are cpu-heavy // and is best called outside the mutex. Finalize(v, size_being_compacted);