Add pause/resume to protocol #36
Conversation
With a slow enough consumer it is possible to freeze the websocket server for that connection causing the ping to timeout. pause and resume messages are now added to the protocol to tell client to stop writing data if the server side buffer is filling up.
| @@ -64,6 +72,11 @@ func (r *readBuffer) Read(b []byte) (int, error) { | |||
| n, err = r.buf.Read(b) | |||
| r.cond.Broadcast() | |||
| if err != io.EOF { | |||
| if r.buf.Len() < MaxBuffer/8 { | |||
There was a problem hiding this comment.
Is there a chance when the buf always has more data than MaxBuffer/8 so that it never resume other write go routines?
There was a problem hiding this comment.
I believe this question remains. Is it possible that on one read, the err == nil and r.buf.Len() >= MaxBuffer/8? Then on the next read err == io.EOF?
It would seem that in this case we would never resume.
There was a problem hiding this comment.
The above comment is not relevant. The implementation of bytes.Reader will return io.EOF only if the buffer is empty on read. If the buffer is empty after the read, then a nil error is returned. So this looks correct to me.
| ) | ||
|
|
||
| const ( | ||
| MaxBuffer = 1 << 20 | ||
| MaxBuffer = 1 << 21 |
There was a problem hiding this comment.
How is this number defined? Can maxBuffer be dynamically increased if it can't hold more data? same like the array size that is dynamically increased when buffer exceeded...
There was a problem hiding this comment.
Talked offline, this is a flow control issue. Quote from darren:
it's a flow control issue. If the tunnel client is sending data at 1mb/s and the user is reading data at 1kb/s we will just fill up memory
| defer b.cond.L.Unlock() | ||
|
|
||
| b.paused = false | ||
| b.cond.Broadcast() |
There was a problem hiding this comment.
Need an advanced sync lecture for this... 😲
|
What kind of scenarios caused this? |
With a slow enough consumer it is possible to freeze the websocket server
for that connection causing the ping to timeout. pause and resume
messages are now added to the protocol to tell client to stop writing
data if the server side buffer is filling up.
For rancher/rancher#34819