Skip to content
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

Hardcoded 128 backlog in TCPListener #55614

Closed
Fensteer opened this issue Nov 2, 2018 · 13 comments
Closed

Hardcoded 128 backlog in TCPListener #55614

Fensteer opened this issue Nov 2, 2018 · 13 comments
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@Fensteer
Copy link

Fensteer commented Nov 2, 2018

In the TCPListener implementation, listen backlog is hardcoded to 128 (here).
It would be usefull to make it parametrized when binding the socket, for specific usages for example :

  • Limit number of connection to the backlog
  • Handle more than 128 or a specific number of connections.

What do you think about ?

@kennytm kennytm added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Nov 2, 2018
@chernomor
Copy link

Tunable backlog available in https://crates.io/crates/net2

@joshtriplett
Copy link
Member

It doesn't look like there's been any activity on this.

Concrete proposal: what if we added a new version of the bind constructor, which took a backlog parameter?

@algesten
Copy link
Contributor

@joshtriplett I stumbled on this today. Was surprised it's not possible to configure using std::net. Additionally it seems net2 is deprecated and backlog parameter never found its way into the replacement socket2.

I actually need this in an async server, but it seems neither tokio nor async-std implements it. However if we agree on an API in std, the async version are likely to follow along.

Concrete proposal: what if we added a new version of the bind constructor, which took a backlog parameter?

Would it maybe make sense to mirror the work done in net2? The signature is:

TcpListener::listen(backlog: i32) -> Result<TcpListener>

This is also similar to the system call.

@joshtriplett
Copy link
Member

joshtriplett commented Jul 31, 2020 via email

@fujita
Copy link

fujita commented Jan 14, 2021

How about using a system-wide configured value as default like Go instead of hardcoded before introducing a large change for detailed configuration? For example, net.core.somaxconn on Linux.

@njaard
Copy link

njaard commented Mar 6, 2021

At least on linux, you could just set the backlog to infinitely high and let the kernel clamp it. I think that should be done to UnixListener and TcpListener.

@BartMassey
Copy link
Contributor

There are situations where you would rather have clients get an error than be stuck in a long queue because service is slow, so being able to specify a small maximum backlog per application is important. The current default max backlog of 128 seems fine for most situations: if that number is too small it's pretty likely a larger number would be also. (That said, I opened #94406 because I needed to specify a larger backlog. Go figure.)

I just need to finish #94407 and get it pulled: I'll try to find time soon.

@smsilva98
Copy link

Any updates on this?

@BartMassey
Copy link
Contributor

Sadly, no. I got stuck in the weeds trying to get the patch to pass tests on weird platforms, and then let it sit until substantial work would be needed to rebase it. Not great.

If someone wants to pick this up, I'd be happy to work with them on it. I can't see finding time on my own in the next month or two: too much going on.

@PureWhiteWu
Copy link

PureWhiteWu commented Jun 9, 2023

Is it acceptable to change the default backlog for linux to the SOMAXCONN in std as Go does?

This may change the default behaviour, though this may not cause problems and this is not documented either.

Ref: https://github.com/golang/go/blob/master/src/net/sock_linux.go

Ref implementation in rust: https://github.com/cloudwego/volo/blob/main/volo/src/net/incoming.rs#L117

@ghost
Copy link

ghost commented Jul 18, 2023

Surprised to see Rust still doesn't allow setting TCP listening backlog 8 years after the 1.0 release. Isn't that just passing 1 numeric parameter to an existing C function, or is it somehow more complicated?

@BartMassey
Copy link
Contributor

It is somehow more complicated. The listen() function is part of the cross-platform API and is expected to do something sensible everywhere. Some of the supported(ish) platforms don't have a backlog parameter, some have weird limits. Tests have to be written that compile and pass on all platforms.

It's not an insane amount of work or anything, just not quite trivial. Patches would be more than welcome! You could start with my old patches, although it is probably easier to re-implement them than rebase them at this point.

@Mark-Simulacrum
Copy link
Member

We are broadly configuring this to the maximum possible on many platforms at this point, so I'm going to close. Remaining platforms can be increased via PR to the relevant std code.

Technically configuration is still not supported, but proposals for API shapes should generally go to the libs-api ACP process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests