From 9efbd85ac9691357355660c7fdf7e5001befbb6b Mon Sep 17 00:00:00 2001 From: sdong Date: Tue, 6 May 2014 14:51:33 -0700 Subject: [PATCH] fsync directory after creating current file in NewDB() Summary: One of our users reported current file corruption. The machine was rebooted during the time. This is the only think I can think of which could cause current file corruption. Just add this paranoid check. Test Plan: make all check Reviewers: haobo, igor Reviewed By: haobo CC: yhchiang, dhruba, leveldb Differential Revision: https://reviews.facebook.net/D18495 --- db/db_impl.cc | 2 +- db/filename.cc | 9 +++++++-- db/filename.h | 4 +++- db/repair.cc | 2 +- db/version_set.cc | 6 ++---- 5 files changed, 14 insertions(+), 9 deletions(-) diff --git a/db/db_impl.cc b/db/db_impl.cc index 25d8a072b..cbdc80c03 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -487,7 +487,7 @@ Status DBImpl::NewDB() { } if (s.ok()) { // Make "CURRENT" file that points to the new manifest file. - s = SetCurrentFile(env_, dbname_, 1); + s = SetCurrentFile(env_, dbname_, 1, db_directory_.get()); } else { env_->DeleteFile(manifest); } diff --git a/db/filename.cc b/db/filename.cc index 4b3ac8e98..d19f0fd53 100644 --- a/db/filename.cc +++ b/db/filename.cc @@ -226,7 +226,8 @@ bool ParseFileName(const std::string& fname, } Status SetCurrentFile(Env* env, const std::string& dbname, - uint64_t descriptor_number) { + uint64_t descriptor_number, + Directory* directory_to_fsync) { // Remove leading "dbname/" and add newline to manifest file name std::string manifest = DescriptorFileName(dbname, descriptor_number); Slice contents = manifest; @@ -237,7 +238,11 @@ Status SetCurrentFile(Env* env, const std::string& dbname, if (s.ok()) { s = env->RenameFile(tmp, CurrentFileName(dbname)); } - if (!s.ok()) { + if (s.ok()) { + if (directory_to_fsync != nullptr) { + directory_to_fsync->Fsync(); + } + } else { env->DeleteFile(tmp); } return s; diff --git a/db/filename.h b/db/filename.h index 8e55f1139..c4c306946 100644 --- a/db/filename.h +++ b/db/filename.h @@ -20,6 +20,7 @@ namespace rocksdb { class Env; +class Directory; enum FileType { kLogFile, @@ -100,7 +101,8 @@ extern bool ParseFileName(const std::string& filename, // Make the CURRENT file point to the descriptor file with the // specified number. extern Status SetCurrentFile(Env* env, const std::string& dbname, - uint64_t descriptor_number); + uint64_t descriptor_number, + Directory* directory_to_fsync); // Make the IDENTITY file for the db extern Status SetIdentityFile(Env* env, const std::string& dbname); diff --git a/db/repair.cc b/db/repair.cc index 8ae64b219..03571a829 100644 --- a/db/repair.cc +++ b/db/repair.cc @@ -363,7 +363,7 @@ class Repairer { // Install new manifest status = env_->RenameFile(tmp, DescriptorFileName(dbname_, 1)); if (status.ok()) { - status = SetCurrentFile(env_, dbname_, 1); + status = SetCurrentFile(env_, dbname_, 1, nullptr); } else { env_->DeleteFile(tmp); } diff --git a/db/version_set.cc b/db/version_set.cc index 00d9caf10..4a89e0226 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -1731,7 +1731,8 @@ Status VersionSet::LogAndApply(ColumnFamilyData* column_family_data, // If we just created a new descriptor file, install it by writing a // new CURRENT file that points to it. if (s.ok() && new_descriptor_log) { - s = SetCurrentFile(env_, dbname_, pending_manifest_file_number_); + s = SetCurrentFile(env_, dbname_, pending_manifest_file_number_, + db_directory); if (s.ok() && pending_manifest_file_number_ > manifest_file_number_) { // delete old manifest file Log(options_->info_log, @@ -1741,9 +1742,6 @@ Status VersionSet::LogAndApply(ColumnFamilyData* column_family_data, // of it later env_->DeleteFile(DescriptorFileName(dbname_, manifest_file_number_)); } - if (!options_->disableDataSync && db_directory != nullptr) { - db_directory->Fsync(); - } } if (s.ok()) {