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

Clarification on Sidekiq patch #33

Open
arizz96 opened this issue Nov 12, 2021 · 1 comment
Open

Clarification on Sidekiq patch #33

arizz96 opened this issue Nov 12, 2021 · 1 comment
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@arizz96
Copy link

arizz96 commented Nov 12, 2021

Hi there, thank you for this useful gem!

I'm using it both with Resque and Sidekiq, and as the README says, in order to properly make Sidekiq queue cleanup and job death working with this gem, the patch needs to be required.

Can you please better explain how this patch works and why is it necessary?
More particularly, why is it always calling ActiveJob::Uniqueness.unlock!, that delete locks using the wildcard?

def unlock!(**args)
lock_manager.delete_locks(ActiveJob::Uniqueness::LockKey.new(**args).wildcard_key)
end

I can understand that wildcard is useful when cleaning a queue, but why is it used also when a job (single job) death?

# Global death handlers are introduced in Sidekiq 5.1
# https://github.com/mperham/sidekiq/blob/e7acb124fbeb0bece0a7c3d657c39a9cc18d72c6/Changes.md#510
if sidekiq_version >= Gem::Version.new('5.1')
Sidekiq.death_handlers << ->(job, _ex) { ActiveJob::Uniqueness.unlock_sidekiq_job!(job) }
end

Thank you in advance!

@sharshenov
Copy link
Member

Hello @arizz96

thank you for this useful gem!

Thank you!

Can you please better explain how this patch works and why is it necessary?

The activejob-uniqueness is designed to be agnostic to ActiveJob adapters. Every adapter has its own queues implementation and jobs management. Deleting jobs does not automatically cleanup their locks.

But developers do expect this behavior, especially if they have used sidekiq + sidekiq-unique-jobs. The activejob-uniqueness has a Sidekiq patch to provide this experience. The patch patches internal Sidekiq API of jobs management in a way similar to sidekiq-unique-jobs approach. The patch is optional in order to give developers more control over locks cleanup.

why is it always calling ActiveJob::Uniqueness.unlock!, that delete locks using the wildcard?
I can understand that wildcard is useful when cleaning a queue, but why is it used also when a job (single job) death?

You are right, there is a probability that the queue might have a job with no arguments alongside with jobs with arguments. Deleting a single job with no arguments will unlock all jobs of this type.

Thank you for the report. This bug needs to be fixed.

@sharshenov sharshenov added good first issue Good for newcomers bug Something isn't working labels Jan 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants