search

open-lms-open-source / moodle-local_redislock

6 stars 17 forks source link

get_lock() should set timeout/expiry #3

Closed mwithheld closed 5 years ago

mwithheld commented 5 years ago

Untested code

mwithheld commented 5 years ago

The CI failures are due to "Plugin "local_redislock" (2017120800) could not be installed. It requires a newer version of Moodle".

mwithheld commented 5 years ago

Hi @polothy - would you be able/willing to look at the idea suggested here?

mwithheld commented 5 years ago

@polothy I'm still hoping to get this lock expiry mechanism in.

polothy commented 5 years ago

Thanks for the contribution, but it is unlikely that we would merge this. We did have TTL at the start, but found that long running processes would have their locks expire, then we get duplicate runs going which is much worse than a stale lock for us. For example, I don't think any scheduled task calls extend_lock to extend the life of their lock. (From memory) I believe even other core locks do not use expire, but I might not be remembering that correctly or it was added after I looked.

You are more than welcome to fork or to maintain the patch yourself. Just be careful, I believe pexpire is for milliseconds whereas expire is for seconds.