Re: [PATCH 2/2] setup: don't fail if commondir reference is deleted.

["Michal Suchánek" <msuchanek@suse.de>]

On Thu, 21 Feb 2019 12:12:28 -0500
Eric Sunshine <sunshine@sunshineco.com> wrote:

> On Thu, Feb 21, 2019 at 12:07 PM Phillip Wood <phillip.wood@talktalk.net> wrote:
> > On 21/02/2019 13:50, Michal Suchánek wrote:  
> > >> On Tue, Feb 19, 2019 at 12:05 AM Michal Suchanek <msuchanek@suse.de> wrote:  
> > > The problem is we don't forbid worktree names ending with ".lock".
> > > Which means that if we start to forbid them now existing worktrees
> > > might become inaccessible.  
> >
> > I think it is also racy as the renaming breaks the use of mkdir erroring
> > out if the directory already exists. One solution is to have a lock
> > entry in $GIT_COMMON_DIR/worktree-locks and make sure the code that
> > iterates over the entries in $GIT_COMMON_DIR/worktrees skips any that
> > have a corresponding ignores in $GIT_COMMON_DIR/worktree-locks. If the
> > worktree-locks/<dir> is created before worktree/<dir> then it should be
> > race free (you will have to remove the lock if the real entry cannot be
> > created and then increment the counter and try again). Entries could
> > also be locked on removal to prevent a race there.  
> 
> I wonder, though, how much this helps or hinders the use-case which
> prompted this patch series in the first place; to wit, creating
> hundreds or thousands of worktrees. Doing so serially was too slow, so
> the many "git worktree add" invocations were instead run in parallel
> (which led to "discovery" of race conditions). Using a global worktree
> lock would serialize worktree creation, thus slowing it down once
> again.

I created thousands of worktrees only for stress-testing. The real
workload needs only a dozen of them. That still leads to hitting a
race condition occasionally and automation failure.

Creating a separate lock directory will probably work. The question is
when do you need to take the lock. Before adding a worktree, sure.
Before deleting it as well. The problem is that deleting a worktree
successfully without creating some broken state needs to exclude
processes that might add stuff in the worktree directory. How many
operations then do *not* need to take the lock?

Thanks

Michal