Patchwork [5,of,7,V3] fncache: move fncache writing to be in a transaction

login
register
mail settings
Submitter Durham Goode
Date April 1, 2014, 10:24 p.m.
Message ID <24b8c6cd26c12a817cc3.1396391055@dev2000.prn2.facebook.com>
Download mbox | patch
Permalink /patch/4189/
State Accepted
Commit cd443c7589cca16a7fbcb3a7526ac10bd773dbda
Headers show

Comments

Durham Goode - April 1, 2014, 10:24 p.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1395700933 25200
#      Mon Mar 24 15:42:13 2014 -0700
# Node ID 24b8c6cd26c12a817cc316d2709d83611b684972
# Parent  043af8d56a80522302c4f0e4e6d6d1cd6c634416
fncache: move fncache writing to be in a transaction

Previously the fncache was written at lock.release time. This meant it was not
tracked by a transaction, and if an error occurred during the fncache write it
would fail to update the fncache, but would not rollback the transaction,
resulting in an fncache that was not in sync with the files on disk (which
causes verify to fail, and causes streaming clones to not copy all the revlogs).

This uses the new transaction backup mechanism to make the fncache transacted.
It also moves the fncache from being written at lock.release time, to being
written at transaction.close time.

Patch

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -823,13 +823,17 @@ 
             raise error.RepoError(
                 _("abandoned transaction found - run hg recover"))
 
+        def onclose():
+            self.store.write(tr)
+
         self._writejournal(desc)
         renames = [(vfs, x, undoname(x)) for vfs, x in self._journalfiles()]
         rp = report and report or self.ui.warn
         tr = transaction.transaction(rp, self.sopener,
                                      "journal",
                                      aftertrans(renames),
-                                     self.store.createmode)
+                                     self.store.createmode,
+                                     onclose)
         self._transref = weakref.ref(tr)
         return tr
 
@@ -1037,7 +1041,6 @@ 
             return l
 
         def unlock():
-            self.store.write()
             if hasunfilteredcache(self, '_phasecache'):
                 self._phasecache.write()
             for k, ce in self._filecache.items():
diff --git a/mercurial/store.py b/mercurial/store.py
--- a/mercurial/store.py
+++ b/mercurial/store.py
@@ -337,7 +337,7 @@ 
     def copylist(self):
         return ['requires'] + _data.split()
 
-    def write(self):
+    def write(self, tr):
         pass
 
     def __contains__(self, path):
@@ -402,8 +402,9 @@ 
                     raise util.Abort(t)
         fp.close()
 
-    def write(self):
+    def write(self, tr):
         if self._dirty:
+            tr.addbackup('fncache')
             fp = self.vfs('fncache', mode='wb', atomictemp=True)
             if self.entries:
                 fp.write(encodedir('\n'.join(self.entries) + '\n'))
@@ -485,8 +486,8 @@ 
         return (['requires', '00changelog.i'] +
                 ['store/' + f for f in d.split()])
 
-    def write(self):
-        self.fncache.write()
+    def write(self, tr):
+        self.fncache.write(tr)
 
     def _exists(self, f):
         ef = self.encode(f)
diff --git a/tests/test-fncache.t b/tests/test-fncache.t
--- a/tests/test-fncache.t
+++ b/tests/test-fncache.t
@@ -177,4 +177,62 @@ 
 
   $ cd ..
 
+Aborting lock does not prevent fncache writes
 
+  $ cat > exceptionext.py <<EOF
+  > import os
+  > from mercurial import commands, util
+  > from mercurial.extensions import wrapfunction
+  > 
+  > def lockexception(orig, vfs, lockname, wait, releasefn, acquirefn, desc):
+  >     def releasewrap():
+  >         raise util.Abort("forced lock failure")
+  >     return orig(vfs, lockname, wait, releasewrap, acquirefn, desc)
+  > 
+  > def reposetup(ui, repo):
+  >     wrapfunction(repo, '_lock', lockexception)
+  > 
+  > cmdtable = {}
+  > 
+  > EOF
+  $ extpath=`pwd`/exceptionext.py
+  $ hg init fncachetxn
+  $ cd fncachetxn
+  $ printf "[extensions]\nexceptionext=$extpath\n" >> .hg/hgrc
+  $ touch y
+  $ hg ci -qAm y
+  abort: forced lock failure
+  [255]
+  $ cat .hg/store/fncache
+  data/y.i
+
+Aborting transaction prevents fncache change
+
+  $ cat > ../exceptionext.py <<EOF
+  > import os
+  > from mercurial import commands, util, transaction
+  > from mercurial.extensions import wrapfunction
+  > 
+  > def wrapper(orig, self, *args, **kwargs):
+  >     origonclose = self.onclose
+  >     def onclose():
+  >         origonclose()
+  >         raise util.Abort("forced transaction failure")
+  >     self.onclose = onclose
+  >     return orig(self, *args, **kwargs)
+  > 
+  > def uisetup(ui):
+  >     wrapfunction(transaction.transaction, 'close', wrapper)
+  > 
+  > cmdtable = {}
+  > 
+  > EOF
+  $ rm "${extpath}c"
+  $ touch z
+  $ hg ci -qAm z
+  transaction abort!
+  rollback completed
+  abort: forced transaction failure
+  [255]
+  $ cat .hg/store/fncache
+  data/y.i