diff options
author | Siying Dong <siying@users.noreply.github.com> | 2017-08-15 12:51:41 -0700 |
---|---|---|
committer | Yi Wu <yiwu@fb.com> | 2017-08-15 14:12:19 -0700 |
commit | 62e9418a7096ed368f6f1866eed5aa9fa969a345 (patch) | |
tree | 0dd04fbd7df6bb9555f8a78cf1bf3c33c6c442e2 | |
parent | 03df818cff11f268e5831e8ed710c90db7e26e84 (diff) |
Fix false removal of tombstone issue in FIFO and kCompactionStyleNone
Summary:
Similar to the bug fixed by https://github.com/facebook/rocksdb/pull/2726, FIFO with compaction and kCompactionStyleNone during user customized CompactFiles() with output level to be 0 can suffer from the same problem. Fix it by leveraging the bottommost_level_ flag.
Closes https://github.com/facebook/rocksdb/pull/2735
Differential Revision: D5626906
Pulled By: siying
fbshipit-source-id: 2b148d0461c61dbd986d74655e384419ae442158
-rw-r--r-- | db/compaction.cc | 44 | ||||
-rw-r--r-- | db/db_test.cc | 40 |
2 files changed, 62 insertions, 22 deletions
diff --git a/db/compaction.cc b/db/compaction.cc index 08bfae973..f4a82ed33 100644 --- a/db/compaction.cc +++ b/db/compaction.cc @@ -283,32 +283,32 @@ bool Compaction::KeyNotExistsBeyondOutputLevel( assert(input_version_ != nullptr); assert(level_ptrs != nullptr); assert(level_ptrs->size() == static_cast<size_t>(number_levels_)); - assert(cfd_->ioptions()->compaction_style != kCompactionStyleFIFO); - if (cfd_->ioptions()->compaction_style == kCompactionStyleUniversal) { - return bottommost_level_; - } - if (cfd_->ioptions()->compaction_style == kCompactionStyleLevel && - output_level_ == 0) { - return false; - } - // 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<FileMetaData*>& 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) { - // We've advanced far enough - if (user_cmp->Compare(user_key, f->smallest.user_key()) >= 0) { - // Key falls in this file's range, so definitely - // exists beyond output level - return false; + if (cfd_->ioptions()->compaction_style == kCompactionStyleLevel) { + if (output_level_ == 0) { + return false; + } + // 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<FileMetaData*>& 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) { + // We've advanced far enough + if (user_cmp->Compare(user_key, f->smallest.user_key()) >= 0) { + // Key falls in this file's range, so definitely + // exists beyond output level + return false; + } + break; } - break; } } + return true; + } else { + return bottommost_level_; } - return true; } // Mark (or clear) each file that is being compacted diff --git a/db/db_test.cc b/db/db_test.cc index 70f54250b..6e93b39b5 100644 --- a/db/db_test.cc +++ b/db/db_test.cc @@ -2806,6 +2806,46 @@ TEST_F(DBTest, FIFOCompactionTestWithCompaction) { options.compaction_options_fifo.max_table_files_size); } +TEST_F(DBTest, FIFOCompactionStyleWithCompactionAndDelete) { + Options options; + options.compaction_style = kCompactionStyleFIFO; + options.write_buffer_size = 20 << 10; // 20K + options.arena_block_size = 4096; + options.compaction_options_fifo.max_table_files_size = 1500 << 10; // 1MB + options.compaction_options_fifo.allow_compaction = true; + options.level0_file_num_compaction_trigger = 3; + options.compression = kNoCompression; + options.create_if_missing = true; + options = CurrentOptions(options); + DestroyAndReopen(options); + + Random rnd(301); + for (int i = 0; i < 3; i++) { + // Each file contains a different key which will be dropped later. + ASSERT_OK(Put("a" + ToString(i), RandomString(&rnd, 500))); + ASSERT_OK(Put("key" + ToString(i), "")); + ASSERT_OK(Put("z" + ToString(i), RandomString(&rnd, 500))); + Flush(); + ASSERT_OK(dbfull()->TEST_WaitForCompact()); + } + ASSERT_EQ(NumTableFilesAtLevel(0), 1); + for (int i = 0; i < 3; i++) { + ASSERT_EQ("", Get("key" + ToString(i))); + } + for (int i = 0; i < 3; i++) { + // Each file contains a different key which will be dropped later. + ASSERT_OK(Put("a" + ToString(i), RandomString(&rnd, 500))); + ASSERT_OK(Delete("key" + ToString(i))); + ASSERT_OK(Put("z" + ToString(i), RandomString(&rnd, 500))); + Flush(); + ASSERT_OK(dbfull()->TEST_WaitForCompact()); + } + ASSERT_EQ(NumTableFilesAtLevel(0), 2); + for (int i = 0; i < 3; i++) { + ASSERT_EQ("NOT_FOUND", Get("key" + ToString(i))); + } +} + // Check that FIFO-with-TTL is not supported with max_open_files != -1. TEST_F(DBTest, FIFOCompactionWithTTLAndMaxOpenFilesTest) { Options options; |