ionelmc / python-redis-lock

Lock context manager implemented via redis SET NX EX and BLPOP.
https://pypi.python.org/pypi/python-redis-lock
BSD 2-Clause "Simplified" License
551 stars 77 forks source link

Do not timeout without timeout #35

Closed tvuotila closed 8 years ago

tvuotila commented 8 years ago

I have a system where I use python-redis-lock to synchronize my workers. I use expire=60 together with auto_renewal=True. If process does not release the lock within 60 seconds, waiting thread throws an error. This is not what I want. I want waiting thread to wait indefinitely until lock is released or lock expires (implying that worker, which had the lock, crashed).

I could increase expire, but it would delay recovery from worker failure and would not be scalable.

I changed code to only timeout when timeout is specified.

How to reproduce the problem:

from redis_lock import Lock
lock = Lock(redis_client=redis, name='expire', expire=60, auto_renewal=True)
lock.acquire()

with other window:

from redis_lock import Lock
lock = Lock(redis_client=redis, name='expire', expire=60, auto_renewal=True)
with lock:
    print('got lock')
ionelmc commented 8 years ago

Interesting, this looks like a bug. How'd I miss this :confused:

About the test, it would need to have some sort of event in the middle to signal the parent that the lock was acquired.

ionelmc commented 8 years ago

On second thought, the test could be done in a single process I think. Just make two lock instances and don't release it from the first instance.

tvuotila commented 8 years ago

I changed the test to use only single process. I had to use expire to automatically release the first lock.

ionelmc commented 8 years ago

Alright, awesome.

PS. The failure is something unrelated, that I need to fix.