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

Switch to rust-rocksdb in kvdb-rocksdb #88

Closed
bkchr opened this issue Dec 4, 2018 · 6 comments · Fixed by #257
Closed

Switch to rust-rocksdb in kvdb-rocksdb #88

bkchr opened this issue Dec 4, 2018 · 6 comments · Fixed by #257

Comments

@bkchr
Copy link
Member

bkchr commented Dec 4, 2018

kvdb-rocksdb currently uses our own fork of rust-rocksdb. The fork is diverged quite a bit. We should switch back to upstream, to get the latest updates. I started doing this in: 616b401

There are some TODOs on what is missing. We also need to upstream some functionality from our fork to upstream.

@dvdplm
Copy link
Contributor

dvdplm commented Dec 5, 2018

Do you think the kvdb-rocksdb test coverage is adequate or is this a good time to improve on that?

@bkchr
Copy link
Member Author

bkchr commented Dec 5, 2018

I did not even know that there are tests 😅
There are not that much tests, but I think that a benchmark would be good to have. So that we can compare with and without switching to rust-rocksdb.
It would also nice to have at least compile tests for windows and Android, so that we know that we at least can compile for these platforms.

@tomtau
Copy link

tomtau commented Jan 17, 2019

btw, here's a bounty on this issue: https://www.bountysource.com/issues/67028819-switch-to-rust-rocksdb-in-kvdb-rocksdb

@ordian
Copy link
Member

ordian commented Apr 11, 2019

I've taken a look at this issue (in ao-upstream-rocksdb branch, only this commit 90b47c1) and the following things are lacking in the upstream:

@dvdplm
Copy link
Contributor

dvdplm commented Apr 11, 2019

We also need to upstream some functionality from our fork to upstream.

@bkchr can you elaborate a bit on what it is that is missing upstream (other than the differences @ordian mentions)?

@bkchr
Copy link
Member Author

bkchr commented Apr 11, 2019

I think the list of @ordian seems to be correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants