diff --git a/utilities/backup/backup_engine.cc b/utilities/backup/backup_engine.cc index 147c8a54f23..40a064385f2 100644 --- a/utilities/backup/backup_engine.cc +++ b/utilities/backup/backup_engine.cc @@ -291,7 +291,7 @@ class BackupEngineImpl { if (!StartsWith(info->filename, kPrivateDirSlash)) { assert(StartsWith(info->filename, kSharedDirSlash) || StartsWith(info->filename, kSharedChecksumDirSlash)); - remaps_[info->GetDbFileName()] = info; + remaps_[GetDbFileName(info->filename)] = info; } } } @@ -628,6 +628,26 @@ class BackupEngineImpl { return file_copy.erase(first_underscore, file_copy.find_last_of('.') - first_underscore); } + + static inline std::string GetDbFileName(const std::string& filename) { + std::string rv; + // extract the filename part + size_t slash = filename.find_last_of('/'); + // file will either be shared/, shared_checksum/, + // shared_checksum/, shared_checksum/, + // or private// + assert(slash != std::string::npos); + rv = filename.substr(slash + 1); + + // if the file was in shared_checksum, extract the real file name + // in this case the file is __., + // _., or __. + if (filename.substr(0, slash) == kSharedChecksumDirName) { + rv = GetFileFromChecksumFile(rv); + } + return rv; + } + inline std::string GetBackupMetaFile(BackupID backup_id, bool tmp) const { return GetAbsolutePath(kMetaDirName) + "/" + (tmp ? "." : "") + std::to_string(backup_id) + (tmp ? ".tmp" : ""); @@ -2049,9 +2069,7 @@ IOStatus BackupEngineImpl::RestoreDBFromBackup( // as existing, on-disk db file metadata matches this unowned backup file // db_session_id and size. if (options.mode == RestoreOptions::Mode::kKeepLatestDbSessionIdFiles) { - size_t slash = file.find_last_of('/'); - assert(slash != std::string::npos); - std::string filename = file.substr(slash + 1); + std::string filename = GetDbFileName(file); uint64_t number; FileType type; @@ -2127,7 +2145,7 @@ IOStatus BackupEngineImpl::RestoreDBFromBackup( Env* src_env = engine_and_file_info.first->backup_env_; // 1. get DB filename - std::string dst = file_info->GetDbFileName(); + std::string dst = GetDbFileName(file_info->filename); // 2. find the filetype uint64_t number; @@ -2923,7 +2941,7 @@ IOStatus BackupEngineImpl::InferDBFilesToRetainInRestore( !ub_db_sid.empty() && ub_size_bytes > 0) { const auto& db_it = file_num_to_filename_and_type.find(number); if (db_it != file_num_to_filename_and_type.end()) { - const auto& [db_f, db_type] = db_it->second; + const auto& [db_f, _] = db_it->second; uint64_t size_bytes = 0; std::string db_file_path = db_dir + "/" + db_f; IOStatus io_st = db_fs_->GetFileSize(db_file_path, io_options_, @@ -2939,9 +2957,12 @@ IOStatus BackupEngineImpl::InferDBFilesToRetainInRestore( rate_limiter, &db_id, &db_session_id); if (s.ok() && (db_session_id == ub_db_sid)) { // Db file # has been already associated with the excluded backup. - // Remove it from the map of db files to be processed. + // Remove it from the temporary map of db files to be processed. file_num_to_filename_and_type.erase(number); + // Retain the file for the purpose of restore. + files_to_keep.insert(number); + Log(options_.info_log, "Excluded backup file '%s' has its' db file equivalent: '%s'", f.c_str(), db_file_path.c_str()); @@ -2967,7 +2988,7 @@ IOStatus BackupEngineImpl::InferDBFilesToRetainInRestore( uint64_t number; FileType type; - std::string filename = engine_and_file_info.second->GetDbFileName(); + std::string filename = GetDbFileName(engine_and_file_info.second->filename); if (!ParseFileName(filename, &number, &type)) { continue; } diff --git a/utilities/backup/backup_engine_test.cc b/utilities/backup/backup_engine_test.cc index d94ab771e55..2b6208d6f63 100644 --- a/utilities/backup/backup_engine_test.cc +++ b/utilities/backup/backup_engine_test.cc @@ -1102,6 +1102,9 @@ class BackupEngineTestWithParam : public BackupEngineTest, }; TEST_F(BackupEngineTest, IncrementalRestore) { + // Required for excluding files. + engine_options_->schema_version = 2; + const int keys_iteration = 100; options_.disable_auto_compactions = true; @@ -1110,6 +1113,10 @@ TEST_F(BackupEngineTest, IncrementalRestore) { std::vector always_copyable_files = { dbname_ + "/MANIFEST-000005", dbname_ + "/OPTIONS-000007", dbname_ + "/CURRENT.tmp", dbname_ + "/000011.log"}; + std::vector all_but_sst_files = { + dbname_ + "/000010.blob", dbname_ + "/000013.blob", + dbname_ + "/MANIFEST-000005", dbname_ + "/OPTIONS-000007", + dbname_ + "/CURRENT.tmp", dbname_ + "/000011.log"}; std::vector all_backed_up_files = { dbname_ + "/000012.sst", dbname_ + "/000009.sst", dbname_ + "/000010.blob", dbname_ + "/000013.blob", @@ -1125,15 +1132,43 @@ TEST_F(BackupEngineTest, IncrementalRestore) { FillDB(db_.get(), 0, keys_iteration); FillDB(db_.get(), keys_iteration, keys_iteration * 2); - ASSERT_OK(backup_engine_->CreateNewBackup(db_.get())); + // 2. Verify expected behavior with exclude files feature. + BackupID first_id; + CreateBackupOptions cbo; + cbo.exclude_files_callback = [](MaybeExcludeBackupFile* files_begin, + MaybeExcludeBackupFile* files_end) { + for (auto* f = files_begin; f != files_end; ++f) { + std::string s = StringSplit(f->info.relative_file, '/').back(); + s = s.substr(0, s.find('_')); + if (s + ".sst" == "000009.sst") { + // Backup will include everything BUT that one very SST file. + f->exclude_decision = true; + } else { + f->exclude_decision = false; + } + } + }; + ASSERT_OK(backup_engine_->CreateNewBackup(cbo, db_.get(), &first_id)); + + test_db_fs_->ClearWrittenFiles(); + RestoreOptions restore_options(false, mode); + IOStatus io_st = backup_engine_->RestoreDBFromLatestBackup(dbname_, dbname_, + restore_options); + if (mode == RestoreOptions::kKeepLatestDbSessionIdFiles) { + EXPECT_OK(io_st); + test_db_fs_->AssertWrittenFiles(all_but_sst_files); + } else { + ASSERT_TRUE(io_st.IsInvalidArgument()); + } + ASSERT_OK(backup_engine_->DeleteBackup(first_id)); + ASSERT_OK(backup_engine_->CreateNewBackup(db_.get())); CloseDBAndBackupEngine(); + DestroyDBWithoutCheck(dbname_, options_); - // 2. Verify clean restore scenario. + // 3. Verify clean restore scenario. test_db_fs_->ClearWrittenFiles(); - - RestoreOptions restore_options(false, mode); OpenBackupEngine(); ASSERT_OK(backup_engine_->RestoreDBFromLatestBackup(dbname_, dbname_, restore_options)); @@ -1142,7 +1177,6 @@ TEST_F(BackupEngineTest, IncrementalRestore) { // Since we started with a blank db, restore copied all the files. test_db_fs_->AssertWrittenFiles(all_backed_up_files); - db_.reset(); db_.reset(OpenDB()); // Check DB contents. @@ -1153,7 +1187,7 @@ TEST_F(BackupEngineTest, IncrementalRestore) { db_.reset(); // CloseDB - // 3. Incremental restore - simple case. + // 4. Incremental restore - simple case. // // Create 'a hole' in a cleanly restored db by manually deleting data files. // At this point new files were created and old files were deleted - which @@ -1188,7 +1222,7 @@ TEST_F(BackupEngineTest, IncrementalRestore) { db_.reset(); // Close DB. - // 4. Incremental restore - corrupted db files. + // 5. Incremental restore - corrupted db files. ASSERT_OK(db_file_manager_->CorruptFile(dbname_ + "/000010.blob", 3)); // First few bytes corruption will be detected by ReadTablePropertiesHelper @@ -4522,7 +4556,6 @@ TEST_F(BackupEngineTest, ExcludeFiles) { // Include files only in given bucket, based on modulus and remainder constexpr int modulus = 4; int remainder = 0; - cbo.exclude_files_callback = [&remainder]( MaybeExcludeBackupFile* files_begin, MaybeExcludeBackupFile* files_end) {