Reduce empty SST creation/deletion in compaction (#4336)
Summary: This is a followup to #4311. Checking `!RangeDelAggregator::IsEmpty()` before opening a dedicated range tombstone SST did not properly prevent empty SSTs from being generated. That's because it relies on `CollapsedRangeDelMap::Size`, which had an underflow bug when the map was empty. This PR fixes that underflow bug. Also fixed an uninitialized variable in db_stress. Pull Request resolved: https://github.com/facebook/rocksdb/pull/4336 Differential Revision: D9600080 Pulled By: ajkr fbshipit-source-id: bc6980ca79d2cd01b825ebc9dbccd51c1a70cfc7
This commit is contained in:
parent
39340abf23
commit
c00296e9fa
@ -116,6 +116,22 @@ private:
|
|||||||
std::vector<std::atomic<int>> compaction_completed_;
|
std::vector<std::atomic<int>> compaction_completed_;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
class SstStatsCollector : public EventListener {
|
||||||
|
public:
|
||||||
|
SstStatsCollector() : num_ssts_creation_started_(0) {}
|
||||||
|
|
||||||
|
void OnTableFileCreationStarted(const TableFileCreationBriefInfo& /* info */) override {
|
||||||
|
++num_ssts_creation_started_;
|
||||||
|
}
|
||||||
|
|
||||||
|
int num_ssts_creation_started() {
|
||||||
|
return num_ssts_creation_started_;
|
||||||
|
}
|
||||||
|
|
||||||
|
private:
|
||||||
|
std::atomic<int> num_ssts_creation_started_;
|
||||||
|
};
|
||||||
|
|
||||||
static const int kCDTValueSize = 1000;
|
static const int kCDTValueSize = 1000;
|
||||||
static const int kCDTKeysPerBuffer = 4;
|
static const int kCDTKeysPerBuffer = 4;
|
||||||
static const int kCDTNumLevels = 8;
|
static const int kCDTNumLevels = 8;
|
||||||
@ -3816,6 +3832,30 @@ TEST_F(DBCompactionTest, CompactFilesOutputRangeConflict) {
|
|||||||
bg_thread.join();
|
bg_thread.join();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
TEST_F(DBCompactionTest, CompactionHasEmptyOutput) {
|
||||||
|
Options options = CurrentOptions();
|
||||||
|
SstStatsCollector* collector = new SstStatsCollector();
|
||||||
|
options.level0_file_num_compaction_trigger = 2;
|
||||||
|
options.listeners.emplace_back(collector);
|
||||||
|
Reopen(options);
|
||||||
|
|
||||||
|
// Make sure the L0 files overlap to prevent trivial move.
|
||||||
|
ASSERT_OK(Put("a", "val"));
|
||||||
|
ASSERT_OK(Put("b", "val"));
|
||||||
|
ASSERT_OK(Flush());
|
||||||
|
ASSERT_OK(Delete("a"));
|
||||||
|
ASSERT_OK(Delete("b"));
|
||||||
|
ASSERT_OK(Flush());
|
||||||
|
|
||||||
|
dbfull()->TEST_WaitForCompact();
|
||||||
|
ASSERT_EQ(NumTableFilesAtLevel(0), 0);
|
||||||
|
ASSERT_EQ(NumTableFilesAtLevel(1), 0);
|
||||||
|
|
||||||
|
// Expect one file creation to start for each flush, and zero for compaction
|
||||||
|
// since no keys are written.
|
||||||
|
ASSERT_EQ(2, collector->num_ssts_creation_started());
|
||||||
|
}
|
||||||
|
|
||||||
INSTANTIATE_TEST_CASE_P(DBCompactionTestWithParam, DBCompactionTestWithParam,
|
INSTANTIATE_TEST_CASE_P(DBCompactionTestWithParam, DBCompactionTestWithParam,
|
||||||
::testing::Values(std::make_tuple(1, true),
|
::testing::Values(std::make_tuple(1, true),
|
||||||
std::make_tuple(1, false),
|
std::make_tuple(1, false),
|
||||||
|
@ -344,7 +344,7 @@ class CollapsedRangeDelMap : public RangeDelMap {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
size_t Size() const { return rep_.size() - 1; }
|
size_t Size() const override { return rep_.empty() ? 0 : rep_.size() - 1; }
|
||||||
|
|
||||||
void InvalidatePosition() { iter_ = rep_.end(); }
|
void InvalidatePosition() { iter_ = rep_.end(); }
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user