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

Cyclic restarts with prune_bundler and Bundler.setup (require) and workers > 1 #2319

Closed
AlexWayfer opened this issue Jul 23, 2020 · 11 comments · Fixed by #2323
Closed

Cyclic restarts with prune_bundler and Bundler.setup (require) and workers > 1 #2319

AlexWayfer opened this issue Jul 23, 2020 · 11 comments · Fixed by #2323
Labels

Comments

@AlexWayfer
Copy link
Contributor

AlexWayfer commented Jul 23, 2020

Describe the bug

I'm trying to start a server via pumactl, but it fails in an infinity loop without backtraces:

> bundle exec pumactl start
[191794] * Pruning Bundler environment
[DEPRECATED] `Bundler.with_clean_env` has been deprecated in favor of `Bundler.with_unbundled_env`. If you instead want the environment before bundler was originally loaded, use `Bundler.with_original_env` (called at /home/alex/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/puma-4.3.5/lib/puma/launcher.rb:289)
[191794] * Pruning Bundler environment
[DEPRECATED] `Bundler.with_clean_env` has been deprecated in favor of `Bundler.with_unbundled_env`. If you instead want the environment before bundler was originally loaded, use `Bundler.with_original_env` (called at /home/alex/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/puma-4.3.5/lib/puma/launcher.rb:289)
[191794] * Pruning Bundler environment
[DEPRECATED] `Bundler.with_clean_env` has been deprecated in favor of `Bundler.with_unbundled_env`. If you instead want the environment before bundler was originally loaded, use `Bundler.with_original_env` (called at /home/alex/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/puma-4.3.5/lib/puma/launcher.rb:289)
...

Puma config:

# frozen_string_literal: true

## Require gems
require 'bundler/setup'
Bundler.require

prune_bundler

rackup 'config.ru'

workers 2

To Reproduce

Bundler.require (or .setup) + prune_bundler + workers > 1.

Run it with:

bundle exec pumactl start

Expected behavior

Server starts or some descriptive errors (once, not in a loop).

Desktop (please complete the following information):

  • OS: Arch Linux
  • Puma Version: 4.3.5 and 5.0.0.beta1
@AlexWayfer AlexWayfer changed the title Cyclic restarts with prune_bundler and Bundler.setup (require) Cyclic restarts with prune_bundler and Bundler.setup (require) and workers > 1 Jul 23, 2020
@dentarg
Copy link
Member

dentarg commented Jul 23, 2020

I can't reproduce, I tried both with Ruby 2.6.6 + Bundler 2.1.2 and Ruby 2.7.1 + Bundler 2.1.4, on macOS 10.14. Code

$ bundle exec pumactl start
Puma starting in single mode...
* Version 4.3.5 (ruby 2.6.6-p146), codename: Mysterious Traveller
* Min threads: 0, max threads: 16
* Environment: development
* Listening on tcp://0.0.0.0:9292
Use Ctrl-C to stop
^C- Gracefully stopping, waiting for requests to finish
=== puma shutdown: 2020-07-23 17:12:50 +0200 ===
- Goodbye!
$ bundle exec pumactl start
Puma starting in single mode...
* Version 4.3.5 (ruby 2.7.1-p83), codename: Mysterious Traveller
* Min threads: 0, max threads: 16
* Environment: development
* Listening on tcp://0.0.0.0:9292
Use Ctrl-C to stop
^C- Gracefully stopping, waiting for requests to finish
=== puma shutdown: 2020-07-23 17:13:34 +0200 ===
- Goodbye!

@AlexWayfer
Copy link
Contributor Author

Ruby 2.7.1 (rbenv), Arch Linux, Bundler 2.1.4.

My code the same as your.

Unfortunately, I even don't know how to debug, add some backtraces or verbose. There are no such options for pumactl.

@AlexWayfer
Copy link
Contributor Author

@dentarg, please, ensure that you have puma.rb inside config/ directory (default config path), or please specify bundle exec pumactl start -F puma.rb. Without this I have the same successful output as your, but there is no info about 2 workers (config just not loaded).

@AlexWayfer
Copy link
Contributor Author

I'm trying to debug. Working with master-version. I have such output:

[18292] * Pruning Bundler environment
[18292] * Pruning Bundler environment
[18292] * Pruning Bundler environment
[18292] * Pruning Bundler environment
...

Even when I've added log '* Pruned' in this method after with_unbundled_env do block.

So, the problem in this block (or with_unbundled_env method).

@AlexWayfer
Copy link
Contributor Author

New info: the problem exactly in Kernel.exec: https://github.com/puma/puma/blob/47f933d/lib/puma/launcher.rb#L310

I've got args:

["/home/alex/.rbenv/versions/2.7.1/bin/ruby", "/home/alex/Projects/ruby/puma/bin/puma-wild", "-I", "/home/alex/Projects/ruby/puma/lib:/home/alex/Projects/ruby/extensions/x86_64-linux/2.7.0/puma-5.0.0.beta1", "nio4r:2.5.2", "-C", "puma.rb", {:close_others=>false}]

and the result command for me looks like:

/home/alex/.rbenv/versions/2.7.1/bin/ruby /home/alex/Projects/ruby/puma/bin/puma-wild -I /home/alex/Projects/ruby/puma/lib:/home/alex/Projects/ruby/extensions/x86_64-linux/2.7.0/puma-5.0.0.beta1 nio4r:2.5.2 -C puma.rb

And it cyclic fails in the same way with the same output from launcher.rb.

I don't know how to debug further.

@AlexWayfer
Copy link
Contributor Author

AlexWayfer commented Jul 27, 2020

Probably it's a simple recursion, what do you think? In any case, it looks like critical error.

@AlexWayfer
Copy link
Contributor Author

Yep, just simple check for ENV['PUMA_BUNDLER_PRUNED'] inside prune_bundler method fixes it. Weird. I'm going to prepare a PR for this.

AlexWayfer added a commit to AlexWayfer/puma that referenced this issue Jul 27, 2020
@MSP-Greg
Copy link
Member

@AlexWayfer

What happens if you remove the following from Puma config?

## Require gems
require 'bundler/setup'
Bundler.require

@AlexWayfer
Copy link
Contributor Author

@AlexWayfer

What happens if you remove the following from Puma config?

## Require gems
require 'bundler/setup'
Bundler.require

It works, but I require in real-life application another file, like config/main.rb, which does Bundler.require (for git-versions of gems) and loads Puma-related (YAML) config. So… I'm not sure is it OK to have Bundler.require and prune_bundler in one runtime or what I should do.

@AlexWayfer
Copy link
Contributor Author

Let's discuss this issue a bit more, please. The decision affect my web applications a lot.

From here: https://github.com/puma/puma/blob/5d02e45/lib/puma/dsl.rb#L572-L585

When set, if Puma detects that it's been invoked in the context of Bundler, it will cleanup the environment and re-run itself outside the Bundler environment, but directly using the files that Bundler has setup.

This means that Puma is now decoupled from your Bundler context and when each worker loads, it will be loading a new Bundler context and thus can float around as the release dictates.

In this context Bundler.require seems non-sense, but it still can help to load some configuration (especially for Puma) via third-party gems (from Gemfile), but (!) it may cause conflicts if some config loading and third-party gem changed in phased-restart or so.

I'm not sure for what this options is. Like… puma master process will be "dirty" in the Bundler environment (gems versions) at its start, but then it will not be changed but its workers will be with a fresh Bundler environment.

Like… what about case if I want to update the Puma from v4 to v5, for example? master process will be at v4 and it will try to start workers v5? I guess conflicts are possible in such cases.

Please, describe, for what cases this option, when conflicts (problems) will be possible and when everything will be 100% OK.

@AlexWayfer
Copy link
Contributor Author

AlexWayfer commented Aug 3, 2020

But in the same time I guess Bundler.setup is similar to bundle exec pumactl, is not it? And the last one is OK for puma, as I know. I still don't understand how it works and how I should design my applications.

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