cockroachdb / c-rocksdb

🚫 DEPRECATED
45 stars 19 forks source link

Deprecated? #45

Closed tessr closed 5 years ago

tessr commented 7 years ago

I see from the repo tagline that this repo is now 🚫 DEPRECATED but I was curious when that happened, and why, and what y'all are now doing instead/recommend doing to build Go projects with rocksdb, especially since the recommended rocksdb package for Go uses it.

Thanks! :bowing_woman:

tamird commented 7 years ago

Pasting my reply from https://github.com/cockroachdb/c-protobuf/issues/23:

CockroachDB is now using ~protobuf~rocksdb's native build system, so this is no longer used, and thus unmaintained.

The problems with our approach of c-* repositories were largely to do with diverse platform support, which was hard to do with cgo which can't express the configure step of these build system.

tessr commented 7 years ago

Thanks for the speedy reply!

So instead, you're building rocksdb and then pointing your Go code at the c-deps directory through cgo flags?

tamird commented 7 years ago

Yep, something like that. The reality is more complicated because of the need to support multiple cross-compilation targets in the same GOPATH, but that's the gist.

We plan to blog about the way we achieve this in the near future; I'll be sure to ping this issue when we do ;)

On May 19, 2017 14:15, "Tess Rinearson" notifications@github.com wrote:

Thanks for the speedy reply!

So instead, you're building rocksdb and then pointing your Go code at the c-deps directory through cgo flags?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/cockroachdb/c-rocksdb/issues/45#issuecomment-302774474, or mute the thread https://github.com/notifications/unsubscribe-auth/ABdsPP-uYHTKdMgEFYbpRmDp9A-x9wjhks5r7dxKgaJpZM4Nf2xX .

tessr commented 7 years ago

Awesome. That would be such a service to everyone trying to use RocksDB with Go :)

JasonCoombs commented 6 years ago

@tamird wrote: "We plan to blog about the way we achieve this in the near future; I'll be sure to ping this issue when we do ;)"

can haz blog?

tamird commented 6 years ago

I no longer work on CockroachDB, but I believe the fundamentals of build system integration haven't changed. @benesch is probably the best person to nag about this :)

benesch commented 5 years ago

It's no longer looking like we're going to write a blog about this, I'm afraid. Here's a quick brain dump that can hopefully suffice.

The short answer is what @tamird said: go build is simply too inflexible to meet our needs. There is an incredible amount of logic in our C/C++ dependencies' build systems. RocksDB, for example, has over 1k lines of CMake: https://github.com/facebook/rocksdb/blob/master/CMakeLists.txt. This build system is updated frequently as new features demand new compiler flags.

A good example is the ROCKSDB_RANGESYNC_PRESENT test that RocksDB performs: https://github.com/facebook/rocksdb/blob/897fe6a4a3e45032efdba08f1965379a52a8c2f9/CMakeLists.txt#L425. On newer versions of Linux, RocksDB can use a special sync_file_range syscall to fsync only part of a file, which speeds up writes quite a bit. But on old versions of Linux, that syscall isn't available, and so RocksDB will fall back to calling fsync on the whole file. It is trivial to detect support for this feature in CMake, and wholly impossible to do so with go build. The Go build system forces you to hardcode these defines up front rather than detecting them at compile time. For example:

https://github.com/cockroachdb/c-rocksdb/blob/0dd42399d1f02019c982d1752beee2f145fdd2dd/cgo_flags.go#L9-L13

This is unmaintainable for two reasons. The first is that adding a new platform is painful. RocksDB compiles on OpenBSD out of the box, but to make this repository compile on OpenBSD, you'd have to come up with the right list of defines, and then hardcode it. And then you have to repeat this for the other four C/C++ dependencies. This work piles up quickly, especially as you consider the various BSD-like systems (FreeBSD, NetBSD, Dragonfly, etc.). The second is that you can't possibly hardcode a list of compiler flags that works for every compiler/CPU/OS that you want to support. What if you're running a version of Linux that doesn't support sync_file_range? There's no way to dynamically choose at compile-time with go build: you have to either turn it on, and break compatibility with old Linuxes, or turn it off, and forgo the performance improvement on new Linuxes.

So what do we do instead? We wrap go build in a Makefile. A (very) simplified version looks like this:

# Makefile
# Assuming a RocksDB submodule at rocksdb/src.

rocksdb/build/Makefile:
    git submodule update --init
    cd rocksdb/build && cmake ../src

librocksdb.a: rocksdb/build/Makefile
    cd rocksdb/build && make

build: librocksdb.a
    go build 
// cgo_flags.go
package main

// #cgo LDFLAGS: ./librocksdb.a
import "C"

The upside is that we get to reuse RocksDB's entire build system for free. The downside is that go build will fail because librocksdb.a doesn't exist, so now all developers will need to have Make and CMake installed and retrain their fingers to type make build. Perhaps worse, anything that depends on this package will have to give up on go build too and use a Makefile that recurses into our Makefile. So it goes. The tradeoffs make sense in our case because we have such a large set of C/C++ dependencies and so many platforms to support. YMMV.

I'm going to close this issue out, but please let me know if you have any questions!