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

feat: support disabling worker_rlimit_nofile #1612

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DoctorFTB
Copy link

Pull Request (PR) description

Add support for disabling "worker_rlimit_nofile" by checking for a value greater than zero

This Pull Request (PR) fixes the following issues

Fixes #1611

@kenyon kenyon changed the title feat: support disabling "worker_rlimit_nofile" feat: support disabling worker_rlimit_nofile Sep 1, 2024
Copy link
Member

@kenyon kenyon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems OK, but I think it would be better if it defaulted to undef, so that you don't have to specify a special value to disable it. I guess that would be a breaking change though. Not sure why the default is 1024 when there is no default upstream: https://nginx.org/en/docs/ngx_core_module.html#worker_rlimit_nofile

@DoctorFTB
Copy link
Author

This seems OK, but I think it would be better if it defaulted to undef, so that you don't have to specify a special value to disable it. I guess that would be a breaking change though. Not sure why the default is 1024 when there is no default upstream: https://nginx.org/en/docs/ngx_core_module.html#worker_rlimit_nofile

Yeah! This module has not a little options with values ​​different from the default values ​​in the nginx docs.

For example:
name -> current module vs nginx docs
keepalive_timeout -> 65s vs 75s
keepalive_requests -> 100 vs 1000
client_max_body_size -> 10m vs 1m

Also has a default value as value, not as undef, like options: client_body_timeout, send_timeout

May be its best practice or .. ?

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.

Support disabling "worker_rlimit_nofile"
2 participants