High CPU usage as a result of constant to CredHub client construction

[vito]

A number of users have reported high CPU usage with v3.14.1. This seems to be coming from the lazy CredHub client construction, which was introduced to fix #2154 but seems to be unintentionally creating a CredHub client repeatedly rather than just once (on first use).

I think this is happening because this doesn't return a pointer: https://github.com/concourse/atc/blob/master/creds/credhub/manager.go#L98 ... and so the client is always assigned on a copy.

We should also make sure this is safe for concurrent use. I believe the current code won't be happy with -race once we make it a pointer. This may be a good time to use *sync.Once: https://golang.org/pkg/sync/#Once

• Concourse version: v3.14.1
• Deployment type (BOSH/Docker/binary): any that use CredHub
• Infrastructure/IaaS: any
• Browser (if applicable): n/a
• Did this used to work? yes

[jmcarp]

😱

[engrun]

We have stumbled upon this issue (see discuss)
Does anyone have an estimated date for a fix?

[jama22]

Update for ya'll, our fix moved through the pipeline so it should be coming in the next release

[iferunsewe]

We don't think the proposed fix works after having investigated with a profiler. More details are in this PR: vmware-archive/atc#291

[heycait]

We (Pivotal RelEng team) deploy with a nightly build containing the initial fix here. But our ATC VMs are still working pretty hard (~30% CPU across 8 cores and 4 ATC VMs) and we're frequently seeing "too many open files" so it may be that the issue isn't completely fixed in the two commits linked above.

[vito]

Yep, we've merged vmware-archive/atc#291 now which should actually fix it. Thanks again @iferunsewe!