[cli] detect number of CPU cores #554
Conversation
mikz
force-pushed
the
worker-processes
branch
from
6f306c2 to
e6157a8
Compare
| @@ -179,7 +179,7 @@ local function configure(cmd) | |||
| cmd:flag("-d --daemon", "Daemonize.") | |||
| cmd:option("-w --workers", | |||
| "Number of worker processes to start.", | |||
| resty_env.value('APICAST_WORKERS') or 1) | |||
| resty_env.value('APICAST_WORKERS') or Environment.default_config.worker_processes) | |||
There was a problem hiding this comment.
Should we call tonumber on the first part of the or? I guess it would work anyway, but I think it would be clearer.
There was a problem hiding this comment.
But what if you want to set APICAST_WORKERS=auto ?
There was a problem hiding this comment.
I think it's not obvious that APICAST_WORKERS accepts 'auto'. People familiar with nginx or the code might guess it, but I think we should document it.
There was a problem hiding this comment.
APICAST_WORKERS is currently undocumented and I think it is pretty low level and stay like that for now.
- in production we auto detect correct number of cores when deployed by our docker image
- development starts on 1 core
So people really should not need tuning the number of workers.
| @@ -40,6 +41,16 @@ local function parse_nameservers() | |||
| end | |||
| end | |||
|
|
|||
| local function cpus() | |||
| local cores = tonumber(resty_env.get('NUMBER_OF_CORES')) | |||
There was a problem hiding this comment.
Do we need this? Is there any reason someone would use NUMBER_OF_CORES instead of the existing APICAST_WORKERS?
There was a problem hiding this comment.
Also, if we start accepting this ENV, we should document it https://github.com/3scale/apicast/blob/master/doc/parameters.md
Just realized that APICAST_WORKERS is not there.
There was a problem hiding this comment.
All software collection images use NUMBER_OF_CORES, so I thought it would make sense to follow that.
But probably it does not make sense because the SCL images override NUMBER_OF_CORES with own value.
Will remove it.
* use nproc to detect number of cores - nproc correctly detects limits set by docker
mikz
force-pushed
the
worker-processes
branch
from
e6157a8 to
0813608
Compare
/cc @jmprusi