use grocksdb to support latest version of rocksdb #42

yihuang · 2023-03-06T02:48:07Z

Closes: #43 #44
also add API NewRocksDBWithRawDB to support read-only open mode and customize options.

The impact on existing user is it requires to build with rocksdb v7.10.2 now, otherwise, it behaves the same as before.

also add API `NewRocksDBWithRawDB` to support read-only open mode and customize options

thanethomson · 2023-03-06T12:01:57Z

@yihuang, could you please provide a more detailed explanation (ideally in an issue) as to why this change is necessary?

Also, it appears as though the tests in CI are failing.

yihuang · 2023-03-08T04:43:05Z

@yihuang, could you please provide a more detailed explanation (ideally in an issue) as to why this change is necessary?

Also, it appears as though the tests in CI are failing.

done, added issue #43 and #44 which are fixed by this PR at once.

The ci failure is due to rocksdb version, I updated the version number in the docker file, not sure if that'll update the image automatically.

adizere · 2023-03-08T13:53:04Z

Thanks for this proposal @yihuang !

Copying @faddat, do you see any concerns in replacing github.com/tecbot/gorocksdb with github.com/linxGnu/grocksdb ?

yihuang · 2023-03-09T00:40:28Z

Thanks for this proposal @yihuang !

Copying @faddat, do you see any concerns in replacing github.com/tecbot/gorocksdb with github.com/linxGnu/grocksdb ?

we already used on production, so far so good, the only thing different is the building dependency change, no side-effect observed in runtime, other than the changes caused by our tuning of rocksdb options.

adizere · 2023-03-09T10:54:54Z

Re: the CI failure for the "Docker testing image" workflow --

Not sure what the fix is, but I managed to repro the same problem in a separate PR (#45). Is it possible that the DockerHub secrets are not propagated into forks (such as this) of this repo and that is causing the failure?

@thanethomson FYI have installed fresh new access tokens (DOCKERHUB_USERNAME/DOCKERHUB_TOKEN) for bftbot in this repo, so the tokens should be fine.

thanethomson · 2023-03-09T12:24:05Z

@yihuang please address the failing test. This failure is independent of the Docker build failure (we're investigating what's causing that).

yihuang · 2023-03-15T09:16:03Z

@yihuang please address the failing test. This failure is independent of the Docker build failure (we're investigating what's causing that).

I think it's just due to incompatible rocksdb dependency, which can be fixed by updating the image with newer rocksdb library.

/go/pkg/mod/github.com/linx!gnu/grocksdb@v1.7.15/cache.go:86:2: could not determine kind of name for C.rocksdb_lru_cache_options_set_num_shard_bits

robert-zaremba

tecbot/gorocksdb is outdated, and missing a maintainer.

linxGnu/grocksdb is definitely a way to go.

pending test fixes

facundomedica · 2023-03-20T16:06:57Z

@yihuang please address the failing test. This failure is independent of the Docker build failure (we're investigating what's causing that).

I think it's just due to incompatible rocksdb dependency, which can be fixed by updating the image with newer rocksdb library.
/go/pkg/mod/github.com/linx!gnu/grocksdb@v1.7.15/cache.go:86:2: could not determine kind of name for C.rocksdb_lru_cache_options_set_num_shard_bits

Looks like that was added on 7.5.3 any ideas why it's failing? Right now building Cosmos SDK with Rocks DB is not possible without using a fork + some replaces :(

alexanderbez

utACK. Let's get this merged and released ASAP :)

alexanderbez · 2023-03-20T18:22:00Z

tools/Dockerfile

@@ -17,7 +17,7 @@ RUN apt-get update && apt-get install -y --no-install-recommends \

 FROM build AS install
 ARG LEVELDB=1.20
-ARG ROCKSDB=6.24.2
+ARG ROCKSDB=7.9.2


Latest version is now v8 BTW

https://github.com/facebook/rocksdb/releases/tag/v8.0.0

linxGnu/grocksdb#111
Need a few tweaks to build with v8.0

yihuang · 2023-03-20T18:58:34Z

@yihuang please address the failing test. This failure is independent of the Docker build failure (we're investigating what's causing that).

I think it's just due to incompatible rocksdb dependency, which can be fixed by updating the image with newer rocksdb library.
/go/pkg/mod/github.com/linx!gnu/grocksdb@v1.7.15/cache.go:86:2: could not determine kind of name for C.rocksdb_lru_cache_options_set_num_shard_bits
Looks like that was added on 7.5.3 any ideas why it's failing? Right now building Cosmos SDK with Rocks DB is not possible without using a fork + some replaces :(

I guess become the CI image not updated?

thanethomson · 2023-03-22T17:47:24Z

@yihuang we updated the CI to now use a Docker image built from the current branch (as opposed to one previously built from Docker Hub) and it still seems as though the test is failing.

…etbft#47) Bumps [docker/setup-buildx-action](https://github.com/docker/setup-buildx-action) from 2.4.1 to 2.5.0. - [Release notes](https://github.com/docker/setup-buildx-action/releases) - [Commits](docker/setup-buildx-action@v2.4.1...v2.5.0) --- updated-dependencies: - dependency-name: docker/setup-buildx-action dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

codecov · 2023-03-24T10:23:41Z

Codecov Report

Merging #42 (5608942) into main (317cc9f) will increase coverage by 0.01%.
The diff coverage is 93.75%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #42      +/-   ##
==========================================
+ Coverage   68.58%   68.59%   +0.01%     
==========================================
  Files          27       27              
  Lines        2123     2124       +1     
==========================================
+ Hits         1456     1457       +1     
  Misses        592      592              
  Partials       75       75

Impacted Files	Coverage Δ
rocksdb.go	`72.05% <92.30%> (+0.20%)`	⬆️
rocksdb_batch.go	`84.31% <100.00%> (ø)`
rocksdb_iterator.go	`93.02% <100.00%> (ø)`

Impacted Files	Coverage Δ
rocksdb.go	`72.05% <92.30%> (+0.20%)`	⬆️
rocksdb_batch.go	`84.31% <100.00%> (ø)`
rocksdb_iterator.go	`93.02% <100.00%> (ø)`

yihuang · 2023-03-24T10:25:27Z

@yihuang we updated the CI to now use a Docker image built from the current branch (as opposed to one previously built from Docker Hub) and it still seems as though the test is failing.

CI passed now.

thanethomson · 2023-03-27T16:48:34Z

How do folks using rocksdb with the current cometbft-db release upgrade?

thanethomson · 2023-03-29T15:37:43Z

Makefile

@@ -43,7 +43,7 @@ test-badgerdb:

 test-all:
 @echo "--> Running go test"
- @go test $(PACKAGES) -tags cleveldb,boltdb,rocksdb,badgerdb -v
+ @go test $(PACKAGES) -tags cleveldb,boltdb,rocksdb,grocksdb_clean_link,badgerdb -v


What does this grocksdb_clean_link tag do?

It's to control the link flags, the default one is suitable for static linking which specifies all indirect dependencies explicitly, the clean version is suitable for dynamic linking case which only specify "-lrocksdb" and a few system dependencies

robert-zaremba · 2023-03-30T10:08:58Z

Does this fix the rocksdb usage with Cosmos SDK 0.46 or 0.47?

lasarojc · 2023-03-30T10:31:13Z

How do folks using rocksdb with the current cometbft-db release upgrade?

Apparently simply upgrading should be fine. According to
https://github.com/facebook/rocksdb/wiki/RocksDB-Compatibility-Between-Different-Releases

Backward compatible: newer version of RocksDB should be able to open DBs generated by all previous releases for normal configuration.

yihuang · 2023-03-31T14:14:59Z

How do folks using rocksdb with the current cometbft-db release upgrade?

In our experience, upgrading rocksdb library is totally backward compatible.

thanethomson

In our experience, upgrading rocksdb library is totally backward compatible.

Excellent. If that's the case, please add a changelog entry under the "Dependencies" section (as per https://github.com/cometbft/cometbft/blob/main/CONTRIBUTING.md#changelog) and then we can merge and ship this in a non-breaking release.

yihuang · 2023-04-12T13:41:09Z

In our experience, upgrading rocksdb library is totally backward compatible.

Excellent. If that's the case, please add a changelog entry under the "Dependencies" section (as per https://github.com/cometbft/cometbft/blob/main/CONTRIBUTING.md#changelog) and then we can merge and ship this in a non-breaking release.

sorry for the delay, just added changelog.

CHANGELOG.md

thanethomson

Awesome, thank you @yihuang!

yihuang requested a review from a team as a code owner March 6, 2023 02:48

yihuang mentioned this pull request Mar 6, 2023

Problem: sdk v0.46.11 is not used crypto-org-chain/cronos#909

Merged

13 tasks

use grocksdb to support latest version of rocksdb

b3e4fd3

also add API `NewRocksDBWithRawDB` to support read-only open mode and customize options

yihuang force-pushed the main branch from 3af958b to b3e4fd3 Compare March 6, 2023 03:16

update rocksdb version for ci testing

595f503

thanethomson added the dependencies Pull requests that update a dependency file label Mar 8, 2023

adizere mentioned this pull request Mar 9, 2023

Testing the source of docker.yml failure #45

Closed

yihuang mentioned this pull request Mar 17, 2023

Rocksdb: Cosmos SDK 0.46 & 0.47 doesn't build cosmos/cosmos-sdk#15421

Closed

robert-zaremba approved these changes Mar 17, 2023

View reviewed changes

alexanderbez approved these changes Mar 20, 2023

View reviewed changes

thanethomson mentioned this pull request Mar 22, 2023

ci: Build Docker image during test #50

Merged

Merge branch 'main' into main

a4fa0ad

dependabot bot and others added 5 commits March 24, 2023 15:14

fix ci test

41bb123

Merge branch 'main' into main

d1fcf33

grocksdb v1.7.16

253f8a3

rocksdb v7.10.2

0ea0811

lasarojc approved these changes Mar 27, 2023

View reviewed changes

thanethomson reviewed Mar 29, 2023

View reviewed changes

thanethomson reviewed Apr 3, 2023

View reviewed changes

changelog

f186aae

thanethomson reviewed Apr 13, 2023

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

yihuang added 2 commits April 13, 2023 09:06

fix changelog

7e677f0

Merge branch 'main' into main

5608942

yihuang requested a review from thanethomson April 13, 2023 01:18

thanethomson approved these changes Apr 13, 2023

View reviewed changes

thanethomson merged commit 3a99e92 into cometbft:main Apr 13, 2023

melekes mentioned this pull request Nov 15, 2023

no API to customize rocksdb options and read-only mode #44

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use grocksdb to support latest version of rocksdb #42

use grocksdb to support latest version of rocksdb #42

yihuang commented Mar 6, 2023 •

edited

Loading

thanethomson commented Mar 6, 2023

yihuang commented Mar 8, 2023 •

edited

Loading

adizere commented Mar 8, 2023

yihuang commented Mar 9, 2023

adizere commented Mar 9, 2023 •

edited

Loading

thanethomson commented Mar 9, 2023

yihuang commented Mar 15, 2023 •

edited

Loading

robert-zaremba left a comment •

edited

Loading

facundomedica commented Mar 20, 2023

alexanderbez left a comment

alexanderbez Mar 20, 2023

alexanderbez Mar 20, 2023

yihuang Mar 22, 2023 •

edited

Loading

alexanderbez Mar 22, 2023

yihuang commented Mar 20, 2023

thanethomson commented Mar 22, 2023

codecov bot commented Mar 24, 2023 •

edited

Loading

yihuang commented Mar 24, 2023

thanethomson commented Mar 27, 2023

thanethomson Mar 29, 2023

yihuang Mar 29, 2023 •

edited

Loading

robert-zaremba commented Mar 30, 2023

lasarojc commented Mar 30, 2023

yihuang commented Mar 31, 2023 •

edited

Loading

thanethomson left a comment

yihuang commented Apr 12, 2023

thanethomson left a comment

use grocksdb to support latest version of rocksdb #42

use grocksdb to support latest version of rocksdb #42

Conversation

yihuang commented Mar 6, 2023 • edited Loading

thanethomson commented Mar 6, 2023

yihuang commented Mar 8, 2023 • edited Loading

adizere commented Mar 8, 2023

yihuang commented Mar 9, 2023

adizere commented Mar 9, 2023 • edited Loading

thanethomson commented Mar 9, 2023

yihuang commented Mar 15, 2023 • edited Loading

robert-zaremba left a comment • edited Loading

Choose a reason for hiding this comment

facundomedica commented Mar 20, 2023

alexanderbez left a comment

Choose a reason for hiding this comment

alexanderbez Mar 20, 2023

Choose a reason for hiding this comment

alexanderbez Mar 20, 2023

Choose a reason for hiding this comment

yihuang Mar 22, 2023 • edited Loading

Choose a reason for hiding this comment

alexanderbez Mar 22, 2023

Choose a reason for hiding this comment

yihuang commented Mar 20, 2023

thanethomson commented Mar 22, 2023

codecov bot commented Mar 24, 2023 • edited Loading

Codecov Report

yihuang commented Mar 24, 2023

thanethomson commented Mar 27, 2023

thanethomson Mar 29, 2023

Choose a reason for hiding this comment

yihuang Mar 29, 2023 • edited Loading

Choose a reason for hiding this comment

robert-zaremba commented Mar 30, 2023

lasarojc commented Mar 30, 2023

yihuang commented Mar 31, 2023 • edited Loading

thanethomson left a comment

Choose a reason for hiding this comment

yihuang commented Apr 12, 2023

thanethomson left a comment

Choose a reason for hiding this comment

yihuang commented Mar 6, 2023 •

edited

Loading

yihuang commented Mar 8, 2023 •

edited

Loading

adizere commented Mar 9, 2023 •

edited

Loading

yihuang commented Mar 15, 2023 •

edited

Loading

robert-zaremba left a comment •

edited

Loading

yihuang Mar 22, 2023 •

edited

Loading

codecov bot commented Mar 24, 2023 •

edited

Loading

yihuang Mar 29, 2023 •

edited

Loading

yihuang commented Mar 31, 2023 •

edited

Loading