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

Add ability to set minimum Puma threads with MIN_THREADS environment variable #21048

Merged
merged 1 commit into from Jan 6, 2023

Conversation

jimeh
Copy link
Sponsor Contributor

@jimeh jimeh commented Nov 18, 2022

No description provided.

@matthewford
Copy link
Contributor

Looks good to me

@ineffyble ineffyble added deployment Related to runtime configuration, production setups performance Runtime performance labels Nov 24, 2022
@Gargron
Copy link
Member

Gargron commented Nov 27, 2022

Moving the Helm chart out to mastodon/chart

@jimeh
Copy link
Sponsor Contributor Author

jimeh commented Nov 29, 2022

@Gargron This isn't directly related to the helm chart, but rather expands control of Puma via environment variables. So I'd still like to see it merged :)

config/puma.rb Outdated
@@ -1,7 +1,8 @@
persistent_timeout ENV.fetch('PERSISTENT_TIMEOUT') { 20 }.to_i

threads_count = ENV.fetch('MAX_THREADS') { 5 }.to_i
threads threads_count, threads_count
min_threads_count = ENV.fetch('MIN_THREADS') { 5 }.to_i
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes behaviour, perhaps fallback to MAX_THREADS value instead? Unless changing the behaviour is a performance improvement of some sort.

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, good catch. Fixed it to default to max_thread_count as you suggested. This should ensure identical behavior if MIN_THREADS is not set.

@Gargron Gargron closed this Jan 6, 2023
@Gargron Gargron reopened this Jan 6, 2023
@Gargron Gargron merged commit 85ec615 into mastodon:main Jan 6, 2023
nametoolong pushed a commit to nametoolong/nuage that referenced this pull request Jan 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployment Related to runtime configuration, production setups performance Runtime performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants