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.

Sign up for GitHub

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

Jump to bottom

preload_app doesn't default to true if more than 1 worker #3044

Open
x-yuri opened this issue Dec 24, 2022 · 3 comments · May be fixed by #3297
Open

preload_app doesn't default to true if more than 1 worker #3044

x-yuri opened this issue Dec 24, 2022 · 3 comments · May be fixed by #3297
Labels
bug
Milestone
7.0.0

Comments

@x-yuri
Copy link

x-yuri commented Dec 24, 2022 •
edited

Describe the bug
The comment and the code suggest that if there are more than 1 worker, preload_app should be true, but it isn't.

Puma config:
The command line option is -w 2.

To Reproduce
a.sh:

docker run --rm --init -it alpine:3.17 sh -euc '
  with_config=$1
  apk add build-base ruby ruby-dev ruby-bundler
  echo "
    source \"https://rubygems.org\"
    gem \"puma\", \"6.0.1\"
  " > Gemfile
  echo "
    run lambda { |env| [200, {\"content-type\" => \"text/plain\"}, [\"OK\"]] }
  " > config.ru
  echo "
    preload_app!
  " > config.rb
  bundle
  puma -w 2 ${with_config:+-C config.rb}
' - "${1-}"
$ sh a.sh
...
[136] Puma starting in cluster mode...
[136] * Puma version: 6.0.1 (ruby 3.1.3-p185) ("Sunflower")
[136] *  Min threads: 0
[136] *  Max threads: 5
[136] *  Environment: development
[136] *   Master PID: 136
[136] *      Workers: 2
[136] *     Restarts: (✔) hot (✔) phased
[136] * Listening on http://0.0.0.0:9292
[136] Use Ctrl-C to stop
[137] + Gemfile in context: /Gemfile
[138] + Gemfile in context: /Gemfile
[136] - Worker 1 (PID: 138) booted in 0.02s, phase: 0
[136] - Worker 0 (PID: 137) booted in 0.02s, phase: 0

$ sh a.sh with-config
[136] Puma starting in cluster mode...
[136] * Puma version: 6.0.1 (ruby 3.1.3-p185) ("Sunflower")
[136] *  Min threads: 0
[136] *  Max threads: 5
[136] *  Environment: development
[136] *   Master PID: 136
[136] *      Workers: 2
[136] *     Restarts: (✔) hot (✖) phased
[136] * Preloading application
[136] * Listening on http://0.0.0.0:9292
[136] Use Ctrl-C to stop
[137] + Gemfile in context: /Gemfile
[136] - Worker 0 (PID: 137) booted in 0.0s, phase: 0
[138] + Gemfile in context: /Gemfile
[136] - Worker 1 (PID: 138) booted in 0.0s, phase: 0

This happens because at this point @options[:workers] is 0, since file options become available here, and user options here.

Just in case, this has nothing to do with docker.

Expected behavior
Either it should default to true, or that piece should be removed.

Desktop (please complete the following information):

  • OS: Linux
  • Puma Version 6.0.1
@dentarg
Copy link
Member

dentarg commented Dec 24, 2022 •
edited

Thanks for this great summary.

I've actually come to the same conclusion before, at #2481 (comment) and in #2483. I think what was discussed in #2483 for Puma 6 must have been forgotten. Something for Puma 7? 😂

Maybe this issue can be the issue tracking it this time...

@dentarg
Copy link
Member

dentarg commented Dec 24, 2022

So, Puma will preload your app if you start it using WEB_CONCURRENCY=N puma ... (N >= 2)

@choallin
Copy link

choallin commented Feb 6, 2023

App preloading (in cluster mode) would solve the issue #2818 too as this would make the localhost gem available.

@dentarg dentarg added this to the 7.0.0 milestone Aug 31, 2023
joshuay03 added a commit to joshuay03/puma that referenced this issue Dec 24, 2023
@joshuay03
[Fix puma#3044] Set conditional config defaults after CLI options are…
f977233 
… parsed
@joshuay03 joshuay03 linked a pull request Dec 24, 2023 that will close this issue
Open
7 tasks
joshuay03 added a commit to joshuay03/puma that referenced this issue Dec 24, 2023
@joshuay03
[Fix puma#3044] Set conditional config defaults after CLI options are…
3ac0f65 
… parsed
joshuay03 added a commit to joshuay03/puma that referenced this issue Dec 24, 2023
@joshuay03
[Fix puma#3044] Set conditional config defaults after CLI options are…
d4f9f5d 
… parsed
joshuay03 added a commit to joshuay03/puma that referenced this issue Dec 24, 2023
@joshuay03
[Fix puma#3044] Set conditional config defaults after CLI options are…
0b9d9ed 
… parsed and config files are loaded
joshuay03 added a commit to joshuay03/puma that referenced this issue Dec 24, 2023
@joshuay03
[Fix puma#3044] Set conditional config defaults after CLI options are…
3caaef8 
… parsed and config files are loaded
joshuay03 added a commit to joshuay03/puma that referenced this issue Dec 25, 2023
@joshuay03
[Fix puma#3044] Set conditional config defaults after CLI options are…
dbb42e6 
… parsed and config files are loaded
joshuay03 added a commit to joshuay03/puma that referenced this issue Jan 2, 2024
@joshuay03
[Fix puma#3044] Set conditional config defaults after CLI options are…
b1d0c46 
… parsed and config files are loaded
joshuay03 added a commit to joshuay03/puma that referenced this issue Jan 2, 2024
@joshuay03
[Fix puma#3044] Set conditional config defaults after CLI options are…
f9ea9e8 
… parsed and config files are loaded
joshuay03 added a commit to joshuay03/puma that referenced this issue Jan 2, 2024
@joshuay03
[Fix puma#3044] Set conditional config defaults after CLI options are…
b1d932f 
… parsed and config files are loaded
joshuay03 added a commit to joshuay03/puma that referenced this issue Jan 2, 2024
@joshuay03
[Fix puma#3044] Set conditional config defaults after CLI options are…
c9f75ec 
… parsed and config files are loaded
joshuay03 added a commit to joshuay03/puma that referenced this issue Jan 2, 2024
@joshuay03
[Fix puma#3044] Set conditional config defaults after CLI options are…
b081b02 
… parsed and config files are loaded
joshuay03 added a commit to joshuay03/puma that referenced this issue Jan 2, 2024
@joshuay03
[Fix puma#3044] Set conditional config defaults after CLI options are…
13f06f1 
… parsed and config files are loaded
joshuay03 added a commit to joshuay03/puma that referenced this issue Jan 2, 2024
@joshuay03
[Fix puma#3044] Set conditional config defaults after CLI options are…
4b7d90d 
… parsed and config files are loaded
joshuay03 added a commit to joshuay03/puma that referenced this issue Jan 2, 2024
@joshuay03
[Fix puma#3044] Set conditional config defaults after CLI options are…
fe5dec5 
… parsed and config files are loaded
@dentarg dentarg mentioned this issue Jan 8, 2024
Closed
@dentarg dentarg mentioned this issue Jan 31, 2024
Merged
joshuay03 added a commit to joshuay03/puma that referenced this issue Apr 16, 2024
@joshuay03
[Fix puma#3044] Set conditional config defaults after CLI options are…
ace3c3f 
… parsed and config files are loaded
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
7.0.0
4 participants
@dentarg @x-yuri @nateberkopec @choallin