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

v1.18: Purges all bank snapshots after fastboot (backport of #35350) #35379

Merged
merged 1 commit into from
Mar 1, 2024

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Feb 29, 2024

This is an automatic backport of pull request #35350 done by Mergify.


Mergify commands and options

More conditions and actions can be found in the documentation.

You can also trigger Mergify actions by commenting on this pull request:

  • @Mergifyio refresh will re-evaluate the rules
  • @Mergifyio rebase will rebase this PR on its base branch
  • @Mergifyio update will merge the base branch into this PR
  • @Mergifyio backport <destination> will backport this PR on <destination> branch

Additionally, on Mergify dashboard you can:

  • look at your merge queues
  • generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.com

@brooksprumo
Copy link
Contributor

Justification for backport:

Without this PR, a node can end up in a crash-boot loop that requires manual intervention. This is pretty bad, and potentially dangerous for the network as a whole. Also, in v1.18, fastboot is enabled by default, so this issue may get triggered more often than it is today on v1.17.

This PR safe, as the fix is to fall back to loading from a snapshot archive, which is safe (and very battle tested).

@jeffwashington jeffwashington self-requested a review February 29, 2024 19:50
Copy link
Contributor

@jeffwashington jeffwashington left a comment

Choose a reason for hiding this comment

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

lgtm. This is worth it to put it in 1.18. I am hitting this frequently with test nodes. It then often requires a manual snapshot load, which delays testing and requires manual intervention.

Copy link
Contributor

@apfitzge apfitzge left a comment

Choose a reason for hiding this comment

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

Straight-forward backport without conflict, the issue this PR addresses is being hit in 1.18 so deserves a backport imo.
I reviewed the initial master PR, so defering on explicit approval for someone from backport review squad

Copy link
Contributor

@steviez steviez left a comment

Choose a reason for hiding this comment

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

This PR safe, as the fix is to fall back to loading from a snapshot archive, which is safe (and very battle tested).

☝️

Copy link

codecov bot commented Feb 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.6%. Comparing base (06161dd) to head (06161dd).

❗ Current head 06161dd differs from pull request most recent head 15838d9. Consider uploading reports for the commit 15838d9 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##            v1.18   #35379   +/-   ##
=======================================
  Coverage    81.6%    81.6%           
=======================================
  Files         827      827           
  Lines      224237   224237           
=======================================
  Hits       183058   183058           
  Misses      41179    41179           

Copy link
Contributor

@willhickey willhickey left a comment

Choose a reason for hiding this comment

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

Approved for beta

@brooksprumo
Copy link
Contributor

@sakridge Can I get your review on this backport?

@brooksprumo brooksprumo merged commit 42c5125 into v1.18 Mar 1, 2024
35 checks passed
@brooksprumo brooksprumo deleted the mergify/bp/v1.18/pr-35350 branch March 1, 2024 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants