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

Pebble #284

Closed
wants to merge 24 commits into from
Closed

Pebble #284

wants to merge 24 commits into from

Conversation

faddat
Copy link
Contributor

@faddat faddat commented Jul 29, 2022

This branch includes work from myself, @catShaark, and @baabeetaa. @baabeetaa solved the final issues in the pebble implementation for tm-db.

It's been running on Sifchain for about 15 hours.

We will soon have pebble snapshots for every chain that Notional validates available at:

https://github.com/notionla-labs/cosmosia

But we didn't just test this against Sif, that has led to failures in the past, so instead, we tested it millions, and millions of times, against both the tm-db test suite and iavl.

Like the other databases, pebble would perform faster if this pr:

were merged.

Like all the PR's here, this pr would pass tests if this pr:

were merged -- it fixes the master branch of this repository.

@faddat
Copy link
Contributor Author

faddat commented Jul 29, 2022

Passes tests because merged #284

BadgerDBBackend BackendType = "badgerdb"
// PebbleDBDBBackend represents pebble (uses github.com/cockroachdb/pebble)
// - EXPERIMENTAL
// - use pebble build tag (go build -tags pebbledb)
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this is pure go so no need for a build flag.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We tag all of them currently (badger, bolt, and pebble)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that if we don't tag, then we end up making chain binaries that use more than one db. This could (or might not) be desirable....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if we want to make it default, then no flag. But I think default is like maybe next month.

Cause it is 10-27x less intense on the disk, no joke.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

27x aint a joke, true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

eh, need to do more benchmarking. its.... wierd.

But:

  • near certain drives will last longer
  • near certain will be faster with cw
  • near certain will enable running on much cheaper nodes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I took the tags off. Not sure what'll happen with tests and such

enable cache
@codecov
Copy link

codecov bot commented Aug 3, 2022

Codecov Report

Merging #284 (2100b88) into master (b599fa5) will increase coverage by 0.88%.
The diff coverage is 75.81%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #284      +/-   ##
==========================================
+ Coverage   68.30%   69.19%   +0.88%     
==========================================
  Files          27       28       +1     
  Lines        2073     2350     +277     
==========================================
+ Hits         1416     1626     +210     
- Misses        586      641      +55     
- Partials       71       83      +12     
Impacted Files Coverage Δ
db.go 35.00% <ø> (ø)
pebble.go 75.81% <75.81%> (ø)
Impacted Files Coverage Δ
db.go 35.00% <ø> (ø)
pebble.go 75.81% <75.81%> (ø)

@itsdevbear
Copy link

@faddat how has pebbledb performance been compared to rocks? I am very much ready to ditch CGO. Any benchmarks yet?

@sascha1337
Copy link

@faddat how has pebbledb performance been compared to rocks? I am very much ready to ditch CGO. Any benchmarks yet?

Check notional org repos, cosmodia was it I think, they testing it in prod. right now on 40 chains or something

@faddat faddat closed this Aug 18, 2022
github-merge-queue bot pushed a commit to cometbft/cometbft that referenced this pull request Jan 29, 2024
This PR integrates pebbledb, a key-value store by Cockroachdb based on
Cockroach Labs findings about cgo:

* https://www.cockroachlabs.com/blog/the-cost-and-complexity-of-cgo/


Unlike any other kv store that cometbft currently supports:

* pebble is actively maintained unlike:
  * goleveldb
  * boltdb
  * badgerdb 
* pebble does not require the use of CGO like:
  * cleveldb
  * rocksdb

In benchmarks:

* pebble performs better than goleveldb, cleveldb and rocksdb
* pebble performs consistently

At Notional Ventures, Pte, we've used and submitted pebble for two years
for:

* archive nodes that would be crushed if they used goleveldb
* high performance RPC infrastructure

Here are the hitstorical pull requests to merge pebble:

* tendermint/tm-db#230
* tendermint/tm-db#231
* tendermint/tm-db#281
* tendermint/tm-db#282
* tendermint/tm-db#283
* tendermint/tm-db#284
* tendermint/tm-db#304
* tendermint/tm-db#321

Pebble snapshots for cosmos blockchains are available here:

* https://snapshot.notional.ventures/

---

#### PR checklist

- [ ] Tests written/updated
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [x] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

---------

Co-authored-by: Anton Kaliaev <[email protected]>
mergify bot pushed a commit to cometbft/cometbft that referenced this pull request Jan 29, 2024
This PR integrates pebbledb, a key-value store by Cockroachdb based on
Cockroach Labs findings about cgo:

* https://www.cockroachlabs.com/blog/the-cost-and-complexity-of-cgo/

Unlike any other kv store that cometbft currently supports:

* pebble is actively maintained unlike:
  * goleveldb
  * boltdb
  * badgerdb
* pebble does not require the use of CGO like:
  * cleveldb
  * rocksdb

In benchmarks:

* pebble performs better than goleveldb, cleveldb and rocksdb
* pebble performs consistently

At Notional Ventures, Pte, we've used and submitted pebble for two years
for:

* archive nodes that would be crushed if they used goleveldb
* high performance RPC infrastructure

Here are the hitstorical pull requests to merge pebble:

* tendermint/tm-db#230
* tendermint/tm-db#231
* tendermint/tm-db#281
* tendermint/tm-db#282
* tendermint/tm-db#283
* tendermint/tm-db#284
* tendermint/tm-db#304
* tendermint/tm-db#321

Pebble snapshots for cosmos blockchains are available here:

* https://snapshot.notional.ventures/

---

#### PR checklist

- [ ] Tests written/updated
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [x] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

---------

Co-authored-by: Anton Kaliaev <[email protected]>
(cherry picked from commit 82fa3c0)
melekes added a commit to cometbft/cometbft that referenced this pull request Jan 29, 2024
This PR integrates pebbledb, a key-value store by Cockroachdb based on
Cockroach Labs findings about cgo:

* https://www.cockroachlabs.com/blog/the-cost-and-complexity-of-cgo/


Unlike any other kv store that cometbft currently supports:

* pebble is actively maintained unlike:
  * goleveldb
  * boltdb
  * badgerdb 
* pebble does not require the use of CGO like:
  * cleveldb
  * rocksdb

In benchmarks:

* pebble performs better than goleveldb, cleveldb and rocksdb
* pebble performs consistently

At Notional Ventures, Pte, we've used and submitted pebble for two years
for:

* archive nodes that would be crushed if they used goleveldb
* high performance RPC infrastructure

Here are the hitstorical pull requests to merge pebble:

* tendermint/tm-db#230
* tendermint/tm-db#231
* tendermint/tm-db#281
* tendermint/tm-db#282
* tendermint/tm-db#283
* tendermint/tm-db#284
* tendermint/tm-db#304
* tendermint/tm-db#321

Pebble snapshots for cosmos blockchains are available here:

* https://snapshot.notional.ventures/

---

#### PR checklist

- [ ] Tests written/updated
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [x] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

---------

Co-authored-by: Anton Kaliaev <[email protected]>
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.

5 participants