-
Notifications
You must be signed in to change notification settings - Fork 34
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
Introduce Valkey Over RDMA protocol #123
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.
Looks good. I have some comments.
This text is useful to add in the documentation, at least some of it, and the links? When this is merged, users will not find the description of the PR. :-) I think the PR description can just be "Add documentation for the RDMA feature added in valkey-io/valkey#477" |
a84c13b
to
a8d9f66
Compare
Fine, I take the background part and paper links to the
So I prefer to keep the RDMA.md as detailed as possible. |
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.
LGTM
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.
LGTM. All nits. Thanks @pizhenwei!
b1bd80a
to
22a4abd
Compare
Thanks for your suggestions! Several changes in the new version as you suggested. |
18f0625
to
a0ae1ce
Compare
Hi @zuiderkwast @PingXie @madolson |
Thanks. In the future you can just add more commits in the same PR. I makes review easier. We squash-merge PRs anyway. |
Hi @zuiderkwast @PingXie @madolson |
@madolson PING! |
Hi @madolson , |
@madolson PING! |
@pizhenwei We can first make a decision about the feature valkey-io/valkey#477. If we decide to accept it, then I don't think there are any major problems with the docs. We, the maintainers team, have been very busy with many features to discuss. |
44492c3
to
f11bba4
Compare
Adds an option to build RDMA support as a module: make BUILD_RDMA=module To start valkey-server with RDMA, use a command line like the following: ./src/valkey-server --loadmodule src/valkey-rdma.so \ port=6379 bind=xx.xx.xx.xx * Implement server side of connection module only, this means we can *NOT* compile RDMA support as built-in. * Add necessary information in README.md * Support 'CONFIG SET/GET', for example, 'CONFIG Set rdma.port 6380', then check this by 'rdma res show cm_id' and valkey-cli (with RDMA support, but not implemented in this patch). * The full listeners show like: listener0:name=tcp,bind=*,bind=-::*,port=6379 listener1:name=unix,bind=/var/run/valkey.sock listener2:name=rdma,bind=xx.xx.xx.xx,bind=yy.yy.yy.yy,port=6379 listener3:name=tls,bind=*,bind=-::*,port=16379 Because the lack of RDMA support from TCL, use a simple C program to test Valkey Over RDMA (under tests/rdma/). This is a quite raw version with basic library dependence: libpthread, libibverbs, librdmacm. Run using the script: ./runtest-rdma [ OPTIONS ] To run RDMA in GitHub actions, a kernel module RXE for emulated soft RDMA, needs to be installed. The kernel module source code is fetched a repo containing only the RXE kernel driver from the Linux kernel, but stored in an separate repo to avoid cloning the whole Linux kernel repo. ---- Since 2021/06, I created a [PR](redis/redis#9161) for *Redis Over RDMA* proposal. Then I did some work to [fully abstract connection and make TLS dynamically loadable](redis/redis#9320), a new connection type could be built into Redis statically, or a separated shared library(loaded by Redis on startup) since Redis 7.2.0. Base on the new connection framework, I created a new [PR](redis/redis#11182), some guys(@xiezhq-hermann @zhangyiming1201 @JSpewock @uvletter @FujiZ) noticed, played and tested this PR. However, because of the lack of time and knowledge from the maintainers, this PR has been pending about 2 years. Related doc: [Introduce *Valkey Over RDMA* specification](valkey-io/valkey-doc#123). (same as Redis, and this should be same) Changes in this PR: - implement *Valkey Over RDMA*. (compact the Valkey style) Finally, if this feature is considered to merge, I volunteer to maintain it. --------- Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Hi, @madolson |
Hi @madolson |
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.
My only comment is about being clear that it's experimental.
RDMA is the abbreviation of remote direct memory access. It is a technology that enables computers in a network to exchange data in the main memory without involving the processor, cache, or operating system of either computer. This means RDMA has a better performance than TCP, the test results show Valkey Over RDMA has a ~2.5X QPS and lower latency. In recent years, RDMA gets popular in the data center, especially RoCE(RDMA over Converged Ethernet) architecture has been widely used. Cloud Vendors also start to support RDMA instance in order to accelerate networking performance. End-user would enjoy the improvement easily. Introduce Valkey Over RDMA protocol as a new transport for Valkey. For now, we defined 4 commands: - GetServerFeature & SetClientFeature: the two commands are used to negotiate features for further extension. There is no feature definition in this version. Flow control and multi-buffer may be supported in the future, this needs feature negotiation. - Keepalive - RegisterXferMemory: the heart to transfer the real payload. The 'TX buffer' and 'RX buffer' are designed by RDMA remote memory with RDMA write/write with imm, it's similar to several mechanisms introduced by papers(but not same): - Socksdirect: datacenter sockets can be fast and compatible <https://dl.acm.org/doi/10.1145/3341302.3342071> - LITE Kernel RDMA Support for Datacenter Applications <https://dl.acm.org/doi/abs/10.1145/3132747.3132762> - FaRM: Fast Remote Memory <https://www.usenix.org/system/files/conference/nsdi14/nsdi14-paper-dragojevic.pdf> Thanks to Daniel House for review suggestions! Link: valkey-io/valkey#477 Co-authored-by: Xinhao Kong <xinhao.kong@duke.edu> Co-authored-by: Huaping Zhou <zhouhuaping.san@bytedance.com> Co-authored-by: zhuo jiang <jiangzhuo.cs@bytedance.com> Co-authored-by: Yiming Zhang <zhangyiming1201@bytedance.com> Co-authored-by: Jianxi Ye <jianxi.ye@bytedance.com> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Comments are addressed; docs for 8.0 are late
Sorry for the delay! |
RDMA is the abbreviation of remote direct memory access. It is a technology that enables computers in a network to exchange data in the main memory without involving the processor, cache, or operating system of either computer. This means RDMA has a better performance than TCP, the test results show Valkey Over RDMA has a ~2.5X QPS and lower latency.
In recent years, RDMA gets popular in the data center, especially RoCE(RDMA over Converged Ethernet) architecture has been widely used. Cloud Vendors also start to support RDMA instance in order to accelerate networking performance. End-user would enjoy the improvement easily.
Introduce Valkey Over RDMA protocol as a new transport for Valkey. For now, we defined 4 commands:
The 'TX buffer' and 'RX buffer' are designed by RDMA remote memory with RDMA write/write with imm, it's similar to several mechanisms introduced by papers(but not same):