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

From: Stefan Beller <sbeller@google.com>
Date: Sep 25, 2017, 18:51 UTC
Message-ID: <CAGZ79kYOeJvQmw-h3GwFpv2w7AKtNWUxf96tUjPKPY1dMuzagA@mail.gmail.com>
In-Reply-To: <20170923033425.6ilykxl5pi7unlj6@sigill.intra.peff.net>

Show 27 quoted lines

>> There might be another option to cope with the situation:
>>
>>  4. Teach all commands to spinlock / busywait shortly for important
>>      locks instead of giving up. In that case git-status rewriting
>>      the index ought to be fine?
>
> We do have all the infrastructure in place to do a reasonable busywait
> with backoff. I think the patch is roughly just:
>
> 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. :)

>
> 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.

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.

Stefan