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

Warn when before_fork (and friends) called when running in single mode #2950

Closed
schneems opened this issue Sep 12, 2022 · 4 comments
Closed
Labels

Comments

@schneems
Copy link
Contributor

Hello, beautiful people of Puma. I've seen a common problem relating to my docs for this feature https://devcenter.heroku.com/articles/language-runtime-metrics-ruby#debugging. I think it is affecting other puma users as well.

Problem

People will put some code in the before_fork block of their puma.rb but then accidentally boot their app in single mode instead of cluster mode. When this happens, before_fork never gets called.

Solution

My proposal is that when before_fork is set, but the app is booting in single mode, we emit a warning. Something like:

Warning: You specified code to run in a `before_fork` block, but Puma is configured to run without forking.

We could warn when code is specified in these configuration cases:

  • after_worker_fork
  • before_fork

Describe alternatives you've considered

Besides warning, we could make it configurable to error. Maybe conditionally based on PUMA_UNUSED_CONFIG=fail or something.

My only concern would be if it's too noisy (I don't know if people run single locally and clustered on prod or not). If that's the case we could possibly gate the feature behind an env var like PUMA_UNUSED_CONFIG=warn.

Additional context

You are all wonderful

@dentarg
Copy link
Member

dentarg commented Sep 12, 2022

Warning sounds good to me, don't think we need/should make it fail (maybe you are just testing out the other mode temporary). If opt-in with PUMA_UNUSED_CONFIG=warn I don't think it will get much usage. We could make it possible to silence it, similar to silence_single_worker_warning.

@nateberkopec
Copy link
Member

I don't know if people run single locally and clustered on prod or not

True, I have seen that often. Like Dentarg said, in that case we'd just make it opt-out.

@schneems
Copy link
Contributor Author

I like it. To recap: Warn by default, no errors with an opt-out "escape valve" to disable the behavior.

We could either follow the silence_single_worker_warning pattern with a silence_fork_callback_warning or via env vars PUMA_UNUSED_CONFIG=disable in your bashrc (or something). Or something else.

@nateberkopec
Copy link
Member

🤷 Why Not Both?

Vuta added a commit to Vuta/puma that referenced this issue Feb 28, 2023
…le mode

Clustered-only hooks won't be called in single mode, so this commit adds a log to warn users about that.
Also adds `silence_fork_callback_warning` option to opt-out of the default warning log.
Vuta added a commit to Vuta/puma that referenced this issue Mar 1, 2023
…le mode

Clustered-only hooks won't be called in single mode, so this commit adds a log to warn users about that.
Also adds `silence_fork_callback_warning` option to opt-out of the default warning log.
Vuta added a commit to Vuta/puma that referenced this issue Mar 1, 2023
…le mode

Clustered-only hooks won't be called in single mode, so this commit adds a log to warn users about that.
Also adds `silence_fork_callback_warning` option to opt-out of the default warning log.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants