Skip to content
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.

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

SSTs overlap with compactions should be able to be ingested into deeper levels #13091

Open
v01dstar opened this issue Oct 24, 2024 · 2 comments
Assignees

Comments

@v01dstar
Copy link
Contributor

v01dstar commented Oct 24, 2024

Expected behavior

When ingesting external SST files. If a SST overlaps with some registered compactions, it should NOT be considered overlap_with_db, but overlap_with_level. In other words, it should still be able to go down the levels, check if deeper levels can be selected for this file. Because, a SST does not overlap with previous levels but overlaps with a registered compaction means they do not have duplicate keys, thus safe to be ingest to a different, deeper level who does not have overlap ranges.

I believe this is also the expected behavior before #10988, the PR changed this behavior, although, I suspect, the change was not the purpose the PR.

Actual behavior

SSTs overlap with compactions are ingested into level 0. Triggers write stall.

for (int lvl = 0; lvl < exclusive_end_level; lvl++) {
if (lvl > 0 && lvl < vstorage->base_level()) {
continue;
}
if (cfd_->RangeOverlapWithCompaction(file_to_ingest->start_ukey,
file_to_ingest->limit_ukey, lvl)) {
// We must use L0 or any level higher than `lvl` to be able to overwrite
// the compaction output keys that we overlap with in this level, We also
// need to assign this file a seqno to overwrite the compaction output
// keys in level `lvl`
overlap_with_db = true;
break;
} else if (vstorage->NumLevelFiles(lvl) > 0) {

Steps to reproduce the behavior

We found this issue during backfilling data using ingestions. The application ensures the data to be back filled does not overlap with existing data. But still got ingested into level 0. Presumably, some of the files overlaps with a ongoing compaction at high level, e.g. SST with range "[10 - 20)" was considered over_with_db, because a level1 compaction was trying to compact "[0,10)" and "[20-30)".

A simple fix would be: main...v01dstar:rocksdb:fix-ingest-lvl

Would like to hear opinions from the maintainers.

@v01dstar
Copy link
Contributor Author

@hx235 @cbi42

@cbi42
Copy link
Member

cbi42 commented Oct 28, 2024

Because, a SST does not overlap with previous levels but overlaps with a registered compaction means they do not have duplicate keys

One case I'm thinking this may not hold is when we move files from lower levels to higher levels. This can happen when doing a manual compaction with target level that is smaller than the output level.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants