Remove LATEST_BACKUP file

Summary:
This has been unused since D42069 but kept around for backward
compatibility. I think it is unlikely anyone will use a much older version of
RocksDB for restore than they use for backup, so I propose removing it. It is
also causing recurring confusion, e.g., https://www.facebook.com/groups/rocksdb.dev/permalink/980454015386446/

Ported from https://reviews.facebook.net/D60735
Closes https://github.com/facebook/rocksdb/pull/1529

Differential Revision: D4194199

Pulled By: ajkr

fbshipit-source-id: 82f9bf4

HISTORY.md

@@ -11,6 +11,7 @@
 * Add avoid_flush_during_shutdown option, which speeds up DB shutdown by not flushing unpersisted data (i.e. with disableWAL = true). Unpersisted data will be lost. The options is dynamically changeable via SetDBOptions().
 * Add memtable_insert_with_hint_prefix_extractor option. The option is mean to reduce CPU usage for inserting keys into memtable, if keys can be group by prefix and insert for each prefix are sequential or almost sequential. See include/rocksdb/options.h for more details.
 * Add LuaCompactionFilter in utilities.  This allows developers to write compaction filters in Lua.  To use this feature, LUA_PATH needs to be set to the root directory of Lua.
+* No longer populate "LATEST_BACKUP" file in backup directory, which formerly contained the number of the latest backup. The latest backup can be determined by finding the highest numbered file in the "meta/" subdirectory.
 
 ## 4.13.0 (10/18/2016)
 ### Public API Change

utilities/backupable/backupable_db.cc

@@ -275,9 +275,6 @@ class BackupEngineImpl : public BackupEngine {
     return file_copy.erase(first_underscore,
                            file_copy.find_last_of('.') - first_underscore);
   }
-  inline std::string GetLatestBackupFile(bool tmp = false) const {
-    return GetAbsolutePath(std::string("LATEST_BACKUP") + (tmp ? ".tmp" : ""));
-  }
   inline std::string GetBackupMetaDir() const {
     return GetAbsolutePath("meta");
   }
@@ -285,8 +282,6 @@ class BackupEngineImpl : public BackupEngine {
     return GetBackupMetaDir() + "/" + rocksdb::ToString(backup_id);
   }
 
-  Status PutLatestBackupFileContents(uint32_t latest_backup);
-
   // If size_limit == 0, there is no size limit, copy everything.
   //
   // Exactly one of src and contents must be non-empty.
@@ -635,13 +630,6 @@ Status BackupEngineImpl::Initialize() {
 
   Log(options_.info_log, "Latest backup is %u", latest_backup_id_);
 
-  if (!read_only_) {
-    auto s = PutLatestBackupFileContents(latest_backup_id_);
-    if (!s.ok()) {
-      return s;
-    }
-  }
-
   // set up threads perform copies from files_to_copy_or_create_ in the
   // background
   for (int t = 0; t < options_.max_background_operations; t++) {
@@ -855,10 +843,6 @@ Status BackupEngineImpl::CreateNewBackupWithMetadata(
     // persist the backup metadata on the disk
     s = new_backup->StoreToFile(options_.sync);
   }
-  if (s.ok()) {
-    // install the newly created backup meta! (atomic)
-    s = PutLatestBackupFileContents(new_backup_id);
-  }
   if (s.ok() && options_.sync) {
     unique_ptr<Directory> backup_private_directory;
     backup_env_->NewDirectory(
@@ -1159,44 +1143,6 @@ Status BackupEngineImpl::VerifyBackup(BackupID backup_id) {
   return Status::OK();
 }
 
-// this operation HAS to be atomic
-// writing 4 bytes to the file is atomic alright, but we should *never*
-// do something like 1. delete file, 2. write new file
-// We write to a tmp file and then atomically rename
-Status BackupEngineImpl::PutLatestBackupFileContents(uint32_t latest_backup) {
-  assert(!read_only_);
-  Status s;
-  unique_ptr<WritableFile> file;
-  EnvOptions env_options;
-  env_options.use_mmap_writes = false;
-  s = backup_env_->NewWritableFile(GetLatestBackupFile(true),
-                                   &file,
-                                   env_options);
-  if (!s.ok()) {
-    backup_env_->DeleteFile(GetLatestBackupFile(true));
-    return s;
-  }
-
-  unique_ptr<WritableFileWriter> file_writer(
-      new WritableFileWriter(std::move(file), env_options));
-  char file_contents[10];
-  int len =
-      snprintf(file_contents, sizeof(file_contents), "%u\n", latest_backup);
-  s = file_writer->Append(Slice(file_contents, len));
-  if (s.ok() && options_.sync) {
-    file_writer->Sync(false);
-  }
-  if (s.ok()) {
-    s = file_writer->Close();
-  }
-  if (s.ok()) {
-    // atomically replace real file with new tmp
-    s = backup_env_->RenameFile(GetLatestBackupFile(true),
-                                GetLatestBackupFile(false));
-  }
-  return s;
-}
-
 Status BackupEngineImpl::CopyOrCreateFile(
     const std::string& src, const std::string& dst, const std::string& contents,
     Env* src_env, Env* dst_env, bool sync, RateLimiter* rate_limiter,

utilities/backupable/backupable_db_test.cc

@@ -202,7 +202,7 @@ class TestEnv : public EnvWrapper {
     std::sort(should_have_written.begin(), should_have_written.end());
     std::sort(written_files_.begin(), written_files_.end());
 
-    ASSERT_TRUE(written_files_ == should_have_written);
+    ASSERT_EQ(should_have_written, written_files_);
   }
 
   void ClearWrittenFiles() {
@@ -754,7 +754,7 @@ INSTANTIATE_TEST_CASE_P(BackupableDBTestWithParam, BackupableDBTestWithParam,
 TEST_F(BackupableDBTest, NoDoubleCopy) {
   OpenDBAndBackupEngine(true, true);
 
-  // should write 5 DB files + LATEST_BACKUP + one meta file
+  // should write 5 DB files + one meta file
   test_backup_env_->SetLimitWrittenFiles(7);
   test_backup_env_->ClearWrittenFiles();
   test_db_env_->SetLimitWrittenFiles(0);
@@ -766,12 +766,11 @@ TEST_F(BackupableDBTest, NoDoubleCopy) {
   std::vector<std::string> should_have_written = {
       "/shared/00010.sst.tmp",    "/shared/00011.sst.tmp",
       "/private/1.tmp/CURRENT",   "/private/1.tmp/MANIFEST-01",
-      "/private/1.tmp/00011.log", "/meta/1.tmp",
-      "/LATEST_BACKUP.tmp"};
+      "/private/1.tmp/00011.log", "/meta/1.tmp"};
   AppendPath(backupdir_, should_have_written);
   test_backup_env_->AssertWrittenFiles(should_have_written);
 
-  // should write 4 new DB files + LATEST_BACKUP + one meta file
+  // should write 4 new DB files + one meta file
   // should not write/copy 00010.sst, since it's already there!
   test_backup_env_->SetLimitWrittenFiles(6);
   test_backup_env_->ClearWrittenFiles();
@@ -783,12 +782,9 @@ TEST_F(BackupableDBTest, NoDoubleCopy) {
   ASSERT_OK(backup_engine_->CreateNewBackup(db_.get(), false));
   // should not open 00010.sst - it's already there
 
-  should_have_written = {"/shared/00015.sst.tmp",
-                         "/private/2.tmp/CURRENT",
+  should_have_written = {"/shared/00015.sst.tmp", "/private/2.tmp/CURRENT",
                          "/private/2.tmp/MANIFEST-01",
-                         "/private/2.tmp/00011.log",
-                         "/meta/2.tmp",
-                         "/LATEST_BACKUP.tmp"};
+                         "/private/2.tmp/00011.log", "/meta/2.tmp"};
   AppendPath(backupdir_, should_have_written);
   test_backup_env_->AssertWrittenFiles(should_have_written);
 
@@ -813,11 +809,10 @@ TEST_F(BackupableDBTest, NoDoubleCopy) {
 // test various kind of corruptions that may happen:
 // 1. Not able to write a file for backup - that backup should fail,
 //      everything else should work
-// 2. Corrupted/deleted LATEST_BACKUP - everything should work fine
-// 3. Corrupted backup meta file or missing backuped file - we should
+// 2. Corrupted backup meta file or missing backuped file - we should
 //      not be able to open that backup, but all other backups should be
 //      fine
-// 4. Corrupted checksum value - if the checksum is not a valid uint32_t,
+// 3. Corrupted checksum value - if the checksum is not a valid uint32_t,
 //      db open should fail, otherwise, it aborts during the restore process.
 TEST_F(BackupableDBTest, CorruptionsTest) {
   const int keys_iteration = 5000;
@@ -843,34 +838,8 @@ TEST_F(BackupableDBTest, CorruptionsTest) {
   CloseDBAndBackupEngine();
   AssertBackupConsistency(0, 0, keys_iteration * 5, keys_iteration * 6);
 
-  // ---------- case 2. - corrupt/delete latest backup -----------
-  ASSERT_OK(file_manager_->CorruptFile(backupdir_ + "/LATEST_BACKUP", 2));
-  AssertBackupConsistency(0, 0, keys_iteration * 5);
-  ASSERT_OK(file_manager_->DeleteFile(backupdir_ + "/LATEST_BACKUP"));
-  AssertBackupConsistency(0, 0, keys_iteration * 5);
-  // create backup 6, point LATEST_BACKUP to 5
-  // behavior change: this used to delete backup 6. however, now we ignore
-  // LATEST_BACKUP contents so BackupEngine sets latest backup to 6.
-  OpenDBAndBackupEngine();
-  FillDB(db_.get(), keys_iteration * 5, keys_iteration * 6);
-  ASSERT_OK(backup_engine_->CreateNewBackup(db_.get(), false));
-  CloseDBAndBackupEngine();
-  ASSERT_OK(file_manager_->WriteToFile(backupdir_ + "/LATEST_BACKUP", "5"));
-  AssertBackupConsistency(0, 0, keys_iteration * 6);
-  // assert that all 6 data is still here
-  ASSERT_OK(file_manager_->FileExists(backupdir_ + "/meta/6"));
-  ASSERT_OK(file_manager_->FileExists(backupdir_ + "/private/6"));
-  // assert that we wrote 6 to LATEST_BACKUP
-  {
-    std::string latest_backup_contents;
-    ReadFileToString(backup_chroot_env_.get(), backupdir_ + "/LATEST_BACKUP",
-                     &latest_backup_contents);
-    ASSERT_EQ(std::atol(latest_backup_contents.c_str()), 6);
-  }
-
-  // --------- case 3. corrupted backup meta or missing backuped file ----
+  // --------- case 2. corrupted backup meta or missing backuped file ----
   ASSERT_OK(file_manager_->CorruptFile(backupdir_ + "/meta/5", 3));
-  ASSERT_OK(file_manager_->CorruptFile(backupdir_ + "/meta/6", 3));
   // since 5 meta is now corrupted, latest backup should be 4
   AssertBackupConsistency(0, 0, keys_iteration * 4, keys_iteration * 5);
   OpenBackupEngine();
@@ -885,7 +854,7 @@ TEST_F(BackupableDBTest, CorruptionsTest) {
   CloseBackupEngine();
   ASSERT_TRUE(!s.ok());
 
-  // --------- case 4. corrupted checksum value ----
+  // --------- case 3. corrupted checksum value ----
   ASSERT_OK(file_manager_->CorruptChecksum(backupdir_ + "/meta/3", false));
   // checksum of backup 3 is an invalid value, this can be detected at
   // db open time, and it reverts to the previous backup automatically