Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jobs are unlocked if they fail and are retried #77

Closed
DarthMax opened this issue Mar 31, 2015 · 3 comments
Closed

Jobs are unlocked if they fail and are retried #77

DarthMax opened this issue Mar 31, 2015 · 3 comments

Comments

@DarthMax
Copy link

I just discovered that jobs are unlocked if they fail. This happens regardless if there are retries left or if the job dies. I'm wondering if this is the intended behavior. As I would understand (and need) that fact a job that will be retired should still be unique.

The code responsible for this is the following: (in /lib/sidekiq_unique_jobs/middleware/server/unique_jobs.rb)

def call(worker, item, _queue, redis_pool = nil)
  ...
  yield
  ensure
    if after_yield? || !defined? unlocked || unlocked != 1
      unlock(lock_key)
    end
end

So are there any reasons for this behavior or do I miss anything?

Update

So after some research I start to understand why unlocking works the way it does. I was not aware of the fact that jobs in the schedule/retry queue are pushed to the worker queues via client push (involving sidekiqs client middlewares). If in that situation the job is still locked the job will never be pushed into its worker queue.

To circumvent this issue it might be possible to save the jid of the job that acquired the lock instead of the [1,2]. So the client middleware can check for the jid and let the job reenqueue if they match.

What do you think of this idea?

@mhenrixon
Copy link
Owner

That sounds like a great idea for fallback when the job had been originally failed or scheduled.

@mhenrixon
Copy link
Owner

So a few updates concerning this went live with v3.0.13. Would you mind trying that and see if the problem still persists? If you are still having a problem just give me a failing test and I will fix it.

@mhenrixon
Copy link
Owner

This was closed by #96 and a few recent commits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants