From b60c14f6ee00dc179d400573d4b172d228a8c5a8 Mon Sep 17 00:00:00 2001 From: Igor Canadi Date: Thu, 2 Jan 2014 03:33:42 -0800 Subject: [PATCH] Support multi-threaded DisableFileDeletions() and EnableFileDeletions() Summary: We don't want two threads to clash if they concurrently call DisableFileDeletions() and EnableFileDeletions(). I'm adding a counter that will enable file deletions only after all DisableFileDeletions() calls have been negated with EnableFileDeletions(). However, we also don't want to break the old behavior, so I added a parameter force to EnableFileDeletions(). If force is true, we will still enable file deletions after every call to EnableFileDeletions(), which is what is happening now. Test Plan: make check Reviewers: dhruba, haobo, sanketh Reviewed By: dhruba CC: leveldb Differential Revision: https://reviews.facebook.net/D14781 --- db/db_filesnapshot.cc | 28 ++++++++++++++++------ db/db_impl.cc | 6 ++--- db/db_impl.h | 9 +++++-- db/db_impl_readonly.h | 2 +- db/db_test.cc | 2 +- include/rocksdb/db.h | 10 +++++++- include/utilities/stackable_db.h | 4 ++-- utilities/backupable/backupable_db_test.cc | 2 +- 8 files changed, 45 insertions(+), 18 deletions(-) diff --git a/db/db_filesnapshot.cc b/db/db_filesnapshot.cc index 7b9c5ddeb..a7232246a 100644 --- a/db/db_filesnapshot.cc +++ b/db/db_filesnapshot.cc @@ -22,20 +22,34 @@ namespace rocksdb { Status DBImpl::DisableFileDeletions() { MutexLock l(&mutex_); - disable_delete_obsolete_files_ = true; - Log(options_.info_log, "File Deletions Disabled"); + ++disable_delete_obsolete_files_; + if (disable_delete_obsolete_files_ == 1) { + // if not, it has already been disabled, so don't log anything + Log(options_.info_log, "File Deletions Disabled"); + } return Status::OK(); } -Status DBImpl::EnableFileDeletions() { +Status DBImpl::EnableFileDeletions(bool force) { DeletionState deletion_state; + bool should_purge_files = false; { MutexLock l(&mutex_); - disable_delete_obsolete_files_ = false; - Log(options_.info_log, "File Deletions Enabled"); - FindObsoleteFiles(deletion_state, true); + if (force) { + // if force, we need to enable file deletions right away + disable_delete_obsolete_files_ = 0; + } else if (disable_delete_obsolete_files_ > 0) { + --disable_delete_obsolete_files_; + } + if (disable_delete_obsolete_files_ == 0) { + Log(options_.info_log, "File Deletions Enabled"); + should_purge_files = true; + FindObsoleteFiles(deletion_state, true); + } + } + if (should_purge_files) { + PurgeObsoleteFiles(deletion_state); } - PurgeObsoleteFiles(deletion_state); LogFlush(options_.info_log); return Status::OK(); } diff --git a/db/db_impl.cc b/db/db_impl.cc index ece08db8b..48dc367da 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -248,7 +248,7 @@ DBImpl::DBImpl(const Options& options, const std::string& dbname) bg_logstats_scheduled_(false), manual_compaction_(nullptr), logger_(nullptr), - disable_delete_obsolete_files_(false), + disable_delete_obsolete_files_(0), delete_obsolete_files_last_run_(options.env->NowMicros()), purge_wal_files_last_run_(0), last_stats_dump_time_microsec_(0), @@ -513,7 +513,7 @@ void DBImpl::FindObsoleteFiles(DeletionState& deletion_state, mutex_.AssertHeld(); // if deletion is disabled, do nothing - if (disable_delete_obsolete_files_) { + if (disable_delete_obsolete_files_ > 0) { return; } @@ -1248,7 +1248,7 @@ Status DBImpl::FlushMemTableToOutputFile(bool* madeProgress, MaybeScheduleLogDBDeployStats(); - if (!disable_delete_obsolete_files_) { + if (disable_delete_obsolete_files_ == 0) { // add to deletion state deletion_state.log_delete_files.insert( deletion_state.log_delete_files.end(), diff --git a/db/db_impl.h b/db/db_impl.h index 2447b31fa..52e8221f5 100644 --- a/db/db_impl.h +++ b/db/db_impl.h @@ -72,7 +72,7 @@ class DBImpl : public DB { virtual const Options& GetOptions() const; virtual Status Flush(const FlushOptions& options); virtual Status DisableFileDeletions(); - virtual Status EnableFileDeletions(); + virtual Status EnableFileDeletions(bool force); // All the returned filenames start with "/" virtual Status GetLiveFiles(std::vector&, uint64_t* manifest_file_size, @@ -416,7 +416,12 @@ class DBImpl : public DB { int64_t volatile last_log_ts; // shall we disable deletion of obsolete files - bool disable_delete_obsolete_files_; + // if 0 the deletion is enabled. + // if non-zero, files will not be getting deleted + // This enables two different threads to call + // EnableFileDeletions() and DisableFileDeletions() + // without any synchronization + int disable_delete_obsolete_files_; // last time when DeleteObsoleteFiles was invoked uint64_t delete_obsolete_files_last_run_; diff --git a/db/db_impl_readonly.h b/db/db_impl_readonly.h index af9c79ed0..4beaedd01 100644 --- a/db/db_impl_readonly.h +++ b/db/db_impl_readonly.h @@ -55,7 +55,7 @@ public: virtual Status DisableFileDeletions() { return Status::NotSupported("Not supported operation in read only mode."); } - virtual Status EnableFileDeletions() { + virtual Status EnableFileDeletions(bool force) { return Status::NotSupported("Not supported operation in read only mode."); } virtual Status GetLiveFiles(std::vector&, diff --git a/db/db_test.cc b/db/db_test.cc index 9615c8969..a0b3d9aaa 100644 --- a/db/db_test.cc +++ b/db/db_test.cc @@ -4462,7 +4462,7 @@ class ModelDB: public DB { virtual Status DisableFileDeletions() { return Status::OK(); } - virtual Status EnableFileDeletions() { + virtual Status EnableFileDeletions(bool force) { return Status::OK(); } virtual Status GetLiveFiles(std::vector&, uint64_t* size, diff --git a/include/rocksdb/db.h b/include/rocksdb/db.h index c4c5aa87f..dd17d9e9b 100644 --- a/include/rocksdb/db.h +++ b/include/rocksdb/db.h @@ -247,7 +247,15 @@ class DB { virtual Status DisableFileDeletions() = 0; // Allow compactions to delete obselete files. - virtual Status EnableFileDeletions() = 0; + // If force == true, the call to EnableFileDeletions() will guarantee that + // file deletions are enabled after the call, even if DisableFileDeletions() + // was called multiple times before. + // If force == false, EnableFileDeletions will only enable file deletion + // after it's been called at least as many times as DisableFileDeletions(), + // enabling the two methods to be called by two threads concurrently without + // synchronization -- i.e., file deletions will be enabled only after both + // threads call EnableFileDeletions() + virtual Status EnableFileDeletions(bool force = true) = 0; // GetLiveFiles followed by GetSortedWalFiles can generate a lossless backup diff --git a/include/utilities/stackable_db.h b/include/utilities/stackable_db.h index 2d86a611b..908fe10b7 100644 --- a/include/utilities/stackable_db.h +++ b/include/utilities/stackable_db.h @@ -123,8 +123,8 @@ class StackableDB : public DB { return db_->DisableFileDeletions(); } - virtual Status EnableFileDeletions() override { - return db_->EnableFileDeletions(); + virtual Status EnableFileDeletions(bool force) override { + return db_->EnableFileDeletions(force); } virtual Status GetLiveFiles(std::vector& vec, uint64_t* mfs, diff --git a/utilities/backupable/backupable_db_test.cc b/utilities/backupable/backupable_db_test.cc index af4af0d02..c64f0170b 100644 --- a/utilities/backupable/backupable_db_test.cc +++ b/utilities/backupable/backupable_db_test.cc @@ -48,7 +48,7 @@ class DummyDB : public StackableDB { return options_; } - virtual Status EnableFileDeletions() override { + virtual Status EnableFileDeletions(bool force) override { ASSERT_TRUE(!deletions_enabled_); deletions_enabled_ = true; return Status::OK();