mhenrixon / sidekiq-unique-jobs

Prevents duplicate Sidekiq jobs
MIT License
1.46k stars 277 forks source link

Version 6 Ignores Jobs Enqueued in Version 5 #345

Closed chadrschroeder closed 5 years ago

chadrschroeder commented 5 years ago

When upgrading from version 5 to version 6, any jobs that were enqueued on version 5 are removed from their Sidekiq queues and marked as processed in the Sidekiq log but they aren't actually executed. I think this is because the GRABBED key doesn't exist on the jobs that were enqueued on version 5 and SidekiqUniqueJobs silently exits before yielding to the worker.

If possible, it would be nice if version 6 could just pick up and process these jobs. Otherwise, it might be worth documenting that the required upgrade steps are currently:

  1. Stop all processes that can enqueue new jobs that have a unique requirement.
  2. Let all the jobs that have been enqueued on version 5 finish processing.
  3. Deploy version 6.
  4. Restart the processes that enqueue jobs.
mhenrixon commented 5 years ago

I actually have tests saying this should work but it is pretty hard to to test this in an automated way. Have a look at: https://github.com/mhenrixon/sidekiq-unique-jobs/blob/master/spec/integration/sidekiq_unique_jobs/legacy_lock_spec.rb I even kept some of the old files to make sure the transition is smooth (and that I can create the same locks as v5 did). Any ideas what I am missing?

mhenrixon commented 5 years ago

Are you guys using redis namespace by any chance?

chadrschroeder commented 5 years ago

No, we aren't using redis namespace.

In version 6, when I call perform_async the client middleware asks the SidekiqUniqueJobs::Locksmith to lock the job. This runs touch_grabbed_token which sets the GRABBED key. The GRABBED key must be present in order for locked? to be true and for the job to run:

# SidekiqUniqueJobs::Lock::UntilExecuted
def execute
  return unless locked?  # Will exit without executing the job here if locked? is false
  with_cleanup { yield }
end

But version 5 didn't set this GRABBED key when perform_async was called. So jobs enqueued in version 5 won't run in version 6.

This is one way to examine the situation using legacy_lock_spec.rb:

context 'with a legacy lock' do
  before do
    result = SidekiqUniqueJobs::Scripts.call(
      :acquire_lock,
      redis_pool,
      keys: [unique_digest],
      argv: [lock_value, lock_expiration],
    )

    expect(result).to eq(1)
    expect(unique_keys).to include(unique_digest)
  end

  context 'test if locked' do
    let(:lock_value) { jid_one }

    it 'is locked' do
      puts "Redis keys: #{SidekiqUniqueJobs.redis(&:keys)}"
      puts "locked?: #{locksmith_one.locked?}"
      locksmith_one.send(:touch_grabbed_token, jid_one)  # Only happens in version 6 during perform_async
      puts "Redis keys: #{SidekiqUniqueJobs.redis(&:keys)}"
      puts "locked?: #{locksmith_one.locked?}"
    end
  end
  ...

This will result in:

Redis keys: ["uniquejobs:test_mutex_key"]
locked?: false
Redis keys: ["uniquejobs:test_mutex_key", "uniquejobs:test_mutex_key:GRABBED"]
locked?: true
mhenrixon commented 5 years ago

Sorry it took so long @chadrschroeder, not sure if you still need it for an upgrade but it is now fixed and will be released with the next version.