Skip to content
This repository has been archived by the owner on Jan 6, 2021. It is now read-only.

Change worker_processes value to auto #318

Closed
wants to merge 1 commit into from
Closed

Change worker_processes value to auto #318

wants to merge 1 commit into from

Conversation

adamradocz
Copy link
Contributor

The nginx documentation says (http://nginx.org/en/docs/ngx_core_module.html#worker_processes): The optimal value depends on many factors including (but not limited to) the number of CPU cores, the number of hard disk drives that store data, and load pattern. When one is in doubt, setting it to the number of available CPU cores would be a good start (the value “auto” will try to autodetect it).

So changing the "worker_processes" value to auto is more general, then the current option.

The nginx documentation says (http://nginx.org/en/docs/ngx_core_module.html#worker_processes): The optimal value depends on many factors including (but not limited to) the number of CPU cores, the number of hard disk drives that store data, and load pattern. When one is in doubt, setting it to the number of available CPU cores would be a good start (the value “auto” will try to autodetect it).

So changing the "worker_processes" value to auto is more general, then the current option.
@CHBMB
Copy link
Member

CHBMB commented Jun 3, 2019

I'm not sure this is going to work inside a docker environment.

nginxinc/docker-nginx#31

kubernetes/ingress-nginx#2948

@nemchik
Copy link
Member

nemchik commented Jun 3, 2019

Here is an interesting potential workaround:

NUMPROCS=$(cat /sys/fs/cgroup/cpuacct/cpuacct.usage_percpu | wc -w)
sed -i "s/worker_processes\\s\\+1;/worker_processes ${NUMPROCS};/" /etc/nginx/nginx.conf

Found here: trajano/nginx-letsencrypt@fa4b90a#diff-c15c3b6492080efd90b52266848ca621

Edit: The snip above would probably be put into an init script in /root/etc/cont-init.d/ in this repo. It may also be wise to include a check for an environment variable to disable automatically setting that variable. Something like this could be beneficial to put in other nginx based images as well (depending how LSIO feels about its inclusion).

@CHBMB
Copy link
Member

CHBMB commented Jun 3, 2019

@nemchik That seems overly complicated an automation step for something that can be easily configured by changing one number.

The container is a starting point for a Nginx install, it can never be all things to all people.

@adamradocz adamradocz closed this Jun 3, 2019
@adamradocz adamradocz deleted the patch-2 branch June 3, 2019 18:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants