Skip to content

Commit

Permalink
Add missing ComputeCompactionScore() for a new universal manual compa…
Browse files Browse the repository at this point in the history
…ction (#7281)

Summary:
Seems it's only causing assert failure during compaction pick, but in production code, the problematic compactions are excluded at a later step.

Pull Request resolved: #7281

Reviewed By: akankshamahajan15

Differential Revision: D23228000

Pulled By: jay-zhuang

fbshipit-source-id: 2e4055aeebe0f5a2b07e299e0a2d51a1ad2e216d
  • Loading branch information
jay-zhuang authored and facebook-github-bot committed Aug 20, 2020
1 parent b9bb59d commit ac7dcfd
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 0 deletions.
1 change: 1 addition & 0 deletions db/compaction/compaction_picker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,7 @@ Compaction* CompactionPicker::CompactRange(
compact_range_options.max_subcompactions, /* grandparents */ {},
/* is manual */ true);
RegisterCompaction(c);
vstorage->ComputeCompactionScore(ioptions_, mutable_cf_options);
return c;
}

Expand Down
44 changes: 44 additions & 0 deletions db/compaction/compaction_picker_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2137,6 +2137,50 @@ TEST_F(CompactionPickerTest, UniversalMarkedL0Overlap2) {
ASSERT_TRUE(file_map_[4].first->being_compacted);
}

TEST_F(CompactionPickerTest, UniversalMarkedManualCompaction) {
const uint64_t kFileSize = 100000;
const int kNumLevels = 7;

// This test makes sure the `files_marked_for_compaction_` is updated after
// creating manual compaction.
ioptions_.compaction_style = kCompactionStyleUniversal;
UniversalCompactionPicker universal_compaction_picker(ioptions_, &icmp_);

NewVersionStorage(kNumLevels, kCompactionStyleUniversal);

// Add 3 files marked for compaction
Add(0, 3U, "301", "350", 4 * kFileSize, 0, 101, 150, 0, true);
Add(0, 4U, "260", "300", 1 * kFileSize, 0, 260, 300, 0, true);
Add(0, 5U, "240", "290", 2 * kFileSize, 0, 201, 250, 0, true);
UpdateVersionStorageInfo();

// All 3 files are marked for compaction
ASSERT_EQ(3U, vstorage_->FilesMarkedForCompaction().size());

bool manual_conflict = false;
InternalKey* manual_end = NULL;
std::unique_ptr<Compaction> compaction(
universal_compaction_picker.CompactRange(
cf_name_, mutable_cf_options_, mutable_db_options_, vstorage_.get(),
ColumnFamilyData::kCompactAllLevels, 6, CompactRangeOptions(), NULL,
NULL, &manual_end, &manual_conflict, port::kMaxUint64));

ASSERT_TRUE(compaction);

ASSERT_EQ(CompactionReason::kManualCompaction,
compaction->compaction_reason());
ASSERT_EQ(kNumLevels - 1, compaction->output_level());
ASSERT_EQ(0, compaction->start_level());
ASSERT_EQ(3U, compaction->num_input_files(0));
ASSERT_TRUE(file_map_[3].first->being_compacted);
ASSERT_TRUE(file_map_[4].first->being_compacted);
ASSERT_TRUE(file_map_[5].first->being_compacted);

// After creating the manual compaction, all files should be cleared from
// `FilesMarkedForCompaction`. So they won't be picked by others.
ASSERT_EQ(0U, vstorage_->FilesMarkedForCompaction().size());
}

#endif // ROCKSDB_LITE

} // namespace ROCKSDB_NAMESPACE
Expand Down

0 comments on commit ac7dcfd

Please sign in to comment.