Rev 4302: Reduce round trips pushing new branches substantially. in http://people.ubuntu.com/~robertc/baz2.0/pending/push.roundtrips

Robert Collins robertc at robertcollins.net
Fri Apr 24 06:08:56 BST 2009


At http://people.ubuntu.com/~robertc/baz2.0/pending/push.roundtrips

------------------------------------------------------------
revno: 4302
revision-id: robertc at robertcollins.net-20090424050851-sdfonaqerfs386t0
parent: robertc at robertcollins.net-20090424004511-8oszlwmvehlqwrla
committer: Robert Collins <robertc at robertcollins.net>
branch nick: push.roundtrips
timestamp: Fri 2009-04-24 15:08:51 +1000
message:
  Reduce round trips pushing new branches substantially.
=== modified file 'NEWS'
--- a/NEWS	2009-04-15 08:20:33 +0000
+++ b/NEWS	2009-04-24 05:08:51 +0000
@@ -73,6 +73,10 @@
   via ~/.bazaar/authentication.conf fails. (Jelmer Vernooij, 
   Vincent Ladeuil, #321918)
 
+* New smart server verb ``BzrDir.initialize_ex`` which implements a
+  refactoring to the core of clone allowing less round trips on new
+  branches. (Robert Collins)
+
 bzr 1.14rc1
 ###########
 :Codename: brisbane-core

=== modified file 'bzrlib/bzrdir.py'
--- a/bzrlib/bzrdir.py	2009-04-24 00:45:11 +0000
+++ b/bzrlib/bzrdir.py	2009-04-24 05:08:51 +0000
@@ -37,6 +37,7 @@
 
 import bzrlib
 from bzrlib import (
+    branch,
     config,
     errors,
     graph,
@@ -61,6 +62,7 @@
 from bzrlib.push import (
     PushResult,
     )
+from bzrlib.repofmt import pack_repo
 from bzrlib.smart.client import _SmartClient
 from bzrlib.store.versioned import WeaveStore
 from bzrlib.transactions import WriteTransaction
@@ -1856,7 +1858,7 @@
     def initialize_on_transport_ex(self, transport, use_existing_dir=False,
         create_prefix=False, force_new_repo=False, stacked_on=None,
         stack_on_pwd=None, repo_format_name=None, make_working_trees=None,
-        shared_repo=False):
+        shared_repo=False, vfs_only=False):
         """Create this format on transport.
 
         The directory to initialize will be created.
@@ -1879,11 +1881,28 @@
             default the format has.
         :param shared_repo: Control whether made repositories are shared or
             not.
+        :param vfs_only: If True do not attempt to use a smart server
         :return: repo, bzrdir, require_stacking, repository_policy. repo is
             None if none was created or found, bzrdir is always valid.
             require_stacking is the result of examining the stacked_on
             parameter and any stacking policy found for the target.
         """
+        if not vfs_only:
+            # Try to hand off to a smart server 
+            try:
+                client_medium = transport.get_smart_medium()
+            except errors.NoSmartMedium:
+                pass
+            else:
+                # TODO: lookup the local format from a server hint.
+                remote_dir_format = RemoteBzrDirFormat()
+                remote_dir_format._network_name = self.network_name()
+                self._supply_sub_formats_to(remote_dir_format)
+                return remote_dir_format.initialize_on_transport_ex(transport,
+                    use_existing_dir=use_existing_dir, create_prefix=create_prefix,
+                    force_new_repo=force_new_repo, stacked_on=stacked_on,
+                    stack_on_pwd=stack_on_pwd, repo_format_name=repo_format_name,
+                    make_working_trees=make_working_trees, shared_repo=shared_repo)
         # XXX: Refactor the create_prefix/no_create_prefix code into a
         #      common helper function
         # The destination may not exist - if so make it according to policy.
@@ -3022,6 +3041,15 @@
         self._supply_sub_formats_to(format)
         return remote.RemoteBzrDir(transport, format)
 
+    def parse_NoneTrueFalse(self, arg):
+        if not arg:
+            return None
+        if arg == 'False':
+            return False
+        if arg == 'True':
+            return True
+        raise AssertionError("invalid arg %r" % arg)
+
     def _serialize_NoneTrueFalse(self, arg):
         if arg is False:
             return 'False'
@@ -3029,6 +3057,9 @@
             return 'True'
         return ''
 
+    def _serialize_NoneString(self, arg):
+        return arg or ''
+
     def initialize_on_transport_ex(self, transport, use_existing_dir=False,
         create_prefix=False, force_new_repo=False, stacked_on=None,
         stack_on_pwd=None, repo_format_name=None, make_working_trees=None,
@@ -3054,21 +3085,31 @@
                 use_existing_dir=use_existing_dir, create_prefix=create_prefix,
                 force_new_repo=force_new_repo, stacked_on=stacked_on,
                 stack_on_pwd=stack_on_pwd, repo_format_name=repo_format_name,
-                make_working_trees=make_working_trees, shared_repo=shared_repo)
-        if not (create_prefix is False and force_new_repo is False and
-            stacked_on is None and stack_on_pwd is None and repo_format_name is
-            None and make_working_trees is None and shared_repo is False):
-            local_dir_format = BzrDirMetaFormat1()
-            self._supply_sub_formats_to(local_dir_format)
-            return local_dir_format.initialize_on_transport_ex(transport,
-                use_existing_dir=use_existing_dir, create_prefix=create_prefix,
-                force_new_repo=force_new_repo, stacked_on=stacked_on,
-                stack_on_pwd=stack_on_pwd, repo_format_name=repo_format_name,
-                make_working_trees=make_working_trees, shared_repo=shared_repo)
+                make_working_trees=make_working_trees, shared_repo=shared_repo,
+                vfs_only=True)
         args = []
         args.append(self._serialize_NoneTrueFalse(use_existing_dir))
+        args.append(self._serialize_NoneTrueFalse(create_prefix))
+        args.append(self._serialize_NoneTrueFalse(force_new_repo))
+        args.append(self._serialize_NoneString(stacked_on))
+        # stack_on_pwd is often/usually our transport
+        if stack_on_pwd:
+            try:
+                stack_on_pwd = transport.relpath(stack_on_pwd)
+                if not stack_on_pwd:
+                    stack_on_pwd = '.'
+            except errors.PathNotChild:
+                pass
+        args.append(self._serialize_NoneString(stack_on_pwd))
+        args.append(self._serialize_NoneString(repo_format_name))
+        args.append(self._serialize_NoneTrueFalse(make_working_trees))
+        args.append(self._serialize_NoneTrueFalse(shared_repo))
+        if self._network_name is None:
+            self._network_name = \
+            BzrDirFormat.get_default_format().network_name()
         try:
-            response = client.call('BzrDirFormat.initialize_ex', path, *args)
+            response = client.call('BzrDirFormat.initialize_ex',
+                self.network_name(), path, *args)
         except errors.UnknownSmartMethod:
             local_dir_format = BzrDirMetaFormat1()
             self._supply_sub_formats_to(local_dir_format)
@@ -3076,10 +3117,37 @@
                 use_existing_dir=use_existing_dir, create_prefix=create_prefix,
                 force_new_repo=force_new_repo, stacked_on=stacked_on,
                 stack_on_pwd=stack_on_pwd, repo_format_name=repo_format_name,
-                make_working_trees=make_working_trees, shared_repo=shared_repo)
+                make_working_trees=make_working_trees, shared_repo=shared_repo,
+                vfs_only=True)
+        repo_path = response[0]
+        bzrdir_name = response[6]
+        require_stacking = response[7]
+        require_stacking = self.parse_NoneTrueFalse(require_stacking)
         format = RemoteBzrDirFormat()
+        format._network_name = bzrdir_name
         self._supply_sub_formats_to(format)
-        return None, remote.RemoteBzrDir(transport, format), None, None
+        bzrdir = remote.RemoteBzrDir(transport, format)
+        if repo_path:
+            repo_format = remote.response_tuple_to_repo_format(response[1:])
+            if repo_path == '.':
+                repo_path = ''
+            if repo_path:
+                repo_bzrdir_format = RemoteBzrDirFormat()
+                repo_bzrdir_format._network_name = response[5]
+                repo_bzr = remote.RemoteBzrDir(transport.clone(repo_path),
+                    repo_bzrdir_format)
+            else:
+                repo_bzr = bzrdir
+            final_stack = response[8] or None
+            final_stack_pwd = response[9] or None
+            remote_repo = remote.RemoteRepository(repo_bzr, repo_format)
+            policy = UseExistingRepository(remote_repo, final_stack,
+                final_stack_pwd, require_stacking)
+            policy.acquire_repository()
+        else:
+            remote_repo = None
+            policy = None
+        return remote_repo, bzrdir, require_stacking, policy
 
     def _open(self, transport):
         return remote.RemoteBzrDir(transport, self)
@@ -3360,8 +3428,13 @@
         stack_on = self._get_full_stack_on()
         if stack_on is None:
             return
-        stacked_dir = BzrDir.open(stack_on,
-                                  possible_transports=possible_transports)
+        try:
+            stacked_dir = BzrDir.open(stack_on,
+                                      possible_transports=possible_transports)
+        except errors.JailBreak:
+            # We keep the stacking details, but we are in the server code so
+            # actually stacking is not needed.
+            return
         try:
             stacked_repo = stacked_dir.open_branch().repository
         except errors.NotBranchError:
@@ -3422,12 +3495,21 @@
                 # network round trips to check - but we only do this
                 # when the source can't stack so it will fade away
                 # as people do upgrade.
+                branch_format = None
+                repo_format = None
                 try:
                     target_dir = BzrDir.open(stack_on,
                         possible_transports=[self._bzrdir.root_transport])
                 except errors.NotBranchError:
                     # Nothing there, don't change formats
                     pass
+                except errors.JailBreak:
+                    # stack_on is inaccessible, JFDI.
+                    if format.repository_format.rich_root_data:
+                        repo_format = pack_repo.RepositoryFormatKnitPack6RichRoot()
+                    else:
+                        repo_format = pack_repo.RepositoryFormatKnitPack6()
+                    branch_format = branch.BzrBranchFormat7()
                 else:
                     try:
                         target_branch = target_dir.open_branch()
@@ -3440,15 +3522,16 @@
                         if not (branch_format.supports_stacking()
                             and repo_format.supports_external_lookups):
                             # Doesn't stack itself, don't force an upgrade
-                            pass
-                        else:
-                            # Does support stacking, use its format.
-                            format.repository_format = repo_format
-                            format.set_branch_format(branch_format)
-                            note('Source format does not support stacking, '
-                                'using format: \'%s\'\n  %s\n',
-                                branch_format.get_format_description(),
-                                repo_format.get_format_description())
+                            branch_format = None
+                            repo_format = None
+                if branch_format and repo_format:
+                    # Does support stacking, use its format.
+                    format.repository_format = repo_format
+                    format.set_branch_format(branch_format)
+                    note('Source format does not support stacking, '
+                        'using format: \'%s\'\n  %s\n',
+                        branch_format.get_format_description(),
+                        repo_format.get_format_description())
             if not self._require_stacking:
                 # We have picked up automatic stacking somewhere.
                 note('Using default stacking branch %s at %s', self._stack_on,

=== modified file 'bzrlib/errors.py'
--- a/bzrlib/errors.py	2009-04-02 10:43:01 +0000
+++ b/bzrlib/errors.py	2009-04-24 05:08:51 +0000
@@ -2960,6 +2960,14 @@
         BzrError.__init__(self, invalid_id=invalid_id)
 
 
+class JailBreak(BzrError):
+
+    _fmt = "An attempt to access a url outside the server jail was made: '%(url)s'."
+
+    def __init__(self, url):
+        BzrError.__init__(self, url=url)
+
+
 class UserAbort(BzrError):
 
     _fmt = 'The user aborted the operation.'

=== modified file 'bzrlib/smart/bzrdir.py'
--- a/bzrlib/smart/bzrdir.py	2009-04-24 00:45:11 +0000
+++ b/bzrlib/smart/bzrdir.py	2009-04-24 05:08:51 +0000
@@ -18,7 +18,12 @@
 
 
 from bzrlib import branch, errors, repository
-from bzrlib.bzrdir import BzrDir, BzrDirFormat, BzrDirMetaFormat1
+from bzrlib.bzrdir import (
+    BzrDir,
+    BzrDirFormat,
+    BzrDirMetaFormat1,
+    network_format_registry,
+    )
 from bzrlib.smart.request import (
     FailedSmartServerResponse,
     SmartServerRequest,
@@ -325,7 +330,7 @@
         return SuccessfulSmartServerResponse(('ok', ))
 
 
-class SmartServerRequestBzrDirInitializeEx(SmartServerRequest):
+class SmartServerRequestBzrDirInitializeEx(SmartServerRequestBzrDir):
 
     def parse_NoneTrueFalse(self, arg):
         if not arg:
@@ -336,16 +341,65 @@
             return True
         raise AssertionError("invalid arg %r" % arg)
 
-    def do(self, path, use_existing_dir):
+    def parse_NoneString(self, arg):
+        return arg or None
+
+    def _serialize_NoneTrueFalse(self, arg):
+        if arg is False:
+            return 'False'
+        if not arg:
+            return ''
+        return 'True'
+
+    def do(self, bzrdir_network_name, path, use_existing_dir, create_prefix,
+        force_new_repo, stacked_on, stack_on_pwd, repo_format_name,
+        make_working_trees, shared_repo):
         """Initialize a bzrdir at path as per BzrDirFormat.initialize_ex
 
         :return: SmartServerResponse()
         """
+        target_transport = self.transport_from_client_path(path)
+        format = network_format_registry.get(bzrdir_network_name)
         use_existing_dir = self.parse_NoneTrueFalse(use_existing_dir)
-        target_transport = self.transport_from_client_path(path)
-        BzrDirFormat.get_default_format().initialize_on_transport_ex(target_transport,
-            use_existing_dir=use_existing_dir)
-        return SuccessfulSmartServerResponse(())
+        create_prefix = self.parse_NoneTrueFalse(create_prefix)
+        force_new_repo = self.parse_NoneTrueFalse(force_new_repo)
+        stacked_on = self.parse_NoneString(stacked_on)
+        stack_on_pwd = self.parse_NoneString(stack_on_pwd)
+        make_working_trees = self.parse_NoneTrueFalse(make_working_trees)
+        shared_repo = self.parse_NoneTrueFalse(shared_repo)
+        if stack_on_pwd == '.':
+            stack_on_pwd = target_transport.base
+        repo_format_name = self.parse_NoneString(repo_format_name)
+        repo, bzrdir, stacking, repository_policy = \
+            format.initialize_on_transport_ex(target_transport,
+            use_existing_dir=use_existing_dir, create_prefix=create_prefix,
+            force_new_repo=force_new_repo, stacked_on=stacked_on,
+            stack_on_pwd=stack_on_pwd, repo_format_name=repo_format_name,
+            make_working_trees=make_working_trees, shared_repo=shared_repo)
+        if repo is None:
+            repo_path = ''
+            repo_name = ''
+            rich_root = tree_ref = external_lookup = ''
+            repo_bzrdir_name = ''
+            final_stack = None
+            final_stack_pwd = None
+        else:
+            repo_path = self._repo_relpath(bzrdir.root_transport, repo)
+            if repo_path == '':
+                repo_path = '.'
+            rich_root, tree_ref, external_lookup = self._format_to_capabilities(
+                repo._format)
+            repo_name = repo._format.network_name()
+            repo_bzrdir_name = repo.bzrdir._format.network_name()
+            final_stack = repository_policy._stack_on
+            final_stack_pwd = repository_policy._stack_on_pwd
+        final_stack = final_stack or ''
+        final_stack_pwd = final_stack_pwd or ''
+        return SuccessfulSmartServerResponse((repo_path, rich_root, tree_ref,
+            external_lookup, repo_name, repo_bzrdir_name,
+            bzrdir._format.network_name(),
+            self._serialize_NoneTrueFalse(stacking), final_stack,
+            final_stack_pwd))
 
 
 class SmartServerRequestOpenBranch(SmartServerRequestBzrDir):

=== modified file 'bzrlib/smart/request.py'
--- a/bzrlib/smart/request.py	2009-04-24 00:45:11 +0000
+++ b/bzrlib/smart/request.py	2009-04-24 05:08:51 +0000
@@ -69,7 +69,7 @@
             continue
         else:
             return
-    raise errors.BzrError('jail break: %r' % (abspath,))
+    raise errors.JailBreak(abspath)
 
 
 _install_hook()

=== modified file 'bzrlib/tests/blackbox/test_push.py'
--- a/bzrlib/tests/blackbox/test_push.py	2009-04-22 02:36:23 +0000
+++ b/bzrlib/tests/blackbox/test_push.py	2009-04-24 05:08:51 +0000
@@ -201,7 +201,7 @@
         # 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(18, self.hpss_calls)
+        self.assertLength(11, self.hpss_calls)
 
     def test_push_smart_stacked_streaming_acceptance(self):
         self.setup_smart_server_with_call_log()
@@ -217,7 +217,7 @@
         # 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(22, self.hpss_calls)
+        self.assertLength(18, self.hpss_calls)
         remote = Branch.open('public')
         self.assertEndsWith(remote.get_stacked_on_url(), '/parent')
 

=== modified file 'bzrlib/tests/bzrdir_implementations/test_bzrdir.py'
--- a/bzrlib/tests/bzrdir_implementations/test_bzrdir.py	2009-04-23 06:07:44 +0000
+++ b/bzrlib/tests/bzrdir_implementations/test_bzrdir.py	2009-04-24 05:08:51 +0000
@@ -1224,7 +1224,9 @@
                 made_repo.bzrdir.root_transport.base)
 
     def test_format_initialize_on_transport_ex_stacked_on(self):
-        # trunk is a stackable format
+        # trunk is a stackable format.  Note that its in the same server area
+        # which is what launchpad does, but not sufficient to exercise the
+        # general case.
         trunk = self.make_branch('trunk', format='1.9')
         t = self.get_transport('stacked')
         old_fmt = bzrdir.format_registry.make_bzrdir('pack-0.92')

=== modified file 'bzrlib/tests/test_errors.py'
--- a/bzrlib/tests/test_errors.py	2009-03-23 14:59:43 +0000
+++ b/bzrlib/tests/test_errors.py	2009-04-24 05:08:51 +0000
@@ -123,6 +123,12 @@
         error = errors.InstallFailed([None])
         self.assertEqual("Could not install revisions:\nNone", str(error))
 
+    def test_jail_break(self):
+        error = errors.JailBreak("some url")
+        self.assertEqualDiff("An attempt to access a url outside the server"
+            " jail was made: 'some url'.",
+            str(error))
+
     def test_lock_active(self):
         error = errors.LockActive("lock description")
         self.assertEqualDiff("The lock for 'lock description' is in use and "

=== modified file 'bzrlib/tests/test_smart.py'
--- a/bzrlib/tests/test_smart.py	2009-04-24 00:45:11 +0000
+++ b/bzrlib/tests/test_smart.py	2009-04-24 05:08:51 +0000
@@ -359,8 +359,12 @@
     def test_empty_dir(self):
         """Initializing an empty dir should succeed and do it."""
         backing = self.get_transport()
+        name = self.make_bzrdir('reference')._format.network_name()
         request = smart.bzrdir.SmartServerRequestBzrDirInitializeEx(backing)
-        self.assertEqual(SmartServerResponse(()), request.execute('', 'True'))
+        self.assertEqual(SmartServerResponse(('', '', '', '', '', '', name,
+            'False', '', '')),
+            request.execute(name, '', 'True', 'False', 'False', '', '', '', '',
+            'False'))
         made_dir = bzrdir.BzrDir.open_from_transport(backing)
         # no branch, tree or repository is expected with the current
         # default formart.
@@ -371,15 +375,19 @@
     def test_missing_dir(self):
         """Initializing a missing directory should fail like the bzrdir api."""
         backing = self.get_transport()
+        name = self.make_bzrdir('reference')._format.network_name()
         request = smart.bzrdir.SmartServerRequestBzrDirInitializeEx(backing)
-        self.assertRaises(errors.NoSuchFile, request.execute, 'subdir/dir', 'False')
+        self.assertRaises(errors.NoSuchFile, request.execute, name,
+            'subdir/dir', 'False', 'False', 'False', '', '', '', '', 'False')
 
     def test_initialized_dir(self):
         """Initializing an extant dirctory should fail like the bzrdir api."""
         backing = self.get_transport()
+        name = self.make_bzrdir('reference')._format.network_name()
         request = smart.bzrdir.SmartServerRequestBzrDirInitializeEx(backing)
         self.make_bzrdir('subdir')
-        self.assertRaises(errors.FileExists, request.execute, 'subdir', 'False')
+        self.assertRaises(errors.FileExists, request.execute, name, 'subdir',
+            'False', 'False', 'False', '', '', '', '', 'False')
 
 
 class TestSmartServerRequestOpenBranch(TestCaseWithChrootedTransport):

=== modified file 'bzrlib/urlutils.py'
--- a/bzrlib/urlutils.py	2009-04-11 00:29:14 +0000
+++ b/bzrlib/urlutils.py	2009-04-24 05:08:51 +0000
@@ -724,9 +724,6 @@
     if host != "" and host[0] == '[' and host[-1] == ']': #IPv6
         host = host[1:-1]
 
-    if host == '':
-        raise errors.InvalidURL('Host empty in: %s' % url)
-
     host = urllib.unquote(host)
     path = urllib.unquote(path)
 




More information about the bazaar-commits mailing list