From 908970b77949e993fda709e8cfa84a51b747daa0 Mon Sep 17 00:00:00 2001 From: Li0k Date: Thu, 23 Jan 2025 20:50:39 +0800 Subject: [PATCH 1/9] feat(compaction): support trivial-move multi ssts --- proto/hummock.proto | 4 + src/common/src/config.rs | 5 + .../src/cmd_impl/hummock/compaction_group.rs | 4 + src/ctl/src/lib.rs | 4 + .../hummock/compaction/compaction_config.rs | 3 + src/meta/src/hummock/compaction/mod.rs | 47 +----- .../hummock/compaction/overlap_strategy.rs | 5 + .../picker/base_level_compaction_picker.rs | 4 + .../picker/intra_compaction_picker.rs | 77 +++++----- .../picker/trivial_move_compaction_picker.rs | 141 ++++++++++++++++-- .../compaction/compaction_group_manager.rs | 3 + .../src/hummock/manager/compaction/mod.rs | 57 +++---- src/meta/src/hummock/metrics_utils.rs | 47 ++++++ src/meta/src/rpc/metrics.rs | 10 ++ src/storage/hummock_sdk/src/compact_task.rs | 48 +++++- 15 files changed, 322 insertions(+), 137 deletions(-) diff --git a/proto/hummock.proto b/proto/hummock.proto index 15f3d61a7cf2b..be6d7435f7530 100644 --- a/proto/hummock.proto +++ b/proto/hummock.proto @@ -661,6 +661,7 @@ message RiseCtlUpdateCompactionConfigRequest { uint32 split_weight_by_vnode = 20; bool disable_auto_group_scheduling = 21; uint64 max_overlapping_level_size = 22; + uint32 sst_allowed_trivial_move_max_count = 23; } } repeated uint64 compaction_group_ids = 1; @@ -863,6 +864,9 @@ message CompactionConfig { // The limitation of the max size of the overlapping-level for the compaction // hummock will reorg the commit-sstables to the multi overlapping-level if the size of the commit-sstables is larger than `max_overlapping_level_size` optional uint64 max_overlapping_level_size = 24; + + // The limitation of the max sst count of the trivial move task + optional uint32 sst_allowed_trivial_move_max_count = 25; } message TableStats { diff --git a/src/common/src/config.rs b/src/common/src/config.rs index dec4a024a6f6b..b26cdbe894698 100644 --- a/src/common/src/config.rs +++ b/src/common/src/config.rs @@ -2215,6 +2215,7 @@ pub mod default { const DEFAULT_MAX_LEVEL: u32 = 6; const DEFAULT_MAX_L0_COMPACT_LEVEL_COUNT: u32 = 42; const DEFAULT_SST_ALLOWED_TRIVIAL_MOVE_MIN_SIZE: u64 = 4 * MB; + const DEFAULT_SST_ALLOWED_TRIVIAL_MOVE_MAX_COUNT: u32 = 64; use crate::catalog::hummock::CompactionFilterFlag; @@ -2297,6 +2298,10 @@ pub mod default { pub fn max_overlapping_level_size() -> u64 { 256 * MB } + + pub fn sst_allowed_trivial_move_max_count() -> u32 { + DEFAULT_SST_ALLOWED_TRIVIAL_MOVE_MAX_COUNT + } } pub mod object_store_config { diff --git a/src/ctl/src/cmd_impl/hummock/compaction_group.rs b/src/ctl/src/cmd_impl/hummock/compaction_group.rs index 1fec82afc1165..b59187e611857 100644 --- a/src/ctl/src/cmd_impl/hummock/compaction_group.rs +++ b/src/ctl/src/cmd_impl/hummock/compaction_group.rs @@ -69,6 +69,7 @@ pub fn build_compaction_config_vec( sst_allowed_trivial_move_min_size: Option, disable_auto_group_scheduling: Option, max_overlapping_level_size: Option, + sst_allowed_trivial_move_max_count: Option, ) -> Vec { let mut configs = vec![]; if let Some(c) = max_bytes_for_level_base { @@ -131,6 +132,9 @@ pub fn build_compaction_config_vec( if let Some(c) = max_overlapping_level_size { configs.push(MutableConfig::MaxOverlappingLevelSize(c)) } + if let Some(c) = sst_allowed_trivial_move_max_count { + configs.push(MutableConfig::SstAllowedTrivialMoveMaxCount(c)) + } configs } diff --git a/src/ctl/src/lib.rs b/src/ctl/src/lib.rs index f7c9d4ceb6ab1..1ab2f0475e06a 100644 --- a/src/ctl/src/lib.rs +++ b/src/ctl/src/lib.rs @@ -196,6 +196,8 @@ enum HummockCommands { disable_auto_group_scheduling: Option, #[clap(long)] max_overlapping_level_size: Option, + #[clap(long)] + sst_allowed_trivial_move_max_count: Option, }, /// Split given compaction group into two. Moves the given tables to the new group. SplitCompactionGroup { @@ -605,6 +607,7 @@ async fn start_impl(opts: CliOpts, context: &CtlContext) -> Result<()> { sst_allowed_trivial_move_min_size, disable_auto_group_scheduling, max_overlapping_level_size, + sst_allowed_trivial_move_max_count, }) => { cmd_impl::hummock::update_compaction_config( context, @@ -638,6 +641,7 @@ async fn start_impl(opts: CliOpts, context: &CtlContext) -> Result<()> { sst_allowed_trivial_move_min_size, disable_auto_group_scheduling, max_overlapping_level_size, + sst_allowed_trivial_move_max_count, ), ) .await? diff --git a/src/meta/src/hummock/compaction/compaction_config.rs b/src/meta/src/hummock/compaction/compaction_config.rs index d50a458273d62..8424fadf530b9 100644 --- a/src/meta/src/hummock/compaction/compaction_config.rs +++ b/src/meta/src/hummock/compaction/compaction_config.rs @@ -72,6 +72,9 @@ impl CompactionConfigBuilder { compaction_config::disable_auto_group_scheduling(), ), max_overlapping_level_size: Some(compaction_config::max_overlapping_level_size()), + sst_allowed_trivial_move_max_count: Some( + compaction_config::sst_allowed_trivial_move_max_count(), + ), }, } } diff --git a/src/meta/src/hummock/compaction/mod.rs b/src/meta/src/hummock/compaction/mod.rs index d5115569eabc9..6ed4cbe0f4f18 100644 --- a/src/meta/src/hummock/compaction/mod.rs +++ b/src/meta/src/hummock/compaction/mod.rs @@ -19,11 +19,11 @@ mod overlap_strategy; use risingwave_common::catalog::{TableId, TableOption}; use risingwave_hummock_sdk::compact_task::CompactTask; use risingwave_hummock_sdk::level::Levels; -use risingwave_pb::hummock::compact_task::{self, TaskType}; +use risingwave_pb::hummock::compact_task::{self}; mod picker; pub mod selector; -use std::collections::{BTreeSet, HashMap, HashSet}; +use std::collections::{BTreeSet, HashMap}; use std::fmt::{Debug, Formatter}; use std::sync::Arc; @@ -32,7 +32,7 @@ use risingwave_hummock_sdk::table_watermark::TableWatermarks; use risingwave_hummock_sdk::version::HummockVersionStateTableInfo; use risingwave_hummock_sdk::{CompactionGroupId, HummockCompactionTaskId}; use risingwave_pb::hummock::compaction_config::CompactionMode; -use risingwave_pb::hummock::{CompactionConfig, LevelType}; +use risingwave_pb::hummock::CompactionConfig; pub use selector::{CompactionSelector, CompactionSelectorContext}; use self::selector::{EmergencySelector, LocalSelectorStatistic}; @@ -145,47 +145,6 @@ impl CompactStatus { None } - pub fn is_trivial_move_task(task: &CompactTask) -> bool { - if task.task_type != TaskType::Dynamic && task.task_type != TaskType::Emergency { - return false; - } - - if task.input_ssts.len() != 2 || task.input_ssts[0].level_type != LevelType::Nonoverlapping - { - return false; - } - - // it may be a manual compaction task - if task.input_ssts[0].level_idx == task.input_ssts[1].level_idx - && task.input_ssts[0].level_idx > 0 - { - return false; - } - - if task.input_ssts[1].level_idx == task.target_level - && task.input_ssts[1].table_infos.is_empty() - { - return true; - } - - false - } - - pub fn is_trivial_reclaim(task: &CompactTask) -> bool { - // Currently all VnodeWatermark tasks are trivial reclaim. - if task.task_type == TaskType::VnodeWatermark { - return true; - } - let exist_table_ids = HashSet::::from_iter(task.existing_table_ids.clone()); - task.input_ssts.iter().all(|level| { - level.table_infos.iter().all(|sst| { - sst.table_ids - .iter() - .all(|table_id| !exist_table_ids.contains(table_id)) - }) - }) - } - pub fn report_compact_task(&mut self, compact_task: &CompactTask) { for level in &compact_task.input_ssts { self.level_handlers[level.level_idx as usize].remove_task(compact_task.task_id); diff --git a/src/meta/src/hummock/compaction/overlap_strategy.rs b/src/meta/src/hummock/compaction/overlap_strategy.rs index d990dfa9ce8e6..60d7e5ed210b6 100644 --- a/src/meta/src/hummock/compaction/overlap_strategy.rs +++ b/src/meta/src/hummock/compaction/overlap_strategy.rs @@ -26,6 +26,7 @@ pub trait OverlapInfo: Debug { fn check_multiple_overlap(&self, others: &[SstableInfo]) -> Range; fn check_multiple_include(&self, others: &[SstableInfo]) -> Range; fn update(&mut self, table: &SstableInfo); + fn clear(&mut self); } pub trait OverlapStrategy: Send + Sync { @@ -134,6 +135,10 @@ impl OverlapInfo for RangeOverlapInfo { } self.target_range = Some(other.clone()); } + + fn clear(&mut self) { + self.target_range = None; + } } #[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 cb966bdc81fb6..e24e86d80c508 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 @@ -142,6 +142,10 @@ impl LevelCompactionPicker { } else { 0 }, + self.config + .sst_allowed_trivial_move_max_count + .unwrap_or(compaction_config::sst_allowed_trivial_move_max_count()) + as usize, ); trivial_move_picker.pick_trivial_move_task( 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 4f71f9b2780b1..82bf4be2899d2 100644 --- a/src/meta/src/hummock/compaction/picker/intra_compaction_picker.rs +++ b/src/meta/src/hummock/compaction/picker/intra_compaction_picker.rs @@ -267,51 +267,52 @@ impl IntraCompactionPicker { continue; } - let trivial_move_picker = TrivialMovePicker::new(0, 0, overlap_strategy.clone(), 0); + let trivial_move_picker = TrivialMovePicker::new( + 0, + 0, + overlap_strategy.clone(), + 0, + self.config + .sst_allowed_trivial_move_max_count + .unwrap_or(compaction_config::sst_allowed_trivial_move_max_count()) + as usize, + ); - let select_sst = trivial_move_picker.pick_trivial_move_sst( + if let Some(select_ssts) = trivial_move_picker.pick_multi_trivial_move_ssts( &l0.sub_levels[idx + 1].table_infos, &level.table_infos, level_handlers, stats, - ); + ) { + let mut overlap = overlap_strategy.create_overlap_info(); + select_ssts.iter().for_each(|ssts| overlap.update(ssts)); - // only pick tables for trivial move - if select_sst.is_none() { - continue; - } - - let select_sst = select_sst.unwrap(); - - // support trivial move cross multi sub_levels - let mut overlap = overlap_strategy.create_overlap_info(); - overlap.update(&select_sst); + assert!(overlap + .check_multiple_overlap(&l0.sub_levels[idx].table_infos) + .is_empty()); - assert!(overlap - .check_multiple_overlap(&l0.sub_levels[idx].table_infos) - .is_empty()); - - let select_input_size = select_sst.sst_size; - let input_levels = vec![ - InputLevel { - level_idx: 0, - level_type: LevelType::Nonoverlapping, - table_infos: vec![select_sst], - }, - InputLevel { - level_idx: 0, - level_type: LevelType::Nonoverlapping, - table_infos: vec![], - }, - ]; - return Some(CompactionInput { - input_levels, - target_level: 0, - target_sub_level_id: level.sub_level_id, - select_input_size, - total_file_count: 1, - ..Default::default() - }); + let select_input_size = select_ssts.iter().map(|sst| sst.sst_size).sum(); + let input_levels = vec![ + InputLevel { + level_idx: 0, + level_type: LevelType::Nonoverlapping, + table_infos: select_ssts, + }, + InputLevel { + level_idx: 0, + level_type: LevelType::Nonoverlapping, + table_infos: vec![], + }, + ]; + return Some(CompactionInput { + input_levels, + target_level: 0, + target_sub_level_id: level.sub_level_id, + select_input_size, + total_file_count: 1, + ..Default::default() + }); + } } None } 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 682628969a65a..08d708a1c8c12 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 @@ -29,6 +29,8 @@ pub struct TrivialMovePicker { // The minimum size of sst that can be selected as trivial move. sst_allowed_trivial_move_min_size: u64, + + sst_allowed_trivial_move_max_count: usize, } impl TrivialMovePicker { @@ -37,25 +39,39 @@ impl TrivialMovePicker { target_level: usize, overlap_strategy: Arc, sst_allowed_trivial_move_min_size: u64, + sst_allowed_trivial_move_max_count: usize, ) -> Self { Self { level, target_level, overlap_strategy, sst_allowed_trivial_move_min_size, + sst_allowed_trivial_move_max_count, } } - pub fn pick_trivial_move_sst( + pub fn pick_multi_trivial_move_ssts( &self, select_tables: &[SstableInfo], target_tables: &[SstableInfo], level_handlers: &[LevelHandler], stats: &mut LocalPickerStatistic, - ) -> Option { + ) -> Option> { let mut skip_by_pending = false; + + // means we have already found some sst but not match `sst_allowed_trivial_move_max_count`. + let mut skip_by_size = false; + let mut result = vec![]; + + let mut overlap_info = self.overlap_strategy.create_overlap_info(); for sst in select_tables { - if sst.sst_size < self.sst_allowed_trivial_move_min_size { + if result.len() >= self.sst_allowed_trivial_move_max_count { + break; + } + + // find the first sst that can be trivial moved. + if sst.sst_size < self.sst_allowed_trivial_move_min_size && result.is_empty() { + skip_by_size = true; continue; } @@ -63,19 +79,34 @@ impl TrivialMovePicker { skip_by_pending = true; continue; } - let mut overlap_info = self.overlap_strategy.create_overlap_info(); overlap_info.update(sst); let overlap_files_range = overlap_info.check_multiple_overlap(target_tables); if overlap_files_range.is_empty() { - return Some(sst.clone()); + result.push(sst.clone()); + } else { + // stop probing if we have already found some sst to move. + if !result.is_empty() { + break; + } + + // reset overlap_info + overlap_info.clear(); } } + if !result.is_empty() { + return Some(result); + } + if skip_by_pending { stats.skip_by_pending_files += 1; } + if skip_by_size { + stats.skip_by_count_limit += 1; + } + None } @@ -86,17 +117,17 @@ impl TrivialMovePicker { level_handlers: &[LevelHandler], stats: &mut LocalPickerStatistic, ) -> Option { - if let Some(trivial_move_sst) = - self.pick_trivial_move_sst(select_tables, target_tables, level_handlers, stats) + if let Some(trivial_move_ssts) = + self.pick_multi_trivial_move_ssts(select_tables, target_tables, level_handlers, stats) { return Some(CompactionInput { - select_input_size: trivial_move_sst.sst_size, + select_input_size: trivial_move_ssts.iter().map(|s| s.sst_size).sum(), total_file_count: 1, input_levels: vec![ InputLevel { level_idx: self.level as u32, level_type: LevelType::Nonoverlapping, - table_infos: vec![trivial_move_sst], + table_infos: trivial_move_ssts, }, InputLevel { level_idx: self.target_level as u32, @@ -143,8 +174,8 @@ pub mod tests { let overlap_strategy = create_overlap_strategy(config.compaction_mode()); { let trivial_move_picker = - super::TrivialMovePicker::new(0, 1, overlap_strategy.clone(), 200); - let trivial_move_task = trivial_move_picker.pick_trivial_move_task( + super::TrivialMovePicker::new(0, 1, overlap_strategy.clone(), 200, 1); + let trivial_move_task = trivial_move_picker.pick_multi_trivial_move_ssts( &[sst.clone()], &[], &levels_handler, @@ -156,9 +187,9 @@ pub mod tests { { let trivial_move_picker = - super::TrivialMovePicker::new(0, 1, overlap_strategy.clone(), 50); + super::TrivialMovePicker::new(0, 1, overlap_strategy.clone(), 50, 1); - let trivial_move_task = trivial_move_picker.pick_trivial_move_task( + let trivial_move_task = trivial_move_picker.pick_multi_trivial_move_ssts( &[sst.clone()], &[], &levels_handler, @@ -168,4 +199,88 @@ pub mod tests { assert!(trivial_move_task.is_some()); } } + + #[test] + fn test_pick_multi_trivial_move_sst() { + let sst1: SstableInfo = SstableInfoInner { + sst_id: 1, + file_size: 100, + sst_size: 100, + ..Default::default() + } + .into(); + let sst2: SstableInfo = SstableInfoInner { + sst_id: 2, + file_size: 100, + sst_size: 100, + ..Default::default() + } + .into(); + let sst3: SstableInfo = SstableInfoInner { + sst_id: 3, + file_size: 100, + sst_size: 100, + ..Default::default() + } + .into(); + let sst4: SstableInfo = SstableInfoInner { + sst_id: 4, + file_size: 100, + sst_size: 100, + ..Default::default() + } + .into(); + + let config = Arc::new( + CompactionConfigBuilder::new() + .level0_tier_compact_file_number(2) + .level0_sub_level_compact_level_count(1) + .sst_allowed_trivial_move_min_size(Some(50)) + .build(), + ); + let levels_handler = vec![LevelHandler::new(0), LevelHandler::new(1)]; + let overlap_strategy = create_overlap_strategy(config.compaction_mode()); + { + let trivial_move_picker = + super::TrivialMovePicker::new(0, 1, overlap_strategy.clone(), 200, 10); + let trivial_move_task = trivial_move_picker.pick_multi_trivial_move_ssts( + &[sst1.clone(), sst2.clone(), sst3.clone(), sst4.clone()], + &[], + &levels_handler, + &mut Default::default(), + ); + + assert!(trivial_move_task.is_none()); + } + + { + let trivial_move_picker = + super::TrivialMovePicker::new(0, 1, overlap_strategy.clone(), 50, 10); + + let trivial_move_task = trivial_move_picker.pick_multi_trivial_move_ssts( + &[sst1.clone(), sst2.clone(), sst3.clone(), sst4.clone()], + &[], + &levels_handler, + &mut Default::default(), + ); + + assert!(trivial_move_task.is_some()); + assert_eq!(trivial_move_task.unwrap().len(), 4); + } + + { + let trivial_move_picker = + super::TrivialMovePicker::new(0, 1, overlap_strategy.clone(), 50, 2); + + let trivial_move_task = trivial_move_picker.pick_multi_trivial_move_ssts( + &[sst1.clone(), sst2.clone(), sst3.clone(), sst4.clone()], + &[], + &levels_handler, + &mut Default::default(), + ); + + assert!(trivial_move_task.is_some()); + assert_eq!(trivial_move_task.unwrap().len(), 2); + } + } } diff --git a/src/meta/src/hummock/manager/compaction/compaction_group_manager.rs b/src/meta/src/hummock/manager/compaction/compaction_group_manager.rs index 36d669190cf55..868cc96c49201 100644 --- a/src/meta/src/hummock/manager/compaction/compaction_group_manager.rs +++ b/src/meta/src/hummock/manager/compaction/compaction_group_manager.rs @@ -605,6 +605,9 @@ fn update_compaction_config(target: &mut CompactionConfig, items: &[MutableConfi MutableConfig::MaxOverlappingLevelSize(c) => { target.max_overlapping_level_size = Some(*c); } + MutableConfig::SstAllowedTrivialMoveMaxCount(c) => { + target.sst_allowed_trivial_move_max_count = Some(*c); + } } } } diff --git a/src/meta/src/hummock/manager/compaction/mod.rs b/src/meta/src/hummock/manager/compaction/mod.rs index 6c50a3d8d4d5a..84d8a4fe3e64b 100644 --- a/src/meta/src/hummock/manager/compaction/mod.rs +++ b/src/meta/src/hummock/manager/compaction/mod.rs @@ -87,7 +87,8 @@ use crate::hummock::manager::transaction::{ }; use crate::hummock::manager::versioning::Versioning; use crate::hummock::metrics_utils::{ - build_compact_task_level_type_metrics_label, trigger_local_table_stat, trigger_sst_stat, + build_compact_task_level_type_metrics_label, trigger_compact_tasks_stat, + trigger_local_table_stat, }; use crate::hummock::model::CompactionGroup; use crate::hummock::sequence::next_compaction_task_id; @@ -147,7 +148,7 @@ fn init_selectors() -> HashMap { fn apply_compact_task(&mut self, compact_task: &CompactTask) { let mut version_delta = self.new_delta(); - let trivial_move = CompactStatus::is_trivial_move_task(compact_task); + let trivial_move = compact_task.is_trivial_move_task(); version_delta.trivial_move = trivial_move; let group_deltas = &mut version_delta @@ -797,8 +798,8 @@ impl HummockManager { ..Default::default() }; - let is_trivial_reclaim = CompactStatus::is_trivial_reclaim(&compact_task); - let is_trivial_move = CompactStatus::is_trivial_move_task(&compact_task); + let is_trivial_reclaim = compact_task.is_trivial_reclaim(); + let is_trivial_move = compact_task.is_trivial_move_task(); if is_trivial_reclaim || (is_trivial_move && can_trivial_move) { let log_label = if is_trivial_reclaim { "TrivialReclaim" @@ -1044,10 +1045,7 @@ impl HummockManager { .await?; tasks.retain(|task| { if task.task_status == TaskStatus::Success { - debug_assert!( - CompactStatus::is_trivial_reclaim(task) - || CompactStatus::is_trivial_move_task(task) - ); + debug_assert!(task.is_trivial_reclaim() || task.is_trivial_move_task()); false } else { true @@ -1072,10 +1070,7 @@ impl HummockManager { if task.task_status != TaskStatus::Success { return Ok(Some(task)); } - debug_assert!( - CompactStatus::is_trivial_reclaim(&task) - || CompactStatus::is_trivial_move_task(&task) - ); + debug_assert!(task.is_trivial_reclaim() || task.is_trivial_move_task()); } Ok(None) } @@ -1281,40 +1276,17 @@ impl HummockManager { compact_task_assignment )?; } - let mut success_groups = vec![]; - for compact_task in tasks { - let task_status = compact_task.task_status; - let task_status_label = task_status.as_str_name(); - let task_type_label = compact_task.task_type.as_str_name(); + let mut success_groups = vec![]; + for compact_task in &tasks { self.compactor_manager .remove_task_heartbeat(compact_task.task_id); - - self.metrics - .compact_frequency - .with_label_values(&[ - "normal", - &compact_task.compaction_group_id.to_string(), - task_type_label, - task_status_label, - ]) - .inc(); - tracing::trace!( "Reported compaction task. {}. cost time: {:?}", - compact_task_to_string(&compact_task), + compact_task_to_string(compact_task), start_time.elapsed(), ); - trigger_sst_stat( - &self.metrics, - compaction - .compaction_statuses - .get(&compact_task.compaction_group_id), - &versioning_guard.current_version, - compact_task.compaction_group_id, - ); - if !deterministic_mode && (matches!(compact_task.task_type, compact_task::TaskType::Dynamic) || matches!(compact_task.task_type, compact_task::TaskType::Emergency)) @@ -1326,10 +1298,17 @@ impl HummockManager { ); } - if task_status == TaskStatus::Success { + if compact_task.task_status == TaskStatus::Success { success_groups.push(compact_task.compaction_group_id); } } + + trigger_compact_tasks_stat( + &self.metrics, + &tasks, + &compaction.compaction_statuses, + &versioning_guard.current_version, + ); drop(versioning_guard); if !success_groups.is_empty() { self.try_update_write_limits(&success_groups).await; diff --git a/src/meta/src/hummock/metrics_utils.rs b/src/meta/src/hummock/metrics_utils.rs index c4355b0d5a42c..d032260115528 100644 --- a/src/meta/src/hummock/metrics_utils.rs +++ b/src/meta/src/hummock/metrics_utils.rs @@ -20,6 +20,7 @@ use std::time::{SystemTime, UNIX_EPOCH}; use itertools::{enumerate, Itertools}; use prometheus::core::{AtomicU64, GenericCounter}; use prometheus::IntGauge; +use risingwave_hummock_sdk::compact_task::CompactTask; use risingwave_hummock_sdk::compaction_group::hummock_version_ext::object_size_map; use risingwave_hummock_sdk::level::Levels; use risingwave_hummock_sdk::table_stats::PbTableStatsMap; @@ -667,3 +668,49 @@ pub fn remove_compact_task_metrics( } } } + +pub fn trigger_compact_tasks_stat( + metrics: &MetaMetrics, + compact_tasks: &[CompactTask], + compact_status: &BTreeMap, + current_version: &HummockVersion, +) { + let mut task_status_label_map = HashMap::new(); + let mut task_type_label_map = HashMap::new(); + let mut group_label_map = HashMap::new(); + + for task in compact_tasks { + let task_status_label = task_status_label_map + .entry(task.task_status) + .or_insert_with(|| task.task_status.as_str_name().to_owned()); + + let task_type_label = task_type_label_map + .entry(task.task_type) + .or_insert_with(|| task.task_type.as_str_name().to_owned()); + + let group_label = group_label_map + .entry(task.compaction_group_id) + .or_insert_with(|| task.compaction_group_id.to_string()); + + metrics + .compact_frequency + .with_label_values(&["normal", group_label, task_type_label, task_status_label]) + .inc(); + + if task.is_trivial_move_task() { + metrics + .compact_task_trivial_move_sst_count + .with_label_values(&[group_label]) + .observe(task.input_ssts[0].table_infos.len() as _); + } + } + + group_label_map.keys().for_each(|group_id| { + trigger_sst_stat( + metrics, + compact_status.get(group_id), + current_version, + *group_id, + ); + }); +} diff --git a/src/meta/src/rpc/metrics.rs b/src/meta/src/rpc/metrics.rs index 0b7863ccbdad4..13e423148c2b1 100644 --- a/src/meta/src/rpc/metrics.rs +++ b/src/meta/src/rpc/metrics.rs @@ -169,6 +169,7 @@ pub struct MetaMetrics { pub split_compaction_group_count: IntCounterVec, pub state_table_count: IntGaugeVec, pub branched_sst_count: IntGaugeVec, + pub compact_task_trivial_move_sst_count: HistogramVec, pub compaction_event_consumed_latency: Histogram, pub compaction_event_loop_iteration_latency: Histogram, @@ -772,6 +773,14 @@ impl MetaMetrics { ) .unwrap(); + let opts = histogram_opts!( + "storage_compact_task_trivial_move_sst_count", + "sst count of compact trivial-move task", + exponential_buckets(1.0, 2.0, 8).unwrap() + ); + let compact_task_trivial_move_sst_count = + register_histogram_vec_with_registry!(opts, &["group"], registry).unwrap(); + Self { grpc_latency, barrier_latency, @@ -834,6 +843,7 @@ impl MetaMetrics { compact_task_size, compact_task_file_count, compact_task_batch_count, + compact_task_trivial_move_sst_count, table_write_throughput, split_compaction_group_count, state_table_count, diff --git a/src/storage/hummock_sdk/src/compact_task.rs b/src/storage/hummock_sdk/src/compact_task.rs index d173f6d252725..5096c272b5a91 100644 --- a/src/storage/hummock_sdk/src/compact_task.rs +++ b/src/storage/hummock_sdk/src/compact_task.rs @@ -12,14 +12,15 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::collections::{BTreeMap, HashMap}; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::mem::size_of; use itertools::Itertools; -use risingwave_pb::hummock::compact_task::{PbTaskStatus, PbTaskType, TaskStatus}; +use risingwave_pb::hummock::compact_task::{PbTaskStatus, PbTaskType, TaskStatus, TaskType}; use risingwave_pb::hummock::subscribe_compaction_event_request::PbReportTask; use risingwave_pb::hummock::{ - PbCompactTask, PbKeyRange, PbTableOption, PbTableSchema, PbTableStats, PbValidationTask, + LevelType, PbCompactTask, PbKeyRange, PbTableOption, PbTableSchema, PbTableStats, + PbValidationTask, }; use crate::key_range::KeyRange; @@ -112,6 +113,47 @@ impl CompactTask { .map(|table_watermark| size_of::() + table_watermark.estimated_encode_len()) .sum::() } + + pub fn is_trivial_move_task(&self) -> bool { + if self.task_type != TaskType::Dynamic && self.task_type != TaskType::Emergency { + return false; + } + + if self.input_ssts.len() != 2 || self.input_ssts[0].level_type != LevelType::Nonoverlapping + { + return false; + } + + // it may be a manual compaction task + if self.input_ssts[0].level_idx == self.input_ssts[1].level_idx + && self.input_ssts[0].level_idx > 0 + { + return false; + } + + if self.input_ssts[1].level_idx == self.target_level + && self.input_ssts[1].table_infos.is_empty() + { + return true; + } + + false + } + + pub fn is_trivial_reclaim(&self) -> bool { + // Currently all VnodeWatermark tasks are trivial reclaim. + if self.task_type == TaskType::VnodeWatermark { + return true; + } + let exist_table_ids = HashSet::::from_iter(self.existing_table_ids.clone()); + self.input_ssts.iter().all(|level| { + level.table_infos.iter().all(|sst| { + sst.table_ids + .iter() + .all(|table_id| !exist_table_ids.contains(table_id)) + }) + }) + } } impl From for CompactTask { From a4624247a04838fd8f516078f8d5d6dec0ae702f Mon Sep 17 00:00:00 2001 From: Li0k Date: Thu, 23 Jan 2025 21:50:32 +0800 Subject: [PATCH 2/9] address comments --- .../picker/trivial_move_compaction_picker.rs | 29 +++++++++++++++++-- src/meta/src/hummock/level_handler.rs | 5 ++++ 2 files changed, 32 insertions(+), 2 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 08d708a1c8c12..3b3024e07d1e1 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 @@ -70,13 +70,23 @@ impl TrivialMovePicker { } // find the first sst that can be trivial moved. - if sst.sst_size < self.sst_allowed_trivial_move_min_size && result.is_empty() { + if sst.sst_size < self.sst_allowed_trivial_move_min_size { skip_by_size = true; + + if !result.is_empty() { + break; + } + continue; } if level_handlers[self.level].is_pending_compact(&sst.sst_id) { skip_by_pending = true; + + if !result.is_empty() { + break; + } + continue; } overlap_info.update(sst); @@ -238,7 +248,7 @@ pub mod tests { .sst_allowed_trivial_move_min_size(Some(50)) .build(), ); - let levels_handler = vec![LevelHandler::new(0), LevelHandler::new(1)]; + let mut levels_handler = vec![LevelHandler::new(0), LevelHandler::new(1)]; let overlap_strategy = create_overlap_strategy(config.compaction_mode()); { let trivial_move_picker = @@ -282,5 +292,20 @@ pub mod tests { assert!(trivial_move_task.is_some()); assert_eq!(trivial_move_task.unwrap().len(), 2); } + + { + levels_handler[0].test_add_pending_sst(2, 1); + let trivial_move_picker = + super::TrivialMovePicker::new(0, 1, overlap_strategy.clone(), 50, 4); + let trivial_move_task = trivial_move_picker.pick_multi_trivial_move_ssts( + &[sst1.clone(), sst2.clone(), sst3.clone(), sst4.clone()], + &[], + &levels_handler, + &mut Default::default(), + ); + + assert!(trivial_move_task.is_some()); + assert_eq!(trivial_move_task.unwrap().len(), 1); + } } } diff --git a/src/meta/src/hummock/level_handler.rs b/src/meta/src/hummock/level_handler.rs index 03a17e3dd60d8..60b4b6fac184b 100644 --- a/src/meta/src/hummock/level_handler.rs +++ b/src/meta/src/hummock/level_handler.rs @@ -137,6 +137,11 @@ impl LevelHandler { pub fn compacting_files(&self) -> &HashMap { &self.compacting_files } + + #[cfg(test)] + pub(crate) fn test_add_pending_sst(&mut self, sst_id: HummockSstableId, task_id: u64) { + self.compacting_files.insert(sst_id, task_id); + } } impl From<&LevelHandler> for risingwave_pb::hummock::LevelHandler { From ec576599b0f1b50311edb507249e24ba06bb9701 Mon Sep 17 00:00:00 2001 From: Li0k Date: Fri, 24 Jan 2025 01:30:17 +0800 Subject: [PATCH 3/9] fix check --- .../src/hummock/compaction/picker/vnode_watermark_picker.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/meta/src/hummock/compaction/picker/vnode_watermark_picker.rs b/src/meta/src/hummock/compaction/picker/vnode_watermark_picker.rs index dcfb4a59daa94..a310ad87d77ed 100644 --- a/src/meta/src/hummock/compaction/picker/vnode_watermark_picker.rs +++ b/src/meta/src/hummock/compaction/picker/vnode_watermark_picker.rs @@ -31,7 +31,7 @@ impl VnodeWatermarkCompactionPicker { } /// The current implementation only picks trivial reclaim task for the bottommost level. - /// Must modify [`crate::hummock::compaction::CompactStatus::is_trivial_reclaim`], if nontrivial reclaim is supported in the future. + /// Must modify `is_trivial_reclaim`, if non-trivial reclaim is supported in the future. pub fn pick_compaction( &mut self, levels: &Levels, From baf45e693f425192f239c220983f3f29949895dc Mon Sep 17 00:00:00 2001 From: Li0k Date: Fri, 24 Jan 2025 15:17:40 +0800 Subject: [PATCH 4/9] typo --- src/common/src/config.rs | 4 ++++ src/config/docs.md | 2 ++ src/config/example.toml | 2 ++ src/meta/src/hummock/compaction/compaction_config.rs | 3 +++ 4 files changed, 11 insertions(+) diff --git a/src/common/src/config.rs b/src/common/src/config.rs index b26cdbe894698..e11215f01dd41 100644 --- a/src/common/src/config.rs +++ b/src/common/src/config.rs @@ -2681,6 +2681,10 @@ pub struct CompactionConfig { pub enable_emergency_picker: bool, #[serde(default = "default::compaction_config::max_level")] pub max_level: u32, + #[serde(default = "default::compaction_config::sst_allowed_trivial_move_min_size")] + pub sst_allowed_trivial_move_min_size: u64, + #[serde(default = "default::compaction_config::sst_allowed_trivial_move_max_count")] + pub sst_allowed_trivial_move_max_count: u32, } /// Note: only applies to meta store backends other than `SQLite`. diff --git a/src/config/docs.md b/src/config/docs.md index f055d53abce6f..030c61697ac13 100644 --- a/src/config/docs.md +++ b/src/config/docs.md @@ -88,6 +88,8 @@ This page is automatically generated by `./risedev generate-example-config` | max_level | | 6 | | max_space_reclaim_bytes | | 536870912 | | max_sub_compaction | | 4 | +| sst_allowed_trivial_move_max_count | | 64 | +| sst_allowed_trivial_move_min_size | | 4194304 | | sub_level_max_compaction_bytes | | 134217728 | | target_file_size_base | | 33554432 | | tombstone_reclaim_ratio | | 40 | diff --git a/src/config/example.toml b/src/config/example.toml index 56c7cd1525734..3dd5993e2d8bc 100644 --- a/src/config/example.toml +++ b/src/config/example.toml @@ -81,6 +81,8 @@ level0_max_compact_file_number = 100 tombstone_reclaim_ratio = 40 enable_emergency_picker = true max_level = 6 +sst_allowed_trivial_move_min_size = 4194304 +sst_allowed_trivial_move_max_count = 64 [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 8424fadf530b9..a70b209d82459 100644 --- a/src/meta/src/hummock/compaction/compaction_config.rs +++ b/src/meta/src/hummock/compaction/compaction_config.rs @@ -104,6 +104,8 @@ impl CompactionConfigBuilder { .level0_max_compact_file_number(opt.level0_max_compact_file_number) .tombstone_reclaim_ratio(opt.tombstone_reclaim_ratio) .max_level(opt.max_level as u64) + .sst_allowed_trivial_move_min_size(Some(opt.sst_allowed_trivial_move_min_size)) + .sst_allowed_trivial_move_max_count(Some(opt.sst_allowed_trivial_move_max_count)) } pub fn build(self) -> CompactionConfig { @@ -165,4 +167,5 @@ builder_field! { level0_overlapping_sub_level_compact_level_count: u32, tombstone_reclaim_ratio: u32, sst_allowed_trivial_move_min_size: Option, + sst_allowed_trivial_move_max_count: Option, } From 7ae96fd91b806e04f0b95400584e5a0e32cf55ca Mon Sep 17 00:00:00 2001 From: Li0k Date: Sun, 26 Jan 2025 14:48:40 +0800 Subject: [PATCH 5/9] fix metrics --- src/meta/src/hummock/manager/compaction/mod.rs | 8 ++++++++ src/meta/src/hummock/metrics_utils.rs | 7 ------- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/meta/src/hummock/manager/compaction/mod.rs b/src/meta/src/hummock/manager/compaction/mod.rs index 84d8a4fe3e64b..56c2e84f48da3 100644 --- a/src/meta/src/hummock/manager/compaction/mod.rs +++ b/src/meta/src/hummock/manager/compaction/mod.rs @@ -906,6 +906,14 @@ impl HummockManager { .compact_task_batch_count .with_label_values(&["batch_trivial_move"]) .observe(trivial_tasks.len() as f64); + + for trivial_task in &trivial_tasks { + self.metrics + .compact_task_trivial_move_sst_count + .with_label_values(&[&trivial_task.compaction_group_id.to_string()]) + .observe(trivial_task.input_ssts[0].table_infos.len() as _); + } + drop(versioning_guard); } else { // We are using a single transaction to ensure that each task has progress when it is diff --git a/src/meta/src/hummock/metrics_utils.rs b/src/meta/src/hummock/metrics_utils.rs index d032260115528..bbc6ecc4c3ee5 100644 --- a/src/meta/src/hummock/metrics_utils.rs +++ b/src/meta/src/hummock/metrics_utils.rs @@ -696,13 +696,6 @@ pub fn trigger_compact_tasks_stat( .compact_frequency .with_label_values(&["normal", group_label, task_type_label, task_status_label]) .inc(); - - if task.is_trivial_move_task() { - metrics - .compact_task_trivial_move_sst_count - .with_label_values(&[group_label]) - .observe(task.input_ssts[0].table_infos.len() as _); - } } group_label_map.keys().for_each(|group_id| { From e6ad11793c7ddbaaa7bfe614ec10e9d0e41fd9af Mon Sep 17 00:00:00 2001 From: Li0k Date: Sun, 26 Jan 2025 17:25:40 +0800 Subject: [PATCH 6/9] add doc --- .../hummock/compaction/picker/trivial_move_compaction_picker.rs | 1 + 1 file changed, 1 insertion(+) 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 3b3024e07d1e1..3cb70aafad8ea 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 @@ -73,6 +73,7 @@ impl TrivialMovePicker { if sst.sst_size < self.sst_allowed_trivial_move_min_size { skip_by_size = true; + // Stop probing if we have already found some sst to move. And should avoid small sst move to target level. if !result.is_empty() { break; } From 6e248ae04c2c0d187549a97644641a54145b4835 Mon Sep 17 00:00:00 2001 From: Li0k Date: Mon, 27 Jan 2025 02:48:24 +0800 Subject: [PATCH 7/9] fix file count --- .../src/hummock/compaction/picker/intra_compaction_picker.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 82bf4be2899d2..6f201c1ef6acd 100644 --- a/src/meta/src/hummock/compaction/picker/intra_compaction_picker.rs +++ b/src/meta/src/hummock/compaction/picker/intra_compaction_picker.rs @@ -292,6 +292,7 @@ impl IntraCompactionPicker { .is_empty()); let select_input_size = select_ssts.iter().map(|sst| sst.sst_size).sum(); + let total_file_count = select_ssts.len() as u64; let input_levels = vec![ InputLevel { level_idx: 0, @@ -309,7 +310,7 @@ impl IntraCompactionPicker { target_level: 0, target_sub_level_id: level.sub_level_id, select_input_size, - total_file_count: 1, + total_file_count, ..Default::default() }); } From 6482f770cfbe97e0d5c27d02c4ea53b3e5142f10 Mon Sep 17 00:00:00 2001 From: Li0k Date: Thu, 6 Feb 2025 14:41:34 +0800 Subject: [PATCH 8/9] address comments --- src/meta/src/hummock/compaction/overlap_strategy.rs | 5 ----- .../compaction/picker/trivial_move_compaction_picker.rs | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/meta/src/hummock/compaction/overlap_strategy.rs b/src/meta/src/hummock/compaction/overlap_strategy.rs index 60d7e5ed210b6..d990dfa9ce8e6 100644 --- a/src/meta/src/hummock/compaction/overlap_strategy.rs +++ b/src/meta/src/hummock/compaction/overlap_strategy.rs @@ -26,7 +26,6 @@ pub trait OverlapInfo: Debug { fn check_multiple_overlap(&self, others: &[SstableInfo]) -> Range; fn check_multiple_include(&self, others: &[SstableInfo]) -> Range; fn update(&mut self, table: &SstableInfo); - fn clear(&mut self); } pub trait OverlapStrategy: Send + Sync { @@ -135,10 +134,6 @@ impl OverlapInfo for RangeOverlapInfo { } self.target_range = Some(other.clone()); } - - fn clear(&mut self) { - self.target_range = None; - } } #[derive(Default)] 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 3cb70aafad8ea..0b275ae4af77a 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 @@ -102,7 +102,7 @@ impl TrivialMovePicker { } // reset overlap_info - overlap_info.clear(); + overlap_info = self.overlap_strategy.create_overlap_info(); } } From b1e1ec910d9b5a975f6e0fe00b87f63456b2d73a Mon Sep 17 00:00:00 2001 From: Li0k Date: Thu, 6 Feb 2025 17:44:47 +0800 Subject: [PATCH 9/9] typo --- proto/hummock.proto | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/proto/hummock.proto b/proto/hummock.proto index 673d1d06c0f6b..1995adceab959 100644 --- a/proto/hummock.proto +++ b/proto/hummock.proto @@ -661,15 +661,16 @@ message RiseCtlUpdateCompactionConfigRequest { uint32 split_weight_by_vnode = 20; bool disable_auto_group_scheduling = 21; uint64 max_overlapping_level_size = 22; - uint32 sst_allowed_trivial_move_max_count = 23; // The emergency compaction limitations for the level0 sstables file count - uint32 emergency_level0_sst_file_count = 24; + uint32 emergency_level0_sst_file_count = 25; // The emergency compaction limitations for the level0 sub level partition - uint32 emergency_level0_sub_level_partition = 25; + uint32 emergency_level0_sub_level_partition = 26; // The limitation of the max sst size of the level0 to trigger the write stop - uint32 level0_stop_write_threshold_max_sst_count = 26; + uint32 level0_stop_write_threshold_max_sst_count = 27; // The limitation of the max sst size of the level0 to trigger the write stop - uint64 level0_stop_write_threshold_max_size = 27; + uint64 level0_stop_write_threshold_max_size = 28; + // The limitation of the max sst count of the trivial move task + uint32 sst_allowed_trivial_move_max_count = 29; } } repeated uint64 compaction_group_ids = 1; @@ -873,18 +874,20 @@ message CompactionConfig { // hummock will reorg the commit-sstables to the multi overlapping-level if the size of the commit-sstables is larger than `max_overlapping_level_size` optional uint64 max_overlapping_level_size = 24; - // The limitation of the max sst count of the trivial move task - optional uint32 sst_allowed_trivial_move_max_count = 25; // The emergency compaction limitations for the level0 sstables file count - optional uint32 emergency_level0_sst_file_count = 26; + optional uint32 emergency_level0_sst_file_count = 25; // The emergency compaction limitations for the level0 sub level partition - optional uint32 emergency_level0_sub_level_partition = 27; + optional uint32 emergency_level0_sub_level_partition = 26; // The limitation of the max sst count of the level0 to trigger the write stop - optional uint32 level0_stop_write_threshold_max_sst_count = 28; + optional uint32 level0_stop_write_threshold_max_sst_count = 27; + // The limitation of the max sst size of the level0 to trigger the write stop - optional uint64 level0_stop_write_threshold_max_size = 29; + optional uint64 level0_stop_write_threshold_max_size = 28; + + // The limitation of the max sst count of the trivial move task + optional uint32 sst_allowed_trivial_move_max_count = 29; } message TableStats {