Skip to content

Commit

Permalink
refactor(storage): remove unnecessary wrap L0 with Option in Levels (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
wenym1 authored Jul 31, 2024
1 parent fe2dff7 commit 36290b0
Show file tree
Hide file tree
Showing 27 changed files with 151 additions and 269 deletions.
8 changes: 4 additions & 4 deletions src/ctl/src/cmd_impl/hummock/list_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ pub async fn list_version(
} else if verbose {
version.levels.iter_mut().for_each(|(_cg_id, levels)| {
// l0
if levels.l0.is_some() {
let l0 = levels.l0.as_mut().unwrap();
{
let l0 = &mut levels.l0;
for sub_level in &mut l0.sub_levels {
for t in &mut sub_level.table_infos {
t.remove_key_range();
Expand All @@ -58,8 +58,8 @@ pub async fn list_version(
println!("CompactionGroup {}", cg);

// l0
if levels.l0.is_some() {
for sub_level in levels.l0.as_ref().unwrap().sub_levels.iter().rev() {
{
for sub_level in levels.l0.sub_levels.iter().rev() {
println!(
"sub_level_id {} type {} sst_num {} size {}",
sub_level.sub_level_id,
Expand Down
10 changes: 1 addition & 9 deletions src/ctl/src/cmd_impl/hummock/validate_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,15 +100,7 @@ async fn print_user_key_in_version(
) -> anyhow::Result<()> {
println!("print key {:?} in version {}", target_key, version.id);
for cg in version.levels.values() {
for level in cg
.l0
.as_ref()
.unwrap()
.sub_levels
.iter()
.rev()
.chain(cg.levels.iter())
{
for level in cg.l0.sub_levels.iter().rev().chain(cg.levels.iter()) {
for sstable_info in &level.table_infos {
let key_range = &sstable_info.key_range;
let left_user_key = FullKey::decode(&key_range.left);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,7 @@ async fn read_hummock_sstables(reader: &SysCatalogReaderImpl) -> Result<Vec<RwHu
fn remove_key_range_from_version(mut version: HummockVersion) -> HummockVersion {
// Because key range is too verbose for manual analysis, just don't expose it.
for cg in version.levels.values_mut() {
for level in cg
.levels
.iter_mut()
.chain(cg.l0.as_mut().unwrap().sub_levels.iter_mut())
{
for level in cg.levels.iter_mut().chain(cg.l0.sub_levels.iter_mut()) {
for sst in &mut level.table_infos {
sst.remove_key_range();
}
Expand All @@ -115,7 +111,7 @@ fn version_to_compaction_group_rows(version: &HummockVersion) -> Vec<RwHummockVe
fn version_to_sstable_rows(version: HummockVersion) -> Vec<RwHummockSstable> {
let mut sstables = vec![];
for cg in version.levels.into_values() {
for level in cg.levels.into_iter().chain(cg.l0.unwrap().sub_levels) {
for level in cg.levels.into_iter().chain(cg.l0.sub_levels) {
for sst in level.table_infos {
let key_range = sst.key_range;
sstables.push(RwHummockSstable {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ impl CompactionPicker for LevelCompactionPicker {
level_handlers: &[LevelHandler],
stats: &mut LocalPickerStatistic,
) -> Option<CompactionInput> {
let l0 = levels.l0.as_ref().unwrap();
let l0 = &levels.l0;
if l0.sub_levels.is_empty() {
return None;
}
Expand Down Expand Up @@ -315,11 +315,11 @@ pub mod tests {
],
);
let mut levels = Levels {
l0: Some(OverlappingLevel {
l0: OverlappingLevel {
total_file_size: l0.total_file_size,
uncompressed_file_size: l0.total_file_size,
sub_levels: vec![l0],
}),
},
levels: vec![generate_level(
1,
vec![
Expand Down Expand Up @@ -353,10 +353,10 @@ pub mod tests {
assert_eq!(ret2.input_levels[1].table_infos[0].sst_id, 5);
}

levels.l0.as_mut().unwrap().sub_levels[0]
levels.l0.sub_levels[0]
.table_infos
.retain(|table| table.sst_id != 4);
levels.l0.as_mut().unwrap().total_file_size -= ret.input_levels[0].table_infos[0].file_size;
levels.l0.total_file_size -= ret.input_levels[0].table_infos[0].file_size;

levels_handler[0].remove_task(0);
levels_handler[1].remove_task(0);
Expand Down Expand Up @@ -415,11 +415,11 @@ pub mod tests {
}];
let mut levels = Levels {
levels,
l0: Some(OverlappingLevel {
l0: OverlappingLevel {
sub_levels: vec![],
total_file_size: 0,
uncompressed_file_size: 0,
}),
},
..Default::default()
};
push_tables_level0_nonoverlapping(&mut levels, vec![generate_table(1, 1, 50, 140, 2)]);
Expand Down Expand Up @@ -477,11 +477,11 @@ pub mod tests {
}];
let mut levels = Levels {
levels,
l0: Some(OverlappingLevel {
l0: OverlappingLevel {
sub_levels: vec![],
total_file_size: 0,
uncompressed_file_size: 0,
}),
},
..Default::default()
};
push_tables_level0_nonoverlapping(
Expand Down Expand Up @@ -543,7 +543,7 @@ pub mod tests {
uncompressed_file_size: 900,
..Default::default()
}],
l0: Some(generate_l0_nonoverlapping_sublevels(vec![])),
l0: generate_l0_nonoverlapping_sublevels(vec![]),
..Default::default()
};
push_tables_level0_nonoverlapping(
Expand All @@ -557,11 +557,7 @@ pub mod tests {

let mut levels_handler = vec![LevelHandler::new(0), LevelHandler::new(1)];
let mut local_stats = LocalPickerStatistic::default();
levels_handler[0].add_pending_task(
1,
4,
&levels.l0.as_ref().unwrap().sub_levels[0].table_infos,
);
levels_handler[0].add_pending_task(1, 4, &levels.l0.sub_levels[0].table_infos);
let ret = picker.pick_compaction(&levels, &levels_handler, &mut local_stats);
// Skip this compaction because the write amplification is too large.
assert!(ret.is_none());
Expand All @@ -583,7 +579,7 @@ pub mod tests {
s.level_type = LevelType::Nonoverlapping;
}
let levels = Levels {
l0: Some(l0),
l0,
levels: vec![generate_level(1, vec![generate_table(3, 1, 0, 100000, 1)])],
..Default::default()
};
Expand Down Expand Up @@ -660,15 +656,15 @@ pub mod tests {
]);

let levels = Levels {
l0: Some(l0),
l0,
levels: vec![generate_level(1, vec![generate_table(3, 1, 0, 100000, 1)])],
..Default::default()
};
let mut levels_handler = vec![LevelHandler::new(0), LevelHandler::new(1)];
let mut local_stats = LocalPickerStatistic::default();

// Create a pending sub-level.
let pending_level = levels.l0.as_ref().unwrap().sub_levels[1].clone();
let pending_level = levels.l0.sub_levels[1].clone();
assert_eq!(pending_level.sub_level_id, 1);
let tier_task_input = CompactionInput {
input_levels: vec![InputLevel {
Expand Down Expand Up @@ -726,7 +722,7 @@ pub mod tests {
]);

let levels = Levels {
l0: Some(l0),
l0,
levels: vec![generate_level(1, vec![generate_table(3, 1, 1, 100, 1)])],
..Default::default()
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ impl EmergencyCompactionPicker {
stats: &mut LocalPickerStatistic,
) -> Option<CompactionInput> {
let unused_validator = Arc::new(CompactionTaskValidator::unused());
let l0 = levels.l0.as_ref().unwrap();
let l0 = &levels.l0;
let overlapping_count = l0
.sub_levels
.iter()
Expand Down Expand Up @@ -100,7 +100,7 @@ impl EmergencyCompactionPicker {
WholeLevelCompactionPicker::new(self.config.clone(), unused_validator.clone());

if let Some(ret) = intral_level_compaction_picker.pick_whole_level(
levels.l0.as_ref().unwrap(),
&levels.l0,
&level_handlers[0],
self.config.split_weight_by_vnode,
stats,
Expand Down
38 changes: 19 additions & 19 deletions src/meta/src/hummock/compaction/picker/intra_compaction_picker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ impl CompactionPicker for IntraCompactionPicker {
level_handlers: &[LevelHandler],
stats: &mut LocalPickerStatistic,
) -> Option<CompactionInput> {
let l0 = levels.l0.as_ref().unwrap();
let l0 = &levels.l0;
if l0.sub_levels.is_empty() {
return None;
}
Expand Down Expand Up @@ -450,11 +450,11 @@ pub mod tests {
}];
let mut levels = Levels {
levels,
l0: Some(OverlappingLevel {
l0: OverlappingLevel {
sub_levels: vec![],
total_file_size: 0,
uncompressed_file_size: 0,
}),
},
..Default::default()
};
push_tables_level0_nonoverlapping(
Expand Down Expand Up @@ -495,10 +495,10 @@ pub mod tests {
table_infos: vec![generate_table(3, 1, 200, 300, 2)],
..Default::default()
}],
l0: Some(generate_l0_nonoverlapping_sublevels(vec![
l0: generate_l0_nonoverlapping_sublevels(vec![
generate_table(1, 1, 100, 210, 2),
generate_table(2, 1, 200, 250, 2),
])),
]),
..Default::default()
};
let mut levels_handler = vec![LevelHandler::new(0), LevelHandler::new(1)];
Expand Down Expand Up @@ -534,7 +534,7 @@ pub mod tests {
],
]);
let levels = Levels {
l0: Some(l0),
l0,
levels: vec![generate_level(1, vec![generate_table(100, 1, 0, 1000, 1)])],
..Default::default()
};
Expand Down Expand Up @@ -582,7 +582,7 @@ pub mod tests {
],
]);
let levels = Levels {
l0: Some(l0),
l0,
levels: vec![generate_level(1, vec![generate_table(100, 1, 0, 1000, 1)])],
..Default::default()
};
Expand Down Expand Up @@ -653,7 +653,7 @@ pub mod tests {
],
]);
let levels = Levels {
l0: Some(l0),
l0,
levels: vec![generate_level(1, vec![generate_table(100, 1, 0, 1000, 1)])],
..Default::default()
};
Expand Down Expand Up @@ -728,7 +728,7 @@ pub mod tests {
generate_table(2, 1, 150, 250, 1),
]]);
let levels = Levels {
l0: Some(l0),
l0,
levels: vec![generate_level(1, vec![generate_table(100, 1, 0, 1000, 1)])],
..Default::default()
};
Expand All @@ -748,7 +748,7 @@ pub mod tests {
vec![generate_table(5, 1, 10, 90, 1)],
]);
let mut levels = Levels {
l0: Some(l0),
l0,
levels: vec![generate_level(1, vec![generate_table(100, 1, 0, 1000, 1)])],
..Default::default()
};
Expand All @@ -757,20 +757,20 @@ pub mod tests {
.is_none());

// Cannot trivial move because latter sub-level is overlapping
levels.l0.as_mut().unwrap().sub_levels[0].level_type = LevelType::Nonoverlapping;
levels.l0.as_mut().unwrap().sub_levels[1].level_type = LevelType::Overlapping;
levels.l0.sub_levels[0].level_type = LevelType::Nonoverlapping;
levels.l0.sub_levels[1].level_type = LevelType::Overlapping;
let ret = picker.pick_compaction(&levels, &levels_handler, &mut local_stats);
assert!(ret.is_none());

// Cannot trivial move because former sub-level is overlapping
levels.l0.as_mut().unwrap().sub_levels[0].level_type = LevelType::Overlapping;
levels.l0.as_mut().unwrap().sub_levels[1].level_type = LevelType::Nonoverlapping;
levels.l0.sub_levels[0].level_type = LevelType::Overlapping;
levels.l0.sub_levels[1].level_type = LevelType::Nonoverlapping;
let ret = picker.pick_compaction(&levels, &levels_handler, &mut local_stats);
assert!(ret.is_none());

// trivial move
levels.l0.as_mut().unwrap().sub_levels[0].level_type = LevelType::Nonoverlapping;
levels.l0.as_mut().unwrap().sub_levels[1].level_type = LevelType::Nonoverlapping;
levels.l0.sub_levels[0].level_type = LevelType::Nonoverlapping;
levels.l0.sub_levels[1].level_type = LevelType::Nonoverlapping;
let ret = picker
.pick_compaction(&levels, &levels_handler, &mut local_stats)
.unwrap();
Expand Down Expand Up @@ -845,7 +845,7 @@ pub mod tests {
let mut local_stats = LocalPickerStatistic::default();

let levels = Levels {
l0: Some(l0),
l0,
levels: vec![generate_level(1, vec![generate_table(100, 1, 0, 1000, 1)])],
..Default::default()
};
Expand All @@ -860,11 +860,11 @@ pub mod tests {
let input = ret.as_ref().unwrap();
assert_eq!(input.input_levels.len(), 2);
assert_ne!(
levels.l0.as_ref().unwrap().sub_levels[0].table_infos.len(),
levels.l0.sub_levels[0].table_infos.len(),
input.input_levels[0].table_infos.len()
);
assert_ne!(
levels.l0.as_ref().unwrap().sub_levels[1].table_infos.len(),
levels.l0.sub_levels[1].table_infos.len(),
input.input_levels[1].table_infos.len()
);
}
Expand Down
Loading

0 comments on commit 36290b0

Please sign in to comment.