Allow tuning net/http MaxIdleConnsPerHost for high throughput about sdk-go HOT 8 CLOSED

The section "Repro #2: Create excessive TIME_WAIT connections by exceeding connection pool" at http://tleyden.github.io/blog/2016/11/21/tuning-the-go-http-client-library-for-load-testing/ has a nice description of what's happening here as well. And, it hints that we may need to allow tuning both MaxIdleConns and MaxIdleConnsPerHost. Or, if the Knative use-case of sending lots of CloudEvents to a single host is too specific, we can tune it there instead of here. It just requires some internal knowledge of how cloudevents sdk-go works to tune that in Knative.

from sdk-go.

I think the use-case of knative is good, it's still generic enough, and useful for a general "from x to cloudEvent" bridge. that may invoke (public) cloud services, distributing CEs, from internal events (wrapped in CEs)

from sdk-go.

The solution for this would be to set these settings on the http client in the cloudevents http transport object, like:

			t, err := cloudevents.NewHTTPTransport(
				cloudevents.WithTarget(env.Target),
				cloudevents.WithEncoding(encoding),
			)
			t.Client = myHttpClient

from sdk-go.

Thanks - I'll give that a try and close this issue if that works as expected.

from sdk-go.

So, with release 0.4.0 I was able to get acceptable throughput here by tuning the DefaultTransport to raise MaxIdleConnsPerHost. The ability to set a Client (and have that Client actually used for requests) appears to have landed around 0.4.2. However, upgrading to that or newer, I can never get acceptable throughput here.

The issue is

Explicitly telling the client request to close like that means the client is forced to constantly tear down and create new connections, thus leading to resource exhaustion under even moderate load when sending all CloudEvents to a single host. From the net/http docs:

        // Close indicates whether to close the connection after
        // replying to this request (for servers) or after sending this
        // request and reading its response (for clients).
        //
        // For server requests, the HTTP server handles this automatically
        // and this field is not needed by Handlers.
        //
        // For client requests, setting this field prevents re-use of
        // TCP connections between requests to the same hosts, as if
        // Transport.DisableKeepAlives were set.

So, we'll need to track down why we started explicitly asking client requests to close. I'm working on an automated test for this in Knative, but it's more of an integration test since it needs to spin up a webserver and run a sustained throughput against it for at least 30 seconds or more.

from sdk-go.

from sdk-go.

Perhaps the issue is on host vs multi-host targets? you want to close for multi, and leave open for a single target?

from sdk-go.

Fixed via #110

from sdk-go.