Fix duplicated file names in PurgeObsoleteFiles (#5603)

Summary:
Currently in `DBImpl::PurgeObsoleteFiles`, the list of candidate files is create through a combination of calling LogFileName using `log_delete_files` and `full_scan_candidate_files`.

In full_scan_candidate_files, the filenames look like this
{file_name = "074715.log", file_path = "/txlogs/3306"},
but LogFileName produces filenames like this that prepends a slash:
{file_name = "/074715.log", file_path = "/txlogs/3306"},

This confuses the dedup step here: bb4178066d/db/db_impl/db_impl_files.cc (L339-L345)

Because duplicates still exist, DeleteFile is called on the same file twice, and hits an error on the second try. Error message: Failed to mark /txlogs/3302/764418.log as trash.

The root cause is the use of `kDumbDbName` when generating file names, it creates file names like /074715.log. This PR removes the use of `kDumbDbName` and create paths without leading '/' when dbname can be ignored.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/5603

Test Plan: make check

Differential Revision: D16413203

Pulled By: miasantreble

fbshipit-source-id: 6ba8288382c55f7d5e3892d722fc94b57d2e4491
main
Zhongyi Xie 5 years ago committed by Facebook Github Bot
parent 1dfc5eaab0
commit d1c9ede195
  1. 5
      db/db_impl/db_impl_files.cc
  2. 21
      file/filename.cc
  3. 4
      file/filename.h

@ -316,10 +316,9 @@ void DBImpl::PurgeObsoleteFiles(JobContext& state, bool schedule_only) {
candidate_files.size() + state.sst_delete_files.size() +
state.log_delete_files.size() + state.manifest_delete_files.size());
// We may ignore the dbname when generating the file names.
const char* kDumbDbName = "";
for (auto& file : state.sst_delete_files) {
candidate_files.emplace_back(
MakeTableFileName(kDumbDbName, file.metadata->fd.GetNumber()),
MakeTableFileName(file.metadata->fd.GetNumber()),
file.path);
if (file.metadata->table_reader_handle) {
table_cache_->Release(file.metadata->table_reader_handle);
@ -329,7 +328,7 @@ void DBImpl::PurgeObsoleteFiles(JobContext& state, bool schedule_only) {
for (auto file_num : state.log_delete_files) {
if (file_num > 0) {
candidate_files.emplace_back(LogFileName(kDumbDbName, file_num),
candidate_files.emplace_back(LogFileName(file_num),
immutable_db_options_.wal_dir);
}
}

@ -57,13 +57,17 @@ static size_t GetInfoLogPrefix(const std::string& path, char* dest, int len) {
return write_idx;
}
static std::string MakeFileName(const std::string& name, uint64_t number,
const char* suffix) {
static std::string MakeFileName(uint64_t number, const char* suffix) {
char buf[100];
snprintf(buf, sizeof(buf), "/%06llu.%s",
snprintf(buf, sizeof(buf), "%06llu.%s",
static_cast<unsigned long long>(number),
suffix);
return name + buf;
return buf;
}
static std::string MakeFileName(const std::string& name, uint64_t number,
const char* suffix) {
return name + "/" + MakeFileName(number, suffix);
}
std::string LogFileName(const std::string& name, uint64_t number) {
@ -71,6 +75,11 @@ std::string LogFileName(const std::string& name, uint64_t number) {
return MakeFileName(name, number, "log");
}
std::string LogFileName(uint64_t number) {
assert(number > 0);
return MakeFileName(number, "log");
}
std::string BlobFileName(const std::string& blobdirname, uint64_t number) {
assert(number > 0);
return MakeFileName(blobdirname, number, kRocksDBBlobFileExt.c_str());
@ -95,6 +104,10 @@ std::string MakeTableFileName(const std::string& path, uint64_t number) {
return MakeFileName(path, number, kRocksDbTFileExt.c_str());
}
std::string MakeTableFileName(uint64_t number) {
return MakeFileName(number, kRocksDbTFileExt.c_str());
}
std::string Rocks2LevelTableFileName(const std::string& fullname) {
assert(fullname.size() > kRocksDbTFileExt.size() + 1);
if (fullname.size() <= kRocksDbTFileExt.size() + 1) {

@ -47,6 +47,8 @@ enum FileType {
// "dbname".
extern std::string LogFileName(const std::string& dbname, uint64_t number);
extern std::string LogFileName(uint64_t number);
extern std::string BlobFileName(const std::string& bdirname, uint64_t number);
extern std::string BlobFileName(const std::string& dbname,
@ -63,6 +65,8 @@ extern std::string ArchivedLogFileName(const std::string& dbname,
extern std::string MakeTableFileName(const std::string& name, uint64_t number);
extern std::string MakeTableFileName(uint64_t number);
// Return the name of sstable with LevelDB suffix
// created from RocksDB sstable suffixed name
extern std::string Rocks2LevelTableFileName(const std::string& fullname);

Loading…
Cancel
Save