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

until_executed and lock_expiration. #676

Open
richkettle opened this issue Jan 12, 2022 · 5 comments
Open

until_executed and lock_expiration. #676

richkettle opened this issue Jan 12, 2022 · 5 comments

Comments

@richkettle
Copy link

richkettle commented Jan 12, 2022

Describe the bug
With the sidkiq options of:

sidekiq_options queue: :medium, retry: 3, lock: :until_executed, lock_expiration: 120 * 60

When this worker has been run:

[1] pry(main)> HardWorker.perform_async
=> 53b93f122fddbb2ebd350332267484ea

The job completes as the perform is an empty method. The lock happens i.e. I see this in redis:
Screenshot 2022-01-12 at 12 36 26

If the AVAILABLE key still exists in redis i.e. before the 5 seconds TTL expires. I can enqueue another job and continue to do this as long as I refresh that TTL and that key exists. However, after the 5 seconds and that key expires I cannot enqueue another job until the EXISTS key expires.

Expected behavior
I expect to be able to enqueue another job after 5 seconds has passed. I'm pretty sure that the EXISTS key should be removed after the job completes correctly.

Current behavior
The job is able to be enqueued but only within the TTL of the AVAILABLE key.

Worker class

class Hardworker
  include Sidekiq::Worker
  sidekiq_options queue: :medium, retry: 3, lock: :until_executed, lock_expiration: 120 * 60 # Make the uniq 2 hours

  def perform; end
end

Additional context
gem 'sidekiq', '4.2.10'
gem 'sidekiq-cron', '1.2.0'
gem 'sidekiq-unique-jobs', '6.0.25'

I am in the process of upgrading sidekiq from a really old version. This is as far as I can go without upgrading other major parts of the system.

@mhenrixon
Copy link
Owner

Thank you for the detailed report. Considering what you write last, I suppose there is no way to persuade you to upgrade to v7 just yet?

@richkettle
Copy link
Author

Thank you for the detailed report. Considering what you write last, I suppose there is no way to persuade you to upgrade to v7 just yet?

Thanks for the reply!

We are planning it. But it is far off in the future. I am happy to contribute and fix this issue if you can point me in the right direction.

@richkettle
Copy link
Author

@mhenrixon How do you want me to proceed with this? Would a failing spec be something you can work from?

@richkettle
Copy link
Author

So I had a go at trying to replicate this with a spec but didn't do so well. But I downgraded to v6.0.0 and it is working as intended. Further investigation shows that the behaviour changed between 6.0.7 and 6.0.8. It seems to be this change that causes the issue: 12924de#diff-c1471768d58ee2883363280a6054208c13024f2aea47d1e4227729ab0e26f061R32 For now I have left our version at 6.0.7.

As there are much higher versions that people should be using. I'm not sure whether you want to fix this or not. Whether it gets fixed or not I hope this helps someone.

@richkettle
Copy link
Author

I've read #359 and it seems to suggest there are issues with redis filling up with keys which is why there is the 5 second expiry. So a permanent fix would be good.

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