state: fix i/o timeout errors

state: fix i/o timeout errors

Since mongo has been fixed to time out sockets,
(in revision 241 of mgo)
the state package was failing quite a lot.
This should make it more robust.
In particular, we use the Timeout field on
mgo.DialInfo to signify the amount of time
we are prepared to wait before connection,
not the amount of time to wait for a response,
so this change makes the latter independent
of the former.

https://code.launchpad.net/~rogpeppe/juju-core/499-state-fix-timeout/+merge/205416

(do not edit description out of merge proposal)

[rog, 2014-02-07 16:37:08]

Please take a look.

[dimitern, 2014-02-07 16:46:34]

LGTM with a couple of suggestions.

https://codereview.appspot.com/61010043/diff/1/state/open.go
File state/open.go (right):

https://codereview.appspot.com/61010043/diff/1/state/open.go#newcode26
state/open.go:26: const mongoSocketTimeout = 10 * time.Second
Some doc comment describing why 10s perhaps?

https://codereview.appspot.com/61010043/diff/1/state/settings_test.go
File state/settings_test.go (right):

https://codereview.appspot.com/61010043/diff/1/state/settings_test.go#newcode38
state/settings_test.go:38: Timeout: testing.LongWait,
Won't this prolong all tests too much? Isn't ShortWait enough?

[gz, 2014-02-07 16:48:34]

LGTM, I also wonder about the LongTimeout on connection, keeping that part short
if we can otherwise set the socket timeout to something reasonable seems best.

[rog, 2014-02-07 18:39:23]

Please take a look.

https://codereview.appspot.com/61010043/diff/1/state/open.go
File state/open.go (right):

https://codereview.appspot.com/61010043/diff/1/state/open.go#newcode26
state/open.go:26: const mongoSocketTimeout = 10 * time.Second
On 2014/02/07 16:46:35, dimitern wrote:
> Some doc comment describing why 10s perhaps?

Done.

https://codereview.appspot.com/61010043/diff/1/state/settings_test.go
File state/settings_test.go (right):

https://codereview.appspot.com/61010043/diff/1/state/settings_test.go#newcode38
state/settings_test.go:38: Timeout: testing.LongWait,
On 2014/02/07 16:46:35, dimitern wrote:
> Won't this prolong all tests too much? Isn't ShortWait enough?

This should not prolong any tests at all, unless they're failing.

The rationale for LongWait is that it should be used
in cases where timing out may break the test.
ShortWait should be used when we are going to be
waiting for that length of time even on the happy path.

I think that's a clear indication that LongWait should
be used here.