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

Add support for block pruning #116

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add support for block pruning #116

wants to merge 2 commits into from

Conversation

lukechampine
Copy link
Member

Been a long time coming. 😅

The strategy here is quite naive, but I think it will be serviceable. Basically, when we apply a block N, we delete block N-P. P is therefore the "prune target," i.e. the maximum number of blocks you want to store.

In practice, this isn't exhaustive: it only deletes blocks from the best chain. It also won't dramatically shrink the size of an existing database. I think this is acceptable, because pruning is most important during the initial sync, and during the initial sync, you'll only be receiving blocks from one chain at a time. Also, we don't want to make pruning too easy; after all, we need a good percentage of nodes to be storing the full chain, so that others can sync to them.

I tested this out locally with a prune target of 1000, and after syncing 400,000 blocks, my consensus.db was around 18 GB. This is disappointing; it should be much smaller. With some investigation, I found that the Bolt database was only storing ~5 GB of data (most of which was the accumulator tree, which we can't prune until after v2). I think this is a combination of a) Bolt grows the DB capacity aggressively in response to writes, and b) Bolt never shrinks the DB capacity. So it's possible that we could reduce this number by tweaking our DB batching parameters. Alternatively, we could provide a tool that copies the DB to a new file. Not the most user-friendly, but again, I think I'm okay with that for now.

Depends on SiaFoundation/core#228

@@ -13,43 +13,32 @@ import (
)

type supplementedBlock struct {
Block types.Block
Header *types.BlockHeader
Copy link
Member

@n8maninger n8maninger Nov 11, 2024

Choose a reason for hiding this comment

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

I'd like to see a way to detect and trigger a resync for changes like this. Currently, we panic and require the user to manually delete the consensus database. That's really bad UX, particularly for users that are doing automatic updates. Simplest solution would probably be to try to decode the supplementedBlock when we open the database. If that fails: log an error, close, erase, and reopen.

Longer term, it would be nice to have an actual migration path like the other databases. However, that may be less important if we don't need to store any of this junk after the v2 require height.

Copy link
Member Author

Choose a reason for hiding this comment

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

agreed. In this case, though, compatibility is preserved: we always encode as v3, but we can decode either v2 or v3. That's possible because the Header field is optional and gets filled in lazily as needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

2 participants