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

[Matt Mackall]

On Mon, 2014-03-31 at 21:51 +0000, Durham Goode wrote:
> On 3/31/14 11:27 AM, "Matt Mackall" <mpm at selenic.com> wrote:
> 
> >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
> >> >>
> >> >> +            # 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)
> 
> I think you misread (or maybe I am). self.invalidate is being passed in as
> the 'acquirefn' to localrepo._lock
> 
>     def _lock(self, vfs, lockname, wait, releasefn, acquirefn, desc):
> 
> And the acquirefn is not passed to the lock object.  'unlock', which is
> the releasefn, doesn't call invalidate:
> 
>          def unlock():
>             self.store.write()
>             if hasunfilteredcache(self, '_phasecache'):
>                 self._phasecache.write()
>             for k, ce in self._filecache.items():
>                 if k == 'dirstate' or k not in self.__dict__:
>                     continue
>                 ce.refresh()
> 
> 'unlock' is actually jumping through a few hoops to *avoid* invalidating
> caches (by calling refresh on each filecache).

Yeah, my mistake here. Our terminology is all a little broken here: the
act of marking data in a cache potentially-stale is also invalidating.
And we need some form of invalidation at both acquisition and release
time.

-- 
Mathematics is the supreme nostalgia of our time.


-- 
Mathematics is the supreme nostalgia of our time.