feat: support build with rocksdb backend in CI #17346

yihuang · 2023-08-10T03:14:04Z

Description

bump cometbft-db dependency to switch to grocksdb
use nix to install librocksdb in CI.

a little doc:

nix build . build simapp in working directory, the result binary is in ./result/bin/simd by default.
nix run . -- version run the simapp directly, it's built on the fly.
nix run github:cosmos/cosmos-sdk -- version, run the simapp from a remote version, it'll clone code, build, and run it automatically.
nix develop, drop into a shell which contains go and librocksdb dependencies.
nix develop -c "go version", in case you don't have interactive shell.

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

confirmed the correct type prefix in the PR title
confirmed ! in the type prefix if API or client breaking change
confirmed all author checklist items have been addressed
reviewed state machine logic
reviewed API design and naming
reviewed documentation is accurate
reviewed tests and test coverage
manually tested (if applicable)

yihuang · 2023-08-10T04:00:19Z

.github/workflows/build.yml

+      - name: Build rocksdb
+        if: |
+          env.GIT_DIFF &&
+          matrix.go-arch == 'amd64'


only do once, since we don't want to cross-compiling librocksdb itself.

tac0turtle

looks good, once we have store in ci then we can run store tests with rocksdb but for now this is fine.

yihuang · 2023-08-10T10:38:21Z

looks good, once we have store in ci then we can run store tests with rocksdb but for now this is fine.

flaky-ed the simapp, more idiomatic nix usage, added a little doc in PR description.

julienrbrt · 2023-08-10T10:43:27Z

Say we run a go mod tidy, do we need to update those nix files as well so they are in sync?
A bit worried it will get out of sync fast.

Would not it be simpler to create a docker image with the necessary rocksdb deps and build simapp using it?

yihuang · 2023-08-10T10:46:52Z

Say we run a go mod tidy, do we need to update those nix files as well so they are in sync? A bit worried it will get out of sync fast.

Would not it be simpler to create a docker image with the necessary rocksdb deps and build simapp using it?

nix expressions are more maintainable than docker image, much more convenient to use IHO. but take some time to get familiar with first.

yihuang · 2023-08-10T11:07:31Z

Say we run a go mod tidy, do we need to update those nix files as well so they are in sync? A bit worried it will get out of sync fast.
Would not it be simpler to create a docker image with the necessary rocksdb deps and build simapp using it?

nix expressions are more maintainable than docker image, much more convenient to use IHO. but take some time to get familiar with first.

it also serves as an example for other chains how to build their binaries with rocksdb properly, it can do cross-compiling, static-linking out of the box, for example:

cross compile a arm64 linux static linked rocksdb powered simd on x86 macos:
nix build .#legacyPackages.x86_64-darwin.pkgsCross.aarch64-multiplatform.pkgsStatic.simd

julienrbrt · 2023-08-10T11:16:36Z

Def sounds interesting, I need to look more into it!

How do you keep gomod2nix.toml in sync?
We run / A bot runs ./scripts/go-mod-tidy-all.sh to go mod tidy our modules. Could you add something in the Makefile and call that from the script so that simapp's go.mod and simapp's gomod2nix.toml stay in sync? Unless it isn't necessary, but I am curious how it updates otherwise.

nix build . build simapp in working directory, the result binary is in ./result/bin/simd by default.

I would love if it could go to ./build instead like make build. Or we should that result folder to the .gitignore.

yihuang · 2023-08-10T11:24:30Z

Def sounds interesting, I need to look more into it!

How do you keep gomod2nix.toml in sync? We run / A bot runs ./scripts/go-mod-tidy-all.sh to go mod tidy our modules. Could you add something in the Makefile and call that from the script so that simapp's go.mod and simapp's gomod2nix.toml stay in sync? Unless it isn't necessary, but I am curious how it updates otherwise.

done, added to the script.

nix build . build simapp in working directory, the result binary is in ./result/bin/simd by default.

I would love if it could go to ./build instead like make build. Or we should that result folder to the .gitignore.

the result file is a symbolic link into somewhere in /nix/store, it's just the default choice of nix command, you can pick a different one using -o flag:

nix build . -o ./build/simd

julienrbrt

utACK.
I need to go read the nix docs now.

julienrbrt · 2023-08-10T11:31:24Z

flake.nix

+          defaultApp = apps.simd;
+          devShell = with pkgs; mkShell {
+            buildInputs = [
+              go_1_20


Can we add a useless comment like // Use Go 1.20 version, that helps when we search / replace the Go version :)

alexanderbez · 2023-08-10T16:17:48Z

This is so cool and extremely helpful, thanks @yihuang! We should really look into nix for binary creation and distribution. Having a pre-built cross-compiled binary with linked libs out of the box is a game changer IMO

alexanderbez · 2023-08-10T16:19:59Z

flake.nix

+            simd = with pkgs; mkShell {
+              buildInputs = [
+                go_1_20 # Use Go 1.20 version
+                rocksdb


How do we specifiy which version of the librocksdb we want?

There are several ways:

Pin to a nixpkgs version that contains the desired rocksdb version, that happens to be the case right now

Override the src tarball in rocksdb derivative, for example in the overlay:

rocksdb = prev.rocksdb.overrides { version = xxx; src = fetchGithub { repo hash etc.. }; };

alexanderbez

LGTM!

Nit: Not sure this actually closes #17341? I think we need to merge this, rebase main onto feature/store-v2 and then update the test-store job.

Also lint needs to be updated prior to merging :)

julienrbrt · 2023-08-10T19:16:40Z

Looks like it is blocked on the lint job, however make lint just works fine locally and in other PR. Could you maybe rebase?

WIP: cosmos#17341 - bump cometbft-db dependency to switch to grocksdb - add nix flakes to help build with rocksdb dependency

yihuang · 2023-08-11T02:02:39Z

Nit: Not sure this actually closes #17341?

changed to WIP

github-actions bot added the Type: CI label Aug 10, 2023

yihuang changed the title ~~support build with rocksdb backend in CI~~ feat: support build with rocksdb backend in CI Aug 10, 2023

github-actions bot added Type: Build C:CLI C:x/evidence C:x/upgrade C:x/feegrant C:Cosmovisor Issues and PR related to Cosmovisor C:x/nft C:Confix Issues and PR related to Confix C:x/circuit C:Hubl Tool: Hubl labels Aug 10, 2023

yihuang commented Aug 10, 2023

View reviewed changes

yihuang force-pushed the nix-rocksdb branch from a5d959e to 495c005 Compare August 10, 2023 04:23

yihuang marked this pull request as ready for review August 10, 2023 07:21

yihuang requested a review from a team as a code owner August 10, 2023 07:21

yihuang requested review from alexanderbez and removed request for kocubinski and julienrbrt August 10, 2023 07:23

tac0turtle approved these changes Aug 10, 2023

View reviewed changes

julienrbrt approved these changes Aug 10, 2023

View reviewed changes

alexanderbez reviewed Aug 10, 2023

View reviewed changes

alexanderbez approved these changes Aug 10, 2023

View reviewed changes

alexanderbez enabled auto-merge August 10, 2023 16:21

support build with rocksdb backend in CI

24ab15c

WIP: cosmos#17341 - bump cometbft-db dependency to switch to grocksdb - add nix flakes to help build with rocksdb dependency

yihuang force-pushed the nix-rocksdb branch from 35e2e56 to 24ab15c Compare August 11, 2023 02:02

fix build

c9a8292

alexanderbez added this pull request to the merge queue Aug 11, 2023

Merged via the queue into cosmos:main with commit d2b93e0 Aug 11, 2023
37 of 41 checks passed

yihuang deleted the nix-rocksdb branch August 11, 2023 08:44

julienrbrt mentioned this pull request Aug 11, 2023

fix(ci): install gomod2nix directly #17365

Merged

20 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support build with rocksdb backend in CI #17346

feat: support build with rocksdb backend in CI #17346

yihuang commented Aug 10, 2023 •

edited

Loading

yihuang Aug 10, 2023

tac0turtle left a comment

yihuang commented Aug 10, 2023

julienrbrt commented Aug 10, 2023 •

edited

Loading

yihuang commented Aug 10, 2023 •

edited

Loading

yihuang commented Aug 10, 2023 •

edited

Loading

julienrbrt commented Aug 10, 2023 •

edited

Loading

yihuang commented Aug 10, 2023 •

edited

Loading

julienrbrt left a comment

julienrbrt Aug 10, 2023 •

edited

Loading

yihuang Aug 10, 2023

alexanderbez commented Aug 10, 2023

alexanderbez Aug 10, 2023

yihuang Aug 11, 2023

alexanderbez left a comment •

edited

Loading

julienrbrt commented Aug 10, 2023

yihuang commented Aug 11, 2023

feat: support build with rocksdb backend in CI #17346

feat: support build with rocksdb backend in CI #17346

Conversation

yihuang commented Aug 10, 2023 • edited Loading

Description

Author Checklist

Reviewers Checklist

yihuang Aug 10, 2023

Choose a reason for hiding this comment

tac0turtle left a comment

Choose a reason for hiding this comment

yihuang commented Aug 10, 2023

julienrbrt commented Aug 10, 2023 • edited Loading

yihuang commented Aug 10, 2023 • edited Loading

yihuang commented Aug 10, 2023 • edited Loading

julienrbrt commented Aug 10, 2023 • edited Loading

yihuang commented Aug 10, 2023 • edited Loading

julienrbrt left a comment

Choose a reason for hiding this comment

julienrbrt Aug 10, 2023 • edited Loading

Choose a reason for hiding this comment

yihuang Aug 10, 2023

Choose a reason for hiding this comment

alexanderbez commented Aug 10, 2023

alexanderbez Aug 10, 2023

Choose a reason for hiding this comment

yihuang Aug 11, 2023

Choose a reason for hiding this comment

alexanderbez left a comment • edited Loading

Choose a reason for hiding this comment

julienrbrt commented Aug 10, 2023

yihuang commented Aug 11, 2023

yihuang commented Aug 10, 2023 •

edited

Loading

julienrbrt commented Aug 10, 2023 •

edited

Loading

yihuang commented Aug 10, 2023 •

edited

Loading

yihuang commented Aug 10, 2023 •

edited

Loading

julienrbrt commented Aug 10, 2023 •

edited

Loading

yihuang commented Aug 10, 2023 •

edited

Loading

julienrbrt Aug 10, 2023 •

edited

Loading

alexanderbez left a comment •

edited

Loading