Merge pull request #5 from XiaoMi/qinzuoyan

not return NoNeedOperate in FlushMemTable() & copy options file in CreateCheckpointQuick()

Author: qinzuoyan

db/db_compaction_test.cc

@@ -1946,7 +1946,7 @@ TEST_P(DBCompactionTestWithParam, ConvertCompactionStyle) {
     ASSERT_OK(Put(Key(i), RandomString(&rnd, 10000)));
   }
   dbfull()->Flush(FlushOptions());
-  ASSERT_TRUE(Flush().IsNoNeedOperate());
+  ASSERT_OK(Flush(1));
   dbfull()->TEST_WaitForCompact();
 
   for (int i = 1; i < options.num_levels; i++) {

db/db_filesnapshot.cc

@@ -177,12 +177,15 @@ Status DBImpl::GetLiveFilesQuick(std::vector<std::string>& ret,
   }
 
   ret.clear();
-  ret.reserve(live.size() + 2); //*.sst + CURRENT + MANIFEST
+  ret.reserve(live.size() + 3); //*.sst + CURRENT + MANIFEST + OPTIONS
 
   // Put current and manifest files firstly to make them copied quickly,
   // because the manifest file may be deleted when copying sstables.
+  // TODO(qinzuoyan): but now it may be not necessary because the manifest file
+  // must not be deleted when copying sstables..
   ret.push_back(CurrentFileName(""));
   ret.push_back(DescriptorFileName("", versions_->manifest_file_number()));
+  ret.push_back(OptionsFileName("", versions_->options_file_number()));
 
   // create names of the live files. The names are not absolute
   // paths, instead they are relative to dbname_;

db/db_impl_compaction_flush.cc

@@ -288,8 +288,7 @@ Status DBImpl::CompactRange(const CompactRangeOptions& options,
   bool exclusive = options.exclusive_manual_compaction;
 
   Status s = FlushMemTable(cfd, FlushOptions());
-  // NOTE: FlushMemTable will return NoNeedOperate when memtable is empty
-  if (!s.ok() && !s.IsNoNeedOperate()) {
+  if (!s.ok()) {
     LogFlush(immutable_db_options_.info_log);
     return s;
   }
@@ -957,7 +956,7 @@ Status DBImpl::FlushMemTable(ColumnFamilyData* cfd,
     if (cfd->imm()->NumNotFlushed() == 0 && cfd->mem()->IsEmpty() &&
         cached_recoverable_state_empty_.load()) {
       // Nothing to flush
-      return Status::NoNeedOperate();
+      return Status::OK();
     }
 
     if (!cfd->mem()->IsEmpty()) {

db/external_sst_file_test.cc

@@ -349,7 +349,7 @@ TEST_F(ExternalSSTFileTest, Basic) {
         std::string value = Key(k) + "_val";
         ASSERT_EQ(Get(Key(k)), value);
       }
-      ASSERT_TRUE(Flush().ok() || Flush().IsNoNeedOperate());
+      ASSERT_TRUE(Flush().ok());
       ASSERT_OK(db_->CompactRange(CompactRangeOptions(), nullptr, nullptr));
     }
 
@@ -593,7 +593,7 @@ TEST_F(ExternalSSTFileTest, AddList) {
         std::string value = Key(k) + "_val";
         ASSERT_EQ(Get(Key(k)), value);
       }
-      ASSERT_TRUE(Flush().ok() || Flush().IsNoNeedOperate());
+      ASSERT_TRUE(Flush().ok());
       ASSERT_OK(db_->CompactRange(CompactRangeOptions(), nullptr, nullptr));
     }
 
@@ -884,7 +884,7 @@ TEST_F(ExternalSSTFileTest, MultiThreaded) {
         std::string value = (k % 100 == 0) ? (key + "_new") : key;
         ASSERT_EQ(Get(key), value);
       }
-      ASSERT_TRUE(Flush().ok() || Flush().IsNoNeedOperate());
+      ASSERT_TRUE(Flush().ok());
       ASSERT_OK(db_->CompactRange(CompactRangeOptions(), nullptr, nullptr));
     }
 

include/rocksdb/status.h

@@ -59,8 +59,6 @@ class Status {
     kBusy = 11,
     kExpired = 12,
     kTryAgain = 13,
-
-    kNoNeedOperate = 101,
   };
 
   Code code() const { return code_; }
@@ -174,11 +172,6 @@ class Status {
     return Status(kAborted, kMemoryLimit, msg, msg2);
   }
 
-  static Status NoNeedOperate(SubCode msg = kNone) { return Status(kNoNeedOperate, msg); }
-  static Status NoNeedOperate(const Slice& msg, const Slice& msg2 = Slice()) {
-    return Status(kNoNeedOperate, msg, msg2);
-  }
-
   // Returns true iff the status indicates success.
   bool ok() const { return code() == kOk; }
 
@@ -228,10 +221,6 @@ class Status {
   // re-attempted.
   bool IsTryAgain() const { return code() == kTryAgain; }
 
-  // Returns true iff the status indicates a NoNeedOperate error.
-  // This usually means that the operation is not need to do.
-  bool IsNoNeedOperate() const { return code() == kNoNeedOperate; }
-
   // Returns true iff the status indicates a NoSpace error
   // This is caused by an I/O error returning the specific "out of space"
   // error condition. Stricto sensu, an NoSpace error is an I/O error

util/status.cc

@@ -84,9 +84,6 @@ std::string Status::ToString() const {
     case kTryAgain:
       type = "Operation failed. Try again.: ";
       break;
-    case kNoNeedOperate:
-      type = "No need to operate: ";
-      break;
     default:
       snprintf(tmp, sizeof(tmp), "Unknown code(%d): ",
                static_cast<int>(code()));

utilities/checkpoint/checkpoint_impl.cc

@@ -442,9 +442,9 @@ Status CheckpointImpl::CreateCheckpointQuick(const std::string& checkpoint_dir,
       s = Status::Corruption("Can't parse file name. This is very bad");
       break;
     }
-    // we should only get sst, manifest and current files here
+    // we should only get sst, manifest, current and options files here
     assert(type == kTableFile || type == kDescriptorFile ||
-           type == kCurrentFile);
+           type == kCurrentFile || type == kOptionsFile);
     assert(live_files[i].size() > 0 && live_files[i][0] == '/');
     const std::string& src_fname = live_files[i];
 

utilities/checkpoint/checkpoint_test.cc

@@ -438,7 +438,7 @@ TEST_F(CheckpointTest, CurrentFileModifiedWhileCheckpointing) {
   rocksdb::port::Thread t([&]() {
     Checkpoint* checkpoint;
     ASSERT_OK(Checkpoint::Create(db_, &checkpoint));
-    ASSERT_EQ(Status::NoNeedOperate(), checkpoint->CreateCheckpoint(kSnapshotName));
+    ASSERT_OK(checkpoint->CreateCheckpoint(kSnapshotName));
     delete checkpoint;
   });
   TEST_SYNC_POINT(