Add worker groups with different settings

[severinraez]

A follow up to https://github.com/salsify/delayed_job_worker_pool/pull/3

I've changed the terminology to worker_groups so we have clear semantics. The worker pool are all the workers, a worker group is a subset of workers sharing the same options.

[severinraez]

Thanks for your review, @jturkel I agree with all the points you raised. Worker group names will help a lot with knowing what's going on. In that spirit, I plan to also set the name_prefix (https://github.com/collectiveidea/delayed_job/blob/master/lib/delayed/worker.rb#L33) of the DJ workers accordingly.

[severinraez]

Hi @jturkel

I've added two commits, handling everything but the worker group names in the first and adding worker group names in the second. Please have a look. I'd be happy to follow up on additional feedback if you have any and then squash the commits to allow for a clean merge.

[severinraez]

Thanks :)

I've implemented your suggested changes. I've thought about a default worker group name before and went for explicit namings just to have a decision. You bringing this up too leads me to believe a default might be simpler.

I did not (yet?) add a test to spec/delayed_job_worker_pool_spec.rb. I had another look, my thinking here is as follows:

• The existing test makes sure no jobs queued on queue inactive are run - thus we can be sure no worker is working all queues.
• Left unchecked is the following contrived edge case: If we'd somehow build a union of the specified per-worker group queues and passed that to each worker group, the test would be green. In dsl_spec.rb we already examine which worker groups get which options.

Thinking further, there's a whole class of checks we don't do around making sure the specified options are really passed through to Delayed::Worker (sleep_delay and so on). Adding those would bloat delayed_job_worker_pool_spec.rb.

I think the proper way to do this would be to separate worker instantiation from forking and looking after children by refactoring WorkerPool, then having the worker instantiation part thoroughly tested (maybe by mocking Delayed::Worker).

Thus, I suggest we let the test be as is and merge this PR (I can squash the commits first). I can open an issue summarizing the refactoring and opportunities for more thorough testing if you're interested. I'm afraid I'll be too short on time to implement this in the coming months. That being said, if you feel more confident with the "worker working all specified queues" also being tested, I can come up with a solution and add a commit. Might get a little tricky to tell from the outside, we'd need to find out which PID is working which queues from the logs (which would have to be split so the forked processes don't write to the same file and garble the output every now and then, leading to flaky tests).

[severinraez]

Thank you too for your guidance, that improved the quality a lot.

I've squashed and rebased :)