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

Support unlocking deadlocks with setnx/getset #4

Merged
merged 5 commits into from
Jan 28, 2013

Conversation

miyagawa
Copy link
Contributor

This patch adds unlocking behavior when a worker somehow gets dead while obtaining a lock without cleaning it up and causes deadlocks. The code uses a design pattern used in http://redis.io/commands/setnx using SETNX and GETSET, to avoid the race condition.

To make it work i changed the value of lock to be an integer (UNIX timestamp) rather than a time string as it used to be.

This addresses #1.

@miyagawa
Copy link
Contributor Author

The commits also bump up the resque dependency to work with the current release of resque. I've been running it on production for a couple of days now and see no problems with the current resque.

@miyagawa
Copy link
Contributor Author

FWIW, 5 days is awfully long for a deadlock situation - I override that by defining our own def lock_timeout but that might not be obvious without looking at the source code. Might be worth adding a documentation?

@wallace
Copy link
Owner

wallace commented Jan 28, 2013

Excellent, thank you!

wallace added a commit that referenced this pull request Jan 28, 2013
Support unlocking deadlocks with setnx/getset
@wallace wallace merged commit 04786eb into wallace:master Jan 28, 2013
@miyagawa
Copy link
Contributor Author

miyagawa commented Mar 8, 2013

There's some race condition reported in redis-mutex, which uses the same SETNX pattern: kenn/redis-mutex#5

I will need to see how they can get the issue fixed and if that can be applied to your gem too :)

@wallace
Copy link
Owner

wallace commented Mar 9, 2013

Hm, I may have time to tackle this later this week.

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