--beacon-db-pruning: Prune state diff buckets when the --enable-state-diff flag is used. - #17289
--beacon-db-pruning: Prune state diff buckets when the --enable-state-diff flag is used.#17289nalepae wants to merge 8 commits into
--beacon-db-pruning: Prune state diff buckets when the --enable-state-diff flag is used.#17289Conversation
d84c89e to
12c0cb9
Compare
Before this commit, pruning occured at the middle of the epoch, and pruned data up to the middle of the epoch. After this commit, pruning still occurs at the middle of the epoch, but it prunes data up to the start of the epoch.
Before this commit:
The state-diff tree was never pruned. A node running with `--beacon-db-pruning`
and `--enable-state-diff` dropped its blocks once they aged past the retention
period, but kept every state it had ever stored, all the way back to its
checkpoint sync.
After this commit:
Rebuilding a state needs, for each level of the tree, the entry stored at the
last boundary of that level at or before that state. For every state at or
after the cutoff, those entries are either at or after the cutoff, or, for each
level, the single one stored at the last boundary before it. Only those are
kept below the cutoff, whatever the retention period is.
With `C` the cutoff slot, and `k0`, `k1` and `k2` the last entry of each level
at or before it. Each level spans twice the one below it, and an entry is
stored at the coarsest level it belongs to. `.` is deleted, `#` is kept, and
`+` is a block kept although older than `C`:
k0 k1 k2 C
slot ------------------------------------------------------------------
epochs | | | | | | | | | | | | | | | | |
level 0 . # #
level 1 . #
level 2 . . . #
blocks .........................................................++++#####
states .........................................................#########
The kept entries support each other: the spans divide each other, so the anchor
of a kept entry is the kept entry of the level above it.
The slots between `C` and the next stored entry have no state of their own, and
are recomputed by replaying blocks from `k2`. Keeping those blocks is the job
of the previous commit.
Deletion happens in batches, spread over as many pruning runs as needed. The
offset moves to `k0` only with the last batch, together with the deletion of
the previous anchor snapshot, so an interrupted run leaves a tree that reads
exactly as before, with unreachable entries that the next run deletes.
12c0cb9 to
2aaa9e2
Compare
| for _, exponent := range exponents { | ||
| if exponent < flags.MinStateDiffExponent || exponent >= 64 { | ||
| return nil, fmt.Errorf("state diff exponent %d out of range for uint64", exponent) | ||
| } |
There was a problem hiding this comment.
you don't really need all these checks. they will get validated upon node startup. (also the len check above)
There was a problem hiding this comment.
the slot<offset is also duplicate since the caller checks it. but it doesn't matter much.
| // Metadata keys are shorter than a tree key, and are never pruned. | ||
| if len(key) < stateDiffTreeKeyLength { | ||
| continue | ||
| } |
There was a problem hiding this comment.
this might be dangerous if we add a metdata key longer than a node key
| continue | ||
| } | ||
|
|
||
| if !bytes.Equal(entryPrefix, key[:stateDiffTreeKeyLength]) { |
There was a problem hiding this comment.
this confused me. maybe add a comment about why we're doing this mentioning the key_s/v/b keys for each node.
| if entries == maxEntries { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
I'd put this check in the for loop condition. (key != nil && entries <= maxEntries;). might be cleaner, but doesn't matter.
|
|
||
| if len(keys) > 0 { | ||
| if err := s.deleteStateDiffKeys(keys); err != nil { | ||
| return 0, fmt.Errorf("delete state diff keys: %w", err) |
There was a problem hiding this comment.
we should use errors.Wrap(). also in other places, I won't comment on them so it's not spammy.
There was a problem hiding this comment.
On the contrary, we should probably stop using errors.Wrap at all.
errors.Wrap depends on an external package that moved into maintenance mode several years ago.
This packages was created (and used by Prysm) because before 1.13, go had no standard way to wrap errors.
Since, 1.13, it is possible to wrap errors without depending on any external library, which is %w way with fmt.Errorf.
There was a problem hiding this comment.
huh. I remember Kasey mentioning that we should use errors.Wrap.
nevermind then.
| if ctx.Err() != nil { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
should we return the error here? because imagine the context gets cancelled before any key is found, if we don't return an error, the caller will think that we are finished with the pruning (cause 0 was returned), and it will wrongfully reset the anchor which might mess up the tree, or on later restarts complain about corrupted db.
| for level := range c.anchors { | ||
| c.anchors[level] = nil | ||
| } |
There was a problem hiding this comment.
instead of this you should call c.clearAnchors(). it will do this and also reset their metric gauges.
There was a problem hiding this comment.
It was not possible to apply the exact suggestion because the lock is already taken, but implemented that instead: 50e2b0d
What type of PR is this?
Feature
What does this PR do? Why is it needed?
Before this PR, when both using
--beacon-db-pruningand--enable-state-diff, the state (diffs) are actually not pruned, which kind of defeat the purpose of pruning.(When using
--beacon-db-pruningwithout--enable-state-diff, states are pruned.)This pull request implements the state diff buckets pruning.
To ensure every state in the retention window is recomputable, we prune the state diffs according to the following pattern:
With
Cthe cutoff slot (first slot for which the state has to be re-computable), andk0,k1andk2the last entry of each level at or before it..is deleted,#is kept, and+is a block kept although older thanCRebuilding a state needs, for each level, the entry stored at the last boundary of that level at or before that state. For every state at or after
C, those are either entries at or afterC, or, for each level, the single entry stored at the last boundary before it. So onlyk0,k1andk2are kept below the cutoff, and everything else there is deleted.The slots between
Cand the next stored entry have no state of their own, and are recomputed by replaying blocks fromk2. This is what the+blocks are for, and their only purpose: they are older than the retention period, and kept so that the states betweenCand the next entry stay recomputable.Note
With the default
--state-diff-exponents, the finest level spans 32 slots, soCis a boundary itself,k2isCand no extra block is kept.Other notes for review
Please read commit by commit, with commit messages.
Acknowledgements