From 56887f6cb854fde8e16c1ca65533716976c2e2f3 Mon Sep 17 00:00:00 2001 From: Wanning Jiang Date: Thu, 9 Jun 2016 19:03:10 -0700 Subject: [PATCH] Backup Options Summary: Backup options file to private directory Test Plan: backupable_db_test.cc, BackupOptions Modify DB options by calling OpenDB for 3 times. Check the latest options file is in the right place. Also check no redundent files are backuped. Reviewers: andrewkr Reviewed By: andrewkr Subscribers: leveldb, dhruba, andrewkr Differential Revision: https://reviews.facebook.net/D59373 --- db/db_filesnapshot.cc | 11 ++-- db/db_impl.cc | 4 +- db/db_impl.h | 2 +- db/db_test.cc | 4 +- db/version_set.h | 3 ++ utilities/backupable/backupable_db.cc | 12 +++-- utilities/backupable/backupable_db_test.cc | 60 +++++++++++++++++----- utilities/checkpoint/checkpoint.cc | 4 +- 8 files changed, 72 insertions(+), 28 deletions(-) diff --git a/db/db_filesnapshot.cc b/db/db_filesnapshot.cc index 89ebb6650..7ed1f1d36 100644 --- a/db/db_filesnapshot.cc +++ b/db/db_filesnapshot.cc @@ -14,19 +14,20 @@ #endif #include +#include #include #include -#include #include "db/db_impl.h" #include "db/filename.h" #include "db/job_context.h" #include "db/version_set.h" +#include "port/port.h" #include "rocksdb/db.h" #include "rocksdb/env.h" -#include "port/port.h" +#include "util/file_util.h" #include "util/mutexlock.h" #include "util/sync_point.h" -#include "util/file_util.h" +#include "util/testharness.h" namespace rocksdb { @@ -83,7 +84,6 @@ int DBImpl::IsFileDeletionsEnabled() const { Status DBImpl::GetLiveFiles(std::vector& ret, uint64_t* manifest_file_size, bool flush_memtable) { - *manifest_file_size = 0; mutex_.Lock(); @@ -126,7 +126,7 @@ Status DBImpl::GetLiveFiles(std::vector& ret, } ret.clear(); - ret.reserve(live.size() + 2); //*.sst + CURRENT + MANIFEST + ret.reserve(live.size() + 3); // *.sst + CURRENT + MANIFEST + OPTIONS // create names of the live files. The names are not absolute // paths, instead they are relative to dbname_; @@ -136,6 +136,7 @@ Status DBImpl::GetLiveFiles(std::vector& ret, ret.push_back(CurrentFileName("")); ret.push_back(DescriptorFileName("", versions_->manifest_file_number())); + ret.push_back(OptionsFileName("", versions_->options_file_number())); // find length of manifest file while holding the mutex lock *manifest_file_size = versions_->manifest_file_size(); diff --git a/db/db_impl.cc b/db/db_impl.cc index 0fda41cd8..2511c2ddf 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -6005,8 +6005,10 @@ Status DBImpl::DeleteObsoleteOptionsFiles() { Status DBImpl::RenameTempFileToOptionsFile(const std::string& file_name) { #ifndef ROCKSDB_LITE Status s; + + versions_->options_file_number_ = versions_->NewFileNumber(); std::string options_file_name = - OptionsFileName(GetName(), versions_->NewFileNumber()); + OptionsFileName(GetName(), versions_->options_file_number_); // Retry if the file name happen to conflict with an existing one. s = GetEnv()->RenameFile(file_name, options_file_name); diff --git a/db/db_impl.h b/db/db_impl.h index 70ab97c9e..cceed0321 100644 --- a/db/db_impl.h +++ b/db/db_impl.h @@ -521,7 +521,7 @@ class DBImpl : public DB { Compaction *c, const Status &st, const CompactionJobStats& job_stats, int job_id); - void NotifyOnMemTableSealed(ColumnFamilyData* cfd, + void NotifyOnMemTableSealed(ColumnFamilyData* cfd, const MemTableInfo& mem_table_info); void NewThreadStatusCfInfo(ColumnFamilyData* cfd) const; diff --git a/db/db_test.cc b/db/db_test.cc index 4160cfe3e..d7dcc2d26 100644 --- a/db/db_test.cc +++ b/db/db_test.cc @@ -2067,8 +2067,8 @@ TEST_F(DBTest, SnapshotFiles) { dbfull()->DisableFileDeletions(); dbfull()->GetLiveFiles(files, &manifest_size); - // CURRENT, MANIFEST, *.sst files (one for each CF) - ASSERT_EQ(files.size(), 4U); + // CURRENT, MANIFEST, OPTIONS, *.sst files (one for each CF) + ASSERT_EQ(files.size(), 5U); uint64_t number = 0; FileType type; diff --git a/db/version_set.h b/db/version_set.h index 5482e56c2..37f645e49 100644 --- a/db/version_set.h +++ b/db/version_set.h @@ -627,6 +627,8 @@ class VersionSet { // Return the current manifest file number uint64_t manifest_file_number() const { return manifest_file_number_; } + uint64_t options_file_number() const { return options_file_number_; } + uint64_t pending_manifest_file_number() const { return pending_manifest_file_number_; } @@ -743,6 +745,7 @@ class VersionSet { const DBOptions* const db_options_; std::atomic next_file_number_; uint64_t manifest_file_number_; + uint64_t options_file_number_; uint64_t pending_manifest_file_number_; std::atomic last_sequence_; uint64_t prev_log_number_; // 0 or backing store for memtable being compacted diff --git a/utilities/backupable/backupable_db.cc b/utilities/backupable/backupable_db.cc index 422896e85..1f18e119e 100644 --- a/utilities/backupable/backupable_db.cc +++ b/utilities/backupable/backupable_db.cc @@ -29,17 +29,17 @@ #include #include #include -#include +#include +#include +#include #include #include #include #include -#include -#include -#include #include #include #include +#include namespace rocksdb { @@ -694,6 +694,7 @@ Status BackupEngineImpl::CreateNewBackupWithMetadata( TEST_SYNC_POINT("BackupEngineImpl::CreateNewBackup:SavedLiveFiles2"); 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(new BackupMeta( @@ -745,7 +746,8 @@ Status BackupEngineImpl::CreateNewBackupWithMetadata( } // we should only get sst, manifest and current files here assert(type == kTableFile || type == kDescriptorFile || - type == kCurrentFile); + type == kCurrentFile || type == kOptionsFile); + if (type == kCurrentFile) { // We will craft the current file manually to ensure it's consistent with // the manifest number. This is necessary because current's file contents diff --git a/utilities/backupable/backupable_db_test.cc b/utilities/backupable/backupable_db_test.cc index c41ce94d2..6af0c7057 100644 --- a/utilities/backupable/backupable_db_test.cc +++ b/utilities/backupable/backupable_db_test.cc @@ -20,10 +20,12 @@ #include "rocksdb/transaction_log.h" #include "rocksdb/types.h" #include "rocksdb/utilities/backupable_db.h" +#include "rocksdb/utilities/options_util.h" #include "util/env_chroot.h" #include "util/file_reader_writer.h" #include "util/mutexlock.h" #include "util/random.h" +#include "util/stderr_logger.h" #include "util/string_util.h" #include "util/sync_point.h" #include "util/testharness.h" @@ -200,6 +202,7 @@ class TestEnv : public EnvWrapper { MutexLock l(&mutex_); std::sort(should_have_written.begin(), should_have_written.end()); std::sort(written_files_.begin(), written_files_.end()); + ASSERT_TRUE(written_files_ == should_have_written); } @@ -756,8 +759,8 @@ TEST_F(BackupableDBTest, NoDoubleCopy) { test_backup_env_->SetLimitWrittenFiles(7); test_backup_env_->ClearWrittenFiles(); test_db_env_->SetLimitWrittenFiles(0); - dummy_db_->live_files_ = { "/00010.sst", "/00011.sst", - "/CURRENT", "/MANIFEST-01" }; + dummy_db_->live_files_ = {"/00010.sst", "/00011.sst", "/CURRENT", + "/MANIFEST-01"}; dummy_db_->wal_files_ = {{"/00011.log", true}, {"/00012.log", false}}; test_db_env_->SetFilenamesForMockedAttrs(dummy_db_->live_files_); ASSERT_OK(backup_engine_->CreateNewBackup(db_.get(), false)); @@ -773,20 +776,20 @@ TEST_F(BackupableDBTest, NoDoubleCopy) { // should not write/copy 00010.sst, since it's already there! test_backup_env_->SetLimitWrittenFiles(6); test_backup_env_->ClearWrittenFiles(); - dummy_db_->live_files_ = { "/00010.sst", "/00015.sst", - "/CURRENT", "/MANIFEST-01" }; + + dummy_db_->live_files_ = {"/00010.sst", "/00015.sst", "/CURRENT", + "/MANIFEST-01"}; dummy_db_->wal_files_ = {{"/00011.log", true}, {"/00012.log", false}}; test_db_env_->SetFilenamesForMockedAttrs(dummy_db_->live_files_); 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", - "/private/2.tmp/MANIFEST-01", - "/private/2.tmp/00011.log", - "/meta/2.tmp", - "/LATEST_BACKUP.tmp" - }; + + should_have_written = {"/shared/00015.sst.tmp", + "/private/2.tmp/CURRENT", + "/private/2.tmp/MANIFEST-01", + "/private/2.tmp/00011.log", + "/meta/2.tmp", + "/LATEST_BACKUP.tmp"}; AppendPath(backupdir_, should_have_written); test_backup_env_->AssertWrittenFiles(should_have_written); @@ -943,6 +946,39 @@ TEST_F(BackupableDBTest, CorruptionsTest) { AssertBackupConsistency(2, 0, keys_iteration * 2, keys_iteration * 5); } +inline std::string OptionsPath(std::string ret, int backupID) { + ret += "/private/"; + ret += std::to_string(backupID); + ret += "/"; + return ret; +} + +// Backup the LATEST options file to +// "/private//OPTIONS" + +TEST_F(BackupableDBTest, BackupOptions) { + OpenDBAndBackupEngine(true); + for (int i = 1; i < 5; i++) { + std::string name; + std::vector filenames; + // Must reset() before reset(OpenDB()) again. + // Calling OpenDB() while *db_ is existing will cause LOCK issue + db_.reset(); + db_.reset(OpenDB()); + ASSERT_OK(backup_engine_->CreateNewBackup(db_.get(), true)); + rocksdb::GetLatestOptionsFileName(db_->GetName(), options_.env, &name); + ASSERT_OK(file_manager_->FileExists(OptionsPath(backupdir_, i) + name)); + backup_chroot_env_->GetChildren(OptionsPath(backupdir_, i), &filenames); + for (auto fn : filenames) { + if (fn.compare(0, 7, "OPTIONS") == 0) { + ASSERT_EQ(name, fn); + } + } + } + + CloseDBAndBackupEngine(); +} + // This test verifies we don't delete the latest backup when read-only option is // set TEST_F(BackupableDBTest, NoDeleteWithReadOnly) { diff --git a/utilities/checkpoint/checkpoint.cc b/utilities/checkpoint/checkpoint.cc index ff89f2446..36b9be43e 100644 --- a/utilities/checkpoint/checkpoint.cc +++ b/utilities/checkpoint/checkpoint.cc @@ -110,9 +110,9 @@ Status CheckpointImpl::CreateCheckpoint(const std::string& checkpoint_dir) { s = Status::Corruption("Can't parse file name. This is very bad"); break; } - // we should only get sst, manifest and current files here + // we should only get sst, options, manifest and current files here assert(type == kTableFile || type == kDescriptorFile || - type == kCurrentFile); + type == kCurrentFile || type == kOptionsFile); assert(live_files[i].size() > 0 && live_files[i][0] == '/'); if (type == kCurrentFile) { // We will craft the current file manually to ensure it's consistent with