From 10ddd59ba7007902a26c8078d1e5488caa5cee64 Mon Sep 17 00:00:00 2001 From: Andrew Kryczka Date: Wed, 6 Sep 2017 11:28:19 -0700 Subject: [PATCH] fix CompactFiles inclusion of older L0 files Summary: if we're moving any L0 files down, we need to include older L0 files since they may contain older versions of the keys being moved down. Closes https://github.com/facebook/rocksdb/pull/2845 Differential Revision: D5773800 Pulled By: ajkr fbshipit-source-id: 9f0770a8eaaeea4c87df2e7a2a1d65bf9d7f4f7e --- db/compaction_picker.cc | 6 ++---- db/db_compaction_test.cc | 22 ++++++++++++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/db/compaction_picker.cc b/db/compaction_picker.cc index c6a56746f..961231e94 100644 --- a/db/compaction_picker.cc +++ b/db/compaction_picker.cc @@ -730,10 +730,6 @@ Status CompactionPicker::SanitizeCompactionInputFilesForAllLevels( auto& levels = cf_meta.levels; auto comparator = icmp_->user_comparator(); - // TODO(yhchiang): If there is any input files of L1 or up and there - // is at least one L0 files. All L0 files older than the L0 file needs - // to be included. Otherwise, it is a false conditoin - // TODO(yhchiang): add is_adjustable to CompactionOptions // the smallest and largest key of the current compaction input @@ -794,6 +790,8 @@ Status CompactionPicker::SanitizeCompactionInputFilesForAllLevels( } last_included++; } + } else if (output_level > 0) { + last_included = static_cast(current_files.size() - 1); } // include all files between the first and the last compaction input files. diff --git a/db/db_compaction_test.cc b/db/db_compaction_test.cc index ca77d5b93..64028768d 100644 --- a/db/db_compaction_test.cc +++ b/db/db_compaction_test.cc @@ -2724,6 +2724,28 @@ TEST_F(DBCompactionTest, OptimizedDeletionObsoleting) { options.statistics->getTickerCount(COMPACTION_KEY_DROP_OBSOLETE)); } +TEST_F(DBCompactionTest, CompactFilesOverlapInL0Bug) { + // Regression test for bug of not pulling in L0 files that overlap the user- + // specified input files in time- and key-ranges. + Put(Key(0), "old_val"); + Flush(); + Put(Key(0), "new_val"); + Flush(); + + ColumnFamilyMetaData cf_meta; + dbfull()->GetColumnFamilyMetaData(dbfull()->DefaultColumnFamily(), &cf_meta); + ASSERT_GE(cf_meta.levels.size(), 2); + ASSERT_EQ(2, cf_meta.levels[0].files.size()); + + // Compacting {new L0 file, L1 file} should pull in the old L0 file since it + // overlaps in key-range and time-range. + std::vector input_filenames; + input_filenames.push_back(cf_meta.levels[0].files.front().name); + ASSERT_OK(dbfull()->CompactFiles(CompactionOptions(), input_filenames, + 1 /* output_level */)); + ASSERT_EQ("new_val", Get(Key(0))); +} + INSTANTIATE_TEST_CASE_P(DBCompactionTestWithParam, DBCompactionTestWithParam, ::testing::Values(std::make_tuple(1, true), std::make_tuple(1, false),