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

blockdb: bound max deleted blocks per blockdb sync #5910

Merged
merged 3 commits into from Jan 23, 2024

Conversation

algorandskiy
Copy link
Contributor

@algorandskiy algorandskiy commented Jan 19, 2024

Summary

If a node switches from Archival to non-archival then the blockdb cleanup can take a long time blocking the blockdb in a write tx. This PR addresses this by scoping max blocks deleted per sync op.

Earliest                       MinToSave         Latest

   |                               |                 |
   +-------------------------------+-----------------+
   ^            ^                  ^                 ^
   |            |                  +-----------------+
   +------------+
   | maxDeletionBatchSize          ^     MaxBlockHistoryLookback
   +-------------------------------+

              Deletion Range (Old)

Test Plan

Added a unit test.
Additionally fixed a data race seen in TestVotersReloadFromDiskAfterOneStateProofCommitted.

ledger/blockqueue.go Outdated Show resolved Hide resolved
ledger/blockqueue.go Outdated Show resolved Hide resolved
@algorandskiy algorandskiy marked this pull request as ready for review January 19, 2024 23:03
Copy link

codecov bot commented Jan 19, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (877090b) 55.14% compared to head (0cb72c6) 55.97%.

Files Patch % Lines
ledger/blockqueue.go 80.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5910      +/-   ##
==========================================
+ Coverage   55.14%   55.97%   +0.83%     
==========================================
  Files         478      478              
  Lines       67602    67612      +10     
==========================================
+ Hits        37280    37848     +568     
+ Misses      27757    27201     -556     
+ Partials     2565     2563       -2     

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

@algorandskiy algorandskiy changed the title WIP: blockdb: bound max deleted blocks per blockdb sync blockdb: bound max deleted blocks per blockdb sync Jan 22, 2024
@onetechnical
Copy link
Contributor

From my testing, this looks good. algod remains available, and it sheds 10,000 rounds at a time, though it takes about 26 seconds per batch. This means that for going archival -> non-archival it would take about a day to completely cut through 35 million rounds. I think this is acceptable, since it remains responsive in the mean time.

@jannotti
Copy link
Contributor

it takes about 26 seconds per batch. This means that for going archival -> non-archival it would take about a day to completely cut through 35 million rounds. I think this is acceptable, since it remains responsive in the mean time.

I think this means that since a sync takes 26 seconds or so, we will only be calling sync every 8 rounds or so. If that's the case, I don't see any point in even going as high as 10,000. If we do 1,000 I guess we might be syncing fast enough to sync almost every round. And 8 rounds or so of 1,000 deletions is nearly as good as doing 10,000 rounds every 26 seconds. It'll still clean up in about a day.

Keep in mind, I may be misunderstanding how sync works. I had thought we wrote a block every round.

@algorandskiy
Copy link
Contributor Author

My intuition is bigger batches should take a bit less overall time than smaller batches because of the index update. Not sure tho if there any optimizations in sqlite for sequential removal from indices.

@gmalouf gmalouf merged commit 3490731 into algorand:master Jan 23, 2024
20 checks passed
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

4 participants