Combine remove_batched_job and batch_complete? into a single function so...

[jakemack]

... that their operations are both surrounded by the same mutex

In the old code, the if one worker finished the call to remove_batched_job and got interrupted before asking for the mutex in batch_complete?, then another worker could execute remove_batched_job before the first executed batch_complete?, bringing the batch size to zero for BOTH workers. Admittedly a rare case, but might have happened to me once already. I actually had the after_batch hook performed four times.

I haven't added tests for this as I'm not sure how to simulate threads being interrupted, but all of the current tests still pass.

[drfeelngood]

Instead of creating another method, how about returning the list size from remove_batched_job. Then, check the returned value in the after_perform_batch hook.

For example:

# ...
if remove_batched_job(id, *args) == 0
# ...

[jakemack]

Sounds acceptable, I had just done it my way in case you wanted the remove_batch_job return value to be the value returned by redis.lrem.

As a side note, batch_complete? seems extraneous at this point as it's not used anywhere else. Shall I remove that as well as part of this refactor?

[drfeelngood]

Let's keep batch_complete? for meow. It can be used for batch status updates etc. Well, possibly buggy status updates from what you've encountered.

[jakemack]

Alright, left batch_complete? in and pushed the refactor. Added a test for testing the return value of remove_batched_job.

[drfeelngood]

Awesome sauce. I'll bump the version and publish a new shinny gem.

[jakemack]

Slick