Deleting Blob files also goes through SstFileManager (#4904)

Summary:
Right now, deleting blob files is not rate limited, even if SstFileManger is specified.
On the other hand, rate limiting blob deletion is not supported. With this change, Blob file
deletion will go through SstFileManager too.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/4904

Differential Revision: D13772545

Pulled By: siying

fbshipit-source-id: bd1b1d0beb26d5167385e00b7ecb8b94b879de84

diff --git a/HISTORY.md b/HISTORY.md
index 9040717fc6459f1cc2274433152a40698ca1c026..e81b3f30f4a1873ef92ffbd7bd8a908464793f53 100644 (file)
--- a/HISTORY.md
+++ b/HISTORY.md
@@ -2,6 +2,7 @@
 ## 5.18.2 (01/23/2019)
 ### Public API Change
 * Change time resolution in FileOperationInfo.
+* Deleting Blob files also go through SStFileManager.
 
 ## 5.18.0 (11/30/2018)
 ### New Features

diff --git a/util/sst_file_manager_impl.cc b/util/sst_file_manager_impl.cc
index ee1394bc91e3e2cfa06b1b3a13f90aaac5ec4450..0b46b24b1ffc246f2a11961fed22c11f4f1aceb8 100644 (file)
--- a/util/sst_file_manager_impl.cc
+++ b/util/sst_file_manager_impl.cc
@@ -403,6 +403,7 @@ bool SstFileManagerImpl::CancelErrorRecovery(ErrorHandler* handler) {
 
 Status SstFileManagerImpl::ScheduleFileDeletion(
     const std::string& file_path, const std::string& path_to_sync) {
+  TEST_SYNC_POINT("SstFileManagerImpl::ScheduleFileDeletion");
   return delete_scheduler_.DeleteFile(file_path, path_to_sync);
 }
 

diff --git a/utilities/blob_db/blob_db_impl.cc b/utilities/blob_db/blob_db_impl.cc
index bdec6546280b3b80843c59ab20219b66fc7354fa..a3a93365cd8dd389086b70bff0baff85ceb5ee24 100644 (file)
--- a/utilities/blob_db/blob_db_impl.cc
+++ b/utilities/blob_db/blob_db_impl.cc
@@ -26,6 +26,7 @@
 #include "util/cast_util.h"
 #include "util/crc32c.h"
 #include "util/file_reader_writer.h"
+#include "util/file_util.h"
 #include "util/filename.h"
 #include "util/logging.h"
 #include "util/mutexlock.h"
@@ -1745,7 +1746,8 @@ std::pair<bool, int64_t> BlobDBImpl::DeleteObsoleteFiles(bool aborted) {
                    bfile->PathName().c_str());
 
     blob_files_.erase(bfile->BlobFileNumber());
-    Status s = env_->DeleteFile(bfile->PathName());
+    Status s = DeleteSSTFile(&(db_impl_->immutable_db_options()),
+                             bfile->PathName(), blob_dir_);
     if (!s.ok()) {
       ROCKS_LOG_ERROR(db_options_.info_log,
                       "File failed to be deleted as obsolete %s",
@@ -1835,7 +1837,7 @@ Status DestroyBlobDB(const std::string& dbname, const Options& options,
     uint64_t number;
     FileType type;
     if (ParseFileName(f, &number, &type) && type == kBlobFile) {
-      Status del = env->DeleteFile(blobdir + "/" + f);
+      Status del = DeleteSSTFile(&soptions, blobdir + "/" + f, blobdir);
       if (status.ok() && !del.ok()) {
         status = del;
       }

diff --git a/utilities/blob_db/blob_db_test.cc b/utilities/blob_db/blob_db_test.cc
index 1c1867e4e2a46e7536133a7152fc0d9b470afa3b..d9cca123e968e8018ffac0be80824225487a2f5d 100644 (file)
--- a/utilities/blob_db/blob_db_test.cc
+++ b/utilities/blob_db/blob_db_test.cc
@@ -18,6 +18,7 @@
 #include "util/cast_util.h"
 #include "util/fault_injection_test_env.h"
 #include "util/random.h"
+#include "util/sst_file_manager_impl.h"
 #include "util/string_util.h"
 #include "util/sync_point.h"
 #include "util/testharness.h"
@@ -762,6 +763,52 @@ TEST_F(BlobDBTest, ReadWhileGC) {
   }
 }
 
+TEST_F(BlobDBTest, SstFileManager) {
+  // run the same test for Get(), MultiGet() and Iterator each.
+  std::shared_ptr<SstFileManager> sst_file_manager(
+      NewSstFileManager(mock_env_.get()));
+  sst_file_manager->SetDeleteRateBytesPerSecond(1);
+  SstFileManagerImpl *sfm =
+      static_cast<SstFileManagerImpl *>(sst_file_manager.get());
+
+  BlobDBOptions bdb_options;
+  bdb_options.min_blob_size = 0;
+  Options db_options;
+
+  int files_deleted_directly = 0;
+  int files_scheduled_to_delete = 0;
+  rocksdb::SyncPoint::GetInstance()->SetCallBack(
+      "SstFileManagerImpl::ScheduleFileDeletion",
+      [&](void * /*arg*/) { files_scheduled_to_delete++; });
+  rocksdb::SyncPoint::GetInstance()->SetCallBack(
+      "DeleteScheduler::DeleteFile",
+      [&](void * /*arg*/) { files_deleted_directly++; });
+  SyncPoint::GetInstance()->EnableProcessing();
+  db_options.sst_file_manager = sst_file_manager;
+
+  Open(bdb_options, db_options);
+
+  // Create one obselete file and clean it.
+  blob_db_->Put(WriteOptions(), "foo", "bar");
+  auto blob_files = blob_db_impl()->TEST_GetBlobFiles();
+  ASSERT_EQ(1, blob_files.size());
+  std::shared_ptr<BlobFile> bfile = blob_files[0];
+  ASSERT_OK(blob_db_impl()->TEST_CloseBlobFile(bfile));
+  GCStats gc_stats;
+  ASSERT_OK(blob_db_impl()->TEST_GCFileAndUpdateLSM(bfile, &gc_stats));
+  blob_db_impl()->TEST_DeleteObsoleteFiles();
+
+  // Even if SSTFileManager is not set, DB is creating a dummy one.
+  ASSERT_EQ(1, files_scheduled_to_delete);
+  ASSERT_EQ(0, files_deleted_directly);
+  Destroy();
+  // Make sure that DestroyBlobDB() also goes through delete scheduler.
+  ASSERT_GE(2, files_scheduled_to_delete);
+  ASSERT_EQ(0, files_deleted_directly);
+  SyncPoint::GetInstance()->DisableProcessing();
+  sfm->WaitForEmptyTrash();
+}
+
 TEST_F(BlobDBTest, SnapshotAndGarbageCollection) {
   BlobDBOptions bdb_options;
   bdb_options.min_blob_size = 0;