Skip to content
This repository has been archived by the owner on Sep 17, 2024. It is now read-only.

Deletion of job from queue does not clear the lock key #10

Closed
driv3r opened this issue May 8, 2013 · 8 comments
Closed

Deletion of job from queue does not clear the lock key #10

driv3r opened this issue May 8, 2013 · 8 comments

Comments

@driv3r
Copy link

driv3r commented May 8, 2013

When you try to remove the unique job from within Web UI, the lock key is not cleared from redis. Also the same thing can occur when deleting the job or clearing the whole queue from ruby. Probably one of the solutions would be to extend the sidekiq api to also clear the lock first, or at least gather the locks for unlock. If I will have a spare second I can take a deeper look into it.

@krasnoukhov
Copy link
Owner

Hello, yes, you are right and it should be fixed. Unfortunately i can't take a look into this now, so any help would be appreciated. Thanks!

@bnorton
Copy link
Collaborator

bnorton commented Jun 7, 2013

The main issue that I see is that the two things are different. In one case you have this key that represents the uniqueness of a job and on the other you have the record of the job. When everything in working normally you go through the right channels and it works as expected but through the command line you can only do so much to keep the lock status updated.

The web ui part should be fixed but otherwise i feel like we are all out of luck with that.

@driv3r
Copy link
Author

driv3r commented Jun 7, 2013

Yes, normally the stuff goes through the middleware, but there's a API so you can fetch a whole job, browse through them and delete. The thing is that with the current implementation, the only thing that can be a bit slow, is deletion of all the jobs. It's because normally it will just clear the redis without checking each job, and we will need to change that into deletion of the jobs one after another.

I already was looking into the tests and implementation but haven't got enough time till now. Probably on this weekend I will try to close it.

Besides that I was wondering why the unique key generation is implemented as separate standalone module instead of something included within the worker class?

@dychen
Copy link

dychen commented Apr 25, 2014

I wrote up a quick fix for this, but it needs the webserver to require all the relevant Job files one way or the other, and there's no way (that I know of) to ensure that via a standalone gem like this. Any thoughts?

@krasnoukhov
Copy link
Owner

Hey @dychen, why do you think this is not possible? If sidekiq-middleware is required after sidekiq it can override some of its code. So, basically, sidekiq-middleware should be required in a sidekiq web interface environment in order to hijack job deletion. Other way (and preferred for me) would be implementing this feature in a separate file (which is not required by default) and ask users to require it manually in web interface envrironment.

What do you think?

@dychen
Copy link

dychen commented Apr 26, 2014

Yeah, that's actually the way I'm doing it (the first way you mentioned), and it works. The caveat is that now you have to make sure that the jobs are required somewhere on the webserver load path (maybe config.ru), whereas before, the webserver didn't need to know about the jobs at all. This is because before, it was just deleting the jobs from Redis - now, it's actually calling unlock! on the jobs because it needs to find the job lock and that logic is handled by the worker. This is a little concerning (at least, in my opinion) because there's no way to make sure the gem user does this. I can submit a PR if you wanna take a look.

@krasnoukhov
Copy link
Owner

@dychen, I think that your solution would work great. Since we prefer implementing it as a separate extension that should be manually required, this just needs to be well documented.

@masterkain
Copy link

using sidekiq-pro: if I stop the sidekiq process while an unique task is running, when I restart sidekiq the job is performed twice. Is this a similar issue?

@driv3r driv3r closed this as completed Jul 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants