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

Add new 'removeOnComplete' option to atomically remove a job when it … #361

Merged
merged 3 commits into from
Oct 17, 2016

Conversation

bradvogel
Copy link
Contributor

…is completed (useful for high-volume queues). Fixes #354

' redis.call("DEL", KEYS[4])',
' return 0',
' else',
' return -1',
' end',
Copy link
Member

@manast manast Oct 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A slight issue here, you need to name differently the script when you specify removeOnComplete, otherwise, since scripts are cached, if you do not use removeOnComplete in the future in the same instance, it will use the old cached one. I know this is probably not going to happen, but for the sake of formality :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes, great catch. Pushed up a fix. Also pushed a small microoptimization for queues with a large number of active jobs (like ours)

…e active list, and that jobs being completed are likely near the end of the list, then we can stop iterating the list after finding the first element starting at the end.
@bradvogel
Copy link
Contributor Author

bradvogel commented Oct 16, 2016

Let me know if there's anything else before it can be merged

@manast
Copy link
Member

manast commented Oct 17, 2016

A future improvement of this feature could be a setting to specify how many jobs to keep. By doing that you could keep a history of the latest x processed jobs, which can be useful info, while not keeping all of them which in same cases is not practical.

@manast manast merged commit e2e6f64 into OptimalBits:master Oct 17, 2016
@bradvogel
Copy link
Contributor Author

Yes, good idea on that.

@bradvogel bradvogel deleted the add-removeOnComplete branch October 17, 2016 17:23
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

Successfully merging this pull request may close these issues.

2 participants