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

diff --git a/db/db_filesnapshot.cc b/db/db_filesnapshot.cc
index f3d580d37e2387455ac957e3f170140aaa575977..f574dda3c38d266c7f72ee4a60253acd3d8a135c 100644 (file)
--- a/db/db_filesnapshot.cc
+++ b/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 (unsigned int i = 0; i < live.size(); i++, it++) {
-    ret[i] = TableFileName("", *it);
+  for (auto live_file : live) {
+    ret.push_back(TableFileName("", live_file));
   }
 
-  ret[live.size()] = CurrentFileName("");
-  ret[live.size()+1] = DescriptorFileName("",
-                                          versions_->ManifestFileNumber());
+  ret.push_back(CurrentFileName(""));
+  ret.push_back(DescriptorFileName("", versions_->ManifestFileNumber()));
 
   // find length of manifest file while holding the mutex lock
   *manifest_file_size = versions_->ManifestFileSize();
@@ -71,7 +70,7 @@ Status DBImpl::GetSortedWalFiles(VectorLogPtr& files) {
   // First get sorted files in archive dir, then append sorted files from main
   // dir to maintain sorted order
 
-  //  list wal files in archive dir.
+  // list wal files in archive dir.
   Status s;
   std::string archivedir = ArchivalDirectory(dbname_);
   if (env_->FileExists(archivedir)) {
@@ -81,11 +80,7 @@ Status DBImpl::GetSortedWalFiles(VectorLogPtr& files) {
     }
   }
   // list wal files in main db dir.
-  s = AppendSortedWalsOfType(dbname_, files, kAliveLogFile);
-  if (!s.ok()) {
-    return s;
-  }
-  return s;
+  return AppendSortedWalsOfType(dbname_, files, kAliveLogFile);
 }
 
 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.
-     * This is efficient because all except for very few wal files will be in
-     * archive. Checking for WalType is not much helpful because alive wal could
-       be archived now.
+    /* Try deleting in the dir that pathname points to for the logfile.
+       This may fail if we try to delete a log file which was live when captured
+       but is archived now. Try deleting it from archive also
      */
-    if (!env_->DeleteFile(archivedir + "/" + wal->Filename()).ok() &&
-        !env_->DeleteFile(dbname_ + "/" + wal->Filename()).ok()) {
-      files_not_deleted.append(wal->Filename());
+    Status st = env_->DeleteFile(dbname_ + "/" + wal->PathName());
+    if (!st.ok()) {
+      if (wal->Type() == kAliveLogFile &&
+          env_->DeleteFile(LogFileName(archivedir, wal->LogNumber())).ok()) {
+        continue;
+      }
+      files_not_deleted.append(wal->PathName());
     }
   }
   if (!files_not_deleted.empty()) {

diff --git a/db/db_impl.cc b/db/db_impl.cc
index d222323d78534507098f63caf69df8b7bf49dfb8..66e44a61135988ab1ad6a15e84eb5bcdc9ed96a7 100644 (file)
--- a/db/db_impl.cc
+++ b/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;
 }
 

diff --git a/db/filename.cc b/db/filename.cc
index 626a271077263ec6aed06400d51598dd6d60199f..5b107d9ab48d4f2b6a1bff882c2f8b4ae9e5b6a2 100644 (file)
--- a/db/filename.cc
+++ b/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) {

diff --git a/db/transaction_log_impl.h b/db/transaction_log_impl.h
index a880cabeea5941ca67c1020c8a1d678eb50a623a..a3ca1c00cab122018a6bd181a634c40183a38524 100644 (file)
--- a/db/transaction_log_impl.h
+++ b/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_; }
 

diff --git a/include/rocksdb/db.h b/include/rocksdb/db.h
index 1809a0cce99ef38df3f4092d3859e2dd36014001..5b4094fa59770ff3ef5b78bbb9cad0df7b013014 100644 (file)
--- a/include/rocksdb/db.h
+++ b/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.

diff --git a/include/rocksdb/transaction_log.h b/include/rocksdb/transaction_log.h
index 43a699206e454f59963a86f8ed39545492286d21..bde29004d411d50c30ce454800174c0456885fd8 100644 (file)
--- a/include/rocksdb/transaction_log.h
+++ b/include/rocksdb/transaction_log.h
@@ -27,8 +27,11 @@ class LogFile {
   LogFile() {}
   virtual ~LogFile() {}
 
-  // Returns log file's name excluding the db path
-  virtual std::string Filename() const = 0;
+  // Returns log file's pathname relative to the main db dir
+  // 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

diff --git a/include/utilities/stackable_db.h b/include/utilities/stackable_db.h
index aa6e6c556c6ad942a8bbc9d58d46a8ed83bac25e..739207b9cfb1d0bc9e29c69664bc8efdc6d7780d 100644 (file)
--- a/include/utilities/stackable_db.h
+++ b/include/utilities/stackable_db.h
@@ -146,9 +146,8 @@ class StackableDB : public DB {
     return sdb_->GetSortedWalFiles(files);
   }
 
-  virtual Status DeleteWalFiles(const VectorLogPtr& files)
-    override{
-      return sdb_->DeleteWalFiles(files);
+  virtual Status DeleteWalFiles(const VectorLogPtr& files) override {
+    return sdb_->DeleteWalFiles(files);
   }
 
   virtual Status GetUpdatesSince(SequenceNumber seq_number,