Skip to content

Conversation

@pizhenwei
Copy link
Contributor

@pizhenwei pizhenwei commented Apr 16, 2025

Allow replicas to use MPTCP in the outgoing replication connection.

A new yes/no config is introduced repl-mptcp, default no.

For MPTCP to be used in replication, the primary needs to be configured with mptcp yes and the replica with repl-mptcp yes. Otherwise, the connection falls back to regular TCP.

Follow-up of #1811.

Cc Linux kernel MPTCP maintainer @matttbe @geliangtang @Dwyane-Yan
Cc @xbasel

@pizhenwei pizhenwei requested a review from zuiderkwast April 16, 2025 03:05
@codecov
Copy link

codecov bot commented Apr 16, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.05%. Comparing base (2399f10) to head (eb6d499).
Report is 24 commits behind head on unstable.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #1961      +/-   ##
============================================
+ Coverage     70.99%   71.05%   +0.06%     
============================================
  Files           123      123              
  Lines         65918    66040     +122     
============================================
+ Hits          46798    46925     +127     
+ Misses        19120    19115       -5     
Files with missing lines Coverage Δ
src/anet.c 72.70% <100.00%> (+0.23%) ⬆️
src/cluster_legacy.c 86.55% <100.00%> (+0.41%) ⬆️
src/config.c 78.43% <ø> (ø)
src/connection.h 87.77% <100.00%> (ø)
src/rdma.c 100.00% <ø> (ø)
src/replication.c 87.32% <ø> (ø)
src/server.h 100.00% <ø> (ø)
src/socket.c 94.25% <100.00%> (+1.14%) ⬆️
src/tls.c 100.00% <ø> (ø)

... and 30 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pizhenwei pizhenwei requested a review from xbasel April 16, 2025 09:17
@pizhenwei
Copy link
Contributor Author

@zuiderkwast @xbasel What do you think about the latest version?

@zuiderkwast
Copy link
Contributor

I haven't looked yet. We are working on patch releases now. I will take a look in a few days.

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.

I think this is mostly fine. I have only some minor comments.

Since commit 4a92db9("Introduce MPTCP (valkey-io#1811)"), valkey server
starts to support MPTCP. Support MPTCP for replica as client side.

Signed-off-by: zhenwei pi <[email protected]>
Signed-off-by: zhenwei pi <[email protected]>
Signed-off-by: zhenwei pi <[email protected]>
Signed-off-by: zhenwei pi <[email protected]>
@pizhenwei
Copy link
Contributor Author

Apply @zuiderkwast 's suggestions, force push a commit over 3 commits to fix DCO error.

@zuiderkwast zuiderkwast moved this to In Progress in Valkey 9.0 Apr 23, 2025
@zuiderkwast zuiderkwast added the release-notes This issue should get a line item in the release notes label Apr 23, 2025
@zuiderkwast
Copy link
Contributor

@valkey-io/core-team This is the follow-up of MTPCP to allow replication to use MPTCP. I think we already decided about this when we discussed and decided about MPTCP in the meeting some weeks ago. Please comment if you disagree.

@zuiderkwast zuiderkwast added the major-decision-approved Major decision approved by TSC team label Apr 23, 2025
@Neustradamus
Copy link

MPTCP is really good!

@zuiderkwast zuiderkwast merged commit c9c49b4 into valkey-io:unstable Apr 27, 2025
51 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Valkey 9.0 Apr 27, 2025
@pizhenwei pizhenwei deleted the repl-mptcp branch May 1, 2025 09:41
rainsupreme pushed a commit to rainsupreme/valkey that referenced this pull request May 14, 2025
Allow replicas to use MPTCP in the outgoing replication connection.

A new yes/no config is introduced `repl-mptcp`, default `no`.

For MPTCP to be used in replication, the primary needs to be configured
with `mptcp yes` and the replica with `repl-mptcp yes`. Otherwise, the
connection falls back to regular TCP.

Follow-up of valkey-io#1811.

---------

Signed-off-by: zhenwei pi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

major-decision-approved Major decision approved by TSC team release-notes This issue should get a line item in the release notes

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants