[MERGE][1.2][#185394] Disconnect and reconnect the smart medium after getting "unknown method" error from server if a request body was sent.
Andrew Bennetts
andrew at canonical.com
Wed Feb 6 06:03:16 GMT 2008
This changes RemoteRepository.get_parent_map and
RemoteRepository.stream_revisions_chunked to disconnect the medium (so it will
reconnect on the next request) if the server says the method is unknown. This
fixes bug #185394, which results in errors like “Generic bzr smart protocol
error: bad request '232'” rather than successfully falling back to slower
methods.
I've hard-coded the logic into those two methods. I could perhaps have done
something more automatic where the client/protocol/medium tracked if the request
had a body and took care of it at that layer, but it would have been more
complicated, and I'm expecting the new protocol version will be ready for 1.3,
which should avoid this problem. So I don't expect we'll need to worry about
this for any other methods. If I'm wrong we can always refactor later.
The medium now has a new instance variable, _remote_is_at_least_1_2, which
starts as True until an error proving otherwise occurs. This allows the code to
avoid disconnecting more than once, which would be excessively annoying.
I've also added a note to the CHANGES section of the NEWS file about the
reconnection behaviour, advising people to upgrade the server if they want to
avoid it.
-Andrew.
-------------- next part --------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: andrew.bennetts at canonical.com-20080206060156-\
# 3im8420jsztowpe5
# target_branch: http://bazaar-vcs.org/bzr/bzr.dev
# testament_sha1: 4ea749abd512b4daf1b3d9838c74da9db2a37446
# timestamp: 2008-02-06 17:02:05 +1100
# source_branch: http://people.ubuntu.com/~andrew/bzr/request-body-\
# recovery
# base_revision_id: pqm at pqm.ubuntu.com-20080205071430-7b9vl83ebnsd6i0g
#
# Begin patch
=== modified file 'NEWS'
--- NEWS 2008-02-05 07:14:30 +0000
+++ NEWS 2008-02-06 06:01:56 +0000
@@ -12,6 +12,11 @@
* Fetching via bzr+ssh will no longer fill ghosts by default (this is
consistent with pack-0.92 fetching over SFTP). (Robert Collins)
+ * Fetching via the smart protocol may need to reconnect once during a fetch
+ if the remote server is running Bazaar 1.1 or earlier, because the client
+ attempts to use more efficient requests that confuse older servers. This
+ won't happen if the server is upgraded to Bazaar 1.2. (Andrew Bennetts)
+
* Formatting of ``bzr plugins`` output is changed to be more human-
friendly. Full path of plugins locations will be shown only with
``--verbose`` command-line option. (Alexander Belchenko)
=== modified file 'bzrlib/remote.py'
--- bzrlib/remote.py 2008-01-29 03:22:52 +0000
+++ bzrlib/remote.py 2008-02-06 05:45:11 +0000
@@ -764,14 +764,19 @@
"""See bzrlib.Graph.get_parent_map()."""
# Hack to build up the caching logic.
ancestry = self._parents_map
- missing_revisions = set(key for key in keys if key not in ancestry)
+ if ancestry is None:
+ # Repository is not locked, so there's no cache.
+ missing_revisions = set(keys)
+ ancestry = {}
+ else:
+ missing_revisions = set(key for key in keys if key not in ancestry)
if missing_revisions:
parent_map = self._get_parent_map(missing_revisions)
if 'hpss' in debug.debug_flags:
mutter('retransmitted revisions: %d of %d',
- len(set(self._parents_map).intersection(parent_map)),
+ len(set(ancestry).intersection(parent_map)),
len(parent_map))
- self._parents_map.update(parent_map)
+ ancestry.update(parent_map)
return dict((k, ancestry[k]) for k in keys if k in ancestry)
def _response_is_unknown_method(self, response, verb):
@@ -794,6 +799,13 @@
def _get_parent_map(self, keys):
"""Helper for get_parent_map that performs the RPC."""
+ medium = self._client.get_smart_medium()
+ if not medium._remote_is_at_least_1_2:
+ # We already found out that the server can't understand
+ # Repository.get_parent_map requests, so just fetch the whole
+ # graph.
+ return self.get_revision_graph()
+
keys = set(keys)
if NULL_REVISION in keys:
keys.discard(NULL_REVISION)
@@ -809,12 +821,15 @@
response = self._client.call_expecting_body(
verb, path, *keys)
if self._response_is_unknown_method(response, verb):
- # Server that does not support this method, get the whole graph.
- response = self._client.call_expecting_body(
- 'Repository.get_revision_graph', path, '')
- if response[0][0] not in ['ok', 'nosuchrevision']:
- reponse[1].cancel_read_body()
- raise errors.UnexpectedSmartServerResponse(response[0])
+ # Server does not support this method, so get the whole graph.
+ # Worse, we have to force a disconnection, because the server now
+ # doesn't realise it has a body on the wire to consume, so the
+ # only way to recover is to abandon the connection.
+ medium.disconnect()
+ # To avoid having to disconnect repeatedly, we keep track of the
+ # fact the server doesn't understand remote methods added in 1.2.
+ medium._remote_is_at_least_1_2 = False
+ return self.get_revision_graph()
elif response[0][0] not in ['ok']:
reponse[1].cancel_read_body()
raise errors.UnexpectedSmartServerResponse(response[0])
@@ -970,6 +985,10 @@
return self._real_repository.has_signature_for_revision_id(revision_id)
def get_data_stream_for_search(self, search):
+ medium = self._client.get_smart_medium()
+ if not medium._remote_is_at_least_1_2:
+ self._ensure_real()
+ return self._real_repository.get_data_stream_for_search(search)
REQUEST_NAME = 'Repository.stream_revisions_chunked'
path = self.bzrdir._path_for_remote_call(self._client)
recipe = search.get_recipe()
@@ -980,19 +999,24 @@
response, protocol = self._client.call_with_body_bytes_expecting_body(
REQUEST_NAME, (path,), body)
+ if self._response_is_unknown_method((response, protocol), REQUEST_NAME):
+ # Server does not support this method, so fall back to VFS.
+ # Worse, we have to force a disconnection, because the server now
+ # doesn't realise it has a body on the wire to consume, so the
+ # only way to recover is to abandon the connection.
+ medium.disconnect()
+ # To avoid having to disconnect repeatedly, we keep track of the
+ # fact the server doesn't understand this remote method.
+ medium._remote_is_at_least_1_2 = False
+ self._ensure_real()
+ return self._real_repository.get_data_stream_for_search(search)
+
if response == ('ok',):
return self._deserialise_stream(protocol)
if response == ('NoSuchRevision', ):
# We cannot easily identify the revision that is missing in this
# situation without doing much more network IO. For now, bail.
raise NoSuchRevision(self, "unknown")
- elif (response == ('error', "Generic bzr smart protocol error: "
- "bad request '%s'" % REQUEST_NAME) or
- response == ('error', "Generic bzr smart protocol error: "
- "bad request u'%s'" % REQUEST_NAME)):
- protocol.cancel_read_body()
- self._ensure_real()
- return self._real_repository.get_data_stream_for_search(search)
else:
raise errors.UnexpectedSmartServerResponse(response)
=== modified file 'bzrlib/smart/medium.py'
--- bzrlib/smart/medium.py 2008-01-22 06:56:59 +0000
+++ bzrlib/smart/medium.py 2008-02-06 03:52:25 +0000
@@ -386,6 +386,10 @@
def __init__(self):
self._current_request = None
+ # Be optimistic: we assume the remote end can accept new remote
+ # requests until we get an error saying otherwise. (1.2 adds some
+ # requests that send bodies, which confuses older servers.)
+ self._remote_is_at_least_1_2 = True
def accept_bytes(self, bytes):
self._accept_bytes(bytes)
=== modified file 'bzrlib/tests/test_remote.py'
--- bzrlib/tests/test_remote.py 2008-01-22 00:27:42 +0000
+++ bzrlib/tests/test_remote.py 2008-02-06 05:38:38 +0000
@@ -139,7 +139,7 @@
self.responses = responses
self._calls = []
self.expecting_body = False
- _SmartClient.__init__(self, FakeMedium(fake_medium_base))
+ _SmartClient.__init__(self, FakeMedium(fake_medium_base, self._calls))
def call(self, method, *args):
self._calls.append(('call', method, args))
@@ -161,8 +161,20 @@
class FakeMedium(object):
- def __init__(self, base):
+ def __init__(self, base, client_calls):
self.base = base
+ self.connection = FakeConnection(client_calls)
+ self._client_calls = client_calls
+
+
+class FakeConnection(object):
+
+ def __init__(self, client_calls):
+ self._remote_is_at_least_1_2 = True
+ self._client_calls = client_calls
+
+ def disconnect(self):
+ self._client_calls.append(('disconnect medium',))
class TestVfsHas(tests.TestCase):
@@ -642,6 +654,27 @@
client._calls)
repo.unlock()
+ def test_get_parent_map_reconnects_if_unknown_method(self):
+ error_msg = (
+ "Generic bzr smart protocol error: "
+ "bad request 'Repository.get_parent_map'")
+ responses = [
+ (('error', error_msg), ''),
+ (('ok',), '')]
+ transport_path = 'quack'
+ repo, client = self.setup_fake_client_and_repository(
+ responses, transport_path)
+ rev_id = 'revision-id'
+ parents = repo.get_parent_map([rev_id])
+ self.assertEqual(
+ [('call_expecting_body', 'Repository.get_parent_map',
+ ('quack/', rev_id)),
+ ('disconnect medium',),
+ ('call_expecting_body', 'Repository.get_revision_graph',
+ ('quack/', ''))],
+ client._calls)
+
+
class TestRepositoryGetRevisionGraph(TestRemoteRepository):
# Begin bundle
IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWQj/WIwADDr/gHRwBABZ////
f///+r////pgE9du69s+gb311verY+7PmtVomWr01Euu3u+B72W97rfeO++5t95W3dc6LWPoDQ+t
dCgG+74SiQBMjRoqeeqfpqZpNqbVP1HqptqepP0U81PUnqeaTNU8p6gD0hoJJIBk0TBMgJqaE9CM
nqZPU2kDIBoPUaNNNABKCaaEJkBNI0TSejSab1GgTQADADRAYEwkRJNTAmEjTaNTxNCZAHqaaA00
AGgAAARSoyan6p6KZtRP1Gap4gmGkaaYEAaABoxAaABUkgIAjQAkZMVP9TNTaMpoZEjNQ9PVP1QD
RkG1Gp0KXIwATJIBy1I5vt2Bv28pBScuEnHl3Arrcq/3G5m6yMrsTSczpD+Vv3HK704CIjVIeBbu
KxS4FVusVf588D0g59cMdwixP53VcR8aYXnvDkzuveajHxZ5voX0N9GVrvIXWIGkpAV8foy+rl4A
97p8fzfB6gJmiCoJIKHUHfTlTtOeymWJJlPRSgmeAjiwkdulgbdCDD2orrNvGlVuGjTi9L3SQekJ
VCyTFPUiE/12CIgjJVCxw64WOxBW0GeHVd1lMa6KwEREJECZgZkzDMl7PWhF2ev2BbSDwhUNVXpD
SWkJK/DhKJRNGC7tRWWxDC2VaD2aw5eCI5dtrFhazFRrYm+MVoPdiIYhZyaVdTFmLKfq8P/NiA6h
xcLP1x7/v7o62PZZ8Og/absbHzGWiYn+RLTy00sxkb3kwe0a1OVOUcZjMKSeqbg3ntE+WtPLadRS
UM+2CxmppmVwbxSVU9GNc6mGvcsifo8D1FD/3mCiFY3NiUY4GnNe93WLXwCm9PtXBAo2eVt6hklU
xhBRFo9HWRUpY3FaVI3FPW7a63yEscVNcoY9LK92k6vD6HDrnoib1FUVQXLSmRxIUbBogVHHBnqi
TRNLbBgRIsvgmpsYzMNSEVhd/ZqRJZkprAbErIFgxLAomuNWr/Jitp070hCo3aPRFGXDdyLwmdbd
DcfaHtCjKD5fEsibVhxzvtrlULt1S6xfW1XF2qqbFyao7usJKuquvEGzmCsORoQ2kLkgQM+0Jgld
42kuozk7pipkA++nUIqi5KxGZoOvseVTWyTALrhdD1zIzE3u1YZx9y52Lx9wR0K1Xzm/N6d4MWtP
dG1waU5OVrmhkx3VbY6zFewQHKnjBB+P8ekWjydJVH0UUtQ9GwcO1SxrTDgvkERBhIovOAgwgtsF
r/pjc0Ue6eLoakVTzrUiT7RzTxeBO+L2J5ZFrR49A7gdIgiFI+eURXPYS3PNJml0KiNrggx4Gzdv
36LOpHBSlXV8JEnoT+cVES5gyjixIUnNz68UUsIYnZFHSeWzh4kjlDoDO6O+twOc5REERGDT2vag
Hn2ZCZymTIufGFAGUYIqoG2l1Yl7GqoiqiSbGKyLsmCs11wrFeslIk8/fkZspizMr7nCYsLGKF6N
DLHGK2ruxZZokvjW0SJTQwWXC+Il7CJc0k1BPbTNnE8R3y4wZshVlR5ldQuvW4aScpUvJZpEltsI
F5GoeXFhVcnVHSTH0aikcCMAPRcJ26TH5G3L2nStUhhbfdJkoX104xyc3OTOsfuqfqUupVfvk1rn
MjJcv6NxNuPVOZlMc9Oz4GYr4hs8737t4tH7r5YAumDMWqIMmqFANjFKgVFbYlkqigly5ykpTcMJ
mLQVoo5n4TybjzxjRR5tQT6hJyR3fMKg3KajE/SXXYIdvIlsj5JLdYpXYzB2l3VTQOZTKmRn0vEi
1dSlUAQYQ0DI6mxEmNZDZtYrOidW3qdOjWw3SHVfKVs7ULg0kwaBkLanUuCDHJtxXtREmiEGxlch
CxDRvxzuwcyxHf/9zauCzia23i1obmWx5nzNKHJU9gQYbAL1HFQ4uZHXzunUtI4y6imQ+BxS4tgV
M2b2oBKI+W8seSHaJK81BJcImSSgVmp+7YKPwMw3FwU5cTsKsEmUm83m8g3ocCpyNDxAe3ftSnA7
6dHtDaTqZPK7m62WS8MdQXeSSVSyIYzbKh2ZNcgucALxRBW3NBAx1MorBxORdWCmafL10qQNTETZ
toJDUAyiRSLLSM5oqzrsJi85ApzMzGtTjcUnVpz2bL1ndsuaWZ2u9m0SzadZoZo33YxcqTXGlz4k
6NPVk9CMNV+7NDXbDKScvJvU4uVgTPdOHGi9rRg+GA0NG3JkxQZDyyi2gkyVM24xKbDA+A9Xg9vX
kZlQ8ydWkrh0mBBpU6HYU2OCj8v5bp6Jz3P5Q1pSySBUUUHDM2AkTIALGRwEuuUW6UuK4BHH+WjU
ZOKKZXtdBdr0oK0LA5bkjErHPB4gT4NOZBJzJLjHMy2KjJmgvFC8iNALoa2LFeOG/qYHhbmUkzAE
0mLKJ0GYegxfr0YqIcmMxAwTQmzyZZOOXFSBRSLaDQp/GdJq7DF1zFngNzJJ6GprSZLmr8kQd+BQ
PKjiQImtF5hAcF5QPO94cMJ8DEowc92Mj6GuJREVKpIogiSKpJjgTS0jDV4HAzLE5SQi5JcyPEBI
QRMWjUjEsY5SVmBYPIkwxQE0CocYgQ+/juDEhlYZDklaOLS4kKDypLgIHXqhGDJzrRwStumUZCsc
bCEiyLNZa4OA6J79iFfyeRVzMFd+6y0LQYNO1L3ID9lzXj46musD9xoOWpyLGh1QTJ+Arht0OKMZ
mg5wNtoDgbDDExEsN5AlMbEd4F8onte5u1Osw+YIBJf0RJGSklHIN9qkdEhaOiRrIw8g7T9g9k2M
5IXYDIKvJ4/k5JB84Zl7CaIDk5GYOiZpkxpsbKEohQRA0xMZcDMGhIvhrhIxLF1GZRcXxaRJ6416
P4nq/Z7T8MfHq/zh4bDic4PLa3aqxhgSBYgg5wNK+Fhjgeq9taPFqkrcorF4Yh5PR1BhOHqjPbkH
5gwcOWkQ8oGwJ6Qe+DoedeL1sao59oOfQyomypQgCgD1BKgbBP90PhuoCz3ddAOBLzA8CZQODZgy
qlltJ5XPbb/1rwYIcQaXG5wPHABQCg97IV2NhXwu9vyj+cz4Jjc1g+CaX+6gL/B069QaZQS5pm5W
jyFXeHCcGgPw8XgC+gqZPh04b9dQHRFDCnUptxBJq7enJSFIDtnOyqI3O6g7Hvk8GAOfihSHZJNE
915oYPTZ38y5w6hMImL9mgg71GbhPdMNA420zI1AziOSwU5A4iuEiVENQmTZvBKQqBWD2Lhc0tGB
elycWpa41tP6aV8naVD7xzpZfQ27u8sNKNRIgwVESobPmm00HMQanGxSy6eAxsgLezCSNnQiSj8A
5cRueSiYlNKcIxvC+Nu5CIgL227qGJHxM2zQ0SJKOu9cTViweUnIMMHwQcEWwqDRiPoHKY1Z8ooU
oamUQSDUyESw27bapUWRtgQOts6q3wsESLgzl47oT7eZGkVNKGA1C4qUpPbrB9q8zwgQ7hw+pXBv
7fFc/fLr/NteoxU+tkucq52u1j9Plvdq/rbGTNob1zGJQR+B3jMs7Q7m6g/IfNPfqpXOkJVmR2XI
mCJWGKkNgSNnClm5Inb2gI9EA4880SsuFhNUYCsxKJqPZVGLF7dpQuMWJKpcyXBr3FEQRP8HUQof
tISuLKapcLNOIu5x4zf5hjl2qI6mBlQSE10mZQway7GA6uJQMyQRUs4AEpbxhaHI4JIY0r/GChFs
aQb3N1eO/tSoLBBXSCSzVN0hqngdRxO7jtfobSd5vVXIxnJbQWSK0g7nRKA8vBmQE8PzY3TjOeol
vB8dZhMSzrYhpGRhkJicsBOIkSPW8j0KnTB9CFLSanaigM9/wth/ZvuHcYdy1uks3DZMeminp11p
VNlYySLD0wiWKbLKzvFERVRVDKOUlTUTY23zi8ZTb2shzPZVWOJgqTSTnKPGeAssiGy5xxrLdahY
xzjAvUxkJUYC1wzKXGu+cNzQ3vI5Wx5+r8SxU7Ccg6ZvqOwqpVVvPNHcK7WNbg3ZAk2QxQhMG1QO
IxGE0Ezy2YXe8OxhawOZnjCiyNlA/VtInq2C9LCNZt5GCcR4+cRWWBgDW6XOENVekL0UCFZjsmKp
OPQjakLvnA1pCNIuUGVoFzyAxQwl+0pODUQUM7t3Byu0r6Wyaoo98HhTkIG9IwmYIUA7pTG1Fjcc
B7tcoqRY44uT+pnONZJ1z88pLwAJ5IjjhLZCi4CkUxpQEsLnRAQELeGI1xICWn2GYFKSsylegG6a
iArIRwgFaICyznLsuuQsHl5Yb/EYEqR73hOx6cXFqRbR+qXmSoAxGRhKKAPewk94DpmUQlVm/izw
CFWGvdvZluBNGUScT4qo4Unk/LzdZ34mTe/mdzvJ3+ys6VKLUWdR2XWRNEwij12Ev9n0XObzfjj1
4GfZaM7SFzmVqEOfKDuDdVZZU9SuRWYPHpae2xn9UiTz3d1x9H1tdqKRM3Ckm9pH3+RU+VmWEjnM
dcR7RaGKV9JHDdv+7g4ZTU2H1+H6ngazaeGhoKkqKVCPK1YVKqo1ScIczDainI3CoJIWwoQN+uZZ
TI7ub7eCZOPlyawGcU6ghSpR75c8fVf9N4OAmsSGMpVFjnBDHGkxR2TjxribUKLj4xDa01DVSuRQ
Ve/NClYgW+P8wKPXMnxoltEHgPt6btiKZ+CHxLx0vS8fw1mtxlRRJKh90fw5BaOeL9zlHRP3U0w4
fjlrqZk/Bg0P8VlP39lX4nlvPecRgzixmrEf0zQjwiCYDciDezg932O/NMF2hXB1s/EHQ5EKsYVE
+fOjK6Sd1BoUbiftLaYdFaZa0tmRa4p7YqRJhHlIzy+K4C5ngqeJQtIksX3Hn0zBIzAtlokSUjum
Dj98WMFMohozsS1RJaPkXB4eEfubbHPSvClpUpZf9vFfOdNyt9h3ScF3S/Td7pgzlgNtRfJmwddx
Et9X5mLjkSXrE8ZUYuBZtkMHItFOIinpqJ/g3maYRFQ1DFBzMMmV+YpWhJAWFhCaAnTdDS6eLHRd
qgdpMRXf7D2H2SF17RskfQOJ+TqdSGO5OpD88nUZSlWhgxOr/SPDecZxwqOSdendx/LuQ3XdL5/Y
h7/E+ckyRz+ueXaToVG6iqWss9VLVcVPNJOknxwvPD+HvvOd77EdpcuT70XvNsLd63QWnfDKWhqG
YwgIRoUGdQoEWxCKHA5VfMpO66/4RV//naeT5C+TWZliYFnuA2lRbq6MjfrRmTR4aNcQ3kh1hClc
whCAkQ0tSywqkfBvtJNN5VKlSVRYo93W7MAhxvfIi5x9atE6rcLMVgYVVXyYdbKjEmwrxfvvLCip
EmSUrWNtoaozWcI231qX4X8byMKTOdk8iyyqlT6VPdstI0bKzue32txnI8ptkYljS3xNxUDohjJ+
nCQzb73IXDxscTasK0qtxqkslSTWX7iNC+RtzYSGPbAWl2qw6XDw4NjvFO3c9Soxi4oSlXo05TDK
Xr5TSwusZxb1+zrQ7CjUM3u0JuNMNK6yYpQxXpS9LH2pKYLiWPsRui+pJgKSzojPl9Cul6lLIbhk
skmdGqS2vYNOA5v6+VJ5LwzmZGRxwodo4sX1Viwoqry1pqq6npvF0iTVsukN2o7d4F8VeUasUVnH
q6XOTn0HqOUx9Mb/01sJHDreKGocIPu5pydPJHpa7aY5d92ZleWb7Rfr0yvQydBOUp9eRl0E2fHq
l7MmjelKFH5BlOXBU9yRdDkEmMoOSRcEA5f7r9Hr5cGcLf151rsYD4+Vwdl+fnETezIBbAVjmDU5
xzI0klOA57J5hKEo44I4OgmooWQhg9cQrMPc5z8RuXU+iSAd0fdCEgVgXsf1/8XckU4UJAI/1iMA
More information about the bazaar
mailing list