localrepo: refresh filecache stats only if transaction finished successfully
authorYuya Nishihara <yuya@tcha.org>
Tue, 15 Sep 2015 00:32:39 +0900
changeset 26251 5c0f5db65c6b
parent 26250 bc1f8a79b4e4
child 26252 a3edce86f430
localrepo: refresh filecache stats only if transaction finished successfully If commit is aborted by pretxncommit hook, in-memory changelog and manifest have entries that would be added. So they must be discarded on invalidate(). But the mechanism introduced by a710936c3037 doesn't handle this case well. It tries to mitigate the penalty of invalidate() by marking in-memory cache as "clean" on unlock assuming that they are identical to the stored data. But this assumption is wrong if stored data are rolled back by tr.abort(). This patch moves the hook to post-close action so that it will never be triggered on abort. This bug was originally reported to thg, which is only reproducible in command-server process on unix, evolve disabled. https://bitbucket.org/tortoisehg/thg/issues/4285/
mercurial/localrepo.py
tests/test-commandserver.t
--- a/mercurial/localrepo.py	Tue Sep 15 21:00:28 2015 +0900
+++ b/mercurial/localrepo.py	Tue Sep 15 00:32:39 2015 +0900
@@ -1021,6 +1021,9 @@
             reporef().hook('txnabort', throw=False, txnname=desc,
                            **tr2.hookargs)
         tr.addabort('txnabort-hook', txnaborthook)
+        # avoid eager cache invalidation. in-memory data should be identical
+        # to stored data if transaction has no error.
+        tr.addpostclose('refresh-filecachestats', self._refreshfilecachestats)
         self._transref = weakref.ref(tr)
         return tr
 
@@ -1198,7 +1201,7 @@
         self.invalidate()
         self.invalidatedirstate()
 
-    def _refreshfilecachestats(self):
+    def _refreshfilecachestats(self, tr):
         """Reload stats of cached files so that they are flagged as valid"""
         for k, ce in self._filecache.items():
             if k == 'dirstate' or k not in self.__dict__:
@@ -1247,7 +1250,7 @@
             l.lock()
             return l
 
-        l = self._lock(self.svfs, "lock", wait, self._refreshfilecachestats,
+        l = self._lock(self.svfs, "lock", wait, None,
                        self.invalidate, _('repository %s') % self.origroot)
         self._lockref = weakref.ref(l)
         return l
--- a/tests/test-commandserver.t	Tue Sep 15 21:00:28 2015 +0900
+++ b/tests/test-commandserver.t	Tue Sep 15 00:32:39 2015 +0900
@@ -415,6 +415,30 @@
   *** runcommand branches
   default                        1:731265503d86
 
+in-memory cache must be reloaded if transaction is aborted. otherwise
+changelog and manifest would have invalid node:
+
+  $ echo a >> a
+  >>> from hgclient import readchannel, runcommand, check
+  >>> @check
+  ... def txabort(server):
+  ...     readchannel(server)
+  ...     runcommand(server, ['commit', '--config', 'hooks.pretxncommit=false',
+  ...                         '-mfoo'])
+  ...     runcommand(server, ['verify'])
+  *** runcommand commit --config hooks.pretxncommit=false -mfoo
+  transaction abort!
+  rollback completed
+  abort: pretxncommit hook exited with status 1
+   [255]
+  *** runcommand verify
+  checking changesets
+  checking manifests
+  crosschecking files in changesets and manifests
+  checking files
+  1 files, 2 changesets, 2 total revisions
+  $ hg revert --no-backup -aq
+
   $ cat >> .hg/hgrc << EOF
   > [experimental]
   > evolution=createmarkers