Skip to content

Commit

Permalink
refactor(storage): remove legacy delta type GroupMetaChange and `Gr…
Browse files Browse the repository at this point in the history 
…oupTableChange` (#18585)
  • Loading branch information
@Li0k
Li0k authored Sep 18, 2024
1 parent 5b625f8 commit ff479f6
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 106 deletions.
21 changes: 4 additions & 17 deletions proto/hummock.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;
}
}
Expand Down
60 changes: 2 additions & 58 deletions src/storage/hummock_sdk/src/compaction_group/hummock_version_ext.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -49,8 +48,6 @@ pub struct GroupDeltasSummary {
pub insert_table_infos: Vec<SstableInfo>,
pub group_construct: Option<GroupConstruct>,
pub group_destroy: Option<CompactionGroupId>,
pub group_meta_changes: Vec<GroupMetaChange>,
pub group_table_change: Option<GroupTableChange>,
pub new_vnode_partition_count: u32,
pub group_merge: Option<GroupMerge>,
}
Expand All @@ -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;

Expand All @@ -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);
Expand All @@ -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,
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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 {:?}",
Expand All @@ -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(),
Expand Down
33 changes: 2 additions & 31 deletions src/storage/hummock_sdk/src/version.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -924,11 +924,6 @@ pub enum GroupDeltaCommon<T> {
IntraLevel(IntraLevelDeltaCommon<T>),
GroupConstruct(PbGroupConstruct),
GroupDestroy(PbGroupDestroy),
GroupMetaChange(PbGroupMetaChange),

#[allow(dead_code)]
GroupTableChange(PbGroupTableChange),

GroupMerge(PbGroupMerge),
}

Expand All @@ -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)
}
Expand All @@ -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)),
},
Expand All @@ -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)),
},
Expand All @@ -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)
}
Expand Down

0 comments on commit ff479f6

Please sign in to comment.