Re: [PATCH] git: add --no-optional-locks option

From: Jeff King <peff@peff.net>
Date: Sep 27, 2017, 06:44 UTC
Message-ID: <20170927064423.twluii6jj57hyk5y@sigill.intra.peff.net>
In-Reply-To: <CAGZ79kYOeJvQmw-h3GwFpv2w7AKtNWUxf96tUjPKPY1dMuzagA@mail.gmail.com>

On Mon, Sep 25, 2017 at 11:51:31AM -0700, Stefan Beller wrote:

Show 20 quoted lines

> > diff --git a/read-cache.c b/read-cache.c
> > index 65f4fe8375..fc1ba122a3 100644
> > --- a/read-cache.c
> > +++ b/read-cache.c
> > @@ -1563,7 +1563,8 @@ static int read_index_extension(struct index_state *istate,
> >
> >  int hold_locked_index(struct lock_file *lk, int lock_flags)
> >  {
> > -       return hold_lock_file_for_update(lk, get_index_file(), lock_flags);
> > +       return hold_lock_file_for_update_timeout(lk, get_index_file(),
> > +                                                lock_flags, 500);
> >  }
> >
> >  int read_index(struct index_state *istate)
> >
> > though I think there are a few sites which manually call
> > hold_lock_file_for_update() on the index that would need similar
> > treatment.
> 
> uh, too bad. The patch above looks really promising, though. :)

There are probably only a handful of other callers, and they'd just need to swap out s/update/&_timeout/. So it really is pretty trivial.

Show 6 quoted lines

> > I suspect it would work OK in practice, unless your index is so big that
> > 500ms isn't enough. The user may also see minor stalls when there's lock
> > contention. I'm not sure how annoying that would be.
> 
> There is only one way to find out. Though we don't want to volunteer
> all users into this experiment, I'd presume.

Yes. One of the nice things about the optional-locks approach is that it only affects callers who specify the option. And the general idea has gotten a year of testing in Visual Studio, which makes me feel good about it.

> Regarding larger indexes, I wonder if we can adapt the 500ms
> to the repo size. At first I thought the abbreviation length could be
> a good proxy to determine the maximum waiting time, but now I am
> not so sure any more.

I think madness that way lies.

-Peff