-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
Update the default Puma configuration #50669
Conversation
railties/lib/rails/generators/rails/app/templates/config/puma.rb.tt
Outdated
Show resolved
Hide resolved
railties/lib/rails/generators/rails/app/templates/config/puma.rb.tt
Outdated
Show resolved
Hide resolved
848523b
to
bf8937d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small text tweaks for improved readability
railties/lib/rails/generators/rails/app/templates/config/puma.rb.tt
Outdated
Show resolved
Hide resolved
railties/lib/rails/generators/rails/app/templates/config/puma.rb.tt
Outdated
Show resolved
Hide resolved
bf8937d
to
e2022d4
Compare
@fractaledmind thanks. |
railties/lib/rails/generators/rails/app/templates/config/puma.rb.tt
Outdated
Show resolved
Hide resolved
# Global VM Lock (GVL) it has diminishing returns, and will degrade the | ||
# response time (latency) of the application. | ||
# | ||
# The ideal number of threads per worker depends both on how much time the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd just lay out the tradeoff:
"Increasing threads per worker increases throughput for applications which spend significant time waiting on I/O, but can also increase latency. Decreasing threads per worker keeps latency consistent and low for all applications, at the cost of maximum throughput."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like it's very similar to the "rule of thumb" I describe above.
railties/lib/rails/generators/rails/app/templates/config/puma.rb.tt
Outdated
Show resolved
Hide resolved
railties/lib/rails/generators/rails/app/templates/config/puma.rb.tt
Outdated
Show resolved
Hide resolved
e2022d4
to
7c4d528
Compare
The main change is the default number of threads is reduced from 5 to 3 as discussed in rails#50450 Pending a potential future "Rails tuning" guide, I tried to include in comments the gist of the tradeoffs involved. I also removed the pidfile except for development. It's useful to prevent booting the server twice there but I don't think it makes much sense in production, especially [since Puma no longer supports daemonization and instead recommend using a process monitor](https://github.com/puma/puma/blob/99f83c50fbb712b0987667f3533cce4ea925b2da/docs/deployment.md#should-i-daemonize). And it makes even less sense in a PaaS or containerized world.
7c4d528
to
06d614a
Compare
Ok, thanks everyone for the comments and suggestions, I think it's in a good enough state now, we can always do touch up later if there are things people feel strongly about. |
Lengthy discussion on default thread count, web concurrency, and pid files in Set new default for the Puma thread count rails/rails#50450 Update the default Puma configuration rails/rails#50669
Thanks for fixing the process detection 06d614a. Which I found out about https://justin.searls.co/posts/brand-new-rails-7-apps-exceed-heroku-s-memory-quotas/. |
if processors_count > 1 | ||
workers worker_count | ||
else | ||
preload_app! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious, why preloading the app if processors count is not > 1?
The previous given instructions were
# Use the
preload_app!
method when specifying aworkers
number.
# This directive tells Puma to first boot the application and load code
# before forking the application. This takes advantage of Copy On Write
# process behavior so workers use less memory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
preload_app!
is always preferable. The I don't set it when processor_count > 1
is because it's the default there: https://www.rubydoc.info/gems/puma/Puma%2FDSL:preload_app!
Preload the application before starting the workers; this conflicts with phased restart feature. On by default if your app uses more than 1 worker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That won't be fully true until Puma v7 puma/puma#3044
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hum, so with the current config preload_app!
is needed regardless of the number of workers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/puma/puma/blob/master/5.0-Upgrade.md#upgrade
(without looking at the code) I think its only if WEB_CONCURRENCY is used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was checking the history a bit to understand how this file evolved in the last time and I found the following:
- 471ab23 - Remove
workers
andpreload_app!
. Let Puma take care of the defaults - 839ac1e - Try to use all CPU cores by default (when WEB_CONCURRENCY not set)
- 06d614a - Go back to a default of 1 for WEB_CONCURRENCY
So maybe all of this is unnecessary and we should go back to 1. ? (at least for the workers part)
This guide explains major concurrency and performance principles for Puma and CRuby, when to possibly not use Puma, and how to do basic load testing for tuning production performance settings. Incorporates comments from Rails issue rails#50450, PR rails#50669 and feedback from Jean Boussier. Co-authored-by: Jean byroot Boussier <jean.boussier+github@shopify.com>
This guide explains major concurrency and performance principles for Puma and CRuby, when to possibly not use Puma, and how to do basic load testing for tuning production performance settings. Incorporates comments from Rails issue rails#50450, PR rails#50669 and feedback from Jean Boussier. Co-authored-by: Jean byroot Boussier <jean.boussier+github@shopify.com>
This guide explains major concurrency and performance principles for Puma and CRuby, when to possibly not use Puma, and how to do basic load testing for tuning production performance settings. Incorporates comments from Rails issue rails#50450, PR rails#50669 and feedback from Jean Boussier. Co-authored-by: Jean byroot Boussier <jean.boussier+github@shopify.com>
This guide explains major concurrency and performance principles for Puma and CRuby, when to possibly not use Puma, and how to do basic load testing for tuning production performance settings. Incorporates comments from Rails issue rails#50450, PR rails#50669 and feedback from Jean Boussier. Co-authored-by: Jean byroot Boussier <jean.boussier+github@shopify.com>
This guide explains major concurrency and performance principles for Puma and CRuby, when to possibly not use Puma, and how to do basic load testing for tuning production performance settings. Incorporates comments from Rails issue rails#50450, PR rails#50669 and feedback from Jean Boussier. Co-authored-by: Jean byroot Boussier <jean.boussier+github@shopify.com>
Fix: #50450
The main change is the default number of threads
is reduced from 5 to 3 as discussed in #50450
Pending a potential future "Rails tuning" guide, I tried to include in comments the gist of the tradeoffs involved.
I also removed the pidfile except for development. It's useful to prevent booting the server twice there but I don't think it makes much sense in production, especially since Puma no longer supports daemonization and instead recommend using a processmonitor. And it makes even less sense in a PaaS or containerized world.