From 56305221c4897f721712ba41c04e4569aa6a3356 Mon Sep 17 00:00:00 2001 From: Mayank Agarwal Date: Wed, 23 Oct 2013 23:39:23 -0700 Subject: [PATCH] Unify DeleteFile and DeleteWalFiles Summary: This is to simplify rocksdb public APIs and improve the code quality. Created an additional parameter to ParseFileName for log sub type and improved the code for deleting a wal file. Wrote exhaustive unit-tests in delete_file_test Unification of other redundant APIs can be taken up in a separate diff Test Plan: Expanded delete_file test Reviewers: dhruba, haobo, kailiu, sdong Reviewed By: dhruba CC: leveldb Differential Revision: https://reviews.facebook.net/D13647 --- db/db_filesnapshot.cc | 25 -------------------- db/db_impl.cc | 34 +++++++++++++++++---------- db/db_impl.h | 1 - db/db_test.cc | 2 +- db/deletefile_test.cc | 38 ++++++++++++++++++++++++++++++- db/filename.cc | 19 +++++++++++++++- db/filename.h | 4 +++- include/rocksdb/db.h | 13 ++++------- include/rocksdb/transaction_log.h | 2 ++ include/utilities/stackable_db.h | 4 ++-- utilities/ttl/db_ttl.cc | 4 ++-- utilities/ttl/db_ttl.h | 2 +- 12 files changed, 92 insertions(+), 56 deletions(-) diff --git a/db/db_filesnapshot.cc b/db/db_filesnapshot.cc index 41a55c106..dcbfebd7e 100644 --- a/db/db_filesnapshot.cc +++ b/db/db_filesnapshot.cc @@ -91,29 +91,4 @@ Status DBImpl::GetSortedWalFiles(VectorLogPtr& files) { return AppendSortedWalsOfType(options_.wal_dir, files, kAliveLogFile); } -Status DBImpl::DeleteWalFiles(const VectorLogPtr& files) { - Status s; - std::string archivedir = ArchivalDirectory(options_.wal_dir); - std::string files_not_deleted; - for (const auto& wal : files) { - /* Try deleting in the dir that pathname points to for the logfile. - This may fail if we try to delete a log file which was live when captured - but is archived now. Try deleting it from archive also - */ - Status st = env_->DeleteFile(options_.wal_dir + "/" + wal->PathName()); - if (!st.ok()) { - if (wal->Type() == kAliveLogFile && - env_->DeleteFile(LogFileName(archivedir, wal->LogNumber())).ok()) { - continue; - } - files_not_deleted.append(wal->PathName()); - } - } - if (!files_not_deleted.empty()) { - return Status::IOError("Deleted all requested files except: " + - files_not_deleted); - } - return Status::OK(); -} - } diff --git a/db/db_impl.cc b/db/db_impl.cc index 97970d6ef..2cca0fc00 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -3149,26 +3149,38 @@ inline void DBImpl::DelayLoggingAndReset() { Status DBImpl::DeleteFile(std::string name) { uint64_t number; FileType type; - if (!ParseFileName(name, &number, &type) || - (type != kTableFile)) { - Log(options_.info_log, - "DeleteFile #%ld FAILED. Invalid file name\n", - number); + WalFileType log_type; + if (!ParseFileName(name, &number, &type, &log_type) || + (type != kTableFile && type != kLogFile)) { + Log(options_.info_log, "DeleteFile %s failed.\n", name.c_str()); return Status::InvalidArgument("Invalid file name"); } + Status status; + if (type == kLogFile) { + // Only allow deleting archived log files + if (log_type != kArchivedLogFile) { + Log(options_.info_log, "DeleteFile %s failed.\n", name.c_str()); + return Status::NotSupported("Delete only supported for archived logs"); + } + status = env_->DeleteFile(options_.wal_dir + "/" + name.c_str()); + if (!status.ok()) { + Log(options_.info_log, "DeleteFile %s failed.\n", name.c_str()); + } + return status; + } + int level; FileMetaData metadata; int maxlevel = NumberLevels(); VersionEdit edit(maxlevel); DeletionState deletion_state; - Status status; { MutexLock l(&mutex_); status = versions_->GetMetadataForFile(number, &level, &metadata); if (!status.ok()) { - Log(options_.info_log, "DeleteFile #%lld FAILED. File not found\n", - static_cast(number)); + Log(options_.info_log, "DeleteFile %s failed. File not found\n", + name.c_str()); return Status::InvalidArgument("File not found"); } assert((level > 0) && (level < maxlevel)); @@ -3176,8 +3188,7 @@ Status DBImpl::DeleteFile(std::string name) { // If the file is being compacted no need to delete. if (metadata.being_compacted) { Log(options_.info_log, - "DeleteFile #%lld Skipped. File about to be compacted\n", - static_cast(number)); + "DeleteFile %s Skipped. File about to be compacted\n", name.c_str()); return Status::OK(); } @@ -3187,8 +3198,7 @@ Status DBImpl::DeleteFile(std::string name) { for (int i = level + 1; i < maxlevel; i++) { if (versions_->NumLevelFiles(i) != 0) { Log(options_.info_log, - "DeleteFile #%lld FAILED. File not in last level\n", - static_cast(number)); + "DeleteFile %s FAILED. File not in last level\n", name.c_str()); return Status::InvalidArgument("File not in last level"); } } diff --git a/db/db_impl.h b/db/db_impl.h index 7833066ba..e769d19f5 100644 --- a/db/db_impl.h +++ b/db/db_impl.h @@ -73,7 +73,6 @@ class DBImpl : public DB { uint64_t* manifest_file_size, bool flush_memtable = true); virtual Status GetSortedWalFiles(VectorLogPtr& files); - virtual Status DeleteWalFiles(const VectorLogPtr& files); virtual SequenceNumber GetLatestSequenceNumber(); virtual Status GetUpdatesSince(SequenceNumber seq_number, unique_ptr* iter); diff --git a/db/db_test.cc b/db/db_test.cc index 53a5347c8..6ee8f285c 100644 --- a/db/db_test.cc +++ b/db/db_test.cc @@ -4104,7 +4104,7 @@ class ModelDB: public DB { return Status::OK(); } - virtual Status DeleteWalFiles(const VectorLogPtr& files) { + virtual Status DeleteFile(std::string name) { return Status::OK(); } diff --git a/db/deletefile_test.cc b/db/deletefile_test.cc index 406b6b129..d169e4202 100644 --- a/db/deletefile_test.cc +++ b/db/deletefile_test.cc @@ -15,6 +15,7 @@ #include "util/testharness.h" #include "util/testutil.h" #include "rocksdb/env.h" +#include "rocksdb/transaction_log.h" #include #include #include @@ -36,6 +37,7 @@ class DeleteFileTest { options_.write_buffer_size = 1024*1024*1000; options_.target_file_size_base = 1024*1024*1000; options_.max_bytes_for_level_base = 1024*1024*1000; + options_.WAL_ttl_seconds = 300; // Used to test log files dbname_ = test::TmpDir() + "/deletefile_test"; DestroyDB(dbname_, options_); numlevels_ = 7; @@ -153,7 +155,6 @@ TEST(DeleteFileTest, AddKeysAndQueryLevels) { CloseDB(); } - TEST(DeleteFileTest, DeleteFileWithIterator) { CreateTwoLevels(); ReadOptions options; @@ -184,6 +185,41 @@ TEST(DeleteFileTest, DeleteFileWithIterator) { delete it; CloseDB(); } + +TEST(DeleteFileTest, DeleteLogFiles) { + AddKeys(10, 0); + VectorLogPtr logfiles; + db_->GetSortedWalFiles(logfiles); + ASSERT_GT(logfiles.size(), 0UL); + // Take the last log file which is expected to be alive and try to delete it + // Should not succeed because live logs are not allowed to be deleted + std::unique_ptr alive_log = std::move(logfiles.back()); + ASSERT_EQ(alive_log->Type(), kAliveLogFile); + ASSERT_TRUE(env_->FileExists(dbname_ + "/" + alive_log->PathName())); + fprintf(stdout, "Deleting alive log file %s\n", + alive_log->PathName().c_str()); + ASSERT_TRUE(!db_->DeleteFile(alive_log->PathName()).ok()); + ASSERT_TRUE(env_->FileExists(dbname_ + "/" + alive_log->PathName())); + logfiles.clear(); + + // Call Flush to bring about a new working log file and add more keys + // Call Flush again to flush out memtable and move alive log to archived log + // and try to delete the archived log file + FlushOptions fopts; + db_->Flush(fopts); + AddKeys(10, 0); + db_->Flush(fopts); + db_->GetSortedWalFiles(logfiles); + ASSERT_GT(logfiles.size(), 0UL); + std::unique_ptr archived_log = std::move(logfiles.front()); + ASSERT_EQ(archived_log->Type(), kArchivedLogFile); + ASSERT_TRUE(env_->FileExists(dbname_ + "/" + archived_log->PathName())); + fprintf(stdout, "Deleting archived log file %s\n", + archived_log->PathName().c_str()); + ASSERT_OK(db_->DeleteFile(archived_log->PathName())); + ASSERT_TRUE(!env_->FileExists(dbname_ + "/" + archived_log->PathName())); +} + } //namespace rocksdb int main(int argc, char** argv) { diff --git a/db/filename.cc b/db/filename.cc index 5c475ddba..cdbd1bc7a 100644 --- a/db/filename.cc +++ b/db/filename.cc @@ -143,7 +143,8 @@ std::string IdentityFileName(const std::string& dbname) { // Disregards / at the beginning bool ParseFileName(const std::string& fname, uint64_t* number, - FileType* type) { + FileType* type, + WalFileType* log_type) { Slice rest(fname); if (fname.length() > 1 && fname[0] == '/') { rest.remove_prefix(1); @@ -194,6 +195,17 @@ bool ParseFileName(const std::string& fname, } else { // Avoid strtoull() to keep filename format independent of the // current locale + bool archive_dir_found = false; + if (rest.starts_with(ARCHIVAL_DIR)) { + if (rest.size() <= ARCHIVAL_DIR.size()) { + return false; + } + rest.remove_prefix(ARCHIVAL_DIR.size() + 1); // Add 1 to remove / also + if (log_type) { + *log_type = kArchivedLogFile; + } + archive_dir_found = true; + } uint64_t num; if (!ConsumeDecimalNumber(&rest, &num)) { return false; @@ -201,6 +213,11 @@ bool ParseFileName(const std::string& fname, Slice suffix = rest; if (suffix == Slice(".log")) { *type = kLogFile; + if (log_type && !archive_dir_found) { + *log_type = kAliveLogFile; + } + } else if (archive_dir_found) { + return false; // Archive dir can contain only log files } else if (suffix == Slice(".sst")) { *type = kTableFile; } else if (suffix == Slice(".dbtmp")) { diff --git a/db/filename.h b/db/filename.h index 463be4094..8e55f1139 100644 --- a/db/filename.h +++ b/db/filename.h @@ -14,6 +14,7 @@ #include #include "rocksdb/slice.h" #include "rocksdb/status.h" +#include "rocksdb/transaction_log.h" #include "port/port.h" namespace rocksdb { @@ -93,7 +94,8 @@ extern std::string IdentityFileName(const std::string& dbname); // filename was successfully parsed, returns true. Else return false. extern bool ParseFileName(const std::string& filename, uint64_t* number, - FileType* type); + FileType* type, + WalFileType* log_type = nullptr); // Make the CURRENT file point to the descriptor file with the // specified number. diff --git a/include/rocksdb/db.h b/include/rocksdb/db.h index a9dd724e3..7d62ccd6c 100644 --- a/include/rocksdb/db.h +++ b/include/rocksdb/db.h @@ -257,10 +257,6 @@ class DB { // Retrieve the sorted list of all wal files with earliest file first virtual Status GetSortedWalFiles(VectorLogPtr& files) = 0; - // Delete wal files in files. These can be either live or archived. - // Returns Status::OK if all files could be deleted, otherwise Status::IOError - virtual Status DeleteWalFiles(const VectorLogPtr& files) = 0; - // The sequence number of the most recent transaction. virtual SequenceNumber GetLatestSequenceNumber() = 0; @@ -278,11 +274,10 @@ class DB { virtual Status GetUpdatesSince(SequenceNumber seq_number, unique_ptr* iter) = 0; - // Delete the file name from the db directory and update the internal - // state to reflect that. - virtual Status DeleteFile(std::string name) { - return Status::OK(); - } + // Delete the file name from the db directory and update the internal state to + // reflect that. Supports deletion of sst and log files only. 'name' must be + // path relative to the db directory. eg. 000001.sst, /archive/000003.log + virtual Status DeleteFile(std::string name) = 0; // Returns a list of all table files with their level, start key // and end key diff --git a/include/rocksdb/transaction_log.h b/include/rocksdb/transaction_log.h index a7553cea8..311b35176 100644 --- a/include/rocksdb/transaction_log.h +++ b/include/rocksdb/transaction_log.h @@ -5,6 +5,8 @@ #include "rocksdb/status.h" #include "rocksdb/types.h" #include "rocksdb/write_batch.h" +#include +#include namespace rocksdb { diff --git a/include/utilities/stackable_db.h b/include/utilities/stackable_db.h index 334935556..d867f2f95 100644 --- a/include/utilities/stackable_db.h +++ b/include/utilities/stackable_db.h @@ -144,8 +144,8 @@ class StackableDB : public DB { return sdb_->GetSortedWalFiles(files); } - virtual Status DeleteWalFiles(const VectorLogPtr& files) override { - return sdb_->DeleteWalFiles(files); + virtual Status DeleteFile(std::string name) override { + return sdb_->DeleteFile(name); } virtual Status GetUpdatesSince(SequenceNumber seq_number, diff --git a/utilities/ttl/db_ttl.cc b/utilities/ttl/db_ttl.cc index 37df18f0e..cac1d0146 100644 --- a/utilities/ttl/db_ttl.cc +++ b/utilities/ttl/db_ttl.cc @@ -280,8 +280,8 @@ Status DBWithTTL::GetSortedWalFiles(VectorLogPtr& files) { return db_->GetSortedWalFiles(files); } -Status DBWithTTL::DeleteWalFiles(const VectorLogPtr& files){ - return db_->DeleteWalFiles(files); +Status DBWithTTL::DeleteFile(std::string name) { + return db_->DeleteFile(name); } Status DBWithTTL::GetUpdatesSince( diff --git a/utilities/ttl/db_ttl.h b/utilities/ttl/db_ttl.h index e88cc34b3..e8332af5e 100644 --- a/utilities/ttl/db_ttl.h +++ b/utilities/ttl/db_ttl.h @@ -78,7 +78,7 @@ class DBWithTTL : public StackableDB { virtual Status GetSortedWalFiles(VectorLogPtr& files); - virtual Status DeleteWalFiles(const VectorLogPtr& files); + virtual Status DeleteFile(std::string name); virtual SequenceNumber GetLatestSequenceNumber(); -- GitLab