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

Change LPUSH/ZREM command execution order in requeue_job_now script #99

Merged
merged 2 commits into from
Dec 12, 2016
Merged

Change LPUSH/ZREM command execution order in requeue_job_now script #99

merged 2 commits into from
Dec 12, 2016

Conversation

keyan
Copy link
Contributor

@keyan keyan commented Dec 12, 2016

This PR reorders the data manipulation commands in the requeue_job_now.lua script in order to ensure an "at least once" execution guarantee.

See #98 for a longer discussion.

Now that LPUSH is run before ZREM, this test is invalid in its current
state. Its purpose was to ensure that trying to requeue a job that isn't
in the "source" queue already would fail. But the changes in this PR
sacrifice this guarantee to obtain a "run at least once" guarantee.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 84.103% when pulling 273d78e on keyan:kp_lpush_before_zrem into 83ddc19 on edgurgel:master.

@edgurgel
Copy link
Owner

Thank you! 👍

@edgurgel edgurgel merged commit ac13da2 into edgurgel:master Dec 12, 2016
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.

3 participants