feat: num_threads counts the threads left for regular requests - #2660
nicolas-grekas wants to merge 2 commits into
Conversation
|
I know I suggested this, at the same time, I really dislike having even more configurations for thread numbers 😅 . But maybe it's the right way forward. I wonder if we could just somehow reinterpret |
523470e to
4a296e7
Compare
|
Done, the PR does that now: no new options, The BC break is a single behaviour: a config that sets WDYT? |
num_threads is a total today and worker threads are part of it, so sizing a server means subtracting them by hand: give a worker four more threads and four fewer are left for the requests no worker serves. Nothing says so either, the docs describe num_threads as "the number of PHP threads to start" and only an error message mentions the subtraction. It now counts the threads for those requests, and the worker threads come on top of it. max_threads keeps its meaning, the limit on everything the process runs, which is what the memory heuristic of auto bounds, and every worker keeps its own num and max_threads. Nothing is added to the configuration and the "num_threads must be greater than the number of worker threads" error goes away with the arithmetic that caused it. Defaults are unchanged: without num_threads a server still starts what is left of 2x the CPUs once the workers have their threads, one at the very least. A configuration that sets num_threads with workers declared starts more threads than before, up to max_threads where it is set; the startup line reports total_threads and worker_threads so the totals are visible.
4a296e7 to
7032cb8
Compare
…he worker threads on top
num_threadsis a total today and worker threads are part of it, so sizing a server means doing the subtraction by hand: give a worker four more threads and four fewer are left for the requests no worker serves. Nothing says so either, config.md describes it as "the number of PHP threads to start" and only an error message mentions the subtraction.It now counts the threads for those requests, and the worker threads come on top of it.
max_threadskeeps its meaning, the limit on everything the process runs, which is what the memory heuristic ofautobounds, and every worker keeps its ownnumandmax_threads. So nothing is added to the configuration, and thenum_threads must be greater than the number of worker threadserror goes away with the arithmetic that caused it.{ frankenphp { num_threads 4 # threads for the requests no worker serves max_threads auto worker { file jobs.php num 8 # on top, and no longer at the expense of the 4 } } }Defaults are unchanged: without
num_threads, a server still starts what is left of 2x the CPUs once the workers have their threads, one at the very least.What changes for an existing configuration: one that sets
num_threadswith workers declared starts more threads than before, extension workers included since they are worker threads too. Wheremax_threadsis set belownum_threadsplus the worker threads, startup now fails with an error naming the three numbers, where it booted before with the workers insidenum_threads: that is the break, and the one to mention in the release notes. Conversely, a configuration that was rejected for anum_threadsat or below its worker threads now boots. The startup line reportstotal_threadsandworker_threadsso the totals are visible, and the tests this breaks in the repo are exactly the ones that had written a total: three Caddyfiles and a scaling test that now say one thread less.Replaces the
num_regular_threads/max_regular_threadsof the first version of this PR, after @AlliBalliBaba pointed out that reinterpreting the existing option beats adding two more, which I agree with: this ends with fewer settings than we have today rather than more.