refactor(meta): deprecate branched sst #16698
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
| filter_ssts(group.get_level(input_level.level_idx as _), &mut sst_ids); | ||
| } | ||
| if !sst_ids.is_empty() { | ||
| warn!(stale_sst_id = ?sst_ids, ?compact_task, "compact task expired"); |
There was a problem hiding this comment.
I don't think this line should ever be reached if there's no bug, i.e. this is_compact_task_expired is no longer necessary.
I was wrong. Whenever split a compaction group, the SST id in parent group will be changed too.
There was a problem hiding this comment.
And it's kind of compute-intensive because it need to iterate through all SSTs in corresponding level.
There was a problem hiding this comment.
I think the cost is acceptable. That's how we currently do it when apply_version_delta.
| .state_table_count | ||
| .with_label_values(&[&group_label]) | ||
| .set(member_table_id_len as _); | ||
| pub fn trigger_split_stat(metrics: &MetaMetrics, version: &HummockVersion) { |
There was a problem hiding this comment.
build_branched_sst_info need be called whenever a compaction task is reported, which I'm concerned is compute-intensive.
@Li0k @Little-Wallace
There was a problem hiding this comment.
I'm OK with simply disabling this stats.
There was a problem hiding this comment.
I have changed to trigger the split stat when we periodically do the checkpoint.
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Remove the code of branched ssts.
Currently it is used in
For 1, we changed to rebuild the full branched sst every time we calculate the stat
For 2, we currently don't have code to move to an existing compaction group, and branched sst related code only exists in cases for existing compaction group, so we can safely remove the usage.
For 3, we change to validate whether the input sst still exists for a reported compact task.
After changing 1, 2 and 3, we don't have any usage on branched sst any more, and we can deprecate it.
Checklist
./risedev check(or alias,./risedev c)Documentation
Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.