From a9c86f51b7f47ee6b6ff278625c29642556407db Mon Sep 17 00:00:00 2001
From: Andrew Kryczka <andrewkr@fb.com>
Date: Thu, 30 Mar 2017 14:44:56 -0700
Subject: [PATCH] backup garbage collect shared_checksum tmp files

Summary:
previously we only cleaned up .tmp files under "shared/" and "private/" directories in case the previous backup failed. we need to do the same for "shared_checksum/"; otherwise, the subsequent backup will fail if it tries to backup at least one of the same files.
Closes https://github.com/facebook/rocksdb/pull/2062

Differential Revision: D4805599

Pulled By: ajkr

fbshipit-source-id: eaa6088
---
 utilities/backupable/backupable_db.cc      | 14 +++++-
 utilities/backupable/backupable_db_test.cc | 52 +++++++++++++++-------
 2 files changed, 48 insertions(+), 18 deletions(-)

diff --git a/utilities/backupable/backupable_db.cc b/utilities/backupable/backupable_db.cc
index ccd22b25a..6a5c7458b 100644
--- a/utilities/backupable/backupable_db.cc
+++ b/utilities/backupable/backupable_db.cc
@@ -1458,7 +1458,12 @@ Status BackupEngineImpl::GarbageCollect() {
     // delete obsolete shared files
     std::vector<std::string> shared_children;
     {
-      auto shared_path = GetAbsolutePath(GetSharedFileRel());
+      std::string shared_path;
+      if (options_.share_files_with_checksum) {
+        shared_path = GetAbsolutePath(GetSharedFileWithChecksumRel());
+      } else {
+        shared_path = GetAbsolutePath(GetSharedFileRel());
+      }
       auto s = backup_env_->FileExists(shared_path);
       if (s.ok()) {
         s = backup_env_->GetChildren(shared_path, &shared_children);
@@ -1470,7 +1475,12 @@ Status BackupEngineImpl::GarbageCollect() {
       }
     }
     for (auto& child : shared_children) {
-      std::string rel_fname = GetSharedFileRel(child);
+      std::string rel_fname;
+      if (options_.share_files_with_checksum) {
+        rel_fname = GetSharedFileWithChecksumRel(child);
+      } else {
+        rel_fname = GetSharedFileRel(child);
+      }
       auto child_itr = backuped_file_infos_.find(rel_fname);
       // if it's not refcounted, delete it
       if (child_itr == backuped_file_infos_.end() ||
diff --git a/utilities/backupable/backupable_db_test.cc b/utilities/backupable/backupable_db_test.cc
index 3e09dea2b..22c93cbb0 100644
--- a/utilities/backupable/backupable_db_test.cc
+++ b/utilities/backupable/backupable_db_test.cc
@@ -1126,22 +1126,42 @@ TEST_F(BackupableDBTest, ShareTableFilesWithChecksumsTransition) {
 }
 
 TEST_F(BackupableDBTest, DeleteTmpFiles) {
-  OpenDBAndBackupEngine();
-  CloseDBAndBackupEngine();
-  std::string shared_tmp = backupdir_ + "/shared/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");
-  file_manager_->CreateDir(private_tmp_dir);
-  file_manager_->WriteToFile(private_tmp_file, "tmp");
-  ASSERT_OK(file_manager_->FileExists(private_tmp_dir));
-  OpenDBAndBackupEngine();
-  // Need to call this explicitly to delete tmp files
-  (void)backup_engine_->GarbageCollect();
-  CloseDBAndBackupEngine();
-  ASSERT_EQ(Status::NotFound(), file_manager_->FileExists(shared_tmp));
-  ASSERT_EQ(Status::NotFound(), file_manager_->FileExists(private_tmp_file));
-  ASSERT_EQ(Status::NotFound(), file_manager_->FileExists(private_tmp_dir));
+  for (bool shared_checksum : {false, true}) {
+    if (shared_checksum) {
+      OpenDBAndBackupEngineShareWithChecksum(
+          false /* destroy_old_data */, false /* dummy */,
+          true /* share_table_files */, true /* share_with_checksums */);
+    } else {
+      OpenDBAndBackupEngine();
+    }
+    CloseDBAndBackupEngine();
+    std::string shared_tmp = backupdir_;
+    if (shared_checksum) {
+      shared_tmp += "/shared_checksum";
+    } else {
+      shared_tmp += "/shared";
+    }
+    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");
+    file_manager_->CreateDir(private_tmp_dir);
+    file_manager_->WriteToFile(private_tmp_file, "tmp");
+    ASSERT_OK(file_manager_->FileExists(private_tmp_dir));
+    if (shared_checksum) {
+      OpenDBAndBackupEngineShareWithChecksum(
+          false /* destroy_old_data */, false /* dummy */,
+          true /* share_table_files */, true /* share_with_checksums */);
+    } else {
+      OpenDBAndBackupEngine();
+    }
+    // Need to call this explicitly to delete tmp files
+    (void)backup_engine_->GarbageCollect();
+    CloseDBAndBackupEngine();
+    ASSERT_EQ(Status::NotFound(), file_manager_->FileExists(shared_tmp));
+    ASSERT_EQ(Status::NotFound(), file_manager_->FileExists(private_tmp_file));
+    ASSERT_EQ(Status::NotFound(), file_manager_->FileExists(private_tmp_dir));
+  }
 }
 
 TEST_F(BackupableDBTest, KeepLogFiles) {