Skip to content

Fix data-channels-flow-control example #846

Closed

Description

@hugoArregui

There is a problem with the current example, the peer may stop sending data here if the max buffer size has not been reached before this point and since the callback is executed as part of the stream processing thread, that will block everything.

So we need to:

  1. Find a better flow control mechanism for the example
  2. Decide if we should execute the callback in another goroutine so we ensure we don't block the stream, or, document the fact that users should make sure their code don't block.

As a suggestion for (1) I wrote this code: https://github.com/pion/webrtc/blob/topic-test-dc-perf/examples/data-channels-flow-control/main.go#L31 which uses a queue and drains it whenever it's possible. It might not the most efficient way having a channel in the middle of the write operation but I like how easy is to reason about it. I think it makes sense for an example.

Regarding (2) I think we should probably execute the callback in another goroutine since that's consistent with do things in other places, but I have no strong opinion on this matter.

Activity

  1. hugoArregui commented on Sep 25, 2019

    @hugoArregui
    MemberAuthor
  2. AeroNotix commented on Sep 25, 2019

    @AeroNotix
    Contributor

    How have you tested the example (suggestion 1)? I have tried it myself and found a few issues. I am still isolating my own code from that example but I would like to see how you tested it to verify for correctness?

  3. hugoArregui commented on Sep 26, 2019

    @hugoArregui
    MemberAuthor

    @AeroNotix I ran it with a 1.5gb file (uncommenting https://github.com/pion/webrtc/blob/topic-test-dc-perf/examples/data-channels-flow-control/main.go#L138 and adding a loop when I wanted more data) and different combinations of max buffer size / queue size / threshold. In this particular case please notice the channel is unreliable (https://github.com/pion/webrtc/blob/topic-test-dc-perf/examples/data-channels-flow-control/main.go#L120), so if you want to try this very same example you may want to set some max retransmits.

  4. AeroNotix commented on Sep 26, 2019

    @AeroNotix
    Contributor

    It would be interesting to have a runnable example, alone, that you specify what parameters it is supposed to exhibit - then I run that - and I can confirm or deny that it exhibits the same or similar performance.

    Without that we are pissing into the dark, with the wind at our face.

  5. AeroNotix commented on Sep 26, 2019

    @AeroNotix
    Contributor

    E.g. I would be interested in these metrics, from your example:

    • How did it perform when running between remote peers. Running networking benchmarks against localhost is fine to get a baseline for potential performance but running against remote peers can allow to find timing issues, or bottlenecks that localhost<>localhost transfers may not expose.
    • When running between remote peers - is all data actually transferred?
  6. added this to the milestone on Jan 23, 2022
  7. removed this from the milestone on May 22, 2022
  8. added 2 commits that reference this issue on Sep 11, 2025
    d42baea
    50d03bf
  9. added a commit that references this issue on Sep 11, 2025
    634a904
  10. added a commit that references this issue on Sep 13, 2025
    474c3d0
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

      ⌂ 换网址