Add support for loading dynamic libraries into the RocksDB environment #5281
Conversation
I think it has been rebased. Somehow I seem to have pulled something from master accidentally. |
mrambacher
force-pushed
the
dynamic-library
branch
from
c6abe55
to
c002441
Compare
|
Hi @mrambacher. Please excuse my pushing commits directly to your branch, but I feel sometimes this can be more efficient and clearer. |
There was a problem hiding this comment.
@riversand963 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
I still see some linking errors in our internal test, will look into it. |
|
@mrambacher has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
@riversand963 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@mrambacher has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
@riversand963 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
I am fine with the changes you have made/suggested. Is there any documentation on the "things to do" before generating a PR to merge anywhere? I did builds on two platforms (MacOS, RedHat) and made sure all of the tests passed. I have no idea what "buckifier" is or how to make that work, but am willing to do more to make the process faster/smoother/easier. |
|
I think making sure the local tests pass is a good start. After generating the PR, any test failure on Travis or Appveyor is also good candidate for investigation. |
Instead use a macro to exclude dynamic library loading support from buck build.
|
@mrambacher has updated the pull request. Re-import the pull request |
|
Chatted with team offline, and we decided not to add glibc as a dependency for our internal build. Instead, we use a macro to exclude this feature when building internally. |
There was a problem hiding this comment.
@riversand963 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
What platform are you using that requires libgc? The man pages say to use
libdl. I am confused as to what you need.
…On Fri, May 24, 2019, 18:15 Facebook Community Bot ***@***.***> wrote:
***@***.**** commented on this pull request.
@riversand963 <https://github.com/riversand963> has imported this pull
request. If you are a Facebook employee, you can view this diff on
Phabricator <https://phabricator.internmc.facebook.com/D15447613>.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5281?email_source=notifications&email_token=AABCTBAORXLGHDN2FI4WAN3PXBSGZA5CNFSM4HLCH7JKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOBZWA2FA#pullrequestreview-241962260>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABCTBDJDDMUSFWPFZJWY6LPXBSGZANCNFSM4HLCH7JA>
.
|
In buck build, we have to specify |
|
@riversand963 merged this pull request in c826712. |
facebook#5281) Summary: This change adds a Dynamic Library class to the RocksDB Env. Dynamic libraries are populated via the Env::LoadLibrary method. The addition of dynamic library support allows for a few different features to be developed: 1. The compression code can be changed to use dynamic library support. This would allow RocksDB to determine at run-time what compression packages were installed. This change would eliminate the need to make sure the build-time and run-time environment had the same library set. It would also simplify some of the Java build issues (where it attempts to build and include various packages inside the RocksDB jars). 2. Along with other features (to be provided in a subsequent PR), this change would allow code/configurations to be added to RocksDB at run-time. For example, the build system includes code for building an "rados" environment and adding "Cassandra" features. Instead of these extensions being built into the base RocksDB code, these extensions could be loaded at run-time as required/appropriate, either by configuration or explicitly. We intend to push out other changes in support of the extending RocksDB at run-time via configurations. Pull Request resolved: facebook#5281 Differential Revision: D15447613 Pulled By: riversand963 fbshipit-source-id: 452cd4f54511c0bceee18f6d9d919aae9fd25fef
This change adds a Dynamic Library class to the RocksDB Env. Dynamic libraries are populated via the Env::LoadLibrary method.
The addition of dynamic library support allows for a few different features to be developed:
The compression code can be changed to use dynamic library support. This would allow RocksDB to determine at run-time what compression packages were installed. This change would eliminate the need to make sure the build-time and run-time environment had the same library set. It would also simplify some of the Java build issues (where it attempts to build and include various packages inside the RocksDB jars).
Along with other features (to be provided in a subsequent PR), this change would allow code/configurations to be added to RocksDB at run-time. For example, the build system includes code for building an "rados" environment and adding "Cassandra" features. Instead of these extensions being built into the base RocksDB code, these extensions could be loaded at run-time as required/appropriate, either by configuration or explicitly.
We intend to push out other changes in support of the extending RocksDB at run-time via configurations.