[PATCH 2 of 7] clone: put streaming clones in a transaction

[Matt Mackall]

On Mon, 2014-03-31 at 17:22 +0000, Durham Goode wrote:
> On 3/26/14 12:24 PM, "Matt Mackall" <mpm at selenic.com> wrote:
> 
> >On Mon, 2014-03-24 at 19:33 -0700, Durham Goode wrote:
> >> # HG changeset patch
> >> # User Durham Goode <durham at fb.com>
> >> # Date 1395700700 25200
> >> #      Mon Mar 24 15:38:20 2014 -0700
> >> # Node ID 08595987c5b0e8af5aa8fec4debd7260f5a79e8f
> >> # Parent  ab3be74e8022c31b7c95975fb09b3602ed7775d8
> >> clone: put streaming clones in a transaction
> >> 
> >> Streaming clones were writing to files outside of a transaction. Soon
> >>the
> >> fncache will be written at transaction close time, so we need streaming
> >>clones
> >> to be in a transaction.
> >
> >Hmm. But you're not adding the streamed files to the transaction?
> 
> The clone command deletes the entire repo if something goes wrong, so
> transacting these writes wasn't necessary.

Sure.

> >> +            # Writing straight to files circumvented the inmemory
> >>caches
> >> +            self.invalidate()
> >
> >Isn't there a lock held here that will invalidate() on release?
> 
> No, things get invalidated at lock acquisition time, but not at lock
> release (since they shouldn't be invalid since no one else should have
> touched the disk while we had the lock).

Actually, both are places where we should be invalidating. But we
actually invalidate on unlock:

localrepo:def lock:

        l = self._lock(self.svfs, "lock", wait, unlock,
                      self.invalidate, _('repository %s') % self.origroot)

lock.py:def init:

    def __init__(self, vfs, file, timeout=-1, releasefn=None, desc=None):

lock.py:def release:

            if self.releasefn:
                self.releasefn()

That's because:

a) be reading before locking is wasted effort we should avoid
b) if we're holding a lock, we may have writers queued up to immediately
change what's on disk when we unlock

-- 
Mathematics is the supreme nostalgia of our time.