Bazaar

Merge lp:~spiv/bzr/hpss-compat-528041 into lp:bzr/2.1

  1. hpss-compat-528041
  2. Merge into 2.1
Proposed by Andrew Bennetts
Status: Merged
Approved by: Vincent Ladeuil
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~spiv/bzr/hpss-compat-528041
Merge into: lp:bzr/2.1
Diff against target: 101 lines (+50/-5)
3 files modified
NEWS (+14/-0)
bzrlib/smart/medium.py (+8/-2)
bzrlib/tests/test_remote.py (+28/-3)
To merge this branch: bzr merge lp:~spiv/bzr/hpss-compat-528041
Reviewer Review Type Date Requested Status
Vincent Ladeuil Approve
Review via email: mp+24202@code.launchpad.net

Description of the change

This fixes the AssertionError from _remember_remote_is_before when opening a branch from a 1.5 (or older) server. The right fix is to simply not raise an error there, all it does is inflict pain on users for a minor (at worst) performance bug. The longer analysis is at <https://bugs.edge.launchpad.net/bzr/+bug/528041/comments/11> if you're curious.

I'm sure Martin was hoping for this method to be gone entirely, but hopefully this smaller step will still make him at least a little be happy :)

To post a comment you must log in.
Revision history for this message
Vincent Ladeuil (vila) wrote :

Sounds good.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2010-04-30 06:27:15 +0000
3+++ NEWS 2010-04-30 08:16:29 +0000
4@@ -16,12 +16,26 @@
5 * ``bzr switch`` does not die if a ConfigurableFileMerger is used.
6 (Aaron Bentley, #559436)
7
8+* Fixed ``AssertionError`` when accessing smart servers running Bazaar
9+ versions before 1.6.
10+ (Andrew Bennetts, #528041)
11+
12 * Reset ``siginterrupt`` flag to False every time we handle a signal
13 installed with ``set_signal_handler(..., restart_syscall=True)`` (from
14 ``bzrlib.osutils``. Reduces the likelihood of "Interrupted System Call"
15 errors after two window resizes.
16 (Andrew Bennetts)
17
18+Internals
19+*********
20+
21+* ``_remember_remote_is_before`` no longer raises AssertionError when
22+ suboptimal network behaviour is noticed; instead it just mutters to the
23+ log file (and warns the user if they have set the ``hpss`` debug flag).
24+ This was causing unnecessary aborts for performance bugs that are minor
25+ at worst.
26+ (Andrew Bennetts, #528041)
27+
28
29 bzr 2.1.1
30 #########
31
32=== modified file 'bzrlib/smart/medium.py'
33--- bzrlib/smart/medium.py 2010-02-17 17:11:16 +0000
34+++ bzrlib/smart/medium.py 2010-04-30 08:16:29 +0000
35@@ -609,10 +609,16 @@
36 # which is newer than a previously supplied older-than version.
37 # This indicates that some smart verb call is not guarded
38 # appropriately (it should simply not have been tried).
39- raise AssertionError(
40+ trace.mutter(
41 "_remember_remote_is_before(%r) called, but "
42 "_remember_remote_is_before(%r) was called previously."
43- % (version_tuple, self._remote_version_is_before))
44+ , version_tuple, self._remote_version_is_before)
45+ if 'hpss' in debug.debug_flags:
46+ ui.ui_factory.show_warning(
47+ "_remember_remote_is_before(%r) called, but "
48+ "_remember_remote_is_before(%r) was called previously."
49+ % (version_tuple, self._remote_version_is_before))
50+ return
51 self._remote_version_is_before = version_tuple
52
53 def protocol_version(self):
54
55=== modified file 'bzrlib/tests/test_remote.py'
56--- bzrlib/tests/test_remote.py 2010-04-23 05:28:35 +0000
57+++ bzrlib/tests/test_remote.py 2010-04-30 08:16:29 +0000
58@@ -417,9 +417,12 @@
59 # Calling _remember_remote_is_before again with a lower value works.
60 client_medium._remember_remote_is_before((1, 5))
61 self.assertTrue(client_medium._is_remote_before((1, 5)))
62- # You cannot call _remember_remote_is_before with a larger value.
63- self.assertRaises(
64- AssertionError, client_medium._remember_remote_is_before, (1, 9))
65+ # If you call _remember_remote_is_before with a higher value it logs a
66+ # warning, and continues to remember the lower value.
67+ self.assertNotContainsRe(self.get_log(), '_remember_remote_is_before')
68+ client_medium._remember_remote_is_before((1, 9))
69+ self.assertContainsRe(self.get_log(), '_remember_remote_is_before')
70+ self.assertTrue(client_medium._is_remote_before((1, 5)))
71
72
73 class TestBzrDirCloningMetaDir(TestRemote):
74@@ -527,6 +530,28 @@
75 self.assertIsInstance(bd, RemoteBzrDir)
76 self.assertFinished(client)
77
78+ def test_backwards_compat_hpss_v2(self):
79+ client, transport = self.make_fake_client_and_transport()
80+ # Monkey-patch fake client to simulate real-world behaviour with v2
81+ # server: upon first RPC call detect the protocol version, and because
82+ # the version is 2 also do _remember_remote_is_before((1, 6)) before
83+ # continuing with the RPC.
84+ orig_check_call = client._check_call
85+ def check_call(method, args):
86+ client._medium._protocol_version = 2
87+ client._medium._remember_remote_is_before((1, 6))
88+ client._check_call = orig_check_call
89+ client._check_call(method, args)
90+ client._check_call = check_call
91+ client.add_expected_call(
92+ 'BzrDir.open_2.1', ('quack/',), 'unknown', ('BzrDir.open_2.1',))
93+ client.add_expected_call(
94+ 'BzrDir.open', ('quack/',), 'success', ('yes',))
95+ bd = RemoteBzrDir(transport, remote.RemoteBzrDirFormat(),
96+ _client=client, _force_probe=True)
97+ self.assertIsInstance(bd, RemoteBzrDir)
98+ self.assertFinished(client)
99+
100
101 class TestBzrDirOpenBranch(TestRemote):
102

Subscribers

People subscribed via source and target branches