Deprecate mplex #661
Deprecate mplex #661
Conversation
| [source](https://github.com/libp2p/rust-libp2p/blob/1c9b3ca355aecffa0bcf83d2495cd4cc1019425b/muxers/mplex/src/config.rs#L130) | ||
| - Block up to a certain amount of time, then reset the stream. | ||
| [source](https://github.com/libp2p/go-mplex/blob/ad9bfb922974b5875cc48c6e7492c4987c0cb94a/multiplex.go#L35-L37) | ||
| - Head of line blocking between streams |
There was a problem hiding this comment.
As long as you use TCP, there is no way to resolve this, right? So I think it's a good idea to note that only using QUIC will resolve this and yamux won't.
There was a problem hiding this comment.
You're right, you can only really solve head of line blocking of packets with QUIC. However, there's another head of line blocking issue that I'm referencing here, that Yamux can avoid due to stream flow control.
Say you have two streams, and one of them writes a 10 MB payload, and the other one then writes a 1 KB message.
In Yamux, you would send the first 256KB of the first stream, then the next 1KB of the other message. You would not send any more data on the first stream until you receive a window update message.
In mplex, you would try to send the full 10MB payload before you make any progress on the 1KB payload.
There was a problem hiding this comment.
This is just stream level flow control, no? You're mentioning the initial stream flow credits of 256kB.
There was a problem hiding this comment.
Would you rather put this as a sub-bullet under - No stream-level flow control. ?
| [source](https://github.com/libp2p/rust-libp2p/blob/1c9b3ca355aecffa0bcf83d2495cd4cc1019425b/muxers/mplex/src/config.rs#L130) | ||
| - Block up to a certain amount of time, then reset the stream. | ||
| [source](https://github.com/libp2p/go-mplex/blob/ad9bfb922974b5875cc48c6e7492c4987c0cb94a/multiplex.go#L35-L37) | ||
| - Head of line blocking between streams |
There was a problem hiding this comment.
This is just stream level flow control, no? You're mentioning the initial stream flow credits of 256kB.
| - No way of propagating errors. | ||
| - Errors that could explain why a peer reset the stream. | ||
| - No way to signal to a peer that you will not read any more data (e.g. QUIC's | ||
| STOP_SENDING frame). | ||
| - Both sides can open streams with the same ID, differing only by who opened the | ||
| stream. which may lead to confusion. | ||
| - stream names are relatively unused (go-libp2p does not use them. I don't think rust-libp2p or js-libp2p uses them either) |
There was a problem hiding this comment.
Aren't all of these fixable? We're deprecating this because of the lack of stream flow control.
There was a problem hiding this comment.
Fixing these would require changing the mplex protocol spec. These points are about where it stands today.
mplex/README.md
Outdated
| mplex does not support stream-level flow control, preventing receivers from | ||
| applying backpressure. This leads issues varying from easy to exploit DoS | ||
| vulnerabilities due to unbounded sender behavior to hard to debug application | ||
| stalls caused by slow a single slow stream receiver. |
There was a problem hiding this comment.
| mplex does not support stream-level flow control, preventing receivers from | |
| applying backpressure. This leads issues varying from easy to exploit DoS | |
| vulnerabilities due to unbounded sender behavior to hard to debug application | |
| stalls caused by slow a single slow stream receiver. | |
| mplex does not support stream-level flow control, preventing receivers from | |
| applying backpressure. This leads to issues varying from easy to exploit DoS | |
| vulnerabilities due to unbounded sender behavior to hard to debug application | |
| stalls caused by a single slow stream receiver. |
MarcoPolo
force-pushed
the
marco/deprecate-mplex
branch
from
1b7fb74 to
17f4088
Compare
We've talked a lot about deprecating mplex for many reasons, but I don't think we've done a good job of documenting the reasons. A lot of the reasons are spread out in various issues, repos, and docs. My goal here is to summarize the reasoning and document why we want to deprecate mplex.
Related to #553