From 6c2ca1d3e623900d8403181bc617753ea7cb99b3 Mon Sep 17 00:00:00 2001 From: Igor Canadi Date: Mon, 27 Jan 2014 09:59:00 -0800 Subject: [PATCH] Move NeedsCompaction() from VersionSet to Version Summary: There is no reason to have functions NeedCompaction(), MaxCompactionScore() and MaxCompactionScoreLevel() in VersionSet, since they don't access any data in VersionSet. Test Plan: make check Reviewers: kailiu, haobo, sdong Reviewed By: kailiu CC: leveldb Differential Revision: https://reviews.facebook.net/D15333 --- db/db_impl.cc | 18 ++++++++---------- db/version_set.cc | 22 ++++++++++++++++++++++ db/version_set.h | 45 +++++++++------------------------------------ 3 files changed, 39 insertions(+), 46 deletions(-) diff --git a/db/db_impl.cc b/db/db_impl.cc index 0bed3bc28..d84e51699 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -1776,7 +1776,7 @@ void DBImpl::MaybeScheduleFlushOrCompaction() { // max_background_compactions hasn't been reached and, in case // bg_manual_only_ > 0, if it's a manual compaction. if ((manual_compaction_ || - versions_->NeedsCompaction() || + versions_->current()->NeedsCompaction() || (is_flush_pending && (options_.max_background_flushes <= 0))) && bg_compaction_scheduled_ < options_.max_background_compactions && (!bg_manual_only_ || manual_compaction_)) { @@ -3350,12 +3350,11 @@ Status DBImpl::MakeRoomForWrite(bool force, RecordTick(options_.statistics.get(), STALL_L0_NUM_FILES_MICROS, stall); stall_level0_num_files_ += stall; stall_level0_num_files_count_++; - } else if ( - allow_hard_rate_limit_delay && - options_.hard_rate_limit > 1.0 && - (score = versions_->MaxCompactionScore()) > options_.hard_rate_limit) { + } else if (allow_hard_rate_limit_delay && options_.hard_rate_limit > 1.0 && + (score = versions_->current()->MaxCompactionScore()) > + options_.hard_rate_limit) { // Delay a write when the compaction score for any level is too large. - int max_level = versions_->MaxCompactionScoreLevel(); + int max_level = versions_->current()->MaxCompactionScoreLevel(); mutex_.Unlock(); uint64_t delayed; { @@ -3377,10 +3376,9 @@ Status DBImpl::MakeRoomForWrite(bool force, allow_hard_rate_limit_delay = false; } mutex_.Lock(); - } else if ( - allow_soft_rate_limit_delay && - options_.soft_rate_limit > 0.0 && - (score = versions_->MaxCompactionScore()) > options_.soft_rate_limit) { + } else if (allow_soft_rate_limit_delay && options_.soft_rate_limit > 0.0 && + (score = versions_->current()->MaxCompactionScore()) > + options_.soft_rate_limit) { // Delay a write when the compaction score for any level is too large. // TODO: add statistics mutex_.Unlock(); diff --git a/db/version_set.cc b/db/version_set.cc index db5f9151a..145a99114 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -756,6 +756,28 @@ void Version::Unref() { } } +bool Version::NeedsCompaction() const { + if (file_to_compact_ != nullptr) { + return true; + } + // In universal compaction case, this check doesn't really + // check the compaction condition, but checks num of files threshold + // only. We are not going to miss any compaction opportunity + // but it's likely that more compactions are scheduled but + // ending up with nothing to do. We can improve it later. + // TODO(sdong): improve this function to be accurate for universal + // compactions. + int num_levels_to_check = + (vset_->options_->compaction_style != kCompactionStyleUniversal) ? + NumberLevels() - 1 : 1; + for (int i = 0; i < num_levels_to_check; i++) { + if (compaction_score_[i] >= 1) { + return true; + } + } + return false; +} + bool Version::OverlapInLevel(int level, const Slice* smallest_user_key, const Slice* largest_user_key) { diff --git a/db/version_set.h b/db/version_set.h index b0922d319..6b91355f7 100644 --- a/db/version_set.h +++ b/db/version_set.h @@ -99,6 +99,15 @@ class Version { void Ref(); void Unref(); + // Returns true iff some level needs a compaction. + bool NeedsCompaction() const; + + // Returns the maxmimum compaction score for levels 1 to max + double MaxCompactionScore() const { return max_compaction_score_; } + + // See field declaration + int MaxCompactionScoreLevel() const { return max_compaction_score_level_; } + void GetOverlappingInputs( int level, const InternalKey* begin, // nullptr means before all keys @@ -368,42 +377,6 @@ class VersionSet { // The caller should delete the iterator when no longer needed. Iterator* MakeInputIterator(Compaction* c); - // Returns true iff some level needs a compaction because it has - // exceeded its target size. - bool NeedsSizeCompaction() const { - // In universal compaction case, this check doesn't really - // check the compaction condition, but checks num of files threshold - // only. We are not going to miss any compaction opportunity - // but it's likely that more compactions are scheduled but - // ending up with nothing to do. We can improve it later. - // TODO: improve this function to be accurate for universal - // compactions. - int num_levels_to_check = - (options_->compaction_style != kCompactionStyleUniversal) ? - NumberLevels() - 1 : 1; - for (int i = 0; i < num_levels_to_check; i++) { - if (current_->compaction_score_[i] >= 1) { - return true; - } - } - return false; - } - // Returns true iff some level needs a compaction. - bool NeedsCompaction() const { - return ((current_->file_to_compact_ != nullptr) || - NeedsSizeCompaction()); - } - - // Returns the maxmimum compaction score for levels 1 to max - double MaxCompactionScore() const { - return current_->max_compaction_score_; - } - - // See field declaration - int MaxCompactionScoreLevel() const { - return current_->max_compaction_score_level_; - } - // Add all files listed in any live version to *live. void AddLiveFiles(std::vector* live_list);