Clarify manual compaction and file ingestion behavior with FIFO compaction (#12618)

Summary:
For manual compaction, FIFO compaction will always skip key range overlapping checking with SST files. If CompactRange() is called with CompactionRangeOptions::change_level=true, a CF with FIFO compaction will now return Status::NotSupported.

For file ingestion, we will always ingest into L0. Previously, it's possible to ingest files into non-L0 levels with FIFO compaction.

These changes also help to fix [this](a178d15baf/db/db_impl/db_impl_compaction_flush.cc (L1269)) assertion failure in crash tests.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/12618

Test Plan: added unit tests to verify the new behavior.

Reviewed By: hx235

Differential Revision: D56962401

Pulled By: cbi42

fbshipit-source-id: 19812a1509650b4162b379ca5bee02f2e9d9569d
This commit is contained in:
Changyu Bi 2024-05-07 12:00:15 -07:00 committed by Facebook GitHub Bot
parent 83d051a8d9
commit 5bf2c00a35
8 changed files with 96 additions and 6 deletions

View File

@ -1126,6 +1126,11 @@ Status DBImpl::CompactRangeInternal(const CompactRangeOptions& options,
if (options.target_path_id >= cfd->ioptions()->cf_paths.size()) {
return Status::InvalidArgument("Invalid target path ID");
}
if (options.change_level &&
cfd->ioptions()->compaction_style == kCompactionStyleFIFO) {
return Status::NotSupported(
"FIFO compaction does not support change_level.");
}
bool flush_needed = true;
@ -1186,6 +1191,16 @@ Status DBImpl::CompactRangeInternal(const CompactRangeOptions& options,
final_output_level, options, begin, end, exclusive,
false /* disable_trivial_move */,
std::numeric_limits<uint64_t>::max(), trim_ts);
} else if (cfd->ioptions()->compaction_style == kCompactionStyleFIFO) {
// FIFOCompactionPicker::CompactRange() will ignore the input key range
// [begin, end] and just try to pick compaction based on the configured
// option `compaction_options_fifo`. So we skip checking if [begin, end]
// overlaps with the DB here.
final_output_level = 0;
s = RunManualCompaction(cfd, /*input_level=*/0, final_output_level, options,
begin, end, exclusive,
false /* disable_trivial_move */,
std::numeric_limits<uint64_t>::max(), trim_ts);
} else {
int first_overlapped_level = kInvalidLevel;
{
@ -1270,8 +1285,7 @@ Status DBImpl::CompactRangeInternal(const CompactRangeOptions& options,
CleanupSuperVersion(super_version);
}
if (s.ok() && first_overlapped_level != kInvalidLevel) {
if (cfd->ioptions()->compaction_style == kCompactionStyleUniversal ||
cfd->ioptions()->compaction_style == kCompactionStyleFIFO) {
if (cfd->ioptions()->compaction_style == kCompactionStyleUniversal) {
assert(first_overlapped_level == 0);
s = RunManualCompaction(
cfd, first_overlapped_level, first_overlapped_level, options, begin,

View File

@ -3844,6 +3844,9 @@ TEST_P(DBTestWithParam, FIFOCompactionTest) {
} else {
CompactRangeOptions cro;
cro.exclusive_manual_compaction = exclusive_manual_compaction_;
cro.change_level = true;
ASSERT_TRUE(db_->CompactRange(cro, nullptr, nullptr).IsNotSupported());
cro.change_level = false;
ASSERT_OK(db_->CompactRange(cro, nullptr, nullptr));
}
// only 5 files should survive

View File

@ -953,13 +953,15 @@ Status ExternalSstFileIngestionJob::AssignLevelAndSeqnoForIngestedFile(
*assigned_seqno = 0;
auto ucmp = cfd_->user_comparator();
const size_t ts_sz = ucmp->timestamp_size();
if (force_global_seqno || files_overlap_) {
if (force_global_seqno || files_overlap_ ||
compaction_style == kCompactionStyleFIFO) {
*assigned_seqno = last_seqno + 1;
// If files overlap, we have to ingest them at level 0.
if (files_overlap_) {
if (files_overlap_ || compaction_style == kCompactionStyleFIFO) {
assert(ts_sz == 0);
file_to_ingest->picked_level = 0;
if (ingestion_options_.fail_if_not_bottommost_level) {
if (ingestion_options_.fail_if_not_bottommost_level &&
cfd_->NumberLevels() > 1) {
status = Status::TryAgain(
"Files cannot be ingested to Lmax. Please make sure key range of "
"Lmax does not overlap with files to ingest.");

View File

@ -3082,6 +3082,68 @@ TEST_P(ExternalSSTFileTest,
delete iter;
}
TEST_F(ExternalSSTFileTest, FIFOCompaction) {
// FIFO always ingests SST files to L0 and assign latest sequence number.
Options options = CurrentOptions();
options.num_levels = 1;
options.compaction_style = kCompactionStyleFIFO;
options.max_open_files = -1;
DestroyAndReopen(options);
std::map<std::string, std::string> true_data;
for (int i = 0; i < 100; ++i) {
ASSERT_OK(Put(Key(i), Key(i) + "_val"));
true_data[Key(i)] = Key(i) + "_val";
}
ASSERT_OK(Flush());
ASSERT_EQ("1", FilesPerLevel());
std::vector<std::pair<std::string, std::string>> file_data;
for (int i = 0; i <= 20; i++) {
file_data.emplace_back(Key(i), Key(i) + "_ingest");
}
// Overlaps with memtable, will trigger flush
ASSERT_OK(GenerateAndAddExternalFile(options, file_data, -1,
/*allow_global_seqno=*/true, true, false,
false, false, &true_data));
ASSERT_EQ("2", FilesPerLevel());
file_data.clear();
for (int i = 100; i <= 120; i++) {
file_data.emplace_back(Key(i), Key(i) + "_ingest");
}
// global sequence number is always assigned, so this will fail
ASSERT_NOK(GenerateAndAddExternalFile(options, file_data, -1,
/*allow_global_seqno=*/false, true,
false, false, false, &true_data));
ASSERT_OK(GenerateAndAddExternalFile(options, file_data, -1,
/*allow_global_seqno=*/true, true, false,
false, false, &true_data));
// Compact to data to lower level to test multi-level FIFO later
options.num_levels = 7;
options.compaction_style = kCompactionStyleUniversal;
ASSERT_OK(TryReopen(options));
CompactRangeOptions cro;
cro.bottommost_level_compaction = BottommostLevelCompaction::kForceOptimized;
ASSERT_OK(db_->CompactRange(cro, nullptr, nullptr));
ASSERT_EQ("0,0,0,0,0,0,1", FilesPerLevel());
options.num_levels = 7;
options.compaction_style = kCompactionStyleFIFO;
ASSERT_OK(TryReopen(options));
file_data.clear();
for (int i = 200; i <= 220; i++) {
file_data.emplace_back(Key(i), Key(i) + "_ingest");
}
// Files are ingested into L0 for multi-level FIFO
ASSERT_OK(GenerateAndAddExternalFile(options, file_data, -1,
/*allow_global_seqno=*/true, true, false,
false, false, &true_data));
ASSERT_EQ("1,0,0,0,0,0,1", FilesPerLevel());
VerifyDBFromMap(true_data);
}
class ExternalSSTFileWithTimestampTest : public ExternalSSTFileTest {
public:
ExternalSSTFileWithTimestampTest() = default;

View File

@ -2583,7 +2583,10 @@ void StressTest::TestCompactRange(ThreadState* thread, int64_t rand_key,
CompactRangeOptions cro;
cro.exclusive_manual_compaction = static_cast<bool>(thread->rand.Next() % 2);
cro.change_level = static_cast<bool>(thread->rand.Next() % 2);
if (static_cast<ROCKSDB_NAMESPACE::CompactionStyle>(FLAGS_compaction_style) !=
ROCKSDB_NAMESPACE::CompactionStyle::kCompactionStyleFIFO) {
cro.change_level = static_cast<bool>(thread->rand.Next() % 2);
}
if (thread->rand.OneIn(2)) {
cro.target_level = thread->rand.Next() % options_.num_levels;
}

View File

@ -1475,6 +1475,9 @@ class DB {
// move the files back to the minimum level capable of holding the data set
// or a given level (specified by non-negative options.target_level).
//
// For FIFO compaction, this will trigger a compaction (if available)
// based on CompactionOptionsFIFO.
//
// In case of user-defined timestamp, if enabled, `begin` and `end` should
// not contain timestamp.
virtual Status CompactRange(const CompactRangeOptions& options,
@ -1858,6 +1861,7 @@ class DB {
// supported. 4) When an ingested file contains point data and range deletion
// for the same key, the point data currently overrides the range deletion
// regardless which one has the higher user-defined timestamps.
// For FIFO compaction, SST files will always be ingested into L0.
//
// (1) External SST files can be created using SstFileWriter
// (2) We will try to ingest the files to the lowest possible level

View File

@ -0,0 +1 @@
* CompactRange() with change_level=true on a CF with FIFO compaction will return Status::NotSupported().

View File

@ -0,0 +1 @@
* External file ingestion with FIFO compaction will always ingest to L0.