Add missing ComputeCompactionScore() for a new universal manual compaction #7281
Conversation
There was a problem hiding this comment.
LGTM.
Putting a note here so we remember why there's no need for a HISTORY.md update:
As far as we can tell, this bug can cause an assertion failure during compaction picking, but no problem for production builds (the problematic compactions are excluded anyways at a later step).
jay-zhuang
changed the title
There was a problem hiding this comment.
@jay-zhuang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Seems it's only causing assert failure during compaction pick, but in production code, the problematic compactions are excluded at a later step.
|
@jay-zhuang has updated the pull request. You must reimport the pull request before landing. |
There was a problem hiding this comment.
@jay-zhuang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@jay-zhuang merged this pull request in ac7dcfd. |
| @@ -2137,6 +2137,50 @@ TEST_F(CompactionPickerTest, UniversalMarkedL0Overlap2) { | |||
| ASSERT_TRUE(file_map_[4].first->being_compacted); | |||
| } | |||
|
|
|||
| TEST_F(CompactionPickerTest, UniversalMarkedManualCompaction) { | |||
There was a problem hiding this comment.
test case LGTM, thanks for adding it!
…ction (facebook#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: facebook#7281 Reviewed By: akankshamahajan15 Differential Revision: D23228000 Pulled By: jay-zhuang fbshipit-source-id: 2e4055aeebe0f5a2b07e299e0a2d51a1ad2e216d
Seems it's only causing assert failure during compaction pick, but in production code, the problematic compactions are excluded at a later step.