Skip to content
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.

Sign up for GitHub

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

Jump to bottom

maindb v4 #868

Open
wants to merge 42 commits into
base: feat/db
Choose a base branch
from
Open

maindb v4 #868

wants to merge 42 commits into from

Conversation

libotony
Copy link
Member

@libotony libotony commented Oct 29, 2024 •
edited
Loading

Description

This PR upgrades maindb to v4. It greatly reduces the storage space occupied and significantly reduces the This PR upgrades maindb to v4. It greatly reduces the storage space occupied and significantly reduces the synchronization time.

  • For MPT
    • The encoding and storage format of nodes are comprehensively improved, reducing the storage space used to store the state trie by 30%. Moreover, the speed of encoding and decoding nodes has been greatly improved.
    • Introducing the concept of Version to identify nodes, which simplifies the implementation code.
    • Optimize the trie interface to make it easier to maintain.
    • Remove leafbank stuff.
    • Improve generation-based node cache to reduce the pressure of GC.
    • Improve node storage key encoding.
  • Optimize block indexing and storage format.

Check out commits for more detail.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • New and existing E2E tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have not added any vulnerable dependencies to my code

@libotony libotony requested a review from a team as a code owner October 29, 2024 08:20
@libotony libotony changed the title Tony/maindb v4 maindb v4 Oct 29, 2024
@libotony libotony marked this pull request as draft October 29, 2024 08:20
@libotony
Copy link
Member Author

This PR is a successor of PR #635.

@darrenvechain
Copy link
Member

@libotony could you sync the branch with feat/db so that the CI runs? I made that change in the GHA, which got merged into feat/db just now

@qianbin @libotony
cmd/thor: update instance dir to v4
a14e1d4
@qianbin @libotony
trie: implement varint-prefix coder
c763d81
@qianbin @libotony
deps: add github.com/qianbin/drlp
8244e7e
@qianbin @libotony
trie: implement appendHexToCompact & compactLen
cfda3d1
@qianbin @libotony
trie: temporarily remove merkle proof stuff
c3604d9
@qianbin @libotony
trie: many changes
9c9c4b5 
* disk usage reduced by 33% (force embedding shortnode)
* new encoding method for storing nodes
* optimize trie hashing
* versioning standalone nodes
* remove extended trie
* improve trie interface
* simplify NodeIterator, remove unused codes
@qianbin @libotony
trie: optimize full-node encoding/decoding
dc608ab
@qianbin @libotony
trie: tweak shortnode encoding
2c1d769
@qianbin @libotony
muxdb: move engine pkg
60476fb
@qianbin @libotony
trie: add Version() method for node interface
e1060bb
@qianbin @libotony
muxdb: refactor due to trie updates and:
32fc127 
* remove leafbank stuff
* simplify muxdb.Trie implementation
* improve root node cache using ttl eviction
* add leaf key filter
@qianbin @libotony
chain: a lot of changes
8c1d747 
* improve block content storage scheme
* remove steady block tracking
* remove tx & receipt cache
@qianbin @libotony
state: changes due to update of trie
4f65e60
@qianbin @libotony
lowrlp: remove this pkg
ea6f347
@qianbin @libotony
txpool: changes due to underlying pkg update
fa7f182
@qianbin @libotony
genesis: changes due to underlying pkg update
c997523
@qianbin @libotony
consensus: changes due to underlying pkg update
58f4696
@qianbin @libotony
builtin: changes due to underlying pkg update
b379548
@qianbin @libotony
runtime: changes due to underlying pkg update
ca14559
@qianbin @libotony
api: changes due to underlying pkg update
47641c6
@qianbin @libotony
cmd/thor/pruner: rename pkg optimizer to pruner
81d037d
@qianbin @libotony
cmd/thor: changes due to underlying pkg update
462d027
@qianbin @libotony
muxdb: abandon leaf filter
d13612d
@libotony
Copy link
Member Author

@libotony could you sync the branch with feat/db so that the CI runs? I made that change in the GHA, which got merged into feat/db just now

Done.

@libotony libotony marked this pull request as ready for review October 29, 2024 08:55
leszek-vechain
leszek-vechain reviewed Oct 30, 2024
View reviewed changes
muxdb/cache_test.go Outdated Show resolved Hide resolved
@libotony libotony closed this Nov 1, 2024
@libotony libotony reopened this Nov 1, 2024
@libotony libotony closed this Nov 1, 2024
@libotony libotony reopened this Nov 1, 2024
@libotony
Copy link
Member Author

libotony commented Nov 1, 2024

Wired that tests won't run with this PR only, might be caused by the on-pull-request.yaml wasn't updated in the master branch, but after PR #871 was opened, CI was trigger for targeting master then with the same commit hash of this PR, the workflow was also linked here.

@darrenvechain
Copy link
Member

darrenvechain commented Nov 5, 2024 •
edited
Loading

@libotony looks like GET /block/{revision} is returning null in the transactions field, according to the e2e tests

-   "transactions": Any<Array>,
+   "transactions": null,
image

@libotony
Copy link
Member Author

libotony commented Nov 6, 2024

@libotony looks like GET /block/{revision} is returning null in the transactions field, according to the e2e tests

-   "transactions": Any<Array>,
+   "transactions": null,
image

Should be fixed.

darrenvechain
darrenvechain reviewed Nov 6, 2024
View reviewed changes
go.mod
@@ -24,6 +24,7 @@ require (
github.com/prometheus/client_model v0.5.0
github.com/prometheus/common v0.45.0
github.com/qianbin/directcache v0.9.7
github.com/qianbin/drlp v0.0.0-20240102101024-e0e02518b5f9
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems the dependency is quite small, should we just copy the files over to this repo?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wdyt？ @qianbin

darrenvechain
darrenvechain reviewed Nov 6, 2024
View reviewed changes
muxdb/cache.go
}

// AddNodeBlob adds encoded node blob into the cache.
func (c *cache) AddNodeBlob(keyBuf *[]byte, name string, path []byte, ver trie.Version, blob []byte, isCommitting bool) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like most of these methods are exported but not used outside the package. Maybe I'm wrong ? Probably doesn't matter too much anyhow

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Itself can be treated a standalone "class" to other ones, even it's inside the same package. IMHO, it's a good practice to define public/private functions even it's internal only.

@libotony libotony mentioned this pull request Nov 7, 2024
Open
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leszek-vechain leszek-vechain leszek-vechain left review comments

@darrenvechain darrenvechain darrenvechain approved these changes

@nwbrettski nwbrettski nwbrettski left review comments

Assignees
No one assigned
Labels
new feature resync-required
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@libotony @darrenvechain @nwbrettski @leszek-vechain @qianbin