drfeelngood / resque-batched-job

Resque plugin that understands individual jobs can belong to something bigger than themselves
https://rubygems.org/gems/resque-batched-job
MIT License
49 stars 23 forks source link

Add another module that integrates resque-batched-job with resque-retry #11

Closed jakemack closed 4 years ago

jakemack commented 12 years ago

I've created for myself a module called Resque::Plugins::BatchedJobRetry which fixes an incompatibility between resque-batched-job and resque-retry and was wondering if that's something people might like integrated into this gem. The two gems will fail spectacularly together if any job in the batch retries even once, as it will add another job to the batch for the job that is requeued to retry, but on completion it can only remove one job from the batch. Thus, the batch is stuck forever. Here's the actual code to fix this, it's quite simple.

module Resque
  module Plugins
    module BatchedJobRetry
      include Resque::Plugins::BatchedJob
      include Resque::Plugins::Retry

      # Resque hook that handles batching the job. (closes #2)
      #
      # @param [Object, #to_s] id Batch identifier. Any Object that responds to #to_s
      def after_enqueue_batch(id, *args)
        unless redis.exists redis_retry_key(id, *args)
          mutex(id) do |bid|
            redis.rpush(bid, encode(:class => self.name, :args => args))
          end
        end
      end
    end
  end
end

The one issue I see with this code is that if a new job is being added to a batch after an existing job with the same parameters has been requeued to retry it, then it won't push another job onto the batch list and thus only one of them might finish before the batch is complete. This seems to be an issue with the retry gem in general, however, as any two jobs with the exact same parameters will share a retry key in redis unless the identifier function is overridden to provide a unique identifier for each job (which essentially boils down to having unique parameters anyway).

drfeelngood commented 12 years ago

I'm reluctant to add resque-retry as a dependency because it would also add resque-schedule as a dependency. Thats too much overhead in my opinion.

To be honest, I've don't know much about Resque::Retry. I'll do a little investigation myself into the problems your encountering. Maybe we can come up with a non-dependency solution, or simply create a gem that combines the powers of both and resolves any beef.

jakemack commented 12 years ago

Yeah, good point. I didn't want to make a separate gem for that tiny bit of code, but definitely don't want to bring resque-rety in as a dependency. If we can come up with some way to do it without adding a dependency, cool, otherwise I'm fine with just using it myself. Only suggested it because I figured it might be useful for other people.

drfeelngood commented 12 years ago

Maybe checking included/extended modules in after_enqueue and performing hacks based on the results.