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.

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): Support in-place disk format upgrades for major database version bumps #8748

Merged
merged 7 commits into from
Aug 9, 2024

Conversation

upbqdn
Copy link
Member

@upbqdn upbqdn commented Aug 8, 2024

Motivation

Close #8745.

Solution

  • Try to move an existing cached state to a new location so it can be used again.

Tests

  • I manually tested the solution.

PR Author's Checklist

  • The PR name will make sense to users.
  • The PR provides a CHANGELOG summary.
  • The solution is tested.
  • The documentation is up to date.
  • The PR has a priority label.

PR Reviewer's Checklist

  • The PR Author's checklist is complete.
  • The PR resolves the issue.

@upbqdn upbqdn added A-state Area: State / database changes P-Medium ⚡ labels Aug 8, 2024
@upbqdn upbqdn self-assigned this Aug 8, 2024
@upbqdn upbqdn requested a review from a team as a code owner August 8, 2024 12:54
@upbqdn upbqdn requested review from oxarbitrage and removed request for a team August 8, 2024 12:54
Copy link
Contributor

@arya2 arya2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking great, thank you!

zebra-state/src/config.rs Show resolved Hide resolved
Copy link
Collaborator

@conradoplg conradoplg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a small fix required for Windows.

I was confused by some of the code but I think I got it. I suggested some comments which should hopefully help whoever reads the code in the future, feel free to tweak them.

Also I noticed the upgrade itself is not here, was it planned to be done in another PR?

mpguerra and others added 2 commits August 9, 2024 09:28
Co-authored-by: Conrado Gouvea <conrado@zfnd.org>
@upbqdn
Copy link
Member Author

upbqdn commented Aug 9, 2024

I was confused by some of the code but I think I got it. I suggested some comments which should hopefully help whoever reads the code in the future, feel free to tweak them.

I originally had similar comments in the code, but I deleted them because I thought the info and warn messages were descriptive enough. Thanks for adding them.

@upbqdn
Copy link
Member Author

upbqdn commented Aug 9, 2024

Also I noticed the upgrade itself is not here, was it planned to be done in another PR?

Yes, this PR only moves the old db to a new path. Zebra then uses the db and rewrites the ValueBalance, stored under the same key as the original one. Current Zebra can read both the original and new format of ValueBalance from the db, but Zebra 1.8 can't, so we needed to bump the major db version. If we didn't move the db, Zebra 1.9 would start syncing the cached state from scratch.

The new ValueBalance has 40 bytes, and the original one has 32. It was trivial to implement deserialization of both formats, so we got backward compatibility with previous Zebra versions, but previous Zebra versions are not forward-compatible with the change since they'll panic when they try to deserialize the new format.

Copy link
Collaborator

@conradoplg conradoplg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, got it, thanks. LGTM!

@mergify mergify bot merged commit 88f9bff into main Aug 9, 2024
132 of 135 checks passed
@mergify mergify bot deleted the move-db branch August 9, 2024 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-state Area: State / database changes P-Medium ⚡
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support in-place disk format upgrades for major database version bumps
4 participants