-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Blob filesystem: prune blobs #13147
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
Blob filesystem: prune blobs #13147
Conversation
* Fix block proposals in the REST validator client * fix graffiti test * return empty graffiti * fallback to old endpoints * logs * handle 404 * everything passes * review from James * log undecoded value * test fixes and additions --------- Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com>
Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com>
* rearrange deadline * naming
beacon-chain/db/filesystem/blob.go
Outdated
// Prune prunes blobs in the base directory based on the retention epoch. | ||
// It deletes blobs older than currentEpoch - (retentionEpoch+bufferEpochs). | ||
// This is so that we keep a slight buffer and blobs are deleted after n+2 epochs. | ||
func (bs *BlobStorage) Prune(cliCtx *cli.Context, currentSlot primitives.Slot) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think that having the cli.Context as an argument to the blob storage is a code smell.
If you don't want the caller to figure out the retention slot, can you initialize BlobStorage with the flag value?
beacon-chain/db/filesystem/blob.go
Outdated
if err != nil { | ||
return nil, err | ||
} | ||
return &BlobStorage{fs: fs, retentionSlot: retentionSlot}, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't you store the flag value instead of computing the retention slot? The retention slot will be something like current_slot - slots_to_keep
which changes over time as current_slot advances.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, should be fixed now
Co-authored-by: Preston Van Loon <[email protected]>
What type of PR is this?
Feature
What does this PR do? Why is it needed?
To best manage and optimize the storage of data we need to be able to efficiently prune the old and unnecessary data. All data beyond the default slot retention period will be removed.
The actual retention policy has been implemented with a 2 epoch buffer such that the blobs are not readable after n epochs and they are pruned / deleted after n+2 epochs.
Which issues(s) does this PR fix?
Fixes #13108
Other notes for review