Ensure PIDFILE has pids of all running workers #1733
Conversation
The workers now append to the PIDFILE instead of overwriting it. Adding file locking to handle race conditions when multiple workers are started using resque:workers. To clear any older data in the specified PIDFILE, it is now truncated once when the rake tasks resque:work and resque:workers are called. Created a proxy task resque:run to achive this. Fixes resque#1721
|
ping |
There was a problem hiding this comment.
Could you help me understand the use case here more fully? I don't see why we have this task in the first place.
To get this merged:
- Could you respond to the message about why we aren't building the PIDFILE logic into
run? - Could you add a changelog entry?
- Rebase on master so CI runs
Thanks for the contribution!
| @@ -20,6 +20,15 @@ | |||
| worker.work(ENV['INTERVAL'] || 5) # interval, will block | |||
| end | |||
|
|
|||
| desc "Start a Resque worker" | |||
| task :work do | |||
There was a problem hiding this comment.
Why are we defining a whole new task here instead of bundling this into run?
| @@ -28,9 +37,13 @@ | |||
| abort "set COUNT env var, e.g. $ COUNT=2 rake resque:workers" | |||
| end | |||
|
|
|||
| if ENV['PIDFILE'] && File.file?(ENV['PIDFILE']) | |||
| File.truncate(ENV['PIDFILE'], 0) | |||
| end | |||
There was a problem hiding this comment.
workers should only be used in dev mode (I don't fully understand why we have this task in the first place). Are you using PIDFILE in devmode? What is your use case?
| Rake::Task['resque:run'].reenable | ||
| Rake::Task['resque:run'].invoke | ||
| end | ||
|
|
||
| desc "Start multiple Resque workers. Should only be used in dev mode." |
There was a problem hiding this comment.
Do you understand the use case for this? Do you use this in production? Resque is designed around forking, so it seems strange that we support this. Also, shelling out to a rake task feels a bit dangerous to me for production-quality deployments.
Hey @iloveitaly! The issue this change tries to fix is #1721. We have a requirement to automate starting and stopping of resque workers. When starting multiple workers in the background, to stop each of the workers, we need the PIDs of the workers and so it would be great to have all the PIDs in the PIDFILE. This is quite an old PR. I'll need some time to look at the changes I made and then I can address the comments. Thanks for reviewing the PR 😄 |
@z0marlin sound great! |
The workers now append to the PIDFILE instead of overwriting it.
Adding file locking to handle race conditions when multiple
workers are started using resque:workers.
To clear any older data in the specified PIDFILE, it is now truncated
once when the rake tasks resque:work and resque:workers are called.
Created a proxy task resque:run to achive this.
Fixes #1721