Patchwork [2,of,2] bundle2: lazily acquire the lock

login
register
mail settings
Submitter Durham Goode
Date Oct. 6, 2015, 10:08 p.m.
Message ID <0798ccc5711a08f17059.1444169304@dev2000.prn2.facebook.com>
Download mbox | patch
Permalink /patch/10836/
State Changes Requested
Headers show

Comments

Durham Goode - Oct. 6, 2015, 10:08 p.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1444087194 25200
#      Mon Oct 05 16:19:54 2015 -0700
# Node ID 0798ccc5711a08f170594db936fb3494fa892668
# Parent  0e304cffebad9b34e941951a7bd0211444c9911f
bundle2: lazily acquire 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 makes the lock lazily acquired. It means that all bundle2 part
handlers that require writting to the repo must first call op.gettransction(),
but they should probably be doing this anyway, since in theory all writes should
be within a transaction.
Pierre-Yves David - Oct. 6, 2015, 10:30 p.m.
On 10/06/2015 03:08 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 0798ccc5711a08f170594db936fb3494fa892668
> # Parent  0e304cffebad9b34e941951a7bd0211444c9911f
> bundle2: lazily acquire the lock

I love the idea of two fold processing to allow runnign some check 
beforte the lock. However I feel like this is too bold as his for now. 
Can we hide all this behind an experimental flag as we play around with 
the concept?

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,17 @@  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]
+
+                op = bundle2.bundleoperation(repo, gettransaction,
                                              captureoutput=captureoutput)
                 try:
                     op = bundle2.processbundle(repo, cg, op=op)
@@ -1470,7 +1475,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 +1487,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