Subject: Re: [PATCHv2 5/5] refs.c: enable large transactions

From: Jeff King <[email-masked]>
Date: Thu, 22 Jan 2015 08:10:18 -0500

On Thu, Jan 22, 2015 at 12:24:23PM +0100, Michael Haggerty wrote:

> I can't figure out where to apply this series or where to fetch it from,
> so I can't see these changes in context, so maybe I'm misunderstanding
> something. It looks like this code is doing
> 
>     open(), close(), open(), fdopen(), write(), fclose(), rename()
> 
> on each lockfile. But don't we have enough information to write the
> SHA-1 into the lockfile the first time we touch it? I.e., couldn't we
> reduce this to
> 
>     open(), fdopen(), write(), fclose(), rename()
> 
> , where the first four calls all happen in the initial loop? If a
> problem is discovered when writing a later reference, we would roll back
> the transaction anyway.
> 
> I understand that this would require a bigger rewrite, so maybe it is
> not worth it.

I had a nagging feeling on the multiple-open thing, too, and would much
prefer to just write out the contents early (since we know what they
are). It looks like we would just need to split write_ref_sha1() into
its two halves:

  1. Write out the lockfile

  2. Commit the change

And then call them at the appropriate spots from ref_transaction_commit().

I guess that is maybe a step backwards for abstracted ref backends,
though.

-Peff