zaidoon1 / rust-rocksdb

rust wrapper for rocksdb
Apache License 2.0
11 stars 3 forks source link

Support building + linking with folly + gflags + coroutines #62

Open girlbossceo opened 4 months ago

girlbossceo commented 4 months ago

When building with io_uring, typically you might want to enable RocksDB's set_async_io apart of your ReadOptions for multi_get. However the problem is you require coroutines support for this with multi_get to be any useful in practice. And coroutines is only apart of folly.

https://github.com/facebook/rocksdb/wiki/Asynchronous-IO#configuration

In addition, we found a possible bug with RocksDB's multi_get + io_uring + set_async_io, and we think disabling optimize_multiget_for_io would mitigate it, but because it's not exposed in the FFI we can't use it, and even then it's required that optimize_multiget_for_io is using coroutines. So perhaps this would be "prep work for exposing it in the FFI".

https://github.com/facebook/rocksdb/blob/b75438f9860e3cff5e713917ed22e0ac394a758c/db/version_set.cc#L2622-L2625

I've made a very dirty way of doing this at https://github.com/girlbossceo/rust-rocksdb-zaidoon1/commit/9ecb597d966efb37ed96f4cfe3f09165013fd14a (I have not tested if this even works in a Rust project) if you're interested.

gflags is also required apart of folly.

zaidoon1 commented 4 months ago

ok this is interesting, I've been wanting to experiment with/start using io-uring but my prod environment doesn't fully support that yet so i've been delaying it, plus rocksdb is still marking this all as experimental. I'll try to look at this to see what needs to be done to make all this work, however, I will likely need your help to test things out as I'm not using this yet

girlbossceo commented 4 months ago

I was informed that coroutines support may be a lot more involved than just making folly build to use it. Something needs to be "driving" the coroutines possibly using folly's core event loop, but Rust projects generally use tokio, so somehow the FFI needs to let us integrate folly's core event loop with tokio, or we need to drive the coroutines ourselves with tokio (this is all just a vague guess).

I'm not sure if you're on Matrix (frebib is here) but in our conduwuit Matrix room (#conduwuit:puppygock.gay), that is what got brought up (some of it is conduwuit specific like the non-async frontend database calls atm):

image

A future want is we'd like to use io_uring + RocksDB async I/O + folly/coroutines + making our frontend database calls async in the hopes of extensively improving getter/iterator performance, especially with multi_get where in expensive operations like Matrix room joins we need to be accessing a ton of things.

Also, to save yourself a RocksDB + liburing headache, you may need to pull in this patch locally: https://github.com/girlbossceo/rocksdb/commit/db6df0b185774778457dabfcbd822cb81760cade

This is due to RocksDB not actually linking with liburing at build time: https://github.com/facebook/rocksdb/issues/11626

Also apparently io_uring is default enabled in RocksDB, but because of that bug no one's ever actually linked with it so it never errors (fun stuff).

Maybe it can be submitted upstream, but not sure. Either way, it would be nice to at least get building and linking with all this implemented into rust-rocksdb first then research and development can go from there.

zaidoon1 commented 4 months ago

This is due to RocksDB not actually linking with liburing at build time: https://github.com/facebook/rocksdb/issues/11626 Also apparently io_uring is default enabled in RocksDB, but because of that bug no one's ever actually linked with it so it never errors (fun stuff).

For this, while this maybe a bug with the cmake, rust-rocksb does not use cmake and sets the flags directly. So I don't think we are hitting this issue.

in general, uring + rust rocksdb should be working after https://github.com/rust-rocksdb/rust-rocksdb/pull/751 was merged. Specifically based on https://rocksdb.org/blog/2022/10/07/asynchronous-io-in-rocksdb.html, the iterator part of io uring should work just fine.

What is left to do is to get multiget working. which based on the conversation you posted above, seems like it may not be an easy/possible change since we need coroutines for that to do anything and I would like to know if it's even possible to add support for coroutines before looking at building/linking/gflags, etc...

With that said, I'm not sure what there is left to do in the short term? I can expose optimize_multiget_for_io via the c api + rust bindings so you can disable it but it's not doing anything right now since we don't have coroutine support right?

zaidoon1 commented 4 months ago

https://crates.io/crates/cxx-async might be promising. One problem is that this is for calling c++ code, rust-rocksdb uses the c api so that is another thing i'm not sure about...

girlbossceo commented 4 months ago

Yeah now that I'm reading this over again a bit, the CMake linking fix was for fixing our CI which builds RocksDB and links later as apart of just how Nix + Crane works. And optimize_multiget_for_io is only available if using coroutines.

My quick glance of cxx-async also agrees that it looks promising (even has mentions of folly, so probably our exact usecase here). My possibly ignorant guess/question here is why can't rust-rocksdb call C++ code if it can call C code just fine?

zaidoon1 commented 4 months ago

My possibly ignorant guess/question here is why can't rust-rocksdb call C++ code if it can call C code just fine?

In general rust/c++ ffi bindings have lots of gotchas and things don't always work out that's why the original rust rocksdb repo hooked into the c api of rocksdb. It might be worth to try cxx-async to see what will break..