Semantics of lifetime could be better-documented
Hi Barry! I'm considering replacing Launchpad's contrib.glock.GlobalLock with flufl.lock.Lock, since it's much better-maintained (although we'll be stuck on an oldish py2-compatible version for a while yet - working on it!) and the locking implementation is conceptually clearer. However, I've run into some confusion with how refreshing locks works, and I'm not sure whether it's just unclear docs or whether it's a bug.
Launchpad uses locks in various situations where the lifetime is initially not very determinate: it's hard to predict how long a given script will take. Ideally I'd prefer to preserve our existing interface and set locks to never be broken automatically; I suppose I can sort of do that by subclassing Lock and overriding _break, but at the moment it doesn't seem possible to avoid confusing log messages in that case. Failing that, though, we can plausibly make an initial guess and refresh the lock every time we find a chunk of work to do.
As I understand it (and please let me know if I've misunderstood anything), a lock's expiry time is recorded in the mtime of the claim file, which is set exclusively by Lock._touch. The documentation indicates that one is supposed to call refresh with a timedelta "if you realize that you need to keep a resource locked longer than you thought". However, it's nowhere defined what this timedelta is relative to. My two guesses were that it might be from the initial acquisition of the lock, or it might be from the current time. It doesn't seem to really be either of those. _touch sets the expiry time as datetime.now() + self._lifetime, but _touch is called from a number of different places (lock, is_locked, transfer_to, and _break), so I guess each one of those will cause the lock's expiry time to be extended to the current time plus the lock's lifetime. I think this needs to be documented somewhere - I had to read the code to figure it out.
For the sort of scenario I outlined above, it looks like a reasonable approach would be to initially acquire the lock with the lifetime set to a time interval within which we expect to be able to either release or refresh the lock, and then refresh it on each chunk of work (job, batch of database rows, that sort of thing). Does that sound right? It took me a while to get to this from a combination of the documentation and the code, and I thought that might be useful feedback; perhaps it's possible to make it clearer what the lifetime means. Thanks.