From 2f4fc539c642f16699ee9425130166844d570bf3 Mon Sep 17 00:00:00 2001 From: Artemiy Kolesnikov Date: Wed, 7 Dec 2016 11:42:49 -0800 Subject: [PATCH] Compaction::IsTrivialMove relaxing Summary: IsTrivialMove returns true if no input file overlaps with output_level+1 with more than max_compaction_bytes_ bytes. Closes https://github.com/facebook/rocksdb/pull/1619 Differential Revision: D4278338 Pulled By: yiwu-arbug fbshipit-source-id: 994c001 --- db/compaction.cc | 51 ++++++++++++++++++++++++------------ db/compaction.h | 2 ++ db/compaction_picker.h | 6 ++--- db/compaction_picker_test.cc | 44 +++++++++++++++++++++++++++++++ 4 files changed, 83 insertions(+), 20 deletions(-) diff --git a/db/compaction.cc b/db/compaction.cc index d168cf75d..2315be5ef 100644 --- a/db/compaction.cc +++ b/db/compaction.cc @@ -150,7 +150,8 @@ Compaction::Compaction(VersionStorageInfo* vstorage, bool _manual_compaction, double _score, bool _deletion_compaction, CompactionReason _compaction_reason) - : start_level_(_inputs[0].level), + : input_vstorage_(vstorage), + start_level_(_inputs[0].level), output_level_(_output_level), max_output_file_size_(_target_file_size), max_compaction_bytes_(_max_compaction_bytes), @@ -204,11 +205,10 @@ Compaction::~Compaction() { } bool Compaction::InputCompressionMatchesOutput() const { - VersionStorageInfo* vstorage = input_version_->storage_info(); - int base_level = vstorage->base_level(); - bool matches = - (GetCompressionType(*cfd_->ioptions(), vstorage, mutable_cf_options_, - start_level_, base_level) == output_compression_); + int base_level = input_vstorage_->base_level(); + bool matches = (GetCompressionType(immutable_cf_options_, input_vstorage_, + mutable_cf_options_, start_level_, + base_level) == output_compression_); if (matches) { TEST_SYNC_POINT("Compaction::InputCompressionMatchesOutput:Matches"); return true; @@ -225,15 +225,14 @@ bool Compaction::IsTrivialMove() const { // filter to be applied to that level, and thus cannot be a trivial move. // Check if start level have files with overlapping ranges - if (start_level_ == 0 && - input_version_->storage_info()->level0_non_overlapping() == false) { + if (start_level_ == 0 && input_vstorage_->level0_non_overlapping() == false) { // We cannot move files from L0 to L1 if the files are overlapping return false; } if (is_manual_compaction_ && - (cfd_->ioptions()->compaction_filter != nullptr || - cfd_->ioptions()->compaction_filter_factory != nullptr)) { + (immutable_cf_options_.compaction_filter != nullptr || + immutable_cf_options_.compaction_filter_factory != nullptr)) { // This is a manual compaction and we have a compaction filter that should // be executed, we cannot do a trivial move return false; @@ -241,15 +240,34 @@ bool Compaction::IsTrivialMove() const { // Used in universal compaction, where trivial move can be done if the // input files are non overlapping - if ((cfd_->ioptions()->compaction_options_universal.allow_trivial_move) && + if ((immutable_cf_options_.compaction_options_universal.allow_trivial_move) && (output_level_ != 0)) { return is_trivial_move_; } - return (start_level_ != output_level_ && num_input_levels() == 1 && + if (!(start_level_ != output_level_ && num_input_levels() == 1 && input(0, 0)->fd.GetPathId() == output_path_id() && - InputCompressionMatchesOutput() && - TotalFileSize(grandparents_) <= max_compaction_bytes_); + InputCompressionMatchesOutput())) { + return false; + } + + // assert inputs_.size() == 1 + + for (const auto& file : inputs_.front().files) { + std::vector file_grand_parents; + if (output_level_ + 1 >= number_levels_) { + continue; + } + input_vstorage_->GetOverlappingInputs(output_level_ + 1, &file->smallest, + &file->largest, &file_grand_parents); + const auto compaction_size = + file->fd.GetFileSize() + TotalFileSize(file_grand_parents); + if (compaction_size > max_compaction_bytes_) { + return false; + } + } + + return true; } void Compaction::AddInputDeletions(VersionEdit* out_edit) { @@ -272,8 +290,7 @@ bool Compaction::KeyNotExistsBeyondOutputLevel( // Maybe use binary search to find right entry instead of linear search? const Comparator* user_cmp = cfd_->user_comparator(); for (int lvl = output_level_ + 1; lvl < number_levels_; lvl++) { - const std::vector& files = - input_version_->storage_info()->LevelFiles(lvl); + const std::vector& files = input_vstorage_->LevelFiles(lvl); for (; level_ptrs->at(lvl) < files.size(); level_ptrs->at(lvl)++) { auto* f = files[level_ptrs->at(lvl)]; if (user_cmp->Compare(user_key, f->largest.user_key()) <= 0) { @@ -345,7 +362,7 @@ void Compaction::ReleaseCompactionFiles(Status status) { void Compaction::ResetNextCompactionIndex() { assert(input_version_ != nullptr); - input_version_->storage_info()->ResetNextCompactionIndex(start_level_); + input_vstorage_->ResetNextCompactionIndex(start_level_); } namespace { diff --git a/db/compaction.h b/db/compaction.h index f0edee841..c2656b77b 100644 --- a/db/compaction.h +++ b/db/compaction.h @@ -258,6 +258,8 @@ class Compaction { static bool IsFullCompaction(VersionStorageInfo* vstorage, const std::vector& inputs); + VersionStorageInfo* input_vstorage_; + const int start_level_; // the lowest level to be compacted const int output_level_; // levels to which output files are stored uint64_t max_output_file_size_; diff --git a/db/compaction_picker.h b/db/compaction_picker.h index 7f3918fe4..58f0ccd4d 100644 --- a/db/compaction_picker.h +++ b/db/compaction_picker.h @@ -119,9 +119,6 @@ class CompactionPicker { const Slice& largest_user_key, int level) const; - protected: - int NumberLevels() const { return ioptions_.num_levels; } - // Stores the minimal range that covers all entries in inputs in // *smallest, *largest. // REQUIRES: inputs is not empty @@ -141,6 +138,9 @@ class CompactionPicker { void GetRange(const std::vector& inputs, InternalKey* smallest, InternalKey* largest) const; + protected: + int NumberLevels() const { return ioptions_.num_levels; } + // Add more files to the inputs on "level" to make sure that // no newer version of a key is compacted to "level+1" while leaving an older // version in a "level". Otherwise, any Get() will search "level" first, diff --git a/db/compaction_picker_test.cc b/db/compaction_picker_test.cc index 5371172fc..ca2ee9393 100644 --- a/db/compaction_picker_test.cc +++ b/db/compaction_picker_test.cc @@ -1177,6 +1177,50 @@ TEST_F(CompactionPickerTest, MaxCompactionBytesNotHit) { ASSERT_EQ(5U, compaction->input(1, 0)->fd.GetNumber()); } +TEST_F(CompactionPickerTest, IsTrivialMoveOn) { + mutable_cf_options_.max_bytes_for_level_base = 10000u; + mutable_cf_options_.max_compaction_bytes = 10001u; + ioptions_.level_compaction_dynamic_level_bytes = false; + NewVersionStorage(6, kCompactionStyleLevel); + // A compaction should be triggered and pick file 2 + Add(1, 1U, "100", "150", 3000U); + Add(1, 2U, "151", "200", 3001U); + Add(1, 3U, "201", "250", 3000U); + Add(1, 4U, "251", "300", 3000U); + + Add(3, 5U, "120", "130", 7000U); + Add(3, 6U, "170", "180", 7000U); + Add(3, 5U, "220", "230", 7000U); + Add(3, 5U, "270", "280", 7000U); + UpdateVersionStorageInfo(); + + std::unique_ptr compaction(level_compaction_picker.PickCompaction( + cf_name_, mutable_cf_options_, vstorage_.get(), &log_buffer_)); + ASSERT_TRUE(compaction.get() != nullptr); + ASSERT_TRUE(compaction->IsTrivialMove()); +} + +TEST_F(CompactionPickerTest, IsTrivialMoveOff) { + mutable_cf_options_.max_bytes_for_level_base = 1000000u; + mutable_cf_options_.max_compaction_bytes = 10000u; + ioptions_.level_compaction_dynamic_level_bytes = false; + NewVersionStorage(6, kCompactionStyleLevel); + // A compaction should be triggered and pick all files from level 1 + Add(1, 1U, "100", "150", 300000U, 0, 0); + Add(1, 2U, "150", "200", 300000U, 0, 0); + Add(1, 3U, "200", "250", 300000U, 0, 0); + Add(1, 4U, "250", "300", 300000U, 0, 0); + + Add(3, 5U, "120", "130", 6000U); + Add(3, 6U, "140", "150", 6000U); + UpdateVersionStorageInfo(); + + std::unique_ptr compaction(level_compaction_picker.PickCompaction( + cf_name_, mutable_cf_options_, vstorage_.get(), &log_buffer_)); + ASSERT_TRUE(compaction.get() != nullptr); + ASSERT_FALSE(compaction->IsTrivialMove()); +} + } // namespace rocksdb int main(int argc, char** argv) {