Patchwork [2,of,2,V3] bundle2: allow lazily acquiring the lock

login
register
mail settings
Submitter Durham Goode
Date Oct. 7, 2015, 10:19 p.m.
Message ID <374ed05cec240732fa4b.1444256386@dev2000.prn2.facebook.com>
Download mbox | patch
Permalink /patch/10868/
State Accepted
Headers show

Comments

Durham Goode - Oct. 7, 2015, 10:19 p.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1444087194 25200
#      Mon Oct 05 16:19:54 2015 -0700
# Node ID 374ed05cec240732fa4ba41d66f92f49a04a4bac
# Parent  1c8b56dd34406c2dfb2942d70ce427ef0d359f0c
bundle2: allow lazily acquiring the lock

In the external pushrebase extension, it is valuable to be able to do some work
without taking the lock (like running expensive hooks). This enables
significantly higher commit throughput.

This patch adds an option to lazily acquire the lock. It means that all bundle2
part handlers that require writing to the repo must first call
op.gettransction(), when in this mode.
Pierre-Yves David - Oct. 8, 2015, 9:24 p.m.
On 10/07/2015 03:19 PM, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1444087194 25200
> #      Mon Oct 05 16:19:54 2015 -0700
> # Node ID 374ed05cec240732fa4ba41d66f92f49a04a4bac
> # Parent  1c8b56dd34406c2dfb2942d70ce427ef0d359f0c
> bundle2: allow lazily acquiring the lock

This two are pushed to the clowncopter, thanks.

> @@ -1470,7 +1480,8 @@ def unbundle(repo, cg, heads, source, ur
>                           repo.ui.pushbuffer(error=True, subproc=True)
>                           def recordout(output):
>                               r.newpart('output', data=output, mandatory=False)
> -                tr.close()
> +                if lockandtr[2]:
> +                    lockandtr[2].close()

PSA: if you test None, test None, changed to
'if lockandtr[2] is not None:'

Patch

diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -1441,7 +1441,8 @@  def unbundle(repo, cg, heads, source, ur
     If the push was raced as PushRaced exception is raised."""
     r = 0
     # need a transaction when processing a bundle2 stream
-    wlock = lock = tr = None
+    # [wlock, lock, tr] - needs to be an array so nested functions can modify it
+    lockandtr = [None, None, None]
     recordout = None
     # quick fix for output mismatch with bundle2 in 3.4
     captureoutput = repo.ui.configbool('experimental', 'bundle2-output-capture',
@@ -1454,13 +1455,22 @@  def unbundle(repo, cg, heads, source, ur
         if util.safehasattr(cg, 'params'):
             r = None
             try:
-                wlock = repo.wlock()
-                lock = repo.lock()
-                tr = repo.transaction(source)
-                tr.hookargs['source'] = source
-                tr.hookargs['url'] = url
-                tr.hookargs['bundle2'] = '1'
-                op = bundle2.bundleoperation(repo, lambda: tr,
+                def gettransaction():
+                    if not lockandtr[2]:
+                        lockandtr[0] = repo.wlock()
+                        lockandtr[1] = repo.lock()
+                        lockandtr[2] = repo.transaction(source)
+                        lockandtr[2].hookargs['source'] = source
+                        lockandtr[2].hookargs['url'] = url
+                        lockandtr[2].hookargs['bundle2'] = '1'
+                    return lockandtr[2]
+
+                # Do greedy locking by default until we're satisfied with lazy
+                # locking.
+                if not repo.ui.configbool('experimental', 'bundle2lazylocking'):
+                    gettransaction()
+
+                op = bundle2.bundleoperation(repo, gettransaction,
                                              captureoutput=captureoutput)
                 try:
                     op = bundle2.processbundle(repo, cg, op=op)
@@ -1470,7 +1480,8 @@  def unbundle(repo, cg, heads, source, ur
                         repo.ui.pushbuffer(error=True, subproc=True)
                         def recordout(output):
                             r.newpart('output', data=output, mandatory=False)
-                tr.close()
+                if lockandtr[2]:
+                    lockandtr[2].close()
             except BaseException as exc:
                 exc.duringunbundle2 = True
                 if captureoutput and r is not None:
@@ -1481,10 +1492,10 @@  def unbundle(repo, cg, heads, source, ur
                         parts.append(part)
                 raise
         else:
-            lock = repo.lock()
+            lockandtr[1] = repo.lock()
             r = changegroup.addchangegroup(repo, cg, source, url)
     finally:
-        lockmod.release(tr, lock, wlock)
+        lockmod.release(lockandtr[2], lockandtr[1], lockandtr[0])
         if recordout is not None:
             recordout(repo.ui.popbuffer())
     return r
diff --git a/tests/test-bundle2-exchange.t b/tests/test-bundle2-exchange.t
--- a/tests/test-bundle2-exchange.t
+++ b/tests/test-bundle2-exchange.t
@@ -7,6 +7,7 @@  Test exchange of common information usin
 
 enable obsolescence
 
+  $ cp $HGRCPATH $TESTTMP/hgrc.orig
   $ cat > $TESTTMP/bundle2-pushkey-hook.sh << EOF
   > echo pushkey: lock state after \"\$HG_NAMESPACE\"
   > hg debuglock
@@ -897,3 +898,47 @@  Check abort from mandatory pushkey
   abort: Clown phase push failed
   [255]
 
+Test lazily acquiring the lock during unbundle
+  $ cp $TESTTMP/hgrc.orig $HGRCPATH
+  $ cat >> $HGRCPATH <<EOF
+  > [ui]
+  > ssh=python "$TESTDIR/dummyssh"
+  > EOF
+
+  $ cat >> $TESTTMP/locktester.py <<EOF
+  > import os
+  > from mercurial import extensions, bundle2, util
+  > def checklock(orig, repo, *args, **kwargs):
+  >     if repo.svfs.lexists("lock"):
+  >         raise util.Abort("Lock should not be taken")
+  >     return orig(repo, *args, **kwargs)
+  > def extsetup(ui):
+  >    extensions.wrapfunction(bundle2, 'processbundle', checklock)
+  > EOF
+
+  $ hg init lazylock
+  $ cat >> lazylock/.hg/hgrc <<EOF
+  > [extensions]
+  > locktester=$TESTTMP/locktester.py
+  > EOF
+
+  $ hg clone -q ssh://user@dummy/lazylock lazylockclient
+  $ cd lazylockclient
+  $ touch a && hg ci -Aqm a
+  $ hg push
+  pushing to ssh://user@dummy/lazylock
+  searching for changes
+  abort: Lock should not be taken
+  [255]
+
+  $ cat >> ../lazylock/.hg/hgrc <<EOF
+  > [experimental]
+  > bundle2lazylocking=True
+  > EOF
+  $ hg push
+  pushing to ssh://user@dummy/lazylock
+  searching for changes
+  remote: adding changesets
+  remote: adding manifests
+  remote: adding file changes
+  remote: added 1 changesets with 1 changes to 1 files