Bazaar

Merge lp:~jameinel/bzr/2.1-client-read-reconnect-819604 into lp:bzr/2.1

  1. 2.1-client-read-reconnect-819604
  2. Merge into 2.1
Proposed by John A Meinel
Status: Work in progress
Proposed branch: lp:~jameinel/bzr/2.1-client-read-reconnect-819604
Merge into: lp:bzr/2.1
Prerequisite: lp:~jameinel/bzr/2.1-categorize-requests-819604
Diff against target: 138 lines (+79/-5)
3 files modified
NEWS (+5/-0)
bzrlib/smart/client.py (+36/-2)
bzrlib/tests/test_smart_transport.py (+38/-3)
To merge this branch: bzr merge lp:~jameinel/bzr/2.1-client-read-reconnect-819604
Reviewer Review Type Date Requested Status
bzr-core Pending
Review via email: mp+78839@code.launchpad.net

Commit message

Bug #819604, retry the request if we get ConnectionReset while reading as well.

Description of the change

This is the final step in the chain.

This adds the ability to retry the request if we get a ConnectionReset while reading the response from the server.

Note that this only retries the request while parsing the response header (ok, error, etc). Not while reading the actual body content. However, as mentioned in the beginning, that is the use case that we really care about. We are trying to allow the server to soft-disconnect us, in which case it shouldn't send any response at all.

(The other thing we could have done is upgraded the RPC to allow the server to send a 'reconnect' message, but that wouldn't have been compatible with older clients, especially since they wouldn't have been listening for that response at the end of a previous message, etc.)

Note that I've tested this by running bzr.dev on devpad, and using 'bzr branch bzr+ssh://...' and then while on devpad, running 'kill -SIGHUP PID' on whatever bzr is currently active. I wasn't able to actually stop the branch from finishing, even though I killed it in a few different places.

And the client appropriately reported that it got ConnectionReset but that it was going to try again.

To post a comment you must log in.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'NEWS'
--- NEWS 2011-08-19 12:49:28 +0000
+++ NEWS 2011-10-10 13:23:11 +0000
@@ -43,6 +43,11 @@
4343
44 (John Arbash Meinel, #609187, #812928)44 (John Arbash Meinel, #609187, #812928)
4545
46* Teach the bzr client how to reconnect if we get ``ConnectionReset``
47 while making an RPC request. This doesn't handle all possible network
48 disconnects, but it should at least handle when the server is asked to
49 shutdown gracefully. (John Arbash Meinel, #819604)
50
4651
47Improvements52Improvements
48************53************
4954
=== modified file 'bzrlib/smart/client.py'
--- bzrlib/smart/client.py 2011-10-10 13:23:11 +0000
+++ bzrlib/smart/client.py 2011-10-10 13:23:11 +0000
@@ -14,6 +14,11 @@
14# along with this program; if not, write to the Free Software14# along with this program; if not, write to the Free Software
15# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA15# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
1616
17from bzrlib import lazy_import
18lazy_import.lazy_import(globals(), """
19from bzrlib.smart import request as _mod_request
20""")
21
17import bzrlib22import bzrlib
18from bzrlib.smart import message, protocol23from bzrlib.smart import message, protocol
19from bzrlib import (24from bzrlib import (
@@ -149,6 +154,22 @@
149 else:154 else:
150 return self._call(protocol_version)155 return self._call(protocol_version)
151156
157 def _is_safe_to_send_twice(self):
158 """Check if the current method is re-entrant safe."""
159 if self.body_stream is not None or 'noretry' in debug.debug_flags:
160 # We can't restart a body stream that has already been consumed.
161 return False
162 request_type = _mod_request.request_handlers.get_info(self.method)
163 if request_type in ('read', 'idem', 'semi'):
164 return True
165 # If we have gotten this far, 'stream' cannot be retried, because we
166 # already consumed the local stream.
167 if request_type in ('semivfs', 'mutate', 'stream'):
168 return False
169 trace.mutter('Unknown request type: %s for method %s'
170 % (request_type, self.method))
171 return False
172
152 def _run_call_hooks(self):173 def _run_call_hooks(self):
153 if not _SmartClient.hooks['call']:174 if not _SmartClient.hooks['call']:
154 return175 return
@@ -164,8 +185,21 @@
164 where the code will be to retry requests if the connection is closed.185 where the code will be to retry requests if the connection is closed.
165 """186 """
166 response_handler = self._send(protocol_version)187 response_handler = self._send(protocol_version)
167 response_tuple = response_handler.read_response_tuple(188 try:
168 expect_body=self.expect_response_body)189 response_tuple = response_handler.read_response_tuple(
190 expect_body=self.expect_response_body)
191 except errors.ConnectionReset, e:
192 self.client._medium.reset()
193 if not self._is_safe_to_send_twice():
194 raise
195 trace.warning('ConnectionReset reading response for %r, retrying'
196 % (self.method,))
197 trace.log_exception_quietly()
198 encoder, response_handler = self._construct_protocol(
199 protocol_version)
200 self._send_no_retry(encoder)
201 response_tuple = response_handler.read_response_tuple(
202 expect_body=self.expect_response_body)
169 return (response_tuple, response_handler)203 return (response_tuple, response_handler)
170204
171 def _call_determining_protocol_version(self):205 def _call_determining_protocol_version(self):
172206
=== modified file 'bzrlib/tests/test_smart_transport.py'
--- bzrlib/tests/test_smart_transport.py 2011-10-10 13:23:11 +0000
+++ bzrlib/tests/test_smart_transport.py 2011-10-10 13:23:11 +0000
@@ -3382,10 +3382,10 @@
33823382
3383class Test_SmartClientRequest(tests.TestCase):3383class Test_SmartClientRequest(tests.TestCase):
33843384
3385 def make_client_with_failing_medium(self, fail_at_write=True):3385 def make_client_with_failing_medium(self, fail_at_write=True, response=''):
3386 response = StringIO()3386 response_io = StringIO(response)
3387 output = StringIO()3387 output = StringIO()
3388 vendor = FirstRejectedStringIOSSHVendor(response, output,3388 vendor = FirstRejectedStringIOSSHVendor(response_io, output,
3389 fail_at_write=fail_at_write)3389 fail_at_write=fail_at_write)
3390 client_medium = medium.SmartSSHClientMedium(3390 client_medium = medium.SmartSSHClientMedium(
3391 'a host', 'a port', 'a user', 'a pass', 'base', vendor,3391 'a host', 'a port', 'a user', 'a pass', 'base', vendor,
@@ -3393,6 +3393,41 @@
3393 smart_client = client._SmartClient(client_medium, headers={})3393 smart_client = client._SmartClient(client_medium, headers={})
3394 return output, vendor, smart_client3394 return output, vendor, smart_client
33953395
3396 def make_response(self, args, body=None, body_stream=None):
3397 response_io = StringIO()
3398 response = _mod_request.SuccessfulSmartServerResponse(args, body=body,
3399 body_stream=body_stream)
3400 responder = protocol.ProtocolThreeResponder(response_io.write)
3401 responder.send_response(response)
3402 return response_io.getvalue()
3403
3404 def test__call_doesnt_retry_append(self):
3405 response = self.make_response(('appended', '8'))
3406 output, vendor, smart_client = self.make_client_with_failing_medium(
3407 fail_at_write=False, response=response)
3408 smart_request = client._SmartClientRequest(smart_client, 'append',
3409 ('foo', ''), body='content\n')
3410 self.assertRaises(errors.ConnectionReset, smart_request._call, 3)
3411
3412 def test__call_retries_get_bytes(self):
3413 response = self.make_response(('ok',), 'content\n')
3414 output, vendor, smart_client = self.make_client_with_failing_medium(
3415 fail_at_write=False, response=response)
3416 smart_request = client._SmartClientRequest(smart_client, 'get',
3417 ('foo',))
3418 response, response_handler = smart_request._call(3)
3419 self.assertEqual(('ok',), response)
3420 self.assertEqual('content\n', response_handler.read_body_bytes())
3421
3422 def test__call_noretry_get_bytes(self):
3423 debug.debug_flags.add('noretry')
3424 response = self.make_response(('ok',), 'content\n')
3425 output, vendor, smart_client = self.make_client_with_failing_medium(
3426 fail_at_write=False, response=response)
3427 smart_request = client._SmartClientRequest(smart_client, 'get',
3428 ('foo',))
3429 self.assertRaises(errors.ConnectionReset, smart_request._call, 3)
3430
3396 def test__send_no_retry_pipes(self):3431 def test__send_no_retry_pipes(self):
3397 client_read, server_write = create_file_pipes()3432 client_read, server_write = create_file_pipes()
3398 server_read, client_write = create_file_pipes()3433 server_read, client_write = create_file_pipes()

Subscribers

People subscribed via source and target branches