Re: Credential Store: Don't acquire lock when only reading the file

[Jeff King <peff@peff.net>]

On Fri, Oct 30, 2020 at 12:05:40PM +0000, Simão Afonso wrote:

> On 2020-10-30 01:55:41, Jeff King wrote:
> >       - teach it to try the lock until hitting a timeout. I think just
> > 	swapping out hold_lock_file_for_update() for
> > 	hold_lock_file_for_update_timeout() would do it (I probably
> > 	would have used it back then, but it didn't exist yet).
> 
> So I tried to patch the credential store with that timeout function and
> it seems to solve it! Thanks.
> 
> https://github.com/git/git/blob/v2.29.2/builtin/credential-store.c#L61
> 
> This is easily reproduced if you do a `fetch --all` in parallel on the
> same repository (seen on the attached image):

Great!

> > $ for n in $(seq 100); do git fetch --all; sleep 0.5; done
> 
> About the timeout, I put it at infinite retries, but should this be
> configurable? Or should it be a different default?
> 
> There aren't many timeout configurations. I found
> "core.filesRefLockTimeout" and "core.packedRefsTimeout", doesn't sound
> like something that should be re-used.
> Should it be something like this with a different name?

Yeah, I don't think it makes sense to reuse those. I kind of doubt that
it is worth making it configurable, as credential-store does not
otherwise read any config at all.

The defaults for those other two values are 100ms and 1s respectively.
So probably just setting a 1s timeout would be plenty, and saves us from
looping forever if we see a stale lockfile.

-Peff