-
Notifications
You must be signed in to change notification settings - Fork 79
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
Turn Stream.WriteSCTP() and Write() a blocking call by default #77
Comments
- The SCTP library doesn't block Write calls (pion/sctp#77), so implement that ourselves - We should hold a lock while chunking up a write. A caller might do concurrent calls and expect them to be sent in either order, but not interleaved.
@enobufs I'm wondering more and more if this should be the strong default. I'm still trying to understand if there's anything SCTP can do better to avoid the issues people see when too much data is sent over the network and we get stuck in RTX, but I wonder if this is the best avoidance solution in addition to whatever fixes may be inside the sctp impl. |
Even after 5 years, I massively agree that the default should be to block when there is too much buffered data in-flight. |
I still have the same feeling, even more strongly also! |
Should we make it part of pion/webrtc@v4 ? We can start a v2 now! |
Do we think a blocking write would be a write that has made it to inflight and that there's no longer a pending queue? That could simplify/remove some code. and yeah let's do it as a v2! We ought to look at https://datatracker.ietf.org/doc/html/rfc9260 and understand what we are missing from the latest spec in addition to 4960. I believe there are some omissions that fit into v1/2 that could improve reliability and perf. |
@enobufs has also had ideas for RACK. This is a good chance to redo those parts that you have been debugging! |
Exactly. |
Your environment.
Background
WebRTC's datachannel.Write() does not block as we follow JavaScript WebRTC API. When sending a large amount of data, it is the application's responsibility to check the buffered amount (in SCTP layer for sending).
This is pretty standard in JavaScript land, but this really does not align with a typical semantics of Go. (i.e. net.Conn). Also, implementing a flow control at the user level IS tedious and not too trivial to get it right and error-prone.
What would you expect?
We, I believe, should keep the current behavior with pion/webrtc API (maintaining JavaScript API semantics as a policy), but we could make some exceptions to it in the following cases:
In these cases, blocking Write() method can be the default behavior, and turn off the blocking when used with pion/webrtc non-detached, etc.
What others thought
@backkem
@AeroNotix
I totally agree with the above comments, and I think SCTP layer should take care of this.
The text was updated successfully, but these errors were encountered: