From: Phillip Wood <phillip.wood@talktalk.net>
Date: Thu, 21 Feb 2019 17:27:04 GMT
Subject: Re: [PATCH 2/2] setup: don't fail if commondir reference is deleted.
Message-ID: <4309841e-2b98-22d4-505e-1b9ea2f5e3bb@talktalk.net>
In-Reply-To: <CAPig+cTQMZFF-oX-SOzB5JR=V1WThBihC+kNm-2wjbpAWf-OHA@mail.gmail.com>

Hi Eric

On 21/02/2019 17:12, Eric Sunshine 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.

The idea is that there are per-worktree lock stored under worktree-locks 
(hence the plural name). Using a separate directory for the locks gets 
round the problems of name clashes between the lock for a worktree 
called foo and one called foo.lock and means we can rely on mkdir 
erroring out if the worktree name already exists as there is no renaming.

Best Wishes

Phillip