Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor(storage): remove legacy delta type GroupMetaChange and GroupTableChange #18585

Merged
merged 2 commits into from
Sep 18, 2024
+8 −106
Merged

refactor(storage): remove legacy delta type GroupMetaChange and GroupTableChange #18585

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
364e7d3
refactor(storage): remove legacy version delta type
Li0k Sep 18, 2024
89a8d22
typo
Li0k Sep 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 4 additions & 17 deletions proto/hummock.proto
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading