-
Notifications
You must be signed in to change notification settings - Fork 307
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
pidfile handling #44
Comments
|
This doesn't really work in continuous deployment (especially over different stages), so you would manually need to stop sidekiq and then deploy. As the number of processes is in the repo(deploy.rb) checking in a changed process count would not adapt sidekiq properly in the automatic deployment. |
Ok, send a PR. let have always sidekiq.pid, sidekiq-1.pid |
Ok, cool, this would solve the problem on scaling up. It just came into my mind that scaling down would likely still create problems, or not? If you have 4 processes and then go back to 3, would it stop all processes or only the 3 because of the sidekiq_processes set to 3 ? |
That the responsibility of the deployer, maybe adding note in the Readme will be enough. |
capistrano-sidekiq is currently overriding the pidfile setting in sidekiq.yml, and that is not acceptable. Can we please revert this change and send it back to w1mvy to fix it so that it will only behave in this (terrible) way if a config option is set? |
I opened a new issue on this: #156. |
Sidekiq currently uses two different methods to generate the name for pidfiles: If only one Sidekiq process is running the file is only called
sidekiq.pid
, if two or more processes are running the pid files are calledsidekiq-0.pid
,sidekiq-1.pid
, ... This leads to problems in deployment, as changing from 1 to 2 processes or back to 1 always leads to not stopped sidekiq workers.So when e.g. changing the sidekiq_processes from 1 to 2 not the process stored in the
sidekiq.pid
is stopped but the non-existing processessidekiq-0/1.pid
. So the old process keeps running.The text was updated successfully, but these errors were encountered: