Remove Ping from api.Connection interface

[mjs]

As per pre-existing comments, there should only be one way to check whether an API connection is alive. Given that connections now do a great job of checking themselves - the results of which are exposed via
Broken - the Ping method was removed.

This had some bigger-than-expected fallout in the agent code but this can be cleaned up by making a similar change to State (later).

[wallyworld]

Don't you dare land this with an interface called "Brokener" :-)

[wallyworld]

nooooooooooooooooooooooooooooooooooooooo :-( :-( :-( :-(

There has to be a better name

[mjs]

$$merge$$

[jujubot]

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

[rogpeppe]

I have mixed feelings about this. On the one hand, it's good to have a single source of "this connection is broken" information. On the other, this seems racy (see comment) and it's useful to have Ping in tests (using pingConn in apiserver seems like a hack, and some of our external code was also using Ping in tests).

[rogpeppe]

Isn't this racy? When we get an error, we want to know if that error was fatal. Before this,
we were sure to be able to tell if the connection is actually still active because we made a Ping
call. By making this change, we are relying on the supposition that if we get an error from
an RPC request and the connection is broken a receive from conn.Broken() immediately
afterwards will succeed.

However I can easily see a scenario where that's not the case - if the act of making a request
triggers the network layer to notice that the connection is down, the api.monitor.run method
may take a while to react and the Broken channel will not be closed by the time we
get around to calling ConnectionIsDead.

I think it would be preferable to have an IsBroken method on Conn (as well as Broken) so
that it has the opportunity to synchronously ask (for example) the rpc.Conn whether it thinks it has broken.

[mjs]

Ok, fair enough. How about I add IsBroken as suggested and have it check the Broken channel first, and if that succeeds it then tries a ping to be sure and avoid the race?

I disagree that pingConn is a hack. Ping is just an API call like any other. Why should it get a special method on api.Connection? That said, given that this broken your code, there's a risk that it'll break other places too. I might make it exported again given where we are with the 2.0 release.