From 99765ed85587ee76d11b0a6554f60b599d00d94f Mon Sep 17 00:00:00 2001 From: Ashish Shenoy Date: Mon, 23 May 2016 15:55:29 -0700 Subject: [PATCH] Clean up the ComputeCompactionScore() API Summary: Make CompactionOptionsFIFO a part of mutable_cf_options Test Plan: UT Reviewers: sdong Reviewed By: sdong Subscribers: andrewkr, lgalanis, dhruba Differential Revision: https://reviews.facebook.net/D58653 --- db/compaction_picker.cc | 12 ++---------- db/compaction_picker_test.cc | 2 +- db/db_impl.cc | 6 +----- db/db_impl_experimental.cc | 3 +-- db/version_set.cc | 8 +++----- db/version_set.h | 4 +--- util/mutable_cf_options.h | 4 +++- 7 files changed, 12 insertions(+), 27 deletions(-) diff --git a/db/compaction_picker.cc b/db/compaction_picker.cc index 794986e5b..2cf4db8a3 100644 --- a/db/compaction_picker.cc +++ b/db/compaction_picker.cc @@ -626,11 +626,7 @@ Compaction* CompactionPicker::CompactRange( // takes running compactions into account (by skipping files that are already // being compacted). Since we just changed compaction score, we recalculate it // here - { // this piece of code recomputes compaction score - CompactionOptionsFIFO dummy_compaction_options_fifo; - vstorage->ComputeCompactionScore(mutable_cf_options, - dummy_compaction_options_fifo); - } + vstorage->ComputeCompactionScore(mutable_cf_options); return compaction; } @@ -1038,11 +1034,7 @@ Compaction* LevelCompactionPicker::PickCompaction( // takes running compactions into account (by skipping files that are already // being compacted). Since we just changed compaction score, we recalculate it // here - { // this piece of code recomputes compaction score - CompactionOptionsFIFO dummy_compaction_options_fifo; - vstorage->ComputeCompactionScore(mutable_cf_options, - dummy_compaction_options_fifo); - } + vstorage->ComputeCompactionScore(mutable_cf_options); TEST_SYNC_POINT_CALLBACK("LevelCompactionPicker::PickCompaction:Return", c); diff --git a/db/compaction_picker_test.cc b/db/compaction_picker_test.cc index 571dca4a6..59c1c9f42 100644 --- a/db/compaction_picker_test.cc +++ b/db/compaction_picker_test.cc @@ -119,7 +119,7 @@ class CompactionPickerTest : public testing::Test { vstorage_->UpdateNumNonEmptyLevels(); vstorage_->GenerateFileIndexer(); vstorage_->GenerateLevelFilesBrief(); - vstorage_->ComputeCompactionScore(mutable_cf_options_, fifo_options_); + vstorage_->ComputeCompactionScore(mutable_cf_options_); vstorage_->GenerateLevel0NonOverlapping(); vstorage_->SetFinalized(); } diff --git a/db/db_impl.cc b/db/db_impl.cc index 86105a831..8d86691d6 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -2081,11 +2081,7 @@ Status DBImpl::CompactFilesImpl( // takes running compactions into account (by skipping files that are already // being compacted). Since we just changed compaction score, we recalculate it // here. - { - CompactionOptionsFIFO dummy_compaction_options_fifo; - version->storage_info()->ComputeCompactionScore( - *c->mutable_cf_options(), dummy_compaction_options_fifo); - } + version->storage_info()->ComputeCompactionScore(*c->mutable_cf_options()); compaction_job.Prepare(); diff --git a/db/db_impl_experimental.cc b/db/db_impl_experimental.cc index af3663e60..90e034cd0 100644 --- a/db/db_impl_experimental.cc +++ b/db/db_impl_experimental.cc @@ -49,8 +49,7 @@ Status DBImpl::SuggestCompactRange(ColumnFamilyHandle* column_family, } // Since we have some more files to compact, we should also recompute // compaction score - vstorage->ComputeCompactionScore(*cfd->GetLatestMutableCFOptions(), - CompactionOptionsFIFO()); + vstorage->ComputeCompactionScore(*cfd->GetLatestMutableCFOptions()); SchedulePendingCompaction(cfd); MaybeScheduleFlushOrCompaction(); } diff --git a/db/version_set.cc b/db/version_set.cc index 0381f736f..28a053ea6 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -1245,8 +1245,7 @@ void VersionStorageInfo::EstimateCompactionBytesNeeded( } void VersionStorageInfo::ComputeCompactionScore( - const MutableCFOptions& mutable_cf_options, - const CompactionOptionsFIFO& compaction_options_fifo) { + const MutableCFOptions& mutable_cf_options) { for (int level = 0; level <= MaxInputLevel(); level++) { double score; if (level == 0) { @@ -1282,7 +1281,7 @@ void VersionStorageInfo::ComputeCompactionScore( if (compaction_style_ == kCompactionStyleFIFO) { score = static_cast(total_size) / - compaction_options_fifo.max_table_files_size; + mutable_cf_options.compaction_options_fifo.max_table_files_size; } else { score = static_cast(num_sorted_runs) / mutable_cf_options.level0_file_num_compaction_trigger; @@ -2138,8 +2137,7 @@ void VersionSet::AppendVersion(ColumnFamilyData* column_family_data, Version* v) { // compute new compaction score v->storage_info()->ComputeCompactionScore( - *column_family_data->GetLatestMutableCFOptions(), - column_family_data->ioptions()->compaction_options_fifo); + *column_family_data->GetLatestMutableCFOptions()); // Mark v finalized v->storage_info_.SetFinalized(); diff --git a/db/version_set.h b/db/version_set.h index b30b37400..5482e56c2 100644 --- a/db/version_set.h +++ b/db/version_set.h @@ -121,9 +121,7 @@ class VersionStorageInfo { // We use compaction scores to figure out which compaction to do next // REQUIRES: db_mutex held!! // TODO find a better way to pass compaction_options_fifo. - void ComputeCompactionScore( - const MutableCFOptions& mutable_cf_options, - const CompactionOptionsFIFO& compaction_options_fifo); + void ComputeCompactionScore(const MutableCFOptions& mutable_cf_options); // Estimate est_comp_needed_bytes_ void EstimateCompactionBytesNeeded( diff --git a/util/mutable_cf_options.h b/util/mutable_cf_options.h index 28b35d30b..a0978070e 100644 --- a/util/mutable_cf_options.h +++ b/util/mutable_cf_options.h @@ -50,7 +50,8 @@ struct MutableCFOptions { paranoid_file_checks(options.paranoid_file_checks), report_bg_io_stats(options.report_bg_io_stats), compression(options.compression), - min_partial_merge_operands(options.min_partial_merge_operands) { + min_partial_merge_operands(options.min_partial_merge_operands), + compaction_options_fifo(ioptions.compaction_options_fifo) { RefreshDerivedOptions(ioptions); } MutableCFOptions() @@ -141,6 +142,7 @@ struct MutableCFOptions { bool report_bg_io_stats; CompressionType compression; uint32_t min_partial_merge_operands; + CompactionOptionsFIFO compaction_options_fifo; // Derived options // Per-level target file size.