From d0d484b132f4476a8aeba3e26ce6546911a5db72 Mon Sep 17 00:00:00 2001 From: anand76 Date: Tue, 29 Jan 2019 14:27:30 -0800 Subject: [PATCH] Always delete Blob DB files in the background (#4928) Summary: Blob DB files are not tracked by the SFM, so they currently don't get deleted in the background. Force them to be deleted in background so rate limiting can be applied Pull Request resolved: https://github.com/facebook/rocksdb/pull/4928 Differential Revision: D13854649 Pulled By: anand1976 fbshipit-source-id: 8031ce66842ff0af440c715d886b377983dad7d8 --- util/delete_scheduler.cc | 7 ++++--- util/delete_scheduler.h | 7 +++++-- util/file_util.cc | 9 ++++++++- util/file_util.h | 5 +++++ util/sst_file_manager_impl.cc | 6 ++++-- util/sst_file_manager_impl.h | 7 +++++-- utilities/blob_db/blob_db_impl.cc | 6 +++--- 7 files changed, 34 insertions(+), 13 deletions(-) diff --git a/util/delete_scheduler.cc b/util/delete_scheduler.cc index a8078b94a..f5ee28448 100644 --- a/util/delete_scheduler.cc +++ b/util/delete_scheduler.cc @@ -52,11 +52,12 @@ DeleteScheduler::~DeleteScheduler() { } Status DeleteScheduler::DeleteFile(const std::string& file_path, - const std::string& dir_to_sync) { + const std::string& dir_to_sync, + const bool force_bg) { Status s; - if (rate_bytes_per_sec_.load() <= 0 || + if (rate_bytes_per_sec_.load() <= 0 || (!force_bg && total_trash_size_.load() > - sst_file_manager_->GetTotalSize() * max_trash_db_ratio_.load()) { + sst_file_manager_->GetTotalSize() * max_trash_db_ratio_.load())) { // Rate limiting is disabled or trash size makes up more than // max_trash_db_ratio_ (default 25%) of the total DB size TEST_SYNC_POINT("DeleteScheduler::DeleteFile"); diff --git a/util/delete_scheduler.h b/util/delete_scheduler.h index cbd13ecef..29b70517b 100644 --- a/util/delete_scheduler.h +++ b/util/delete_scheduler.h @@ -46,8 +46,11 @@ class DeleteScheduler { rate_bytes_per_sec_.store(bytes_per_sec); } - // Mark file as trash directory and schedule it's deletion - Status DeleteFile(const std::string& fname, const std::string& dir_to_sync); + // Mark file as trash directory and schedule it's deletion. If force_bg is + // set, it forces the file to always be deleted in the background thread, + // except when rate limiting is disabled + Status DeleteFile(const std::string& fname, const std::string& dir_to_sync, + const bool force_bg = false); // Wait for all files being deleteing in the background to finish or for // destructor to be called. diff --git a/util/file_util.cc b/util/file_util.cc index bf56592ef..3f730f3e8 100644 --- a/util/file_util.cc +++ b/util/file_util.cc @@ -89,16 +89,23 @@ Status CreateFile(Env* env, const std::string& destination, Status DeleteSSTFile(const ImmutableDBOptions* db_options, const std::string& fname, const std::string& dir_to_sync) { + return DeleteDBFile(db_options, fname, dir_to_sync, false); +} + +Status DeleteDBFile(const ImmutableDBOptions* db_options, + const std::string& fname, const std::string& dir_to_sync, + const bool force_bg) { #ifndef ROCKSDB_LITE auto sfm = static_cast(db_options->sst_file_manager.get()); if (sfm) { - return sfm->ScheduleFileDeletion(fname, dir_to_sync); + return sfm->ScheduleFileDeletion(fname, dir_to_sync, force_bg); } else { return db_options->env->DeleteFile(fname); } #else (void)dir_to_sync; + (void)force_bg; // SstFileManager is not supported in ROCKSDB_LITE return db_options->env->DeleteFile(fname); #endif diff --git a/util/file_util.h b/util/file_util.h index 5c05c9def..cd054518e 100644 --- a/util/file_util.h +++ b/util/file_util.h @@ -25,4 +25,9 @@ extern Status DeleteSSTFile(const ImmutableDBOptions* db_options, const std::string& fname, const std::string& path_to_sync); +extern Status DeleteDBFile(const ImmutableDBOptions* db_options, + const std::string& fname, + const std::string& path_to_sync, + const bool force_bg); + } // namespace rocksdb diff --git a/util/sst_file_manager_impl.cc b/util/sst_file_manager_impl.cc index 0b46b24b1..733cd9cf6 100644 --- a/util/sst_file_manager_impl.cc +++ b/util/sst_file_manager_impl.cc @@ -402,9 +402,11 @@ bool SstFileManagerImpl::CancelErrorRecovery(ErrorHandler* handler) { } Status SstFileManagerImpl::ScheduleFileDeletion( - const std::string& file_path, const std::string& path_to_sync) { + const std::string& file_path, const std::string& path_to_sync, + const bool force_bg) { TEST_SYNC_POINT("SstFileManagerImpl::ScheduleFileDeletion"); - return delete_scheduler_.DeleteFile(file_path, path_to_sync); + return delete_scheduler_.DeleteFile(file_path, path_to_sync, + force_bg); } void SstFileManagerImpl::WaitForEmptyTrash() { diff --git a/util/sst_file_manager_impl.h b/util/sst_file_manager_impl.h index d11035df8..211b4fa71 100644 --- a/util/sst_file_manager_impl.h +++ b/util/sst_file_manager_impl.h @@ -111,9 +111,12 @@ class SstFileManagerImpl : public SstFileManager { // not guaranteed bool CancelErrorRecovery(ErrorHandler* db); - // Mark file as trash and schedule it's deletion. + // Mark file as trash and schedule it's deletion. If force_bg is set, it + // forces the file to be deleting in the background regardless of DB size, + // except when rate limited delete is disabled virtual Status ScheduleFileDeletion(const std::string& file_path, - const std::string& dir_to_sync); + const std::string& dir_to_sync, + const bool force_bg = false); // Wait for all files being deleteing in the background to finish or for // destructor to be called. diff --git a/utilities/blob_db/blob_db_impl.cc b/utilities/blob_db/blob_db_impl.cc index 19318760b..b69947d99 100644 --- a/utilities/blob_db/blob_db_impl.cc +++ b/utilities/blob_db/blob_db_impl.cc @@ -1743,8 +1743,8 @@ std::pair BlobDBImpl::DeleteObsoleteFiles(bool aborted) { bfile->PathName().c_str()); blob_files_.erase(bfile->BlobFileNumber()); - Status s = DeleteSSTFile(&(db_impl_->immutable_db_options()), - bfile->PathName(), blob_dir_); + Status s = DeleteDBFile(&(db_impl_->immutable_db_options()), + bfile->PathName(), blob_dir_, true); if (!s.ok()) { ROCKS_LOG_ERROR(db_options_.info_log, "File failed to be deleted as obsolete %s", @@ -1834,7 +1834,7 @@ Status DestroyBlobDB(const std::string& dbname, const Options& options, uint64_t number; FileType type; if (ParseFileName(f, &number, &type) && type == kBlobFile) { - Status del = DeleteSSTFile(&soptions, blobdir + "/" + f, blobdir); + Status del = DeleteDBFile(&soptions, blobdir + "/" + f, blobdir, true); if (status.ok() && !del.ok()) { status = del; }