Re: [PATCH 5/5] reflog expire: don't assert the OID when locking refs
- Date: Wed, 13 Mar 2019 20:44:10 -0400
- From: Jeff King <peff@xxxxxxxx>
- Subject: Re: [PATCH 5/5] reflog expire: don't assert the OID when locking refs
On Thu, Mar 14, 2019 at 12:54:39AM +0100, Ævar Arnfjörð Bjarmason wrote:
> The locking protocol for reflogs involves getting a *.lock file on the
> loose ref (even if the actual ref is packed). This isn't needed to
> expire the reflog, and needlessly results promotes reference update
> contention to hard errors in e.g. "gc".
This first paragraph threw me off. It sounds like you are saying we
don't need to take a lock, but we absolutely do. It's just that we don't
need to care about the lock having some particular value. Which you do
go on to explain, but I think it would be more clear by simply removing
this first paragraph.
> If we instead lock the reference without considering what OID we last
> saw it at, we won't encounter user-visible contention to the extent
> that core.filesRefLockTimeout mitigates it. See 4ff0f01cb7 ("refs:
> retry acquiring reference locks for 100ms", 2017-08-21).
I think this part is true. I'd love to get a confirmation from Michael
Haggerty, who has spent way more time thinking about ref and reflog
locking than any mortal should have to. Hopefully he even still
remembers some of it. :)