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

Increasing Puma's worker timeout in development #40503

Conversation

juanmanuelramallo
Copy link
Contributor

Summary

After Puma's version 5, a worker timeout check was added, causing all
debugging sessions (byebug, binding.irb, ...) to be terminated after a
minute is passed.

One minute timeout is fine for production environments but not for
development, when we usually debug stuff and need more than a minute to
do so.

This PR updates Puma's config template file to include a worker timeout
of one hour, only for development environments.

Other Information

This panned out after puma/puma#2443 (comment) @nateberkopec's comment.

@rails-bot rails-bot bot added the railties label Nov 1, 2020
@nateberkopec
Copy link
Contributor

causing all
debugging sessions (byebug, binding.irb, ...)

Actually, I think it's just byebug.

I can't think of a better way to solve this issue tbh.

# Specifies the `worker_timeout` threshold that Puma will use to wait before
# terminating a worker in development environments.
#
worker_timeout 3600 if ENV.fetch("RAILS_ENV") == "development"
Copy link
Member

Choose a reason for hiding this comment

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

RAILS_ENV can be unset so ENV.fetch("RAILS_ENV") will fail. I think we should default to "development" inside a block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will this work? Or is the block format better?

Suggested change
worker_timeout 3600 if ENV.fetch("RAILS_ENV") == "development"
worker_timeout 3600 if ENV.fetch("RAILS_ENV", "development") == "development"

Copy link
Member

Choose a reason for hiding this comment

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

This also work. Feel free to apply and make sure all commits are squashed in one.

After Puma's version 5, a worker timeout check was added, causing all
debugging sessions (byebug, binding.irb, ...) to be terminated after a
minute is passed.

One minute timeout is fine for production environments but not for
development, when we usually debug stuff and need more than a minute to
do so.

This PR updates Puma's config template file to include a worker timeout
of one hour, only for development environments.
@juanmanuelramallo juanmanuelramallo force-pushed the increase-worker-timeout-in-development branch from e5851c6 to b43ede6 Compare November 6, 2020 15:57
# Specifies the `worker_timeout` threshold that Puma will use to wait before
# terminating a worker in development environments.
#
worker_timeout 3600 if ENV.fetch("RAILS_ENV", "development") == "development"
Copy link
Contributor

Choose a reason for hiding this comment

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

👋 since you don’t care whether RAILS_ENV is set or not, would it make sense to express it as something more concise?

Suggested change
worker_timeout 3600 if ENV.fetch("RAILS_ENV", "development") == "development"
worker_timeout 3600 if ENV["RAILS_ENV"] == "development"

Copy link
Member

Choose a reason for hiding this comment

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

If ENV["RAILS_ENV"] is unset, the environment is development by default, and the setting should be enabled.

@rafaelfranca rafaelfranca merged commit 363cd6d into rails:master Nov 30, 2020
rafaelfranca added a commit that referenced this pull request Nov 30, 2020
…out-in-development

Increasing Puma's worker timeout in development
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 this pull request may close these issues.

None yet

5 participants