From 08b8cea69f9a2d60498306369be264d08b518b0d 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 51a75d6a2..b020e5d4e 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -12,6 +12,7 @@ * Remove geodb, spatial_db, document_db, json_document, date_tiered_db, and redis_lists. * With "ldb ----try_load_options", when wal_dir specified by the option file doesn't exist, ignore it. * Change time resolution in FileOperationInfo. +* Deleting Blob files also go through SStFileManager. ### Bug Fixes * Fix a deadlock caused by compaction and file ingestion waiting for each other in the event of write stalls. diff --git a/util/sst_file_manager_impl.cc b/util/sst_file_manager_impl.cc index ee1394bc9..0b46b24b1 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 c878ae2d5..19318760b 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" @@ -1742,7 +1743,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", @@ -1832,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 = 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 1c1867e4e..d9cca123e 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;