Use cleanhttp.DefaultPooledClient() by default

[mitar]

Which version of Elastic are you using?

[x] elastic.v7 (for Elasticsearch 7.x)
[ ] elastic.v6 (for Elasticsearch 6.x)
[ ] elastic.v5 (for Elasticsearch 5.x)
[ ] elastic.v3 (for Elasticsearch 2.x)
[ ] elastic.v2 (for Elasticsearch 1.x)

I have been doing many .Get requests in a loop (10k per second or so) and I noticed that soon I get dial tcp 172.17.0.2:9200: connect: cannot assign requested address error. After debugging and trying different things (fixing SetURL to a fixed value, disabling SetSniff) I realized that at one point idle connections just start growing and growing (I used nestat and grep :9200 in there), to 30k or so, after which the program crashed with the error above.

I tried different things, e.g., adding another defer to PerformRequest, because body has to be both fully consumed and closed for it to be returned to the pool (I suspected that when LimitReader is used, body is not fully read):

			defer res.Body.Close()
			defer io.Copy(ioutil.Discard, res.Body)

I also read suggestions from Docker wiki page, but in my case it does not apply, my program can access ES at both 127.0.0.1:9200 and 172.17.0.2:9200. It does happen that it starts with 127.0.0.1:9200 and then sniffing changes it to 172.17.0.2:9200, which makes it have bunch of idle connections, which can make the issue reported here happen faster (this might be fixed with #1507, I haven't tested it), but fundamentally it is not the reason it collects 30k idle connections, only factor 2x for a bit.

But it did not help. What it did solve the issue at the end was using cleanhttp.DefaultPooledClient as default client:

	client, err := elastic.NewClient(
		elastic.SetHttpClient(cleanhttp.DefaultPooledClient()),
	)

I suspect it addresses the issue because it has larger MaxIdleConnsPerHost value.

So maybe this should be the default instead of http.DefaultClient?

[olivere]

If this solved the problem for you, that’s a good default. To be honest, we’re using this library as-is in a number of significantly big project, and I’ve never seen an issue with it. Your traffic must be significantly different to ours. But then again, using a nicely configured, custom HTTP client is good.

[mitar]

I am surprised that my pattern would be special in any way:

• I run ES on same machine inside Docker.
• I run a Go script which uses scroll to go over all documents in an index.
• For each document it makes 100s additional get requests towards same index (not batched, one by one).
• It writes (using bulk processor) the processed document back to the same index.

That fetching of 100s of additional requests without batching seems to produce that many idle connections.

BTW, from same people that made cleanhttp client, there is also retryablehttp client. I have not yet tested it, but it might be a better API to not have retrying code inside this package but simply leave it to the caller to specify a HTTP client which knows how to retry. I am mentioning this because when I was looking at the code, I was worried a bit that if there is any retrying, res.Body might not be closed properly, because defer is called only after the block about retrying.

[olivere]

As a library user, I would expect the client uses the standard HTTP client of the Go standard library by default. Anything else would be confusing to me. I agree that you would want to configure a custom HTTP client for your production workloads. But then I would do that explicitly, not implicitly. Also, it's yet another dependency, which I really try to avoid these days.