Skip to content

Commit

Permalink
BackupEngine gluster-friendly file naming convention
Browse files Browse the repository at this point in the history 
Summary:
Use the rsync tempfile naming convention in our `BackupEngine`. The temp file follows the format, `.<filename>.<suffix>`, which is later renamed to `<filename>`. We fix `tmp` as the `<suffix>` as we don't need to use random bytes for now. The benefit is gluster treats this tempfile naming convention specially and applies hashing only to `<filename>`, so the file won't need to be linked or moved when it's renamed. Our gluster team suggested this will make things operationally easier.
Closes facebook#3463

Differential Revision: D6893333

Pulled By: ajkr

fbshipit-source-id: fd7622978f4b2487fce33cde40dd3124f16bcaa8
  • Loading branch information
@ajkr @facebook-github-bot
ajkr authored and facebook-github-bot committed Feb 22, 2018
1 parent 828211e commit b092977
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 23 deletions.
47 changes: 29 additions & 18 deletions utilities/backupable/backupable_db.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,17 @@ class BackupEngineImpl : public BackupEngine {

class BackupMeta {
public:
BackupMeta(const std::string& meta_filename,
BackupMeta(
const std::string& meta_filename, const std::string& meta_tmp_filename,
std::unordered_map<std::string, std::shared_ptr<FileInfo>>* file_infos,
Env* env)
: timestamp_(0), sequence_number_(0), size_(0),
meta_filename_(meta_filename), file_infos_(file_infos), env_(env) {}
: timestamp_(0),
sequence_number_(0),
size_(0),
meta_filename_(meta_filename),
meta_tmp_filename_(meta_tmp_filename),
file_infos_(file_infos),
env_(env) {}

BackupMeta(const BackupMeta&) = delete;
BackupMeta& operator=(const BackupMeta&) = delete;
Expand Down Expand Up @@ -228,6 +234,7 @@ class BackupEngineImpl : public BackupEngine {
uint64_t size_;
std::string app_metadata_;
std::string const meta_filename_;
std::string const meta_tmp_filename_;
// files with relative paths (without "/" prefix!!)
std::vector<std::shared_ptr<FileInfo>> files_;
std::unordered_map<std::string, std::shared_ptr<FileInfo>>* file_infos_;
Expand Down Expand Up @@ -257,12 +264,14 @@ class BackupEngineImpl : public BackupEngine {
inline std::string GetSharedFileRel(const std::string& file = "",
bool tmp = false) const {
assert(file.size() == 0 || file[0] != '/');
return "shared/" + file + (tmp ? ".tmp" : "");
return std::string("shared/") + (tmp ? "." : "") + file +
(tmp ? ".tmp" : "");
}
inline std::string GetSharedFileWithChecksumRel(const std::string& file = "",
bool tmp = false) const {
assert(file.size() == 0 || file[0] != '/');
return GetSharedChecksumDirRel() + "/" + file + (tmp ? ".tmp" : "");
return GetSharedChecksumDirRel() + "/" + (tmp ? "." : "") + file +
(tmp ? ".tmp" : "");
}
inline std::string GetSharedFileWithChecksum(const std::string& file,
const uint32_t checksum_value,
Expand All @@ -283,8 +292,9 @@ class BackupEngineImpl : public BackupEngine {
inline std::string GetBackupMetaDir() const {
return GetAbsolutePath("meta");
}
inline std::string GetBackupMetaFile(BackupID backup_id) const {
return GetBackupMetaDir() + "/" + rocksdb::ToString(backup_id);
inline std::string GetBackupMetaFile(BackupID backup_id, bool tmp) const {
return GetBackupMetaDir() + "/" + (tmp ? "." : "") +
rocksdb::ToString(backup_id) + (tmp ? ".tmp" : "");
}

// If size_limit == 0, there is no size limit, copy everything.
Expand Down Expand Up @@ -605,10 +615,11 @@ Status BackupEngineImpl::Initialize() {
continue;
}
assert(backups_.find(backup_id) == backups_.end());
backups_.insert(
std::make_pair(backup_id, unique_ptr<BackupMeta>(new BackupMeta(
GetBackupMetaFile(backup_id),
&backuped_file_infos_, backup_env_))));
backups_.insert(std::make_pair(
backup_id, unique_ptr<BackupMeta>(new BackupMeta(
GetBackupMetaFile(backup_id, false /* tmp */),
GetBackupMetaFile(backup_id, true /* tmp */),
&backuped_file_infos_, backup_env_))));
}

latest_backup_id_ = 0;
Expand Down Expand Up @@ -736,10 +747,11 @@ Status BackupEngineImpl::CreateNewBackupWithMetadata(
BackupID new_backup_id = latest_backup_id_ + 1;

assert(backups_.find(new_backup_id) == backups_.end());
auto ret = backups_.insert(
std::make_pair(new_backup_id, unique_ptr<BackupMeta>(new BackupMeta(
GetBackupMetaFile(new_backup_id),
&backuped_file_infos_, backup_env_))));
auto ret = backups_.insert(std::make_pair(
new_backup_id, unique_ptr<BackupMeta>(new BackupMeta(
GetBackupMetaFile(new_backup_id, false /* tmp */),
GetBackupMetaFile(new_backup_id, true /* tmp */),
&backuped_file_infos_, backup_env_))));
assert(ret.second == true);
auto& new_backup = ret.first->second;
new_backup->RecordTimestamp();
Expand Down Expand Up @@ -1708,8 +1720,7 @@ Status BackupEngineImpl::BackupMeta::StoreToFile(bool sync) {
EnvOptions env_options;
env_options.use_mmap_writes = false;
env_options.use_direct_writes = false;
s = env_->NewWritableFile(meta_filename_ + ".tmp", &backup_meta_file,
env_options);
s = env_->NewWritableFile(meta_tmp_filename_, &backup_meta_file, env_options);
if (!s.ok()) {
return s;
}
Expand Down Expand Up @@ -1751,7 +1762,7 @@ Status BackupEngineImpl::BackupMeta::StoreToFile(bool sync) {
s = backup_meta_file->Close();
}
if (s.ok()) {
s = env_->RenameFile(meta_filename_ + ".tmp", meta_filename_);
s = env_->RenameFile(meta_tmp_filename_, meta_filename_);
}
return s;
}
Expand Down
10 changes: 5 additions & 5 deletions utilities/backupable/backupable_db_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -810,9 +810,9 @@ TEST_F(BackupableDBTest, NoDoubleCopy) {
test_db_env_->SetFilenamesForMockedAttrs(dummy_db_->live_files_);
ASSERT_OK(backup_engine_->CreateNewBackup(db_.get(), false));
std::vector<std::string> should_have_written = {
"/shared/00010.sst.tmp", "/shared/00011.sst.tmp",
"/shared/.00010.sst.tmp", "/shared/.00011.sst.tmp",
"/private/1.tmp/CURRENT", "/private/1.tmp/MANIFEST-01",
"/private/1.tmp/00011.log", "/meta/1.tmp"};
"/private/1.tmp/00011.log", "/meta/.1.tmp"};
AppendPath(backupdir_, should_have_written);
test_backup_env_->AssertWrittenFiles(should_have_written);

Expand All @@ -828,9 +828,9 @@ TEST_F(BackupableDBTest, NoDoubleCopy) {
ASSERT_OK(backup_engine_->CreateNewBackup(db_.get(), false));
// should not open 00010.sst - it's already there

should_have_written = {"/shared/00015.sst.tmp", "/private/2.tmp/CURRENT",
should_have_written = {"/shared/.00015.sst.tmp", "/private/2.tmp/CURRENT",
"/private/2.tmp/MANIFEST-01",
"/private/2.tmp/00011.log", "/meta/2.tmp"};
"/private/2.tmp/00011.log", "/meta/.2.tmp"};
AppendPath(backupdir_, should_have_written);
test_backup_env_->AssertWrittenFiles(should_have_written);

Expand Down Expand Up @@ -1169,7 +1169,7 @@ TEST_F(BackupableDBTest, DeleteTmpFiles) {
} else {
shared_tmp += "/shared";
}
shared_tmp += "/00006.sst.tmp";
shared_tmp += "/.00006.sst.tmp";
std::string private_tmp_dir = backupdir_ + "/private/10.tmp";
std::string private_tmp_file = private_tmp_dir + "/00003.sst";
file_manager_->WriteToFile(shared_tmp, "tmp");
Expand Down

0 comments on commit b092977

Please sign in to comment.