Rev 4021: Reduce the number of round trips required to create a repository over the network. in http://people.ubuntu.com/~robertc/baz2.0/push.roundtrips

Robert Collins robertc at robertcollins.net
Thu Feb 19 07:28:40 GMT 2009


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

------------------------------------------------------------
revno: 4021
revision-id: robertc at robertcollins.net-20090219072837-vznmfrq7lz1grtti
parent: robertc at robertcollins.net-20090219033543-s63jjuz9vcgrnio9
committer: Robert Collins <robertc at robertcollins.net>
branch nick: push.roundtrips
timestamp: Thu 2009-02-19 18:28:37 +1100
message:
  Reduce the number of round trips required to create a repository over the network.
=== modified file 'NEWS'
--- a/NEWS	2009-02-19 03:09:55 +0000
+++ b/NEWS	2009-02-19 07:28:37 +0000
@@ -65,6 +65,9 @@
     * ``bzrlib.tests.run_suite`` accepts a runner_class parameter
       supporting the use of different runners. (Robert Collins)
 
+    * Creating a repository on a bzr+ssh:// server will now make a single
+      call rather than many VFS calls. (Robert Collins)
+
     * New hook Commands['extend_command'] to allow plugins to access a
       command object before the command is run (or help generated from
       it), without overriding the command. (Robert Collins)

=== modified file 'bzrlib/bzrdir.py'
--- a/bzrlib/bzrdir.py	2009-02-19 03:35:43 +0000
+++ b/bzrlib/bzrdir.py	2009-02-19 07:28:37 +0000
@@ -2757,7 +2757,10 @@
         if custom_format:
             # We will use the custom format to create repositories over the
             # wire; expose its details like rich_root_data for code to query
-            result._custom_format = custom_format
+            if isinstance(custom_format, remote.RemoteRepositoryFormat):
+                result._custom_format = custom_format._custom_format
+            else:
+                result._custom_format = custom_format
             result.rich_root_data = custom_format.rich_root_data
         return result
 

=== modified file 'bzrlib/remote.py'
--- a/bzrlib/remote.py	2009-02-19 03:09:55 +0000
+++ b/bzrlib/remote.py	2009-02-19 07:28:37 +0000
@@ -21,6 +21,7 @@
 
 from bzrlib import (
     branch,
+    bzrdir,
     debug,
     errors,
     graph,
@@ -281,25 +282,65 @@
     def __init__(self):
         repository.RepositoryFormat.__init__(self)
         self._custom_format = None
+        self._network_name = None
 
     def initialize(self, a_bzrdir, shared=False):
+        # Being asked to create on a non RemoteBzrDir:
+        if not isinstance(a_bzrdir, RemoteBzrDir):
+            if self._custom_format:
+                # Custom format requested
+                return self._custom_format.initialize(a_bzrdir, shared=shared)
+            else:
+                # Use the format that the repository we were created to back
+                # has.
+                prior_repo = self._creating_bzrdir.open_repository()
+                prior_repo._ensure_real()
+                return prior_repo._real_repository._format.initialize(
+                    a_bzrdir, shared=shared)
+        # Creating on a remote bzr dir.
+        # 1) get the network name to use.
         if self._custom_format:
-            # This returns a custom instance - e.g. a pack repo, not a remote
-            # repo.
-            return self._custom_format.initialize(a_bzrdir, shared=shared)
-        if not isinstance(a_bzrdir, RemoteBzrDir):
-            prior_repo = self._creating_bzrdir.open_repository()
-            prior_repo._ensure_real()
-            return prior_repo._real_repository._format.initialize(
-                a_bzrdir, shared=shared)
-        # delegate to a real object at this point (remoteBzrDir delegate to the
-        # repository format which would lead to infinite recursion).
-        a_bzrdir._ensure_real()
-        result = a_bzrdir._real_bzrdir.create_repository(shared=shared)
-        if not isinstance(result, RemoteRepository):
-            return self.open(a_bzrdir)
-        else:
-            return result
+            network_name = self._custom_format.network_name()
+        else:
+            # Select the current bzrlib default and ask for that.
+            reference_bzrdir_format = bzrdir.format_registry.get('default')()
+            reference_format = reference_bzrdir_format.repository_format
+            network_name = reference_format.network_name()
+        # 2) try direct creation via RPC
+        path = a_bzrdir._path_for_remote_call(a_bzrdir._client)
+        verb = 'BzrDir.create_repository'
+        if shared:
+            shared_str = 'True'
+        else:
+            shared_str = 'False'
+        try:
+            response = a_bzrdir._call(verb, path, network_name, shared_str)
+        except errors.UnknownSmartMethod:
+            # Fallback - vfs methods
+            if self._custom_format:
+                # This returns a custom instance - e.g. a pack repo, not a remote
+                # repo.
+                return self._custom_format.initialize(a_bzrdir, shared=shared)
+            # delegate to a real object at this point (remoteBzrDir delegate to the
+            # repository format which would lead to infinite recursion).
+            a_bzrdir._ensure_real()
+            result = a_bzrdir._real_bzrdir.create_repository(shared=shared)
+            if not isinstance(result, RemoteRepository):
+                return self.open(a_bzrdir)
+            else:
+                return result
+        else:
+            # Turn the response into a RemoteRepository object.
+            format = RemoteRepositoryFormat()
+            format.rich_root_data = (response[1] == 'yes')
+            format.supports_tree_reference = (response[2] == 'yes')
+            format.supports_external_lookups = (response[3] == 'yes')
+            format._network_name = response[4]
+            # Used to support creating a real format instance when needed.
+            format._creating_bzrdir = a_bzrdir
+            remote_repo = RemoteRepository(a_bzrdir, format)
+            format._creating_repo = remote_repo
+            return remote_repo
     
     def open(self, a_bzrdir):
         if not isinstance(a_bzrdir, RemoteBzrDir):
@@ -322,6 +363,8 @@
                 'Does not support nested trees', target_format)
 
     def network_name(self):
+        if self._network_name:
+            return self._network_name
         self._creating_repo._ensure_real()
         return self._creating_repo._real_repository._format.network_name()
 

=== modified file 'bzrlib/smart/bzrdir.py'
--- a/bzrlib/smart/bzrdir.py	2008-03-27 06:10:18 +0000
+++ b/bzrlib/smart/bzrdir.py	2009-02-19 07:28:37 +0000
@@ -24,6 +24,7 @@
     SmartServerRequest,
     SuccessfulSmartServerResponse,
     )
+from bzrlib.repository import network_format_registry
 
 
 class SmartServerRequestOpenBzrDir(SmartServerRequest):
@@ -51,7 +52,7 @@
         return SuccessfulSmartServerResponse((answer,))
 
 
-class SmartServerRequestFindRepository(SmartServerRequest):
+class SmartServerRequestBzrDir(SmartServerRequest):
 
     def _boolean_to_yes_no(self, a_boolean):
         if a_boolean:
@@ -59,6 +60,50 @@
         else:
             return 'no'
 
+    def _format_to_capabilities(self, repo_format):
+        rich_root = self._boolean_to_yes_no(repo_format.rich_root_data)
+        tree_ref = self._boolean_to_yes_no(
+            repo_format.supports_tree_reference)
+        external_lookup = self._boolean_to_yes_no(
+            repo_format.supports_external_lookups)
+        return rich_root, tree_ref, external_lookup
+
+
+class SmartServerRequestCreateRepository(SmartServerRequestBzrDir):
+
+    def do(self, path, network_name, shared):
+        """Create a repository in the bzr dir at path.
+        
+        This operates precisely like 'bzrdir.create_repository'.
+        
+        If a bzrdir is not present, an exception is propogated
+        rather than 'no branch' because these are different conditions (and
+        this method should only be called after establishing that a bzr dir
+        exists anyway).
+
+        This is the initial version of this method introduced to the smart
+        server for 1.13.
+
+        :param path: The path to the bzrdir.
+        :param network_name: The network name of the repository type to create.
+        :param shared: The value to pass create_repository for the shared
+            parameter.
+        :return: (ok, rich_root, tree_ref, external_lookup, network_name)
+        """
+        bzrdir = BzrDir.open_from_transport(
+            self.transport_from_client_path(path))
+        shared = shared == 'True'
+        format = network_format_registry.get(network_name)
+        bzrdir.repository_format = format
+        result = format.initialize(bzrdir, shared=shared)
+        rich_root, tree_ref, external_lookup = self._format_to_capabilities(
+            result._format)
+        return SuccessfulSmartServerResponse(('ok', rich_root, tree_ref,
+            external_lookup, result._format.network_name()))
+
+
+class SmartServerRequestFindRepository(SmartServerRequestBzrDir):
+
     def _find(self, path):
         """try to find a repository from path upwards
         
@@ -81,11 +126,8 @@
             segments = ['..'] * len(relpath.split('/'))
         else:
             segments = []
-        rich_root = self._boolean_to_yes_no(repository.supports_rich_root())
-        tree_ref = self._boolean_to_yes_no(
-            repository._format.supports_tree_reference)
-        external_lookup = self._boolean_to_yes_no(
-            repository._format.supports_external_lookups)
+        rich_root, tree_ref, external_lookup = self._format_to_capabilities(
+            repository._format)
         return '/'.join(segments), rich_root, tree_ref, external_lookup
 
 

=== modified file 'bzrlib/smart/request.py'
--- a/bzrlib/smart/request.py	2009-02-11 09:54:36 +0000
+++ b/bzrlib/smart/request.py	2009-02-19 07:28:37 +0000
@@ -401,6 +401,8 @@
 request_handlers.register_lazy(
     'Branch.unlock', 'bzrlib.smart.branch', 'SmartServerBranchRequestUnlock')
 request_handlers.register_lazy(
+    'BzrDir.create_repository', 'bzrlib.smart.bzrdir', 'SmartServerRequestCreateRepository')
+request_handlers.register_lazy(
     'BzrDir.find_repository', 'bzrlib.smart.bzrdir', 'SmartServerRequestFindRepositoryV1')
 request_handlers.register_lazy(
     'BzrDir.find_repositoryV2', 'bzrlib.smart.bzrdir', 'SmartServerRequestFindRepositoryV2')

=== modified file 'bzrlib/tests/blackbox/test_push.py'
--- a/bzrlib/tests/blackbox/test_push.py	2009-02-19 02:06:54 +0000
+++ b/bzrlib/tests/blackbox/test_push.py	2009-02-19 07:28:37 +0000
@@ -192,7 +192,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.assertEqual(94, rpc_count)
+        self.assertEqual(75, rpc_count)
 
     def test_push_smart_stacked_streaming_acceptance(self):
         self.setup_smart_server_with_call_log()
@@ -209,7 +209,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.assertEqual(119, rpc_count)
+        self.assertEqual(100, rpc_count)
         remote = Branch.open('public')
         self.assertEndsWith(remote.get_stacked_on_url(), '/parent')
 

=== modified file 'bzrlib/tests/blackbox/test_shared_repository.py'
--- a/bzrlib/tests/blackbox/test_shared_repository.py	2009-02-19 03:35:43 +0000
+++ b/bzrlib/tests/blackbox/test_shared_repository.py	2009-02-19 07:28:37 +0000
@@ -120,4 +120,4 @@
         # 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.assertEqual(40, rpc_count)
+        self.assertEqual(19, rpc_count)

=== modified file 'bzrlib/tests/test_remote.py'
--- a/bzrlib/tests/test_remote.py	2009-02-13 00:52:18 +0000
+++ b/bzrlib/tests/test_remote.py	2009-02-19 07:28:37 +0000
@@ -34,6 +34,7 @@
     pack,
     remote,
     repository,
+    smart,
     tests,
     urlutils,
     )
@@ -446,6 +447,48 @@
             RemoteBzrDirFormat.probe_transport, OldServerTransport())
 
 
+class TestBzrDirCreateRepository(tests.TestCaseWithMemoryTransport):
+
+    def test_backwards_compat(self):
+        self.setup_smart_server_with_call_log()
+        bzrdir = self.make_bzrdir('.')
+        self.reset_smart_call_log()
+        request_handlers = smart.request.request_handlers
+        orig_method = request_handlers.get('BzrDir.create_repository')
+        request_handlers.remove('BzrDir.create_repository')
+        try:
+            repo = bzrdir.create_repository()
+            create_repo_call_count = len([call for call in self.hpss_calls if
+                call[0].method == 'BzrDir.create_repository'])
+            self.assertEqual(1, create_repo_call_count)
+        finally:
+            request_handlers.register('BzrDir.create_repository', orig_method)
+
+    def test_current_server(self):
+        transport = self.get_transport('.')
+        transport = transport.clone('quack')
+        self.make_bzrdir('quack')
+        client = FakeClient(transport.base)
+        reference_bzrdir_format = bzrdir.format_registry.get('default')()
+        reference_format = reference_bzrdir_format.repository_format
+        network_name = reference_format.network_name()
+        client.add_expected_call(
+            'BzrDir.create_repository', ('quack/',
+                'Bazaar pack repository format 1 (needs bzr 0.92)\n', 'False'),
+            'success', ('ok', 'no', 'no', 'no', network_name))
+        a_bzrdir = RemoteBzrDir(transport, remote.RemoteBzrDirFormat(),
+            _client=client)
+        repo = a_bzrdir.create_repository()
+        # We should have got a remote repository
+        self.assertIsInstance(repo, remote.RemoteRepository)
+        # its format should have the settings from the response
+        format = repo._format
+        self.assertFalse(format.rich_root_data)
+        self.assertFalse(format.supports_tree_reference)
+        self.assertFalse(format.supports_external_lookups)
+        self.assertEqual(network_name, format.network_name())
+
+
 class TestBzrDirOpenRepository(tests.TestCase):
 
     def test_backwards_compat_1_2(self):
@@ -453,7 +496,7 @@
         transport.mkdir('quack')
         transport = transport.clone('quack')
         client = FakeClient(transport.base)
-        client.add_unknown_method_response('RemoteRepository.find_repositoryV2')
+        client.add_unknown_method_response('BzrDir.find_repositoryV2')
         client.add_success_response('ok', '', 'no', 'no')
         bzrdir = RemoteBzrDir(transport, remote.RemoteBzrDirFormat(),
             _client=client)

=== modified file 'bzrlib/tests/test_smart.py'
--- a/bzrlib/tests/test_smart.py	2008-11-20 10:43:39 +0000
+++ b/bzrlib/tests/test_smart.py	2009-02-19 07:28:37 +0000
@@ -160,6 +160,23 @@
             request.transport_from_client_path('foo/').base)
 
 
+class TestSmartServerRequestCreateRepository(tests.TestCaseWithMemoryTransport):
+    """Tests for BzrDir.create_repository."""
+
+    def test_makes_repository(self):
+        """When there is a bzrdir present, the call succeeds."""
+        backing = self.get_transport()
+        self.make_bzrdir('.')
+        request_class = bzrlib.smart.bzrdir.SmartServerRequestCreateRepository
+        request = request_class(backing)
+        reference_bzrdir_format = bzrdir.format_registry.get('default')()
+        reference_format = reference_bzrdir_format.repository_format
+        network_name = reference_format.network_name()
+        expected = SuccessfulSmartServerResponse(
+            ('ok', 'no', 'no', 'no', network_name))
+        self.assertEqual(expected, request.execute('', network_name, 'True'))
+
+
 class TestSmartServerRequestFindRepository(tests.TestCaseWithMemoryTransport):
     """Tests for BzrDir.find_repository."""
 




More information about the bazaar-commits mailing list