Skip to content

Commit

Permalink
Revert workaround for puma 4.1.0 output bug (#48)
Browse files Browse the repository at this point in the history
Puma 4.1.0 had a bug where its output was not properly detached prior to
daemonizing. To work around this, I added output redirection to the
`bundle exec puma` command to force the output to be detached from the
console.

Now that puma has fixed this bug, I am removing the output redirection.
The redirection had a downside: it hid messages from the console that
were valuable for troubleshooting. For example if puma had an invalid
config file that prevented it from starting, the error would have been
hidden.

To avoid a breaking change, I am keeping the `:puma_stdout_path` and
`:puma_stderr_path` tomo settings around. But rather than using them
for output direction, they will be passed to puma's `--redirect-stdout`
and `--redirect-stderr` options. These also redirect puma's output, but
_after_ puma has successfully been started and daemonized (i.e after
tomo's deploy has finished).
  • Loading branch information
mattbrictson authored Sep 9, 2019
1 parent becf0df commit 7e4818d
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 10 deletions.
14 changes: 14 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,20 @@ By default, tomo uses the ["accept-new"](https://www.openssh.com/txt/release-7.6
set ssh_strict_host_key_checking: true # or false
```

#### Why does my deploy hang after starting puma?

Puma 4.1.0 [has a bug](https://github.com/puma/puma/issues/1906) where its output isn't properly detached prior to daemonzing. This causes tomo to hang waiting for output. You may see something like this prior to the deploy freezing:

```
Puma starting in single mode...
* Version 4.1.0 (ruby 2.6.4-p104), codename: Fourth and One
* Min threads: 5, max threads: 5
* Environment: production
* Daemonizing...
```

To fix, upgrade to puma 4.1.1 or newer.

## Support

This project is a labor of love and I can only spend a few hours a week maintaining it, at most. If you'd like to help by submitting a pull request, or if you've discovered a bug that needs my attention, please let me know. Check out [CONTRIBUTING.md](https://github.com/mattbrictson/tomo/blob/master/CONTRIBUTING.md) to get started. Happy hacking! —Matt
Expand Down
12 changes: 6 additions & 6 deletions docs/plugins/puma.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ The puma plugin provides basic, zero-configuration support for the default Rails

## Settings

| Name | Purpose | Default |
| -------------------- | ------------------------------------------------------------ | ------------------------------- |
| `puma_control_token` | Auth token to use when connecting to the puma control server | `"tomo"` |
| `puma_control_url` | Connection URL for the puma control server | `"tcp://127.0.0.1:9293"` |
| `puma_stdout_path` | File where puma's stdout will be written | `"%<shared_path>/log/puma.out"` |
| `puma_stderr_path` | File where puma's stderr will be written | `"%<shared_path>/log/puma.err"` |
| Name | Purpose | Default |
| -------------------- | ----------------------------------------------------------------------------- | ------------------------------- |
| `puma_control_token` | Auth token to use when connecting to the puma control server | `"tomo"` |
| `puma_control_url` | Connection URL for the puma control server | `"tcp://127.0.0.1:9293"` |
| `puma_stdout_path` | File where puma's stdout will be written using the `--redirect-stdout` option | `"%<shared_path>/log/puma.out"` |
| `puma_stderr_path` | File where puma's stderr will be written using the `--redirect-stderr` option | `"%<shared_path>/log/puma.err"` |

## Tasks

Expand Down
11 changes: 8 additions & 3 deletions lib/tomo/plugin/puma/tasks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,7 @@ def start

remote.chdir(paths.current) do
remote.bundle(
"exec", "puma", "--daemon", *control_options,
raw(">"), paths.puma_stdout,
raw("2>"), paths.puma_stderr
"exec", "puma", "--daemon", *control_options, *output_options
)
end
end
Expand All @@ -51,5 +49,12 @@ def control_options
"--control-token", settings[:puma_control_token]
]
end

def output_options
options = []
options << ["--redirect-stdout", paths.puma_stdout] if paths.puma_stdout
options << ["--redirect-stderr", paths.puma_stderr] if paths.puma_stderr
options.flatten
end
end
end
2 changes: 1 addition & 1 deletion test/tomo/plugin/puma/tasks_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def test_restart_starts_puma_if_pumactl_fails
assert_equal(
"cd /app/current && bundle exec puma --daemon "\
"--control-url tcp://127.0.0.1:9293 --control-token test "\
"> /log/puma.out 2> /log/puma.err",
"--redirect-stdout /log/puma.out --redirect-stderr /log/puma.err",
@tester.executed_scripts.last
)
end
Expand Down

0 comments on commit 7e4818d

Please sign in to comment.