dolysis
/
rocksdb
mirror of https://github.com/facebook/rocksdb.git
3
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Use same logic to assign level for non-overlapping files in universal compaction (#13059) 

Summary:
When the input files are not overlapping, a.k.a `files_overlap_=false`, it's best to assign them to non L0 levels so that they are not one sorted run each.  This can be done regardless of compaction style being leveled or universal without any side effects.

Just my guessing, this special handling may be there because universal compaction used to have an invariant that sequence number on higher levels should not be smaller than sequence number in lower levels. File ingestion used to try to keep up to that promise by doing "sequence number stealing" from the to be assigned level. However, that invariant is no longer true after deletion triggered compaction is added for universal compaction, and we also removed the sequence stealing logic from file ingestion.

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

Test Plan: Updated existing tests

Reviewed By: cbi42

Differential Revision: D64220100

Pulled By: jowlyzhang

fbshipit-source-id: 70a83afba7f4c52d502c393844e6b3273d5cf628
This commit is contained in:
Yu Zhang 2024-10-11 11:18:45 -07:00 committed by Facebook GitHub Bot
parent 2c9aa69a93
commit a571cbed17
4 changed files with 5 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
db/external_sst_file_basic_test.cc
View File

@ -2045,7 +2045,7 @@ TEST_F(ExternalSSTFileBasicTest, FailIfNotBottommostLevel) {
ifo.fail_if_not_bottommost_level = true; ifo.fail_if_not_bottommost_level = true;
ifo.snapshot_consistency = true; ifo.snapshot_consistency = true;
const Status s = db_->IngestExternalFile({file_path}, ifo); const Status s = db_->IngestExternalFile({file_path}, ifo);
ASSERT_TRUE(s.IsTryAgain()); ASSERT_TRUE(s.ok());
} }
// Test level compaction // Test level compaction

2
db/external_sst_file_ingestion_job.cc
View File

@ -1065,8 +1065,6 @@ Status ExternalSstFileIngestionJob::AssignLevelAndSeqnoForIngestedFile(
overlap_with_db = true; overlap_with_db = true;
break; break;
} }
} else if (compaction_style == kCompactionStyleUniversal) {
continue;
} }
// We don't overlap with any keys in this level, but we still need to check // We don't overlap with any keys in this level, but we still need to check

6
db/external_sst_file_test.cc
View File

@ -1941,9 +1941,9 @@ TEST_P(ExternalSSTFileTest, IngestFileWithGlobalSeqnoAssignedUniversal) {
options, file_data, -1, true, write_global_seqno, options, file_data, -1, true, write_global_seqno,
verify_checksums_before_ingest, false, false, &true_data)); verify_checksums_before_ingest, false, false, &true_data));
// This file overlap with files in L4, we will ingest it into the last // This file overlap with files in L4, we will ingest it into the closest
// non-overlapping and non-empty level, in this case, it's L0. // non-overlapping level, in this case, it's L3.
ASSERT_EQ("3,0,0,0,3", FilesPerLevel()); ASSERT_EQ("2,0,0,1,3", FilesPerLevel());
size_t kcnt = 0; size_t kcnt = 0;
VerifyDBFromMap(true_data, &kcnt, false); VerifyDBFromMap(true_data, &kcnt, false);

1
unreleased_history/behavior_changes/universal_level_assgiment.md Normal file
View File

@ -0,0 +1 @@
*Assigning levels for external files are done in the same way for universal compaction and leveled compaction. The old behavior tends to assign files to L0 while the new behavior will assign the files to the lowest level possible.