From: Derrick Stolee <stolee@gmail.com>
Date: Fri, 23 Feb 2018 19:41:01 GMT
Subject: Re: [PATCH v4 08/13] commit-graph: implement --delete-expired
Message-ID: <8c958ada-b96b-cdc7-594c-c6f6cdb02b70@gmail.com>
In-Reply-To: <xmqqo9kftsa5.fsf@gitster-ct.c.googlers.com>

On 2/23/2018 2:33 PM, Junio C Hamano wrote:
> Derrick Stolee <stolee@gmail.com> writes:
>
>> The (unlikely, but possible) race condition involves two processes (P1
>> and P2):
>>
>> 1. P1 reads from graph-latest to see commit graph file F1.
>> 2. P2 updates graph-latest to point to F2 and deletes F1.
>> 3. P1 tries to read F1 and fails.
>>
>> I could explicitly mention this condition in the message, or we can
>> just let P2 fail by deleting all files other than the one referenced
>> by 'graph-latest'. Thoughts?
> The established way we do this is not to have -latest pointer, I
> would think, and instead, make -latest be the actual thing.  That is
> how $GIT_DIR/index is updated, for example, by first writing into a
> temporary file and then moving it to the final destination.  Is
> there a reason why the same pattern cannot be used?

My thought was that using a fixed name (e.g. 
.git/objects/info/commit-graph) would block making the graph 
incremental. Upon thinking again, this is not the case. That feature 
could be designed with a fixed name for the small, frequently-updated 
file and use .../info/graph-<hash>.graph names for the "base" graph files.

I'll spend some time thinking about the ramifications of this fixed-name 
concept. At the moment, it would remove two commits from this patch 
series, which is nice.

Thanks,
-Stolee