-
Notifications
You must be signed in to change notification settings - Fork 10.1k
Added argument to drain that provides the list of packets that are drained #5362
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
9def17f to
3be3a76
Compare
Hmm, shouldn't you go a bit deeper and catch the drain event emitted by the HTTP response? In order to be sure that the packet has been written to the kernel buffer and can safely be released.
References:
- https://nodejs.org/api/http.html#responsewritechunk-encoding-callback
- https://nodejs.org/api/http.html#event-drain
Related: #5353
Hmm, shouldn't you go a bit deeper and catch the
drainevent emitted by the HTTP response? In order to be sure that the packet has been written to the kernel buffer and can safely be released.References:
- https://nodejs.org/api/http.html#responsewritechunk-encoding-callback
- https://nodejs.org/api/http.html#event-drain
Related: #5353
For Websockets, the ws library is handling that I think and it won't callback until its finished writing out. Do you see something different?
You are possibly right about long-polling though; I can defer our drain until the HTTP drain. I just pushed out that change. Though does long-polling even support buffers? I was under the impression it was base64-ing them.
I am not seeing a mechanism for uWebSockets to tell us when they have actually written out the buffer.
Hi, just wanted to check in on this PR @darrachequesne; I can update it to eliminate the conflicts but I don't want to keep doing it if it won't merge soon.
@darrachequesne Can I do anything to help get more engagement on this PR? Thanks!
The kind of change this PR does introduce
Current behavior
'drain'event occurs with no argumentsNew behavior
'drain'event occurs with an argument containing the list of packets that were sent by the transportOther information (e.g. related issues)