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

From: Jeff King <peff@peff.net>
Date: Jan 22, 2015, 13:10 UTC
Message-ID: <20150122131018.GE19681@peff.net>
In-Reply-To: <54C0DDE7.8030708@alum.mit.edu>

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

Show 18 quoted lines

> 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