sidekiq-unique-jobs icon indicating copy to clipboard operation
sidekiq-unique-jobs copied to clipboard

Don't lock on failure when retry: false (until_and_while_executing)

Open ajw725 opened this issue 3 years ago • 1 comments

Is your feature request related to a problem? Please describe. I'm using the until_and_while_executing locking strategy combined with retry: false. When an exception is raised in the worker's perform method, the UntilAndWhileExecuting lock class rescues the exception and replaces the lock before re-raising the exception. The result is that my job has failed and will not retry, but the lock remains in place and I cannot run another job with the same arguments until I manually remove the lock from Redis.

Describe the solution you'd like I would like the UntilAndWhileExecuting lock not to restore the lock on exceptions when the worker is configured with retry: false. This looks like it could be done with a quick check of item['retry'] == false or something along those lines in #lock_on_failure.

Describe alternatives you've considered I'm not sure. Using retry: 0 doesn't change anything, but I can understand why some people might want the lock to be preserved when the job gets moved to the dead set. If I'm using the lock incorrectly or if there's a reason for this behavior I'm missing, please let me know!

Additional context The use case here is a cron job that runs every few minutes. I don't need the worker to retry because it will run again soon, I want to use a lock in case it somehow gets delayed or enqueued more than once, and I don't want the lock preserved on failure because it blocks future scheduled jobs from running.

If you think this is a reasonable request, I'm happy to take a shot at implementing it.

ajw725 avatar Mar 29 '21 22:03 ajw725

~i guess i have another related thought/comment, which is about the lock ttl. the readme says that the default value is nil, which means "don't expire keys." this is true in the context of redis - no ttl means the key will persist indefinitely - but it's not true in the context of this gem. a lock will only be deleted upon completion of a job if the job does not have an explicit ttl, and otherwise the key will be left alone to expire naturally in redis.~

~if the lock is going to be replaced on failure, i guess i would like it to have some limited ttl...but then if the job succeeds, i want the lock to be removed immediately. i realize this is sort of a different question from the original feature request, but do you think that would be feasible? kind of a ttl with a clear-on-success option? i'm not sure if that really makes sense...just brainstorming a little about what behavior i'd like to see in this situation so we don't end up with long-term orphaned locks if something goes wrong.~

sorry, ignore that - forgot we had upgraded to v7 in one project but not the other. seems that piece has been addressed.

ajw725 avatar Mar 29 '21 23:03 ajw725