Better destroydb

Summary:
Delete archive directory before WAL folder
  since archive may be contained as a subfolder.
  Also improve loop readability.
Closes https://github.com/facebook/rocksdb/pull/3797

Differential Revision: D7866378

Pulled By: riversand963

fbshipit-source-id: 0c45d97677ce6fbefa3f8d602ef5e2a2a925e6f5

db/db_impl.cc

@@ -2088,6 +2088,7 @@ void DBImpl::ReleaseFileNumberFromPendingOutputs(
 Status DBImpl::GetUpdatesSince(
     SequenceNumber seq, unique_ptr<TransactionLogIterator>* iter,
     const TransactionLogIterator::ReadOptions& read_options) {
+
   RecordTick(stats_, GET_UPDATES_SINCE_CALLS);
   if (seq > versions_->LastSequence()) {
     return Status::NotFound("Requested sequence not yet written in the db");
@@ -2436,11 +2437,11 @@ Status DestroyDB(const std::string& dbname, const Options& options,
     uint64_t number;
     FileType type;
     InfoLogPrefix info_log_prefix(!soptions.db_log_dir.empty(), dbname);
-    for (size_t i = 0; i < filenames.size(); i++) {
+    for (const auto& fname : filenames) {
-      if (ParseFileName(filenames[i], &number, info_log_prefix.prefix, &type) &&
+      if (ParseFileName(fname, &number, info_log_prefix.prefix, &type) &&
-          type != kDBLockFile) {  // Lock file will be deleted at end
+        type != kDBLockFile) {  // Lock file will be deleted at end
         Status del;
-        std::string path_to_delete = dbname + "/" + filenames[i];
+        std::string path_to_delete = dbname + "/" + fname;
         if (type == kMetaDatabase) {
           del = DestroyDB(path_to_delete, options);
         } else if (type == kTableFile) {
@@ -2456,13 +2457,12 @@ Status DestroyDB(const std::string& dbname, const Options& options,
 
     std::vector<std::string> paths;
 
-    for (size_t path_id = 0; path_id < options.db_paths.size(); path_id++) {
+    for (const auto& path : options.db_paths) {
-      paths.emplace_back(options.db_paths[path_id].path);
+      paths.emplace_back(path.path);
     }
-    for (auto& cf : column_families) {
+    for (const auto& cf : column_families) {
-      for (size_t path_id = 0; path_id < cf.options.cf_paths.size();
+      for (const auto& path : cf.options.cf_paths) {
-           path_id++) {
+        paths.emplace_back(path.path);
-        paths.emplace_back(cf.options.cf_paths[path_id].path);
       }
     }
 
@@ -2473,56 +2473,64 @@ Status DestroyDB(const std::string& dbname, const Options& options,
     std::sort(paths.begin(), paths.end());
     paths.erase(std::unique(paths.begin(), paths.end()), paths.end());
 
-    for (auto& path : paths) {
+    for (const auto& path : paths) {
-      env->GetChildren(path, &filenames);
+      if (env->GetChildren(path, &filenames).ok()) {
-      for (size_t i = 0; i < filenames.size(); i++) {
+        for (const auto& fname : filenames) {
-        if (ParseFileName(filenames[i], &number, &type) &&
+          if (ParseFileName(fname, &number, &type) &&
             type == kTableFile) {  // Lock file will be deleted at end
-          std::string table_path = path + "/" + filenames[i];
+            std::string table_path = path + "/" + fname;
-          Status del = DeleteSSTFile(&soptions, table_path, path);
+            Status del = DeleteSSTFile(&soptions, table_path, dbname);
-          if (result.ok() && !del.ok()) {
+            if (result.ok() && !del.ok()) {
-            result = del;
+              result = del;
+            }
           }
         }
+        env->DeleteDir(path);
       }
     }
 
     std::vector<std::string> walDirFiles;
     std::string archivedir = ArchivalDirectory(dbname);
+    bool wal_dir_exists = false;
     if (dbname != soptions.wal_dir) {
-      env->GetChildren(soptions.wal_dir, &walDirFiles);
+      wal_dir_exists = env->GetChildren(soptions.wal_dir, &walDirFiles).ok();
       archivedir = ArchivalDirectory(soptions.wal_dir);
     }
 
-    // Delete log files in the WAL dir
+    // Archive dir may be inside wal dir or dbname and should be
-    for (const auto& file : walDirFiles) {
+    // processed and removed before those otherwise we have issues
-      if (ParseFileName(file, &number, &type) && type == kLogFile) {
+    // removing them
-        Status del = env->DeleteFile(LogFileName(soptions.wal_dir, number));
-        if (result.ok() && !del.ok()) {
-          result = del;
-        }
-      }
-    }
-
     std::vector<std::string> archiveFiles;
-    env->GetChildren(archivedir, &archiveFiles);
+    if (env->GetChildren(archivedir, &archiveFiles).ok()) {
-    // Delete archival files.
+      // Delete archival files.
-    for (size_t i = 0; i < archiveFiles.size(); ++i) {
+      for (const auto& file : archiveFiles) {
-      if (ParseFileName(archiveFiles[i], &number, &type) && type == kLogFile) {
+        if (ParseFileName(file, &number, &type) &&
-        Status del = env->DeleteFile(archivedir + "/" + archiveFiles[i]);
+          type == kLogFile) {
-        if (result.ok() && !del.ok()) {
+          Status del = env->DeleteFile(archivedir + "/" + file);
-          result = del;
+          if (result.ok() && !del.ok()) {
+            result = del;
+          }
         }
       }
+      env->DeleteDir(archivedir);
     }
 
-    // ignore case where no archival directory is present
+    // Delete log files in the WAL dir
-    env->DeleteDir(archivedir);
+    if (wal_dir_exists) {
+      for (const auto& file : walDirFiles) {
+        if (ParseFileName(file, &number, &type) && type == kLogFile) {
+          Status del = env->DeleteFile(LogFileName(soptions.wal_dir, number));
+          if (result.ok() && !del.ok()) {
+            result = del;
+          }
+        }
+      }
+      env->DeleteDir(soptions.wal_dir);
+    }
 
     env->UnlockFile(lock);  // Ignore error since state is already gone
     env->DeleteFile(lockname);
     env->DeleteDir(dbname);  // Ignore error in case dir contains other files
-    env->DeleteDir(soptions.wal_dir);
   }
   return result;
 }

port/win/env_win.cc

@@ -515,15 +515,19 @@ Status WinEnvIO::CreateDir(const std::string& name) {
 Status  WinEnvIO::CreateDirIfMissing(const std::string& name) {
   Status result;
 
+  if (DirExists(name)) {
+    return result;
+  }
+
   BOOL ret = CreateDirectoryA(name.c_str(), NULL);
   if (!ret) {
     auto lastError = GetLastError();
     if (lastError != ERROR_ALREADY_EXISTS) {
       result = IOErrorFromWindowsError(
         "Failed to create a directory: " + name, lastError);
-    } else if (!DirExists(name)) {
+    } else {
       result =
-        Status::IOError("`" + name + "' exists but is not a directory");
+        Status::IOError(name + ": exists but is not a directory");
     }
   }
   return result;