-
Notifications
You must be signed in to change notification settings - Fork 123
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
change(state): Refactor format upgrades into trait #9263
base: main
Are you sure you want to change the base?
Conversation
fn description(&self) -> &'static str; | ||
|
||
/// Runs disk format upgrade. | ||
fn run( |
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.
run_migration()
may be a better name for this method.
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.
Both are fine I think.
199059a
to
59c0b24
Compare
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.
This looks good overall, but I'm confused about some parts. (Sorry, not super familiar with the original code)
zebra-state/src/service/finalized_state/disk_format/upgrade/no_migration.rs
Outdated
Show resolved
Hide resolved
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.
Looks great, thanks! I left just a couple of documentation suggestions.
Sorry for taking this long - I wanted to learn about the overall database upgrade process since I'm also going to work with it
zebra-state/src/service/finalized_state/disk_format/upgrade/prune_trees.rs
Outdated
Show resolved
Hide resolved
fn description(&self) -> &'static str; | ||
|
||
/// Runs disk format upgrade. | ||
fn run( |
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.
Both are fine I think.
…uct, `PruneTrees`, and moves the logic for tree deduplication to the trait impl
…des to use new trait
- Avoids duplicate validation of format upgrades at startup when db is already upgraded, - Minor refactors - Doc fixes and cleanups
deff106
to
a9a17e2
Compare
Motivation
This PR organizes Zebra's disk format upgrades into trait implementations to make them more maintainable as more upgrades are gradually added.
Closes #7932
Solution
DiskFormatUpgrade
traitTests
This change should be covered by existing tests for applying db format upgrades.
Follow-up Work
The bundle of db format changes needed to match zcashd's RPC outputs.
PR Author's Checklist
PR Reviewer's Checklist