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

Retry now script needs to LPUSH before ZREM #98

Closed
edgurgel opened this issue Dec 12, 2016 · 2 comments
Closed

Retry now script needs to LPUSH before ZREM #98

edgurgel opened this issue Dec 12, 2016 · 2 comments
Labels

Comments

@edgurgel
Copy link
Owner

https://github.com/edgurgel/verk/blob/master/priv/requeue_job_now.lua

  local removed = redis.call("ZREM", KEYS[1], job)
    if removed == 1 then
      redis.call("LPUSH", queue_key, job)
    else

If the ZREM succeeds but something happens afterwards that LPUSH is not called we may never enqueue again this job. We would rather have 2 jobs than 0 jobs to be processed.

cc/ @keyan

I'm not sure if we want to try to LREM if the ZREM failed? What do you think?

@edgurgel edgurgel added the bug label Dec 12, 2016
@edgurgel
Copy link
Owner Author

edgurgel commented Dec 12, 2016

It is supposed to be a rare case that Redis fails to LPUSH after a successful ZREM but if we can avoid this case by ordering the commands we might as well do it.

@keyan
Copy link
Contributor

keyan commented Dec 12, 2016

Hey Ed, thanks for the write up.

We would rather have 2 jobs than 0 jobs to be processed.

I used this ordering because I was afraid a job might be processed right before we asked to requeue it. But yeah your concern sounds reasonable.

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

No branches or pull requests

2 participants