Extend the test to cover kVerifyChecksum mode

facebook · Jan 3, 2025 · b665268 · b665268
1 parent 6d5c8e5
commit b665268
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 24 deletions.
diff --git a/utilities/backup/backup_engine.cc b/utilities/backup/backup_engine.cc
@@ -1385,7 +1385,6 @@ IOStatus BackupEngineImpl::Initialize() {
               &result.checksum_hex, work_item.src_temperature);
           result.db_id = work_item.db_id;
           result.db_session_id = work_item.db_session_id;
-          work_item.result.set_value(std::move(result));
         } else {
           result.io_status = IOStatus::InvalidArgument(
               "Unknown work item type: " + std::to_string(work_item.type));

diff --git a/utilities/backup/backup_engine_test.cc b/utilities/backup/backup_engine_test.cc
@@ -1108,21 +1108,19 @@ TEST_F(BackupEngineTest, IncrementalRestore) {
   const int keys_iteration = 100;
 
   std::vector<std::string> always_copyable_files = {
-      "/tempdb/MANIFEST-000005", "/tempdb/OPTIONS-000007",
-      "/tempdb/CURRENT.tmp", "/tempdb/000011.log"};
+      dbname_ + "/MANIFEST-000005", dbname_ + "/OPTIONS-000007",
+      dbname_ + "/CURRENT.tmp", dbname_ + "/000011.log"};
   std::vector<std::string> all_backed_up_files = {
-      "/tempdb/000012.sst",      "/tempdb/000009.sst",
-      "/tempdb/000010.blob",     "/tempdb/000013.blob",
-      "/tempdb/MANIFEST-000005", "/tempdb/OPTIONS-000007",
-      "/tempdb/CURRENT.tmp",     "/tempdb/000011.log"};
-
-  // TODO(mszeszko): Fix async checksum computation scheduling in next
-  // iteration.
-  for (const auto& mode : {kKeepLatestDbSessionIdFiles /* kVerifyChecksum */}) {
+      dbname_ + "/000012.sst",      dbname_ + "/000009.sst",
+      dbname_ + "/000010.blob",     dbname_ + "/000013.blob",
+      dbname_ + "/MANIFEST-000005", dbname_ + "/OPTIONS-000007",
+      dbname_ + "/CURRENT.tmp",     dbname_ + "/000011.log"};
+
+  for (const auto& mode : {kKeepLatestDbSessionIdFiles, kVerifyChecksum}) {
     OpenDBAndBackupEngine(true /* destroy_old_data */, false /* dummy */,
                           kShareWithChecksum);
 
-    // 1. Populate the db with data (2 SST files, 2 blob files).
+    // 1. Populate db with seed data (2 SST files, 2 blob files).
     FillDB(db_.get(), 0, keys_iteration);
     FillDB(db_.get(), keys_iteration, keys_iteration * 2);
 
@@ -1163,9 +1161,9 @@ TEST_F(BackupEngineTest, IncrementalRestore) {
     // NOTE: It's perfectly fine if those files don't exist in first place.
     //       In which case, 'hole' (from restore standpoint) is already there.
     IOOptions io_opts;
-    ASSERT_OK(test_db_fs_->DeleteFile("/tempdb/000012.sst", io_opts,
+    ASSERT_OK(test_db_fs_->DeleteFile(dbname_ + "/000012.sst", io_opts,
                                       nullptr /* dbg */));
-    ASSERT_OK(test_db_fs_->DeleteFile("/tempdb/000010.blob", io_opts,
+    ASSERT_OK(test_db_fs_->DeleteFile(dbname_ + "/000010.blob", io_opts,
                                       nullptr /* dbg */));
 
     test_db_fs_->ClearWrittenFiles();
@@ -1176,11 +1174,11 @@ TEST_F(BackupEngineTest, IncrementalRestore) {
     CloseBackupEngine();
 
     std::vector<std::string> should_have_written = always_copyable_files;
-    should_have_written.emplace_back("/tempdb/000012.sst");
-    should_have_written.emplace_back("/tempdb/000010.blob");
+    should_have_written.emplace_back(dbname_ + "/000012.sst");
+    should_have_written.emplace_back(dbname_ + "/000010.blob");
     if (mode == kKeepLatestDbSessionIdFiles) {
       // We only support incremental restore for blobs in kVerifyChecksum mode.
-      should_have_written.emplace_back("/tempdb/000013.blob");
+      should_have_written.emplace_back(dbname_ + "/000013.blob");
     }
 
     // 'Hole' has been patched, 'in-policy' db files were retained.
@@ -1193,11 +1191,13 @@ TEST_F(BackupEngineTest, IncrementalRestore) {
     db_.reset();  // Close DB.
 
     // 4. Incremental restore - corrupted db files.
-    ASSERT_OK(db_file_manager_->CorruptFile("/tempdb/000010.blob", 3));
+    ASSERT_OK(db_file_manager_->CorruptFile(dbname_ + "/000010.blob", 3));
 
-    // First few bytes corruption will be captured by ReadTablePropertiesHelper
-    // at the time of reading the footer metadata.
-    ASSERT_OK(db_file_manager_->CorruptFileMiddle("/tempdb/000012.sst"));
+    // First few bytes corruption will be detected by ReadTablePropertiesHelper
+    // at the time of reading the metadata footer. We must try a bit harder
+    // (by overriding bytes beyond the footer) to corrupt a file in a more
+    // harmful way...
+    ASSERT_OK(db_file_manager_->CorruptFileMiddle(dbname_ + "/000012.sst"));
 
     test_db_fs_->ClearWrittenFiles();
 
@@ -1207,12 +1207,12 @@ TEST_F(BackupEngineTest, IncrementalRestore) {
     CloseBackupEngine();
 
     should_have_written = always_copyable_files;
-    should_have_written.emplace_back("/tempdb/000010.blob");
+    should_have_written.emplace_back(dbname_ + "/000010.blob");
     if (mode == kVerifyChecksum) {
       // Restore running in kVerifyChecksum mode would have caught the mismatch
       // between crc32c and db file contents. Such file would have been deleted
       // and restored from the backup.
-      should_have_written.emplace_back("/tempdb/000012.sst");
+      should_have_written.emplace_back(dbname_ + "/000012.sst");
     } else if (mode == kKeepLatestDbSessionIdFiles) {
       // This mode is more prone to subtle db file corruptions as we do not run
       // any CPU intensive computations (like checksum) and the match between
@@ -1223,7 +1223,7 @@ TEST_F(BackupEngineTest, IncrementalRestore) {
       // the cracks and only be detected at the time of running CRC32c.
 
       // Separately, this mode does not support incremental restore for blobs.
-      should_have_written.emplace_back("/tempdb/000013.blob");
+      should_have_written.emplace_back(dbname_ + "/000013.blob");
     }
 
     // 'Hole' has been patched, 'in-policy' db files were retained.