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

Restarting via USR2 forgets about ARGV arguments #1363

Closed
perlun opened this issue Jul 11, 2017 · 13 comments
Closed

Restarting via USR2 forgets about ARGV arguments #1363

perlun opened this issue Jul 11, 2017 · 13 comments
Labels

Comments

@perlun
Copy link
Contributor

perlun commented Jul 11, 2017

Steps to reproduce

$ bundle exec uxfactory --port 8777 --color
INFO  [2017-07-11 12:19:18.988] UxFactoryServer: Version 8.2.2 started (powered by Puma 3.8.2 and Ruby MRI 2.4.1).
INFO  [2017-07-11 12:19:18.991] UxFactoryServer: Listening at http://localhost:8777.
INFO  [2017-07-11 12:19:19.012] UxFactory::BackgroundWorkersManager: Started 1 background workers (ConsoleLogger).
INFO  [2017-07-11 12:19:40.721] UxFactoryServer: Version 8.2.2 started (powered by Puma 3.8.2 and Ruby MRI 2.4.1).
INFO  [2017-07-11 12:19:40.724] UxFactoryServer: Listening at http://localhost:8000.
INFO  [2017-07-11 12:19:40.739] UxFactory::BackgroundWorkersManager: Started 1 background workers (ConsoleLogger).
FATAL [2017-07-11 12:19:40.792] UxFactoryServer: Address already in use - bind(2) for "0.0.0.0" port 8000. Ensure that port 8000 is not occupied by some other program.
INFO  [2017-07-11 12:19:40.923] UxFactory::BackgroundWorkersManager: Stopped 1 background workers (ConsoleLogger)

(uxfactory is our own in-house application server, built on top of Sinatra, puma etc)

Expected behavior

When restarting, the same port as used when starting puma should be used.

Actual behavior

It fell back to using port 8000, which is the default port used by uxfactory.

System configuration

Ruby version: 2.4.1
Rails version: N/A

Extended description

Here is the code for our Puma startup:

  def server
    @server ||= Rack::Server.new(
      config: File.join(UxFactory::ServerConfig.instance_path, 'server/config.ru'),
      server: 'puma',

      Port: UxFactory::ServerConfig.port,
      Silent: true
    )
  end

I wonder if the problem here could be with command line arguments or something? As can be seen in this case, the specific port number (8777) is being passed in as a command line argument to the initial process. When the USR2 handler detects a restart coming, does it pass in the original command line arguments which were passed to the parent process or how does it work?

@perlun
Copy link
Contributor Author

perlun commented Jul 31, 2017

(The same behavior exists when restarting via USR1 btw)

I experimented a bit and created a dummy config.ru file, and started puma like this instead:

$ bundle exec puma -b tcp://127.0.0.1:9292

Then I gave it a USR1 signal:

Puma starting in single mode...
* Version 3.9.1 (ruby 2.4.1-p111), codename: Private Caller
* Min threads: 0, max threads: 16
* Environment: development
* Listening on tcp://127.0.0.1:9292
Use Ctrl-C to stop
* phased-restart called but not available, restarting normally.
* Restarting...
Puma starting in single mode...
* Version 3.9.1 (ruby 2.4.1-p111), codename: Private Caller
* Min threads: 0, max threads: 16
* Environment: development
* Inherited tcp://127.0.0.1:9292
Use Ctrl-C to stop

As can be seen, it used the right port here; it properly handled the USR1 signal.

So I am suspecting it is specifically when puma is started up via the Rack::Server.new approach that the options originally passed is not properly propagated through to the new process when it receives the restart signal, or something similar to this. @nateberkopec, any ideas on how I might help nailing it down further?

@stereobooster
Copy link
Contributor

any ideas on how I might help nailing it down further?

small example with reproduction based on Rack::Server.new?

@perlun
Copy link
Contributor Author

perlun commented Aug 8, 2017

@stereobooster

small example with reproduction based on Rack::Server.new?

Here goes:

# config.ru

class HelloWorld
  def call(env)
    [200, {"Content-Type" => "text/html"}, ["Hello World!"]]
  end
end

run HelloWorld.new

# server.rb
#!/usr/bin/env ruby

require 'rack'

server = Rack::Server.new(
  config: 'config.ru',
  server: 'puma',

  Port: ARGV[0]
)
server.start

Here is the output when running this. Note how it forgets the port number when restarted. I would guess this is because the ARGV values are not propagated to the new process on restart or something? If you have a workaround, I'm more than willing to try it!

$ bundle exec ruby server.rb 12345
Puma starting in single mode...
* Version 3.9.1 (ruby 2.4.1-p111), codename: Private Caller
* Min threads: 0, max threads: 16
* Environment: development
* Listening on tcp://0.0.0.0:12345
Use Ctrl-C to stop
* Restarting...
Puma starting in single mode...
* Version 3.9.1 (ruby 2.4.1-p111), codename: Private Caller
* Min threads: 0, max threads: 16
* Environment: development
* Listening on tcp://0.0.0.0:9292
* Closing unused inherited connection: tcp://0.0.0.0:12345
Use Ctrl-C to stop

@perlun perlun changed the title Restarting via USR2 uses the wrong port Restarting via USR2 forgets about ARGV arguments Aug 10, 2017
@perlun
Copy link
Contributor Author

perlun commented Aug 22, 2017

@stereobooster Any ideas?

@nateberkopec nateberkopec removed the bug label Aug 22, 2017
@nateberkopec
Copy link
Member

Yeah, starting a server via Rack::Server like this and then expecting the port to be remembered is Not Supported.

Example of something you could do:

require 'puma/cli'

cli = Puma::CLI.new(["--port=12345"])

cli.run

An alternative might be to use Puma::Launcher directly with a configuration object.

@perlun
Copy link
Contributor Author

perlun commented Sep 6, 2017

Thanks @nateberkopec, sorry for the delay on my part. Our problem is that we do not only use the command line arguments for the port number, but we also have other things like --license-file and --apps-folder being passed in via command line arguments.

We can use ENV variables for all of this, but it feels a bit clumsy. Are you saying this is the expected behavior, the command line arguments are expected to be erased between restarts? Is this something that could be fixed, would you accept PRs that changes the behavior or do you consider this to be "the way it should behave"?

@nateberkopec
Copy link
Member

Are you saying this is the expected behavior, the command line arguments are expected to be erased between restarts?

Well, it does work when you use Puma::CLI, which is what is supported.

@perlun
Copy link
Contributor Author

perlun commented Sep 13, 2017

Well, it does work when you use Puma::CLI, which is what is supported.

Ah, I see - thanks for making that super-clear! Keep up the great work here and elsewhere, @nateberkopec. 👍

@perlun
Copy link
Contributor Author

perlun commented Sep 19, 2017

@nateberkopec Trying out this approach now, but I am struggling with getting it accepting my config.ru:

    def start_server
      config_path = File.join(UxFactory::ServerConfig.instance_path, 'server/config.ru')
      cli = Puma::CLI.new(["--port=12345 #{config_path}"])
      cli.run
    end

...but this is what it outputs:

Puma starting in single mode...
* Version 3.10.0 (ruby 2.4.2-p198), codename: Russell's Teapot
* Min threads: 0, max threads: 16
* Environment: development
ERROR: No application configured, nothing to run

The config.ru looks roughly like this, somewhat simplified:

rackup = Rack::Builder.app do
  use Rack::Chunked

  use Rack::Deflater unless UxFactory::ServerConfig.disable_compression
  run UxFactory::Server::SinatraApp.new
end

run rackup

bundle exec puma full/path/config.ru gives other output (since not all the dependencies are loaded if I do it that way). Any obvious thing I'm doing wrong here?

@perlun
Copy link
Contributor Author

perlun commented Sep 28, 2017

Anyone?

@nateberkopec
Copy link
Member

@perlun Puma::CLI.new takes an array of arguments: Puma::CLI.new(["--port=12345", config_path])

@perlun
Copy link
Contributor Author

perlun commented Oct 4, 2017

Doh! Silly me. 😃 Thanks, will try that instead.

@perlun
Copy link
Contributor Author

perlun commented Oct 5, 2017

FWIW, the Puma::CLI.new approach did not work, since it will start the original binary with the Puma arguments on restart - which doesn't work, since these arguments are not what the original executable expected:

$ bundle exec uxfactory --port 12345
INFO  [2017-10-05 11:09:10.984] UxFactoryServer: Version 8.7.3 started (powered by Puma 3.10.0 and ruby 2.4.2)
INFO  [2017-10-05 11:09:10.984] UxFactoryServer: Listening at http://localhost:12345.
INFO  [2017-10-05 11:09:10.997] UxFactory::BackgroundWorkersManager: Started 1 background workers (ConsoleLogger).
ERROR: "uxfactory start" was called with arguments ["--quiet", "/Volumes/extra/git/ecraft/ecraft.uxfactory.server-8.x/src/server/config.ru"]
Usage: "uxfactory start [--port=12345] [--license-file=/my/license/file] [--apps_folder=~/git]"

Will try the Puma::Launcher approach instead, which seems (at first glance) to work better (but involves a bit more plumbing.) Puma::Launcher worked fine, thanks for suggesting that.

nateberkopec pushed a commit that referenced this issue Oct 9, 2017
I noted this while trying to apply suggestions from #1363.
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