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

backport: pd: refuse to bootstrap services if the app is not ready (#4436) #4450

Merged
merged 1 commit into from
May 23, 2024

Conversation

conorsch
Copy link
Contributor

Describe your changes

Backports changes from #4436, towards #4448.

Checklist before requesting a review

  • If this code contains consensus-breaking changes, I have added the "consensus-breaking" label. Otherwise, I declare my belief that there are not consensus-breaking changes, for the following reason:

This code was carefully resolved to avoid breaking changes, so that we can ship a chain migration safely (#4402).

Previously, the halting logic was structured such that full nodes would
partially crash two of their four ABCI services (`Consensus` and
`Mempool`); relying on future CometBFT consensus requests to crash the
node.

This PR adds an `App::is_ready` method that callers (pd) SHOULD call in
order to make sure that the application is ready, so that they can avoid
to spin up any services unless an override flag (`--force`) is
specified.

Fix #4432. Fix #4443.

- [x] If this code contains consensus-breaking changes, I have added the
"consensus-breaking" label. Otherwise, I declare my belief that there
are not consensus-breaking changes, for the following reason:

  > Full node mechanical refactor

(cherry picked from commit 2c9c3f3)
@conorsch conorsch force-pushed the pd-halt-fix-for-0.75.x branch from 51e8d20 to 4f33f1a Compare May 23, 2024 19:32
Copy link
Member

@erwanor erwanor left a comment

Choose a reason for hiding this comment

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

This looks good to me. Tested by syncing against the testnet, and on a private devnet.

@conorsch conorsch marked this pull request as ready for review May 23, 2024 21:04
@conorsch
Copy link
Contributor Author

There's a single CI failure on this PR, the docs-lint job is failing: https://github.com/penumbra-zone/penumbra/actions/runs/9213808726/job/25348533450?pr=4450 That's because we didn't backport the CI fix in #4415 to this release branch. I'm not going to bother doing that, because docs are built from main anyway, and I want to get this out the door as 0.75.1. Proceeding with merge.

@conorsch conorsch merged commit d0b0291 into release/v0.75.x May 23, 2024
13 of 14 checks passed
@conorsch conorsch deleted the pd-halt-fix-for-0.75.x branch May 23, 2024 21:06
@conorsch conorsch changed the title pd: refuse to bootstrap services if the app is not ready (#4436) backport: pd: refuse to bootstrap services if the app is not ready (#4436) May 23, 2024
@conorsch conorsch mentioned this pull request May 23, 2024
15 tasks
@erwanor erwanor added this to the Sprint 7 milestone May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants