From f151adb62e31158df2fe17d30d4dfd68a429cedf Mon Sep 17 00:00:00 2001 From: Li0k Date: Mon, 22 Jan 2024 17:30:29 +0800 Subject: [PATCH 1/8] chore(storage): commit fix(storage): commit fix(storage): min overlap picker with config enable_trivial_move chore(storage): add check for skip sub_level Revert "fix(storage): min overlap picker with config enable_trivial_move" This reverts commit 0777d919c2ce30e83f09233cc4a842d4ed8c16fc. fix(storage): fix example config chore(storage): fix opt and add enable_check_skip_level_overlap fix(storage): update check fix(storage): fix task check --- proto/hummock.proto | 2 + src/common/src/config.rs | 12 +++++ src/config/example.toml | 2 + .../hummock/compaction/compaction_config.rs | 7 +++ .../hummock/compaction/overlap_strategy.rs | 6 +++ .../picker/base_level_compaction_picker.rs | 9 +++- .../picker/intra_compaction_picker.rs | 8 ++- .../picker/min_overlap_compaction_picker.rs | 54 ++++++++++++++++++- .../picker/trivial_move_compaction_picker.rs | 11 ++++ 9 files changed, 107 insertions(+), 4 deletions(-) diff --git a/proto/hummock.proto b/proto/hummock.proto index 9b39022f7f734..54d24d6ec8e57 100644 --- a/proto/hummock.proto +++ b/proto/hummock.proto @@ -837,6 +837,8 @@ message CompactionConfig { uint32 level0_overlapping_sub_level_compact_level_count = 18; uint32 tombstone_reclaim_ratio = 19; bool enable_emergency_picker = 20; + bool enable_trivial_move = 21; + bool enable_check_task_level_overlap = 22; } message TableStats { diff --git a/src/common/src/config.rs b/src/common/src/config.rs index b8c40cdb9442e..6815590a16c4a 100644 --- a/src/common/src/config.rs +++ b/src/common/src/config.rs @@ -1491,6 +1491,14 @@ pub mod default { pub fn enable_emergency_picker() -> bool { DEFAULT_EMERGENCY_PICKER } + + pub fn enable_trivial_move() -> bool { + true + } + + pub fn enable_check_task_level_overlap() -> bool { + false + } } pub mod object_store_config { @@ -1644,6 +1652,10 @@ pub struct CompactionConfig { pub tombstone_reclaim_ratio: u32, #[serde(default = "default::compaction_config::enable_emergency_picker")] pub enable_emergency_picker: bool, + #[serde(default = "default::compaction_config::enable_trivial_move")] + pub enable_trivial_move: bool, + #[serde(default = "default::compaction_config::enable_check_task_level_overlap")] + pub enable_check_task_level_overlap: bool, } #[cfg(test)] diff --git a/src/config/example.toml b/src/config/example.toml index 914cfe63889d4..63f514df09615 100644 --- a/src/config/example.toml +++ b/src/config/example.toml @@ -66,6 +66,8 @@ max_space_reclaim_bytes = 536870912 level0_max_compact_file_number = 100 tombstone_reclaim_ratio = 40 enable_emergency_picker = true +enable_trivial_move = true +enable_check_task_level_overlap = false [meta.developer] meta_cached_traces_num = 256 diff --git a/src/meta/src/hummock/compaction/compaction_config.rs b/src/meta/src/hummock/compaction/compaction_config.rs index 6048e301c97d6..bb7498081f00f 100644 --- a/src/meta/src/hummock/compaction/compaction_config.rs +++ b/src/meta/src/hummock/compaction/compaction_config.rs @@ -66,6 +66,9 @@ impl CompactionConfigBuilder { compaction_config::level0_overlapping_sub_level_compact_level_count(), tombstone_reclaim_ratio: compaction_config::tombstone_reclaim_ratio(), enable_emergency_picker: compaction_config::enable_emergency_picker(), + enable_trivial_move: compaction_config::enable_trivial_move(), + enable_check_task_level_overlap: compaction_config::enable_check_task_level_overlap( + ), }, } } @@ -94,6 +97,8 @@ impl CompactionConfigBuilder { .max_space_reclaim_bytes(opt.max_space_reclaim_bytes) .level0_max_compact_file_number(opt.level0_max_compact_file_number) .tombstone_reclaim_ratio(opt.tombstone_reclaim_ratio) + .enable_trivial_move(opt.enable_trivial_move) + .enable_check_task_level_overlap(opt.enable_check_task_level_overlap) } pub fn build(self) -> CompactionConfig { @@ -154,4 +159,6 @@ builder_field! { level0_sub_level_compact_level_count: u32, level0_overlapping_sub_level_compact_level_count: u32, tombstone_reclaim_ratio: u32, + enable_trivial_move: bool, + enable_check_task_level_overlap: bool, } diff --git a/src/meta/src/hummock/compaction/overlap_strategy.rs b/src/meta/src/hummock/compaction/overlap_strategy.rs index 9619a083e35b4..5cea7c4bab409 100644 --- a/src/meta/src/hummock/compaction/overlap_strategy.rs +++ b/src/meta/src/hummock/compaction/overlap_strategy.rs @@ -25,6 +25,8 @@ pub trait OverlapInfo { fn check_multiple_overlap(&self, others: &[SstableInfo]) -> Range; fn check_multiple_include(&self, others: &[SstableInfo]) -> Range; fn update(&mut self, table: &SstableInfo); + + fn debug_info(&self) -> String; } pub trait OverlapStrategy: Send + Sync { @@ -144,6 +146,10 @@ impl OverlapInfo for RangeOverlapInfo { } self.target_range = Some(other.clone()); } + + fn debug_info(&self) -> String { + format!("range {:?}", self.target_range) + } } #[derive(Default)] diff --git a/src/meta/src/hummock/compaction/picker/base_level_compaction_picker.rs b/src/meta/src/hummock/compaction/picker/base_level_compaction_picker.rs index ba3c08b8a90fc..b17e127dbb8f5 100644 --- a/src/meta/src/hummock/compaction/picker/base_level_compaction_picker.rs +++ b/src/meta/src/hummock/compaction/picker/base_level_compaction_picker.rs @@ -115,8 +115,12 @@ impl LevelCompactionPicker { stats: &mut LocalPickerStatistic, ) -> Option { let overlap_strategy = create_overlap_strategy(self.config.compaction_mode()); - let trivial_move_picker = - TrivialMovePicker::new(0, self.target_level, overlap_strategy.clone()); + let trivial_move_picker = TrivialMovePicker::new( + 0, + self.target_level, + overlap_strategy.clone(), + self.config.enable_trivial_move, + ); trivial_move_picker.pick_trivial_move_task( &l0.sub_levels[0].table_infos, @@ -148,6 +152,7 @@ impl LevelCompactionPicker { // The maximum number of sub_level compact level per task self.config.level0_max_compact_file_number, overlap_strategy.clone(), + self.config.enable_check_task_level_overlap, ); let mut max_vnode_partition_idx = 0; diff --git a/src/meta/src/hummock/compaction/picker/intra_compaction_picker.rs b/src/meta/src/hummock/compaction/picker/intra_compaction_picker.rs index 00ad9a74744a1..651c030252bd4 100644 --- a/src/meta/src/hummock/compaction/picker/intra_compaction_picker.rs +++ b/src/meta/src/hummock/compaction/picker/intra_compaction_picker.rs @@ -216,6 +216,7 @@ impl IntraCompactionPicker { self.config.level0_sub_level_compact_level_count as usize, self.config.level0_max_compact_file_number, overlap_strategy.clone(), + self.config.enable_check_task_level_overlap, ); let l0_select_tables_vec = non_overlap_sub_level_picker @@ -303,7 +304,12 @@ impl IntraCompactionPicker { continue; } - let trivial_move_picker = TrivialMovePicker::new(0, 0, overlap_strategy.clone()); + let trivial_move_picker = TrivialMovePicker::new( + 0, + 0, + overlap_strategy.clone(), + self.config.enable_trivial_move, + ); let select_sst = trivial_move_picker.pick_trivial_move_sst( &l0.sub_levels[idx + 1].table_infos, diff --git a/src/meta/src/hummock/compaction/picker/min_overlap_compaction_picker.rs b/src/meta/src/hummock/compaction/picker/min_overlap_compaction_picker.rs index 8a590a44c7fb9..c6e50825449b4 100644 --- a/src/meta/src/hummock/compaction/picker/min_overlap_compaction_picker.rs +++ b/src/meta/src/hummock/compaction/picker/min_overlap_compaction_picker.rs @@ -185,6 +185,8 @@ pub struct NonOverlapSubLevelPicker { min_depth: usize, max_file_count: u64, overlap_strategy: Arc, + + enable_check_task_level_overlap: bool, } impl NonOverlapSubLevelPicker { @@ -194,6 +196,7 @@ impl NonOverlapSubLevelPicker { min_depth: usize, max_file_count: u64, overlap_strategy: Arc, + enable_check_task_level_overlap: bool, ) -> Self { Self { min_compaction_bytes, @@ -201,6 +204,7 @@ impl NonOverlapSubLevelPicker { min_depth, max_file_count, overlap_strategy, + enable_check_task_level_overlap, } } @@ -226,6 +230,7 @@ impl NonOverlapSubLevelPicker { select_sst_id_set.insert(sst.sst_id); } + let mut last_target_index = 0; for (target_index, target_level) in levels.iter().enumerate().skip(1) { if target_level.level_type() != LevelType::Nonoverlapping { break; @@ -244,6 +249,7 @@ impl NonOverlapSubLevelPicker { overlap_files_range = overlap_info.check_multiple_overlap(&target_level.table_infos); } + // We allow a layer in the middle without overlap, so we need to continue to // the next layer to search for overlap let mut pending_compact = false; @@ -337,9 +343,10 @@ impl NonOverlapSubLevelPicker { for (reverse_index, files) in extra_overlap_levels { ret.sstable_infos[reverse_index].extend(files); } + + last_target_index = std::cmp::max(last_target_index, target_index); } - ret.sstable_infos.retain(|ssts| !ssts.is_empty()); // sort sst per level due to reverse expand ret.sstable_infos.iter_mut().for_each(|level_ssts| { level_ssts.sort_by(|sst1, sst2| { @@ -348,6 +355,44 @@ impl NonOverlapSubLevelPicker { a.compare(b) }); }); + + if self.enable_check_task_level_overlap && !ret.sstable_infos.is_empty() { + let mut overlap_info = self.overlap_strategy.create_overlap_info(); + let mut skip_count = 0; + // find the first not empty level + for (level_idx, ssts) in ret.sstable_infos.iter().rev().enumerate() { + if !ssts.is_empty() { + skip_count = level_idx + 1; + for sst in ssts { + overlap_info.update(sst); + } + + break; + } + } + + for (level_idx, ssts) in ret.sstable_infos.iter().enumerate().rev().skip(skip_count) { + let level = levels.get(level_idx).unwrap(); + let overlap_sst_range = overlap_info.check_multiple_overlap(&level.table_infos); + + for sst in &level.table_infos[overlap_sst_range.clone()] { + assert!( + ssts.contains(sst), + "level_idx {} overlap_sst_range {:?} level_sst_count {} select_sst_count {}", + level_idx, + overlap_sst_range, + level.table_infos.len(), + ssts.len(), + ); + } + + for sst in ssts { + overlap_info.update(sst); + } + } + } + + ret.sstable_infos.retain(|ssts| !ssts.is_empty()); ret } @@ -621,6 +666,7 @@ pub mod tests { 1, 10000, Arc::new(RangeOverlapStrategy::default()), + true, ); let ret = picker.pick_l0_multi_non_overlap_level(&levels, &levels_handlers[0]); assert_eq!(6, ret.len()); @@ -634,6 +680,7 @@ pub mod tests { 1, 10000, Arc::new(RangeOverlapStrategy::default()), + true, ); let ret = picker.pick_l0_multi_non_overlap_level(&levels, &levels_handlers[0]); assert_eq!(6, ret.len()); @@ -647,6 +694,7 @@ pub mod tests { 1, 5, Arc::new(RangeOverlapStrategy::default()), + true, ); let ret = picker.pick_l0_multi_non_overlap_level(&levels, &levels_handlers[0]); assert_eq!(6, ret.len()); @@ -721,6 +769,7 @@ pub mod tests { 1, 10000, Arc::new(RangeOverlapStrategy::default()), + true, ); let ret = picker.pick_l0_multi_non_overlap_level(&levels, &levels_handlers[0]); assert_eq!(6, ret.len()); @@ -735,6 +784,7 @@ pub mod tests { 1, 10000, Arc::new(RangeOverlapStrategy::default()), + true, ); let ret = picker.pick_l0_multi_non_overlap_level(&levels, &levels_handlers[0]); assert_eq!(6, ret.len()); @@ -749,6 +799,7 @@ pub mod tests { 1, max_file_count, Arc::new(RangeOverlapStrategy::default()), + true, ); let ret = picker.pick_l0_multi_non_overlap_level(&levels, &levels_handlers[0]); assert_eq!(6, ret.len()); @@ -771,6 +822,7 @@ pub mod tests { min_depth, 10000, Arc::new(RangeOverlapStrategy::default()), + true, ); let ret = picker.pick_l0_multi_non_overlap_level(&levels, &levels_handlers[0]); assert_eq!(3, ret.len()); diff --git a/src/meta/src/hummock/compaction/picker/trivial_move_compaction_picker.rs b/src/meta/src/hummock/compaction/picker/trivial_move_compaction_picker.rs index bab0dca0c3244..cd58163246239 100644 --- a/src/meta/src/hummock/compaction/picker/trivial_move_compaction_picker.rs +++ b/src/meta/src/hummock/compaction/picker/trivial_move_compaction_picker.rs @@ -24,6 +24,7 @@ pub struct TrivialMovePicker { level: usize, target_level: usize, overlap_strategy: Arc, + enable: bool, } impl TrivialMovePicker { @@ -31,11 +32,13 @@ impl TrivialMovePicker { level: usize, target_level: usize, overlap_strategy: Arc, + enable: bool, ) -> Self { Self { level, target_level, overlap_strategy, + enable, } } @@ -46,6 +49,10 @@ impl TrivialMovePicker { level_handlers: &[LevelHandler], stats: &mut LocalPickerStatistic, ) -> Option { + if !self.enable { + return None; + } + let mut skip_by_pending = false; for sst in select_tables { if level_handlers[self.level].is_pending_compact(&sst.sst_id) { @@ -75,6 +82,10 @@ impl TrivialMovePicker { level_handlers: &[LevelHandler], stats: &mut LocalPickerStatistic, ) -> Option { + if !self.enable { + return None; + } + if let Some(trivial_move_sst) = self.pick_trivial_move_sst(select_tables, target_tables, level_handlers, stats) { From 59289fe9cb33a15b504909281105d5f045ac8a0c Mon Sep 17 00:00:00 2001 From: Li0k Date: Thu, 25 Jan 2024 19:16:29 +0800 Subject: [PATCH 2/8] refactor(storage): refactor config --- proto/hummock.proto | 2 - src/common/src/config.rs | 26 +++++----- src/config/example.toml | 4 +- src/meta/node/src/lib.rs | 5 ++ .../hummock/compaction/compaction_config.rs | 7 --- src/meta/src/hummock/compaction/mod.rs | 19 +++++++ .../picker/base_level_compaction_picker.rs | 49 ++++++++++++++----- .../picker/emergency_compaction_picker.rs | 10 +++- .../picker/intra_compaction_picker.rs | 30 ++++++++---- .../picker/manual_compaction_picker.rs | 6 ++- .../picker/space_reclaim_compaction_picker.rs | 10 ++++ .../picker/ttl_reclaim_compaction_picker.rs | 11 ++++- .../compaction/selector/emergency_selector.rs | 18 +++++-- .../compaction/selector/level_selector.rs | 39 ++++++++++++--- .../compaction/selector/manual_selector.rs | 9 +++- .../src/hummock/compaction/selector/mod.rs | 6 ++- .../selector/space_reclaim_selector.rs | 9 +++- .../selector/tombstone_compaction_selector.rs | 8 ++- .../compaction/selector/ttl_selector.rs | 9 +++- src/meta/src/hummock/manager/compaction.rs | 8 ++- src/meta/src/hummock/manager/mod.rs | 6 ++- src/meta/src/hummock/metrics_utils.rs | 7 ++- src/meta/src/manager/env.rs | 8 +++ 23 files changed, 233 insertions(+), 73 deletions(-) diff --git a/proto/hummock.proto b/proto/hummock.proto index 54d24d6ec8e57..9b39022f7f734 100644 --- a/proto/hummock.proto +++ b/proto/hummock.proto @@ -837,8 +837,6 @@ message CompactionConfig { uint32 level0_overlapping_sub_level_compact_level_count = 18; uint32 tombstone_reclaim_ratio = 19; bool enable_emergency_picker = 20; - bool enable_trivial_move = 21; - bool enable_check_task_level_overlap = 22; } message TableStats { diff --git a/src/common/src/config.rs b/src/common/src/config.rs index 6815590a16c4a..08e1b1ee80563 100644 --- a/src/common/src/config.rs +++ b/src/common/src/config.rs @@ -406,6 +406,12 @@ pub struct MetaDeveloperConfig { /// in the meta node. #[serde(default = "default::developer::meta_cached_traces_memory_limit_bytes")] pub cached_traces_memory_limit_bytes: usize, + + /// Compaction picker config + #[serde(default = "default::developer::enable_trivial_move")] + pub enable_trivial_move: bool, + #[serde(default = "default::developer::enable_check_task_level_overlap")] + pub enable_check_task_level_overlap: bool, } /// The section `[server]` in `risingwave.toml`. @@ -1406,6 +1412,14 @@ pub mod default { pub fn stream_hash_agg_max_dirty_groups_heap_size() -> usize { 64 << 20 // 64MB } + + pub fn enable_trivial_move() -> bool { + true + } + + pub fn enable_check_task_level_overlap() -> bool { + false + } } pub use crate::system_param::default as system; @@ -1491,14 +1505,6 @@ pub mod default { pub fn enable_emergency_picker() -> bool { DEFAULT_EMERGENCY_PICKER } - - pub fn enable_trivial_move() -> bool { - true - } - - pub fn enable_check_task_level_overlap() -> bool { - false - } } pub mod object_store_config { @@ -1652,10 +1658,6 @@ pub struct CompactionConfig { pub tombstone_reclaim_ratio: u32, #[serde(default = "default::compaction_config::enable_emergency_picker")] pub enable_emergency_picker: bool, - #[serde(default = "default::compaction_config::enable_trivial_move")] - pub enable_trivial_move: bool, - #[serde(default = "default::compaction_config::enable_check_task_level_overlap")] - pub enable_check_task_level_overlap: bool, } #[cfg(test)] diff --git a/src/config/example.toml b/src/config/example.toml index 63f514df09615..4337d29573486 100644 --- a/src/config/example.toml +++ b/src/config/example.toml @@ -66,12 +66,12 @@ max_space_reclaim_bytes = 536870912 level0_max_compact_file_number = 100 tombstone_reclaim_ratio = 40 enable_emergency_picker = true -enable_trivial_move = true -enable_check_task_level_overlap = false [meta.developer] meta_cached_traces_num = 256 meta_cached_traces_memory_limit_bytes = 134217728 +meta_enable_trivial_move = true +meta_enable_check_task_level_overlap = false [batch] enable_barrier_read = false diff --git a/src/meta/node/src/lib.rs b/src/meta/node/src/lib.rs index 2989ceaa5f5c6..fb5365edf3e84 100644 --- a/src/meta/node/src/lib.rs +++ b/src/meta/node/src/lib.rs @@ -345,6 +345,11 @@ pub fn start(opts: MetaNodeOpts) -> Pin + Send>> { .meta .developer .cached_traces_memory_limit_bytes, + enable_trivial_move: config.meta.developer.enable_trivial_move, + enable_check_task_level_overlap: config + .meta + .developer + .enable_check_task_level_overlap, }, config.system.into_init_system_params(), ) diff --git a/src/meta/src/hummock/compaction/compaction_config.rs b/src/meta/src/hummock/compaction/compaction_config.rs index bb7498081f00f..6048e301c97d6 100644 --- a/src/meta/src/hummock/compaction/compaction_config.rs +++ b/src/meta/src/hummock/compaction/compaction_config.rs @@ -66,9 +66,6 @@ impl CompactionConfigBuilder { compaction_config::level0_overlapping_sub_level_compact_level_count(), tombstone_reclaim_ratio: compaction_config::tombstone_reclaim_ratio(), enable_emergency_picker: compaction_config::enable_emergency_picker(), - enable_trivial_move: compaction_config::enable_trivial_move(), - enable_check_task_level_overlap: compaction_config::enable_check_task_level_overlap( - ), }, } } @@ -97,8 +94,6 @@ impl CompactionConfigBuilder { .max_space_reclaim_bytes(opt.max_space_reclaim_bytes) .level0_max_compact_file_number(opt.level0_max_compact_file_number) .tombstone_reclaim_ratio(opt.tombstone_reclaim_ratio) - .enable_trivial_move(opt.enable_trivial_move) - .enable_check_task_level_overlap(opt.enable_check_task_level_overlap) } pub fn build(self) -> CompactionConfig { @@ -159,6 +154,4 @@ builder_field! { level0_sub_level_compact_level_count: u32, level0_overlapping_sub_level_compact_level_count: u32, tombstone_reclaim_ratio: u32, - enable_trivial_move: bool, - enable_check_task_level_overlap: bool, } diff --git a/src/meta/src/hummock/compaction/mod.rs b/src/meta/src/hummock/compaction/mod.rs index 562027b91c8da..edf631e74c740 100644 --- a/src/meta/src/hummock/compaction/mod.rs +++ b/src/meta/src/hummock/compaction/mod.rs @@ -104,6 +104,7 @@ impl CompactStatus { stats: &mut LocalSelectorStatistic, selector: &mut Box, table_id_to_options: HashMap, + developer_config: Arc, ) -> Option { // When we compact the files, we must make the result of compaction meet the following // conditions, for any user key, the epoch of it in the file existing in the lower @@ -115,6 +116,7 @@ impl CompactStatus { &mut self.level_handlers, stats, table_id_to_options, + developer_config, ) } @@ -216,3 +218,20 @@ pub fn get_compression_algorithm( compaction_config.compression_algorithm[idx].clone() } } + +pub struct CompactionDeveloperConfig { + /// l0 picker whether to select trivial move task + pub enable_trivial_move: bool, + + /// l0 multi level picker whether to check the overlap accuracy between sub levels + pub enable_check_task_level_overlap: bool, +} + +impl Default for CompactionDeveloperConfig { + fn default() -> Self { + Self { + enable_trivial_move: true, + enable_check_task_level_overlap: true, + } + } +} diff --git a/src/meta/src/hummock/compaction/picker/base_level_compaction_picker.rs b/src/meta/src/hummock/compaction/picker/base_level_compaction_picker.rs index b17e127dbb8f5..a298f57581341 100644 --- a/src/meta/src/hummock/compaction/picker/base_level_compaction_picker.rs +++ b/src/meta/src/hummock/compaction/picker/base_level_compaction_picker.rs @@ -24,14 +24,15 @@ use super::{ CompactionInput, CompactionPicker, CompactionTaskValidator, LocalPickerStatistic, ValidationRuleType, }; -use crate::hummock::compaction::create_overlap_strategy; use crate::hummock::compaction::picker::TrivialMovePicker; +use crate::hummock::compaction::{create_overlap_strategy, CompactionDeveloperConfig}; use crate::hummock::level_handler::LevelHandler; pub struct LevelCompactionPicker { target_level: usize, config: Arc, compaction_task_validator: Arc, + developer_config: Arc, } impl CompactionPicker for LevelCompactionPicker { @@ -87,11 +88,16 @@ impl CompactionPicker for LevelCompactionPicker { impl LevelCompactionPicker { #[cfg(test)] - pub fn new(target_level: usize, config: Arc) -> LevelCompactionPicker { + pub fn new( + target_level: usize, + config: Arc, + developer_config: Arc, + ) -> LevelCompactionPicker { LevelCompactionPicker { target_level, compaction_task_validator: Arc::new(CompactionTaskValidator::new(config.clone())), config, + developer_config, } } @@ -99,11 +105,13 @@ impl LevelCompactionPicker { target_level: usize, config: Arc, compaction_task_validator: Arc, + developer_config: Arc, ) -> LevelCompactionPicker { LevelCompactionPicker { target_level, config, compaction_task_validator, + developer_config, } } @@ -119,7 +127,7 @@ impl LevelCompactionPicker { 0, self.target_level, overlap_strategy.clone(), - self.config.enable_trivial_move, + self.developer_config.enable_trivial_move, ); trivial_move_picker.pick_trivial_move_task( @@ -152,7 +160,7 @@ impl LevelCompactionPicker { // The maximum number of sub_level compact level per task self.config.level0_max_compact_file_number, overlap_strategy.clone(), - self.config.enable_check_task_level_overlap, + self.developer_config.enable_check_task_level_overlap, ); let mut max_vnode_partition_idx = 0; @@ -269,7 +277,9 @@ pub mod tests { use super::*; use crate::hummock::compaction::compaction_config::CompactionConfigBuilder; use crate::hummock::compaction::selector::tests::*; - use crate::hummock::compaction::{CompactionMode, TierCompactionPicker}; + use crate::hummock::compaction::{ + CompactionDeveloperConfig, CompactionMode, TierCompactionPicker, + }; fn create_compaction_picker_for_test() -> LevelCompactionPicker { let config = Arc::new( @@ -278,7 +288,7 @@ pub mod tests { .level0_sub_level_compact_level_count(1) .build(), ); - LevelCompactionPicker::new(1, config) + LevelCompactionPicker::new(1, config, Arc::new(CompactionDeveloperConfig::default())) } #[test] @@ -378,7 +388,8 @@ pub mod tests { .level0_sub_level_compact_level_count(1) .build(), ); - let mut picker = LevelCompactionPicker::new(1, config); + let mut picker = + LevelCompactionPicker::new(1, config, Arc::new(CompactionDeveloperConfig::default())); let levels = vec![Level { level_idx: 1, @@ -502,7 +513,11 @@ pub mod tests { .max_bytes_for_level_multiplier(1) .level0_sub_level_compact_level_count(2) .build(); - let mut picker = LevelCompactionPicker::new(1, Arc::new(config)); + let mut picker = LevelCompactionPicker::new( + 1, + Arc::new(config), + Arc::new(CompactionDeveloperConfig::default()), + ); let mut levels = Levels { levels: vec![Level { @@ -576,7 +591,8 @@ pub mod tests { ); // Only include sub-level 0 results will violate MAX_WRITE_AMPLIFICATION. // So all sub-levels are included to make write amplification < MAX_WRITE_AMPLIFICATION. - let mut picker = LevelCompactionPicker::new(1, config); + let mut picker = + LevelCompactionPicker::new(1, config, Arc::new(CompactionDeveloperConfig::default())); let ret = picker .pick_compaction(&levels, &levels_handler, &mut local_stats) .unwrap(); @@ -602,7 +618,8 @@ pub mod tests { .level0_sub_level_compact_level_count(1) .build(), ); - let mut picker = LevelCompactionPicker::new(1, config); + let mut picker = + LevelCompactionPicker::new(1, config, Arc::new(CompactionDeveloperConfig::default())); let ret = picker .pick_compaction(&levels, &levels_handler, &mut local_stats) .unwrap(); @@ -668,7 +685,11 @@ pub mod tests { // Only include sub-level 0 results will violate MAX_WRITE_AMPLIFICATION. // But stopped by pending sub-level when trying to include more sub-levels. - let mut picker = LevelCompactionPicker::new(1, config.clone()); + let mut picker = LevelCompactionPicker::new( + 1, + config.clone(), + Arc::new(CompactionDeveloperConfig::default()), + ); let ret = picker.pick_compaction(&levels, &levels_handler, &mut local_stats); assert!(ret.is_none()); @@ -678,7 +699,8 @@ pub mod tests { } // No more pending sub-level so we can get a task now. - let mut picker = LevelCompactionPicker::new(1, config); + let mut picker = + LevelCompactionPicker::new(1, config, Arc::new(CompactionDeveloperConfig::default())); picker .pick_compaction(&levels, &levels_handler, &mut local_stats) .unwrap(); @@ -711,7 +733,8 @@ pub mod tests { .build(), ); - let mut picker = LevelCompactionPicker::new(1, config); + let mut picker = + LevelCompactionPicker::new(1, config, Arc::new(CompactionDeveloperConfig::default())); let ret = picker .pick_compaction(&levels, &levels_handler, &mut local_stats) .unwrap(); diff --git a/src/meta/src/hummock/compaction/picker/emergency_compaction_picker.rs b/src/meta/src/hummock/compaction/picker/emergency_compaction_picker.rs index 9ff6da2d0f7d0..c6380ef476cc2 100644 --- a/src/meta/src/hummock/compaction/picker/emergency_compaction_picker.rs +++ b/src/meta/src/hummock/compaction/picker/emergency_compaction_picker.rs @@ -21,18 +21,25 @@ use super::{ CompactionInput, CompactionPicker, CompactionTaskValidator, LevelCompactionPicker, LocalPickerStatistic, TierCompactionPicker, }; +use crate::hummock::compaction::CompactionDeveloperConfig; use crate::hummock::level_handler::LevelHandler; pub struct EmergencyCompactionPicker { target_level: usize, config: Arc, + developer_config: Arc, } impl EmergencyCompactionPicker { - pub fn new(target_level: usize, config: Arc) -> Self { + pub fn new( + target_level: usize, + config: Arc, + developer_config: Arc, + ) -> Self { Self { target_level, config, + developer_config, } } @@ -48,6 +55,7 @@ impl EmergencyCompactionPicker { self.target_level, self.config.clone(), unused_validator.clone(), + self.developer_config.clone(), ); if let Some(ret) = diff --git a/src/meta/src/hummock/compaction/picker/intra_compaction_picker.rs b/src/meta/src/hummock/compaction/picker/intra_compaction_picker.rs index 651c030252bd4..cb9bd32610fbe 100644 --- a/src/meta/src/hummock/compaction/picker/intra_compaction_picker.rs +++ b/src/meta/src/hummock/compaction/picker/intra_compaction_picker.rs @@ -22,13 +22,15 @@ use super::{ CompactionInput, CompactionPicker, CompactionTaskValidator, LocalPickerStatistic, ValidationRuleType, }; -use crate::hummock::compaction::create_overlap_strategy; use crate::hummock::compaction::picker::TrivialMovePicker; +use crate::hummock::compaction::{create_overlap_strategy, CompactionDeveloperConfig}; use crate::hummock::level_handler::LevelHandler; pub struct IntraCompactionPicker { config: Arc, compaction_task_validator: Arc, + + developer_config: Arc, } impl CompactionPicker for IntraCompactionPicker { @@ -76,20 +78,26 @@ impl CompactionPicker for IntraCompactionPicker { impl IntraCompactionPicker { #[cfg(test)] - pub fn new(config: Arc) -> IntraCompactionPicker { + pub fn new( + config: Arc, + developer_config: Arc, + ) -> IntraCompactionPicker { IntraCompactionPicker { compaction_task_validator: Arc::new(CompactionTaskValidator::new(config.clone())), config, + developer_config, } } pub fn new_with_validator( config: Arc, compaction_task_validator: Arc, + developer_config: Arc, ) -> IntraCompactionPicker { IntraCompactionPicker { config, compaction_task_validator, + developer_config, } } @@ -216,7 +224,7 @@ impl IntraCompactionPicker { self.config.level0_sub_level_compact_level_count as usize, self.config.level0_max_compact_file_number, overlap_strategy.clone(), - self.config.enable_check_task_level_overlap, + self.developer_config.enable_check_task_level_overlap, ); let l0_select_tables_vec = non_overlap_sub_level_picker @@ -308,7 +316,7 @@ impl IntraCompactionPicker { 0, 0, overlap_strategy.clone(), - self.config.enable_trivial_move, + self.developer_config.enable_trivial_move, ); let select_sst = trivial_move_picker.pick_trivial_move_sst( @@ -391,7 +399,7 @@ pub mod tests { .level0_sub_level_compact_level_count(1) .build(), ); - IntraCompactionPicker::new(config) + IntraCompactionPicker::new(config, Arc::new(CompactionDeveloperConfig::default())) } #[test] @@ -505,7 +513,8 @@ pub mod tests { .level0_overlapping_sub_level_compact_level_count(4) .build(), ); - let mut picker = IntraCompactionPicker::new(config); + let mut picker = + IntraCompactionPicker::new(config, Arc::new(CompactionDeveloperConfig::default())); let mut local_stats = LocalPickerStatistic::default(); let ret = picker .pick_compaction(&levels, &levels_handler, &mut local_stats) @@ -550,7 +559,8 @@ pub mod tests { .level0_sub_level_compact_level_count(1) .build(), ); - let mut picker = IntraCompactionPicker::new(config); + let mut picker = + IntraCompactionPicker::new(config, Arc::new(CompactionDeveloperConfig::default())); let mut local_stats = LocalPickerStatistic::default(); let ret = picker .pick_compaction(&levels, &levels_handler, &mut local_stats) @@ -619,7 +629,8 @@ pub mod tests { .level0_sub_level_compact_level_count(1) .build(), ); - let mut picker = IntraCompactionPicker::new(config); + let mut picker = + IntraCompactionPicker::new(config, Arc::new(CompactionDeveloperConfig::default())); let mut local_stats = LocalPickerStatistic::default(); let ret = picker .pick_compaction(&levels, &levels_handler, &mut local_stats) @@ -671,7 +682,8 @@ pub mod tests { .level0_sub_level_compact_level_count(20) // reject intra .build(), ); - let mut picker = IntraCompactionPicker::new(config); + let mut picker = + IntraCompactionPicker::new(config, Arc::new(CompactionDeveloperConfig::default())); // Cannot trivial move because there is only 1 sub-level. let l0 = generate_l0_overlapping_sublevels(vec![vec![ diff --git a/src/meta/src/hummock/compaction/picker/manual_compaction_picker.rs b/src/meta/src/hummock/compaction/picker/manual_compaction_picker.rs index b5f187a3253d0..27a00b05ffcc4 100644 --- a/src/meta/src/hummock/compaction/picker/manual_compaction_picker.rs +++ b/src/meta/src/hummock/compaction/picker/manual_compaction_picker.rs @@ -339,7 +339,7 @@ pub mod tests { generate_l0_overlapping_sublevels, generate_level, generate_table, }; use crate::hummock::compaction::selector::{CompactionSelector, ManualCompactionSelector}; - use crate::hummock::compaction::LocalSelectorStatistic; + use crate::hummock::compaction::{CompactionDeveloperConfig, LocalSelectorStatistic}; use crate::hummock::model::CompactionGroup; use crate::hummock::test_utils::iterator_test_key_of_epoch; @@ -1201,6 +1201,7 @@ pub mod tests { &mut levels_handler, &mut local_stats, HashMap::default(), + Arc::new(CompactionDeveloperConfig::default()), ) .unwrap(); assert_compaction_task(&task, &levels_handler); @@ -1237,6 +1238,7 @@ pub mod tests { &mut levels_handler, &mut local_stats, HashMap::default(), + Arc::new(CompactionDeveloperConfig::default()), ) .unwrap(); assert_compaction_task(&task, &levels_handler); @@ -1309,6 +1311,7 @@ pub mod tests { &mut levels_handler, &mut local_stats, HashMap::default(), + Arc::new(CompactionDeveloperConfig::default()), ) .unwrap(); assert_compaction_task(&task, &levels_handler); @@ -1347,6 +1350,7 @@ pub mod tests { &mut levels_handler, &mut local_stats, HashMap::default(), + Arc::new(CompactionDeveloperConfig::default()), ) .unwrap(); assert_compaction_task(&task, &levels_handler); diff --git a/src/meta/src/hummock/compaction/picker/space_reclaim_compaction_picker.rs b/src/meta/src/hummock/compaction/picker/space_reclaim_compaction_picker.rs index e14baaf086c1f..506815dbed40a 100644 --- a/src/meta/src/hummock/compaction/picker/space_reclaim_compaction_picker.rs +++ b/src/meta/src/hummock/compaction/picker/space_reclaim_compaction_picker.rs @@ -169,6 +169,7 @@ impl SpaceReclaimCompactionPicker { mod test { use std::collections::HashMap; + use std::sync::Arc; use itertools::Itertools; use risingwave_pb::hummock::compact_task; @@ -183,6 +184,7 @@ mod test { use crate::hummock::compaction::selector::{ CompactionSelector, LocalSelectorStatistic, SpaceReclaimCompactionSelector, }; + use crate::hummock::compaction::CompactionDeveloperConfig; use crate::hummock::model::CompactionGroup; #[test] @@ -253,6 +255,7 @@ mod test { &mut levels_handler, &mut local_stats, HashMap::default(), + Arc::new(CompactionDeveloperConfig::default()), ) .unwrap(); assert_compaction_task(&task, &levels_handler); @@ -268,6 +271,7 @@ mod test { &mut levels_handler, &mut local_stats, HashMap::default(), + Arc::new(CompactionDeveloperConfig::default()), ) .unwrap(); assert_eq!(task.input.input_levels.len(), 2); @@ -307,6 +311,7 @@ mod test { &mut levels_handler, &mut local_stats, HashMap::default(), + Arc::new(CompactionDeveloperConfig::default()), ) .unwrap(); assert_compaction_task(&task, &levels_handler); @@ -332,6 +337,7 @@ mod test { &mut levels_handler, &mut local_stats, HashMap::default(), + Arc::new(CompactionDeveloperConfig::default()), ) .is_none()) } @@ -354,6 +360,7 @@ mod test { &mut levels_handler, &mut local_stats, HashMap::default(), + Arc::new(CompactionDeveloperConfig::default()), ); assert!(task.is_none()); } @@ -375,6 +382,7 @@ mod test { &mut levels_handler, &mut local_stats, HashMap::default(), + Arc::new(CompactionDeveloperConfig::default()), ) .unwrap(); assert_compaction_task(&task, &levels_handler); @@ -412,6 +420,7 @@ mod test { &mut levels_handler, &mut local_stats, HashMap::default(), + Arc::new(CompactionDeveloperConfig::default()), ) .unwrap(); @@ -466,6 +475,7 @@ mod test { &mut levels_handler, &mut local_stats, HashMap::default(), + Arc::new(CompactionDeveloperConfig::default()), ) .unwrap(); diff --git a/src/meta/src/hummock/compaction/picker/ttl_reclaim_compaction_picker.rs b/src/meta/src/hummock/compaction/picker/ttl_reclaim_compaction_picker.rs index 4f7d4b2bd5da8..2e20b129b1c39 100644 --- a/src/meta/src/hummock/compaction/picker/ttl_reclaim_compaction_picker.rs +++ b/src/meta/src/hummock/compaction/picker/ttl_reclaim_compaction_picker.rs @@ -202,6 +202,8 @@ impl TtlReclaimCompactionPicker { #[cfg(test)] mod test { + use std::sync::Arc; + use itertools::Itertools; use risingwave_pb::hummock::compact_task; pub use risingwave_pb::hummock::{Level, LevelType}; @@ -213,7 +215,7 @@ mod test { generate_table_with_ids_and_epochs, }; use crate::hummock::compaction::selector::{CompactionSelector, TtlCompactionSelector}; - use crate::hummock::compaction::LocalSelectorStatistic; + use crate::hummock::compaction::{CompactionDeveloperConfig, LocalSelectorStatistic}; use crate::hummock::model::CompactionGroup; #[test] @@ -379,6 +381,7 @@ mod test { &mut levels_handler, &mut local_stats, table_id_to_options, + Arc::new(CompactionDeveloperConfig::default()), ) .unwrap(); assert_compaction_task(&task, &levels_handler); @@ -428,6 +431,7 @@ mod test { &mut levels_handler, &mut local_stats, table_id_to_options.clone(), + Arc::new(CompactionDeveloperConfig::default()), ) .unwrap(); assert_compaction_task(&task, &levels_handler); @@ -460,6 +464,7 @@ mod test { &mut levels_handler, &mut local_stats, table_id_to_options.clone(), + Arc::new(CompactionDeveloperConfig::default()), ) .unwrap(); assert_compaction_task(&task, &levels_handler); @@ -515,6 +520,7 @@ mod test { &mut levels_handler, &mut local_stats, table_id_to_options, + Arc::new(CompactionDeveloperConfig::default()), ) .unwrap(); assert_compaction_task(&task, &levels_handler); @@ -555,6 +561,7 @@ mod test { &mut levels_handler, &mut local_stats, HashMap::default(), + Arc::new(CompactionDeveloperConfig::default()), ); // empty table_options does not select any files @@ -616,6 +623,7 @@ mod test { &mut levels_handler, &mut local_stats, table_id_to_options.clone(), + Arc::new(CompactionDeveloperConfig::default()), ) .unwrap(); @@ -707,6 +715,7 @@ mod test { &mut levels_handler, &mut local_stats, table_id_to_options.clone(), + Arc::new(CompactionDeveloperConfig::default()), ) .unwrap(); diff --git a/src/meta/src/hummock/compaction/selector/emergency_selector.rs b/src/meta/src/hummock/compaction/selector/emergency_selector.rs index 7b8897a891246..c386aee5c8644 100644 --- a/src/meta/src/hummock/compaction/selector/emergency_selector.rs +++ b/src/meta/src/hummock/compaction/selector/emergency_selector.rs @@ -13,6 +13,7 @@ // limitations under the License. use std::collections::HashMap; +use std::sync::Arc; use risingwave_common::catalog::TableOption; use risingwave_hummock_sdk::HummockCompactionTaskId; @@ -21,7 +22,9 @@ use risingwave_pb::hummock::hummock_version::Levels; use super::{CompactionSelector, DynamicLevelSelectorCore, LocalSelectorStatistic}; use crate::hummock::compaction::picker::{EmergencyCompactionPicker, LocalPickerStatistic}; -use crate::hummock::compaction::{create_compaction_task, CompactionTask}; +use crate::hummock::compaction::{ + create_compaction_task, CompactionDeveloperConfig, CompactionTask, +}; use crate::hummock::level_handler::LevelHandler; use crate::hummock::model::CompactionGroup; @@ -37,11 +40,18 @@ impl CompactionSelector for EmergencySelector { level_handlers: &mut [LevelHandler], selector_stats: &mut LocalSelectorStatistic, _table_id_to_options: HashMap, + developer_config: Arc, ) -> Option { - let dynamic_level_core = DynamicLevelSelectorCore::new(group.compaction_config.clone()); + let dynamic_level_core = DynamicLevelSelectorCore::new( + group.compaction_config.clone(), + developer_config.clone(), + ); let ctx = dynamic_level_core.calculate_level_base_size(levels); - let picker = - EmergencyCompactionPicker::new(ctx.base_level, group.compaction_config.clone()); + let picker = EmergencyCompactionPicker::new( + ctx.base_level, + group.compaction_config.clone(), + developer_config, + ); let mut stats = LocalPickerStatistic::default(); if let Some(compaction_input) = picker.pick_compaction(levels, level_handlers, &mut stats) { diff --git a/src/meta/src/hummock/compaction/selector/level_selector.rs b/src/meta/src/hummock/compaction/selector/level_selector.rs index 39f3a1acce875..a24f88cbf0fc6 100644 --- a/src/meta/src/hummock/compaction/selector/level_selector.rs +++ b/src/meta/src/hummock/compaction/selector/level_selector.rs @@ -33,7 +33,9 @@ use crate::hummock::compaction::picker::{ CompactionPicker, CompactionTaskValidator, IntraCompactionPicker, LocalPickerStatistic, MinOverlappingPicker, }; -use crate::hummock::compaction::{create_overlap_strategy, CompactionTask, LocalSelectorStatistic}; +use crate::hummock::compaction::{ + create_overlap_strategy, CompactionDeveloperConfig, CompactionTask, LocalSelectorStatistic, +}; use crate::hummock::level_handler::LevelHandler; use crate::hummock::model::CompactionGroup; @@ -81,14 +83,21 @@ pub struct SelectContext { pub struct DynamicLevelSelectorCore { config: Arc, + developer_config: Arc, } #[derive(Default)] pub struct DynamicLevelSelector {} impl DynamicLevelSelectorCore { - pub fn new(config: Arc) -> Self { - Self { config } + pub fn new( + config: Arc, + developer_config: Arc, + ) -> Self { + Self { + config, + developer_config, + } } pub fn get_config(&self) -> &CompactionConfig { @@ -110,10 +119,12 @@ impl DynamicLevelSelectorCore { picker_info.target_level, self.config.clone(), compaction_task_validator, + self.developer_config.clone(), )), PickerType::Intra => Box::new(IntraCompactionPicker::new_with_validator( self.config.clone(), compaction_task_validator, + self.developer_config.clone(), )), PickerType::BottomLevel => { assert_eq!(picker_info.select_level + 1, picker_info.target_level); @@ -419,9 +430,12 @@ impl CompactionSelector for DynamicLevelSelector { level_handlers: &mut [LevelHandler], selector_stats: &mut LocalSelectorStatistic, _table_id_to_options: HashMap, + developer_config: Arc, ) -> Option { - let dynamic_level_core = - DynamicLevelSelectorCore::new(compaction_group.compaction_config.clone()); + let dynamic_level_core = DynamicLevelSelectorCore::new( + compaction_group.compaction_config.clone(), + developer_config, + ); let overlap_strategy = create_overlap_strategy(compaction_group.compaction_config.compaction_mode()); let ctx = dynamic_level_core.get_priority_levels(levels, level_handlers); @@ -485,6 +499,7 @@ pub mod tests { use crate::hummock::compaction::selector::{ CompactionSelector, DynamicLevelSelector, DynamicLevelSelectorCore, LocalSelectorStatistic, }; + use crate::hummock::compaction::CompactionDeveloperConfig; use crate::hummock::level_handler::LevelHandler; use crate::hummock::model::CompactionGroup; @@ -498,7 +513,10 @@ pub mod tests { .level0_tier_compact_file_number(2) .compaction_mode(CompactionMode::Range as i32) .build(); - let selector = DynamicLevelSelectorCore::new(Arc::new(config)); + let selector = DynamicLevelSelectorCore::new( + Arc::new(config), + Arc::new(CompactionDeveloperConfig::default()), + ); let levels = vec![ generate_level(1, vec![]), generate_level(2, generate_tables(0..5, 0..1000, 3, 10)), @@ -602,6 +620,7 @@ pub mod tests { &mut levels_handlers, &mut local_stats, HashMap::default(), + Arc::new(CompactionDeveloperConfig::default()), ) .unwrap(); assert_compaction_task(&compaction, &levels_handlers); @@ -628,6 +647,7 @@ pub mod tests { &mut levels_handlers, &mut local_stats, HashMap::default(), + Arc::new(CompactionDeveloperConfig::default()), ) .unwrap(); assert_compaction_task(&compaction, &levels_handlers); @@ -646,6 +666,7 @@ pub mod tests { &mut levels_handlers, &mut local_stats, HashMap::default(), + Arc::new(CompactionDeveloperConfig::default()), ) .unwrap(); assert_compaction_task(&compaction, &levels_handlers); @@ -681,6 +702,7 @@ pub mod tests { &mut levels_handlers, &mut local_stats, HashMap::default(), + Arc::new(CompactionDeveloperConfig::default()), ); assert!(compaction.is_none()); } @@ -710,7 +732,10 @@ pub mod tests { ..Default::default() }; - let dynamic_level_core = DynamicLevelSelectorCore::new(Arc::new(config)); + let dynamic_level_core = DynamicLevelSelectorCore::new( + Arc::new(config), + Arc::new(CompactionDeveloperConfig::default()), + ); let ctx = dynamic_level_core.calculate_level_base_size(&levels); assert_eq!(1, ctx.base_level); assert_eq!(1000, levels.l0.as_ref().unwrap().total_file_size); // l0 diff --git a/src/meta/src/hummock/compaction/selector/manual_selector.rs b/src/meta/src/hummock/compaction/selector/manual_selector.rs index 175ef49085fdd..427efadf3914d 100644 --- a/src/meta/src/hummock/compaction/selector/manual_selector.rs +++ b/src/meta/src/hummock/compaction/selector/manual_selector.rs @@ -18,6 +18,7 @@ // (found in the LICENSE.Apache file in the root directory). use std::collections::{HashMap, HashSet}; +use std::sync::Arc; use risingwave_common::catalog::TableOption; use risingwave_hummock_sdk::compaction_group::StateTableId; @@ -29,7 +30,9 @@ use super::{CompactionSelector, DynamicLevelSelectorCore, LocalSelectorStatistic use crate::hummock::compaction::picker::{ CompactionPicker, LocalPickerStatistic, ManualCompactionPicker, }; -use crate::hummock::compaction::{create_compaction_task, create_overlap_strategy, CompactionTask}; +use crate::hummock::compaction::{ + create_compaction_task, create_overlap_strategy, CompactionDeveloperConfig, CompactionTask, +}; use crate::hummock::level_handler::LevelHandler; use crate::hummock::model::CompactionGroup; @@ -79,8 +82,10 @@ impl CompactionSelector for ManualCompactionSelector { level_handlers: &mut [LevelHandler], _selector_stats: &mut LocalSelectorStatistic, _table_id_to_options: HashMap, + developer_config: Arc, ) -> Option { - let dynamic_level_core = DynamicLevelSelectorCore::new(group.compaction_config.clone()); + let dynamic_level_core = + DynamicLevelSelectorCore::new(group.compaction_config.clone(), developer_config); let overlap_strategy = create_overlap_strategy(group.compaction_config.compaction_mode()); let ctx = dynamic_level_core.calculate_level_base_size(levels); let (mut picker, base_level) = { diff --git a/src/meta/src/hummock/compaction/selector/mod.rs b/src/meta/src/hummock/compaction/selector/mod.rs index b6900bb5cbbe8..a342a661ecb7b 100644 --- a/src/meta/src/hummock/compaction/selector/mod.rs +++ b/src/meta/src/hummock/compaction/selector/mod.rs @@ -25,6 +25,7 @@ mod tombstone_compaction_selector; mod ttl_selector; use std::collections::HashMap; +use std::sync::Arc; pub use emergency_selector::EmergencySelector; pub use level_selector::{DynamicLevelSelector, DynamicLevelSelectorCore}; @@ -38,7 +39,9 @@ pub use tombstone_compaction_selector::TombstoneCompactionSelector; pub use ttl_selector::TtlCompactionSelector; use super::picker::LocalPickerStatistic; -use super::{create_compaction_task, LevelCompactionPicker, TierCompactionPicker}; +use super::{ + create_compaction_task, CompactionDeveloperConfig, LevelCompactionPicker, TierCompactionPicker, +}; use crate::hummock::compaction::CompactionTask; use crate::hummock::level_handler::LevelHandler; use crate::hummock::model::CompactionGroup; @@ -53,6 +56,7 @@ pub trait CompactionSelector: Sync + Send { level_handlers: &mut [LevelHandler], selector_stats: &mut LocalSelectorStatistic, table_id_to_options: HashMap, + developer_config: Arc, ) -> Option; fn report_statistic_metrics(&self, _metrics: &MetaMetrics) {} diff --git a/src/meta/src/hummock/compaction/selector/space_reclaim_selector.rs b/src/meta/src/hummock/compaction/selector/space_reclaim_selector.rs index d48678e0b32a4..c48cb0fe605c5 100644 --- a/src/meta/src/hummock/compaction/selector/space_reclaim_selector.rs +++ b/src/meta/src/hummock/compaction/selector/space_reclaim_selector.rs @@ -18,6 +18,7 @@ // (found in the LICENSE.Apache file in the root directory). use std::collections::HashMap; +use std::sync::Arc; use risingwave_common::catalog::TableOption; use risingwave_hummock_sdk::HummockCompactionTaskId; @@ -26,7 +27,9 @@ use risingwave_pb::hummock::hummock_version::Levels; use super::{CompactionSelector, DynamicLevelSelectorCore}; use crate::hummock::compaction::picker::{SpaceReclaimCompactionPicker, SpaceReclaimPickerState}; -use crate::hummock::compaction::{create_compaction_task, CompactionTask, LocalSelectorStatistic}; +use crate::hummock::compaction::{ + create_compaction_task, CompactionDeveloperConfig, CompactionTask, LocalSelectorStatistic, +}; use crate::hummock::level_handler::LevelHandler; use crate::hummock::model::CompactionGroup; @@ -44,8 +47,10 @@ impl CompactionSelector for SpaceReclaimCompactionSelector { level_handlers: &mut [LevelHandler], _selector_stats: &mut LocalSelectorStatistic, _table_id_to_options: HashMap, + developer_config: Arc, ) -> Option { - let dynamic_level_core = DynamicLevelSelectorCore::new(group.compaction_config.clone()); + let dynamic_level_core = + DynamicLevelSelectorCore::new(group.compaction_config.clone(), developer_config); let mut picker = SpaceReclaimCompactionPicker::new( group.compaction_config.max_space_reclaim_bytes, levels.member_table_ids.iter().cloned().collect(), diff --git a/src/meta/src/hummock/compaction/selector/tombstone_compaction_selector.rs b/src/meta/src/hummock/compaction/selector/tombstone_compaction_selector.rs index 7445e53e73191..5cbae609caa86 100644 --- a/src/meta/src/hummock/compaction/selector/tombstone_compaction_selector.rs +++ b/src/meta/src/hummock/compaction/selector/tombstone_compaction_selector.rs @@ -13,6 +13,7 @@ // limitations under the License. use std::collections::HashMap; +use std::sync::Arc; use risingwave_common::catalog::TableOption; use risingwave_hummock_sdk::HummockCompactionTaskId; @@ -24,7 +25,8 @@ use crate::hummock::compaction::picker::{ TombstoneReclaimCompactionPicker, TombstoneReclaimPickerState, }; use crate::hummock::compaction::{ - create_compaction_task, create_overlap_strategy, CompactionTask, LocalSelectorStatistic, + create_compaction_task, create_overlap_strategy, CompactionDeveloperConfig, CompactionTask, + LocalSelectorStatistic, }; use crate::hummock::level_handler::LevelHandler; use crate::hummock::model::CompactionGroup; @@ -43,13 +45,15 @@ impl CompactionSelector for TombstoneCompactionSelector { level_handlers: &mut [LevelHandler], _selector_stats: &mut LocalSelectorStatistic, _table_id_to_options: HashMap, + developer_config: Arc, ) -> Option { if group.compaction_config.tombstone_reclaim_ratio == 0 { // it might cause full-compaction when tombstone_reclaim_ratio == 0 return None; } - let dynamic_level_core = DynamicLevelSelectorCore::new(group.compaction_config.clone()); + let dynamic_level_core = + DynamicLevelSelectorCore::new(group.compaction_config.clone(), developer_config); let ctx = dynamic_level_core.calculate_level_base_size(levels); let picker = TombstoneReclaimCompactionPicker::new( create_overlap_strategy(group.compaction_config.compaction_mode()), diff --git a/src/meta/src/hummock/compaction/selector/ttl_selector.rs b/src/meta/src/hummock/compaction/selector/ttl_selector.rs index 0989e8f8b3c44..ed099ede4158f 100644 --- a/src/meta/src/hummock/compaction/selector/ttl_selector.rs +++ b/src/meta/src/hummock/compaction/selector/ttl_selector.rs @@ -18,6 +18,7 @@ // (found in the LICENSE.Apache file in the root directory). use std::collections::HashMap; +use std::sync::Arc; use risingwave_common::catalog::TableOption; use risingwave_hummock_sdk::HummockCompactionTaskId; @@ -26,7 +27,9 @@ use risingwave_pb::hummock::hummock_version::Levels; use super::{CompactionSelector, DynamicLevelSelectorCore}; use crate::hummock::compaction::picker::{TtlPickerState, TtlReclaimCompactionPicker}; -use crate::hummock::compaction::{create_compaction_task, CompactionTask, LocalSelectorStatistic}; +use crate::hummock::compaction::{ + create_compaction_task, CompactionDeveloperConfig, CompactionTask, LocalSelectorStatistic, +}; use crate::hummock::level_handler::LevelHandler; use crate::hummock::model::CompactionGroup; @@ -44,8 +47,10 @@ impl CompactionSelector for TtlCompactionSelector { level_handlers: &mut [LevelHandler], _selector_stats: &mut LocalSelectorStatistic, table_id_to_options: HashMap, + developer_config: Arc, ) -> Option { - let dynamic_level_core = DynamicLevelSelectorCore::new(group.compaction_config.clone()); + let dynamic_level_core = + DynamicLevelSelectorCore::new(group.compaction_config.clone(), developer_config); let ctx = dynamic_level_core.calculate_level_base_size(levels); let picker = TtlReclaimCompactionPicker::new(table_id_to_options); let state = self.state.entry(group.group_id).or_default(); diff --git a/src/meta/src/hummock/manager/compaction.rs b/src/meta/src/hummock/manager/compaction.rs index c1cd3f5784024..e0322a3a6a351 100644 --- a/src/meta/src/hummock/manager/compaction.rs +++ b/src/meta/src/hummock/manager/compaction.rs @@ -13,6 +13,7 @@ // limitations under the License. use std::collections::BTreeMap; +use std::sync::Arc; use function_name::named; use itertools::Itertools; @@ -21,7 +22,7 @@ use risingwave_pb::hummock::{CompactStatus as PbCompactStatus, CompactTaskAssign use crate::hummock::compaction::selector::level_selector::PickerInfo; use crate::hummock::compaction::selector::DynamicLevelSelectorCore; -use crate::hummock::compaction::CompactStatus; +use crate::hummock::compaction::{CompactStatus, CompactionDeveloperConfig}; use crate::hummock::manager::read_lock; use crate::hummock::HummockManager; @@ -94,7 +95,10 @@ impl HummockManager { } } }; - let dynamic_level_core = DynamicLevelSelectorCore::new(config.compaction_config); + let dynamic_level_core = DynamicLevelSelectorCore::new( + config.compaction_config, + Arc::new(CompactionDeveloperConfig::default()), + ); let ctx = dynamic_level_core.get_priority_levels(&levels, &status.level_handlers); ctx.score_levels } diff --git a/src/meta/src/hummock/manager/mod.rs b/src/meta/src/hummock/manager/mod.rs index 25453d0932bc7..7ace244b862ff 100644 --- a/src/meta/src/hummock/manager/mod.rs +++ b/src/meta/src/hummock/manager/mod.rs @@ -79,7 +79,7 @@ use crate::hummock::compaction::selector::{ DynamicLevelSelector, LocalSelectorStatistic, ManualCompactionOption, ManualCompactionSelector, SpaceReclaimCompactionSelector, TombstoneCompactionSelector, TtlCompactionSelector, }; -use crate::hummock::compaction::CompactStatus; +use crate::hummock::compaction::{CompactStatus, CompactionDeveloperConfig}; use crate::hummock::error::{Error, Result}; use crate::hummock::metrics_utils::{ build_compact_task_level_type_metrics_label, trigger_delta_log_stats, trigger_lsm_stat, @@ -955,6 +955,10 @@ impl HummockManager { &mut stats, selector, table_id_to_option.clone(), + Arc::new(CompactionDeveloperConfig { + enable_trivial_move: self.env.opts.enable_trivial_move, + enable_check_task_level_overlap: self.env.opts.enable_check_task_level_overlap, + }), ); stats.report_to_metrics(compaction_group_id, self.metrics.as_ref()); let compact_task = match compact_task { diff --git a/src/meta/src/hummock/metrics_utils.rs b/src/meta/src/hummock/metrics_utils.rs index 38377d4eb440e..f352ae02d5d3a 100644 --- a/src/meta/src/hummock/metrics_utils.rs +++ b/src/meta/src/hummock/metrics_utils.rs @@ -31,8 +31,8 @@ use risingwave_pb::hummock::{ CompactionConfig, HummockPinnedSnapshot, HummockPinnedVersion, HummockVersionStats, LevelType, }; -use super::compaction::get_compression_algorithm; use super::compaction::selector::DynamicLevelSelectorCore; +use super::compaction::{get_compression_algorithm, CompactionDeveloperConfig}; use crate::hummock::checkpoint::HummockVersionCheckpoint; use crate::hummock::compaction::CompactStatus; use crate::rpc::metrics::MetaMetrics; @@ -435,7 +435,10 @@ pub fn trigger_lsm_stat( ) { let group_label = compaction_group_id.to_string(); // compact_pending_bytes - let dynamic_level_core = DynamicLevelSelectorCore::new(compaction_config.clone()); + let dynamic_level_core = DynamicLevelSelectorCore::new( + compaction_config.clone(), + Arc::new(CompactionDeveloperConfig::default()), + ); let ctx = dynamic_level_core.calculate_level_base_size(levels); { let compact_pending_bytes_needed = diff --git a/src/meta/src/manager/env.rs b/src/meta/src/manager/env.rs index 490ad93189bc2..26601f6af0373 100644 --- a/src/meta/src/manager/env.rs +++ b/src/meta/src/manager/env.rs @@ -213,6 +213,12 @@ pub struct MetaOpts { /// The maximum memory usage in bytes for the tracing collector embedded /// in the meta node. pub cached_traces_memory_limit_bytes: usize, + + /// l0 picker whether to select trivial move task + pub enable_trivial_move: bool, + + /// l0 multi level picker whether to check the overlap accuracy between sub levels + pub enable_check_task_level_overlap: bool, } impl MetaOpts { @@ -264,6 +270,8 @@ impl MetaOpts { advertise_addr: "".to_string(), cached_traces_num: 1, cached_traces_memory_limit_bytes: usize::MAX, + enable_trivial_move: true, + enable_check_task_level_overlap: true, } } } From 4f4cb6134f49e95a2cdd02336a494a11aeaa7f0b Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Fri, 26 Jan 2024 20:42:33 +0800 Subject: [PATCH 3/8] Simplify check logic and enrich check output --- src/meta/src/hummock/compaction/mod.rs | 19 ++--- .../hummock/compaction/overlap_strategy.rs | 10 +-- .../picker/min_overlap_compaction_picker.rs | 70 +++++++++++-------- src/meta/src/hummock/manager/compaction.rs | 4 +- src/meta/src/hummock/manager/mod.rs | 7 +- src/meta/src/hummock/metrics_utils.rs | 1 + 6 files changed, 57 insertions(+), 54 deletions(-) diff --git a/src/meta/src/hummock/compaction/mod.rs b/src/meta/src/hummock/compaction/mod.rs index edf631e74c740..b6bf39064fa8b 100644 --- a/src/meta/src/hummock/compaction/mod.rs +++ b/src/meta/src/hummock/compaction/mod.rs @@ -39,6 +39,7 @@ use crate::hummock::compaction::picker::CompactionInput; use crate::hummock::level_handler::LevelHandler; use crate::hummock::model::CompactionGroup; +#[derive(Clone)] pub struct CompactStatus { pub compaction_group_id: CompactionGroupId, pub level_handlers: Vec, @@ -60,15 +61,6 @@ impl PartialEq for CompactStatus { } } -impl Clone for CompactStatus { - fn clone(&self) -> Self { - Self { - compaction_group_id: self.compaction_group_id, - level_handlers: self.level_handlers.clone(), - } - } -} - pub struct CompactionTask { pub input: CompactionInput, pub base_level: usize, @@ -227,6 +219,15 @@ pub struct CompactionDeveloperConfig { pub enable_check_task_level_overlap: bool, } +impl CompactionDeveloperConfig { + pub fn new_from_meta_opts(opts: &MetaOpts) { + Self { + enable_trivial_move: opts.enable_trivial_move, + enable_check_task_level_overlap: opts.enable_check_task_level_overlap, + } + } +} + impl Default for CompactionDeveloperConfig { fn default() -> Self { Self { diff --git a/src/meta/src/hummock/compaction/overlap_strategy.rs b/src/meta/src/hummock/compaction/overlap_strategy.rs index 5cea7c4bab409..aa94bedae5730 100644 --- a/src/meta/src/hummock/compaction/overlap_strategy.rs +++ b/src/meta/src/hummock/compaction/overlap_strategy.rs @@ -20,13 +20,11 @@ use risingwave_hummock_sdk::key_range::KeyRangeCommon; use risingwave_hummock_sdk::KeyComparator; use risingwave_pb::hummock::{KeyRange, SstableInfo}; -pub trait OverlapInfo { +pub trait OverlapInfo: Debug { fn check_overlap(&self, a: &SstableInfo) -> bool; fn check_multiple_overlap(&self, others: &[SstableInfo]) -> Range; fn check_multiple_include(&self, others: &[SstableInfo]) -> Range; fn update(&mut self, table: &SstableInfo); - - fn debug_info(&self) -> String; } pub trait OverlapStrategy: Send + Sync { @@ -69,7 +67,7 @@ pub trait OverlapStrategy: Send + Sync { fn create_overlap_info(&self) -> Box; } -#[derive(Default)] +#[derive(Default, Debug)] pub struct RangeOverlapInfo { target_range: Option, } @@ -146,10 +144,6 @@ impl OverlapInfo for RangeOverlapInfo { } self.target_range = Some(other.clone()); } - - fn debug_info(&self) -> String { - format!("range {:?}", self.target_range) - } } #[derive(Default)] diff --git a/src/meta/src/hummock/compaction/picker/min_overlap_compaction_picker.rs b/src/meta/src/hummock/compaction/picker/min_overlap_compaction_picker.rs index c6e50825449b4..9676f82332f7a 100644 --- a/src/meta/src/hummock/compaction/picker/min_overlap_compaction_picker.rs +++ b/src/meta/src/hummock/compaction/picker/min_overlap_compaction_picker.rs @@ -15,6 +15,7 @@ use std::collections::BTreeSet; use std::sync::Arc; +use risingwave_hummock_sdk::append_sstable_info_to_string; use risingwave_hummock_sdk::compaction_group::hummock_version_ext::HummockLevelsExt; use risingwave_hummock_sdk::prost_key_range::KeyRangeExt; use risingwave_pb::hummock::hummock_version::Levels; @@ -230,7 +231,6 @@ impl NonOverlapSubLevelPicker { select_sst_id_set.insert(sst.sst_id); } - let mut last_target_index = 0; for (target_index, target_level) in levels.iter().enumerate().skip(1) { if target_level.level_type() != LevelType::Nonoverlapping { break; @@ -343,8 +343,6 @@ impl NonOverlapSubLevelPicker { for (reverse_index, files) in extra_overlap_levels { ret.sstable_infos[reverse_index].extend(files); } - - last_target_index = std::cmp::max(last_target_index, target_index); } // sort sst per level due to reverse expand @@ -356,38 +354,48 @@ impl NonOverlapSubLevelPicker { }); }); - if self.enable_check_task_level_overlap && !ret.sstable_infos.is_empty() { - let mut overlap_info = self.overlap_strategy.create_overlap_info(); - let mut skip_count = 0; - // find the first not empty level - for (level_idx, ssts) in ret.sstable_infos.iter().rev().enumerate() { - if !ssts.is_empty() { - skip_count = level_idx + 1; - for sst in ssts { - overlap_info.update(sst); + if self.enable_check_task_level_overlap { + use crate::hummock::compaction::overlap_strategy::OverlapInfo; + let mut overlap_info: Option> = None; + for (level_idx, ssts) in ret.sstable_infos.iter().enumerate().rev() { + if let Some(overlap_info) = overlap_info.as_mut() { + // skip the check if `overlap_info` is not initialized (i.e. the first non-empty level is not met) + let level = levels.get(level_idx).unwrap(); + let overlap_sst_range = overlap_info.check_multiple_overlap(&level.table_infos); + let expected_sst_ids = level.table_infos[overlap_sst_range.clone()] + .iter() + .map(|s| s.object_id) + .collect_vec(); + let actual_sst_ids = ssts.iter().map(|s| s.object_id).collect_vec(); + if actual_sst_ids != expected_sst_ids { + let mut actual_sst_infos = String::new(); + ssts.iter() + .for_each(|s| append_sstable_info_to_string(&mut actual_sst_infos, s)); + let mut expected_sst_infos = String::new(); + level.table_infos[overlap_sst_range.clone()] + .iter() + .for_each(|s| { + append_sstable_info_to_string(&mut expected_sst_infos, s) + }); + let mut ret_sst_infos = String::new(); + ret.sstable_infos.iter().enumerate().for_each(|idx, ssts| { + writeln!( + ret_sst_infos, + "sub level {}", + levels.get(idx).unwrap().sub_level_id + ); + ssts.iter() + .for_each(|s| append_sstable_info_to_string(&mut ret_sst_infos, s)); + }); + panic!("Compact task overlap check fails. Actual: {:?} Expected: {:?} Ret {:?}", actual_sst_infos, expected_sst_infos, ret_sst_infos); } - - break; - } - } - - for (level_idx, ssts) in ret.sstable_infos.iter().enumerate().rev().skip(skip_count) { - let level = levels.get(level_idx).unwrap(); - let overlap_sst_range = overlap_info.check_multiple_overlap(&level.table_infos); - - for sst in &level.table_infos[overlap_sst_range.clone()] { - assert!( - ssts.contains(sst), - "level_idx {} overlap_sst_range {:?} level_sst_count {} select_sst_count {}", - level_idx, - overlap_sst_range, - level.table_infos.len(), - ssts.len(), - ); + } else if !ssts.is_empty() { + // init the `overlap_info` when meeting the first non-empty level. + overlap_info = Some(self.overlap_strategy.create_overlap_info()); } for sst in ssts { - overlap_info.update(sst); + overlap_info.as_mut().unwrap().update(sst); } } } diff --git a/src/meta/src/hummock/manager/compaction.rs b/src/meta/src/hummock/manager/compaction.rs index e0322a3a6a351..b414f8afa85e0 100644 --- a/src/meta/src/hummock/manager/compaction.rs +++ b/src/meta/src/hummock/manager/compaction.rs @@ -80,7 +80,7 @@ impl HummockManager { &self, compaction_group_id: CompactionGroupId, ) -> Vec { - let (status, levels, config) = { + let (status, levels, group) = { let compaction = read_lock!(self, compaction).await; let versioning = read_lock!(self, versioning).await; let config_manager = self.compaction_group_manager.read().await; @@ -96,7 +96,7 @@ impl HummockManager { } }; let dynamic_level_core = DynamicLevelSelectorCore::new( - config.compaction_config, + group.compaction_config, Arc::new(CompactionDeveloperConfig::default()), ); let ctx = dynamic_level_core.get_priority_levels(&levels, &status.level_handlers); diff --git a/src/meta/src/hummock/manager/mod.rs b/src/meta/src/hummock/manager/mod.rs index 7ace244b862ff..bab6ef381d95e 100644 --- a/src/meta/src/hummock/manager/mod.rs +++ b/src/meta/src/hummock/manager/mod.rs @@ -955,10 +955,9 @@ impl HummockManager { &mut stats, selector, table_id_to_option.clone(), - Arc::new(CompactionDeveloperConfig { - enable_trivial_move: self.env.opts.enable_trivial_move, - enable_check_task_level_overlap: self.env.opts.enable_check_task_level_overlap, - }), + Arc::new(CompactionDeveloperConfig::new_from_meta_opts( + &self.env.opts, + )), ); stats.report_to_metrics(compaction_group_id, self.metrics.as_ref()); let compact_task = match compact_task { diff --git a/src/meta/src/hummock/metrics_utils.rs b/src/meta/src/hummock/metrics_utils.rs index f352ae02d5d3a..cd0fd437c379c 100644 --- a/src/meta/src/hummock/metrics_utils.rs +++ b/src/meta/src/hummock/metrics_utils.rs @@ -435,6 +435,7 @@ pub fn trigger_lsm_stat( ) { let group_label = compaction_group_id.to_string(); // compact_pending_bytes + // we don't actually generate a compaction task here so developer config can be ignored. let dynamic_level_core = DynamicLevelSelectorCore::new( compaction_config.clone(), Arc::new(CompactionDeveloperConfig::default()), From c09fe8cdc9d99cc6b78fbf97efe2ab7af4f2ec0c Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Fri, 26 Jan 2024 20:55:26 +0800 Subject: [PATCH 4/8] fix check --- src/meta/src/hummock/compaction/mod.rs | 2 +- src/meta/src/hummock/compaction/overlap_strategy.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/meta/src/hummock/compaction/mod.rs b/src/meta/src/hummock/compaction/mod.rs index b6bf39064fa8b..7b791117e5831 100644 --- a/src/meta/src/hummock/compaction/mod.rs +++ b/src/meta/src/hummock/compaction/mod.rs @@ -21,7 +21,6 @@ use risingwave_pb::hummock::compact_task::{self, TaskType}; mod picker; pub mod selector; - use std::collections::{HashMap, HashSet}; use std::fmt::{Debug, Formatter}; use std::sync::Arc; @@ -38,6 +37,7 @@ use crate::hummock::compaction::overlap_strategy::{OverlapStrategy, RangeOverlap use crate::hummock::compaction::picker::CompactionInput; use crate::hummock::level_handler::LevelHandler; use crate::hummock::model::CompactionGroup; +use crate::MetaOpts; #[derive(Clone)] pub struct CompactStatus { diff --git a/src/meta/src/hummock/compaction/overlap_strategy.rs b/src/meta/src/hummock/compaction/overlap_strategy.rs index aa94bedae5730..001b480b24255 100644 --- a/src/meta/src/hummock/compaction/overlap_strategy.rs +++ b/src/meta/src/hummock/compaction/overlap_strategy.rs @@ -13,6 +13,7 @@ // limitations under the License. use std::cmp; +use std::fmt::Debug; use std::ops::Range; use itertools::Itertools; From dce3bf943e811ec2a2ddbc90869ea61684ddd16e Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Fri, 26 Jan 2024 21:11:52 +0800 Subject: [PATCH 5/8] fix check --- src/meta/src/hummock/compaction/mod.rs | 2 +- .../picker/min_overlap_compaction_picker.rs | 26 ++++++++++++------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/meta/src/hummock/compaction/mod.rs b/src/meta/src/hummock/compaction/mod.rs index 7b791117e5831..197a565090a37 100644 --- a/src/meta/src/hummock/compaction/mod.rs +++ b/src/meta/src/hummock/compaction/mod.rs @@ -220,7 +220,7 @@ pub struct CompactionDeveloperConfig { } impl CompactionDeveloperConfig { - pub fn new_from_meta_opts(opts: &MetaOpts) { + pub fn new_from_meta_opts(opts: &MetaOpts) -> Self { Self { enable_trivial_move: opts.enable_trivial_move, enable_check_task_level_overlap: opts.enable_check_task_level_overlap, diff --git a/src/meta/src/hummock/compaction/picker/min_overlap_compaction_picker.rs b/src/meta/src/hummock/compaction/picker/min_overlap_compaction_picker.rs index 9676f82332f7a..d41566b7e87e8 100644 --- a/src/meta/src/hummock/compaction/picker/min_overlap_compaction_picker.rs +++ b/src/meta/src/hummock/compaction/picker/min_overlap_compaction_picker.rs @@ -355,6 +355,10 @@ impl NonOverlapSubLevelPicker { }); if self.enable_check_task_level_overlap { + use std::fmt::Write; + + use itertools::Itertools; + use crate::hummock::compaction::overlap_strategy::OverlapInfo; let mut overlap_info: Option> = None; for (level_idx, ssts) in ret.sstable_infos.iter().enumerate().rev() { @@ -378,15 +382,19 @@ impl NonOverlapSubLevelPicker { append_sstable_info_to_string(&mut expected_sst_infos, s) }); let mut ret_sst_infos = String::new(); - ret.sstable_infos.iter().enumerate().for_each(|idx, ssts| { - writeln!( - ret_sst_infos, - "sub level {}", - levels.get(idx).unwrap().sub_level_id - ); - ssts.iter() - .for_each(|s| append_sstable_info_to_string(&mut ret_sst_infos, s)); - }); + ret.sstable_infos + .iter() + .enumerate() + .for_each(|(idx, ssts)| { + writeln!( + ret_sst_infos, + "sub level {}", + levels.get(idx).unwrap().sub_level_id + ); + ssts.iter().for_each(|s| { + append_sstable_info_to_string(&mut ret_sst_infos, s) + }); + }); panic!("Compact task overlap check fails. Actual: {:?} Expected: {:?} Ret {:?}", actual_sst_infos, expected_sst_infos, ret_sst_infos); } } else if !ssts.is_empty() { From 8f4685edd1d19359c58ce65941b46d19277076b7 Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Fri, 26 Jan 2024 21:45:50 +0800 Subject: [PATCH 6/8] fix --- .../hummock/compaction/picker/min_overlap_compaction_picker.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/meta/src/hummock/compaction/picker/min_overlap_compaction_picker.rs b/src/meta/src/hummock/compaction/picker/min_overlap_compaction_picker.rs index d41566b7e87e8..055af06f31923 100644 --- a/src/meta/src/hummock/compaction/picker/min_overlap_compaction_picker.rs +++ b/src/meta/src/hummock/compaction/picker/min_overlap_compaction_picker.rs @@ -390,7 +390,8 @@ impl NonOverlapSubLevelPicker { ret_sst_infos, "sub level {}", levels.get(idx).unwrap().sub_level_id - ); + ) + .unwrap(); ssts.iter().for_each(|s| { append_sstable_info_to_string(&mut ret_sst_infos, s) }); From 36658584682af6b19312890b92ffa7bec5d7796d Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Mon, 29 Jan 2024 11:22:48 +0800 Subject: [PATCH 7/8] fix trivial move picker and sst overlap check --- .../picker/base_level_compaction_picker.rs | 12 +-- .../picker/intra_compaction_picker.rs | 11 ++- .../picker/min_overlap_compaction_picker.rs | 74 ++++++++++++------- .../picker/trivial_move_compaction_picker.rs | 7 -- 4 files changed, 57 insertions(+), 47 deletions(-) diff --git a/src/meta/src/hummock/compaction/picker/base_level_compaction_picker.rs b/src/meta/src/hummock/compaction/picker/base_level_compaction_picker.rs index a298f57581341..849cacd694113 100644 --- a/src/meta/src/hummock/compaction/picker/base_level_compaction_picker.rs +++ b/src/meta/src/hummock/compaction/picker/base_level_compaction_picker.rs @@ -122,13 +122,13 @@ impl LevelCompactionPicker { level_handlers: &[LevelHandler], stats: &mut LocalPickerStatistic, ) -> Option { + if !self.developer_config.enable_trivial_move { + return None; + } + let overlap_strategy = create_overlap_strategy(self.config.compaction_mode()); - let trivial_move_picker = TrivialMovePicker::new( - 0, - self.target_level, - overlap_strategy.clone(), - self.developer_config.enable_trivial_move, - ); + let trivial_move_picker = + TrivialMovePicker::new(0, self.target_level, overlap_strategy.clone()); trivial_move_picker.pick_trivial_move_task( &l0.sub_levels[0].table_infos, diff --git a/src/meta/src/hummock/compaction/picker/intra_compaction_picker.rs b/src/meta/src/hummock/compaction/picker/intra_compaction_picker.rs index cb9bd32610fbe..c61b51ff15671 100644 --- a/src/meta/src/hummock/compaction/picker/intra_compaction_picker.rs +++ b/src/meta/src/hummock/compaction/picker/intra_compaction_picker.rs @@ -295,6 +295,10 @@ impl IntraCompactionPicker { level_handlers: &[LevelHandler], stats: &mut LocalPickerStatistic, ) -> Option { + if !self.developer_config.enable_trivial_move { + return None; + } + let overlap_strategy = create_overlap_strategy(self.config.compaction_mode()); for (idx, level) in l0.sub_levels.iter().enumerate() { @@ -312,12 +316,7 @@ impl IntraCompactionPicker { continue; } - let trivial_move_picker = TrivialMovePicker::new( - 0, - 0, - overlap_strategy.clone(), - self.developer_config.enable_trivial_move, - ); + let trivial_move_picker = TrivialMovePicker::new(0, 0, overlap_strategy.clone()); let select_sst = trivial_move_picker.pick_trivial_move_sst( &l0.sub_levels[idx + 1].table_infos, diff --git a/src/meta/src/hummock/compaction/picker/min_overlap_compaction_picker.rs b/src/meta/src/hummock/compaction/picker/min_overlap_compaction_picker.rs index 055af06f31923..23cee45b95ca6 100644 --- a/src/meta/src/hummock/compaction/picker/min_overlap_compaction_picker.rs +++ b/src/meta/src/hummock/compaction/picker/min_overlap_compaction_picker.rs @@ -366,37 +366,55 @@ impl NonOverlapSubLevelPicker { // skip the check if `overlap_info` is not initialized (i.e. the first non-empty level is not met) let level = levels.get(level_idx).unwrap(); let overlap_sst_range = overlap_info.check_multiple_overlap(&level.table_infos); - let expected_sst_ids = level.table_infos[overlap_sst_range.clone()] - .iter() - .map(|s| s.object_id) - .collect_vec(); - let actual_sst_ids = ssts.iter().map(|s| s.object_id).collect_vec(); - if actual_sst_ids != expected_sst_ids { - let mut actual_sst_infos = String::new(); - ssts.iter() - .for_each(|s| append_sstable_info_to_string(&mut actual_sst_infos, s)); - let mut expected_sst_infos = String::new(); - level.table_infos[overlap_sst_range.clone()] + if !overlap_sst_range.is_empty() { + let expected_sst_ids = level.table_infos[overlap_sst_range.clone()] .iter() - .for_each(|s| { - append_sstable_info_to_string(&mut expected_sst_infos, s) - }); - let mut ret_sst_infos = String::new(); - ret.sstable_infos + .map(|s| s.object_id) + .collect_vec(); + let actual_sst_ids = ssts.iter().map(|s| s.object_id).collect_vec(); + // `actual_sst_ids` can be larger than `expected_sst_ids` because we may use a larger key range to select SSTs. + // `expected_sst_ids` must be a sub-range of `actual_sst_ids` to ensure correctness. + let start_idx = actual_sst_ids .iter() - .enumerate() - .for_each(|(idx, ssts)| { - writeln!( - ret_sst_infos, - "sub level {}", - levels.get(idx).unwrap().sub_level_id - ) - .unwrap(); - ssts.iter().for_each(|s| { - append_sstable_info_to_string(&mut ret_sst_infos, s) - }); + .position(|sst_id| sst_id == expected_sst_ids.first().unwrap()); + if start_idx.is_none() + || actual_sst_ids[start_idx.unwrap()..] != expected_sst_ids + { + // Print SstableInfo for `actual_sst_ids` + let mut actual_sst_infos = String::new(); + ssts.iter().for_each(|s| { + append_sstable_info_to_string(&mut actual_sst_infos, s) }); - panic!("Compact task overlap check fails. Actual: {:?} Expected: {:?} Ret {:?}", actual_sst_infos, expected_sst_infos, ret_sst_infos); + + // Print SstableInfo for `expected_sst_ids` + let mut expected_sst_infos = String::new(); + level.table_infos[overlap_sst_range.clone()] + .iter() + .for_each(|s| { + append_sstable_info_to_string(&mut expected_sst_infos, s) + }); + + // Print SstableInfo for selected ssts in all sub-levels + let mut ret_sst_infos = String::new(); + ret.sstable_infos + .iter() + .enumerate() + .for_each(|(idx, ssts)| { + writeln!( + ret_sst_infos, + "sub level {}", + levels.get(idx).unwrap().sub_level_id + ) + .unwrap(); + ssts.iter().for_each(|s| { + append_sstable_info_to_string(&mut ret_sst_infos, s) + }); + }); + panic!( + "Compact task overlap check fails. Actual: {} Expected: {} Ret {}", + actual_sst_infos, expected_sst_infos, ret_sst_infos + ); + } } } else if !ssts.is_empty() { // init the `overlap_info` when meeting the first non-empty level. diff --git a/src/meta/src/hummock/compaction/picker/trivial_move_compaction_picker.rs b/src/meta/src/hummock/compaction/picker/trivial_move_compaction_picker.rs index cd58163246239..5204fc2ce1868 100644 --- a/src/meta/src/hummock/compaction/picker/trivial_move_compaction_picker.rs +++ b/src/meta/src/hummock/compaction/picker/trivial_move_compaction_picker.rs @@ -24,7 +24,6 @@ pub struct TrivialMovePicker { level: usize, target_level: usize, overlap_strategy: Arc, - enable: bool, } impl TrivialMovePicker { @@ -32,13 +31,11 @@ impl TrivialMovePicker { level: usize, target_level: usize, overlap_strategy: Arc, - enable: bool, ) -> Self { Self { level, target_level, overlap_strategy, - enable, } } @@ -49,10 +46,6 @@ impl TrivialMovePicker { level_handlers: &[LevelHandler], stats: &mut LocalPickerStatistic, ) -> Option { - if !self.enable { - return None; - } - let mut skip_by_pending = false; for sst in select_tables { if level_handlers[self.level].is_pending_compact(&sst.sst_id) { From eeba96e9bb4297788e4e43368b2865422b77b0e3 Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Mon, 29 Jan 2024 12:15:03 +0800 Subject: [PATCH 8/8] fix check --- .../compaction/picker/trivial_move_compaction_picker.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/meta/src/hummock/compaction/picker/trivial_move_compaction_picker.rs b/src/meta/src/hummock/compaction/picker/trivial_move_compaction_picker.rs index 5204fc2ce1868..bab0dca0c3244 100644 --- a/src/meta/src/hummock/compaction/picker/trivial_move_compaction_picker.rs +++ b/src/meta/src/hummock/compaction/picker/trivial_move_compaction_picker.rs @@ -75,10 +75,6 @@ impl TrivialMovePicker { level_handlers: &[LevelHandler], stats: &mut LocalPickerStatistic, ) -> Option { - if !self.enable { - return None; - } - if let Some(trivial_move_sst) = self.pick_trivial_move_sst(select_tables, target_tables, level_handlers, stats) {