From 35c05bca0f5de46f9151dcf5db7dc5924dd581b4 Mon Sep 17 00:00:00 2001 From: Siying Dong Date: Tue, 22 Jan 2019 16:57:40 -0800 Subject: [PATCH] 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 --- HISTORY.md | 1 + util/sst_file_manager_impl.cc | 1 + utilities/blob_db/blob_db_impl.cc | 6 ++-- utilities/blob_db/blob_db_test.cc | 47 +++++++++++++++++++++++++++++++ 4 files changed, 53 insertions(+), 2 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 9040717f..e81b3f30 100644 --- 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 ee1394bc..0b46b24b 100644 --- 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 bdec6546..a3a93365 100644 --- 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 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 1c1867e4..d9cca123 100644 --- 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 sst_file_manager( + NewSstFileManager(mock_env_.get())); + sst_file_manager->SetDeleteRateBytesPerSecond(1); + SstFileManagerImpl *sfm = + static_cast(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 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; -- 2.47.3