Remove the need for LATEST_BACKUP in BackupEngine
Summary: In the first implementation of BackupEngine, LATEST_BACKUP was the commit point. The backup became committed after the write to LATEST_BACKUP completed. However, we can avoid the need for LATEST_BACKUP. Instead of write to LATEST_BACKUP, the commit point can be the rename from `meta/<backup_id>.tmp` to `meta/<backup_id>`. Once we see that there exists a file `meta/<backup_id>` (without tmp), we can assume that backup is valid. In this diff, we still write out the file LATEST_BACKUP. We need to do this so that we can maintain backward compatibility. However, the new version doesn't depend on this file anymore. We get the latest backup by `ls`-ing `meta` directory. This diff depends on D41925 Test Plan: Adjusted backupable_db_test to this new behavior Reviewers: benj, yhchiang, sdong, AaronFeldman Reviewed By: sdong Subscribers: dhruba, leveldb Differential Revision: https://reviews.facebook.net/D42069
This commit is contained in:
parent
0f763db20a
commit
b42cd6bed5
@ -5,6 +5,7 @@
|
||||
### New Features
|
||||
* DB::GetProperty() now accept "rocksdb.aggregated-table-properties" and "rocksdb.aggregated-table-properties-at-levelN", in which case it returns aggregated table properties of the target column family, or the aggregated table properties of the specified level N if the "at-level" version is used.
|
||||
* Add compression option kZSTDNotFinalCompression for people to experiment ZSTD although its format is not finalized.
|
||||
* We removed the need for LATEST_BACKUP file in BackupEngine. We still keep writing it when we create new backups (because of backward compatibility), but we don't read it anymore.
|
||||
|
||||
### Public API Changes
|
||||
* Removed class Env::RandomRWFile and Env::NewRandomRWFile().
|
||||
|
@ -307,7 +307,6 @@ class BackupEngineImpl : public BackupEngine {
|
||||
return GetBackupMetaDir() + "/" + rocksdb::ToString(backup_id);
|
||||
}
|
||||
|
||||
Status GetLatestBackupFileContents(uint32_t* latest_backup);
|
||||
Status PutLatestBackupFileContents(uint32_t latest_backup);
|
||||
// if size_limit == 0, there is no size limit, copy everything
|
||||
Status CopyFile(const std::string& src,
|
||||
@ -572,6 +571,7 @@ Status BackupEngineImpl::Initialize() {
|
||||
&backuped_file_infos_, backup_env_)))));
|
||||
}
|
||||
|
||||
latest_backup_id_ = 0;
|
||||
if (options_.destroy_old_data) { // Destroy old data
|
||||
assert(!read_only_);
|
||||
Log(options_.info_log,
|
||||
@ -584,8 +584,6 @@ Status BackupEngineImpl::Initialize() {
|
||||
if (!s.ok()) {
|
||||
return s;
|
||||
}
|
||||
// start from beginning
|
||||
latest_backup_id_ = 0;
|
||||
} else { // Load data from storage
|
||||
// load the backups if any
|
||||
for (auto& backup : backups_) {
|
||||
@ -598,52 +596,17 @@ Status BackupEngineImpl::Initialize() {
|
||||
} else {
|
||||
Log(options_.info_log, "Loading backup %" PRIu32 " OK:\n%s",
|
||||
backup.first, backup.second->GetInfoString().c_str());
|
||||
latest_backup_id_ = std::max(latest_backup_id_, backup.first);
|
||||
}
|
||||
}
|
||||
|
||||
for (const auto& corrupt : corrupt_backups_) {
|
||||
backups_.erase(backups_.find(corrupt.first));
|
||||
}
|
||||
|
||||
Status s = GetLatestBackupFileContents(&latest_backup_id_);
|
||||
|
||||
// If latest backup file is corrupted or non-existent
|
||||
// set latest backup as the biggest backup we have
|
||||
// or 0 if we have no backups
|
||||
if (!s.ok() ||
|
||||
backups_.find(latest_backup_id_) == backups_.end()) {
|
||||
auto itr = backups_.end();
|
||||
latest_backup_id_ = (itr == backups_.begin()) ? 0 : (--itr)->first;
|
||||
}
|
||||
}
|
||||
|
||||
Log(options_.info_log, "Latest backup is %u", latest_backup_id_);
|
||||
|
||||
// delete any backups that claim to be later than latest
|
||||
std::vector<BackupID> later_ids;
|
||||
for (auto itr = backups_.lower_bound(latest_backup_id_ + 1);
|
||||
itr != backups_.end(); itr++) {
|
||||
Log(options_.info_log,
|
||||
"Found backup claiming to be later than latest: %" PRIu32, itr->first);
|
||||
later_ids.push_back(itr->first);
|
||||
}
|
||||
for (auto id : later_ids) {
|
||||
Status s;
|
||||
if (!read_only_) {
|
||||
s = DeleteBackup(id);
|
||||
} else {
|
||||
auto backup = backups_.find(id);
|
||||
// We just found it couple of lines earlier!
|
||||
assert(backup != backups_.end());
|
||||
s = backup->second->Delete(false);
|
||||
backups_.erase(backup);
|
||||
}
|
||||
if (!s.ok()) {
|
||||
Log(options_.info_log, "Failed deleting backup %" PRIu32 " -- %s", id,
|
||||
s.ToString().c_str());
|
||||
}
|
||||
}
|
||||
|
||||
if (!read_only_) {
|
||||
auto s = PutLatestBackupFileContents(latest_backup_id_);
|
||||
if (!s.ok()) {
|
||||
@ -1089,38 +1052,6 @@ Status BackupEngineImpl::RestoreDBFromBackup(
|
||||
return s;
|
||||
}
|
||||
|
||||
// latest backup id is an ASCII representation of latest backup id
|
||||
Status BackupEngineImpl::GetLatestBackupFileContents(uint32_t* latest_backup) {
|
||||
Status s;
|
||||
unique_ptr<SequentialFile> file;
|
||||
s = backup_env_->NewSequentialFile(GetLatestBackupFile(),
|
||||
&file,
|
||||
EnvOptions());
|
||||
if (!s.ok()) {
|
||||
return s;
|
||||
}
|
||||
|
||||
char buf[11];
|
||||
Slice data;
|
||||
unique_ptr<SequentialFileReader> file_reader(
|
||||
new SequentialFileReader(std::move(file)));
|
||||
|
||||
s = file_reader->Read(10, &data, buf);
|
||||
if (!s.ok() || data.size() == 0) {
|
||||
return s.ok() ? Status::Corruption("Latest backup file corrupted") : s;
|
||||
}
|
||||
buf[data.size()] = 0;
|
||||
|
||||
*latest_backup = 0;
|
||||
sscanf(data.data(), "%u", latest_backup);
|
||||
|
||||
s = backup_env_->FileExists(GetBackupMetaFile(*latest_backup));
|
||||
if (s.IsNotFound()) {
|
||||
s = Status::Corruption("Latest backup file corrupted");
|
||||
}
|
||||
return s;
|
||||
}
|
||||
|
||||
// this operation HAS to be atomic
|
||||
// writing 4 bytes to the file is atomic alright, but we should *never*
|
||||
// do something like 1. delete file, 2. write new file
|
||||
|
@ -669,20 +669,28 @@ TEST_F(BackupableDBTest, CorruptionsTest) {
|
||||
ASSERT_OK(file_manager_->DeleteFile(backupdir_ + "/LATEST_BACKUP"));
|
||||
AssertBackupConsistency(0, 0, keys_iteration * 5);
|
||||
// create backup 6, point LATEST_BACKUP to 5
|
||||
// behavior change: this used to delete backup 6. however, now we ignore
|
||||
// LATEST_BACKUP contents so BackupEngine sets latest backup to 6.
|
||||
OpenDBAndBackupEngine();
|
||||
FillDB(db_.get(), keys_iteration * 5, keys_iteration * 6);
|
||||
ASSERT_OK(backup_engine_->CreateNewBackup(db_.get(), false));
|
||||
CloseDBAndBackupEngine();
|
||||
ASSERT_OK(file_manager_->WriteToFile(backupdir_ + "/LATEST_BACKUP", "5"));
|
||||
AssertBackupConsistency(0, 0, keys_iteration * 5, keys_iteration * 6);
|
||||
// assert that all 6 data is gone!
|
||||
ASSERT_EQ(Status::NotFound(),
|
||||
file_manager_->FileExists(backupdir_ + "/meta/6"));
|
||||
ASSERT_EQ(Status::NotFound(),
|
||||
file_manager_->FileExists(backupdir_ + "/private/6"));
|
||||
AssertBackupConsistency(0, 0, keys_iteration * 6);
|
||||
// assert that all 6 data is still here
|
||||
ASSERT_OK(file_manager_->FileExists(backupdir_ + "/meta/6"));
|
||||
ASSERT_OK(file_manager_->FileExists(backupdir_ + "/private/6"));
|
||||
// assert that we wrote 6 to LATEST_BACKUP
|
||||
{
|
||||
std::string latest_backup_contents;
|
||||
ReadFileToString(env_, backupdir_ + "/LATEST_BACKUP",
|
||||
&latest_backup_contents);
|
||||
ASSERT_EQ(std::atol(latest_backup_contents.c_str()), 6);
|
||||
}
|
||||
|
||||
// --------- case 3. corrupted backup meta or missing backuped file ----
|
||||
ASSERT_OK(file_manager_->CorruptFile(backupdir_ + "/meta/5", 3));
|
||||
ASSERT_OK(file_manager_->CorruptFile(backupdir_ + "/meta/6", 3));
|
||||
// since 5 meta is now corrupted, latest backup should be 4
|
||||
AssertBackupConsistency(0, 0, keys_iteration * 4, keys_iteration * 5);
|
||||
OpenBackupEngine();
|
||||
@ -783,10 +791,11 @@ TEST_F(BackupableDBTest, NoDeleteWithReadOnly) {
|
||||
ASSERT_OK(file_manager_->FileExists(backupdir_ + "/meta/5"));
|
||||
ASSERT_OK(file_manager_->FileExists(backupdir_ + "/private/5"));
|
||||
|
||||
// even though 5 is here, we should only see 4 backups
|
||||
// Behavior change: We now ignore LATEST_BACKUP contents. This means that
|
||||
// we should have 5 backups, even if LATEST_BACKUP says 4.
|
||||
std::vector<BackupInfo> backup_info;
|
||||
read_only_backup_engine->GetBackupInfo(&backup_info);
|
||||
ASSERT_EQ(4UL, backup_info.size());
|
||||
ASSERT_EQ(5UL, backup_info.size());
|
||||
delete read_only_backup_engine;
|
||||
}
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user