-
Notifications
You must be signed in to change notification settings - Fork 6.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Provide an allocator for new memory type to be used with RocksDB block cache #6214
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution. I have some comments.
memory/memkind_kmem_allocator.cc
Outdated
* | ||
* SPDX-License-Identifier: Apache-2.0 | ||
* | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RocksDB supports GPLv2 + Apache v2 dual license. Having some source code with only one of them confuses people and will make adaption more complicated for potential users. I would be reluctant to accept source file like this. Ideally, you give the copyright to RocksDB too. I don't think these source files contain any IP worth protecting so hopefully it's easy for you to do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Headers are updated to include dual license and Facebook copyright.
size_t size = allocator.UsableSize(p, 1024); | ||
ASSERT_GE(size, 1024); | ||
allocator.Deallocate(p); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be interesting to add a test of full DB with block cache to allocate from this allocator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a full DB test using the new allocator. The database is populated and a flush is triggered, so the block cache is used for subsequent reads.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
A release of the memkind library with KMEM DAX support is available (v1.10.0). The PR description is updated to reference the release rather than a development commit. |
- Add MemkindKmemAllocator - Add --use_cache_memkind_kmem_allocator db_bench option (to create an LRU block cache with the new allocator) - Add detection of memkind library with KMEM DAX support - Add tests for MemkindKmemAllocator
b4f301f
to
5c3bffb
Compare
@lucagiac81 has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
This pull request has been merged in 66a95f0. |
@lucagiac81 great patch, this will enable users to keep the block cache in persistent RAM. I have two questions:
|
Summary: Two recent diffs can be autoformatted. Also add HISTORY.md entry for facebook#6214 Test Plan: Run all existing tests
@dhruba thanks! About your questions:
I’d also like to clarify that the persistent nature of the memory is not used here. It is used in volatile mode. |
Summary: Improve memkind library detection in build_detect_platform: - The current position of -lmemkind does not work with all versions of gcc - LDFLAGS allows specifying non-standard library path through EXTRA_LDFLAGS After the change, the options match TBB detection. This is a follow-up to #6214. Pull Request resolved: #9134 Reviewed By: ajkr, mrambacher Differential Revision: D32192028 fbshipit-source-id: 115fafe8d93f1fe6aaf80afb32b2cb67aad074c7
New memory technologies are being developed by various hardware vendors (Intel DCPMM is one such technology currently available). These new memory types require different libraries for allocation and management (such as PMDK and memkind). The high capacities available make it possible to provision large caches (up to several TBs in size), beyond what is achievable with DRAM.
The new allocator provided in this PR uses the memkind library to allocate memory on different media.
Performance
We tested the new allocator using db_bench.
The plot shows throughput (ops/s) relative to a configuration with no block cache and default allocator.
For all tests, p99 latency is below 500 us.
Changes
Minimum Requirements
Memory Configuration
The allocator uses the MEMKIND_DAX_KMEM memory kind. Follow the instructions on memkind’s GitHub page to set up NVDIMM memory accordingly.
Note on memory allocation with NVDIMM memory exposed as system memory.
Usage
When creating an LRU cache, pass a MemkindKmemAllocator object as argument.
For example (replace capacity with the desired value in bytes):
Refer to RocksDB’s block cache documentation to assign the LRU cache as block cache for a database.