chore(trie): Implement clear trie changeset methods#18730
chore(trie): Implement clear trie changeset methods#18730mediocregopher merged 13 commits into18460-trie-changesetsfrom
Conversation
These methods will be used in various places when it's necessary to delete changeset data. This includes pruning, unwinding, and also when populating the data during pipeline sync in certain cases. A new type BlockNumberHashedAddressRange is implemented for convenience.
mediocregopher
requested review from
joshieDo,
rakita,
rkrasiuk and
shekhirin
as code owners
|
|
||
| fn clear_trie_changesets_range( | ||
| &self, | ||
| range: impl RangeBounds<BlockNumber>, |
There was a problem hiding this comment.
something we might want to consider immediately now is that if we move trie changesets to static files, we would not be able to provide a range, it will just be "truncate N from the end". Maybe we should change it to be a truncate?
There was a problem hiding this comment.
Changed to clear_trie_changesets_from.
I'm not sure it's going to make sense to have these in static files tho, at least in their current state. Without an accompanying index the data isn't very useful beyond reverting a relatively small amount of blocks, so storing in static files would have us keep way more than we could use for no benefit.
There was a problem hiding this comment.
yeah, the indices would live in the DB same as for account/storage history, but the changesets themselves in static files. This can improve storage and perf of querying actual trie changesets.
Rjected
left a comment
Rjected
left a comment
There was a problem hiding this comment.
unless we use the BlockNumberHashedAddressRange somewhere else, thinking we can make this simpler
| let range: BlockNumberHashedAddressRange = (from..).into(); | ||
| let mut cursor = tx.cursor_dup_write::<tables::StoragesTrieChangeSets>()?; | ||
| let mut walker = cursor.walk_range(range)?; |
There was a problem hiding this comment.
so this is the only place we really use BlockNumberHashedAddressRange? maybe we could just remove the struct and do
let range: RangeFrom<BlockNumberHashedAddress> = (from, B256::ZERO).into()..;
There was a problem hiding this comment.
Good point, the range type made more sense when the clear method actually accepted a range, removed.
…diocregopher/clear-trie-changesets
mattsse
added
the
C-perf
mattsse
left a comment
mattsse
left a comment
There was a problem hiding this comment.
lgtm, one pedantic doc suggestion
| } | ||
|
|
||
| { | ||
| let range: RangeFrom<BlockNumberHashedAddress> = (from, B256::ZERO).into()..; |
There was a problem hiding this comment.
I see, this would be the first (num, hash) pair for BlockNumberHashedAddress
4 participants
These methods will be used in various places when it's necessary to delete changeset data. This includes pruning, unwinding, and also when populating the data during pipeline sync in certain cases.
A new type BlockNumberHashedAddressRange is implemented for convenience.