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

adjust file descriptor limits #34

Closed
wants to merge 1 commit into from

Conversation

Bruce0203
Copy link

No description provided.

@Snowiiii
Copy link
Member

Hey, Can you provide a description?

@Bruce0203
Copy link
Author

The file descriptor is used to manage TCP sockets in the operating system. The number of file descriptors is limited per process, which can lead to a maximum limit of TCP sockets.

@Snowiiii
Copy link
Member

Should'nt we let the user decide whats the File descriptors limit, I can imagine that common server distros change this value or unlimit it. I wonder how Netty/Vanilla Minecraft handles it

@lukas0008
Copy link
Contributor

lukas0008 commented Aug 20, 2024

Correct me if I'm wrong, but isn't the soft limit of file handles on linux usually unlimited? Only the hard limit which cannot be changed by unprivileged users is usually set (although also usually to an unnecessarily high value like >=2048). But to change those, pumpkin would need to run as root, which is also against security practices. The soft limit being lower also would probably mean that the user themselves set it that way for a reason.

PS: I think this also crashes on all non-linux systems, since it's calling unsafely to libc's linux exclusive api. (not sure if it even compiles for others)

@Snowiiii
Copy link
Member

Snowiiii commented Sep 4, 2024

Netty also uses the global OS value. I think we should use it aswell. There may will be useres who want to have an connection limit. I also don't really want to deal with libc. I will close this for now. Feel free to reopen or make an issue. To give good points to set the limits ourselfs

@Snowiiii Snowiiii closed this Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants