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

Blocks: Bump min to save based on catchpoint support #5927

Merged
merged 5 commits into from Feb 5, 2024

Conversation

gmalouf
Copy link
Contributor

@gmalouf gmalouf commented Feb 2, 2024

Summary

Support minimum number of rounds saved that takes the being able to catchup from a recent catchpoint into account.

Test Plan

Added a new unit test that exercises the impact of enabling/disabling catchpoint support on ledger's minToSave calculation.

…atchup from a recent catchpoint into account.
Copy link

codecov bot commented Feb 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b5927a6) 56.00% compared to head (703a2e3) 55.95%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5927      +/-   ##
==========================================
- Coverage   56.00%   55.95%   -0.05%     
==========================================
  Files         478      478              
  Lines       67561    67569       +8     
==========================================
- Hits        37835    37807      -28     
- Misses      27174    27205      +31     
- Partials     2552     2557       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@jannotti jannotti left a comment

Choose a reason for hiding this comment

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

Seems ok to me, with a little less float and named returns.

I didn't realize that CatchpointInterval was a config value. Seems weird to me.

ledger/ledger.go Outdated Show resolved Hide resolved
@gmalouf gmalouf marked this pull request as ready for review February 2, 2024 19:04
ledger/ledger.go Outdated Show resolved Hide resolved
@gmalouf gmalouf requested a review from jannotti February 2, 2024 22:42
ledger/ledger.go Outdated Show resolved Hide resolved
@gmalouf gmalouf self-assigned this Feb 5, 2024
ledger/ledger_test.go Outdated Show resolved Hide resolved
ledger/ledger_test.go Outdated Show resolved Hide resolved
ledger/ledger.go Outdated Show resolved Hide resolved
@gmalouf gmalouf requested a review from jannotti February 5, 2024 20:31
@gmalouf gmalouf merged commit 1747aba into algorand:master Feb 5, 2024
18 checks passed
@gmalouf gmalouf deleted the catchpoint-interval-min-retention branch February 5, 2024 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants