Hello Alexey Serbin, Andrew Wong, ye yuqiang, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14620
to look at the new patch set (#3).
Change subject: KUDU-2990: remove memkind/libnuma and dynamically link memkind
at runtime
......................................................................
KUDU-2990: remove memkind/libnuma and dynamically link memkind at runtime
This patch removes memkind and libnuma from the thirdparty tree and changes
the NVM cache implementation to dynamically link memkind at runtime using
dlopen() and friends. KUDU-2990 explains why we need to do this in great
detail so I won't repeat that here. The alternatives I considered:
1. Patch memkind to dlopen() libnuma.
This is probably the most user-friendly approach because libnuma is found in
most systems (or at least in most package repositories), but a new enough
memkind is not, and having Kudu distribute memkind eases adoption of the NVM
cache. However, I was nervous about performing deep surgery in memkind as
it's a codebase with which I'm not familiar, and I wanted to minimize risk
because we'll be backporting this patch to several older branches.
2. Patch memkind to define weak libnuma symbols.
If done correctly, behavior is unchanged when libnuma is present on the host
system, but if it's not there, calls from memkind to libnuma will crash.
Again, I didn't feel comfortable hacking into memkind, plus I've found weak
symbols difficult to use in the past.
3. Remove the NVM cache from Kudu altogether.
This is obviously the safest and simplest option, but it punishes Kudu users
who actually use the NVM cache.
4. Gate the build of the NVM cache behind a CMake option.
This ought to satisfy the ASF's definition of an "optional" feature, but
does nothing for binary redistributors who wish to offer the NVM cache and
who build Kudu as a statically linked "fat" binary.
5. Build as we do today, but forbid static linkage of libnuma.
Binary redistributors will need to choose between including libnuma in their
distributions, or forcing Kudu to look for libnuma at runtime. The former
still violates ASF policy, and the latter means Kudu won't start on a system
lacking libnuma, regardless of whether the NVM cache is actually in use.
So what are the ramifications of the chosen approach?
- Kudu no longer distributes memkind or libnuma. To use the NVM cache, the
host system must provide both, and memkind must be version 1.6.0 or newer.
CentOS 6.6, CentOS 7.3, and Ubuntu 18.04 repositories all carried memkind
1.1.0, so unless you're on a cutting edge distro you'll have to build
memkind from source.
- Tests that exercise the NVM cache will be skipped if they can't find a
conformant memkind (and libnuma).
- When starting Kudu, if you don't set --block_cache_type=NVM, you shoudn't
notice any change.
- If you do, Kudu will crash at startup if it can't find a conformant
memkind. This affects upgrades: if you were already an NVM cache user but
you didn't have memkind installed, your Kudu will crash post-upgrade.
Note: this doesn't preclude implementing alternative #1 (the one I think is
ideal) in the future; we'll just have to revert the bulk of this patch when
we do so.
To test, I ran cfile-test and cache-test as follows:
- Without memkind installed: DRAM tests passed, NVM tests were skipped
- With an old memkind installed: DRAM tests passed, NVM tests were skipped
- With LD_LIBRARY_PATH=/path/to/memkind-1.9.0: All tests passed
I also manually ran a Kudu master with --block_cache_type=NVM to verify the
crashing behavior.
Change-Id: I4f474196aa98b5fa6e5966b9a3aea9a7e466805c
---
M CMakeLists.txt
D cmake_modules/FindMemkind.cmake
D cmake_modules/FindNuma.cmake
M src/kudu/cfile/CMakeLists.txt
M src/kudu/cfile/block_cache.cc
M src/kudu/cfile/cfile-test.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/cache-test.cc
M src/kudu/util/nvm_cache.cc
M src/kudu/util/nvm_cache.h
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
D thirdparty/patches/memkind-fix-build-with-old-autoconf.patch
D thirdparty/patches/memkind-fix-build-with-old-glibc.patch
D thirdparty/patches/memkind-fix-jemalloc-build-with-old-autoconf.patch
M thirdparty/vars.sh
17 files changed, 197 insertions(+), 397 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/14620/3
--
To view, visit http://gerrit.cloudera.org:8080/14620
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4f474196aa98b5fa6e5966b9a3aea9a7e466805c
Gerrit-Change-Number: 14620
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-Reviewer: ye yuqiang <yuqiang...@intel.com>
