Bazaar

Merge lp://qastaging/~jameinel/bzr/2.5-client-reconnect-819604 into lp://qastaging/bzr

2.5-client-reconnect-819604
Merge into bzr.dev

Proposed by John A Meinel on 2011-10-10

Status:

Superseded

Proposed branch:

lp://qastaging/~jameinel/bzr/2.5-client-reconnect-819604

Merge into:

lp://qastaging/bzr

Diff against target:

1324 lines (+799/-199)

13 files modified

bzrlib/help_topics/en/debug-flags.txt (+2/-0)
bzrlib/smart/client.py (+208/-86)
bzrlib/smart/medium.py (+20/-5)
bzrlib/smart/protocol.py (+5/-3)
bzrlib/smart/request.py (+145/-100)
bzrlib/tests/test_smart.py (+5/-2)
bzrlib/tests/test_smart_request.py (+10/-0)
bzrlib/tests/test_smart_transport.py (+376/-0)
doc/en/release-notes/bzr-2.1.txt (+5/-0)
doc/en/release-notes/bzr-2.2.txt (+5/-0)
doc/en/release-notes/bzr-2.3.txt (+5/-0)
doc/en/release-notes/bzr-2.4.txt (+8/-3)
doc/en/release-notes/bzr-2.5.txt (+5/-0)

To merge this branch:

bzr merge lp://qastaging/~jameinel/bzr/2.5-client-reconnect-819604

High

Fix Released

Link a bug report

Reviewer	Date Requested	Status
Jelmer Vernooij (community)		Approve on 2011-11-25
Martin Pool	2011-10-10	Approve on 2011-11-25
Review via email: mp+78856@code.qastaging.launchpad.net

This proposal has been superseded by a proposal from 2011-11-25.

Commit message

Bug #819604, allow clients to reconnect if they get ConnectionReset while trying to send an RPC.

Description of the change

And finally, this is a merge-up of all the previous versions of my client-reconnect patch, targetting bzr.dev.

The main conflicts were just the test_smart_transport.py which had some refactorings because of my other work in the area. Otherwise, pretty much the same as the 2.4 patch.

Revision history for this message

Martin Pool (mbp) wrote on 2011-10-13:

If this is just a merge-up, feel free to just land it. Otherwise I'll
read it later.

Revision history for this message

John A Meinel (jameinel) wrote on 2011-10-13:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 10/13/2011 7:07 AM, Martin Pool wrote:
> If this is just a merge-up, feel free to just land it. Otherwise
> I'll read it later.
>

This is a merge up (+ resolving conflicts, etc), but the 2.1 code
still needs to be approved.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk6WuOIACgkQJdeBCYSNAAPN/gCfVVd1FRdM7GDecRGo4jda8k0K
7M8An39FrO40evTwnSn9boZ4jqxXZhhb
=ncmU
-----END PGP SIGNATURE-----

Revision history for this message

Martin Pool (mbp) wrote on 2011-11-25:

I think we should land this feature and give it a good shake out on 2.5 first, before even thinking about putting it back into previous series, partly because it's just a big patch, partly because so many different cases can come up in handling disconnects. So partly because of that, and to see everything that would land, and to avoid reviewing the backport shims right now.

I really like the split out of the request object; that is pretty clear.

[fix] Obviously if we land this only in trunk first, we have to remove those lines from the old news.

I am glad you added a flag to turn it off.

I think this is pretty clear and I don't see any problems. I don't see any unaddressed problems in the other reviews (are there?) I wouldn't be surprised if we see some problems on the real network but the only way to tell is to bring it in.

Does anyone else have objections or see problems?

review: Approve

Revision history for this message

Jelmer Vernooij (jelmer) wrote on 2011-11-25:

mgz and I just discussed this independently as well, before reading poolie's review and came to the same conclusion.

Let's land this as early in the cycle as we can so we can shake out bugs.

Independently, we should look into the issues (spurious failures, etc) with some of the smart server tests, some of which have become more frequent after the prerequisites of this branch landed.

We're a bit worried about this feature hiding issues during tests, e.g. with servers dropping connections after requests. One way to work around that would be to add tests that make sure that commands only open X connections. This would also catch some other bugs, such as command implementations not using possible_transports.

review: Approve

Revision history for this message

Jelmer Vernooij (jelmer) wrote on 2011-11-25:

sent to pqm by email