Skip to content

compaction: add a new option 'check_range_overlap_on_bottom_level' for manual compact #421

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ti-chi-bot merged 7 commits into from
Nov 18, 2025
Merged

compaction: add a new option 'check_range_overlap_on_bottom_level' for manual compact #421

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1b0c172
add a new option 'bottom_level_check_range_overlap' for manual compact
glorv Aug 19, 2025
424ac4f
add a comment
glorv Nov 17, 2025
209d988
Merge branch '8.10.tikv' into manual-compact-bottom-check
glorv Nov 17, 2025
94c0908
Merge branch '8.10.tikv' into manual-compact-bottom-check
glorv Nov 18, 2025
4bc323c
rename the new config name
glorv Nov 18, 2025
30a4f92
Merge branch 'manual-compact-bottom-check' of ssh://github.com/glorv/…
glorv Nov 18, 2025
c24fc99
rename
glorv Nov 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion db/db_impl/db_impl_compaction_flush.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1234,7 +1234,10 @@ Status DBImpl::CompactRangeInternal(const CompactRangeOptions& options,
check_overlap_within_file = false;
}
}
if (!check_overlap_within_file) {
if (!check_overlap_within_file ||
Copy link
Member

@v01dstar v01dstar Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why !check_overlap_within_file is not sufficient?

rocksdb/db/db_impl/db_impl_compaction_flush.cc

Lines 1225 to 1227 in 2b04d05

if (!partitioner->CanDoTrivialMove(*begin, *end)) {
check_overlap_within_file = false;
}

Seems that if there are partitioner boundaries within the target range, it will trigger a compaction, not trivial move (by setting overlap to the current level)

Copy link
Author

@glorv glorv Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right, it's better to just change the CanDoTrivialMove implement in the SSTPartitioner. No need to add a new option.

Copy link
Member

@hbisheng hbisheng Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it seems CanDoTrivialMove can't tell whether we are at the bottommost level? It only sees the key range

Copy link
Author

@glorv glorv Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but I think we should force partition/manual compact the overlapped SST files in all the level, not only the bottom level. This approach should gain better compatibility. So reuse CanDoTrivialMove is enough here.

(!overlap && options.bottom_level_check_range_overlap &&
Copy link
Member

@Connor1996 Connor1996 Aug 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a comment to elaborate on why this can ensure the SST file in the bottom level with a huge range can be force partitioned with manual compaction.

Copy link
Author

@glorv glorv Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

level ==
current_version->storage_info()->num_non_empty_levels() - 1)) {
overlap = current_version->storage_info()->OverlapInLevel(level,
begin, end);
}
Expand Down
10 changes: 10 additions & 0 deletions db/external_sst_file_ingestion_job.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -925,6 +925,9 @@ Status ExternalSstFileIngestionJob::AssignLevelAndSeqnoForIngestedFile(
// the keys that we overlap with in this level, We also need to assign
// this file a seqno to overwrite the existing keys in level `lvl`
overlap_with_db = true;
ROCKS_LOG_INFO(db_options_.info_log,
"Ingest file overlap with level %d, file: %s", lvl,
file_to_ingest->internal_file_path.c_str());
break;
}

Expand Down Expand Up @@ -1110,13 +1113,20 @@ bool ExternalSstFileIngestionJob::IngestedFileFitInLevel(
&file_largest_user_key)) {
// File overlap with another files in this level, we cannot
// add it to this level
ROCKS_LOG_INFO(
db_options_.info_log,
"Ingest file overlap with level file range, level %d, ingset_file: %s",
level, file_to_ingest->internal_file_path.c_str());
return false;
}

if (cfd_->RangeOverlapWithCompaction(file_smallest_user_key,
file_largest_user_key, level)) {
// File overlap with a running compaction output that will be stored
// in this level, we cannot add this file to this level
ROCKS_LOG_INFO(db_options_.info_log,
"Ingest file overlap with compaction in level %d, file: %s",
level, file_to_ingest->internal_file_path.c_str());
return false;
}

Expand Down
5 changes: 5 additions & 0 deletions include/rocksdb/options.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2020,6 +2020,11 @@ struct CompactRangeOptions {
// user-provided setting. This enables customers to selectively override the
// age cutoff.
double blob_garbage_collection_age_cutoff = -1;

// If set to true, it will check file range overlap instead of keys overlap
// for the bottom level. This is used in manual compact for SST ingestion
// scenario.
bool bottom_level_check_range_overlap = false;
Copy link

@hhwyt hhwyt Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest renaming it to check_range_overlap_on_bottom_level to match the existing naming style.

};

// IngestExternalFileOptions is used by IngestExternalFile()
Expand Down