bufferedAmount property is broken
#492
Comments
Fixes #4 No backpressure in node. The `ws` module has a buggy `bufferedAmount` property. See: websockets/ws#492
|
Is If so, would someone please help me with the code that would detect if the receive buffer is full, or how full it is. Currently I am trying to get a connection working where the server is quickly sending large amounts of data in separate paged messages, and I need to have client-side detection of receive buffer size used and size available (or at least it would be very useful, as it may be better than some other workarounds). |
|
If I understand the issue correctly, If that was the issue, then yes, |
|
In ws The data in the socket buffer ( 10 bytes for a message of 3 ASCII seem a bit too much though. The WebSocket frame size for a message of 3 bytes should be 5 or 7 bytes depending on whether it was masked or not (sent by the server or the client). Perhaps the payload is compressed? |
|
Actually the mask is 4 bytes so the WebSocket frame for a 3 bytes message is 5 or 9 bytes depending on whether the payload is masked or not. The extra byte probably comes from the issue linked above. |
|
Thanks, it makes sense that the mask is accounting for some of the extra bytes. However, on latest (v10.8.0) I'm seeing 18 bytes. And on LTS 8.11.3, I'm getting 78 bytes!! |
|
I don't know, maybe that's the result of multiple buffered frames (minus the bytes that are written)? Check with plain HTTP first. |
|
With 7.3.0, I can see that the property does go down to zero when all the data has been sent. |
|
The Code: public sendData(data: Data): void {
const sendNumber = this.sendNumber++;
console.log("[" + sendNumber + "]");
console.log("Byte length: " + Buffer.byteLength(data));
const beforeBufferedAmount: number = this.socket.bufferedAmount;
console.log("Before buffered amount: " + beforeBufferedAmount);
this.socket.send(data, () => {
console.log("[" + sendNumber + "] Sent!");
});
const afterBufferedAmount: number = this.socket.bufferedAmount;
console.log("After buffered amount: " + afterBufferedAmount);
const deltaBufferedAmount: number = afterBufferedAmount - beforeBufferedAmount;
console.log("Delta buffered amount: " + deltaBufferedAmount);
console.log();
}Output: I would expect the buffered amount to be the byte length of the data sent across the wire (including websocket frame bytes). It seems like the send function is able to send the data synchronously without buffering the data at all, but this removes the ability to use this property to find how much data will be sent over the wire (before using any kind of compression). |
|
Thanks @BTOdell, this is similar to what I'm seeing, I am sending packets locally between two devices in my home WiFi. I do see it drop to zero eventually, in one go, after waiting long enough. I couldn't pinpoint the phase when this happens. |
|
@BTOdell that is expected. It is inconsistent with the browser |
|
@lpinca I see. Thanks for the explanation. |
|
The callbacks are called in order and the timing is reliable so yes it should be possible. I think one issue with a |
|
Actually I read it wrong: what I was seeing is not that |
|
@corwin-of-amber that happens if the connection gets closed and you keep calling |
|
Thanks for your reply @lpinca but this is not what happens: the packets are being sent. I am sending an entire file through the WebSocket and it is received, eventually, by the host. When all packets have been received, and only then, the |
|
I'm seeing some strange behavior with the "Preferred bandwidth" is printed each time new data is sent by my application. The number is a moving average bandwidth calculation in bytes per second. You can see from the first "available bandwidth" print out, that the delta time (since the last sent packet) was over 18 seconds. The send callbacks are all being executed periodically in batches. Looking further down in the logs you can see another large packet: This time the packet is ~931KB but it only takes a fraction of a millisecond, resulting in a calculated speed of 6.7GB/s! I don't think my network is that fast either! It's clear the |
|
Also, if what @lpinca said is true regarding the 'foo' could be sent immediately and then the tests fail. |
|
That test uses compression so the message is not sent immediately.
Yes that is expected because
That's how Node.js That probably happens becuase the the entire file is buffered in memory. |
|
Another strange I track the "pending" size by adding a byte length to a FIFO queue before I send the data over the WebSocket, and I remove it from the queue when I get the callback. I'm calculating the data length using I read through the source code of I am using the |
I don't think so because the const Buf = require('buffer').Buffer;
console.log(Buffer.isBuffer(Buf.from('Hello'))); // Prints true.Are you using compression? The frame header length on the client ranges from 6 to 14 bytes depending on the payload length. |
|
No, I don't have the I'm using TypeScript to import the import {Buffer} from "buffer";but I imagine it converts to something like this in JavaScript: const {Buffer} = require("buffer");Will that import form work with Node.js? You said it'll work seamlessly unless I use the |
|
On the client permssage-deflate is enabled by default, if it is enabled on the server it will used. By default only messages whose size is > 1024 bytes will be compressed. Try to use a plain |
|
How would I do that if I'm connecting to the server from a browser? Browsers can't initiate direct TCP connections. I'll think of a way how to debug this better tomorrow. |
|
Test it from Node.js, |
|
I modified the sendFrame(list, cb) {
if (list.length === 2) {
this._socket.cork();
this._socket.write(list[0]);
this._socket.write(list[1], cb);
console.log("Write(2): " + list[0].length + " + " + list[1].length);
this._socket.uncork();
} else {
this._socket.write(list[0], cb);
console.log("Write(1): " + list[0].length);
}
}and this is the output: So, the |
|
I tracked down the implementation of the Half of |
|
When not using permessage-deflate, |
The
bufferedAmountproperty doesn't work correctly.This code:
Prints:
In Chrome and Firefox, on the other hand, this is what gets output:
Without a functioning
bufferedAmountproperty, it's not possible to use the code I've written to implement backpressure in the browser on the server as well.cc @mafintosh @maxogden
The text was updated successfully, but these errors were encountered: