search

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
550 stars 78 forks source link

Two celery workers got the lock at the same time #93

Open fz-gaojian opened 2 years ago

fz-gaojian commented 2 years ago

I have deployed Celery in both machines, and is the same code,and Broker is the same redis,but I just wants celery beat to execute once, so I used python-redis-lock.

But two celery workers got the lock at the same time.

I configure it every 10 minutes to execute it,task code is roughly like this:

from redis import Redis
import redis_lock
conn = Redis()
lock = redis_lock.Lock(conn, "test-lock", expire=60 * 10 - 1)
if lock.acquire(blocking=False):
    print("Doing some work ...")

Then I look at most of the logs is the expected result,but I noticed that there is a result that does not meet the expected expectations. One of the machines logs:

[2022-04-08 18:10:00,463: INFO/MainProcess] Received task: test_task[xxxxxxxxxxxxxxxxx1]
[2022-04-08 18:10:00,467: INFO/ForkPoolWorker-3] Got lock for 'lock:test-lock'.
[2022-04-08 18:10:00,493: INFO/ForkPoolWorker-3] Task test_task[xxxxxxxxxxxxxxxxx1] succeeded in 0.027071014046669006s: xxxxxxxxxxx

Another:

[2022-04-08 18:10:00,323: INFO/MainProcess] Received task: test_task[xxxxxxxxxxxxxxxxx2]
[2022-04-08 18:10:00,326: INFO/ForkPoolWorker-3] Got lock for 'lock:test-lock'.
[2022-04-08 18:10:00,546: INFO/ForkPoolWorker-3] Task test_task[xxxxxxxxxxxxxxxxx2] succeeded in 0.22096195071935654s: xxxxxxxxxxx

If you can tell me the reason, I will be grateful.

ionelmc commented 2 years ago

Not to sound coy but are you sure you're using the same redis server, and no redis cluster or something crazy?

Also I'm curious why celery beat sends the task to two workers - are you using some fanout or at-least-once-delivery type of broker?

ionelmc commented 2 years ago

Another thing - are you sure redis doesn't evict keys? You're not using it for caching right?

Also can you edit your logs to include messages for log releasing? You could also set the redis_lock loggers to DEBUG.

fz-gaojian commented 2 years ago

Not to sound coy but are you sure you're using the same redis server, and no redis cluster or something crazy?

Also I'm curious why celery beat sends the task to two workers - are you using some fanout or at-least-once-delivery type of broker?

Thank you for your reply. I'm sure using the same redis server.

The answer to the second question is: I started two celery, so celery beat will send two tasks to broker at the same time, but I expect workers to run only one task, not twice.

ionelmc commented 2 years ago

Well please post more logs. Pretty sure the locks don't overlap. Eitherway using redis-lock won't solve your problem ... eg: if your machines have clocks slightly out of sync and that task is fast enough you'll still get the task to run two times.

The correct solution would be to not run celery beat two times...

fz-gaojian commented 2 years ago

eg: if your machines have clocks slightly out of sync and that task is fast enough you'll still get the task to run two times.

Thank you for your reply, I had a hard time reproducing and I fought to get more logs to provide. Actually my task code lock expired nearly 10 minutes, and my machine time couldn't have been different that much because I was watching the log generation at the time. I'm also pretty sure the locks won't overlap, so there shouldn't be workers getting this lock within 10 minutes.

nikhilnimbalkar1 commented 7 months ago

You use blocking=False when your acquire the lock, that should be the reason both your workers get the lock.