Return pathname relative to db dir in LogFile and cleanup AppendSortedWalsOfType

Summary: So that replication can just download from wherever LogFile.Pathname is pointing them.

Test Plan: make all check;./db_repl_stress

Reviewers: dhruba, haobo

Reviewed By: dhruba

CC: leveldb

Differential Revision: https://reviews.facebook.net/D12609

db/db_filesnapshot.cc

@@ -48,18 +48,17 @@ Status DBImpl::GetLiveFiles(std::vector<std::string>& ret,
   std::set<uint64_t> live;
   versions_->AddLiveFilesCurrentVersion(&live);
 
-  ret.resize(live.size() + 2); //*.sst + CURRENT + MANIFEST
+  ret.clear();
+  ret.reserve(live.size() + 2); //*.sst + CURRENT + MANIFEST
 
   // create names of the live files. The names are not absolute
   // paths, instead they are relative to dbname_;
-  std::set<uint64_t>::iterator it = live.begin();
+  for (auto live_file : live) {
-  for (unsigned int i = 0; i < live.size(); i++, it++) {
+    ret.push_back(TableFileName("", live_file));
-    ret[i] = TableFileName("", *it);
   }
 
-  ret[live.size()] = CurrentFileName("");
+  ret.push_back(CurrentFileName(""));
-  ret[live.size()+1] = DescriptorFileName("",
+  ret.push_back(DescriptorFileName("", versions_->ManifestFileNumber()));
-                                          versions_->ManifestFileNumber());
 
   // find length of manifest file while holding the mutex lock
   *manifest_file_size = versions_->ManifestFileSize();
@@ -81,11 +80,7 @@ Status DBImpl::GetSortedWalFiles(VectorLogPtr& files) {
     }
   }
   // list wal files in main db dir.
-  s = AppendSortedWalsOfType(dbname_, files, kAliveLogFile);
+  return AppendSortedWalsOfType(dbname_, files, kAliveLogFile);
-  if (!s.ok()) {
-    return s;
-  }
-  return s;
 }
 
 Status DBImpl::DeleteWalFiles(const VectorLogPtr& files) {
@@ -93,14 +88,17 @@ Status DBImpl::DeleteWalFiles(const VectorLogPtr& files) {
   std::string archivedir = ArchivalDirectory(dbname_);
   std::string files_not_deleted;
   for (const auto& wal : files) {
-    /* Try deleting in archive dir. If fails, try deleting in main db dir.
+    /* Try deleting in the dir that pathname points to for the logfile.
-     * This is efficient because all except for very few wal files will be in
+       This may fail if we try to delete a log file which was live when captured
-     * archive. Checking for WalType is not much helpful because alive wal could
+       but is archived now. Try deleting it from archive also
-       be archived now.
      */
-    if (!env_->DeleteFile(archivedir + "/" + wal->Filename()).ok() &&
+    Status st = env_->DeleteFile(dbname_ + "/" + wal->PathName());
-        !env_->DeleteFile(dbname_ + "/" + wal->Filename()).ok()) {
+    if (!st.ok()) {
-      files_not_deleted.append(wal->Filename());
+      if (wal->Type() == kAliveLogFile &&
+          env_->DeleteFile(LogFileName(archivedir, wal->LogNumber())).ok()) {
+        continue;
+      }
+      files_not_deleted.append(wal->PathName());
     }
   }
   if (!files_not_deleted.empty()) {

db/db_impl.cc

@@ -1213,6 +1213,12 @@ Status DBImpl::AppendSortedWalsOfType(const std::string& path,
     return status;
   }
   log_files.reserve(log_files.size() + all_files.size());
+  VectorLogPtr::iterator pos_start;
+  if (!log_files.empty()) {
+    pos_start = log_files.end() - 1;
+  } else {
+    pos_start = log_files.begin();
+  }
   for (const auto& f : all_files) {
     uint64_t number;
     FileType type;
@@ -1238,7 +1244,7 @@ Status DBImpl::AppendSortedWalsOfType(const std::string& path,
     }
   }
   CompareLogByPointer compare_log_files;
-  std::sort(log_files.begin(), log_files.end(), compare_log_files);
+  std::sort(pos_start, log_files.end(), compare_log_files);
   return status;
 }
 

db/filename.cc

@@ -62,7 +62,7 @@ std::string ArchivalDirectory(const std::string& dbname) {
 }
 std::string ArchivedLogFileName(const std::string& name, uint64_t number) {
   assert(number > 0);
-  return MakeFileName(name + "/archive", number, "log");
+  return MakeFileName(name + "/" + ARCHIVAL_DIR, number, "log");
 }
 
 std::string TableFileName(const std::string& name, uint64_t number) {

db/transaction_log_impl.h

@@ -31,7 +31,12 @@ class LogFileImpl : public LogFile {
     sizeFileBytes_(sizeBytes) {
   }
 
-  std::string Filename() const { return LogFileName("", logNumber_); }
+  std::string PathName() const {
+    if (type_ == kArchivedLogFile) {
+      return ArchivedLogFileName("", logNumber_);
+    }
+    return LogFileName("", logNumber_);
+  }
 
   uint64_t LogNumber() const { return logNumber_; }
 

include/rocksdb/db.h

@@ -251,7 +251,6 @@ class DB {
 
   // Delete wal files in files. These can be either live or archived.
   // Returns Status::OK if all files could be deleted, otherwise Status::IOError
-  // which contains information about files that could not be deleted.
   virtual Status DeleteWalFiles(const VectorLogPtr& files) = 0;
 
   // The sequence number of the most recent transaction.

include/rocksdb/transaction_log.h

@@ -27,8 +27,11 @@ class LogFile {
   LogFile() {}
   virtual ~LogFile() {}
 
-  // Returns log file's name excluding the db path
+  // Returns log file's pathname relative to the main db dir
-  virtual std::string Filename() const = 0;
+  // Eg. For a live-log-file = /000003.log
+  //     For an archived-log-file = /archive/000003.log
+  virtual std::string PathName() const = 0;
+
 
   // Primary identifier for log file.
   // This is directly proportional to creation time of the log file

include/utilities/stackable_db.h

@@ -146,8 +146,7 @@ class StackableDB : public DB {
     return sdb_->GetSortedWalFiles(files);
   }
 
-  virtual Status DeleteWalFiles(const VectorLogPtr& files)
+  virtual Status DeleteWalFiles(const VectorLogPtr& files) override {
-    override{
     return sdb_->DeleteWalFiles(files);
   }