Makefile support to statically link external plugin code

[ajkr]

Added support for detecting plugins linked in the "plugin/" directory and building them from our Makefile in a standardized way. See "plugin/README.md" for details. An example of a plugin that can be built in this way can be found in https://github.com/ajkr/dedupfs.

There will be more to do in terms of making this process more convenient and adding support for CMake.

Test Plan: my own plugin (https://github.com/ajkr/dedupfs) and also heard this patch worked with ZenFS.

[facebook-github-bot]

@ajkr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[facebook-github-bot]

@ajkr has updated the pull request. You must reimport the pull request before landing.

[facebook-github-bot]

@ajkr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[mrambacher]

I am a little confused as to how this is supposed to work. I am not certain I understand how adding the plugin to the master library solves anything (how do any of the function in the plugin get called in something like db_bench), and I do not understand how the directory tree gets established.

[ajkr]

I am not certain I understand how adding the plugin to the master library solves anything (how do any of the function in the plugin get called in something like db_bench)

This is exactly demonstrated in the example: https://github.com/ajkr/dedupfs

I do not understand how the directory tree gets established.

By the user. See the example

[facebook-github-bot]

@ajkr has updated the pull request. You must reimport the pull request before landing.

[facebook-github-bot]

@ajkr has updated the pull request. You must reimport the pull request before landing.

[facebook-github-bot]

@ajkr has updated the pull request. You must reimport the pull request before landing.

[facebook-github-bot]

@ajkr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[pdillinger]

This looks to be a great first implementation of the agreed-upon design 👍👍

[pdillinger]

Debug print?

[ajkr]

Good catch, will remove it.

[ajkr]

Thanks very much for the review!

[ajkr]

Good catch, will remove it.

[mrambacher]

I agree that this is a good first pass and is something that could potentially be marked "experimental". There are quite a few issues I see before we could mark this is "ready":

• The size of the executables grows substantially in this mode;
• There is no way to tell what plugins are installed/available in the library;
• Without CMAKE support, this will not work on Windows and potentially other platforms
• There needs to be a means to build and run the tests against the plugins
• Support for shared libraries (and potentially external libraries) is also needed

This work dovetails with some of the work I have been doing in #6591. I will write something up to bring those two efforts together and try to address some of these issues.

[facebook-github-bot]

@ajkr has updated the pull request. You must reimport the pull request before landing.

[mrambacher]

Should there also be a way to add/build custom tests? For example, if one viewed the "BackupableDB" suite as a plugin, how would one build those tests using this model?

[ajkr]

I agree that this is a good first pass and is something that could potentially be marked "experimental". There are quite a few issues I see before we could mark this is "ready":

The release note claims a "mechanism for building external plugin code into the RocksDB libraries/binaries". In my view, the public interface consists of the *.mk files in the plugin directory, which can specify source files, header files, and linker flags. I am pretty convinced we should provide the mechanism, and haven't heard concerns about the interface as described, so do not see a reason to mark the support introduced here as experimental.

• The size of the executables grows substantially in this mode;

This is addressed by no longer adding -Wl,--whole-archive.

• There is no way to tell what plugins are installed/available in the library;
• Without CMAKE support, this will not work on Windows and potentially other platforms
• There needs to be a means to build and run the tests against the plugins
• Support for shared libraries (and potentially external libraries) is also needed

I agree these would be nice to have. However, the release note only claims a "mechanism for building external plugin code into the RocksDB libraries/binaries".

This work dovetails with some of the work I have been doing in #6591. I will write something up to bring those two efforts together and try to address some of these issues.

I think it is a great long-term direction. As it relates to this PR, I expect the "building-in" mechanism introduced here will still be needed.

edit: Actually, the release note should be constrained further to say the mechanism is only available to Makefile users. I will fix that before landing.

[facebook-github-bot]

@ajkr has updated the pull request. You must reimport the pull request before landing.

[facebook-github-bot]

@ajkr has updated the pull request. You must reimport the pull request before landing.

[facebook-github-bot]

@ajkr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[facebook-github-bot]

@ajkr merged this pull request in c16d5a4.