BenchmarkMedium panicking on memdb #908
Labels
Milestone
Comments
ajnavarro
added
🐞 bug
thehowl
added a commit
that referenced
this issue
Mar 1, 2024
Changes mostly involve shuffling code around. The `db` package now has its basic types and helper functions in `pkg/db`, and then sub-packages for each DB which is actually used. This allows a package using it to only import the database code it needs. The end goal I wanted was to remove the transitive dependency of `cmd/gno` on `goleveldb`. Turns out this is more complicated than I thought. In any case, I already did it in another branch; will make a second PR shortly after this one. Notes for reviewing: - `pkg/db` has also the helper packages `_all` and `_tags`. - `_all` imports all the databases while respecting the `cgo` build tag.[^1] (This is automatically set by the go compiler depending on whether cgo is enabled). - The `_tags` package, instead, imports the databases specified by the build tags. I meant this for end-user binaries, like gno.land eventually (currently it only supports GoLevelDB), so a user can compile with the given build tags. - I removed `badger` and `gorocksdb`. They were so important that they didn't compile and nobody noticed. - `badger` probably makes sense to be re-added and tried out eventually, but it's outside of the scope of this PR. - `gorocksdb` is unmaintained and will not compile with the latest version of RocksDB. In fact, we already had a DB implementation for [grocksdb](https://github.com/linxGnu/grocksdb), the maintained fork. I fixed some code there too to actually make it compile. - Removing the two dependencies made our `go.mods` a bunch lighter. :tada: - Some changes also involve the CI. - I removed splitting the build for each database being tested, as the tests on the package run quickly anyway so there is no real need to split them. - I also severely downgraded `grocksdb` so that its version is compatible with the version of RocksDB present on ubuntu 22.04's repositories. There could be a variety of different changes to the CI to allow building on a more recent version of the database; but after having seen that the compilation of RocksDB on a beefy machine with `make -j4` is already beyond 5 mintues, I decided we don't need a slow CI for a feature literally no-one uses, so here we are. - This PR also fixes the benchmarks in `tm2/pkg/iavl/benchmarks` (fixes #908). [The fix itself is stupid.](87ea39d#diff-112673453ecf482fb6bfd8004ebc11eff7536b812cc48bd6dff3328767550908R227-R229) [^1]: A while ago I was upset that my go compilation was taking long and then found out that it was using cgo for no apparent reason -- it's actually because a few places in the standard library will by default use cgo unless it's disabled. For this reason I actually set `CGO_ENABLED=0` in my `.profile`. --------- Co-authored-by: Antonio Navarro <antnavper@gmail.com>
leohhhn
pushed a commit
to leohhhn/gno
that referenced
this issue
Mar 6, 2024
Changes mostly involve shuffling code around. The `db` package now has its basic types and helper functions in `pkg/db`, and then sub-packages for each DB which is actually used. This allows a package using it to only import the database code it needs. The end goal I wanted was to remove the transitive dependency of `cmd/gno` on `goleveldb`. Turns out this is more complicated than I thought. In any case, I already did it in another branch; will make a second PR shortly after this one. Notes for reviewing: - `pkg/db` has also the helper packages `_all` and `_tags`. - `_all` imports all the databases while respecting the `cgo` build tag.[^1] (This is automatically set by the go compiler depending on whether cgo is enabled). - The `_tags` package, instead, imports the databases specified by the build tags. I meant this for end-user binaries, like gno.land eventually (currently it only supports GoLevelDB), so a user can compile with the given build tags. - I removed `badger` and `gorocksdb`. They were so important that they didn't compile and nobody noticed. - `badger` probably makes sense to be re-added and tried out eventually, but it's outside of the scope of this PR. - `gorocksdb` is unmaintained and will not compile with the latest version of RocksDB. In fact, we already had a DB implementation for [grocksdb](https://github.com/linxGnu/grocksdb), the maintained fork. I fixed some code there too to actually make it compile. - Removing the two dependencies made our `go.mods` a bunch lighter. :tada: - Some changes also involve the CI. - I removed splitting the build for each database being tested, as the tests on the package run quickly anyway so there is no real need to split them. - I also severely downgraded `grocksdb` so that its version is compatible with the version of RocksDB present on ubuntu 22.04's repositories. There could be a variety of different changes to the CI to allow building on a more recent version of the database; but after having seen that the compilation of RocksDB on a beefy machine with `make -j4` is already beyond 5 mintues, I decided we don't need a slow CI for a feature literally no-one uses, so here we are. - This PR also fixes the benchmarks in `tm2/pkg/iavl/benchmarks` (fixes gnolang#908). [The fix itself is stupid.](gnolang@87ea39d#diff-112673453ecf482fb6bfd8004ebc11eff7536b812cc48bd6dff3328767550908R227-R229) [^1]: A while ago I was upset that my go compilation was taking long and then found out that it was using cgo for no apparent reason -- it's actually because a few places in the standard library will by default use cgo unless it's disabled. For this reason I actually set `CGO_ENABLED=0` in my `.profile`. --------- Co-authored-by: Antonio Navarro <antnavper@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
BenchmarkMedium panicking on memdb
Description
When executing benchmark
BenchmarkMediumit panics with a nil pointer exception.Steps to reproduce
BenchmarkMediumLogs
The text was updated successfully, but these errors were encountered: