-
Notifications
You must be signed in to change notification settings - Fork 592
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.
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
refactor(storage): remove most usage of max_committed_epoch #18641
Conversation
…-wait-hummock-version
…max-committed-epoch
…d-max-committed-epoch
…d-max-committed-epoch
…d-max-committed-epoch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -73,7 +73,7 @@ impl MetaSnapshotMetadata { | |||
id, | |||
hummock_version_id: v.id, | |||
ssts: v.get_object_ids(), | |||
max_committed_epoch: v.visible_table_committed_epoch(), | |||
max_committed_epoch: v.max_committed_epoch_for_meta(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The field max_committed_epoch
can be removed too.
Unknown fields will be ignored when deserializing MetaSnapshotMetadata.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The field will be deprecated in #18644
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Remove most usage of
max_committed_epoch
. Previously, themax_committed_epoch
has the semantic as the committed epoch of all visible tables. In this PR, all previous usage ofvisible_table_committed_epoch
is removed. Themax_committed_epoch
is only used meta node during recovery, to get the initial prev epoch after recovery. For this reason, themax_committed_epoch
is exposed only via methodmax_committed_epoch_for_meta
.Notable changes in this PR:
version()
method ofPinnedVersion
is removed. Instead we useimpl Deref<Target = HummockVersion> for PinnedVersion
.sstable_object_id_manager
.ConflictDetector
is removed, which was not used previously.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.