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

Database transition #695

Open
3 of 8 tasks
Tracked by #1172
faddat opened this issue Dec 28, 2021 · 20 comments
Open
3 of 8 tasks
Tracked by #1172

Database transition #695

faddat opened this issue Dec 28, 2021 · 20 comments
Assignees
Labels
T:task ⚙️ A task belongs to a story

Comments

@faddat
Copy link
Member

faddat commented Dec 28, 2021

Summary

Goleveldb is much slower than Rocksdb, but rocksdb requires cgo and therefore build gymnastics, too. The best path forward is PebbleDB, which is a forwards compatible, pure go implementation of Rocksdb.

Problem Definition

  • relayers spend too much time syncing
  • validators spend too much time syncing
  • query performance
  • developer friction for tasks requiring an osmosis node

Proposal

I propose that we disqualify all cgo based databases, for two reasons:

  • scope restriction
  • peformance when using cosmwasm

This would mean that we drop support for rocksdb and cleveldb altogether.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@faddat
Copy link
Member Author

faddat commented Dec 28, 2021

PS: I haven't assigned this issue to myself because I think that this will take a concerted effort over time and that it should not be done suddenly, but instead gradually, validator by validator, perhaps speeding up over time.

@faddat
Copy link
Member Author

faddat commented Dec 28, 2021

Comparison videos

tendermint/tm-db#211

@ValarDragon
Copy link
Member

ValarDragon commented Dec 30, 2021

Steps to doing a switch from leveldb to rocksdb imo:

  • Double check all rocksDB options, so we don't do dumb things like try to compress all the hashes (we do this in leveldb rn). It will be a pain to change this later
  • Talk to chainlayer about getting them to serve archive nodes on alternate db (requires them resyncing from scratch)
  • Get state sync working - Maybe moot, we're going to break it again with the cosmwasm Add hooks to allow app modules to add things to state-sync cosmos/cosmos-sdk#7340
  • Make a warning emitted if your using a mismatched db version between config and whats in folder
    • One way to do this would be by making a text file in each db folder specifying db type thats on disk

@faddat faddat mentioned this issue Jan 2, 2022
4 tasks
@faddat
Copy link
Member Author

faddat commented Jan 2, 2022

NB: rumor has it that @gainsalex from juno has made cw+state sync happy

@ValarDragon ValarDragon self-assigned this Jan 6, 2022
@faddat
Copy link
Member Author

faddat commented Jan 12, 2022

@robert-zaremba
Copy link
Contributor

PS: we forked the go interface of rocksdb for the upcoming store/v2 release: https://github.com/cosmos/gorocksdb

@czarcas7ic
Copy link
Member

@ValarDragon cosmos/cosmos-sdk#10791 As discussed, mentioned issue here

@ValarDragon
Copy link
Member

Ah thanks! That explains why I didn't see it, was looking on Tendermint

@faddat faddat mentioned this issue Feb 4, 2022
4 tasks
@faddat
Copy link
Member Author

faddat commented Feb 4, 2022

Since we're adding wasm, this looks like a good candidate

cosmos/cosmos-sdk#7340

@faddat
Copy link
Member Author

faddat commented Apr 12, 2022

tendermint/tm-db#230

and

tendermint/tm-db#229

Both deal with this. the Terra team mentioned diminishing returns on using RocksDB, so last week I began to experiment with PebbleDB. Since it is still possible to choose RocksDB, especially for maturity reasons, we've also updated tm-db to use grocksdb, which will make cosmos/gorocksdb unnecessary, and allow us to support RocksDB v7.x.

#1234 also touches on these matters, fixing both state sync scripts. That said, I can't get them to work, specifically because Osmosis no longer crashes when it has applied the last snapshot.

@faddat
Copy link
Member Author

faddat commented Apr 12, 2022

Make a warning emitted if your using a mismatched db version between config and whats in folder

@ValarDragon like if configured to use rocks, but actually have goleveldb?

@faddat
Copy link
Member Author

faddat commented Apr 18, 2022

Brief synopsis of new findings, based on some experimentation with pebbledb and using rocksdb v7:

long term it is likely that pebble is better and pebble reads rocks, so there is an upgrade path that does not involve everyone updating their images.

there is some kind of bug and it is in one of these places

  • pebble itself (i sort of doubt this but surely rocks is more mature, I consider it least likely)
  • pebbledb_iterator.go
  • pebbledb_batch.go

If the bug isn't in pebbledb, it also means that we should improve the tests in tmdb / iavl. the implementation passes all tests. The work on PebbleDB is here:

osmosis-labs/tm-db#4

but regarding this bug, I could not find it, Khanh could not find it, Vuong could not find it, and Tuan could not find it. I thought it might be related to fast iavl so I tried all this on gaia and was able to reproduce the same kind of issue

The rocksdb release notes make for interesting reading:

https://github.com/facebook/rocksdb/releases/tag/v7.0.1

@faddat faddat mentioned this issue Apr 18, 2022
4 tasks
@faddat
Copy link
Member Author

faddat commented Apr 18, 2022

Here's how to do it:

install ipfs: you need to download a 1 tb file
install squashfs-tools (varies by distro)

git clone https://github.com/osmosis-labs/osmosis --branch v6.x-forceprune
go install -ldflags '-w -s -X github.com/cosmos/cosmos-sdk/types.DBBackend=rocksdb' -tags rocksdb ./...
ipfs get QmfEuEx4r6sTZmp1odCMrytV4TWvyrn4ZxYHHVSgt1P38M -o osmo-v6-rocks.squashfs
osmosisd init
unsquashfs osmo-v6-rocks.squashfs data -d ~/.osmosisd/data
osmosisd start --db_backend rocksdb

@ValarDragon
Copy link
Member

What exactly is the observed bug?

@faddat
Copy link
Member Author

faddat commented Apr 18, 2022

ah, the observed bug in pebbledb is here:

osmosis-labs/tm-db#4 (comment)

Often manifests looking like a type error and this is in fact the reason for the "speedrun" setup I'm making. I had similarish errors with badger and gaia, I should add.

But I really don't think we will have a hard and fast and clear answer to this until we are able to make changes to -- for example, tm-db and /or iavl and then run through the sequence, maybe pruning along the way.

If I had to choose between pebble and rocks-- assuming that the upstream pebble is not the source of the problem (and indeed I bet that the bug is in our implementation) then the choice would easily be pebble.

I assume that when it's referring to "traversals" that may have something to do with the iterator?

@robert-zaremba
Copy link
Contributor

@faddat do you think https://github.com/linxGnu/grocksdb is better than gorocksdb?

What's the advantage of pebble against baddger? If I remember we disqualified pebble because of their limited (storage wise) checkpoint functionality: https://pkg.go.dev/github.com/cockroachdb/pebble#DB.Checkpoint . Did you benchmark pebble against grocksdb or gorocksdb? Maybe it will be worth to add it to cosmos-sdk/db, what do you think?

@robert-zaremba
Copy link
Contributor

OK, I've created cosmos/cosmos-sdk#11683 and it seams grocksdb has more support, is active and includes optimizations.

@faddat faddat changed the title Rocksdb transition badgerdb transition May 1, 2022
@faddat
Copy link
Member Author

faddat commented May 1, 2022

@ValarDragon @czarcas7ic

Hello, I wanted to provide an update on this. Since some recent update, when using anything other than golveldb, osmosis will apply the 2nd last state sync snapshot, then complain of the app version mismatch, and crash.

I think we need a lasting solution on the app version mismatch, weather from upstream, a fork, or here.

@ValarDragon
Copy link
Member

Lets figure out how its set in a DB-agnostic way? Then perhaps we do it in the next upgrade, so we don't have any concerns around it being state machine breaking?

@faddat
Copy link
Member Author

faddat commented May 2, 2022

Sounds great

@faddat faddat changed the title badgerdb transition Database transition May 7, 2022
@czarcas7ic czarcas7ic removed their assignment Jun 23, 2022
@faddat faddat self-assigned this Nov 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T:task ⚙️ A task belongs to a story
Projects
Status: Needs Triage 🔍
Development

No branches or pull requests

5 participants