api: add DialTimeout

[axw]

Description of change

Add DialTimeout to api.DialOpts, and use
it in worker/apicaller instead of Timeout.
This will cause the workers to not timeout
in case of server-side rate-limiting.

QA steps

1. juju bootstrap localhost
2. juju enable-ha

Modify apiserver/admin.go to add a Sleep(2*time.Second) before responding to Login. This emulates a highly loaded apiserver which is rate-limiting clients.

3. juju upgrade-juju -m controller --build-agent
4. juju debug-log -m controller
   (observe no errors; there were "context deadline exceeded" errors without the DialTimeout changes)

Documentation changes

None.

Bug reference

Fixes https://bugs.launchpad.net/juju/+bug/1701438

[wallyworld]

Please ensure --race passes

[axw]

Please ensure --race passes

Yup, have done.

[axw]

$$merge$$

[jujubot]

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

[jameinel]

This is 2 cases of bare channel sends without an associated select + a tomb so that we can close the object.
Is this sane to be doing?
(I realize the original code was doing a bare receive as well.)
But its often a big red-flag for potential deadlocks if there are any bugs in other code.

[axw]

It's in a test, which controls both the sender and the receiver. IMO this is fine.

[jameinel]

ah, was confused about the types. IMO, I'd still like to see these turned into: select { case <-time.After(testing.LongWait): c.Fatal(blah) case <-a.r.unblock(): } That way when poking at the test, or if we have something go wrong you don't end up with a deadlock and the giant panic after 10min from the test runner.

…

On Tue, Jul 11, 2017 at 1:02 PM, Andrew Wilkins ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In api/apiclient_test.go <#7617 (comment)>: > @@ -1125,6 +1240,10 @@ type loginTimeoutAPIAdmin struct { } func (a *loginTimeoutAPIAdmin) Login(req params.LoginRequest) (params.LoginResult, error) { - <-a.r.unblock + ch, ok := <-a.r.unblock It's in a test, which controls both the sender and the receiver. IMO this is fine. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#7617 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAMYfcWxBKs9c5RffZkEdwnoww7n2Fatks5sMzotgaJpZM4OTq7U> .