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

Don't kill old master process. Allow unicorn to either upgrade or rev… #113

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dsturrock
Copy link

@dsturrock dsturrock commented Feb 22, 2021

…ert it

The oldsig QUIT instruction no longer seems to be necessary. Sending USR2 to unicorn is enough to spin up the new master and either drop the old master process once the new master starts successfully or roll back to the old master if the new master fails.

The upgrade step also sometimes incorrectly returns a success message when unicorn has not started, or failed to upgrade. Currently it waits two seconds then attempts to send a 0 signal to the PID. This has been updated to wait 20 seconds to give unicorn plenty of time to start, and will also check to see if the PID changed or if it was reset back to the old master, returning a failure message to indicate this.

This may be a breaking change if there are older versions of unicorn/other gems that DO require the oldsig QUIT to be sent, so if you'd prefer this could be implemented as a 'safe_upgrade' command rather than changing the upgrade path.

#114

@rhomeister
Copy link
Contributor

Hi @dsturrock,

I'm no longer maintaining this gem. We have moved on from using capistrano, and switched to kubernetes. Since it is not possible for me to test these changes anymore, and I don't want to take the risk of potentially causing issues for other users of this gem, I'd encourage you to use the fork that you've created in your projects.

Thanks for your contribution. This issue will remain open so others will be directed to your fork if they experience the same problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants