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

@@ -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();
 }

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(),

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<std::string>&,
                               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_;

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<std::string>&,

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<std::string>&, uint64_t* size,

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
 

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<std::string>& vec, uint64_t* mfs,

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();