Rev 4573: (andrew) Add Branch.set_tags_bytes verb, removing more VFS calls. in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Mon Jul 27 10:33:40 BST 2009


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 4573 [merge]
revision-id: pqm at pqm.ubuntu.com-20090727093330-882xn6s1tt1zbnw6
parent: pqm at pqm.ubuntu.com-20090727073227-no0xwxw2i11oadqt
parent: andrew.bennetts at canonical.com-20090727080252-1r4s9oqwlkzgywx7
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Mon 2009-07-27 10:33:30 +0100
message:
  (andrew) Add Branch.set_tags_bytes verb, removing more VFS calls.
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/remote.py               remote.py-20060720103555-yeeg2x51vn0rbtdp-1
  bzrlib/smart/branch.py         branch.py-20061124031907-mzh3pla28r83r97f-1
  bzrlib/smart/request.py        request.py-20061108095550-gunadhxmzkdjfeek-1
  bzrlib/tests/blackbox/test_push.py test_push.py-20060329002750-929af230d5d22663
  bzrlib/tests/test_smart.py     test_smart.py-20061122024551-ol0l0o0oofsu9b3t-2
  bzrlib/tests/test_smart_request.py test_smart_request.p-20090211070731-o38wayv3asm25d6a-1
=== modified file 'NEWS'
--- a/NEWS	2009-07-24 18:26:21 +0000
+++ b/NEWS	2009-07-27 05:24:02 +0000
@@ -94,6 +94,11 @@
 * Can now rename/move files even if they have been removed from the inventory.
   (Marius Kruger)
 
+* Pushing branches with tags via ``bzr://`` and ``bzr+ssh://`` is much
+  faster, using a new ``Branch.set_tags_bytes`` smart server verb rather
+  than VFS methods.  For example, pushes of small branches with tags take
+  11 rather than 18 smart server requests.  (Andrew Bennetts, #398608)
+
 Documentation
 *************
 

=== modified file 'bzrlib/remote.py'
--- a/bzrlib/remote.py	2009-07-20 10:14:42 +0000
+++ b/bzrlib/remote.py	2009-07-27 08:02:52 +0000
@@ -59,6 +59,12 @@
         except errors.ErrorFromSmartServer, err:
             self._translate_error(err, **err_context)
 
+    def _call_with_body_bytes(self, method, args, body_bytes, **err_context):
+        try:
+            return self._client.call_with_body_bytes(method, args, body_bytes)
+        except errors.ErrorFromSmartServer, err:
+            self._translate_error(err, **err_context)
+
     def _call_with_body_bytes_expecting_body(self, method, args, body_bytes,
                                              **err_context):
         try:
@@ -2162,6 +2168,23 @@
             return self._vfs_get_tags_bytes()
         return response[0]
 
+    def _vfs_set_tags_bytes(self, bytes):
+        self._ensure_real()
+        return self._real_branch._set_tags_bytes(bytes)
+
+    def _set_tags_bytes(self, bytes):
+        medium = self._client._medium
+        if medium._is_remote_before((1, 18)):
+            self._vfs_set_tags_bytes(bytes)
+        try:
+            args = (
+                self._remote_path(), self._lock_token, self._repo_lock_token)
+            response = self._call_with_body_bytes(
+                'Branch.set_tags_bytes', args, bytes)
+        except errors.UnknownSmartMethod:
+            medium._remember_remote_is_before((1, 18))
+            self._vfs_set_tags_bytes(bytes)
+
     def lock_read(self):
         self.repository.lock_read()
         if not self._lock_mode:
@@ -2221,10 +2244,6 @@
             self.repository.lock_write(self._repo_lock_token)
         return self._lock_token or None
 
-    def _set_tags_bytes(self, bytes):
-        self._ensure_real()
-        return self._real_branch._set_tags_bytes(bytes)
-
     def _unlock(self, branch_token, repo_token):
         err_context = {'token': str((branch_token, repo_token))}
         response = self._call(

=== modified file 'bzrlib/smart/branch.py'
--- a/bzrlib/smart/branch.py	2009-04-15 02:07:35 +0000
+++ b/bzrlib/smart/branch.py	2009-07-27 04:32:56 +0000
@@ -103,6 +103,42 @@
         return SuccessfulSmartServerResponse((bytes,))
 
 
+class SmartServerBranchSetTagsBytes(SmartServerLockedBranchRequest):
+
+    def __init__(self, backing_transport, root_client_path='/'):
+        SmartServerLockedBranchRequest.__init__(
+            self, backing_transport, root_client_path)
+        self.locked = False
+        
+    def do_with_locked_branch(self, branch):
+        """Call _set_tags_bytes for a branch.
+
+        New in 1.18.
+        """
+        # We need to keep this branch locked until we get a body with the tags
+        # bytes.
+        self.branch = branch
+        self.branch.lock_write()
+        self.locked = True
+
+    def do_body(self, bytes):
+        self.branch._set_tags_bytes(bytes)
+        return SuccessfulSmartServerResponse(())
+
+    def do_end(self):
+        # TODO: this request shouldn't have to do this housekeeping manually.
+        # Some of this logic probably belongs in a base class.
+        if not self.locked:
+            # We never acquired the branch successfully in the first place, so
+            # there's nothing more to do.
+            return
+        try:
+            return SmartServerLockedBranchRequest.do_end(self)
+        finally:
+            # Only try unlocking if we locked successfully in the first place
+            self.branch.unlock()
+
+
 class SmartServerBranchRequestGetStackedOnURL(SmartServerBranchRequest):
 
     def do_with_branch(self, branch):

=== modified file 'bzrlib/smart/request.py'
--- a/bzrlib/smart/request.py	2009-07-17 01:47:01 +0000
+++ b/bzrlib/smart/request.py	2009-07-27 02:06:05 +0000
@@ -398,7 +398,7 @@
     elif isinstance(err, errors.TokenMismatch):
         return ('TokenMismatch', err.given_token, err.lock_token)
     elif isinstance(err, errors.LockContention):
-        return ('LockContention', err.lock, err.msg)
+        return ('LockContention',)
     # Unserialisable error.  Log it, and return a generic error
     trace.log_exception_quietly()
     return ('error', str(err))
@@ -451,6 +451,9 @@
     'Branch.get_tags_bytes', 'bzrlib.smart.branch',
     'SmartServerBranchGetTagsBytes')
 request_handlers.register_lazy(
+    'Branch.set_tags_bytes', 'bzrlib.smart.branch',
+    'SmartServerBranchSetTagsBytes')
+request_handlers.register_lazy(
     'Branch.get_stacked_on_url', 'bzrlib.smart.branch', 'SmartServerBranchRequestGetStackedOnURL')
 request_handlers.register_lazy(
     'Branch.last_revision_info', 'bzrlib.smart.branch', 'SmartServerBranchRequestLastRevisionInfo')

=== modified file 'bzrlib/tests/blackbox/test_push.py'
--- a/bzrlib/tests/blackbox/test_push.py	2009-07-20 04:26:55 +0000
+++ b/bzrlib/tests/blackbox/test_push.py	2009-07-22 10:55:35 +0000
@@ -239,6 +239,20 @@
         remote = branch.Branch.open('public')
         self.assertEndsWith(remote.get_stacked_on_url(), '/parent')
 
+    def test_push_smart_tags_streaming_acceptance(self):
+        self.setup_smart_server_with_call_log()
+        t = self.make_branch_and_tree('from')
+        rev_id = t.commit(allow_pointless=True, message='first commit')
+        t.branch.tags.set_tag('new-tag', rev_id)
+        self.reset_smart_call_log()
+        self.run_bzr(['push', self.get_url('to-one')], working_dir='from')
+        # This figure represent the amount of work to perform this use case. It
+        # is entirely ok to reduce this number if a test fails due to rpc_count
+        # being too low. If rpc_count increases, more network roundtrips have
+        # become necessary for this use case. Please do not adjust this number
+        # upwards without agreement from bzr's network support maintainers.
+        self.assertLength(11, self.hpss_calls)
+
     def test_push_smart_with_default_stacking_url_path_segment(self):
         # If the default stacked-on location is a path element then branches
         # we push there over the smart server are stacked and their

=== modified file 'bzrlib/tests/test_smart.py'
--- a/bzrlib/tests/test_smart.py	2009-07-20 05:13:04 +0000
+++ b/bzrlib/tests/test_smart.py	2009-07-23 07:37:05 +0000
@@ -603,6 +603,43 @@
         branch.unlock()
 
 
+class TestSmartServerBranchRequestSetTagsBytes(TestLockedBranch):
+    # Only called when the branch format and tags match [yay factory
+    # methods] so only need to test straight forward cases.
+
+    def test_set_bytes(self):
+        base_branch = self.make_branch('base')
+        tag_bytes = base_branch._get_tags_bytes()
+        # get_lock_tokens takes out a lock.
+        branch_token, repo_token = self.get_lock_tokens(base_branch)
+        request = smart.branch.SmartServerBranchSetTagsBytes(
+            self.get_transport())
+        response = request.execute('base', branch_token, repo_token)
+        self.assertEqual(None, response)
+        response = request.do_chunk(tag_bytes)
+        self.assertEqual(None, response)
+        response = request.do_end()
+        self.assertEquals(
+            SuccessfulSmartServerResponse(()), response)
+        base_branch.unlock()
+
+    def test_lock_failed(self):
+        base_branch = self.make_branch('base')
+        base_branch.lock_write()
+        tag_bytes = base_branch._get_tags_bytes()
+        request = smart.branch.SmartServerBranchSetTagsBytes(
+            self.get_transport())
+        self.assertRaises(errors.TokenMismatch, request.execute,
+            'base', 'wrong token', 'wrong token')
+        # The request handler will keep processing the message parts, so even
+        # if the request fails immediately do_chunk and do_end are still
+        # called.
+        request.do_chunk(tag_bytes)
+        request.do_end()
+        base_branch.unlock()
+
+
+
 class SetLastRevisionTestBase(TestLockedBranch):
     """Base test case for verbs that implement set_last_revision."""
 
@@ -872,8 +909,8 @@
 
 
 class TestSmartServerBranchRequestGetTagsBytes(tests.TestCaseWithMemoryTransport):
-# Only called when the branch format and tags match [yay factory
-# methods] so only need to test straight forward cases.
+    # Only called when the branch format and tags match [yay factory
+    # methods] so only need to test straight forward cases.
 
     def test_get_bytes(self):
         base_branch = self.make_branch('base')

=== modified file 'bzrlib/tests/test_smart_request.py'
--- a/bzrlib/tests/test_smart_request.py	2009-04-27 03:34:12 +0000
+++ b/bzrlib/tests/test_smart_request.py	2009-07-27 02:11:25 +0000
@@ -159,9 +159,11 @@
             ('NoSuchFile', 'path'), errors.NoSuchFile('path'))
 
     def test_LockContention(self):
+        # For now, LockContentions are always transmitted with no details.
+        # Eventually they should include a relpath or url or something else to
+        # identify which lock is busy.
         self.assertTranslationEqual(
-            ('LockContention', 'lock', 'msg'),
-            errors.LockContention('lock', 'msg'))
+            ('LockContention',), errors.LockContention('lock', 'msg'))
 
     def test_TokenMismatch(self):
         self.assertTranslationEqual(




More information about the bazaar-commits mailing list