-
Notifications
You must be signed in to change notification settings - Fork 20.1k
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
all: implement path-based state scheme #25963
Conversation
12a5898
to
58abfe0
Compare
d19122a
to
6d4b635
Compare
fe9e710
to
ef9504b
Compare
5747cf5
to
1ee650b
Compare
Benchmark results on mainnet Overall performance:Finish mainnet full sync in approximately 10 days, which is 11 hours ahead of master branch. IOWait: Master branch has a high iowait. Memory usage: The memory usage has no big difference between these two. Allocation: PBSS has a higher allocation rate DatabaseOverall:
Compaction overhead: The compaction overhead of master is obviously larger |
Wow, great work Gary!!!
|
|
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka +------------------------------+---------------------+------------+------------+
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | DATABASE | CATEGORY | SIZE | ITEMS |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka +------------------------------+---------------------+------------+------------+
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Key-Value store | Headers | 2.41 MiB | 4150 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Key-Value store | Bodies | 478.43 MiB | 4150 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Key-Value store | Receipt lists | 267.74 MiB | 4150 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Key-Value store | Difficulties | 214.79 KiB | 4150 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Key-Value store | Block number->hash | 169.39 KiB | 4130 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Key-Value store | Block hash->number | 683.85 MiB | 17489356 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Key-Value store | Transaction index | 12.24 GiB | 362206976 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Key-Value store | Bloombit index | 3.38 GiB | 8747182 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Key-Value store | Contract codes | 6.06 GiB | 947830 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Key-Value store | Account trie nodes | 32.99 GiB | 283918605 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Key-Value store | Storage trie nodes | 135.02 GiB | 1350456903 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Key-Value store | Legacy trie nodes | 0.00 B | 0 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Key-Value store | State lookups | 154.75 KiB | 3865 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Key-Value store | Trie preimages | 0.00 B | 0 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Key-Value store | Account snapshot | 9.70 GiB | 210877140 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Key-Value store | Storage snapshot | 71.38 GiB | 997110424 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Key-Value store | Beacon sync headers | 590.00 B | 1 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Key-Value store | Clique snapshots | 0.00 B | 0 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Key-Value store | Singleton metadata | 232.61 MiB | 18 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Light client | CHT trie nodes | 0.00 B | 0 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Light client | Bloom trie nodes | 0.00 B | 0 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Ancient store (Chain) | Headers | 7.96 GiB | 17485207 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Ancient store (Chain) | Hashes | 633.66 MiB | 17485207 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Ancient store (Chain) | Bodies | 343.17 GiB | 17485207 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Ancient store (Chain) | Receipts | 148.79 GiB | 17485207 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Ancient store (Chain) | Diffs | 276.48 MiB | 17485207 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Ancient store (Statehistory) | History.Meta | 267.78 KiB | 3862 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Ancient store (Statehistory) | Account.Index | 47.18 MiB | 3862 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Ancient store (Statehistory) | Storage.Index | 49.55 MiB | 3862 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Ancient store (Statehistory) | Account.Data | 35.23 MiB | 3862 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | Ancient store (Statehistory) | Storage.Data | 12.01 MiB | 3862 |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka +------------------------------+---------------------+------------+------------+
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka | TOTAL | 773.36 GIB | |
Jun 16 10:43:30 bench05.ethdevops.io tender_ishizaka +------------------------------+---------------------+------------+------------+ |
The spec of benchmark machine
And it has 64GB memory. Just for reference. |
This PR moves some trie-related db accessor methods to a different file, and also removes the schema type. Instead of the schema type, a string is used to distinguish between hashbased/pathbased db accessors. This also moves some code from trie package to rawdb package. This PR is intended to be a no-functionality-change prep PR for #25963 . --------- Co-authored-by: Gary Rong <garyrong0905@gmail.com>
33c7580
to
72d4d6d
Compare
return err | ||
} | ||
// Clean up all state histories in freezer. Theoretically | ||
// all root->id mappings should be removed as well. Since |
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.
If we don't remove the root->id mappings, won't that potentially cause Recoverable
to return true for non existing states?
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.
Ah, I guess the check against the bottom layer ensure that only "overwritten" states can be recovered, but dangling junk cannot. Is my reasoning correct here?
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.
If we don't remove the root->id mappings, won't that potentially cause Recoverable to return true for non existing states?
Nope, Recoverable check contains two steps:
- Ensure the target is known by checking the relevant state id
- Ensure all the histories from id+1 until disk layer are all present
If we leave root->id mappings in disk, the non-existing state will be "known", but it lacks of corresponding state history, so it's still Unrecoverable.
// Ensure the requested state is a canonical state and all state | ||
// histories in range [id+1, disklayer.ID] are present and complete. | ||
parent := root | ||
return checkHistories(db.freezer, *id+1, dl.stateID()-*id, func(m *meta) 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.
Apart from being a sanity check, this should never ever ever fail, right?
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.
yep, normally if the state is known, then the corresponding state histories are supposed to be present as well.
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.
The only exception is, if one of them histories is incomplete(has a large self-destruction inside, and it's can be handled because of memory limitation), then this check will return false(the target is unrecoverable).
For this situation, the only way to rewind to a past block is to "resync". But we can make the threshold of self-destruction configurable(512 MB by default), so that big machine can still cross these big deletions.
All in all, it's not expected, but still handleable.
} | ||
root, result := tr.Commit(false) | ||
if root != prevRoot { | ||
return nil, fmt.Errorf("failed to revert state, want %#x, got %#x", prevRoot, root) |
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 guess this clause will implicitly catch the issue if there are incomplete accounts?
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.
Exactly, not only incomplete accounts, all invalid state history should be captured by this check.
The trie(account + storage tries) is supposed to be reverted to previous state, otherwise, bail out.
// storage trie nodes, 'owner' is the hash of the account address that containing the | ||
// storage. | ||
// | ||
// TODO(rjl493456442): remove the 'hash' parameter, it's redundant in PBSS. |
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.
Is the hash truly redundant, or do we use it to double check that the retrieved item has the correct hash? Perhaps if the latter, we can keep it indefinitely?
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 plan to remove the hash in the pbss later.
For MPT, whenever we load a trie node from pbss(with owner, path), we can verify the hash outside of db reader, in this way we still meaningfully check the hash, and it's the same as now.
For Verkle, we want to drop the node hash from parent, there is no hash for verification at all, but the benefit is it's performant.
But anyway, it should be done in the following PR and I will just keep it now.
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.
LGTM (as much as I can review such a large PR :P)
* all: implement path-based state scheme * all: edits from review * core/rawdb, trie/triedb/pathdb: review changes * core, light, trie, eth, tests: reimplement pbss history * core, trie/triedb/pathdb: track block number in state history * trie/triedb/pathdb: add history documentation * core, trie/triedb/pathdb: address comments from Peter's review Important changes to list: - Cache trie nodes by path in clean cache - Remove root->id mappings when history is truncated * trie/triedb/pathdb: fallback to disk if unexpect node in clean cache * core/rawdb: fix tests * trie/triedb/pathdb: rename metrics, change clean cache key * trie/triedb: manage the clean cache inside of disk layer * trie/triedb/pathdb: move journal function * trie/triedb/path: fix tests * trie/triedb/pathdb: fix journal * trie/triedb/pathdb: fix history * trie/triedb/pathdb: try to fix tests on windows * core, trie: address comments * trie/triedb/pathdb: fix test issues --------- Co-authored-by: Felix Lange <fjl@twurst.com> Co-authored-by: Martin Holst Swende <martin@swende.se>
This reverts commit b7ea2f2.
This reverts commit b7ea2f2.
This PR adds the path-based implementation, but it's not used yet. The main intention for this PR is reviewers can review the main part but not worrying breaking the live code.