Signed my patch.

ping @anusha-ragunathan PTAL

gofmt errors:
--> Making bundle: validate-gofmt (in bundles/1.13.0-dev/validate-gofmt)
These files are not properly gofmt'd:

• pkg/plugins/client.go

@chenchun also

---> Making bundle: validate-lint (in bundles/1.13.0-dev/validate-lint)
Errors from golint:
pkg/plugins/client.go:20:2: const defaultHttpTimeOut should be defaultHTTPTimeOut

Please fix the above errors. You can test via "golint" and commit the result.

@anusha-ragunathan @tophj-ibm Fixed the format errors.

@chenchun : Plugin calls already have a timeout mechanism. It is not at the HTTP Client request level. It is application specific (plugin specific in this case). There's exponential backoff (2, 4, 8, 16 seconds...), with retries, with a maximum timeout of 30 seconds. You were probably waiting in a longer backoff time period (eg 16 seconds) when you experienced the issue. You should see daemon log entries such as https://github.com/docker/docker/blob/master/pkg/plugins/client.go#L135

Given that, I think this PR can be closed.

I don't keep the log. But I believe it is not the backoff retry that was happening. HTTP request timeout is a basic need if the server takes a long time to reply or even not given a reply like the test I write. In this situation the application specific retry won't work. So I don't agree with you.

Both cases you mention should be handled by the current plugin timeout mechanism.

@anusha-ragunathan It happens again and it hangs on callWithRetry for 182 minutes. It is much more than 1+2+4+..+16 seconds.
And there isn't any "Unable to connect to plugin" warnings in the log. I hope you reconsider this.

If the http server never reply, the c.http.Do(req) statement will continue to block forever and it won't return an error.

The full stack-trace helps. You are correct. The application specific backoff/retry logic is for errors that are returned from the http request (eg, connection refused, no socket available to connect, etc). If the call is blocked, then we will not timeout.

Reopening, since this PR fixes a valid issue.

LGTM

@chenchun I know it was there before your PR, but if we could add to the timeout constants a * time.Second that'd be appreciated. It will need some type conversions modifications in the code I believe. If it's too complex then forget it. Otherwise PR looks good.