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

Make "puma-heroku" the default config #1949

Closed
nateberkopec opened this issue Sep 5, 2019 · 6 comments · Fixed by #2143
Closed

Make "puma-heroku" the default config #1949

nateberkopec opened this issue Sep 5, 2019 · 6 comments · Fixed by #2143
Labels
Milestone

Comments

@nateberkopec
Copy link
Member

The gem is a bit misnamed, it's just "good practices in config". I think we could mainline most of the settings in there for 5.0 and sunset the gem.

@nateberkopec nateberkopec added this to the 5.0.0 milestone Sep 5, 2019
@olleolleolle
Copy link
Contributor

olleolleolle commented Oct 7, 2019

The items in puma/puma-heroku to be moved into lib/puma/configuration.rb could include:

  • Worker Count via ENV variable where supported: query the environment variable WEB_CONCURRENCY (default: 1) for workers count
    • (unless JRuby or Windows, then do not set it)
  • if using workers, preload the app
  • Set both min and max threads via ENV variables RAILS_MAX_THREADS, MAX_THREADS:
    • Integer(ENV['RAILS_MAX_THREADS'] || ENV['MAX_THREADS'] || default_value)
    • default_value: 16 on non-MRI platforms
    • default_value: 5 on MRI
  • Listen to RAILS_ENV for environment: :environment => -> { ENV['RACK_ENV'] || "development" }, to also include ENV['RAILS_ENV'] - Configuration: Get environment from RAILS_ENV, too #2022
  • Unsupported, not to be included Rails 5.2 and lower: pre-fork & on_worker_boot database connecting

Q: Any these items that have higher priority? A: Not particularly.

The list has been updated.

@nateberkopec
Copy link
Member Author

@olleolleolle Not particularly. I'm think we can omit the 5.2-and-lower database-connecting stuff, as those versions as no longer officially supported.

RAILS_MIN_THREADS - who uses that?

@olleolleolle
Copy link
Contributor

@nateberkopec From memory: I've only used it because the option setting was there, and then used the same values for "max". Merging them into one useful setting: a good thing.

@nateberkopec
Copy link
Member Author

Also re: Set min and max threads via ENV variable(s): min_threads, max_threads set to Integer(ENV['RAILS_MAX_THREADS'] || ENV['MAX_THREADS'] || 5) (perhaps w/ a RAILS_MIN_THREADS, too?)

We should keep the default of 16 on non-MRI platforms - so we should use a new default of 5 on MRI only.

@jalevin
Copy link
Contributor

jalevin commented Mar 2, 2020

#2143 might need a hand with this.. having issues repeating failing tests locally.

@ukolovda
Copy link

ukolovda commented Mar 19, 2020

Hello!
Also it will be useful in Docker containers.
I need read default parameters from environment variables, not from command-line arguments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants