Gabble master queries now 5 proxies at the same time. I don't close the bug now as we could still improve this by sending queries only when we are going to need them.

Changing severity and retitling.

*** Bug 25279 has been marked as a duplicate of this bug. ***

http://git.collabora.co.uk/?p=user/cassidy/telepathy-gabble;a=shortlog;h=refs/heads/proxy

This branch fixes 2 things:

A)  Remove all the hardcoded proxies and use proxy.telepathy.im instead (our future DNS round robin proxy).

B) Stop querying fallback proxies when connected. Instead query them when:
- requesting a FT channel
- requesting a 1-1 D-Bus tube channel
- receiving an incoming Stream tube channel
Proxy discovered on the connection (so provided by your server) are still queried right away.

There are still room for improvements:

C) gabble_bytestream_socks5_initiate should become async, so we'll be able to wait until we receive replies from server if we don't have any proxy yet.

D) the bytestream factory could query more than once the same server. That will give us a chance to get different proxies from the DNS round robin server.

I think we could consider merging this branch as it as that's already a big improvement comparing to the current situation.
Current code could lead to some "regressions" (if trying to send the SOCKS5 stanza before receiving the proxy replies) but in the worst case we should fallback to IBB (assuming the other side is running Gabble as well).

Furthemore, the SOCKS stanza is not sent before we receive the reply from the SI request from the other peer (involving a network round trip and user UI interactions) so that shouldn't be too bad.

http://git.collabora.co.uk/?p=user/cassidy/telepathy-gabble;a=commitdiff;h=2775d305c67fb

I'd prefer the style "fd.o#21151: Don't query SOCKS5 proxies when we are connected" in future. Otherwise, review+ from me.

Does it backport readily to 0.8? We need this on both branches.

review+ from me too, but two queries before we merge given that we obviously end up querying the same streamhost repeatedly:
1. What happens when this has happened >5 times? Do we discard the oldest 6th, or do we end up with an ever-growing list of streamhosts?
2. In the likely case our server has remained connected to the same proxy, do we remove duplicates from the answers, or do we possibly end up sending the same stream host 5 times in a S5B initiation?

(In reply to comment #5)
> http://git.collabora.co.uk/?p=user/cassidy/telepathy-gabble;a=commitdiff;h=2775d305c67fb
> 
> I'd prefer the style "fd.o#21151: Don't query SOCKS5 proxies when we are
> connected" in future. Otherwise, review+ from me.

I rebased on top of the new master and fixed the commit msg.

> Does it backport readily to 0.8? We need this on both branches.

Didn't check yet but hopefully that should be fine.

 (In reply to comment #6)
> review+ from me too, but two queries before we merge given that we obviously
> end up querying the same streamhost repeatedly:
> 1. What happens when this has happened >5 times? Do we discard the oldest 6th,
> or do we end up with an ever-growing list of streamhosts?
> 2. In the likely case our server has remained connected to the same proxy, do
> we remove duplicates from the answers, or do we possibly end up sending the
> same stream host 5 times in a S5B initiation?

Actually, with current code, we'll query the proxy.telepathy.im only once (proxy is removed from the socks5_potential_proxies list once we went the query).

D) will fix that by improving the bytestream factory to work better with the DNS round robin server:
- don't remove proxy from the list once the query has been sent
- be sure to not add twice the same (jid, host, port) to our list of discovered proxies
- remove old proxies from the list after we disocevered $N newer proxies

(In reply to comment #7)
> D) will fix that by improving the bytestream factory to work better with the
> DNS round robin server:
> - don't remove proxy from the list once the query has been sent
> - be sure to not add twice the same (jid, host, port) to our list of discovered
> proxies
> - remove old proxies from the list after we disocevered $N newer proxies

I'd consider that a merge blocker then - otherwise we'll be very unreliable at dealing with servers coming and going from the proxy pool. Shouldn't be too hard to fix?

I merged the patches improving the general proxy mgt (only query when needed). Closing this bug as that was the main issue. I'll open other bugs about the improved we discussed.

For the record: bug #25303 and bug #25304

(In reply to comment #9)
> I merged the patches improving the general proxy mgt (only query when needed).

These patches have been merged to master and 0.8 btw.