Don't store version number in MANIFEST

Summary: Talked to <insert internal project name> folks and they found it really scary that they won't be able to roll back once they upgrade to 2.8. We should fix this.

Test Plan: make check

Reviewers: haobo, ljin

Reviewed By: ljin

CC: leveldb

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

Author: igorcanadi

HISTORY.md

@@ -10,7 +10,7 @@
 * Added "virtual void WaitForJoin()" in class Env. Default operation is no-op.
 * Removed BackupEngine::DeleteBackupsNewerThan() function
 * Added new option -- verify_checksums_in_compaction
-* Chagned Options.prefix_extractor from raw pointer to shared_ptr (take ownership)
+* Changed Options.prefix_extractor from raw pointer to shared_ptr (take ownership)
   Changed HashSkipListRepFactory and HashLinkListRepFactory constructor to not take SliceTransform object (use Options.prefix_extractor implicitly)
 * Added Env::GetThreadPoolQueueLen(), which returns the waiting queue length of thread pools
 * Added a command "checkconsistency" in ldb tool, which checks

db/db_impl.cc

@@ -568,7 +568,6 @@ uint64_t DBImpl::TEST_Current_Manifest_FileNo() {
 
 Status DBImpl::NewDB() {
   VersionEdit new_db;
-  new_db.SetVersionNumber();
   new_db.SetComparatorName(user_comparator()->Name());
   new_db.SetLogNumber(0);
   new_db.SetNextFile(2);

db/version_edit.cc

@@ -29,18 +29,15 @@ enum Tag {
 
   // these are new formats divergent from open source leveldb
   kNewFile2             = 100,  // store smallest & largest seqno
-  kVersionNumber        = 101,  // manifest version number, available after 2.8
 };
 
 void VersionEdit::Clear() {
-  version_number_ = 0;
   comparator_.clear();
   max_level_ = 0;
   log_number_ = 0;
   prev_log_number_ = 0;
   last_sequence_ = 0;
   next_file_number_ = 0;
-  has_version_number_ = false;
   has_comparator_ = false;
   has_log_number_ = false;
   has_prev_log_number_ = false;
@@ -51,10 +48,6 @@ void VersionEdit::Clear() {
 }
 
 void VersionEdit::EncodeTo(std::string* dst) const {
-  if (has_version_number_) {
-    PutVarint32(dst, kVersionNumber);
-    PutVarint32(dst, version_number_);
-  }
   if (has_comparator_) {
     PutVarint32(dst, kComparator);
     PutLengthPrefixedSlice(dst, comparator_);
@@ -133,14 +126,6 @@ Status VersionEdit::DecodeFrom(const Slice& src) {
 
   while (msg == nullptr && GetVarint32(&input, &tag)) {
     switch (tag) {
-      case kVersionNumber:
-        if (GetVarint32(&input, &version_number_)) {
-          has_version_number_ = true;
-        } else {
-          msg = "version number";
-        }
-        break;
-
       case kComparator:
         if (GetLengthPrefixedSlice(&input, &str)) {
           comparator_ = str.ToString();

db/version_edit.h

@@ -46,10 +46,6 @@ class VersionEdit {
 
   void Clear();
 
-  void SetVersionNumber() {
-    has_version_number_ = true;
-    version_number_ = kManifestVersion;
-  }
   void SetComparatorName(const Slice& name) {
     has_comparator_ = true;
     comparator_ = name.ToString();
@@ -114,13 +110,11 @@ class VersionEdit {
   bool GetLevel(Slice* input, int* level, const char** msg);
 
   int max_level_;
-  uint32_t version_number_;
   std::string comparator_;
   uint64_t log_number_;
   uint64_t prev_log_number_;
   uint64_t next_file_number_;
   SequenceNumber last_sequence_;
-  bool has_version_number_;
   bool has_comparator_;
   bool has_log_number_;
   bool has_prev_log_number_;
@@ -129,10 +123,6 @@ class VersionEdit {
 
   DeletedFileSet deleted_files_;
   std::vector<std::pair<int, FileMetaData> > new_files_;
-
-  enum {
-    kManifestVersion = 1
-  };
 };
 
 }  // namespace rocksdb

db/version_set.cc

@@ -1768,8 +1768,6 @@ Status VersionSet::Recover() {
     return s;
   }
 
-  bool have_version_number = false;
-  bool log_number_decrease = false;
   bool have_log_number = false;
   bool have_prev_log_number = false;
   bool have_next_file = false;
@@ -1810,17 +1808,15 @@ Status VersionSet::Recover() {
 
       builder.Apply(&edit);
 
-      if (edit.has_version_number_) {
-        have_version_number = true;
-      }
-
       // Only a flush's edit or a new snapshot can write log number during
       // LogAndApply. Since memtables are flushed and inserted into
       // manifest_writers_ queue in order, the log number in MANIFEST file
       // should be monotonically increasing.
       if (edit.has_log_number_) {
         if (have_log_number && log_number >= edit.log_number_) {
-          log_number_decrease = true;
+          Log(options_->info_log,
+              "decreasing of log_number is detected "
+              "in MANIFEST\n");
         } else {
           log_number = edit.log_number_;
           have_log_number = true;
@@ -1842,20 +1838,6 @@ Status VersionSet::Recover() {
         have_last_sequence = true;
       }
     }
-
-    if (s.ok() && log_number_decrease) {
-      // Since release 2.8, version number is added into MANIFEST file.
-      // Prior release 2.8, a bug in LogAndApply() can cause log_number
-      // to be smaller than the one from previous edit. To ensure backward
-      // compatibility, only fail for MANIFEST genearated by release 2.8
-      // and after.
-      if (have_version_number) {
-        s = Status::Corruption("log number decreases");
-      } else {
-        Log(options_->info_log, "decreasing of log_number is detected "
-            "in MANIFEST\n");
-      }
-    }
   }
 
   if (s.ok()) {
@@ -2125,7 +2107,6 @@ Status VersionSet::WriteSnapshot(log::Writer* log) {
 
   // Save metadata
   VersionEdit edit;
-  edit.SetVersionNumber();
   edit.SetComparatorName(icmp_.user_comparator()->Name());
 
   // Save files