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

Create DB Migrator and Allow use of Rocks DB and Badger DB #604

Merged
merged 297 commits into from
Apr 21, 2022

Conversation

dwedul-figure
Copy link
Contributor

@dwedul-figure dwedul-figure commented Jan 18, 2022

Description

closes: #702
closes: #696

This PR does several things:

  1. Causes provenanced to be built to support BadgerDB.
  2. Allows provenanced to be built to support RocksDB, but doesn't include it by default due to reliance on a shared library on the user's system (similar to cleveldb).
  3. Adds the target make rocksdb to build and install the rocksdb shared library.
  4. Adds the target make cleveldb to build and install the cleveldb shared library.
  5. Adds the target make build-dbmigrate to build dbmigrate. It is a utility that can be used to convert a Provenance node's databases to a different backend.
  6. Added the file docs/Building.md to document some information about building provenanced and dbmigrate.
  7. Overhauls the Github workflow for the sims test to run them on various OSs and with various DB Backends.
  8. Tweaks the Release Github workflow to run the build jobs for all PRs, but only actually create the release stuff on a release tag.

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer
  • Review Codecov Report in the comment section below once CI passes

@codecov
Copy link

codecov bot commented Feb 16, 2022

Codecov Report

Merging #604 (a713608) into main (6d9fec0) will decrease coverage by 0.58%.
The diff coverage is 36.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #604      +/-   ##
==========================================
- Coverage   56.75%   56.16%   -0.59%     
==========================================
  Files         177      179       +2     
  Lines       21166    21805     +639     
==========================================
+ Hits        12012    12247     +235     
- Misses       8243     8639     +396     
- Partials      911      919       +8     
Impacted Files Coverage Δ
cmd/provenanced/cmd/root.go 61.84% <0.00%> (ø)
cmd/dbmigrate/utils/migrator.go 36.57% <36.57%> (ø)
cmd/dbmigrate/utils/cleveldb.go 100.00% <100.00%> (ø)

dwedul-figure and others added 13 commits February 15, 2022 17:29
* Change go.mod to use wasmd fork to support old address length

* Linked issue
* adding upgrade handler indigo.
# Conflicts:
#	CHANGELOG.md
#	Makefile
#	app/upgrades.go
#	go.sum
…rable changes (e.g. from a log_level flag or some env var or something).
… and allow the control of those through env vars prefixed with DBM. This is so that the log_level and log_format config values aren't accidentally changed when updating the config for the db value. It also makes it so that the dbmigrate utility doesn't use the values from the config files, which are designed for a daemon rather than a one-off program. Basically, if the config has log_level = error, I still want dbmigrate to output what it's doing. But I also want to allow control of the output of dbmigrate.
derekadams
derekadams previously approved these changes Apr 14, 2022
# Conflicts:
#	.github/workflows/release.yml
#	.github/workflows/test.yml
#	go.sum
iramiller
iramiller previously approved these changes Apr 16, 2022
Copy link
Member

@iramiller iramiller left a comment

Choose a reason for hiding this comment

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

I wasn't able to get rocksdb to build right on my ancient laptop but the alternate database migration support and badgerdb all worked perfectly.

@dwedul-figure dwedul-figure enabled auto-merge (squash) April 21, 2022 16:33
Copy link
Member

@iramiller iramiller left a comment

Choose a reason for hiding this comment

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

Its a bit of a concern that "you can always open a cleveldb using goleveldb, not the vice versa" ... there were many goleveldb users that had to switch to cleveldb in the 1.8.0 upgrade to fix the concurrency issues...

@dwedul-figure
Copy link
Contributor Author

Its a bit of a concern that "you can always open a cleveldb using goleveldb, not the vice versa" ... there were many goleveldb users that had to switch to cleveldb in the 1.8.0 upgrade to fix the concurrency issues...

Yeah., I wonder if it's a mac vs linux thing or something. On my mac, I ran into issues where I'd make run, stop it after a few blocks and try to migrate it to badgerdb. But it would fail because it was trying to read it with cleveldb and couldn't. But that's backwards from what we saw on the mainnet nodes; they couldn't open the cleveldb database using goleveldb.

Maybe a followup is flag to indicate which to prefer to use for reading. We can't just provide a "read as" flag because not all dbs have to be the same type. But when it comes down to the choice between goleveldb and cleveldb, maybe it's better to default to one and have a flag to override that default.

The way I've got it now, though, when it's down to either goleveldb or cleveldb, it will actually try to open the db and read a few entries. But I don't like that because the function to open it will create it if it doesn't exist. That can cause problems if, for example, the db is actually rocksdb, and you're trying to open it as a leveldb.

At one point, I was kind of cheesing the cleveldb and goleveldb detection tests by just manipulating the PossibleDBTypes variable, but I decided that was the wrong thing to do, and that a comment about weirdness would be better. I just don't know what exactly happens after a fresh make run that makes the application db unreadable by cleveldb, but readable by goleveldb.

@dwedul-figure dwedul-figure merged commit 1980f2b into main Apr 21, 2022
@dwedul-figure dwedul-figure deleted the dwedul/rocksdb branch April 21, 2022 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI Command line interface feature documentation Improvements or additions to documentation enhancement New feature or request github_actions Pull requests that update Github_actions code go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow RocksDB and BadgerDB to be used. Support migration of data to databases other than leveldb
8 participants