From ff479f60996f9ac61b0d8b5ddf40370c8b0217f8 Mon Sep 17 00:00:00 2001 From: Li0k Date: Wed, 18 Sep 2024 20:36:50 +0800 Subject: [PATCH] refactor(storage): remove legacy delta type `GroupMetaChange` and `GroupTableChange` (#18585) --- proto/hummock.proto | 21 ++----- .../compaction_group/hummock_version_ext.rs | 60 +------------------ src/storage/hummock_sdk/src/version.rs | 33 +--------- 3 files changed, 8 insertions(+), 106 deletions(-) diff --git a/proto/hummock.proto b/proto/hummock.proto index dc24870972566..0eb6aae8d5124 100644 --- a/proto/hummock.proto +++ b/proto/hummock.proto @@ -87,21 +87,6 @@ message GroupConstruct { CompatibilityVersion version = 6; } -message GroupMetaChange { - option deprecated = true; - repeated uint32 table_ids_add = 1 [deprecated = true]; - repeated uint32 table_ids_remove = 2 [deprecated = true]; -} - -message GroupTableChange { - option deprecated = true; - repeated uint32 table_ids = 1 [deprecated = true]; - uint64 target_group_id = 2; - uint64 origin_group_id = 3; - uint64 new_sst_start_id = 4; - CompatibilityVersion version = 5; -} - message GroupDestroy {} message GroupMerge { @@ -110,12 +95,14 @@ message GroupMerge { } message GroupDelta { + reserved 4; + reserved "group_meta_change"; + reserved 5; + reserved "group_table_change"; oneof delta_type { IntraLevelDelta intra_level = 1; GroupConstruct group_construct = 2; GroupDestroy group_destroy = 3; - GroupMetaChange group_meta_change = 4 [deprecated = true]; - GroupTableChange group_table_change = 5 [deprecated = true]; GroupMerge group_merge = 6; } } diff --git a/src/storage/hummock_sdk/src/compaction_group/hummock_version_ext.rs b/src/storage/hummock_sdk/src/compaction_group/hummock_version_ext.rs index 376626e844242..c1d566b995a1c 100644 --- a/src/storage/hummock_sdk/src/compaction_group/hummock_version_ext.rs +++ b/src/storage/hummock_sdk/src/compaction_group/hummock_version_ext.rs @@ -22,8 +22,7 @@ use itertools::Itertools; use risingwave_common::catalog::TableId; use risingwave_common::hash::VnodeBitmapExt; use risingwave_pb::hummock::{ - CompactionConfig, CompatibilityVersion, GroupConstruct, GroupMerge, GroupMetaChange, - GroupTableChange, PbLevelType, + CompactionConfig, CompatibilityVersion, GroupConstruct, GroupMerge, PbLevelType, }; use tracing::warn; @@ -49,8 +48,6 @@ pub struct GroupDeltasSummary { pub insert_table_infos: Vec, pub group_construct: Option, pub group_destroy: Option, - pub group_meta_changes: Vec, - pub group_table_change: Option, pub new_vnode_partition_count: u32, pub group_merge: Option, } @@ -66,8 +63,6 @@ pub fn summarize_group_deltas( let mut insert_table_infos = vec![]; let mut group_construct = None; let mut group_destroy = None; - let mut group_meta_changes = vec![]; - let mut group_table_change = None; let mut new_vnode_partition_count = 0; let mut group_merge = None; @@ -93,12 +88,6 @@ pub fn summarize_group_deltas( assert!(group_destroy.is_none()); group_destroy = Some(compaction_group_id); } - GroupDelta::GroupMetaChange(meta_delta) => { - group_meta_changes.push(meta_delta.clone()); - } - GroupDelta::GroupTableChange(meta_delta) => { - group_table_change = Some(meta_delta.clone()); - } GroupDelta::GroupMerge(merge_delta) => { assert!(group_merge.is_none()); group_merge = Some(*merge_delta); @@ -118,8 +107,6 @@ pub fn summarize_group_deltas( insert_table_infos, group_construct, group_destroy, - group_meta_changes, - group_table_change, new_vnode_partition_count, group_merge, } @@ -404,8 +391,7 @@ impl HummockVersion { } } return; - } - if !self.levels.contains_key(&parent_group_id) { + } else if !self.levels.contains_key(&parent_group_id) { warn!(parent_group_id, "non-existing parent group id to init from"); return; } @@ -617,38 +603,6 @@ impl HummockVersion { member_table_ids, group_construct.get_new_sst_start_id(), ); - } else if let Some(group_change) = &summary.group_table_change { - // TODO: may deprecate this branch? This enum variant is not created anywhere - assert!( - group_change.version <= CompatibilityVersion::NoTrivialSplit as _, - "DeltaType::GroupTableChange is not used anymore after CompatibilityVersion::NoMemberTableIds is added" - ); - #[expect(deprecated)] - // for backward-compatibility of previous hummock version delta - self.init_with_parent_group( - group_change.origin_group_id, - group_change.target_group_id, - BTreeSet::from_iter(group_change.table_ids.clone()), - group_change.new_sst_start_id, - ); - - let levels = self - .levels - .get_mut(&group_change.origin_group_id) - .expect("compaction group should exist"); - #[expect(deprecated)] - // for backward-compatibility of previous hummock version delta - let mut moving_tables = levels - .member_table_ids - .extract_if(|t| group_change.table_ids.contains(t)) - .collect_vec(); - #[expect(deprecated)] - // for backward-compatibility of previous hummock version delta - self.levels - .get_mut(compaction_group_id) - .expect("compaction group should exist") - .member_table_ids - .append(&mut moving_tables); } else if let Some(group_merge) = &summary.group_merge { tracing::info!( "group_merge left {:?} right {:?}", @@ -662,16 +616,6 @@ impl HummockVersion { let levels = self.levels.get_mut(compaction_group_id).unwrap_or_else(|| { panic!("compaction group {} does not exist", compaction_group_id) }); - #[expect(deprecated)] // for backward-compatibility of previous hummock version delta - for group_meta_delta in &summary.group_meta_changes { - levels - .member_table_ids - .extend(group_meta_delta.table_ids_add.clone()); - levels - .member_table_ids - .retain(|t| !group_meta_delta.table_ids_remove.contains(t)); - levels.member_table_ids.sort(); - } assert!( visible_table_committed_epoch <= version_delta.visible_table_committed_epoch(), diff --git a/src/storage/hummock_sdk/src/version.rs b/src/storage/hummock_sdk/src/version.rs index 4aecfcde0cf48..fe2825cc8ad0c 100644 --- a/src/storage/hummock_sdk/src/version.rs +++ b/src/storage/hummock_sdk/src/version.rs @@ -25,8 +25,8 @@ use risingwave_pb::hummock::group_delta::PbDeltaType; use risingwave_pb::hummock::hummock_version_delta::PbGroupDeltas; use risingwave_pb::hummock::{ CompactionConfig, PbGroupConstruct, PbGroupDelta, PbGroupDestroy, PbGroupMerge, - PbGroupMetaChange, PbGroupTableChange, PbHummockVersion, PbHummockVersionDelta, - PbIntraLevelDelta, PbSstableInfo, PbStateTableInfo, StateTableInfo, StateTableInfoDelta, + PbHummockVersion, PbHummockVersionDelta, PbIntraLevelDelta, PbSstableInfo, PbStateTableInfo, + StateTableInfo, StateTableInfoDelta, }; use tracing::warn; @@ -924,11 +924,6 @@ pub enum GroupDeltaCommon { IntraLevel(IntraLevelDeltaCommon), GroupConstruct(PbGroupConstruct), GroupDestroy(PbGroupDestroy), - GroupMetaChange(PbGroupMetaChange), - - #[allow(dead_code)] - GroupTableChange(PbGroupTableChange), - GroupMerge(PbGroupMerge), } @@ -949,12 +944,6 @@ where Some(PbDeltaType::GroupDestroy(pb_group_destroy)) => { GroupDeltaCommon::GroupDestroy(pb_group_destroy) } - Some(PbDeltaType::GroupMetaChange(pb_group_meta_change)) => { - GroupDeltaCommon::GroupMetaChange(pb_group_meta_change) - } - Some(PbDeltaType::GroupTableChange(pb_group_table_change)) => { - GroupDeltaCommon::GroupTableChange(pb_group_table_change) - } Some(PbDeltaType::GroupMerge(pb_group_merge)) => { GroupDeltaCommon::GroupMerge(pb_group_merge) } @@ -978,12 +967,6 @@ where GroupDeltaCommon::GroupDestroy(pb_group_destroy) => PbGroupDelta { delta_type: Some(PbDeltaType::GroupDestroy(pb_group_destroy)), }, - GroupDeltaCommon::GroupMetaChange(pb_group_meta_change) => PbGroupDelta { - delta_type: Some(PbDeltaType::GroupMetaChange(pb_group_meta_change)), - }, - GroupDeltaCommon::GroupTableChange(pb_group_table_change) => PbGroupDelta { - delta_type: Some(PbDeltaType::GroupTableChange(pb_group_table_change)), - }, GroupDeltaCommon::GroupMerge(pb_group_merge) => PbGroupDelta { delta_type: Some(PbDeltaType::GroupMerge(pb_group_merge)), }, @@ -1006,12 +989,6 @@ where GroupDeltaCommon::GroupDestroy(pb_group_destroy) => PbGroupDelta { delta_type: Some(PbDeltaType::GroupDestroy(*pb_group_destroy)), }, - GroupDeltaCommon::GroupMetaChange(pb_group_meta_change) => PbGroupDelta { - delta_type: Some(PbDeltaType::GroupMetaChange(pb_group_meta_change.clone())), - }, - GroupDeltaCommon::GroupTableChange(pb_group_table_change) => PbGroupDelta { - delta_type: Some(PbDeltaType::GroupTableChange(pb_group_table_change.clone())), - }, GroupDeltaCommon::GroupMerge(pb_group_merge) => PbGroupDelta { delta_type: Some(PbDeltaType::GroupMerge(*pb_group_merge)), }, @@ -1034,12 +1011,6 @@ where Some(PbDeltaType::GroupDestroy(pb_group_destroy)) => { GroupDeltaCommon::GroupDestroy(*pb_group_destroy) } - Some(PbDeltaType::GroupMetaChange(pb_group_meta_change)) => { - GroupDeltaCommon::GroupMetaChange(pb_group_meta_change.clone()) - } - Some(PbDeltaType::GroupTableChange(pb_group_table_change)) => { - GroupDeltaCommon::GroupTableChange(pb_group_table_change.clone()) - } Some(PbDeltaType::GroupMerge(pb_group_merge)) => { GroupDeltaCommon::GroupMerge(*pb_group_merge) }