Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Open
bytenik wants to merge 3 commits into socketio:main
base: main
Choose a base branch
Loading
from thinkalpha:drain-param

Conversation

@bytenik
Copy link
Contributor

@bytenik bytenik commented Jun 17, 2025

The kind of change this PR does introduce

  • a bug fix
  • a new feature
  • an update to the documentation
  • a code change that improves performance
  • other

Current behavior

'drain' event occurs with no arguments

New behavior

'drain' event occurs with an argument containing the list of packets that were sent by the transport

Other information (e.g. related issues)

Copy link
Member

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:

Related: #5353

Copy link
Contributor Author

bytenik commented Jun 17, 2025

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:

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.

@bytenik bytenik marked this pull request as ready for review June 29, 2025 19:42
Copy link
Contributor Author

bytenik commented Aug 13, 2025

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.

Copy link
Contributor Author

bytenik commented Aug 31, 2025

@darrachequesne Can I do anything to help get more engagement on this PR? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /