w3c / webrtc-nv-use-cases

Use cases for WebRTC NV
https://w3c.github.io/webrtc-nv-use-cases/
Other
32 stars 13 forks source link

(How) does SCTP handle CPU-bound congestion on JavaScript thread? #30

Closed henbos closed 5 years ago

henbos commented 5 years ago

How is the JavaScript application supposed to be able to apply back pressure? CC @marinaciocea

The spec says:

When an RTCDataChannel message has been received via the underlying data transport with type type and data rawData, the user agent MUST queue a task to run the following steps:

[Convert data to the appropriate JS object and fire "onmessage".]

I don't see any feedback mechanism to tell the underlying data transport that we're actually done consuming the message. It looks like the underlying transport is free to queue tasks ad-infinitum to fire messages. The message is just taken out of the SCTP buffer and put on a PostTask to another thread. So SCTP should be free to receive more messages, whether or not JavaScript can keep up.

What if that other thread (JavaScript) is really slow at processing "onmessage"? E.g. I'm being sent 10000 numbers p*q per second and I spend 1 second per message calculating the values of p and q. Will the task queue get overflooded?

Am I missing something?

lgrahl commented 5 years ago

You're not missing anything. There is no mechanism for that (and FWIW the same applies to WebSockets).

1732 will fix this issue.

aboba commented 5 years ago

This is a design problem with the eventing model of WebSockets/RTCDataChannel. A better approach (if events were used at all) would have been to event the availability of data, then let the application retrieve the data all at once, as a sequence of messages. That way the buildup of received (and unprocessed) data would apply backpressure. Plus the transfer of data could avoid copies.

henbos commented 5 years ago

It sounds like it would be easy to address this by introducing a mode to say "don't auto-empty the buffer" and a method "popMessage()".

lgrahl commented 5 years ago

This could be a slight improvement for WebSockets but it would not work for SCTP-based data channels. SCTP has only one receive buffer per association. Thus, you would risk that one data channel filling up (for example a low priority file transfer or, even worse, a channel that is being completely ignored and never bound by the application) blocks receiving messages from another data channel (for example real-time sensor data). Furthermore, doing this on a message-based level is insufficient since messages can be of arbitrary size (although Chrome refuses to accept that messages can be larger than 64 KiB :smirk:). If one side sends a 10 MiB message then you still have to buffer 10 MiB. The same applies to 100 MiB, and so on. That breaks the backpressure mechanism.

So, we not only need an API that allows to receive parts of a message (#1732) but also find a solution for the one receive buffer per association problem which requires some protocol work. I have some ideas how it could be done by using PPIDs to signal backpressure.

murillo128 commented 5 years ago

I agree with @lgrahl that a per-message api is needed, so we are able to receive messages of and arbitrary size. and avoid buffering of the sctp chunks before delivering it to the app.

Not sure if I agree on the one receive buffer per association, @lgrahl can you elaborate? Shouldn't interleave mode solve this?

murillo128 commented 5 years ago

BTW regarding backpressure, couldn't this be done by the app itself sending back an "ack" messages in another datachannel? Don't think we need an API and protocol change for that.

El vie., 25 ene. 2019 16:23, Lennart Grahl notifications@github.com escribió:

This could be a slight improvement for WebSockets but it would not work for SCTP-based data channels. SCTP has only one receive buffer per association. Thus, you would risk that one data channel filling up (for example a low priority file transfer or, even worse, a channel that is being completely ignored and never bound by the application) blocks receiving messages from another data channel (for example real-time sensor data). Furthermore, doing this on a message-based level is insufficient since messages can be of arbitrary size (although Chrome refuses to accept that messages can be larger than 64 KiB 😛). If one side sends a 10 MiB message then you still have to buffer 10 MiB. The same applies to 100 MiB, and so on. That breaks the backpressure mechanism.

So, we not only need an API that allows to receive parts of a message (

1732 https://github.com/w3c/webrtc-pc/issues/1732) but also find a

solution for the one receive buffer per association problem which requires some protocol work. I have some ideas how it could be done by using PPIDs to signal backpressure.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/w3c/webrtc-pc/issues/2086#issuecomment-457607907, or mute the thread https://github.com/notifications/unsubscribe-auth/ABBW8yO6vvUiT3ojELvlScc-zhVG3R_wks5vGyFwgaJpZM4aS7_J .

tuexen commented 5 years ago

If the JS layer doesn't consume the message from the SCTP stack, SCTP flow control will kick in. SCTP has a limited buffer and this is honored by the peer... The flow control is not per data channel, but per peer connection.

aboba commented 5 years ago

Per data channel flow control won't necessarily solve the problem if there is also association flow control. We are learning this in QUIC where flow control of a file transfer on one stream can impact real-time data on another stream (or potentially DATAGRAM traffic as well). If the application can't keep up on one stream then that stream can be flow controlled but this also may consume much of the per-association buffer limit, resulting in flow control on other streams even if their per-stream limit is not reached. But the alternative - filling the event queue with file transfer messages - also affects the timeliness of delivery of real time data, and potentially in a worse manner if there is no back pressure on the file transfer.

murillo128 commented 5 years ago

wouldn't that be a different issue? SCTP already has a congestion control (better or worse) that provides back pressure on sender side, causing buffering on sender.

El vie., 25 ene. 2019 18:34, Bernard Aboba notifications@github.com escribió:

Per data channel flow control won't necessarily solve the problem if there is also association flow control. We are learning this in QUIC where flow control of a file transfer on one stream can impact real-time data on another stream (or potentially DATAGRAM traffic as well). If the application can't keep up on one stream then that stream can be flow controlled but this also may consume much of the per-association buffer limit, resulting in flow control on other streams even if their per-stream limit is not reached. But the alternative - filling the event queue with file transfer messages - also affects the timeliness of delivery of real time data, and potentially in a worse manner if there is no back pressure on the file transfer.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/w3c/webrtc-pc/issues/2086#issuecomment-457652220, or mute the thread https://github.com/notifications/unsubscribe-auth/ABBW8yXF0vnffEJuU7bGemqOocSHM60Tks5vG0AlgaJpZM4aS7_J .

aboba commented 5 years ago

@murillo128 @lgrahl Was just pointing out that per-data channel flow control will not completely solve the problem of side-effects from an application not keeping up on another data channel. Today implementations don't limit the message events in the event queue, and the message is considered "consumed" once it is put into the event queue, so flow control does not kick in.

murillo128 commented 5 years ago

So it is an implementation problem, not a spec problem.

SCTP allows to signal back the a_rwnd in tha SACK feedback message:

   Each SACK an endpoint receives contains an a_rwnd value.  This value
   represents the amount of buffer space the data receiver, at the time
   of transmitting the SACK, has left of its total receive buffer space
   (as specified in the INIT/INIT ACK).  Using a_rwnd, Cumulative TSN
   Ack, and Gap Ack Blocks, the data sender can develop a representation
   of the peer's receive buffer space.

and

      As DATA chunks are delivered to the ULP and released from the
      receive buffers, increment a_rwnd by the number of bytes delivered
      to the upper layer.  This is, in effect, opening up rwnd on the
      data sender and allowing it to send more data.  The data receiver
      SHOULD NOT increment a_rwnd unless it has released bytes from its
      receive buffer.  For example, if the receiver is holding
      fragmented DATA chunks in a reassembly queue, it should not
      increment a_rwnd.

The issue is that we are considering ULP to be the datachannel protocol, and not the js application. If the a_rwnd is not decremented when the data chunk are delivered to the datachannel layer for reassembling, but when the event is actually deliver to the js application, we have the back-pressure mechanism that will solve the issue you are describing.

This however imposes a hard limit of 4GB of the amount of buffer that can be "buffered" by the browser (and on individual message length). I think that is reasonable, as to be able to handle messages bigger than that we should allow a more fine grained API to be able to deliver partial data to the js layer of each message as the sctp data chunks are received (maybe with a bit of buffering for ordering).

henbos commented 5 years ago

The spec is not explicit about when the receive buffer is emptied, but there is no syncing with the JavaScript thread unless we make that explicit, so I do believe this is more than an implementation problem. Could the spec say something like this instead?

When an RTCDataChannel message has been received via the underlying data transport with type type and data rawData, the user agent MUST queue a task to run the following steps (note: at this point, the receive buffer of the underlying data channel has not been decreased for rawData):

  • In parallel...
    • Decrement the receive buffer of the underlying data transport for the size of rawData.
  • [Convert data to the appropriate JS object and fire "onmessage" (same steps as before).]
tuexen commented 5 years ago

Most implementations right now have some buffer space within the JS layer. When SCTP passes user data to its upper layer (the JS layer) it is not buffered by SCTP anymore and therefore the buffer space is considered freed. You would have to avoid the buffering in the JS layer...

murillo128 commented 5 years ago

@henbos if we need to make it explicit, something like that would work for me. We could also specify (in the IETF datachannel draft maybe) that the ULP of the sctp layer is not the datachannel layer, but the javascript app.

lgrahl commented 5 years ago

Shouldn't interleave mode solve this?

It doesn't add per-stream send/receive buffers.

This however imposes a hard limit of 4GB of the amount of buffer that can be "buffered" by the browser (and on individual message length).

IIRC that 4 GiB hard limit would also be shared across all data channels, so the actual limit per message is not predictable. Also, this is hypothetical since all browsers use usrsctp which uses a fixed size buffer (which obviously is much lower than 4 GiB). As @tuexen explained, you either take the data out and usrsctp considers the buffer space free'd or you don't and the flow control gets you which leads to the problem of one channel blocking another. Sure, it's an implementation issue... but even if one had a signalling mechanism for the receive window and it would follow the mechanism as described by @henbos, if that implementation uses a fixed receive buffer for the SCTP association it would come to a halt in case one incoming message surpasses that buffer's size.

lgrahl commented 5 years ago

@aboba If I understand you correctly then what you're saying is that for an ideal world we'd need both: A flow control that doesn't ignore the JS application and a per-channel flow control. Since we already have a priority mechanism with SCTP ndata, I'd agree.

aboba commented 5 years ago

@lgrahl Yes. To avoid "buffer bloat", implementations probably should limit buffering of received data in the message queue on a per-data channel basis. If implemented well, ndata could probably help avoid starvation in situations where an application can't keep up on one data channel.

murillo128 commented 5 years ago

IIRC that 4 GiB hard limit would also be shared across all data channels, so the actual limit per message is not predictable. Also, this is hypothetical since all browsers use usrsctp which uses a fixed size buffer (which obviously is much lower than 4 GiB).

Yes, ot is shared between all the datachannels, and actual limits will be much lower, so spec wise it is good. To increase the actual message limits wr would need to change the API to deliver individual chunks of a datachannel message to the app as soon as they are received and dont buffer it.

As @tuexen explained, you either take the data out and usrsctp considers the buffer space free'd or you don't and the flow control gets you which leads to the problem of one channel blocking another. Sure, it's an implementation issue... but even if one had a signalling mechanism for the receive window and it would follow the mechanism as described by @henbos, if that implementation uses a fixed receive buffer for the SCTP association it would come to a halt in case one incoming message surpasses that buffer's size.

that's an implementation issue, either modify usersctp to support it or use anothet datachannel implementation.

lgrahl commented 5 years ago

that's an implementation issue

I said that. :slightly_smiling_face: However, ...

either modify usersctp to support it or use anothet datachannel implementation.

But you do realise that this is pretty unrealistic? Announcing a 4 GiB receive window is essentially lying if the buffer is dynamic. And it is even more unrealistic in the kernel.

murillo128 commented 5 years ago

What kernel?

All I am saying is that the 4GB limit in the spec is enough and we don't need another mechanism on the wire protocol for that, as the bottleneck is in another place.

As I said before, an API for delivering data to the app as soon as it is received on the sctp chunks, ndata for interleaving and the change to the a_rnwd handling would solve all the issues.

For the specific topic of this issue, the a_rnwd change is enough.

El sáb., 26 ene. 2019 9:18, Lennart Grahl notifications@github.com escribió:

that's an implementation issue

I said that. 🙂 However, ...

either modify usersctp to support it or use anothet datachannel implementation.

But you do realise that this is pretty unrealistic? Announcing a 4 GiB receive window is essentially lying if the buffer is dynamic. And it is even more unrealistic in the kernel.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/w3c/webrtc-pc/issues/2086#issuecomment-457812740, or mute the thread https://github.com/notifications/unsubscribe-auth/ABBW87j8HuDCOzdB3U0ZBBEr0dMbm8Eiks5vHA9JgaJpZM4aS7_J .

lgrahl commented 5 years ago

an API for delivering data to the app as soon as it is received on the sctp chunks, ndata for interleaving and the change to the a_rnwd handling would solve all the issues.

Even if we had all of that I'd disagree. The receive buffer is message and stream agnostic. As long as streams are sharing this buffer, one stream not being read from would eventually bring all other streams to a standstill.

We can happily discuss that further but I think we're slowly drifting off-topic. IIRC the original question of @henbos kind of went into the direction whether there is an easy fix for the current 1.0 API... and at this point I don't think there is one.

murillo128 commented 5 years ago

The solution proposed by @henbos would work api wise for the given problem.

El sáb., 26 ene. 2019 10:31, Lennart Grahl notifications@github.com escribió:

an API for delivering data to the app as soon as it is received on the sctp chunks, ndata for interleaving and the change to the a_rnwd handling would solve all the issues.

Even if we had all of that I'd disagree. The receive buffer is message and stream agnostic. As long as streams are sharing this buffer, one stream not being read from would eventually bring all other streams to a standstill.

We can happily discuss that further but I think we're slowly drifting off-topic. IIRC the original question of @henbos https://github.com/henbos kind of went into the direction whether there is an easy fix for the current 1.0 API... and at this point I don't think there is one.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/w3c/webrtc-pc/issues/2086#issuecomment-457816843, or mute the thread https://github.com/notifications/unsubscribe-auth/ABBW81cd_MFKTTQ5pzhxrmf-s9pNnMs6ks5vHCCKgaJpZM4aS7_J .

lgrahl commented 5 years ago

Please explain how it would prevent the receive buffer from filling up when using multiple channels and not popping from one of them.

murillo128 commented 5 years ago

the issue is not about preventing the receive buffers to fill up, but to prevent the datachannel stack to enque events in the js loop faster than they can be dispatched to the js app.

If the a_rcwnd is not freed until the event is actually delivered to the js app, it will make the sctp buffer to fill up and the sender will not push more data. Once the data is delivered, the new a_rcwnd will be transmitted back to the sender and it will start sending data again.

That way the server will pace its sending to the amount of data that can be consumed by the js app.

If it is from a single datachannel or multiple ones is an orthogonal issue.

El sáb., 26 ene. 2019 12:11, Lennart Grahl notifications@github.com escribió:

Please explain how it would prevent the receive buffer from filling up when using multiple channels.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/w3c/webrtc-pc/issues/2086#issuecomment-457822769, or mute the thread https://github.com/notifications/unsubscribe-auth/ABBW84xCGKnJ0qtPevgSonuYXzYAIR6hks5vHDfogaJpZM4aS7_J .

henbos commented 5 years ago

Whenever there are multiple streams of traffic on the "same wire" there will be competition, some packets will get dropped and some packets will arrive. @lgrahl Why do you think that if the buffer is shared one data channel would end up dominating it? (How does SCTP prioritize different channels?) Is your concern in general use cases, or is this only a concern in edge cases such as if one data channel is sending messages the size of the entire buffer, leaving no room for other messages, and SCTP prioritizing the same channel every time? (Follow-up question: If there is a legitimate concern, is there no good way for the application to get around it?)

The fix I'm proposing does not change how SCTP work but it does provide a feedback mechanism between the JS thread and the network thread. JS will still have "onmessage" fire in accordance to SCTP receiving messages, it will however ensure that the buffer does not get emptied faster than it is processed by the application.

murillo128 commented 5 years ago

the issue that @lgrahl is highlighting is an original one, IMO. With current api, data is buffered by the dc stack until all the chuncks are received.

So if ndata is in use, it may happen that non completed messages may drain the receive buffer, stalling the sender.

But, again, that's a different problem that requires a different solution. Maybe a new issue should be opened for not mixing both discussions?

El sáb., 26 ene. 2019 12:45, henbos notifications@github.com escribió:

Whenever there are multiple streams of traffic on the "same wire" there will be competition, some packets will get dropped and some packets will arrive. @lgrahl https://github.com/lgrahl Why do you think that if the buffer is shared one data channel would end up dominating it? (How does SCTP prioritize different channels?) Is your concern in general use cases, or is this only a concern in edge cases such as if one data channel is sending messages the size of the entire buffer, leaving no room for other messages, and SCTP prioritizing the same channel every time? (Follow-up question: If there is a legitimate concern, is there no good way for the application to get around it?)

The fix I'm proposing does not change how SCTP work but it does provide a feedback mechanism between the JS thread and the network thread. JS will still have "onmessage" fire in accordance to SCTP receiving messages, it will however ensure that the buffer does not get emptied faster than it is processed by the application.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/w3c/webrtc-pc/issues/2086#issuecomment-457824741, or mute the thread https://github.com/notifications/unsubscribe-auth/ABBW83CZWihS4nx22U5oxLLiqSuWoj9qks5vHD_lgaJpZM4aS7_J .

henbos commented 5 years ago

Are these the two issues or am I confused?

  1. (This issue) How to get SCTP congestion to kick in when you are CPU bound on the JavaScript thread (with proposal earlier). This is about the receive-side.
  2. (Unrelated to this issue) How to prioritize data channels when the send-side buffer is full so that one data channel does not end up dominating the send buffer. E.g. one channel is sending lots of tiny messages so that the buffer is always full, making it impossible for another channel to send its large messages because the buffer never has sufficient space.
murillo128 commented 5 years ago

they're two different issues IMO too

El sáb., 26 ene. 2019 13:02, henbos notifications@github.com escribió:

Are these the two issues or am I confused?

  1. (This issue) How to get SCTP congestion to kick in when you are CPU bound on the JavaScript thread (with proposal earlier https://github.com/w3c/webrtc-pc/issues/2086#issuecomment-457726283).
  2. (Unrelated to this issue) How to prioritize data channels when the send-side buffer is full so that one data channel does not end up dominating the send buffer. E.g. one channel is sending lots of tiny messages so that the buffer is always full, making it impossible for another channel to send its large messages because the buffer never has sufficient space.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/w3c/webrtc-pc/issues/2086#issuecomment-457825656, or mute the thread https://github.com/notifications/unsubscribe-auth/ABBW8_ett7Y6tMrg89moVuK0y7QTGSM6ks5vHEPogaJpZM4aS7_J .

henbos commented 5 years ago

We can file separate issues in that case, I just wanted to make sure we're on the same page about what the issues being discussed are. Did I get 2) right @lgrahl ?

lgrahl commented 5 years ago

The issue is that there are N potential streams sharing 1 single receive buffer. Let the receive buffer/window W be 1 MiB. Let there be streams A and B where B is, for some reason, not being read from. The sender sends 1 MiB to stream B and then to stream A. Also, terminology here is that it's a stream on SCTP level and a channel on JS level.

With the current API, this would happen:

  1. Chunks of size S for stream B would be read from W into a separate reassembly buffer allocated by the browser for channel B. This frees the amount of data from W.
  2. All chunks of the message have been received, the message is being reassembled and bubbled via onmessage on channel B.
  3. No one listens on B but it doesn't matter.
  4. The same happens for stream/channel A.

With the proposed change, this would happen:

  1. Chunks of size S for stream B would be received and a) either copied from W into a separate reassembly buffer allocated by the browser for channel B, or b) just left in W until the message is complete. In both cases this does not free the amount of data from W.
  2. All chunks of the message have been received, the message is being reassembled and bubbled via onmessage on channel B.
  3. No one listens on B, thus W is now clogged as it only has a capacity of 1 MiB.
  4. The message on stream/channel A cannot be received.

Same example, but now B is being read from and we send 2 MiB sized messages instead of just 1 MiB.

With the current API, nothing changes compared to the above example.

With the proposed change, this would happen:

  1. Chunks of size S for stream B would be received and a) either copied from W into a separate reassembly buffer allocated by the browser for channel B, or b) just left in W until the message is complete. In both cases this does not free the amount of data from W.
  2. This continues until W has reached its capacity (1 MiB). But the message is of size 2 MiB, so W is clogged and the association stalls.

@henbos I'm trying to make a point that one receive buffer per association is a problem for 1). I haven't really talked about the sender side 2) but IIRC ndata does indeed solve the priorisation issue (by the use of stream schedulers), so I don't think we need to file an issue for that. It may have gotten a little confusing with all the discussion if it would make a difference with a different API, ndata & co.

murillo128 commented 5 years ago

can you open a different issue for that? that is definitively a different one

El sáb., 26 ene. 2019 13:33, Lennart Grahl notifications@github.com escribió:

The issue is that there are N potential streams sharing 1 single receive buffer. Let the receive buffer/window W be 1 MiB. Let there be streams A and B where B is, for some reason, not being read from. The sender sends 1 MiB to stream B and then to stream A. Also, terminology here is that it's a stream on SCTP level and a channel on JS level.

With the current API, this would happen:

  1. Chunks of size S for stream B would be read from W into a separate reassembly buffer allocated by the browser for channel B. This frees the amount of data from W.
  2. All chunks of the message have been received, the message is being reassembled and bubbled via onmessage on channel B.
  3. No one listens on B but it doesn't matter.
  4. The same happens for stream/channel A.

With the proposed change, this would happen:

  1. Chunks of size S for stream B would be received and a) either copied from W into a separate reassembly buffer allocated by the browser for channel B, or b) just left in W until the message is complete. In both cases this does not free the amount of data from W.
  2. All chunks of the message have been received, the message is being reassembled and bubbled via onmessage on channel B.
  3. No one listens on B, thus W is now clogged as it only has a capacity of 1 MiB.
  4. The message on stream/channel A cannot be received.

Same example, but now B is being read from and we send 2 MiB sized messages instead of just 1 MiB.

With the current API, nothing changes compared to the above example.

With the proposed change, this would happen:

  1. Chunks of size S for stream B would be received and a) either copied from W into a separate reassembly buffer allocated by the browser for channel B, or b) just left in W until the message is complete. In both cases this does not free the amount of data from W.
  2. This continues until W has reached its capacity (1 MiB). But the message is of size 2 MiB, so W is clogged and the association stalls.

@henbos https://github.com/henbos I'm trying to make a point that one receive buffer per association is a problem for 1). I haven't really talked about the sender side 2) but IIRC ndata does indeed solve the priorisation issue (by the use of stream schedulers), so I don't think we need to file an issue for that. It may have gotten a little confusing with all the discussion if it would make a difference with a different API, ndata & co.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/w3c/webrtc-pc/issues/2086#issuecomment-457827612, or mute the thread https://github.com/notifications/unsubscribe-auth/ABBW87XKZ3XukXPzxyqVHaCZrQL792XUks5vHEsIgaJpZM4aS7_J .

lgrahl commented 5 years ago

Then please, both of you, tell me what you believe this issue is about:

  1. Feeding back a signal to the SCTP stack to let the flow control kick in and stop receiving data due to CPU load on the JS thread?
  2. Feeding back a signal to whatever thread is handling the SCTP stack to stop delivering messages to the JS thread? (No, this does not allow the flow control to kick in or it would be equivalent to 1.)
murillo128 commented 5 years ago

the issue is sending back a signal to the sender so it doesn't send more data until the event queue has been processed and the data delivered to the js app.

El sáb., 26 ene. 2019 14:15, Lennart Grahl notifications@github.com escribió:

Then please, both of you, tell me what you believe this issue is about:

  1. Feeding back a signal to the SCTP stack to let the flow control kick in and stop receiving data due to CPU load on the JS thread?
  2. Feeding back a signal to whatever thread is handling the SCTP stack to stop delivering messages to the JS thread? (No, this does not allow the flow control to kick in or it would be equivalent to 1.)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/w3c/webrtc-pc/issues/2086#issuecomment-457830261, or mute the thread https://github.com/notifications/unsubscribe-auth/ABBW8_LlMNinD-isbDtxzImsV3htJBDCks5vHFTlgaJpZM4aS7_J .

lgrahl commented 5 years ago

Then we're talking about the same thing. Please, explain where my example is incorrect.

murillo128 commented 5 years ago

your issue is a valid one but it's different than the current one even If the solutions may be similar, that's why we propose you to open a different issue to solve it.

El sáb., 26 ene. 2019 14:41, Lennart Grahl notifications@github.com escribió:

Then we're talking about the same thing. Please, explain where my example is incorrect.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/w3c/webrtc-pc/issues/2086#issuecomment-457832013, or mute the thread https://github.com/notifications/unsubscribe-auth/ABBW8x_SkPlj-lqTiPGggkqJIWbfKhwpks5vHFrwgaJpZM4aS7_J .

murillo128 commented 5 years ago

let's put it in a different view. Taking into account just one single datachannel will the proposed solution solve the issue?

Also, does the problem about multichannel buffering still exist without taking into account the event loop queueing issue?

you may have a solution for not problems simultaneously, but it also requires a change in w3c apis and ietf specs.

To solve this issue we only need one line in the spec without api change.

El sáb., 26 ene. 2019 14:52, Sergio Garcia Murillo < sergio.garcia.murillo@gmail.com> escribió:

your issue is a valid one but it's different than the current one even If the solutions may be similar, that's why we propose you to open a different issue to solve it.

El sáb., 26 ene. 2019 14:41, Lennart Grahl notifications@github.com escribió:

Then we're talking about the same thing. Please, explain where my example is incorrect.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/w3c/webrtc-pc/issues/2086#issuecomment-457832013, or mute the thread https://github.com/notifications/unsubscribe-auth/ABBW8x_SkPlj-lqTiPGggkqJIWbfKhwpks5vHFrwgaJpZM4aS7_J .

lgrahl commented 5 years ago

let's put it in a different view. Taking into account just one single datachannel will the proposed solution solve the issue?

No, not with the message-based API that we have. My second example in https://github.com/w3c/webrtc-pc/issues/2086#issuecomment-457827612 (2 MiB message, 1 MiB receive buffer) would still apply for just a single channel.

Also, does the problem about multichannel buffering still exist without taking into account the event loop queueing issue?

Yes. We can pause receiving on the whole association (read: all streams simultaneously) which lets the flow control kick in. We cannot selectively pause receiving on individual streams. It's either receiving data on all streams or (eventually) receiving no data.

you may have a solution for not problems simultaneously, but it also requires a change in w3c apis and ietf specs.

Yeah, there's no easy fix. It's just how it is. :woman_shrugging: @jan-ivar and I briefly touched this topic as part of the streams extension. My idea to solve work around this goes into the direction of kindly asking the sender to pause/continue sending data on a particular stream by using a special PPID.

To solve this issue we only need one line in the spec without api change.

Unfortunately not.

henbos commented 5 years ago

As long as streams are sharing this buffer, one stream not being read from would eventually bring all other streams to a standstill.

As I understand it, it is impossible not to read from a stream under normal conditions. As soon as the message has arrived, W is emptied and "onmessage" fires (or with my proposal: W is emptied when JS thread is no longer blocked). It doesn't matter if anybody listens to this event or not, from SCTP's point of view the message has been delivered. The only way a stream would not be read from is if it never arrives in its entirety, a half-received message clogging up W forever. If messages are sent sequentially, I believe this is an impossibility (B's message would complete before A's messages are put in W). If messages are sent "in parallel" (doesn't have to be true parallelism, but assuming there is competition here), there may be a possibility of lots of small messages on one channel "choking" a large message on another channel that never fully completes in W. If so this could potentially be a problem for both the the receiver's receive buffer (discussed here) and sender's send buffer (analogous problem). I do believe this is a very important but separate problem from:

Feeding back a signal to the SCTP stack to let the flow control kick in and stop receiving data due to CPU load on the JS thread?

As for this:

Feeding back a signal to whatever thread is handling the SCTP stack to stop delivering messages to the JS thread? (No, this does not allow the flow control to kick in or it would be equivalent to 1.)

If congestion control kicks in then a consequence of that would be that messages are not delivered on the JS thread, because they have not fully arrived on W.

henbos commented 5 years ago

As for receiving messages larger than the entire buffer, I think that is a third issue, and would require copying chunks from the SCTP buffer to a pre-delivery buffer, which I'm not sure is an "implementation detail" or something that needs to be specified.

murillo128 commented 5 years ago

I agree with @henbos there are already 3 different issues here, which should be easier to track in different issues and not all at once.

murillo128 commented 5 years ago

To solve this issue we only need one line in the spec without api change.

Unfortunately not.

I mean strictly solve this issue, i.e. preventing a the DC stack to push events to the event queue faster than what the main js queue can drain them.

The other issues are obviously not solved.

murillo128 commented 5 years ago

Let's try to recap, theese are the three issues:

  1. With current API model, the DC stack can push event faster to the event queue that what the main js thread is able to handle, so event queue grows indefinetivelly.
  2. If interleaving is used, it could be possible that an large, low priority message fills the sctp buffer halting the sender and higher priority but smaller messages can't be received.
  3. It is possible that a message bigger than the receive buffer fills the sctp buffer, draining the buffer and halting the sender.

Can we agree on this?

murillo128 commented 5 years ago

Also, shouldn't 3) be solved by correctly negotiating the max-message-size in the SDP O/A ?

https://tools.ietf.org/html/draft-ietf-mmusic-sctp-sdp-26#page-8

lgrahl commented 5 years ago

@henbos

As I understand it, it is impossible not to read from a stream under normal conditions. As soon as the message has arrived [...]

Let's stop right there. With the proposed change, the message (of size M) will never arrive if M is greater W because of...

[...] at this point, the receive buffer of the underlying data channel has not been decreased for rawData [...]

A suitable comparison would be a TCP socket with a receive buffer W where one is only peeking (!) for data of size M. This will stall the TCP connection if M > W.

@murillo128

[...] 1), 2) and 3). Can we agree on this?

1) is indeed what this was originally about but the proposed change would result in 2) and 3) becoming a problem in the first place. 2) and 3) would never happen with the current specification.

Also, shouldn't 3) be solved by correctly negotiating the max-message-size in the SDP O/A ?

The receive buffer has nothing to do and should never impact the maximum message size (important: it would with the proposed change here which is why I'm still arguing!). For example, Firefox uses an SCTP receive buffer of 1 MiB but can receive messages up to 2 GiB. It takes the messages out of the SCTP receive buffer and adds it to a separate reassembly buffer on the channel instance. However, there is no flow control signal because if that would get fed back to the receive buffer, 2) and 3) would suddenly become an issue.

murillo128 commented 5 years ago

For example, Firefox uses an SCTP receive buffer of 1 MiB but can receive messages up to 2 GiB.

What prevents Firefox for declaring a 2GiB buffer size on the first place?

henbos commented 5 years ago

If you empty the receive buffer and put it in a temporary message buffer then you can receive messages larger than the receive buffer, but having another buffer does not solve the congestion control problem. Eventually, the temporary message buffer will fill up and then either 1) you stop emptying the receive buffer to get congestion control to kick in, which is a variety of the existing proposal, or you 2) create more message buffers, in which case we have the same problem that we do now (it's just that the buffer is stored in "message buffers" instead of in in-queue PostTasks). If we are CPU-bound we have to have some sort of congestion control. Blocking messages from arriving sounds like a less severe problem than resource exhaustion.

murillo128 commented 5 years ago

@henbos I agree, the a_rwnd buffer limit should not be there to protect the sctp stack but the whole browser.

Having a nice looking 1MiB used in the sctp stack but 16GiB accumulating on other places of the browser doesn't seems be the correct usage of it.

murillo128 commented 5 years ago

@lgrahl note that with the same mechanism (reducing the a_rwnd when the data is sent to the js app and not when passed from the sctp stack to the dc stack) would work when using your WATHWG api proposal and you would not need any extra per-datachannel buffer size back pressure mechanism.

You will decrease the a_rwnd when the read operation is done, so the only way of one datachannel draining the queue and stalling it is that the js app does not read from the datachannel (on purpose or due to a bug) . In any case, stalling is the correct behavior there IMO.

lgrahl commented 5 years ago

@murillo128

What prevents Firefox for declaring a 2GiB buffer size on the first place?

It would be an abuse of the receive buffer to work around an API issue. And it would break once someone activates ndata (which is a spec requirement). Needless to say that transport protocol implementations usually allocate the buffer at once... I think you can imagine what would happen. To quote RFC 4960:

Advertised Receiver Window Credit (a_rwnd): 32 bits (unsigned integer)

This value represents the dedicated buffer space, in number of bytes, the sender of the INIT has reserved in association with this window. During the life of the association, this buffer space SHOULD NOT be lessened (i.e., dedicated buffers taken away from this association); however, an endpoint MAY change the value of a_rwnd it sends in SACK chunks.

Also, let's discuss stream API things in a separate issue.


@henbos

If you empty the receive buffer and put it in a temporary message buffer then you can receive messages larger than the receive buffer, but having another buffer does not solve the congestion control problem.

It doesn't solve the problem. I just wanted to clarify that receive buffer size != maximum message size by an example.

Eventually, the temporary message buffer will fill up and then either 1) you stop emptying the receive buffer to get congestion control to kick in, which is a variety of the existing proposal, or you 2) create more message buffers, in which case we have the same problem that we do now (it's just that the buffer is stored in "message buffers" instead of in in-queue PostTasks).

We can't do option 1) because as explained above (which is why Firefox doesn't). It would just expand the time period/message size but the issue would still occur eventually. 2) is, as you already said, not useful.

If we are CPU-bound we have to have some sort of congestion control. Blocking messages from arriving sounds like a less severe problem than resource exhaustion.

Sure, but the current proposal has a much more severe effect than it's sole intention to block individual messages as I explained above.

Let's please call this flow control and not congestion control. :slightly_smiling_face: Congestion control is a different mechanism.


So, here's a suggestion what could be done which would (probably) not stall the SCTP association: Don't feed decrypted packets from the DTLS stack to the SCTP stack when the event loop queue is congested. They will eventually be retransmitted if it's a reliable channel. Edit: This does indeed trigger the congestion control, so it's only a workaround for a crippled flow control and may have severe throughput impacts.

murillo128 commented 5 years ago

So, here's a suggestion what could be done which wouldn't result in all kinds of unwanted side effects: Don't feed decrypted packets from the DTLS stack to the SCTP stack when the event loop queue is congested. They will eventually be retransmitted if it's a reliable channel.

That will cause retransmissions and congestion on the wire possible causing disruption of the audio and video feed