Skip to content
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

Merged
merged 1 commit into from
Oct 20, 2024
Merged

Conversation

pizhenwei
Copy link
Contributor

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):

Copy link
Contributor

@zuiderkwast zuiderkwast left a 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.

topics/RDMA.md Outdated Show resolved Hide resolved
topics/RDMA.md Outdated Show resolved Hide resolved
topics/RDMA.md Outdated Show resolved Hide resolved
topics/RDMA.md Outdated Show resolved Hide resolved
topics/RDMA.md Show resolved Hide resolved
topics/RDMA.md Outdated Show resolved Hide resolved
topics/RDMA.md Outdated Show resolved Hide resolved
topics/RDMA.md Outdated Show resolved Hide resolved
topics/index.md Outdated Show resolved Hide resolved
topics/index.md Outdated Show resolved Hide resolved
@zuiderkwast zuiderkwast added the waiting-for-upstream Docs for features not yet merged label May 30, 2024
@zuiderkwast
Copy link
Contributor

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):

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"

@pizhenwei pizhenwei force-pushed the RDMA branch 6 times, most recently from a84c13b to a8d9f66 Compare May 31, 2024 02:12
@pizhenwei
Copy link
Contributor Author

pizhenwei commented May 31, 2024

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):

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"

Fine, I take the background part and paper links to the RDMA.md in the new version. But I still remain these in the commit message because:

  • texts change in the RDMA.md in future, we can still find detail of the initial version. This is friendly to git blame xxx and git show xxx.
  • in theory, we must have a specification to define the ecosystem first, then implement the server&client on different OS. Only linking valkey-io/valkey#477 in this description seems less detailed.

So I prefer to keep the RDMA.md as detailed as possible.

Copy link
Contributor

@zuiderkwast zuiderkwast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@PingXie PingXie left a 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!

topics/RDMA.md Outdated Show resolved Hide resolved
topics/RDMA.md Outdated Show resolved Hide resolved
topics/RDMA.md Outdated Show resolved Hide resolved
topics/RDMA.md Outdated Show resolved Hide resolved
topics/RDMA.md Outdated Show resolved Hide resolved
topics/RDMA.md Outdated Show resolved Hide resolved
topics/RDMA.md Outdated Show resolved Hide resolved
topics/RDMA.md Outdated Show resolved Hide resolved
topics/RDMA.md Outdated Show resolved Hide resolved
topics/RDMA.md Outdated Show resolved Hide resolved
@pizhenwei pizhenwei force-pushed the RDMA branch 2 times, most recently from b1bd80a to 22a4abd Compare June 1, 2024 01:23
@pizhenwei
Copy link
Contributor Author

LGTM. All nits. Thanks @pizhenwei!

Thanks for your suggestions! Several changes in the new version as you suggested.

topics/RDMA.md Outdated Show resolved Hide resolved
topics/RDMA.md Outdated Show resolved Hide resolved
topics/RDMA.md Outdated Show resolved Hide resolved
topics/RDMA.md Show resolved Hide resolved
topics/RDMA.md Show resolved Hide resolved
topics/RDMA.md Show resolved Hide resolved
topics/RDMA.md Outdated Show resolved Hide resolved
topics/RDMA.md Show resolved Hide resolved
topics/RDMA.md Show resolved Hide resolved
@pizhenwei pizhenwei force-pushed the RDMA branch 3 times, most recently from 18f0625 to a0ae1ce Compare June 3, 2024 07:01
@pizhenwei
Copy link
Contributor Author

pizhenwei commented Jun 3, 2024

Hi @zuiderkwast @PingXie @madolson
The total changes between and the previous version is this URL.

@zuiderkwast
Copy link
Contributor

Hi @zuiderkwast @PingXie @madolson The total changes between and the previous version is this URL.

Thanks. In the future you can just add more commits in the same PR. I makes review easier. We squash-merge PRs anyway.

@pizhenwei
Copy link
Contributor Author

Hi @zuiderkwast @PingXie @madolson The total changes between and the previous version is this URL.

Hi @zuiderkwast @PingXie @madolson
I'm looking forward to seeing your suggestions for the next step.

@pizhenwei
Copy link
Contributor Author

Hi @zuiderkwast @PingXie @madolson The total changes between and the previous version is this URL.

Hi @zuiderkwast @PingXie @madolson I'm looking forward to seeing your suggestions for the next step.

@madolson PING!

@pizhenwei
Copy link
Contributor Author

Hi @zuiderkwast @PingXie @madolson The total changes between and the previous version is this URL.

Hi @zuiderkwast @PingXie @madolson I'm looking forward to seeing your suggestions for the next step.

@madolson PING!

Hi @madolson ,
Did I misunderstand anything? Please let me know to move forward.

@pizhenwei
Copy link
Contributor Author

Hi @zuiderkwast @PingXie @madolson The total changes between and the previous version is this URL.

Hi @zuiderkwast @PingXie @madolson I'm looking forward to seeing your suggestions for the next step.

@madolson PING!

Hi @madolson , Did I misunderstand anything? Please let me know to move forward.

@madolson PING!

@zuiderkwast
Copy link
Contributor

@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.

@pizhenwei pizhenwei force-pushed the RDMA branch 2 times, most recently from 44492c3 to f11bba4 Compare July 10, 2024 05:04
topics/RDMA.md Outdated Show resolved Hide resolved
@pizhenwei pizhenwei requested a review from madolson July 15, 2024 01:39
zuiderkwast pushed a commit to valkey-io/valkey that referenced this pull request Jul 15, 2024
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>
@pizhenwei
Copy link
Contributor Author

Hi, @madolson
would you please review this again?

@zuiderkwast zuiderkwast removed the waiting-for-upstream Docs for features not yet merged label Aug 14, 2024
@pizhenwei
Copy link
Contributor Author

Hi @madolson
Does anything blocks this PR? Please let me know.

Copy link
Contributor

@zuiderkwast zuiderkwast left a 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.

topics/RDMA.md Show resolved Hide resolved
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>
@zuiderkwast zuiderkwast dismissed madolson’s stale review October 20, 2024 21:13

Comments are addressed; docs for 8.0 are late

@zuiderkwast zuiderkwast merged commit e48583b into valkey-io:main Oct 20, 2024
2 checks passed
@zuiderkwast
Copy link
Contributor

Sorry for the delay!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants