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

Bugfix: use lock.extend replace pexpire #128

Merged
merged 5 commits into from Jul 29, 2019

Conversation

laixintao
Copy link

@laixintao laixintao commented May 29, 2019

monkey patch lua LUA_EXTEND_TO_SCRIPT

close #127
close #84
close #83

monkey patch lua LUA_EXTEND_TO_SCRIPT

close sibson#127
@laixintao
Copy link
Author

laixintao commented May 29, 2019

Currently the process will die if it found out that it no longer own the lock:

[2019-05-29 23:38:09,366: DEBUG/MainProcess] beat: Releasing Lock
[2019-05-29 23:38:09,366: CRITICAL/MainProcess] beat raised exception <class 'redis.exceptions.LockNotOwnedError'>: LockNotOwnedError("Cannot release a lock that's no longer owned")
Traceback (most recent call last):
  File "/Users/laixintao/.virtualenvs/redbeat/lib/python3.7/site-packages/celery/beat.py", line 597, in start
    interval = self.scheduler.tick()
  File "/Users/laixintao/Program/redbeat/redbeat/schedulers.py", line 469, in tick
    self.lock.extend(int(self.lock_timeout))
  File "/Users/laixintao/.virtualenvs/redbeat/lib/python3.7/site-packages/redis/lock.py", line 246, in extend
    return self.do_extend(additional_time)
  File "/Users/laixintao/.virtualenvs/redbeat/lib/python3.7/site-packages/redis/lock.py", line 253, in do_extend
    raise LockNotOwnedError("Cannot extend a lock that's"
redis.exceptions.LockNotOwnedError: Cannot extend a lock that's no longer owned

I think it would be better to make this process get blocked and trying to reacquiring the lock?

@laixintao laixintao changed the title [WIP] Bugfix: use lock.extend replace pexpire Bugfix: use lock.extend replace pexpire Jun 3, 2019
@laixintao
Copy link
Author

Hi @sibson some tests fails, but it doesn't seem to relevant to this PR. PTAL

@laixintao
Copy link
Author

I update the lock extend logic and update readme. What do you think?

redbeat/schedulers.py Outdated Show resolved Hide resolved
@sibson
Copy link
Owner

sibson commented Jun 12, 2019

This generally looks good to me. I'm taken the approach of exiting on terminal error conditions rather than deal with the complexity of trying to recover.

@laixintao
Copy link
Author

Conflict resolved. PTAL @sibson

@sibson
Copy link
Owner

sibson commented Jun 13, 2019

LGTM, thanks. I'm going to see if we can resolve the failing test on master before merging this. If that doesn't happen soon, I'll merge this regardless.

@laixintao
Copy link
Author

ping @sibson

@sibson sibson merged commit d7f38c7 into sibson:master Jul 29, 2019
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.

Distributed lock cloud be acquired more then once. Lock disappearing?
2 participants