s3: use pooled transport for http client

[vishalnayak]

No description provided.

[jefferai]

I'm a little hesitant to merge this right before a release if it doesn't solve the original problem that was reported; on the other hand, we have seen issues with the aws sdk in the past and I can't think of why this would cause a problem. Maybe let's wait to hear back?

[vishalnayak]

@jefferai That sounds reasonable. There is absolutely no rush on this.

[briankassouf]

@vishalnayak If we decide to make this change the MaxIdleConnsPerHost variable will need to be upped. It should match how we do it in consul https://github.com/hashicorp/vault/blob/master/physical/consul.go#L164

[vishalnayak]

@briankassouf Ah, yes! I'll do that. We need the parallel loading of leases to work properly :-)

[deverton]

Just curious if this is a perf issue with parallel loads from S3? We've noticed some very slow starts (~50 minutes) with S3 as a storage backend and had started to wonder if there was something broken with the S3 client. #2466 was the start of trying to normalise the S3 backend with other backends for example.

[jefferai]

Nope -- this doesn't have anything to do with performance and only has to do with ensuring the client is using an isolated Go http client for reasons (see the readme at https://github.com/hashicorp/go-cleanhttp ).

Long startup times are usually due to very high numbers of leases. In 0.6.5+ you can see in the log (might need debug or trace levels, I forget) how many total leases need to be loaded, and it will also display some running progress updates. If your data storage and/or Vault are on AWS, this can be exacerbated by running out of provisioned IOPS even if the storage layer itself is capable of providing leases much more quickly.

If you're having long startup periods due to high numbers of leases, please post on the mailing list and we'll help you figure out how to avoid generating such high numbers. If that's not what's going on, please file a separate issue about it and we can investigate.

[jefferai]

Quick ping to @vishalnayak , let's merge this in once the max idle conns value is updated.

[briankassouf]

Looks great!