Skip to content

Turn Stream.WriteSCTP() and Write() a blocking call by default #77

Open

Description

@enobufs

Your environment.

  • Version: v1.7.0 (or earlier)

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:

  • When data channel is detached
  • When pion/datachannel or pion/sctp was used directly

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

What if we add a default implementation to SCTP itself? E.g. default threshold and if you pass it stream.Write starts blocking.
If you overwrite, you're on your own.
...
Yea so, if you use SCTP or DataChannel directly -> Default blocking implementation (blocking by default seems rather idiomatic).
If you use WebRTC -> We overwrite the OnBufferedAmountLow and it disables the default implementation and otherwise confirms to the WebRTC spec. 

@AeroNotix

that default implementation will suit 99% of people's needs
"block if exceed buffer size"

I totally agree with the above comments, and I think SCTP layer should take care of this.

Activity

  1. added a commit that references this issue on Jun 28, 2021
  2. edaniels commented on Feb 22, 2024

    @edaniels
    Member

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

  3. AeroNotix commented on Feb 22, 2024

    @AeroNotix
    Contributor

    Even after 5 years, I massively agree that the default should be to block when there is too much buffered data in-flight.

  4. enobufs commented on Feb 24, 2024

    @enobufs
    MemberAuthor

    I still have the same feeling, even more strongly also!
    When we do blocking write, I prefer ditching the 'buffered amount' flow control mechanism (it's for javascript and it's been the source of confusion for Go programmers!) from supporting both modes. (thoughts?)
    Let us plan for pion/sctp v2.

  5. Sean-Der commented on Feb 24, 2024

    @Sean-Der
    Member

    Should we make it part of pion/webrtc@v4 ? We can start a v2 now!

  6. edaniels commented on Feb 24, 2024

    @edaniels
    Member

    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.

  7. Sean-Der commented on Feb 24, 2024

    @Sean-Der
    Member

    @enobufs has also had ideas for RACK.

    This is a good chance to redo those parts that you have been debugging!

  8. enobufs commented on Feb 24, 2024

    @enobufs
    MemberAuthor

    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.

    Exactly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions

      ⌂ 换网址