Add shared library for musl-libc by double16 · Pull Request #3143 · meta/rocksdb

double16 · 2017-11-08T05:20:10Z

Add the jni library for musl-libc, specifically for incorporating into Alpine based docker images. The classifier is musl64.

I have signed the CLA electronically.

adamretter · 2017-11-08T12:32:11Z

I would think that this would be better as boolean and not Boolean?

I am trying to avoid auto boxing mishaps since these fields are populated using reflection. I've found less complication when using reflection to use objects vs. primitives. If you feel strongly about using a primitive I'll work on it.

adamretter · 2017-11-08T12:48:35Z

@double16 In general this seems quite a desirable feature, however I have a few concerns or suggestions for improvement:

I see that you are only building musl libraries for Linux 64 bit. However RocksDB is shipped for Linux x86, Linux x86_64 and PPCle64. It would seem sensible to me to add builds for musl binaries on those platforms too.
The build that you perform with Vagrant, looks rather complicated. If I understand correctly, you use a Vagrant CentOS 7 to launch Docker, to then run an Alpine Linux? IMHO this should be simplified to just launch a Alpine Vagrant Box.
I see that you are using Alpine 3.6, which appears to use musl 1.1.16-r14. This appears to be a very recent version of Alpine (and therefore musl). My concern here is that we have found with Glibc that it is better to link RocksDB releases against an older version of Glibc; so that we have the greatest chance of the binaries being supported on as many possible platforms as possible (as newer versions of Glibc are mostly backwards compatible). Correct me if I am wrong, but if musl follows the same backwards-compatibility approach as Glibc, would it not be better to use as old an Alpine (and hence musl) version as we can for the release builds?
I am concerned by the line private static boolean MUSL_LIBC = new File("/lib/libc.musl-x86_64.so.1").canRead();. This seems very fragile to me. I wonder if there is not a better way for us to detect musl? Perhaps we could even attempt to load the Glibc version of RocksJava .so file and if that fails, fallback to trying the musl version?
Also worth noting is that whilst we do have Vagrant boxes for cross-building RocksJava, and these were previously used for RocksJava releases (and still function). We now actually use Docker for building the releases. The targets in the Makefile for that are rocksdbjavastaticreleasedocker and rocksdbjavastaticdockerppc64le.

I am happy to discuss the above with you and work with you to help make this happen. The Dockerfiles are currently maintained here: https://github.com/evolvedbinary/docker-rocksjava. You will see some embryonic Alpine attempts there, although I could not get them to work. I have access to a PPC64le system at IBM if we want to test and produce builds for musl on ppc64le as well.

Cheers

double16 · 2017-11-08T13:30:53Z

@adamretter All good points, I'll work through these and see how far I get. Before digging into the code, a couple questions:

IIRC, Docker requires 64-bit, how do you produce the 32-bit versions on Docker?

QEMU has a PPC64 emulator, do you have experience on how well that emulates the PPC64le target? I'm thinking to do the initial work using QEMU and then if you'd like to test with real hardware later, that would speed up the development cycle.

I wasn't too happy with the new File("/lib/libc.musl-x86_64.so.1") myself, I couldn't think of a better way that fit in the existing logic. Fallback sounds good, although it's not immediately apparent how that would work in an automated test, unless there are tests using Docker. Thoughts?

Any opinions on back-porting? My primary use case is running Kafka in containers and Alpine is MUCH smaller for JVM work loads. However, even recent versions of Kafka aren't using very recent RocksDB.

adamretter · 2017-11-08T13:41:35Z

IIRC, Docker requires 64-bit, how do you produce the 32-bit versions on Docker?

Docker runs on a 64-bit host, but can happily run a 32-bit container. We also use the linux32 tool inside the container to ensure that the 32 bit binaries (compiler, linker, etc) and libraries are used, and not the 64 bit ones. For example see: https://github.com/evolvedbinary/docker-rocksjava/blob/master/centos6_x86/Dockerfile

QEMU has a PPC64 emulator, do you have experience on how well that emulates the PPC64le target? I'm thinking to do the initial work using QEMU and then if you'd like to test with real hardware later, that would speed up the development cycle.

I have no experience with QEMU PPC64 emulator. Don't worry about the PPC64le version. If you can get the x86 and x86_64 versions correct, I am happy to add the PPC64le stuff.

Fallback sounds good, although it's not immediately apparent how that would work in an automated test, unless there are tests using Docker. Thoughts?

Hmm good point. I am not sure yet.

Any opinions on back-porting? My primary use case is running Kafka in containers and Alpine is MUCH smaller for JVM work loads. However, even recent versions of Kafka aren't using very recent RocksDB.

Nope. I think the best thing is to move forwards rather than back-port. Regards Kafka specifically, I understand that they are just one minor version behind. They have opened this issue with 5.8.0 #3072 but we are waiting to hear back from them still...

double16 · 2017-11-09T16:25:29Z

@adamretter I need to change this condition to something like /lib/libc.musl*.so*. Will that work for ppc64le?

Hmmm I am a little worried about the fragility there also, what if musl is installed somewhere else? Is there a better way to detect it?

I looked at a node.js library that detects libc variants and there are two checks before the filesystem check:

getconf GNU_LIBC_VERSION (I could not find a similar MUSL_LIBC_VERSION variable)

ldd /bin/sh | grep 'musl'

I prefer the ldd check. The first one assumes if it's not glibc, assume musl.

double16 · 2017-11-09T18:04:01Z

@adamretter I have the alpine docker images building rocksdb. Should I update the '-v1' Dockerfiles, or create new '-v2' ?

Concerning the Vagrant boxes, several of the Alpine boxes I've tried do not have rsync, so I'd need to create new base boxes. I'd rather not spend the time to do that unless it's needed.

adamretter · 2017-11-09T19:21:48Z

@double16 Feel free to replace the Alpine Dockers completely. I can delete the old "-v1" ones, so if you name them without a "-vn" prefix that should be fine.

Regards Vagrant, I guess not to worry too much as we are moving to Docker AFAIK, so perhaps just remove the Vagrant stuff from your PR?

I think the ldd check will suffice for now.

facebook-github-bot · 2017-11-10T03:30:24Z

@double16 has updated the pull request. View: changes

double16 · 2018-03-06T12:42:59Z

Is there anything holding this PR back that I could address?

adamretter · 2018-03-06T14:51:47Z

@double16 Sorry I dropped the ball on this one. Let me take a look in the next day or so and get back to you...

denisa · 2018-06-29T13:07:00Z

Is there any place to upvote this?
I’m converting my docker images to alpine and would love to have this merged.

adamretter · 2018-06-29T15:06:56Z

Hmm this does seem like a good thing. I just need to find some time to work on it...

adamretter · 2018-06-29T15:07:25Z

@denisa Do you realize that these Docker images are only for building RocksDB, not for running an app with RocksDB.

denisa · 2018-06-29T16:38:43Z

umh… I currently need to take one of the openjdk/alpine image, install some libraries, then install my rocksdb-using application.

I understand that once this has been merged/released, I will only need to take one of the openjdk/alpine image, then install my rocksdb-using application.

JL2210 · 2019-03-08T22:11:24Z

What would be the case if ldd was not available? It's not usually installed with musl, you have to manually symlink it. And what if /bin/sh does not exist or is a wrapper script?

siying · 2019-09-09T23:52:00Z

@adamretter do you still think you are able to find some time to review it?

adamretter · 2019-11-24T21:06:24Z

@siying Sure, I will get back on this.

adamretter · 2019-11-26T12:14:44Z

@double16 Okay I just rebased this, and I will now work through some of the remaining issues

adamretter · 2019-11-27T00:12:33Z

@double16 I think this is in good shape now.

@siying this one is now ready to merge. In the next release of RocksJava I can now include additional musl-libc compatible binaries for x86, x86_64, ppc64le, and arm64v8.

facebook-github-bot

@siying is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot · 2019-11-27T06:09:08Z

This pull request has been merged in 8ae149e.

* upstream/master: (572 commits) Work around weird unused errors with Mingw (meta#6075) Support options.max_open_files = -1 with periodic_compaction_seconds (meta#6090) Fix HISTORY.md for 6.6.0 (meta#6096) Expose and elaborate FilterBuildingContext (meta#6088) Fix compilation under MSVC VS2015 (meta#6081) Add shared library for musl-libc (meta#3143) Refactor and clean up the code that reads a blob from a file (meta#6093) Allow fractional bits/key in BloomFilterPolicy (meta#6092) Refactor blob file creation logic (meta#6066) Use lowercase for shlwapi.lib rpcrt4.lib (meta#6076) Fix naming of library on PPC64LE (meta#6080) Small improvements to Docker build for RocksJava (meta#6079) Remove unused/undefined ImmutableCFOptions() (meta#6086) Update 3rd-party libraries used by RocksJava (meta#6084) Make default value of options.ttl to be 30 days when it is supported. (meta#6073) Ignore value of BackupableDBOptions::max_valid_backups_to_open when B… (meta#6072) Update HISTORY.md for forward compatibility (meta#6085) Support ttl in Universal Compaction (meta#6071) Fix the constness issues around autovector::iterator_impl's dereference operators (meta#6057) Support options.ttl with options.max_open_files = -1 (meta#6060) ...

Summary: Add the jni library for musl-libc, specifically for incorporating into Alpine based docker images. The classifier is `musl64`. I have signed the CLA electronically. Pull Request resolved: meta#3143 Differential Revision: D18719372 fbshipit-source-id: 6189d149310b6436d6def7d808566b0234b23313

facebook-github-bot added the CLA Signed label Nov 8, 2017

adamretter reviewed Nov 8, 2017

View reviewed changes

adamretter added java-api enhancement labels Nov 8, 2017

adamretter self-assigned this Nov 8, 2017

double16 commented Nov 9, 2017

View reviewed changes

sagar0 mentioned this pull request May 15, 2018

segfault error when use rockdb on alpline container #3848

Closed

adrianmo mentioned this pull request Feb 11, 2019

Sporadic Failure while starting Segmentstore due to RocksDB initialization error pravega/pravega#3363

Closed

JL2210 reviewed Mar 8, 2019

View reviewed changes

valeriansaliou mentioned this pull request Apr 15, 2019

MUSL build for Linux x64 valeriansaliou/sonic#128

Closed

siying assigned vjnadimpalli Sep 9, 2019

double16 and others added 4 commits November 26, 2019 22:36

Add shared library for musl-libc

0f5114b

Back-port musl64 support and publish to bintray double16 account

73de4c6

Use correct Docker images for MUSL libc

8c85097

Add PPC64LE and ARM64v8 MUSL libc docker builds

0025863

adamretter added 4 commits November 26, 2019 22:51

Reinstate Vagrant build option for RocksJava MUSL libc

ff5f024

Correct the docs

432e0a3

Better to check /usr/bin/env rather than /bin/sh for musl-libc

a03ab4f

Adjust naming of RocksJava musl-libc artifacts

23089a1

facebook-github-bot reviewed Nov 27, 2019

View reviewed changes

facebook-github-bot closed this in 8ae149e Nov 27, 2019

facebook-github-bot added the Merged label Nov 27, 2019

gavv mentioned this pull request Jun 8, 2020

Build and publish JAR roc-streaming/roc-java#10

Open

DRoppelt mentioned this pull request May 6, 2022

Auto-detection of musl assumes shell access to call ldd #9956

Closed

Conversation

double16 commented Nov 8, 2017

Uh oh!

adamretter Nov 8, 2017

Choose a reason for hiding this comment

Uh oh!

double16 Nov 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamretter commented Nov 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

double16 commented Nov 8, 2017

Uh oh!

adamretter commented Nov 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

double16 Nov 9, 2017

Choose a reason for hiding this comment

Uh oh!

adamretter Nov 9, 2017

Choose a reason for hiding this comment

Uh oh!

double16 Nov 9, 2017

Choose a reason for hiding this comment

Uh oh!

double16 commented Nov 9, 2017

Uh oh!

adamretter commented Nov 9, 2017

Uh oh!

facebook-github-bot commented Nov 10, 2017

Uh oh!

double16 commented Mar 6, 2018

Uh oh!

adamretter commented Mar 6, 2018

Uh oh!

denisa commented Jun 29, 2018

Uh oh!

adamretter commented Jun 29, 2018

Uh oh!

adamretter commented Jun 29, 2018

Uh oh!

denisa commented Jun 29, 2018

Uh oh!

JL2210 Mar 8, 2019

Choose a reason for hiding this comment

Uh oh!

siying commented Sep 9, 2019

Uh oh!

adamretter commented Nov 24, 2019

Uh oh!

adamretter commented Nov 26, 2019

Uh oh!

adamretter commented Nov 27, 2019

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Nov 27, 2019

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

double16 Nov 8, 2017 •

edited

Loading

adamretter commented Nov 8, 2017 •

edited

Loading

adamretter commented Nov 8, 2017 •

edited

Loading