[PATCH] transaction: move changelog finalizer to be before bookmark finalizer

Durham Goode durham at fb.com
Thu Mar 17 02:31:50 EDT 2016


# HG changeset patch
# User Durham Goode <durham at fb.com>
# Date 1458196059 25200
#      Wed Mar 16 23:27:39 2016 -0700
# Node ID cdf4a94fdc46d27196f5411fc7a4008690834fba
# Parent  ed75909c4c670a7d9db4a2bef9817a0d5f0b4d9c
transaction: move changelog finalizer to be before bookmark finalizer

Previously, transaction close would run the file generators before running the
finalizers (see the list below for what is in each). Since file generators
contain the bookmarks and the dirstate, this meant we made the dirstate and
bookmarks visible to external readers before we actually wrote the commits into
the changelog, which could result in missing bookmarks and missing working copy
parents (especially on servers with high commit throughput, since pulls might
fail to see certain bookmarks in this situation).

By moving the changelog writing to be before the bookmark/dirstate writing, we
ensure the commits are present before they are referenced.

For reference, file generators currently consist of: bookmarks, dirstate, and
phases. Finalizers currently consist of: changelog, revbranchcache, and fncache.
All of the former reference the latter, so therefore the latter should be
written first.

Technically there's still plenty of race conditions (can the order of finalizers
affect how external readers see the repo?), but this is a step forward at least.

diff --git a/mercurial/transaction.py b/mercurial/transaction.py
--- a/mercurial/transaction.py
+++ b/mercurial/transaction.py
@@ -407,10 +407,10 @@ class transaction(object):
         '''commit the transaction'''
         if self.count == 1:
             self.validator(self)  # will raise exception if needed
-            self._generatefiles()
             categories = sorted(self._finalizecallback)
             for cat in categories:
                 self._finalizecallback[cat](self)
+            self._generatefiles()
 
         self.count -= 1
         if self.count != 0:
diff --git a/tests/test-bookmarks.t b/tests/test-bookmarks.t
--- a/tests/test-bookmarks.t
+++ b/tests/test-bookmarks.t
@@ -846,3 +846,52 @@ updates the working directory and curren
   6:81dcce76aa0b
   $ hg -R ../cloned-bookmarks-update bookmarks | grep ' Y '
    * Y                         6:81dcce76aa0b
+
+  $ cd ..
+
+ensure changelog is written before bookmarks
+  $ hg init orderrepo
+  $ cd orderrepo
+  $ touch a
+  $ hg commit -Aqm one
+  $ hg book mybook
+  $ echo a > a
+
+  $ cat > $TESTTMP/pausefinalize.py <<EOF
+  > from mercurial import extensions, localrepo
+  > import os, time
+  > def transaction(orig, self, desc, report=None):
+  >    tr = orig(self, desc, report)
+  >    def sleep(*args, **kwargs):
+  >        retry = 20
+  >        while retry > 0 and not os.path.exists("$TESTTMP/unpause"):
+  >            retry -= 1
+  >            time.sleep(0.5)
+  >        if os.path.exists("$TESTTMP/unpause"):
+  >            os.remove("$TESTTMP/unpause")
+  >    # It is important that this finalizer start with 'a', so it runs before
+  >    # the changelog finalizer appends to the changelog.
+  >    tr.addfinalize('a-sleep', sleep)
+  >    return tr
+  > 
+  > def extsetup(ui):
+  >    # This extension inserts an artifical pause during the transaction
+  >    # finalizer, so we can run commands mid-transaction-close.
+  >    extensions.wrapfunction(localrepo.localrepository, 'transaction',
+  >                            transaction)
+  > EOF
+  $ hg commit -qm two --config extensions.pausefinalize=$TESTTMP/pausefinalize.py &
+  $ sleep 2
+  $ hg log -r .
+  changeset:   0:867bc5792c8c
+  bookmark:    mybook
+  tag:         tip
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  summary:     one
+  
+  $ hg bookmarks
+   * mybook                    0:867bc5792c8c
+  $ touch $TESTTMP/unpause
+
+  $ cd ..


More information about the Mercurial-devel mailing list