core, triedb/pathdb: final integration (snapshot integration pt 5) #30661
Conversation
rjl493456442
force-pushed
the
snapshot-integration-p5
branch
from
d249035 to
133900e
Compare
holiman
changed the title
rjl493456442
force-pushed
the
snapshot-integration-p5
branch
5 times, most recently
from
b879386 to
985a84e
Compare
| @@ -362,6 +430,7 @@ func (db *Database) Enable(root common.Hash) error { | |||
| // reset the persistent state id back to zero. | |||
| batch := db.diskdb.NewBatch() | |||
| rawdb.DeleteTrieJournal(batch) | |||
| rawdb.DeleteSnapshotRoot(batch) | |||
There was a problem hiding this comment.
Could you please explain why we need to delete the snapshot root here, I don't really understand
There was a problem hiding this comment.
This database entry indicates that the snapshot data on disk is associated with the specified state root.
The function Enable(root) means the trie node data on disk associates with the given state root, any leftover snapshot data should be purged and regenerated.
Deleting the SnapshotRoot entry signifies that all leftover storage data is considered obsolete.
| } | ||
| } | ||
| } | ||
| for addrHash, storages := range storageData { |
There was a problem hiding this comment.
I'm wondering whether it makes a difference in the speed of flushing if we sort the keys before trying to insert them, iirc some databases handle ordered key insertions better than random keys. This way we could also break on the first key that exceeds the genMarker
There was a problem hiding this comment.
The random insertion here won't significantly impact the performance, as the data receiver is a in-memory batch. But I would love to measure the performance difference later.
triedb/pathdb/generate.go
Outdated
| // into two parts. | ||
| func splitMarker(marker []byte) ([]byte, []byte) { | ||
| var accMarker []byte | ||
| if len(marker) > 0 { // []byte{} is the start, use nil for that |
There was a problem hiding this comment.
Do we need to check here that len(marker) >= HashLength?
There was a problem hiding this comment.
the generation marker could only be:
- []byte{}
- []byte{common.AddressLength}
- []byte{common.AddressLength + common.HashLength}
If the marker is corrupted, then the system could be very wrong and I don't mind panic here.
|
Took a first look now and it looks pretty good so far. I added a few questions/things I didn't understand while reviewing. The biggest changes (generator, generator_test, context) are modified copies from the snapshot package |
rjl493456442
force-pushed
the
snapshot-integration-p5
branch
from
985a84e to
9635f1d
Compare
Note: This PR is on hold until after the Prague fork release, because it changes the snapshot data layout in an incompatible way, and downgrades will become impossible.