defunkt/resque — pull request #895 by jonhyman (2013-03-22 23:50:06+00:00)
Title: Adds support for multiple jobs per fork.

I pushed out 1.24.0 to my worker machines and the load averages on my machine spiked. Downgrading fixed the issue, so I did some investigation as to what was going on. We have lots of small running jobs, so we use the [resque-multi-job-forks](https://github.com/stulentsev/resque-multi-job-forks) gem, which monkey patches `perform` to change the value of `@cant_fork`. Since the forking code now uses Kernel.fork's block form, the monkey patch does nothing. All the different versions of "multiple jobs per fork" gems I could find seem to be broken with 1.23.1 and higher due to this change.

I'm not a big fan of monkey patching due to things exactly like this. As such, I thought that it would make sense for Resque to support this natively.

This pull request adds support for multiple jobs per fork with a timed limit on the fork's life. I also did a bit of refactoring to move responsibilities around to things that I think made more sense (e.g., why was `run_hook :after_fork` in `perform` but `run_hook :before_fork` is in `fork`...). In doing so, I tried to separate out whether or not the thing doing work knows about it being a child or not. That is, stuff like:

```ruby
if @child = fork(job) do
unregister_signal_handlers if term_child
reconnect
perform(job, &block)
exit! unless run_at_exit_hooks
end

srand # Reseeding
procline "Forked #{@child} at #{Time.now.to_i}"
begin
Process.waitpid(@child)
rescue SystemCallError
nil
end
job.fail(DirtyExit.new($?.to_s)) if $?.signaled?
else
reconnect
perform(job, &block)
end
```

has duplicate code in the else (reconnect and perform). Also, that seems to be buggy anyway since you'll reconnect to Redis before each job when `@cant_fork = false` which doesn't seem necessary.

There were a few other things in this pull I didn't do that I think might make sense for tech debt issues for v2:
* The $TESTING concept in `will_fork?` is pretty bad, it makes it so you have to use the underlying ivar elsewhere (such as `#fork` uses `@cant_fork` because if you change it to `will_fork?` you'll break tests). Instead, `will_fork?` should just be stubbed out for tests that don't want to fork. Typically, adding in code to production files for the purposes of testing is a smell. This will also let you consolidate the `ENV["FORK_PER_JOB"] == 'false'` code since it seems like a bug that `#fork` has `return if @cant_fork` instead of `return if @cant_fork || ENV["FORK_PER_JOB"] == 'false'` -- I didn't make that change but that seems like a bug.
* `run_hook :before_first_fork` runs at startup, so even if ENV["FORK_PER_JOB"] == 'false' or @cant_fork = true, that hook will run. Seems like a bug, but was fearful of removing without a bump to 1.25.0. Added a TODO.

Let me know your thoughts, I'm happy to do whatever cleanup you think is necessary to get this in. Regardless, I'll probably have to either use my fork in production or stay on 1.23.0 due to the CPU issues given that the multiple jobs per fork gems all break.