ComputeCompactionScore in CompactionPicker

Summary:
As it turns out, we need the call to ComputeCompactionScore (previously: Finalize) in CompactionPicker.

The issue caused a deadlock in db_stress: http://ci-builds.fb.com/job/rocksdb_crashtest/290/console

The last two lines before a deadlock were:
2014/03/18-22:43:41.481029 7facafbee700 (Original Log Time 2014/03/18-22:43:41.480989) Compaction nothing to do
2014/03/18-22:43:41.481041 7faccf7fc700 wait for fewer level0 files...

"Compaction nothing to do" and other thread waiting for fewer level0 files. Hm hm.

I moved the pre-sorting to SaveTo, which should fix both the original and the new issue.

Test Plan: make check for now, will run db_stress in jenkins

Reviewers: dhruba, haobo, sdong

Reviewed By: haobo

CC: leveldb

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

db/compaction_picker.cc

@@ -377,6 +377,12 @@ Compaction* LevelCompactionPicker::PickCompaction(Version* version,
   Compaction* c = nullptr;
   int level = -1;
 
+  // Compute the compactions needed. It is better to do it here
+  // and also in LogAndApply(), otherwise the values could be stale.
+  std::vector<uint64_t> size_being_compacted(NumberLevels() - 1);
+  SizeBeingCompacted(size_being_compacted);
+  version->ComputeCompactionScore(size_being_compacted);
+
   // We prefer compactions triggered by too much data in a level over
   // the compactions triggered by seeks.
   //

db/version_set.cc

@@ -461,7 +461,6 @@ Version::Version(VersionSet* vset, uint64_t version_number)
       prev_(this),
       refs_(0),
       num_levels_(vset->num_levels_),
-      finalized_(false),
       files_(new std::vector<FileMetaData*>[num_levels_]),
       files_by_size_(num_levels_),
       next_file_to_compact_by_size_(num_levels_),
@@ -479,7 +478,6 @@ void Version::Get(const ReadOptions& options,
                   GetStats* stats,
                   const Options& db_options,
                   bool* value_found) {
-  assert(finalized_);
   Slice ikey = k.internal_key();
   Slice user_key = k.user_key();
   const Comparator* ucmp = vset_->icmp_.user_comparator();
@@ -643,16 +641,8 @@ bool Version::UpdateStats(const GetStats& stats) {
   return false;
 }
 
-void Version::Finalize(std::vector<uint64_t>& size_being_compacted) {
-  assert(!finalized_);
-  finalized_ = true;
-  // Pre-sort level0 for Get()
-  if (vset_->options_->compaction_style == kCompactionStyleUniversal) {
-    std::sort(files_[0].begin(), files_[0].end(), NewestFirstBySeqNo);
-  } else {
-    std::sort(files_[0].begin(), files_[0].end(), NewestFirst);
-  }
-
+void Version::ComputeCompactionScore(
+    std::vector<uint64_t>& size_being_compacted) {
   double max_score = 0;
   int max_score_level = 0;
 
@@ -1398,6 +1388,13 @@ class VersionSet::Builder {
       }
     }
 
+    // TODO(icanadi) do it in the loop above, which already sorts the files
+    // Pre-sort level0 for Get()
+    if (v->vset_->options_->compaction_style == kCompactionStyleUniversal) {
+      std::sort(v->files_[0].begin(), v->files_[0].end(), NewestFirstBySeqNo);
+    } else {
+      std::sort(v->files_[0].begin(), v->files_[0].end(), NewestFirst);
+    }
     CheckConsistency(v);
   }
 
@@ -1575,9 +1572,9 @@ Status VersionSet::LogAndApply(VersionEdit* edit, port::Mutex* mu,
       }
     }
 
-    // The calls to Finalize and UpdateFilesBySize are cpu-heavy
+    // The calls to ComputeCompactionScore and UpdateFilesBySize are cpu-heavy
     // and is best called outside the mutex.
-    v->Finalize(size_being_compacted);
+    v->ComputeCompactionScore(size_being_compacted);
     v->UpdateFilesBySize();
 
     // Write new record to MANIFEST log
@@ -1870,7 +1867,7 @@ Status VersionSet::Recover() {
     // Install recovered version
     std::vector<uint64_t> size_being_compacted(v->NumberLevels() - 1);
     compaction_picker_->SizeBeingCompacted(size_being_compacted);
-    v->Finalize(size_being_compacted);
+    v->ComputeCompactionScore(size_being_compacted);
 
     manifest_file_size_ = manifest_file_size;
     AppendVersion(v);
@@ -2074,7 +2071,7 @@ Status VersionSet::DumpManifest(Options& options, std::string& dscname,
     // Install recovered version
     std::vector<uint64_t> size_being_compacted(v->NumberLevels() - 1);
     compaction_picker_->SizeBeingCompacted(size_being_compacted);
-    v->Finalize(size_being_compacted);
+    v->ComputeCompactionScore(size_being_compacted);
 
     AppendVersion(v);
     manifest_file_number_ = next_file;

db/version_set.h

@@ -94,8 +94,9 @@ class Version {
 
   // Updates internal structures that keep track of compaction scores
   // We use compaction scores to figure out which compaction to do next
-  // Also pre-sorts level0 files for Get()
-  void Finalize(std::vector<uint64_t>& size_being_compacted);
+  // REQUIRES: If Version is not yet saved to current_, it can be called without
+  // a lock. Once a version is saved to current_, call only with mutex held
+  void ComputeCompactionScore(std::vector<uint64_t>& size_being_compacted);
 
   // Reference count management (so Versions do not disappear out from
   // under live iterators)
@@ -227,7 +228,6 @@ class Version {
   Version* prev_;               // Previous version in linked list
   int refs_;                    // Number of live refs to this version
   int num_levels_;              // Number of levels
-  bool finalized_;              // True if Finalized is called
 
   // List of files per level, files in each level are arranged
   // in increasing order of keys