diff --git a/HISTORY.md b/HISTORY.md index 582cba0ff..351e0fc90 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -2,6 +2,7 @@ ## Unreleased ### Public API Change * `BackupableDBOptions::max_valid_backups_to_open == 0` now means no backups will be opened during BackupEngine initialization. Previously this condition disabled limiting backups opened. +* Deprecate trash_dir param in NewSstFileManager, right now we will rename deleted files to .trash instead of moving them to trash directory ### New Features * `DBOptions::bytes_per_sync` and `DBOptions::wal_bytes_per_sync` can now be changed dynamically, `DBOptions::wal_bytes_per_sync` will flush all memtables and switch to a new WAL file. diff --git a/db/db_impl_open.cc b/db/db_impl_open.cc index db8031bff..64854b441 100644 --- a/db/db_impl_open.cc +++ b/db/db_impl_open.cc @@ -124,6 +124,17 @@ DBOptions SanitizeOptions(const std::string& dbname, const DBOptions& src) { result.avoid_flush_during_recovery = false; } +#ifndef ROCKSDB_LITE + // When the DB is stopped, it's possible that there are some .trash files that + // were not deleted yet, when we open the DB we will find these .trash files + // and schedule them to be deleted (or delete immediately if SstFileManager + // was not used) + auto sfm = static_cast(result.sst_file_manager.get()); + for (size_t i = 0; i < result.db_paths.size(); i++) { + DeleteScheduler::CleanupDirectory(result.env, sfm, result.db_paths[i].path); + } +#endif + return result; } diff --git a/db/db_sst_test.cc b/db/db_sst_test.cc index 56e00df83..60726d152 100644 --- a/db/db_sst_test.cc +++ b/db/db_sst_test.cc @@ -327,11 +327,10 @@ TEST_F(DBSSTTest, RateLimitedDelete) { options.disable_auto_compactions = true; options.env = env_; - std::string trash_dir = test::TmpDir(env_) + "/trash"; int64_t rate_bytes_per_sec = 1024 * 10; // 10 Kbs / Sec Status s; options.sst_file_manager.reset( - NewSstFileManager(env_, nullptr, trash_dir, 0, false, &s)); + NewSstFileManager(env_, nullptr, "", 0, false, &s)); ASSERT_OK(s); options.sst_file_manager->SetDeleteRateBytesPerSecond(rate_bytes_per_sec); auto sfm = static_cast(options.sst_file_manager.get()); @@ -394,11 +393,10 @@ TEST_F(DBSSTTest, DeleteSchedulerMultipleDBPaths) { options.db_paths.emplace_back(dbname_ + "_2", 1024 * 100); options.env = env_; - std::string trash_dir = test::TmpDir(env_) + "/trash"; int64_t rate_bytes_per_sec = 1024 * 1024; // 1 Mb / Sec Status s; - options.sst_file_manager.reset(NewSstFileManager( - env_, nullptr, trash_dir, rate_bytes_per_sec, false, &s)); + options.sst_file_manager.reset( + NewSstFileManager(env_, nullptr, "", rate_bytes_per_sec, false, &s)); ASSERT_OK(s); auto sfm = static_cast(options.sst_file_manager.get()); sfm->delete_scheduler()->TEST_SetMaxTrashDBRatio(1.1); @@ -460,9 +458,8 @@ TEST_F(DBSSTTest, DestroyDBWithRateLimitedDelete) { Options options = CurrentOptions(); options.disable_auto_compactions = true; options.env = env_; - std::string trash_dir = test::TmpDir(env_) + "/trash"; options.sst_file_manager.reset( - NewSstFileManager(env_, nullptr, trash_dir, 0, false, &s)); + NewSstFileManager(env_, nullptr, "", 0, false, &s)); ASSERT_OK(s); DestroyAndReopen(options); diff --git a/include/rocksdb/sst_file_manager.h b/include/rocksdb/sst_file_manager.h index 692007d31..7dc22deed 100644 --- a/include/rocksdb/sst_file_manager.h +++ b/include/rocksdb/sst_file_manager.h @@ -8,6 +8,7 @@ #include #include #include +#include #include "rocksdb/status.h" @@ -19,6 +20,7 @@ class Logger; // SstFileManager is used to track SST files in the DB and control there // deletion rate. // All SstFileManager public functions are thread-safe. +// SstFileManager is not extensible. class SstFileManager { public: virtual ~SstFileManager() {} @@ -64,17 +66,13 @@ class SstFileManager { // @param info_log: If not nullptr, info_log will be used to log errors. // // == Deletion rate limiting specific arguments == -// @param trash_dir: Path to the directory where deleted files will be moved -// to be deleted in a background thread while applying rate limiting. If this -// directory doesn't exist, it will be created. This directory should not be -// used by any other process or any other SstFileManager, Set to "" to -// disable deletion rate limiting. +// @param trash_dir: Deprecated, this argument have no effect // @param rate_bytes_per_sec: How many bytes should be deleted per second, If // this value is set to 1024 (1 Kb / sec) and we deleted a file of size 4 Kb // in 1 second, we will wait for another 3 seconds before we delete other // files, Set to 0 to disable deletion rate limiting. -// @param delete_existing_trash: If set to true, the newly created -// SstFileManager will delete files that already exist in trash_dir. +// @param delete_existing_trash: Deprecated, this argument have no effect, but +// if user provide trash_dir we will schedule deletes for files in the dir // @param status: If not nullptr, status will contain any errors that happened // during creating the missing trash_dir or deleting existing files in trash. extern SstFileManager* NewSstFileManager( diff --git a/util/delete_scheduler.cc b/util/delete_scheduler.cc index 93fc16697..12b267f3c 100644 --- a/util/delete_scheduler.cc +++ b/util/delete_scheduler.cc @@ -19,11 +19,10 @@ namespace rocksdb { -DeleteScheduler::DeleteScheduler(Env* env, const std::string& trash_dir, - int64_t rate_bytes_per_sec, Logger* info_log, +DeleteScheduler::DeleteScheduler(Env* env, int64_t rate_bytes_per_sec, + Logger* info_log, SstFileManagerImpl* sst_file_manager) : env_(env), - trash_dir_(trash_dir), total_trash_size_(0), rate_bytes_per_sec_(rate_bytes_per_sec), pending_files_(0), @@ -63,11 +62,11 @@ Status DeleteScheduler::DeleteFile(const std::string& file_path) { } // Move file to trash - std::string path_in_trash; - s = MoveToTrash(file_path, &path_in_trash); + std::string trash_file; + s = MarkAsTrash(file_path, &trash_file); + if (!s.ok()) { - ROCKS_LOG_ERROR(info_log_, "Failed to move %s to trash directory (%s)", - file_path.c_str(), trash_dir_.c_str()); + ROCKS_LOG_ERROR(info_log_, "Failed to mark %s as trash", file_path.c_str()); s = env_->DeleteFile(file_path); if (s.ok()) { sst_file_manager_->OnDeleteFile(file_path); @@ -75,10 +74,15 @@ Status DeleteScheduler::DeleteFile(const std::string& file_path) { return s; } + // Update the total trash size + uint64_t trash_file_size = 0; + env_->GetFileSize(trash_file, &trash_file_size); + total_trash_size_.fetch_add(trash_file_size); + // Add file to delete queue { InstrumentedMutexLock l(&mu_); - queue_.push(path_in_trash); + queue_.push(trash_file); pending_files_++; if (pending_files_ == 1) { cv_.SignalAll(); @@ -92,44 +96,83 @@ std::map DeleteScheduler::GetBackgroundErrors() { return bg_errors_; } -Status DeleteScheduler::MoveToTrash(const std::string& file_path, - std::string* path_in_trash) { +const std::string DeleteScheduler::kTrashExtension = ".trash"; +bool DeleteScheduler::IsTrashFile(const std::string& file_path) { + return (file_path.size() >= kTrashExtension.size() && + file_path.rfind(kTrashExtension) == + file_path.size() - kTrashExtension.size()); +} + +Status DeleteScheduler::CleanupDirectory(Env* env, SstFileManagerImpl* sfm, + const std::string& path) { Status s; - // Figure out the name of the file in trash folder + // Check if there are any files marked as trash in this path + std::vector files_in_path; + s = env->GetChildren(path, &files_in_path); + if (!s.ok()) { + return s; + } + for (const std::string& current_file : files_in_path) { + if (!DeleteScheduler::IsTrashFile(current_file)) { + // not a trash file, skip + continue; + } + + Status file_delete; + std::string trash_file = path + "/" + current_file; + if (sfm) { + // We have an SstFileManager that will schedule the file delete + sfm->OnAddFile(trash_file); + file_delete = sfm->ScheduleFileDeletion(trash_file); + } else { + // Delete the file immediately + file_delete = env->DeleteFile(trash_file); + } + + if (s.ok() && !file_delete.ok()) { + s = file_delete; + } + } + + return s; +} + +Status DeleteScheduler::MarkAsTrash(const std::string& file_path, + std::string* trash_file) { + // Sanity check of the path size_t idx = file_path.rfind("/"); if (idx == std::string::npos || idx == file_path.size() - 1) { return Status::InvalidArgument("file_path is corrupted"); } - *path_in_trash = trash_dir_ + file_path.substr(idx); - std::string unique_suffix = ""; - if (*path_in_trash == file_path) { - // This file is already in trash + Status s; + if (DeleteScheduler::IsTrashFile(file_path)) { + // This is already a trash file return s; } + *trash_file = file_path + kTrashExtension; // TODO(tec) : Implement Env::RenameFileIfNotExist and remove // file_move_mu mutex. + int cnt = 0; InstrumentedMutexLock l(&file_move_mu_); while (true) { - s = env_->FileExists(*path_in_trash + unique_suffix); + s = env_->FileExists(*trash_file); if (s.IsNotFound()) { // We found a path for our file in trash - *path_in_trash += unique_suffix; - s = env_->RenameFile(file_path, *path_in_trash); + s = env_->RenameFile(file_path, *trash_file); break; } else if (s.ok()) { // Name conflict, generate new random suffix - unique_suffix = env_->GenerateUniqueId(); + *trash_file = file_path + std::to_string(cnt) + kTrashExtension; } else { // Error during FileExists call, we cannot continue break; } + cnt++; } if (s.ok()) { - uint64_t trash_file_size = 0; - sst_file_manager_->OnMoveFile(file_path, *path_in_trash, &trash_file_size); - total_trash_size_.fetch_add(trash_file_size); + sst_file_manager_->OnMoveFile(file_path, *trash_file); } return s; } diff --git a/util/delete_scheduler.h b/util/delete_scheduler.h index 4c07ed67c..1e8bcb0d7 100644 --- a/util/delete_scheduler.h +++ b/util/delete_scheduler.h @@ -24,7 +24,7 @@ class Logger; class SstFileManagerImpl; // DeleteScheduler allows the DB to enforce a rate limit on file deletion, -// Instead of deleteing files immediately, files are moved to trash_dir +// Instead of deleteing files immediately, files are marked as trash // and deleted in a background thread that apply sleep penlty between deletes // if they are happening in a rate faster than rate_bytes_per_sec, // @@ -32,8 +32,7 @@ class SstFileManagerImpl; // case DeleteScheduler will delete files immediately. class DeleteScheduler { public: - DeleteScheduler(Env* env, const std::string& trash_dir, - int64_t rate_bytes_per_sec, Logger* info_log, + DeleteScheduler(Env* env, int64_t rate_bytes_per_sec, Logger* info_log, SstFileManagerImpl* sst_file_manager); ~DeleteScheduler(); @@ -46,7 +45,7 @@ class DeleteScheduler { return rate_bytes_per_sec_.store(bytes_per_sec); } - // Move file to trash directory and schedule it's deletion + // Mark file as trash directory and schedule it's deletion Status DeleteFile(const std::string& fname); // Wait for all files being deleteing in the background to finish or for @@ -64,8 +63,16 @@ class DeleteScheduler { max_trash_db_ratio_ = r; } + static const std::string kTrashExtension; + static bool IsTrashFile(const std::string& file_path); + + // Check if there are any .trash filse in path, and schedule their deletion + // Or delete immediately if sst_file_manager is nullptr + static Status CleanupDirectory(Env* env, SstFileManagerImpl* sfm, + const std::string& path); + private: - Status MoveToTrash(const std::string& file_path, std::string* path_in_trash); + Status MarkAsTrash(const std::string& file_path, std::string* path_in_trash); Status DeleteTrashFile(const std::string& path_in_trash, uint64_t* deleted_bytes); @@ -73,17 +80,15 @@ class DeleteScheduler { void BackgroundEmptyTrash(); Env* env_; - // Path to the trash directory - std::string trash_dir_; - // total size of trash directory + // total size of trash files std::atomic total_trash_size_; // Maximum number of bytes that should be deleted per second std::atomic rate_bytes_per_sec_; // Mutex to protect queue_, pending_files_, bg_errors_, closing_ InstrumentedMutex mu_; - // Queue of files in trash that need to be deleted + // Queue of trash files that need to be deleted std::queue queue_; - // Number of files in trash that are waiting to be deleted + // Number of trash files that are waiting to be deleted int32_t pending_files_; // Errors that happened in BackgroundEmptyTrash (file_path => error) std::map bg_errors_; diff --git a/util/delete_scheduler_test.cc b/util/delete_scheduler_test.cc index 208bdd741..c64076bec 100644 --- a/util/delete_scheduler_test.cc +++ b/util/delete_scheduler_test.cc @@ -30,8 +30,6 @@ class DeleteSchedulerTest : public testing::Test { DeleteSchedulerTest() : env_(Env::Default()) { dummy_files_dir_ = test::TmpDir(env_) + "/delete_scheduler_dummy_data_dir"; DestroyAndCreateDir(dummy_files_dir_); - trash_dir_ = test::TmpDir(env_) + "/delete_scheduler_trash"; - DestroyAndCreateDir(trash_dir_); } ~DeleteSchedulerTest() { @@ -46,11 +44,31 @@ class DeleteSchedulerTest : public testing::Test { EXPECT_OK(env_->CreateDir(dir)); } - int CountFilesInDir(const std::string& dir) { + int CountNormalFiles() { std::vector files_in_dir; - EXPECT_OK(env_->GetChildren(dir, &files_in_dir)); - // Ignore "." and ".." - return static_cast(files_in_dir.size()) - 2; + EXPECT_OK(env_->GetChildren(dummy_files_dir_, &files_in_dir)); + + int normal_cnt = 0; + for (auto& f : files_in_dir) { + if (!DeleteScheduler::IsTrashFile(f) && f != "." && f != "..") { + printf("%s\n", f.c_str()); + normal_cnt++; + } + } + return normal_cnt; + } + + int CountTrashFiles() { + std::vector files_in_dir; + EXPECT_OK(env_->GetChildren(dummy_files_dir_, &files_in_dir)); + + int trash_cnt = 0; + for (auto& f : files_in_dir) { + if (DeleteScheduler::IsTrashFile(f)) { + trash_cnt++; + } + } + return trash_cnt; } std::string NewDummyFile(const std::string& file_name, uint64_t size = 1024) { @@ -65,9 +83,8 @@ class DeleteSchedulerTest : public testing::Test { } void NewDeleteScheduler() { - ASSERT_OK(env_->CreateDirIfMissing(trash_dir_)); sst_file_mgr_.reset( - new SstFileManagerImpl(env_, nullptr, trash_dir_, rate_bytes_per_sec_)); + new SstFileManagerImpl(env_, nullptr, rate_bytes_per_sec_)); delete_scheduler_ = sst_file_mgr_->delete_scheduler(); // Tests in this file are for DeleteScheduler component and dont create any // DBs, so we need to use set this value to 100% (instead of default 25%) @@ -76,7 +93,6 @@ class DeleteSchedulerTest : public testing::Test { Env* env_; std::string dummy_files_dir_; - std::string trash_dir_; int64_t rate_bytes_per_sec_; DeleteScheduler* delete_scheduler_; std::unique_ptr sst_file_mgr_; @@ -124,7 +140,7 @@ TEST_F(DeleteSchedulerTest, BasicRateLimiting) { for (int i = 0; i < num_files; i++) { ASSERT_OK(delete_scheduler_->DeleteFile(generated_files[i])); } - ASSERT_EQ(CountFilesInDir(dummy_files_dir_), 0); + ASSERT_EQ(CountNormalFiles(), 0); uint64_t delete_start_time = env_->NowMicros(); TEST_SYNC_POINT("DeleteSchedulerTest::BasicRateLimiting:1"); @@ -144,7 +160,7 @@ TEST_F(DeleteSchedulerTest, BasicRateLimiting) { } ASSERT_GT(time_spent_deleting, expected_penlty * 0.9); - ASSERT_EQ(CountFilesInDir(trash_dir_), 0); + ASSERT_EQ(CountTrashFiles(), 0); rocksdb::SyncPoint::GetInstance()->DisableProcessing(); } } @@ -226,8 +242,8 @@ TEST_F(DeleteSchedulerTest, RateLimitingMultiThreaded) { } ASSERT_GT(time_spent_deleting, expected_penlty * 0.9); - ASSERT_EQ(CountFilesInDir(dummy_files_dir_), 0); - ASSERT_EQ(CountFilesInDir(trash_dir_), 0); + ASSERT_EQ(CountNormalFiles(), 0); + ASSERT_EQ(CountTrashFiles(), 0); rocksdb::SyncPoint::GetInstance()->DisableProcessing(); } } @@ -251,8 +267,8 @@ TEST_F(DeleteSchedulerTest, DisableRateLimiting) { std::string dummy_file = NewDummyFile("dummy.data"); ASSERT_OK(delete_scheduler_->DeleteFile(dummy_file)); ASSERT_TRUE(env_->FileExists(dummy_file).IsNotFound()); - ASSERT_EQ(CountFilesInDir(dummy_files_dir_), 0); - ASSERT_EQ(CountFilesInDir(trash_dir_), 0); + ASSERT_EQ(CountNormalFiles(), 0); + ASSERT_EQ(CountTrashFiles(), 0); } ASSERT_EQ(bg_delete_file, 0); @@ -281,14 +297,14 @@ TEST_F(DeleteSchedulerTest, ConflictNames) { std::string dummy_file = NewDummyFile("conflict.data"); ASSERT_OK(delete_scheduler_->DeleteFile(dummy_file)); } - ASSERT_EQ(CountFilesInDir(dummy_files_dir_), 0); + ASSERT_EQ(CountNormalFiles(), 0); // 10 files ("conflict.data" x 10) in trash - ASSERT_EQ(CountFilesInDir(trash_dir_), 10); + ASSERT_EQ(CountTrashFiles(), 10); // Hold BackgroundEmptyTrash TEST_SYNC_POINT("DeleteSchedulerTest::ConflictNames:1"); delete_scheduler_->WaitForEmptyTrash(); - ASSERT_EQ(CountFilesInDir(trash_dir_), 0); + ASSERT_EQ(CountTrashFiles(), 0); auto bg_errors = delete_scheduler_->GetBackgroundErrors(); ASSERT_EQ(bg_errors.size(), 0); @@ -317,15 +333,15 @@ TEST_F(DeleteSchedulerTest, BackgroundError) { std::string file_name = "data_" + ToString(i) + ".data"; ASSERT_OK(delete_scheduler_->DeleteFile(NewDummyFile(file_name))); } - ASSERT_EQ(CountFilesInDir(dummy_files_dir_), 0); - ASSERT_EQ(CountFilesInDir(trash_dir_), 10); + ASSERT_EQ(CountNormalFiles(), 0); + ASSERT_EQ(CountTrashFiles(), 10); // Delete 10 files from trash, this will cause background errors in // BackgroundEmptyTrash since we already deleted the files it was // goind to delete for (int i = 0; i < 10; i++) { - std::string file_name = "data_" + ToString(i) + ".data"; - ASSERT_OK(env_->DeleteFile(trash_dir_ + "/" + file_name)); + std::string file_name = "data_" + ToString(i) + ".data.trash"; + ASSERT_OK(env_->DeleteFile(dummy_files_dir_ + "/" + file_name)); } // Hold BackgroundEmptyTrash @@ -359,10 +375,10 @@ TEST_F(DeleteSchedulerTest, StartBGEmptyTrashMultipleTimes) { std::string file_name = "data_" + ToString(i) + ".data"; ASSERT_OK(delete_scheduler_->DeleteFile(NewDummyFile(file_name))); } - ASSERT_EQ(CountFilesInDir(dummy_files_dir_), 0); + ASSERT_EQ(CountNormalFiles(), 0); delete_scheduler_->WaitForEmptyTrash(); ASSERT_EQ(bg_delete_file, 10 * run); - ASSERT_EQ(CountFilesInDir(trash_dir_), 0); + ASSERT_EQ(CountTrashFiles(), 0); auto bg_errors = delete_scheduler_->GetBackgroundErrors(); ASSERT_EQ(bg_errors.size(), 0); @@ -397,35 +413,7 @@ TEST_F(DeleteSchedulerTest, DestructorWithNonEmptyQueue) { sst_file_mgr_.reset(); ASSERT_LT(bg_delete_file, 100); - ASSERT_GT(CountFilesInDir(trash_dir_), 0); - - rocksdb::SyncPoint::GetInstance()->DisableProcessing(); -} - -// 1- Delete the trash directory -// 2- Delete 10 files using DeleteScheduler -// 3- Make sure that the 10 files were deleted immediately since DeleteScheduler -// failed to move them to trash directory -TEST_F(DeleteSchedulerTest, MoveToTrashError) { - int bg_delete_file = 0; - rocksdb::SyncPoint::GetInstance()->SetCallBack( - "DeleteScheduler::DeleteTrashFile:DeleteFile", - [&](void* arg) { bg_delete_file++; }); - rocksdb::SyncPoint::GetInstance()->EnableProcessing(); - - rate_bytes_per_sec_ = 1024; // 1 Kb / sec - NewDeleteScheduler(); - - // We will delete the trash directory, that mean that DeleteScheduler wont - // be able to move files to trash and will delete files them immediately. - ASSERT_OK(test::DestroyDir(env_, trash_dir_)); - for (int i = 0; i < 10; i++) { - std::string file_name = "data_" + ToString(i) + ".data"; - ASSERT_OK(delete_scheduler_->DeleteFile(NewDummyFile(file_name))); - } - - ASSERT_EQ(CountFilesInDir(dummy_files_dir_), 0); - ASSERT_EQ(bg_delete_file, 0); + ASSERT_GT(CountTrashFiles(), 0); rocksdb::SyncPoint::GetInstance()->DisableProcessing(); } @@ -480,7 +468,7 @@ TEST_F(DeleteSchedulerTest, DISABLED_DynamicRateLimiting1) { for (int i = 0; i < num_files; i++) { ASSERT_OK(delete_scheduler_->DeleteFile(generated_files[i])); } - ASSERT_EQ(CountFilesInDir(dummy_files_dir_), 0); + ASSERT_EQ(CountNormalFiles(), 0); if (rate_bytes_per_sec_ > 0) { uint64_t delete_start_time = env_->NowMicros(); @@ -508,7 +496,7 @@ TEST_F(DeleteSchedulerTest, DISABLED_DynamicRateLimiting1) { ASSERT_EQ(fg_delete_file, num_files); } - ASSERT_EQ(CountFilesInDir(trash_dir_), 0); + ASSERT_EQ(CountTrashFiles(), 0); rocksdb::SyncPoint::GetInstance()->DisableProcessing(); } } @@ -548,6 +536,27 @@ TEST_F(DeleteSchedulerTest, ImmediateDeleteOn25PercDBSize) { rocksdb::SyncPoint::GetInstance()->DisableProcessing(); } +TEST_F(DeleteSchedulerTest, IsTrashCheck) { + // Trash files + ASSERT_TRUE(DeleteScheduler::IsTrashFile("x.trash")); + ASSERT_TRUE(DeleteScheduler::IsTrashFile(".trash")); + ASSERT_TRUE(DeleteScheduler::IsTrashFile("abc.sst.trash")); + ASSERT_TRUE(DeleteScheduler::IsTrashFile("/a/b/c/abc..sst.trash")); + ASSERT_TRUE(DeleteScheduler::IsTrashFile("log.trash")); + ASSERT_TRUE(DeleteScheduler::IsTrashFile("^^^^^.log.trash")); + ASSERT_TRUE(DeleteScheduler::IsTrashFile("abc.t.trash")); + + // Not trash files + ASSERT_FALSE(DeleteScheduler::IsTrashFile("abc.sst")); + ASSERT_FALSE(DeleteScheduler::IsTrashFile("abc.txt")); + ASSERT_FALSE(DeleteScheduler::IsTrashFile("/a/b/c/abc.sst")); + ASSERT_FALSE(DeleteScheduler::IsTrashFile("/a/b/c/abc.sstrash")); + ASSERT_FALSE(DeleteScheduler::IsTrashFile("^^^^^.trashh")); + ASSERT_FALSE(DeleteScheduler::IsTrashFile("abc.ttrash")); + ASSERT_FALSE(DeleteScheduler::IsTrashFile(".ttrash")); + ASSERT_FALSE(DeleteScheduler::IsTrashFile("abc.trashx")); +} + } // namespace rocksdb int main(int argc, char** argv) { diff --git a/util/sst_file_manager_impl.cc b/util/sst_file_manager_impl.cc index 511df32ac..92ab37c0c 100644 --- a/util/sst_file_manager_impl.cc +++ b/util/sst_file_manager_impl.cc @@ -17,14 +17,12 @@ namespace rocksdb { #ifndef ROCKSDB_LITE SstFileManagerImpl::SstFileManagerImpl(Env* env, std::shared_ptr logger, - const std::string& trash_dir, int64_t rate_bytes_per_sec) : env_(env), logger_(logger), total_files_size_(0), max_allowed_space_(0), - delete_scheduler_(env, trash_dir, rate_bytes_per_sec, logger.get(), - this) {} + delete_scheduler_(env, rate_bytes_per_sec, logger.get(), this) {} SstFileManagerImpl::~SstFileManagerImpl() {} @@ -132,26 +130,25 @@ SstFileManager* NewSstFileManager(Env* env, std::shared_ptr info_log, int64_t rate_bytes_per_sec, bool delete_existing_trash, Status* status) { SstFileManagerImpl* res = - new SstFileManagerImpl(env, info_log, trash_dir, rate_bytes_per_sec); + new SstFileManagerImpl(env, info_log, rate_bytes_per_sec); + // trash_dir is deprecated and not needed anymore, but if user passed it + // we will still remove files in it. Status s; - if (trash_dir != "") { - s = env->CreateDirIfMissing(trash_dir); - if (s.ok() && delete_existing_trash) { - std::vector files_in_trash; - s = env->GetChildren(trash_dir, &files_in_trash); - if (s.ok()) { - for (const std::string& trash_file : files_in_trash) { - if (trash_file == "." || trash_file == "..") { - continue; - } - - std::string path_in_trash = trash_dir + "/" + trash_file; - res->OnAddFile(path_in_trash); - Status file_delete = res->ScheduleFileDeletion(path_in_trash); - if (s.ok() && !file_delete.ok()) { - s = file_delete; - } + if (delete_existing_trash && trash_dir != "") { + std::vector files_in_trash; + s = env->GetChildren(trash_dir, &files_in_trash); + if (s.ok()) { + for (const std::string& trash_file : files_in_trash) { + if (trash_file == "." || trash_file == "..") { + continue; + } + + std::string path_in_trash = trash_dir + "/" + trash_file; + res->OnAddFile(path_in_trash); + Status file_delete = res->ScheduleFileDeletion(path_in_trash); + if (s.ok() && !file_delete.ok()) { + s = file_delete; } } } diff --git a/util/sst_file_manager_impl.h b/util/sst_file_manager_impl.h index b737bf768..0043bb5d1 100644 --- a/util/sst_file_manager_impl.h +++ b/util/sst_file_manager_impl.h @@ -25,7 +25,6 @@ class Logger; class SstFileManagerImpl : public SstFileManager { public: explicit SstFileManagerImpl(Env* env, std::shared_ptr logger, - const std::string& trash_dir, int64_t rate_bytes_per_sec); ~SstFileManagerImpl(); @@ -68,7 +67,7 @@ class SstFileManagerImpl : public SstFileManager { // Update the delete rate limit in bytes per second. virtual void SetDeleteRateBytesPerSecond(int64_t delete_rate) override; - // Move file to trash directory and schedule it's deletion. + // Mark file as trash and schedule it's deletion. virtual Status ScheduleFileDeletion(const std::string& file_path); // Wait for all files being deleteing in the background to finish or for @@ -94,9 +93,7 @@ class SstFileManagerImpl : public SstFileManager { std::unordered_map tracked_files_; // The maximum allowed space (in bytes) for sst files. uint64_t max_allowed_space_; - // DeleteScheduler used to throttle file deletition, if SstFileManagerImpl was - // created with rate_bytes_per_sec == 0 or trash_dir == "", delete_scheduler_ - // rate limiting will be disabled and will simply delete the files. + // DeleteScheduler used to throttle file deletition. DeleteScheduler delete_scheduler_; };