fix

utilities/checkpoint/checkpoint_impl.cc

@@ -46,28 +46,38 @@ Status Checkpoint::CreateCheckpoint(const std::string& /*checkpoint_dir*/,
   return Status::NotSupported("");
 }
 
-void CheckpointImpl::CleanStagingDirectory(const std::string& full_private_path,
-                                           Logger* info_log) {
+Status CheckpointImpl::CleanStagingDirectory(
+    const std::string& full_private_path, Logger* info_log) {
   std::vector<std::string> subchildren;
   Status s = db_->GetEnv()->FileExists(full_private_path);
   if (s.IsNotFound()) {
-    return;
+    // Nothing to clean
+    return Status::OK();
   }
   ROCKS_LOG_INFO(info_log, "File exists %s -- %s", full_private_path.c_str(),
                  s.ToString().c_str());
+
   s = db_->GetEnv()->GetChildren(full_private_path, &subchildren);
   if (s.ok()) {
     for (auto& subchild : subchildren) {
+      Status del_s;
       std::string subchild_path = full_private_path + "/" + subchild;
-      s = db_->GetEnv()->DeleteFile(subchild_path);
+      del_s = db_->GetEnv()->DeleteFile(subchild_path);
       ROCKS_LOG_INFO(info_log, "Delete file %s -- %s", subchild_path.c_str(),
-                     s.ToString().c_str());
+                     del_s.ToString().c_str());
+      if (!del_s.ok() && s.ok()) {
+        s = del_s;
+      }
     }
   }
-  // finally delete the private dir
-  s = db_->GetEnv()->DeleteDir(full_private_path);
-  ROCKS_LOG_INFO(info_log, "Delete dir %s -- %s", full_private_path.c_str(),
-                 s.ToString().c_str());
+
+  // Then delete the private dir
+  if (s.ok()) {
+    s = db_->GetEnv()->DeleteDir(full_private_path);
+    ROCKS_LOG_INFO(info_log, "Delete dir %s -- %s", full_private_path.c_str(),
+                   s.ToString().c_str());
+  }
+  return s;
 }
 
 Status Checkpoint::ExportColumnFamily(
@@ -82,14 +92,17 @@ Status CheckpointImpl::CreateCheckpoint(const std::string& checkpoint_dir,
                                         uint64_t* sequence_number_ptr) {
   DBOptions db_options = db_->GetDBOptions();
 
-  Status s = db_->GetEnv()->FileExists(checkpoint_dir);
-  if (s.ok()) {
+  Status file_exists_s = db_->GetEnv()->FileExists(checkpoint_dir);
+  if (file_exists_s.ok()) {
     return Status::InvalidArgument("Directory exists");
-  } else if (!s.IsNotFound()) {
-    assert(s.IsIOError());
-    return s;
-  }
+  } else if (!file_exists_s.IsNotFound()) {
+    assert(file_exists_s.IsIOError());
+    return file_exists_s;
+  } else {
+    assert(file_exists_s.IsNotFound());
+  };
 
+  Status s;
   ROCKS_LOG_INFO(
       db_options.info_log,
       "Started the snapshot process -- creating snapshot in directory %s",
@@ -109,7 +122,14 @@ Status CheckpointImpl::CreateCheckpoint(const std::string& checkpoint_dir,
   ROCKS_LOG_INFO(db_options.info_log,
                  "Snapshot process -- using temporary directory %s",
                  full_private_path.c_str());
-  CleanStagingDirectory(full_private_path, db_options.info_log.get());
+
+  s = CleanStagingDirectory(full_private_path, db_options.info_log.get());
+  if (!s.ok()) {
+    return Status::Aborted(
+        "Failed to clean the temporary directory " + full_private_path +
+        " needed before checkpoint creation : " + s.ToString());
+  }
+
   // create snapshot directory
   s = db_->GetEnv()->CreateDir(full_private_path);
   uint64_t sequence_number = 0;
@@ -180,10 +200,15 @@ Status CheckpointImpl::CreateCheckpoint(const std::string& checkpoint_dir,
     ROCKS_LOG_INFO(db_options.info_log, "Snapshot sequence number: %" PRIu64,
                    sequence_number);
   } else {
-    // clean all the files we might have created
     ROCKS_LOG_INFO(db_options.info_log, "Snapshot failed -- %s",
                    s.ToString().c_str());
-    CleanStagingDirectory(full_private_path, db_options.info_log.get());
+    // clean all the files and directory we might have created
+    Status del_s =
+        CleanStagingDirectory(full_private_path, db_options.info_log.get());
+    ROCKS_LOG_INFO(db_options.info_log,
+                   "Clean files or directory we might have created %s: %s",
+                   full_private_path.c_str(), del_s.ToString().c_str());
+    del_s.PermitUncheckedError();
   }
   return s;
 }

utilities/checkpoint/checkpoint_impl.h

@@ -44,7 +44,7 @@ class CheckpointImpl : public Checkpoint {
       bool get_live_table_checksum = false);
 
  private:
-  void CleanStagingDirectory(const std::string& path, Logger* info_log);
+  Status CleanStagingDirectory(const std::string& path, Logger* info_log);
 
   // Export logic customization by providing callbacks for link or copy.
   Status ExportFilesInMetaData(
@@ -61,4 +61,3 @@ class CheckpointImpl : public Checkpoint {
 };
 
 }  // namespace ROCKSDB_NAMESPACE
-