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

fix(state): If a format change is cancelled, also cancel the format check, and don't mark the database as upgraded #7442

Merged
merged 1 commit into from
Sep 1, 2023

Conversation

teor2345
Copy link
Contributor

Motivation

Zebra can keep running the post-upgrade code when a format upgrade is cancelled.

If the format check is exhaustive, this can cause a panic on shutdown.

If some data is not covered by the format check, it can either:

  • cause a panic on shutdown, or
  • incorrectly mark the database as upgraded.

Specifications

don't panic

Complex Code or Requirements

This is a race condition, so it only happens sometimes, and it depends on other activity within Zebra, and on the machine it's running on.

Solution

  • Return an error when an upgrade is cancelled
  • When an upgrade is cancelled, skip the upgrade check, and marking the database as upgraded
  • Move an upgrade log to the correct location

Review

This fix needs to be part of the 1.2.0 release, it is urgent.

Reviewer Checklist

  • Will the PR name make sense to users?
    • Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
  • Are the PR labels correct?
  • Does the code do what the ticket and PR says?
    • Does it change concurrent code, unsafe code, or consensus rules?
  • How do you know it works? Does it have tests?

@teor2345 teor2345 added C-bug Category: This is a bug P-Critical 🚑 I-panic Zebra panics with an internal error message A-state Area: State / database changes A-concurrency Area: Async code, needs extra work to make it work properly. labels Aug 31, 2023
@teor2345 teor2345 requested a review from a team as a code owner August 31, 2023 22:48
@teor2345 teor2345 requested review from upbqdn and removed request for a team August 31, 2023 22:48
@teor2345 teor2345 self-assigned this Aug 31, 2023
Copy link
Contributor

@oxarbitrage oxarbitrage left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@teor2345 teor2345 mentioned this pull request Sep 1, 2023
40 tasks
@mergify mergify bot merged commit 978b163 into main Sep 1, 2023
@mergify mergify bot deleted the fix-upgrade-cancel-panic branch September 1, 2023 01:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-concurrency Area: Async code, needs extra work to make it work properly. A-state Area: State / database changes C-bug Category: This is a bug I-panic Zebra panics with an internal error message
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants