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.

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

core, eth, node: break rawdb -> {leveldb, pebble} dependency #30689

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

karalabe
Copy link
Member

@karalabe karalabe commented Oct 28, 2024

The rawdb package is a generic package that can operate on arbitrary ethdb.KeyValueStore types. It's purpose was always to be data store agnostic. At some point however we sprinkled a few LevelDB and Pebble references in there, making it all of a sudden depend on both of them.

Looking through the code however, that dependency was mostly moot:

  • One complex piece of code was multiplexing on the content of our datadir and opening either leveldb or pebble, based on its content. Whilst this is indeed useful code, this was only ever used in the node package, which is kind of the nobrainer good choice for this logic either way. I moved it there.
  • Some blockchain tests used rawdb.Open as a shorthand, but these call sites are so simple, that the inlining of the actual call content is just 1 line longer than what Open was originally, however, by inlining, the dependency on pebble moves into the code blockchain tests, out of rawdb. Wonderful.
  • There were some ancient log filtering benchmarks that used leveldb... which were disabled because they modified a live mainnet database that they needed to be provided. I deleted these benchmarks are pointless really.

With this PR:

$ go list -deps ./core/rawdb | grep go-ethereum

github.com/ethereum/go-ethereum/common/hexutil
github.com/ethereum/go-ethereum/common
github.com/ethereum/go-ethereum/common/mclock
github.com/ethereum/go-ethereum/common/prque
github.com/ethereum/go-ethereum/common/math
github.com/ethereum/go-ethereum/crypto/secp256k1
github.com/ethereum/go-ethereum/rlp/internal/rlpstruct
github.com/ethereum/go-ethereum/rlp
github.com/ethereum/go-ethereum/crypto
github.com/ethereum/go-ethereum/crypto/kzg4844
github.com/ethereum/go-ethereum/log
github.com/ethereum/go-ethereum/params/forks
github.com/ethereum/go-ethereum/params
github.com/ethereum/go-ethereum/core/types
github.com/ethereum/go-ethereum/consensus/misc/eip4844
github.com/ethereum/go-ethereum/ethdb
github.com/ethereum/go-ethereum/ethdb/memorydb
github.com/ethereum/go-ethereum/metrics
github.com/ethereum/go-ethereum/core/rawdb

$ go list -deps ./core/rawdb | wc -l
     213

Without this PR:

$ go list -deps ./core/rawdb | grep go-ethereum

github.com/ethereum/go-ethereum/common/hexutil
github.com/ethereum/go-ethereum/common
github.com/ethereum/go-ethereum/common/mclock
github.com/ethereum/go-ethereum/common/prque
github.com/ethereum/go-ethereum/common/math
github.com/ethereum/go-ethereum/crypto/secp256k1
github.com/ethereum/go-ethereum/rlp/internal/rlpstruct
github.com/ethereum/go-ethereum/rlp
github.com/ethereum/go-ethereum/crypto
github.com/ethereum/go-ethereum/crypto/kzg4844
github.com/ethereum/go-ethereum/log
github.com/ethereum/go-ethereum/params/forks
github.com/ethereum/go-ethereum/params
github.com/ethereum/go-ethereum/core/types
github.com/ethereum/go-ethereum/consensus/misc/eip4844
github.com/ethereum/go-ethereum/ethdb
github.com/ethereum/go-ethereum/metrics
github.com/ethereum/go-ethereum/ethdb/leveldb         <---------
github.com/ethereum/go-ethereum/ethdb/memorydb
github.com/ethereum/go-ethereum/ethdb/pebble          <---------
github.com/ethereum/go-ethereum/core/rawdb

$ go list -deps ./core/rawdb | wc -l
     427

FWIW, I left the memorydb dependency in since it was used quite heavily in tests as a shorthand and it's a tiny dep-free package anyway, so no point in making it harder for us.

Copy link
Member

@rjl493456442 rjl493456442 left a comment

Choose a reason for hiding this comment

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

sgtm

@karalabe karalabe added this to the 1.14.12 milestone Oct 29, 2024
@karalabe karalabe merged commit 7180d26 into ethereum:master Oct 29, 2024
3 checks passed
zfy0701 pushed a commit to sentioxyz/go-ethereum that referenced this pull request Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants