fix(storage): fix the trivial-move loop caused by config and pick_who…

…le_level (#17721)
risingwavelabs · Jul 17, 2024 · 4605773 · 4605773
1 parent 30fd4d8
commit 4605773
Show file tree

Hide file tree

Showing 7 changed files with 58 additions and 15 deletions.
diff --git a/proto/hummock.proto b/proto/hummock.proto
@@ -905,7 +905,7 @@ message CompactionConfig {
   bool enable_emergency_picker = 20;
 
   // The limitation of the level count of l0 compaction
-  uint32 max_l0_compact_level_count = 21;
+  optional uint32 max_l0_compact_level_count = 21;
 }
 
 message TableStats {

diff --git a/src/meta/src/hummock/compaction/compaction_config.rs b/src/meta/src/hummock/compaction/compaction_config.rs
@@ -64,7 +64,7 @@ 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(),
-                max_l0_compact_level_count: compaction_config::max_l0_compact_level_count(),
+                max_l0_compact_level_count: Some(compaction_config::max_l0_compact_level_count()),
             },
         }
     }

diff --git a/src/meta/src/hummock/compaction/mod.rs b/src/meta/src/hummock/compaction/mod.rs
@@ -26,7 +26,7 @@ use std::fmt::{Debug, Formatter};
 use std::sync::Arc;
 
 use picker::{LevelCompactionPicker, TierCompactionPicker};
-use risingwave_hummock_sdk::{can_concat, CompactionGroupId, HummockCompactionTaskId};
+use risingwave_hummock_sdk::{CompactionGroupId, HummockCompactionTaskId};
 use risingwave_pb::hummock::compaction_config::CompactionMode;
 use risingwave_pb::hummock::hummock_version::Levels;
 use risingwave_pb::hummock::{CompactTask, CompactionConfig, LevelType};
@@ -140,10 +140,7 @@ impl CompactStatus {
             return false;
         }
 
-        if task.input_ssts.len() == 1 {
-            return task.input_ssts[0].level_idx == 0
-                && can_concat(&task.input_ssts[0].table_infos);
-        } else if task.input_ssts.len() != 2
+        if task.input_ssts.len() != 2
             || task.input_ssts[0].level_type() != LevelType::Nonoverlapping
         {
             return false;

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
@@ -16,6 +16,7 @@ use std::cell::RefCell;
 use std::sync::Arc;
 
 use itertools::Itertools;
+use risingwave_common::config::default::compaction_config;
 use risingwave_hummock_sdk::compaction_group::hummock_version_ext::HummockLevelsExt;
 use risingwave_pb::hummock::hummock_version::Levels;
 use risingwave_pb::hummock::{CompactionConfig, InputLevel, Level, LevelType, OverlappingLevel};
@@ -166,7 +167,9 @@ impl LevelCompactionPicker {
             self.config.level0_max_compact_file_number,
             overlap_strategy.clone(),
             self.developer_config.enable_check_task_level_overlap,
-            self.config.max_l0_compact_level_count as usize,
+            self.config
+                .max_l0_compact_level_count
+                .unwrap_or(compaction_config::max_l0_compact_level_count()) as usize,
         );
 
         let mut max_vnode_partition_idx = 0;

diff --git a/src/meta/src/hummock/compaction/picker/compaction_task_validator.rs b/src/meta/src/hummock/compaction/picker/compaction_task_validator.rs
@@ -15,6 +15,7 @@
 use std::collections::HashMap;
 use std::sync::Arc;
 
+use risingwave_common::config::default::compaction_config;
 use risingwave_pb::hummock::CompactionConfig;
 
 use super::{CompactionInput, LocalPickerStatistic};
@@ -90,7 +91,12 @@ struct TierCompactionTaskValidationRule {
 impl CompactionTaskValidationRule for TierCompactionTaskValidationRule {
     fn validate(&self, input: &CompactionInput, stats: &mut LocalPickerStatistic) -> bool {
         if input.total_file_count >= self.config.level0_max_compact_file_number
-            || input.input_levels.len() >= self.config.max_l0_compact_level_count as usize
+            || input.input_levels.len()
+                >= self
+                    .config
+                    .max_l0_compact_level_count
+                    .unwrap_or(compaction_config::max_l0_compact_level_count())
+                    as usize
         {
             return true;
         }
@@ -124,7 +130,12 @@ impl CompactionTaskValidationRule for IntraCompactionTaskValidationRule {
     fn validate(&self, input: &CompactionInput, stats: &mut LocalPickerStatistic) -> bool {
         if (input.total_file_count >= self.config.level0_max_compact_file_number
             && input.input_levels.len() > 1)
-            || input.input_levels.len() >= self.config.max_l0_compact_level_count as usize
+            || input.input_levels.len()
+                >= self
+                    .config
+                    .max_l0_compact_level_count
+                    .unwrap_or(compaction_config::max_l0_compact_level_count())
+                    as usize
         {
             return true;
         }
@@ -172,7 +183,12 @@ struct BaseCompactionTaskValidationRule {
 impl CompactionTaskValidationRule for BaseCompactionTaskValidationRule {
     fn validate(&self, input: &CompactionInput, stats: &mut LocalPickerStatistic) -> bool {
         if input.total_file_count >= self.config.level0_max_compact_file_number
-            || input.input_levels.len() >= self.config.max_l0_compact_level_count as usize
+            || input.input_levels.len()
+                >= self
+                    .config
+                    .max_l0_compact_level_count
+                    .unwrap_or(compaction_config::max_l0_compact_level_count())
+                    as usize
         {
             return true;
         }

diff --git a/src/meta/src/hummock/compaction/picker/intra_compaction_picker.rs b/src/meta/src/hummock/compaction/picker/intra_compaction_picker.rs
@@ -14,6 +14,7 @@
 
 use std::sync::Arc;
 
+use risingwave_common::config::default::compaction_config;
 use risingwave_pb::hummock::hummock_version::Levels;
 use risingwave_pb::hummock::{CompactionConfig, InputLevel, LevelType, OverlappingLevel};
 
@@ -54,10 +55,33 @@ impl CompactionPicker for IntraCompactionPicker {
         if let Some(ret) =
             self.pick_whole_level(l0, &level_handlers[0], vnode_partition_count, stats)
         {
+            if ret.input_levels.len() < 2 {
+                tracing::error!(
+                    ?ret,
+                    vnode_partition_count,
+                    "pick_whole_level failed to pick enough levels"
+                );
+                return None;
+            }
+
             return Some(ret);
         }
 
-        self.pick_l0_intra(l0, &level_handlers[0], vnode_partition_count, stats)
+        if let Some(ret) = self.pick_l0_intra(l0, &level_handlers[0], vnode_partition_count, stats)
+        {
+            if ret.input_levels.len() < 2 {
+                tracing::error!(
+                    ?ret,
+                    vnode_partition_count,
+                    "pick_l0_intra failed to pick enough levels"
+                );
+                return None;
+            }
+
+            return Some(ret);
+        }
+
+        None
     }
 }
 
@@ -144,7 +168,10 @@ impl IntraCompactionPicker {
                 self.config.level0_max_compact_file_number,
                 overlap_strategy.clone(),
                 self.developer_config.enable_check_task_level_overlap,
-                self.config.max_l0_compact_level_count as usize,
+                self.config
+                    .max_l0_compact_level_count
+                    .unwrap_or(compaction_config::max_l0_compact_level_count())
+                    as usize,
             );
 
             let l0_select_tables_vec = non_overlap_sub_level_picker
@@ -357,7 +384,7 @@ impl WholeLevelCompactionPicker {
                     table_infos: next_level.table_infos.clone(),
                 });
             }
-            if !select_level_inputs.is_empty() {
+            if select_level_inputs.len() > 1 {
                 let vnode_partition_count =
                     if select_input_size > self.config.sub_level_max_compaction_bytes / 2 {
                         partition_count

diff --git a/src/meta/src/hummock/manager/compaction_group_manager.rs b/src/meta/src/hummock/manager/compaction_group_manager.rs
@@ -761,7 +761,7 @@ fn update_compaction_config(target: &mut CompactionConfig, items: &[MutableConfi
                     .clone_from(&c.compression_algorithm);
             }
             MutableConfig::MaxL0CompactLevelCount(c) => {
-                target.max_l0_compact_level_count = *c;
+                target.max_l0_compact_level_count = Some(*c);
             }
         }
     }